Complete parity with wgpu 30.0, part 1 - #617
Conversation
16661df to
d8a079a
Compare
Vipitis
left a comment
Vipitis
left a comment
There was a problem hiding this comment.
started reading the diff, some notes... will take me a few more days to get through it and then trying some parts of it too.
| WGPUSType_PrimitiveStateExtras = 0x0003000A, | ||
| /** Identifies @ref WGPUSamplerDescriptorExtras. */ | ||
| WGPUSType_SamplerDescriptorExtras = 0x0003000B, | ||
| /** Identifies @ref WGPUComputePipelineDescriptorExtras. */ |
There was a problem hiding this comment.
enum skips 0x0003000C and 0x00030011
| * This is a native only feature. | ||
| */ | ||
| WGPUNativeFeature_StorageTextureArrayNonUniformIndexing = 0x00030010, | ||
| /** |
There was a problem hiding this comment.
looks like it's the same upstream, but you need to enable a featured called AddressMode_ClampToZero (and also AddeessMode_ClampToBorder?) to now use the BorderColor extra with color zero?
I guess clamp to zero existed before and then other border colors got added so it got generalized away.
If that's the only way to use it, perhaps a docstring (or reference to example/test) could help.
E: looks like it was the other way around: gfx-rs/wgpu#2364 still odd to me and something I want to try, even if unrelated to this PR.
| // WGPUNativeFeature_MeshShaderMultiview = 0x00030034, | ||
| // WGPUNativeFeature_ExtendedAccelerationStructureVertexFormats = 0x00030035, | ||
| // WGPUNativeFeature_PassthroughShaders = 0x00030036, | ||
| WGPUNativeFeature_MeshShader = 0x00030032, |
There was a problem hiding this comment.
no docstring on the first one
| * | ||
| * Assignable to any @ref WGPULoadOp field (e.g. @ref WGPURenderPassColorAttachment::loadOp). | ||
| */ | ||
| #define WGPULoadOp_DontCare 0x00030001 |
There was a problem hiding this comment.
I don't believe these values should be done via a macro. It should likely be a WGPUNativeLoadOp
similar with WGPUTextureAspect further down
| * The maximum number of views that can be used in multiview rendering. | ||
| */ | ||
| uint32_t maxMultiviewViewCount; | ||
| /** |
There was a problem hiding this comment.
if these are limits, why are they InstanceExtras instead of NativeLimits?
| * | ||
| * This is an experimental, native-only feature. | ||
| */ | ||
| WGPUNativeFeature_ExperimentalRayTracingPipelines = 0x00030043, |
There was a problem hiding this comment.
naming of Experimental*, as well as the docstring is not consistent. I think it's fine to name all experimental features experimental - as you also need to enable that for the adapter.
| typedef struct WGPUDisplayChromaticity | ||
| { | ||
| WGPUBool present; | ||
| float redX, redY; |
There was a problem hiding this comment.
I would prefer if they were one per line.
|
I will look at these comments and I appreciate them. But I feel a little guilty because I have barely glanced at the actual header changes, you have done more in that regard. |
| * The other pair should be NULL/0. | ||
| * Pass to @ref wgpuDeviceCreateBlas. | ||
| */ | ||
| typedef struct WGPUBlasSizeDescriptors |
There was a problem hiding this comment.
I was curious on how this kind of union could be solved. Would it make sense to combined the sizes? Also do we need the nullable prefix? Similar example from webgpu.h is GPUBindGroupEntry
| * | ||
| * Bit values match those of @c wgpu_types::TextureFormatFeatureFlags. | ||
| */ | ||
| typedef uint32_t WGPUNativeTextureFormatFeatureFlags; |
There was a problem hiding this comment.
There is a 64bit flag type, probably best to use that.
| */ | ||
| WGPUBlas wgpuQueueCompactBlas(WGPUQueue queue, WGPUBlas blas); | ||
|
|
||
| /** Query internal wgpu-core/HAL resource counters for debugging. */ |
There was a problem hiding this comment.
The struct don't include core counters, but it also looks like that is cute and empty placeholder?
3 participants
Checklist
cargo clippyreports no issuescargo docreports no issuescargo denyissues have been fixed or added todeny.tomlAdded new functionality. @githubnameDescription
Broken off from #594. See discussion there.
This brings basically all functionality from wpgu into wgpu-native, and has a few fixes for broken existing functionality. It was tested but that won't be part of this PR and will come later.
Related Issues