Add texture usage flags to CanvasWebGPUAccessOptions. [chromium/src : main]

39 views
Skip to first unread message

Corentin Wallez (Gerrit)

unread,
Apr 23, 2024, 4:57:18 AMApr 23
to John Stiles, Aaron Krajeski, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, jmedle...@chromium.org
Attention needed from Aaron Krajeski and John Stiles

Corentin Wallez added 2 comments

Commit Message
Line 12, Patchset 7 (Latest):We can't directly query a texture's usage flags, so we test by
Corentin Wallez . unresolved

You can, with `myTexture.usage` but in this case we want CopySrc to not be allowed for JS.

File third_party/blink/renderer/modules/canvas/canvas2d/base_rendering_context_2d.cc
Line 3475, Patchset 7 (Latest): access_options->usage() | WGPUTextureUsage_CopySrc);
Corentin Wallez . unresolved

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.

Open in Gerrit

Related details

Attention is currently required from:
  • Aaron Krajeski
  • John Stiles
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I6bb8e5e1fe336dd8baa770d022414ae9b129e876
Gerrit-Change-Number: 5448084
Gerrit-PatchSet: 7
Gerrit-Owner: John Stiles <johns...@google.com>
Gerrit-Reviewer: Aaron Krajeski <aar...@chromium.org>
Gerrit-Reviewer: Corentin Wallez <cwa...@chromium.org>
Gerrit-Reviewer: John Stiles <johns...@google.com>
Gerrit-Attention: John Stiles <johns...@google.com>
Gerrit-Attention: Aaron Krajeski <aar...@chromium.org>
Gerrit-Comment-Date: Tue, 23 Apr 2024 08:57:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

John Stiles (Gerrit)

unread,
Apr 23, 2024, 11:17:59 AMApr 23
to Corentin Wallez, Aaron Krajeski, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, jmedle...@chromium.org
Attention needed from Aaron Krajeski and Corentin Wallez

John Stiles added 1 comment

File third_party/blink/renderer/modules/canvas/canvas2d/base_rendering_context_2d.cc
Line 3475, Patchset 7 (Latest): access_options->usage() | WGPUTextureUsage_CopySrc);
Corentin Wallez . unresolved

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.

John Stiles

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.

Open in Gerrit

Related details

Attention is currently required from:
  • Aaron Krajeski
  • Corentin Wallez
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I6bb8e5e1fe336dd8baa770d022414ae9b129e876
Gerrit-Change-Number: 5448084
Gerrit-PatchSet: 7
Gerrit-Owner: John Stiles <johns...@google.com>
Gerrit-Reviewer: Aaron Krajeski <aar...@chromium.org>
Gerrit-Reviewer: Corentin Wallez <cwa...@chromium.org>
Gerrit-Reviewer: John Stiles <johns...@google.com>
Gerrit-Attention: Corentin Wallez <cwa...@chromium.org>
Gerrit-Attention: Aaron Krajeski <aar...@chromium.org>
Gerrit-Comment-Date: Tue, 23 Apr 2024 15:17:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Corentin Wallez <cwa...@chromium.org>
satisfied_requirement
unsatisfied_requirement
open
diffy

John Stiles (Gerrit)

unread,
Apr 23, 2024, 11:19:00 AMApr 23
to Corentin Wallez, Aaron Krajeski, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, jmedle...@chromium.org
Attention needed from Aaron Krajeski and Corentin Wallez

John Stiles added 1 comment

Commit Message
Line 12, Patchset 7 (Latest):We can't directly query a texture's usage flags, so we test by
Corentin Wallez . unresolved

You can, with `myTexture.usage` but in this case we want CopySrc to not be allowed for JS.

John Stiles

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.

Gerrit-Comment-Date: Tue, 23 Apr 2024 15:18:48 +0000
satisfied_requirement
unsatisfied_requirement
open
diffy

John Stiles (Gerrit)

unread,
Apr 24, 2024, 11:17:42 AMApr 24
to Corentin Wallez, Aaron Krajeski, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, jmedle...@chromium.org
Attention needed from Aaron Krajeski and Corentin Wallez

John Stiles added 2 comments

Commit Message
Line 12, Patchset 7:We can't directly query a texture's usage flags, so we test by
Corentin Wallez . resolved

