Add usage to GPUTextureViewDescriptor #4746

toji · 2024-07-11T17:02:03Z

Adds usage to texture views, defaulting them to the full set of usages from the texture. Validates that they must be a subset of the texture usage and validates that the view format is storage compatible if the STORAGE_BINDING usage is specified.

CC @teoxoy and @austinEng

github-actions · 2024-07-11T17:06:38Z

Previews, as seen when this build job started (799d217):
WebGPU _{webgpu.idl | Explainer | Correspondence Reference}
WGSL _{grammar.js | wgsl.lalr.txt}

teoxoy

I think we also need to update bind group creation validation and the "renderable texture view" validation to refer to the view's usages instead of the texture's.

toji · 2024-07-11T20:11:39Z

Good catch, thanks! Updated those two algorithms.

greggman · 2024-07-11T21:23:50Z

I feel like it would be nice to include a note about what this is trying to solve.

It's still not clear to me what problem this is solving from #4426

You create rgba8unorm texture with STORAGE_BINDING usage and then try to bind it with a view of rgba8unorm-srgb as storage texture. This could already fail without requiring usage on the view, just updating the validation rules.

You create rgba8unorm-srgb texture with STORAGE_BINDING usage and then try to bind it with a view of rgba8unorm as storage texture. This will already fail when trying to create the texture.

greggman · 2024-07-12T23:46:37Z

