Add textureBindingViewDimension and compat limits#580
Add textureBindingViewDimension and compat limits#580beaufortfrancois wants to merge 2 commits intowebgpu-native:mainfrom
Conversation
|
@kainino0x Please review |
6d960cb to
4a25f7f
Compare
4a25f7f to
6fb8407
Compare
webgpu.yml
Outdated
| TODO | ||
| type: bool | ||
| default: false | ||
| - name: texture_binding_view_dimension_descriptor |
There was a problem hiding this comment.
nit: I don't remember the naming convention 100% but we might not need "descrpitor" here.
There was a problem hiding this comment.
+1, removing "descriptor" would be most consistent with other names. It would require a little work to rename in Dawn but shouldn't be too much.
There was a problem hiding this comment.
Actually... extensions also are supposed to have prefixes. Compat has a reserved enum block, but doesn't have a name prefix defined yet:
https://github.com/webgpu-native/webgpu-headers/blob/main/doc/articles/Extensions.md#registry-of-prefixes-and-enum-blocks
We need to pick one, I guess CompatibilityMode?
There was a problem hiding this comment.
Maybe since compatibility mode is special (and now that we know there's only one struct) we should just do without a prefix.
webgpu.yml
Outdated
| TODO | ||
| type: bool | ||
| default: false | ||
| - name: texture_binding_view_dimension_descriptor |
There was a problem hiding this comment.
+1, removing "descriptor" would be most consistent with other names. It would require a little work to rename in Dawn but shouldn't be too much.
| #define WGPU_COMPATIBILITY_MODE_LIMITS_INIT _wgpu_MAKE_INIT_STRUCT(WGPUCompatibilityModeLimits, { \ | ||
| /*.chain=*/_wgpu_MAKE_INIT_STRUCT(WGPUChainedStruct, { \ | ||
| /*.next=*/NULL _wgpu_COMMA \ | ||
| /*.sType=*/WGPUSType_CompatibilityModeLimits _wgpu_COMMA \ |
There was a problem hiding this comment.
Need to add this SType to the yml. However, in #547, we never resolved if this would go in the compat block or the core block. (I was reminded about this while searching for open bugs).
Considering the way they work in the WebGPU spec, it seems it should be in the core block (i.e. option 2), so please do that for this PR.
There was a problem hiding this comment.
(And note in the documentation why it's in the core block and not in the compat block.)
There was a problem hiding this comment.
Officially will be resolved by #582, so this is blocked on that.
| #define WGPU_TEXTURE_BINDING_VIEW_DIMENSION_DESCRIPTOR_INIT _wgpu_MAKE_INIT_STRUCT(WGPUTextureBindingViewDimensionDescriptor, { \ | ||
| /*.chain=*/_wgpu_MAKE_INIT_STRUCT(WGPUChainedStruct, { \ | ||
| /*.next=*/NULL _wgpu_COMMA \ | ||
| /*.sType=*/WGPUSType_TextureBindingViewDimensionDescriptor _wgpu_COMMA \ |
There was a problem hiding this comment.
Need to add this SType to the yml in the Compat block.
There was a problem hiding this comment.
Oh also, add both new INIT macros to tests/compile/main.inl, that would catch this error.
(ugh, we really should autogenerate that, I had completely forgotten about it)
There was a problem hiding this comment.
Hmmmm I just remembered, I don't think the yml has a way to add things to the compat block, because the enum_prefix is global. We'll have to do something about that...
There was a problem hiding this comment.
Started a fix in #583, this is blocked on that.
Fixes #470