You can, with `myTexture.usage` but in this case we want CopySrc to not be allowed for JS.

John Stiles

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

Done

File third_party/blink/renderer/modules/canvas/canvas2d/base_rendering_context_2d.cc
Line 3475, Patchset 7: access_options->usage() | WGPUTextureUsage_CopySrc);
Corentin Wallez . resolved

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.

John Stiles

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.

John Stiles

Done

Open in Gerrit

Related details

Attention is currently required from:
  • Aaron Krajeski
  • Corentin Wallez
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Review
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I6bb8e5e1fe336dd8baa770d022414ae9b129e876
Gerrit-Change-Number: 5448084
Gerrit-PatchSet: 9
Gerrit-Owner: John Stiles <johns...@google.com>
Gerrit-Reviewer: Aaron Krajeski <aar...@chromium.org>
Gerrit-Reviewer: Corentin Wallez <cwa...@chromium.org>
Gerrit-Reviewer: John Stiles <johns...@google.com>
Gerrit-Attention: Corentin Wallez <cwa...@chromium.org>
Gerrit-Attention: Aaron Krajeski <aar...@chromium.org>
Gerrit-Comment-Date: Wed, 24 Apr 2024 15:17:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: John Stiles <johns...@google.com>
Comment-In-Reply-To: Corentin Wallez <cwa...@chromium.org>
satisfied_requirement
unsatisfied_requirement
open
diffy

Corentin Wallez (Gerrit)

unread,
Apr 25, 2024, 9:06:53 AMApr 25
to John Stiles, Aaron Krajeski, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, jmedle...@chromium.org
Attention needed from Aaron Krajeski and John Stiles

Corentin Wallez voted and added 5 comments

Votes added by Corentin Wallez

Code-Review+1

5 comments

Patchset-level comments
File-level comment, Patchset 11 (Latest):
Corentin Wallez . resolved

LGTM

File third_party/blink/renderer/modules/canvas/canvas2d/base_rendering_context_2d.cc
Line 3455, Patchset 11 (Latest): constexpr int kSupportedUsageFlags =
Corentin Wallez . unresolved

Why wouldn't storage be supported?

