-
Notifications
You must be signed in to change notification settings - Fork 340
Allow GPUTexture where GPUTextureView is used #5228
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
Allow GPUTexture where GPUTextureView is used #5228
Conversation
Previews, as seen when this build job started (b3898a4): |
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.
I missed last weeks meeting so I don't know if this was agreed upon behavior. Doesn't seem like the linked issue reached a resolution, at least.
Assuming that the group has agreed on this, though, the PR looks good. Just a couple of nits.
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.
@toji Thanks for the review! My understanding from past meeting notes is that it had reached consensus.
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 the updates! WebGPU spec LGTM, but would you mind getting someone from the WGSL side (like @dneto0) to approve the WGSL spec changes?
@dneto0 Please review |
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.
Kai's edits LGTM.
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. Thanks!
@kainino0x Shall we update webgpu.h as well or is it only for Javascript? |
@beaufortfrancois no, we'll do this only for Javascript. It's just an implicit |
I'm happy to do the Blink change unless this is something you wanted to tackle, @beaufortfrancois? |
I already had one ;) Feel free to review https://chromium-review.googlesource.com/c/chromium/src/+/6656722 |
This PR reflects the following spec changes: - gpuweb/gpuweb#5228
Spec PR: gpuweb/gpuweb#5228 Change-Id: I260164692b6476bb298ec8ad23bf8d1696d68778 Bug: 425906323 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6656722 Commit-Queue: Fr <[email protected]> Reviewed-by: Brandon Jones <[email protected]> Cr-Commit-Position: refs/heads/main@{#1484331}
…`s are allowed r=#webgpu-reviewers! gpuweb/cts#4407 gpuweb/gpuweb#5228
…`s are allowed r=#webgpu-reviewers! Changed in spec. upstream by [gpuweb#5228](gpuweb/gpuweb#5228), tested by CTS in [gpuweb/cts#4407](gpuweb/cts#4407) (which is what caused the `FAIL`s removed here to be introduced with bug 1976874). Differential Revision: https://phabricator.services.mozilla.com/D257054
…`s are allowed r=#webgpu-reviewers! Changed in spec. upstream by [gpuweb#5228](gpuweb/gpuweb#5228), tested by CTS in [gpuweb/cts#4407](gpuweb/cts#4407) (which is what caused the `FAIL`s removed here to be introduced with bug 1976874). Differential Revision: https://phabricator.services.mozilla.com/D257054
…`s are allowed r=#webgpu-reviewers! Changed in spec. upstream by [gpuweb#5228](gpuweb/gpuweb#5228), tested by CTS in [gpuweb/cts#4407](gpuweb/cts#4407) (which is what caused the `FAIL`s removed here to be introduced with bug 1976874). Differential Revision: https://phabricator.services.mozilla.com/D257054
This comment was marked as spam.
This comment was marked as spam.
…`s are allowed r=#webgpu-reviewers! Changed in spec. upstream by [gpuweb#5228](gpuweb/gpuweb#5228), tested by CTS in [gpuweb/cts#4407](gpuweb/cts#4407) (which is what caused the `FAIL`s removed here to be introduced with bug 1976874). Differential Revision: https://phabricator.services.mozilla.com/D257054
…`s are allowed r=#webgpu-reviewers! Changed in spec. upstream by [gpuweb#5228](gpuweb/gpuweb#5228), tested by CTS in [gpuweb/cts#4407](gpuweb/cts#4407) (which is what caused the `FAIL`s removed here to be introduced with bug 1976874). Differential Revision: https://phabricator.services.mozilla.com/D257054
…`s are allowed r=webgpu-reviewers,webidl,smaug,nical Changed in spec. upstream by [gpuweb#5228](gpuweb/gpuweb#5228), tested by CTS in [gpuweb/cts#4407](gpuweb/cts#4407) (which is what caused the `FAIL`s removed here to be introduced with bug 1976874). Differential Revision: https://phabricator.services.mozilla.com/D257054
…`s are allowed r=webgpu-reviewers,webidl,smaug,nical Changed in spec. upstream by [gpuweb#5228](gpuweb/gpuweb#5228), tested by CTS in [gpuweb/cts#4407](gpuweb/cts#4407) (which is what caused the `FAIL`s removed here to be introduced with bug 1976874). Differential Revision: https://phabricator.services.mozilla.com/D257054
gpuweb/gpuweb#5228 allows using GPUTexture instead of GPUTextureView in a number of places.
No functional changes since the object types are not strongly checked in the Wasm -> JS bridge.
This PR adds GPUTexture support to the following attributes:
It also updates parts of the WebGPU and WGSL specs that I've missed when adding GPUBuffer in GPUBindingResource with #5193
FIX #5215