-
Notifications
You must be signed in to change notification settings - Fork 195
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support 16-bit unorm/snorm formats #2210
Conversation
This looks interesting, but before I review in detail: We can't unconditionally extend WGSL, since that affects Firefox's implementation of the WebGPU standard, and we'd introduce incompatibilities between browsers. This probably needs to be conditional on some new flag in |
Looks like #2162 lays out what the capability should be. I'll see about adding suitable validation checks when I have a chance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but could you also add them to the SPV front-end?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The WGSL changes need to be conditional, as noted in the conversation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@teoxoy points out that #2161 covers making the non-WebGPU formats we already support in the WGSL front end conditional, and that's in milestone 4. So I think this is fine to merge as it is.
Sorry, I approved it without reading the comments properly. I thought gfx-rs/wgpu#4412 would cover this and could be done in a separate PR since there are a bunch of other formats. But ideally this PR should close #2162 (we don't yet have the framework for naga specific extensions but we can move that point from the issue into gfx-rs/wgpu#4410). @fintelia adding the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for introducing the new capability!
Could you also add the new formats to the spv front-end?
This PR adds handling for 16-bit unorm and snorm formats.
After making a few tweaks to wgpu, I was able to use this patch to load and run several GLSL compute shaders operating on r16 unorm textures. I haven't tested the other frontends/backends but the code I added mirrors the existing cases, so there's a good chance they should behave properly