Line 3460, Patchset 11 (Latest): if (tex_usage == 0 || (tex_usage & ~kSupportedUsageFlags)) {
Corentin Wallez . unresolved

Tex usage being 0 seems like the user's problem ^^

File third_party/blink/web_tests/wpt_internal/webgpu/canvas_webgpu_access/webgpu-helpers.js
Line 344, Patchset 11 (Latest): const testOneUsageFlag = function(adapterInfo, device, canvas, usageFlags) {
Corentin Wallez . unresolved

nit: unused

Line 352, Patchset 11 (Latest): // action requiring that usage, and verifying no GPUValidationError occurs.
Corentin Wallez . unresolved

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.

Open in Gerrit

Related details

Attention is currently required from:
  • Aaron Krajeski
  • John Stiles
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I6bb8e5e1fe336dd8baa770d022414ae9b129e876
Gerrit-Change-Number: 5448084
Gerrit-PatchSet: 11
Gerrit-Owner: John Stiles <johns...@google.com>
Gerrit-Reviewer: Aaron Krajeski <aar...@chromium.org>
Gerrit-Reviewer: Corentin Wallez <cwa...@chromium.org>
Gerrit-Reviewer: John Stiles <johns...@google.com>
Gerrit-Attention: John Stiles <johns...@google.com>
Gerrit-Attention: Aaron Krajeski <aar...@chromium.org>
Gerrit-Comment-Date: Thu, 25 Apr 2024 13:06:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

John Stiles (Gerrit)

unread,
Apr 25, 2024, 10:21:21 AMApr 25
to Corentin Wallez, Aaron Krajeski, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, jmedle...@chromium.org
Attention needed from Aaron Krajeski and Corentin Wallez

John Stiles added 3 comments

File third_party/blink/renderer/modules/canvas/canvas2d/base_rendering_context_2d.cc
Line 3455, Patchset 11 (Latest): constexpr int kSupportedUsageFlags =
Corentin Wallez . unresolved

Why wouldn't storage be supported?

John Stiles

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.

Line 3460, Patchset 11 (Latest): if (tex_usage == 0 || (tex_usage & ~kSupportedUsageFlags)) {
Corentin Wallez . unresolved

Tex usage being 0 seems like the user's problem ^^

John Stiles

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.

File third_party/blink/web_tests/wpt_internal/webgpu/canvas_webgpu_access/webgpu-helpers.js
Line 352, Patchset 11 (Latest): // action requiring that usage, and verifying no GPUValidationError occurs.
Corentin Wallez . unresolved

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.

John Stiles

OK, I'll write a new test for this.

Open in Gerrit

Related details

Attention is currently required from:
  • Aaron Krajeski
  • Corentin Wallez
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I6bb8e5e1fe336dd8baa770d022414ae9b129e876
Gerrit-Change-Number: 5448084
Gerrit-PatchSet: 11
Gerrit-Owner: John Stiles <johns...@google.com>
Gerrit-Reviewer: Aaron Krajeski <aar...@chromium.org>
Gerrit-Reviewer: Corentin Wallez <cwa...@chromium.org>
Gerrit-Reviewer: John Stiles <johns...@google.com>
Gerrit-Attention: Corentin Wallez <cwa...@chromium.org>
Gerrit-Attention: Aaron Krajeski <aar...@chromium.org>
Gerrit-Comment-Date: Thu, 25 Apr 2024 14:21:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Corentin Wallez <cwa...@chromium.org>
satisfied_requirement
unsatisfied_requirement
open
diffy

Corentin Wallez (Gerrit)

unread,
Apr 25, 2024, 10:28:15 AMApr 25
to John Stiles, Aaron Krajeski, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, jmedle...@chromium.org
Attention needed from Aaron Krajeski and John Stiles

Corentin Wallez added 2 comments

File third_party/blink/renderer/modules/canvas/canvas2d/base_rendering_context_2d.cc
Line 3455, Patchset 11 (Latest): constexpr int kSupportedUsageFlags =
Corentin Wallez . resolved

Why wouldn't storage be supported?

John Stiles

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.

Corentin Wallez

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.

Line 3460, Patchset 11 (Latest): if (tex_usage == 0 || (tex_usage & ~kSupportedUsageFlags)) {
Corentin Wallez . unresolved

Tex usage being 0 seems like the user's problem ^^

John Stiles

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.

Corentin Wallez

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.

Open in Gerrit

Related details

Attention is currently required from:
  • Aaron Krajeski
  • John Stiles
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I6bb8e5e1fe336dd8baa770d022414ae9b129e876
Gerrit-Change-Number: 5448084
Gerrit-PatchSet: 11
Gerrit-Owner: John Stiles <johns...@google.com>
Gerrit-Reviewer: Aaron Krajeski <aar...@chromium.org>
Gerrit-Reviewer: Corentin Wallez <cwa...@chromium.org>
Gerrit-Reviewer: John Stiles <johns...@google.com>
Gerrit-Attention: John Stiles <johns...@google.com>
Gerrit-Attention: Aaron Krajeski <aar...@chromium.org>
Gerrit-Comment-Date: Thu, 25 Apr 2024 14:28:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

John Stiles (Gerrit)

unread,
Apr 25, 2024, 11:53:52 AMApr 25
to Corentin Wallez, Aaron Krajeski, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, jmedle...@chromium.org
Attention needed from Aaron Krajeski and Corentin Wallez

John Stiles added 3 comments

File third_party/blink/renderer/modules/canvas/canvas2d/base_rendering_context_2d.cc
Line 3460, Patchset 11: if (tex_usage == 0 || (tex_usage & ~kSupportedUsageFlags)) {
Corentin Wallez . resolved

Tex usage being 0 seems like the user's problem ^^

John Stiles

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.

Corentin Wallez

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.

John Stiles

Done

File third_party/blink/web_tests/wpt_internal/webgpu/canvas_webgpu_access/webgpu-helpers.js
Line 344, Patchset 11: const testOneUsageFlag = function(adapterInfo, device, canvas, usageFlags) {
Corentin Wallez . resolved

nit: unused

John Stiles

Done

Line 352, Patchset 11: // action requiring that usage, and verifying no GPUValidationError occurs.
Corentin Wallez . resolved

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.

John Stiles

OK, I'll write a new test for this.

John Stiles

Done

Open in Gerrit

Related details

Attention is currently required from:
  • Aaron Krajeski
  • Corentin Wallez
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Review
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I6bb8e5e1fe336dd8baa770d022414ae9b129e876
Gerrit-Change-Number: 5448084
Gerrit-PatchSet: 12
Gerrit-Owner: John Stiles <johns...@google.com>
Gerrit-Reviewer: Aaron Krajeski <aar...@chromium.org>
Gerrit-Reviewer: Corentin Wallez <cwa...@chromium.org>
Gerrit-Reviewer: John Stiles <johns...@google.com>
Gerrit-Attention: Corentin Wallez <cwa...@chromium.org>
Gerrit-Attention: Aaron Krajeski <aar...@chromium.org>
Gerrit-Comment-Date: Thu, 25 Apr 2024 15:53:12 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Corentin Wallez <cwa...@chromium.org>
Comment-In-Reply-To: John Stiles <johns...@google.com>
satisfied_requirement
unsatisfied_requirement
open
diffy

Aaron Krajeski (Gerrit)

unread,
Apr 25, 2024, 1:02:03 PMApr 25
to John Stiles, Corentin Wallez, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, jmedle...@chromium.org
Attention needed from Corentin Wallez and John Stiles

Aaron Krajeski added 1 comment

File third_party/blink/renderer/modules/canvas/canvas2d/canvas_web_gpu_access_option.idl
Line 18, Patchset 12 (Latest): GPUTextureUsageFlags usage = 0x14; // TEXTURE_BINDING | RENDER_ATTACHMENT
Aaron Krajeski . unresolved

Would 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?

Open in Gerrit

Related details

Attention is currently required from:
  • Corentin Wallez
  • John Stiles
Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I6bb8e5e1fe336dd8baa770d022414ae9b129e876
    Gerrit-Change-Number: 5448084
    Gerrit-PatchSet: 12
    Gerrit-Owner: John Stiles <johns...@google.com>
    Gerrit-Reviewer: Aaron Krajeski <aar...@chromium.org>
    Gerrit-Reviewer: Corentin Wallez <cwa...@chromium.org>
    Gerrit-Reviewer: John Stiles <johns...@google.com>
    Gerrit-Attention: John Stiles <johns...@google.com>
    Gerrit-Attention: Corentin Wallez <cwa...@chromium.org>
    Gerrit-Comment-Date: Thu, 25 Apr 2024 17:01:50 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    John Stiles (Gerrit)

    unread,
    Apr 25, 2024, 1:05:22 PMApr 25
    to Corentin Wallez, Aaron Krajeski, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, jmedle...@chromium.org
    Attention needed from Aaron Krajeski and Corentin Wallez

    John Stiles added 1 comment

    File third_party/blink/renderer/modules/canvas/canvas2d/canvas_web_gpu_access_option.idl
    Line 18, Patchset 12 (Latest): GPUTextureUsageFlags usage = 0x14; // TEXTURE_BINDING | RENDER_ATTACHMENT
    Aaron Krajeski . unresolved

    Would 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?

    John Stiles

    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

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Aaron Krajeski
    • Corentin Wallez
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I6bb8e5e1fe336dd8baa770d022414ae9b129e876
    Gerrit-Change-Number: 5448084
    Gerrit-PatchSet: 12
    Gerrit-Owner: John Stiles <johns...@google.com>
    Gerrit-Reviewer: Aaron Krajeski <aar...@chromium.org>
    Gerrit-Reviewer: Corentin Wallez <cwa...@chromium.org>
    Gerrit-Reviewer: John Stiles <johns...@google.com>
    Gerrit-Attention: Corentin Wallez <cwa...@chromium.org>
    Gerrit-Attention: Aaron Krajeski <aar...@chromium.org>
    Gerrit-Comment-Date: Thu, 25 Apr 2024 17:05:10 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Aaron Krajeski <aar...@chromium.org>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    John Stiles (Gerrit)

    unread,
    Apr 25, 2024, 1:12:24 PMApr 25
    to Corentin Wallez, Aaron Krajeski, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, jmedle...@chromium.org
    Attention needed from Aaron Krajeski and Corentin Wallez

    John Stiles added 1 comment

    File third_party/blink/renderer/modules/canvas/canvas2d/base_rendering_context_2d.cc
    Line 3499, Patchset 12 (Latest): webgpu_access_texture_ = MakeGarbageCollected<GPUTexture>(
    John Stiles . unresolved

    ^

    Gerrit-Comment-Date: Thu, 25 Apr 2024 17:12:00 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    John Stiles (Gerrit)

    unread,
    Apr 25, 2024, 1:25:10 PMApr 25
    to Corentin Wallez, Aaron Krajeski, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, jmedle...@chromium.org
    Attention needed from Aaron Krajeski and Corentin Wallez

    John Stiles added 1 comment

    File third_party/blink/renderer/modules/canvas/canvas2d/base_rendering_context_2d.cc
    Line 3499, Patchset 12 (Latest): webgpu_access_texture_ = MakeGarbageCollected<GPUTexture>(
    John Stiles . resolved

    ^

    John Stiles

    Acknowledged

    Gerrit-Comment-Date: Thu, 25 Apr 2024 17:24:55 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: John Stiles <johns...@google.com>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Aaron Krajeski (Gerrit)

    unread,
    Apr 25, 2024, 3:21:38 PMApr 25
    to John Stiles, Corentin Wallez, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, jmedle...@chromium.org
    Attention needed from Corentin Wallez and John Stiles

    Aaron Krajeski added 1 comment

    File third_party/blink/renderer/modules/canvas/canvas2d/canvas_web_gpu_access_option.idl
    Line 18, Patchset 12 (Latest): GPUTextureUsageFlags usage = 0x14; // TEXTURE_BINDING | RENDER_ATTACHMENT
    Aaron Krajeski . unresolved

    Would 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?

    John Stiles

    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

    Aaron Krajeski

    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.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Corentin Wallez
    • John Stiles
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I6bb8e5e1fe336dd8baa770d022414ae9b129e876
    Gerrit-Change-Number: 5448084
    Gerrit-PatchSet: 12
    Gerrit-Owner: John Stiles <johns...@google.com>
    Gerrit-Reviewer: Aaron Krajeski <aar...@chromium.org>
    Gerrit-Reviewer: Corentin Wallez <cwa...@chromium.org>
    Gerrit-Reviewer: John Stiles <johns...@google.com>
    Gerrit-Attention: John Stiles <johns...@google.com>
    Gerrit-Attention: Corentin Wallez <cwa...@chromium.org>
    Gerrit-Comment-Date: Thu, 25 Apr 2024 19:21:28 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: John Stiles <johns...@google.com>
    Comment-In-Reply-To: Aaron Krajeski <aar...@chromium.org>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Aaron Krajeski (Gerrit)

    unread,
    Apr 25, 2024, 3:21:48 PMApr 25
    to John Stiles, Corentin Wallez, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, jmedle...@chromium.org
    Attention needed from Corentin Wallez and John Stiles

    Aaron Krajeski voted Code-Review+1

    Code-Review+1
    Open in Gerrit

    Related details

    Attention is currently required from:
    • Corentin Wallez
    • John Stiles
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I6bb8e5e1fe336dd8baa770d022414ae9b129e876
    Gerrit-Change-Number: 5448084
    Gerrit-PatchSet: 12
    Gerrit-Owner: John Stiles <johns...@google.com>
    Gerrit-Reviewer: Aaron Krajeski <aar...@chromium.org>
    Gerrit-Reviewer: Corentin Wallez <cwa...@chromium.org>
    Gerrit-Reviewer: John Stiles <johns...@google.com>
    Gerrit-Attention: John Stiles <johns...@google.com>
    Gerrit-Attention: Corentin Wallez <cwa...@chromium.org>
    Gerrit-Comment-Date: Thu, 25 Apr 2024 19:21:30 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: Yes
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    John Stiles (Gerrit)

    unread,
    Apr 25, 2024, 4:52:14 PMApr 25
    to Aaron Krajeski, Corentin Wallez, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, jmedle...@chromium.org
    Attention needed from Aaron Krajeski and Corentin Wallez

    John Stiles added 1 comment

    File third_party/blink/renderer/modules/canvas/canvas2d/canvas_web_gpu_access_option.idl
    Line 18, Patchset 12: GPUTextureUsageFlags usage = 0x14; // TEXTURE_BINDING | RENDER_ATTACHMENT
    Aaron Krajeski . unresolved

    Would 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?

    John Stiles

    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

    Aaron Krajeski

    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.

    John Stiles

    I see it in the WebGPU spec just like so... https://gpuweb.github.io/gpuweb/#dom-gpucanvasconfiguration-usage

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Aaron Krajeski
    • Corentin Wallez
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I6bb8e5e1fe336dd8baa770d022414ae9b129e876
    Gerrit-Change-Number: 5448084
    Gerrit-PatchSet: 13
    Gerrit-Owner: John Stiles <johns...@google.com>
    Gerrit-Reviewer: Aaron Krajeski <aar...@chromium.org>
    Gerrit-Reviewer: Corentin Wallez <cwa...@chromium.org>
    Gerrit-Reviewer: John Stiles <johns...@google.com>
    Gerrit-Attention: Corentin Wallez <cwa...@chromium.org>
    Gerrit-Attention: Aaron Krajeski <aar...@chromium.org>
    Gerrit-Comment-Date: Thu, 25 Apr 2024 20:51:58 +0000
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    John Stiles (Gerrit)

    unread,
    Apr 25, 2024, 4:54:56 PMApr 25
    to Aaron Krajeski, Corentin Wallez, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, jmedle...@chromium.org
    Attention needed from Aaron Krajeski and Corentin Wallez

    John Stiles added 1 comment

    File third_party/blink/renderer/modules/canvas/canvas2d/base_rendering_context_2d.cc
    Line 3475, Patchset 7: access_options->usage() | WGPUTextureUsage_CopySrc);
    Corentin Wallez . resolved

    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.

    John Stiles

    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.

    John Stiles

    Done

    John Stiles

    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.

    Gerrit-Comment-Date: Thu, 25 Apr 2024 20:54:42 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: John Stiles <johns...@google.com>
    Comment-In-Reply-To: Corentin Wallez <cwa...@chromium.org>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    John Stiles (Gerrit)

    unread,
    Apr 25, 2024, 4:55:04 PMApr 25
    to Aaron Krajeski, Corentin Wallez, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, jmedle...@chromium.org
    Attention needed from Aaron Krajeski and Corentin Wallez

    John Stiles added 1 comment

    File third_party/blink/renderer/modules/canvas/canvas2d/canvas_web_gpu_access_option.idl
    Line 18, Patchset 12: GPUTextureUsageFlags usage = 0x14; // TEXTURE_BINDING | RENDER_ATTACHMENT
    Aaron Krajeski . resolved

    Would 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?

    John Stiles

    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

    Aaron Krajeski

    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.

    John Stiles

    I see it in the WebGPU spec just like so... https://gpuweb.github.io/gpuweb/#dom-gpucanvasconfiguration-usage

    John Stiles

    Acknowledged

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Aaron Krajeski
    • Corentin Wallez
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement satisfiedCode-Review
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I6bb8e5e1fe336dd8baa770d022414ae9b129e876
    Gerrit-Change-Number: 5448084
    Gerrit-PatchSet: 13
    Gerrit-Owner: John Stiles <johns...@google.com>
    Gerrit-Reviewer: Aaron Krajeski <aar...@chromium.org>
    Gerrit-Reviewer: Corentin Wallez <cwa...@chromium.org>
    Gerrit-Reviewer: John Stiles <johns...@google.com>
    Gerrit-Attention: Corentin Wallez <cwa...@chromium.org>
    Gerrit-Attention: Aaron Krajeski <aar...@chromium.org>
    Gerrit-Comment-Date: Thu, 25 Apr 2024 20:54:48 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: John Stiles <johns...@google.com>
    Comment-In-Reply-To: Aaron Krajeski <aar...@chromium.org>
    satisfied_requirement
    open
    diffy

    John Stiles (Gerrit)

    unread,
    Apr 25, 2024, 5:07:59 PMApr 25
    to Aaron Krajeski, Corentin Wallez, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, jmedle...@chromium.org
    Attention needed from Aaron Krajeski and Corentin Wallez

    John Stiles voted Commit-Queue+2

    Commit-Queue+2
    Gerrit-Comment-Date: Thu, 25 Apr 2024 21:07:41 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: Yes
    satisfied_requirement
    open
    diffy

    Chromium LUCI CQ (Gerrit)

    unread,
    Apr 25, 2024, 6:42:41 PMApr 25