Add texture usage flags to CanvasWebGPUAccessOptions. [chromium/src : main]
Corentin Wallez (Gerrit)
Corentin Wallez added 2 comments![]( "Open in Gerrit")
We can't directly query a texture's usage flags, so we test byYou can, with `myTexture.usage` but in this case we want CopySrc to not be allowed for JS.
access_options->usage() | WGPUTextureUsage_CopySrc);This will make the CopySrc usage visible to Javascript even if it didn't request it. Dawn has a mechanism call "internal usages" that allows adding usages without making them visible to JS. See https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/modules/webgpu/gpu_canvas_context.cc;drc=8b6408f0204513cc5e1ce5a0bd71ecc8f2f92529;l=449 and https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/modules/webgpu/gpu_canvas_context.cc;drc=8b6408f0204513cc5e1ce5a0bd71ecc8f2f92529;l=702 (I'm not sure off-hand which function is used to copy the texture back to the canvas (it might require a different usage depending on the command used to copy)).
See https://dawn.googlesource.com/dawn/+/refs/heads/main/docs/dawn/features/dawn_internal_usages.md
We should add a small validation test that CpySrc is not visible unless it is requested, and that texture.format is exactly what was passed to the canvas, even if it contains garbage.
Related details
- Aaron Krajeski
- John Stiles
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
John Stiles (Gerrit)
John Stiles added 1 comment![]( "Open in Gerrit")
access_options->usage() | WGPUTextureUsage_CopySrc);This will make the CopySrc usage visible to Javascript even if it didn't request it. Dawn has a mechanism call "internal usages" that allows adding usages without making them visible to JS. See https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/modules/webgpu/gpu_canvas_context.cc;drc=8b6408f0204513cc5e1ce5a0bd71ecc8f2f92529;l=449 and https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/modules/webgpu/gpu_canvas_context.cc;drc=8b6408f0204513cc5e1ce5a0bd71ecc8f2f92529;l=702 (I'm not sure off-hand which function is used to copy the texture back to the canvas (it might require a different usage depending on the command used to copy)).
See https://dawn.googlesource.com/dawn/+/refs/heads/main/docs/dawn/features/dawn_internal_usages.md
We should add a small validation test that CpySrc is not visible unless it is requested, and that texture.format is exactly what was passed to the canvas, even if it contains garbage.
Cool! Thanks for all the doc links.
This seems like the right thing to do, but it will require some plumbing since `WebGPUMailboxTexture` doesn't currently seem to have any mechanism for internal usages. I'll look into plumbing that through, probably in a CL that lands first, and then this CL can build on top of that.
Related details
- Aaron Krajeski
- Corentin Wallez
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
John Stiles (Gerrit)
John Stiles added 1 comment![]( "Open in Gerrit")
We can't directly query a texture's usage flags, so we test byYou can, with `myTexture.usage` but in this case we want CopySrc to not be allowed for JS.
Oops, you're absolutely right, I could have just checked the usage bits and called it a day. I'll update the test to also verify the usage bits directly.
John Stiles (Gerrit)
John Stiles added 2 comments![]( "Open in Gerrit")
We can't directly query a texture's usage flags, so we test byJohn StilesYou can, with `myTexture.usage` but in this case we want CopySrc to not be allowed for JS.
Oops, you're absolutely right, I could have just checked the usage bits and called it a day. I'll update the test to also verify the usage bits directly.
Done
access_options->usage() | WGPUTextureUsage_CopySrc);John StilesThis will make the CopySrc usage visible to Javascript even if it didn't request it. Dawn has a mechanism call "internal usages" that allows adding usages without making them visible to JS. See https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/modules/webgpu/gpu_canvas_context.cc;drc=8b6408f0204513cc5e1ce5a0bd71ecc8f2f92529;l=449 and https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/modules/webgpu/gpu_canvas_context.cc;drc=8b6408f0204513cc5e1ce5a0bd71ecc8f2f92529;l=702 (I'm not sure off-hand which function is used to copy the texture back to the canvas (it might require a different usage depending on the command used to copy)).
See https://dawn.googlesource.com/dawn/+/refs/heads/main/docs/dawn/features/dawn_internal_usages.md
We should add a small validation test that CpySrc is not visible unless it is requested, and that texture.format is exactly what was passed to the canvas, even if it contains garbage.
Cool! Thanks for all the doc links.
This seems like the right thing to do, but it will require some plumbing since `WebGPUMailboxTexture` doesn't currently seem to have any mechanism for internal usages. I'll look into plumbing that through, probably in a CL that lands first, and then this CL can build on top of that.
Done
Related details
- Aaron Krajeski
- Corentin Wallez
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Corentin Wallez (Gerrit)
Corentin Wallez voted and added 5 comments![]( "Open in Gerrit")
Votes added by Corentin Wallez
| Code-Review | +1 |
5 comments
constexpr int kSupportedUsageFlags =Why wouldn't storage be supported?
if (tex_usage == 0 || (tex_usage & ~kSupportedUsageFlags)) {Tex usage being 0 seems like the user's problem ^^
const testOneUsageFlag = function(adapterInfo, device, canvas, usageFlags) {nit: unused
// action requiring that usage, and verifying no GPUValidationError occurs.We could also test that the usages that are not in the usageFlags produce a validation error! That's what the internalUsage stuff in Dawn is meant to do.
Related details
- Aaron Krajeski
- John Stiles
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
John Stiles (Gerrit)
John Stiles added 3 comments![]( "Open in Gerrit")
constexpr int kSupportedUsageFlags =Why wouldn't storage be supported?
Philosophically, the canvas texture is meant for display to the user; it's not storage.
From a technical perspective, there are lots of weird restrictions that would be hard to untangle. The biggest restriction is that read_write is only allowed on single-channel-red textures, which will never be the canvas format. I think it's much cleaner to just disallow.
From https://webgpufundamentals.org/webgpu/lessons/webgpu-storage-textures.html :
That said, there are limits on storage textures.
Only certain formats can be read_write.
Those are r32float, r32sint, and r32uint.
Other supported formats can only be read or write within a single shader.
Only certain formats can be used as storage textures.
There are a large number of texture formats but only certain ones can be usage as storage textures.
rgba8(unorm/snorm/sint/uint)
rgba16(float/sint/uint)
rg32(float/sint/uint)
rgba32(float/sint/uint)
One format to notice missing is bgra8unorm which we’ll cover below.
if (tex_usage == 0 || (tex_usage & ~kSupportedUsageFlags)) {Tex usage being 0 seems like the user's problem ^^
Are you suggesting that we change this to allow it? I didn't see a good rationale for allowing the creation of a useless texture.
// action requiring that usage, and verifying no GPUValidationError occurs.We could also test that the usages that are not in the usageFlags produce a validation error! That's what the internalUsage stuff in Dawn is meant to do.
- Aaron Krajeski
- Corentin Wallez
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Corentin Wallez (Gerrit)
Corentin Wallez added 2 comments![]( "Open in Gerrit")
constexpr int kSupportedUsageFlags =John StilesWhy wouldn't storage be supported?
Philosophically, the canvas texture is meant for display to the user; it's not storage.
From a technical perspective, there are lots of weird restrictions that would be hard to untangle. The biggest restriction is that read_write is only allowed on single-channel-red textures, which will never be the canvas format. I think it's much cleaner to just disallow.
From https://webgpufundamentals.org/webgpu/lessons/webgpu-storage-textures.html :
That said, there are limits on storage textures.
Only certain formats can be read_write.
Those are r32float, r32sint, and r32uint.
Other supported formats can only be read or write within a single shader.
Only certain formats can be used as storage textures.
There are a large number of texture formats but only certain ones can be usage as storage textures.
rgba8(unorm/snorm/sint/uint)
rgba16(float/sint/uint)
rg32(float/sint/uint)
rgba32(float/sint/uint)
One format to notice missing is bgra8unorm which we’ll cover below.
Ok, that makes sense. In the future storage textures might be loosened a bit, but we can figure out what to do for beginWebGPUAccess at that point.
if (tex_usage == 0 || (tex_usage & ~kSupportedUsageFlags)) {John StilesTex usage being 0 seems like the user's problem ^^
Are you suggesting that we change this to allow it? I didn't see a good rationale for allowing the creation of a useless texture.
Yeah I'm suggesting to allow it. The user could be programmatically building a bitfield that ends up being 0 and then they would never use the texture. Gracefully doing nothing seems better than breaking the control flow by throwing an exception. That said that's something that will be bikeshed anyway when a spec is made for this API.
Related details
- Aaron Krajeski
- John Stiles
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
John Stiles (Gerrit)
John Stiles added 3 comments![]( "Open in Gerrit")
if (tex_usage == 0 || (tex_usage & ~kSupportedUsageFlags)) {John StilesTex usage being 0 seems like the user's problem ^^
Corentin WallezAre you suggesting that we change this to allow it? I didn't see a good rationale for allowing the creation of a useless texture.
Yeah I'm suggesting to allow it. The user could be programmatically building a bitfield that ends up being 0 and then they would never use the texture. Gracefully doing nothing seems better than breaking the control flow by throwing an exception. That said that's something that will be bikeshed anyway when a spec is made for this API.
Done
const testOneUsageFlag = function(adapterInfo, device, canvas, usageFlags) {John Stilesnit: unused
Done
// action requiring that usage, and verifying no GPUValidationError occurs.John StilesWe could also test that the usages that are not in the usageFlags produce a validation error! That's what the internalUsage stuff in Dawn is meant to do.
OK, I'll write a new test for this.
- Aaron Krajeski
- Corentin Wallez
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Aaron Krajeski (Gerrit)
Aaron Krajeski added 1 comment![]( "Open in Gerrit")
GPUTextureUsageFlags usage = 0x14; // TEXTURE_BINDING | RENDER_ATTACHMENTWould it be possible to do this in a way that's more directly readable, or are we stuck with this hex nonesense? What does the user interface for it look like? Are users just expected to pass in a number? Is there a doc explaining this?
Related details
- Corentin Wallez
- John Stiles
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
John Stiles (Gerrit)
John Stiles added 1 comment![]( "Open in Gerrit")
GPUTextureUsageFlags usage = 0x14; // TEXTURE_BINDING | RENDER_ATTACHMENTWould it be possible to do this in a way that's more directly readable, or are we stuck with this hex nonesense? What does the user interface for it look like? Are users just expected to pass in a number? Is there a doc explaining this?
Javascript users can pass in enums like this
```
GPUTextureUsage.COPY_SRC |
GPUTextureUsage.COPY_DST |
GPUTextureUsage.TEXTURE_BINDING |
GPUTextureUsage.RENDER_ATTACHMENT
```
I don't think that works in an IDL file though. Existing cases in IDL all look like this, so I followed their lead: https://source.chromium.org/chromium/chromium/src/+/main:third_party/devtools-frontend/src/node_modules/@webref/idl/webgpu.idl;l=1130;drc=6e09692deb19474d035f0e117b11ed9bab51297f
Related details
- Aaron Krajeski
- Corentin Wallez
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
John Stiles (Gerrit)
John Stiles added 1 comment![]( "Open in Gerrit")
webgpu_access_texture_ = MakeGarbageCollected<GPUTexture>(^
John Stiles (Gerrit)
John Stiles added 1 comment![]( "Open in Gerrit")
webgpu_access_texture_ = MakeGarbageCollected<GPUTexture>(John Stiles^
Acknowledged
Aaron Krajeski (Gerrit)
Aaron Krajeski added 1 comment![]( "Open in Gerrit")
GPUTextureUsageFlags usage = 0x14; // TEXTURE_BINDING | RENDER_ATTACHMENTJohn StilesWould it be possible to do this in a way that's more directly readable, or are we stuck with this hex nonesense? What does the user interface for it look like? Are users just expected to pass in a number? Is there a doc explaining this?
Javascript users can pass in enums like this
```
GPUTextureUsage.COPY_SRC |
GPUTextureUsage.COPY_DST |
GPUTextureUsage.TEXTURE_BINDING |
GPUTextureUsage.RENDER_ATTACHMENT
```I don't think that works in an IDL file though. Existing cases in IDL all look like this, so I followed their lead: https://source.chromium.org/chromium/chromium/src/+/main:third_party/devtools-frontend/src/node_modules/@webref/idl/webgpu.idl;l=1130;drc=6e09692deb19474d035f0e117b11ed9bab51297f
Are there other javascript interfaces that use bitwise operations like this? I assume this interface hasn't been specced yet?
Anyhow, LGTM, though I imagine at the spec stage people might get mad at the interface. Not sure what the alternative would be though. Maybe a dict of booleans.
Related details
- Corentin Wallez
- John Stiles
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Aaron Krajeski (Gerrit)
Aaron Krajeski voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
- Corentin Wallez
- John Stiles
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
John Stiles (Gerrit)
John Stiles added 1 comment![]( "Open in Gerrit")
GPUTextureUsageFlags usage = 0x14; // TEXTURE_BINDING | RENDER_ATTACHMENTJohn StilesWould it be possible to do this in a way that's more directly readable, or are we stuck with this hex nonesense? What does the user interface for it look like? Are users just expected to pass in a number? Is there a doc explaining this?
Aaron KrajeskiJavascript users can pass in enums like this
```
GPUTextureUsage.COPY_SRC |
GPUTextureUsage.COPY_DST |
GPUTextureUsage.TEXTURE_BINDING |
GPUTextureUsage.RENDER_ATTACHMENT
```I don't think that works in an IDL file though. Existing cases in IDL all look like this, so I followed their lead: https://source.chromium.org/chromium/chromium/src/+/main:third_party/devtools-frontend/src/node_modules/@webref/idl/webgpu.idl;l=1130;drc=6e09692deb19474d035f0e117b11ed9bab51297f
Are there other javascript interfaces that use bitwise operations like this? I assume this interface hasn't been specced yet?
Anyhow, LGTM, though I imagine at the spec stage people might get mad at the interface. Not sure what the alternative would be though. Maybe a dict of booleans.
I see it in the WebGPU spec just like so... https://gpuweb.github.io/gpuweb/#dom-gpucanvasconfiguration-usage
Related details
- Aaron Krajeski
- Corentin Wallez
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
John Stiles (Gerrit)
John Stiles added 1 comment![]( "Open in Gerrit")
access_options->usage() | WGPUTextureUsage_CopySrc);John StilesThis will make the CopySrc usage visible to Javascript even if it didn't request it. Dawn has a mechanism call "internal usages" that allows adding usages without making them visible to JS. See https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/modules/webgpu/gpu_canvas_context.cc;drc=8b6408f0204513cc5e1ce5a0bd71ecc8f2f92529;l=449 and https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/modules/webgpu/gpu_canvas_context.cc;drc=8b6408f0204513cc5e1ce5a0bd71ecc8f2f92529;l=702 (I'm not sure off-hand which function is used to copy the texture back to the canvas (it might require a different usage depending on the command used to copy)).
See https://dawn.googlesource.com/dawn/+/refs/heads/main/docs/dawn/features/dawn_internal_usages.md
We should add a small validation test that CpySrc is not visible unless it is requested, and that texture.format is exactly what was passed to the canvas, even if it contains garbage.
John StilesCool! Thanks for all the doc links.
This seems like the right thing to do, but it will require some plumbing since `WebGPUMailboxTexture` doesn't currently seem to have any mechanism for internal usages. I'll look into plumbing that through, probably in a CL that lands first, and then this CL can build on top of that.
Done
After talking it through with Austin, looks like we don't actually need internal usage. Our internal needs (copying back and forth from/to the IOSurface) use RasterInterface, not WebGPU, so nothing is going to be looking for a CopySrc usage bit when this is done.
John Stiles (Gerrit)
John Stiles added 1 comment![]( "Open in Gerrit")
GPUTextureUsageFlags usage = 0x14; // TEXTURE_BINDING | RENDER_ATTACHMENTJohn StilesWould it be possible to do this in a way that's more directly readable, or are we stuck with this hex nonesense? What does the user interface for it look like? Are users just expected to pass in a number? Is there a doc explaining this?
Aaron KrajeskiJavascript users can pass in enums like this
```
GPUTextureUsage.COPY_SRC |
GPUTextureUsage.COPY_DST |
GPUTextureUsage.TEXTURE_BINDING |
GPUTextureUsage.RENDER_ATTACHMENT
```I don't think that works in an IDL file though. Existing cases in IDL all look like this, so I followed their lead: https://source.chromium.org/chromium/chromium/src/+/main:third_party/devtools-frontend/src/node_modules/@webref/idl/webgpu.idl;l=1130;drc=6e09692deb19474d035f0e117b11ed9bab51297f
John StilesAre there other javascript interfaces that use bitwise operations like this? I assume this interface hasn't been specced yet?
Anyhow, LGTM, though I imagine at the spec stage people might get mad at the interface. Not sure what the alternative would be though. Maybe a dict of booleans.
I see it in the WebGPU spec just like so... https://gpuweb.github.io/gpuweb/#dom-gpucanvasconfiguration-usage
Acknowledged
Related details
- Aaron Krajeski
- Corentin Wallez
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
John Stiles (Gerrit)
John Stiles voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
12 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: third_party/blink/renderer/modules/canvas/canvas2d/base_rendering_context_2d.cc
Insertions: 2, Deletions: 5.
The diff is too large to show. Please review the diff.
```
Change information
Add texture usage flags to CanvasWebGPUAccessOptions.
Setting the flags is pretty simple. We disallow storage-texture
bindings, since storage texture formats are unlikely to be
compatible with renderable texture formats. (See comment chain at
https://chromium-review.googlesource.com/c/chromium/src/+/5448084/comment/6217d152_fa8bc533/ )
Tests verify both that the requested flags were set, _and_ that the
flags actually work. We check that proper usage flags allow drawing,
and incorrect flags lead to a GPUValidationError.