So Kai explained to me (which I didn't know). "views" can be actual native API objects and are created by createView.

With the change above, creating rgba8unorm texture with storage usage and then creating an rgbaunorm-srgb view of it will fail. You're required to remove the storage usage flag. It would be nice to mention that example in the spec.

kainino0x

Mostly LGTM

spec/index.bs

kainino0x

IIUC after the last discussion on this topic this PR is still the way we decided to go. I'm not sure what Kelsey was going to write up, but I think we have it here already (once comments are addressed).

spec/index.bs

kainino0x · 2024-08-28T22:25:30Z

spec/index.bs

+    ::
+        The allowed {{GPUTextureUsage|usage(s)}} for the texture view. Must be a subset of the
+        {{GPUTexture/usage}} flags of the texture. Defaults to the full set of {{GPUTexture/usage}}
+        flags of the texture.


Hmm, should we error if you request some viewFormat that has zero overlap with the usage of the texture?

Not sure I understand this?

Example GPUTextureDescriptor:
{ format: 'rgba8unorm', viewFormats: ['rgba8unorm-srgb'], usage: GPUTextureUsage.STORAGE_BINDING }
This makes it impossible to actually use that viewFormat so should it be an error to create this texture?

Similar to how we disallow usage from being 0.

Ah, got it. So something like (obvious psuedocode) usage & viewFormat.allowedUsages != 0. Makes sense.

TBH I'm not sure that this is needed, it seems to add preemptive validation just because it might be an interesting warning for developers. Maybe it should just be a warning in this case. IIRC there are other cases where we don't error even if the operation makes things that cannot be used.

Added a note to the texture validation algorithm encouraging implementations to warn in this scenario.

After discussion in the WG this needs to be changed to a TODO.

Kangz · 2024-09-02T12:04:34Z

spec/index.bs

         : GPUObjectDescriptorBase {
    GPUTextureFormat format;
    GPUTextureViewDimension dimension;
+    GPUTextureUsageFlags usage;


Note: I imagine for webgpu.h we'll want usage == None to mean inheriting the usages, so we might need something special to say that the usages are exactly 0.

Given that creating a view with no usages won't be useful (there's validation to ensure the texture usages aren't 0) is that a scenario we need to support? If you are asking explicitly for a usage of 0 we can pretty confidently say you are doing it wrong, and thus don't need to allow it when 0 can have a reasonable and useful alternative interpretation.

Sounds good, would this be a type error or something still surfaced as a validation error on the GPUDevice? (in both cases it is fairly implementable)

Maybe I misunderstood, but I thought None would be given the value of 0? In which case the default behavior is to inherit, but you can also specify any combination of other usages (which will be validated against the view format and the texture usages.) The only thing you can't do is specify a view usage of 0, which is OK because that's useless anyway. No new validation needed.

Cleared up the confusion offline. I thought the concern was primarily for native interfaces but it's actually about the JS interface where undefined and 0 are distinct.

My proposal to handle that is to have the dictionary default to 0 and define that a view usage of 0 means the usage is inherited. That way the native and JS interfaces function more similarly and the JS interface still ends up interpreting undefined as "inherit".

The alternative is that we only treat undefined as "inherit", throw a validation error if 0 is passed (which matches the texture validation) and native will either differ and treat 0 as inherit or introduce a new, explicit inherit enum. Given that this route involves more opportunity for errors with no real upside AND makes native and JS differ I'm not in favor of it.

WG approved of defaulting to 0 and having that mean inherit.

toji · 2024-09-04T21:10:43Z

Changed non-normative note to a TODO comment pointing at #4852. PR should match the WG's decisions now.

spec/index.bs

Fixes #4426 Adds usage to texture views, defaulting them to the full set of usages from the texture. Validates that they must be a subset of the texture usage and validates that the view format is storage compatible if the `STORAGE_BINDING` usage is specified.

Co-authored-by: Kai Ninomiya <kainino@chromium.org>

Kangz · 2024-09-10T15:11:50Z

GPU Web WG 2024-09-04 Atlantic-time

Need to decide pattern for specifying that the view usage is inherited from the texture usage.
BJ: generally this is you don't define a specific view. How does this translate down to native API? If I say 0 in view usages, is that analogous to inheriting definition from texture, or is it an error, like on the texture side? On native side, NONE is like inheriting the texture's usage. But on JS side, funneling both "undefined" and 0 down separately is difficult.
BJ: my proposal: in dictionary for TextureViewDescriptor: default for usage is 0, and that's treated as INHERIT. You don't have a way to say I want zero usages, but that's useless anyway and we'd need a separate error for it anyway.
BJ: alternative is "undefined" and 0 are separate on the JS side; need more validation rules then. No perceived benefit.
KG: sure (sounds fine)
- Don't think there's a real argument that the native behavior of "no usages" is compelling
MW: thumbs up
Consensus: default to 0, make it mean inherited!
KN: there was another thread on the PR
BJ: if you only specify STORAGE usage and specify a view format that can't do STORAGE, that view format can't be used. Validate at texture creation?
KG: think the consensus was that that was an error.
BJ: not sure we talked about that and got consensus on it.
KG: if there's something more to discuss here can we write it down in the bug
BJ: behavior in the PR (which doesn't address this) is fine, you can use it as-is and you won't get into an invalid state. Just can't make texture with a view that won't be used down the road. If we want to tighten it up, can discuss that as a separate topic to this one.
CW: problem was that it wasn't clear what to look at in the PR. Let's try to do better at that. Here there's agreement on everything except this one issue. Let's try to land something in the spec.
BJ: the reason this didn't come up in the agenda, this was basically resolved with a non-normative note for warnings. We talked about it on the pull request, came to some resolution.
CW: procedurally it's a validation error; we can add TODOs to the spec that have to be resolved before CR, and let's tightly scope agenda items for the next meeting.

toji requested review from jimblandy and kainino0x July 11, 2024 17:02

teoxoy reviewed Jul 11, 2024

View reviewed changes

kainino0x reviewed Jul 25, 2024

View reviewed changes

spec/index.bs Outdated Show resolved Hide resolved

spec/index.bs Outdated Show resolved Hide resolved

kainino0x added needs-cts-issue This change requires tests (or would need tests if accepted), but may not have a CTS issue filed yet api resolved Resolved - waiting for a change to the API specification api WebGPU API labels Jul 25, 2024

kainino0x reviewed Aug 28, 2024

View reviewed changes

toji force-pushed the view-usage branch from 17f8354 to aef04f5 Compare August 29, 2024 22:29

kainino0x added this to the Milestone 0 milestone Aug 30, 2024

Kangz reviewed Sep 2, 2024

View reviewed changes

toji force-pushed the view-usage branch 2 times, most recently from 92974c7 to f65c54c Compare September 4, 2024 17:52

toji mentioned this pull request Sep 4, 2024

Error on viewFormat with no compatible usage? #4852

Closed

toji force-pushed the view-usage branch from c17384f to 340f73d Compare September 4, 2024 21:09

kainino0x approved these changes Sep 5, 2024

View reviewed changes

spec/index.bs Outdated Show resolved Hide resolved

spec/index.bs Outdated Show resolved Hide resolved

spec/index.bs Show resolved Hide resolved

toji and others added 8 commits September 5, 2024 09:33

Update bind group creation and renderable view validation

68b4060

Update spec/index.bs

20713cf

Co-authored-by: Kai Ninomiya <kainino@chromium.org>

Address Kai's feedback

53f36e9

Note about warning for view formats with no valid usages

3357ea6

Default view usage to 0

4c45519

Add TODO about viewFormat/usage early validation

3602b7c

Addressing final feedback from Kai

799d217

toji force-pushed the view-usage branch from 340f73d to 799d217 Compare September 5, 2024 16:46

toji enabled auto-merge (squash) September 5, 2024 16:47

toji merged commit b39d86d into main Sep 5, 2024

kainino0x deleted the view-usage branch September 5, 2024 17:17

juj added a commit to juj/wasm_webgpu that referenced this pull request Sep 6, 2024

IDL: Add support for GPUTextureViewDescriptor.usage. gpuweb/gpuweb#4746

e46b3fc

This was referenced Sep 12, 2024

Add usage field to WGPUTextureViewDescriptor webgpu-native/webgpu-headers#327

Merged

Add tests for the usage field of GPUTextureViewDescriptor gpuweb/cts#3953

Closed

This comment was marked as spam.

Sign in to view

kainino0x removed the needs-cts-issue This change requires tests (or would need tests if accepted), but may not have a CTS issue filed yet label Nov 8, 2025

Add usage to GPUTextureViewDescriptor #4746

Add usage to GPUTextureViewDescriptor #4746

Uh oh!

Conversation

toji commented Jul 11, 2024

Uh oh!

github-actions bot commented Jul 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

teoxoy left a comment

Choose a reason for hiding this comment

Uh oh!

toji commented Jul 11, 2024

Uh oh!

greggman commented Jul 11, 2024

Uh oh!

greggman commented Jul 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kainino0x left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kainino0x left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kainino0x Aug 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

toji commented Sep 4, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Kangz commented Sep 10, 2024

Uh oh!

This comment was marked as spam.

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

github-actions bot commented Jul 11, 2024 •

edited

Loading

greggman commented Jul 12, 2024 •

edited

Loading

kainino0x Aug 30, 2024 •

edited

Loading