Should texture swizzle move usage restrictions from createView to createBindGroup/beginRenderPass?

[greggman]

In the current texture swizzle proposal, a non-default swizzle is not allowed on a texture view that has usage RENDER_ATTACHMENT or STORAGE_BINDING. I'm curious if it would be better to move that restriction and make it so:

1. a view passed as a colorAttachment or depthStencilAttachment or resolveTarget to beginRenderPass must have the default swizzle
2. a view bound to a storage texture binding in a bind group must have the default swizzle.

As the proposal is now, if you declare a texture with either RENDER_ATTACHMENT or STORAGE_BINDING then anytime you call createView with a swizzle you have to remember to remove those usages or you'll get a validation error. This is extra work for the user

const texture = device.createTexture({
   format: 'rgba8unorm',
   size,
   usage: GPUTextureUsage.TEXTURE_BINDING |
          GPUTextureUsage.STORAGE_BINDING |
          GPUTextureUsage.RENDER_ATTACHMENT,
});

const bindGroup = device.createBindGroup({
   layout,
   entries: [
      { binding: 0, resource: texture.createView({ swizzle: nonDefaultSwizzle }), // <======= ERROR!
   ],
});

On the other hand, if we switched to the validation as suggested above then, in general, you don't have to remember this detail. You'll only get an error if you try to use a swizzle on a storage binding or a render pass attachment. You won't get an error in the common usecase.

This doesn't seem like it has any big performance difference, but it makes it easier to use swizzles. They end up just working more often than the current proposal.

[kainino0x]

We could implicitly remove certain usages if swizzle is used [without setting the view usages explicitly]. That would be basically the same, we would just need to make sure that error messages explain what happened, otherwise it's very confusing.

[greggman]

We could implicitly remove certain usages if swizzle is used [without setting the view usages explicitly]. That would be basically the same, we would just need to make sure that error messages explain what happened, otherwise it's very confusing.

Is that better?

The presense of a non-default swizzle is not proof that's the reason STORAGE_BINDING or RENDER_ATTACHMENT usage is missing. It can be missing for 3 reasons:

1. it was auto removed because swizzle
2. the user removed it
3. it wasn't on the texture

To give the correct info you'd have to track if the user removed the usage OR it was auto-removed.

Conversely, error because swizzle is not allowed has just one reason:

1. because swizzle

Auto remove usages is also not forward compatible. If (and this will likely never happen), APIs were to add a write swizzle option, then you need some new option to not auto-remove usages.

A forth option is just to allow swizzle for storage texture and render target but specifiy it's ignored. That's what the acutal APIs do right? I get that's not forward compatible either, if for whatever reason APIs supported a write swizzle (not likely). I also get it might be confusing. createBindGroup(... resource: someStorageTexture.createView({ swizzle: bgra }), ...) is ignored. I just thought I'd mention it and reasons why probably not what we want.

[magcius]

Explicit seems better than implicit here; requiring users to explicitly subset usage seems better to me.

[greggman]

Explicit seems better than implicit here; requiring users to explicitly subset usage seems better to me.

There are 3 options being suggested, 2 of them are explicit, 1 of them requires subsetting

1. The current proposal (swizzle is not allowed with RENDER_ATTACHMENT, STORAGE_TEXTURE)

   This is explicit but also annoying to use since it requires explicit subsetting.
   Needing subseting will be common because RENDER_ATTACHMENT is needed for
   copyExternalImageToTexture and one or the other usage is needed for mipmap generation.

2. My proposal (swizzle is not allowed when binding to a storage binding, nor render attachment)

   This is also explicit but not annoying.

3. Kai's proposal (RENDER_ATTACHMENT and STORAGE_TEXTURE usage are implicitly removed)

   This one is implicit and not annoying

There is nothing less explicit between 1 and 2. The user gets exactly what they ask for and gets errors when it's incompatible with usage.

[magcius]

With your proposal:

const texture = device.createTexture({ usage: GPUTextureUsage.RENDER_ATTACHMENT });
const view = texture.createView({ swizzle: 'bgra', usage: GPUTextureUsage.RENDER_ATTACHMENT }); // no error, but creates a view that can't ever be used correctly

I'm in favor of erroring out as early as possible.

[greggman]

With your proposal:

const texture = device.createTexture({ usage: GPUTextureUsage.RENDER_ATTACHMENT });
const view = texture.createView({ swizzle: 'bgra', usage: GPUTextureUsage.RENDER_ATTACHMENT }); // no error, but creates a view that can't ever be used correctly

This is incorrect. That view is usable with my proposal as a TEXTURE_BINDING

I mis-understood your example. You're correct. That view is unusable. I'd prefer to consider checking that case rather than making the API tedious to use. We could add a check in createView that is 'error if non-default swizzles and usage doesn't include TEXTURE_BINDING'. I'm not sure how many errors that would actually catch though.

---

... reply left over from before I realized my mistake ...

// This is arguably the most or 2nd most common usage
// TEXTURE_BINDING for reading
// RENDER_ATTACHMENT for making mipmaps or for calling `copyExternalImageToTexture`
// COPY_DST to put data inside and/or for calling `copyExternalImageToTexture`
const texture = device.createTexture({
   usage: GPUTextureUsage.RENDER_ATTACHMENT |
          GPUTextureUsage.TEXTURE_BINDING |
          GPUTextureUsage.COPY_DST,
});

const bg = device.createBindGroup({
   layout,
   entries: [
     // as texture binding, most common usage - just works
     { binding, resource: texture.createView({ swizzle: 'bgra' }),
     // as storage texture binding also works
     { binding, resource: texture.createView(),
   ],
});

const pass = encoder.beginRenderPass({
  colorAttachments: [
    { view: texture.createView(), ... },  // also works
  ],
});

You don't get errors in common patterns.

I don't see this as any different than many other validations in WebGPU. For example, creating an rgba32float texture and not getting an error until createBindGroup time that it can't be attached to sampleType: 'float' texture binding because it's unfilterable-float. Or similarly, creating an rgba8uint texture and not getting an error until beginRenderPass time that it can't be resolved.

[kainino0x]

To be clear I'm fine with Gregg's proposal, 2. I prefer it over 1.

[kainino0x]

Actually after looking at the implementation more, I think the deciding factor here might be: would doing this require any backend to create multiple texture view objects for one GPUTextureView?

For example, for a TEXTURE_BINDING | RENDER_ATTACHMENT view with rrrr swizzle, would we need to create:

• a backend view with the equivalent of TEXTURE_BINDING and rrrr swizzle
• a backend view with the equivalent of RENDER_ATTANCHMENT and no swizzle

or can we just create one backend view and have it behave in that way?

Is this valid in all backends, and do all backends disregard the swizzle in the same cases? For example do read-only storage attachments consistently ignore the swizzle?

---

OR do we want to say that we're OK with backends needing to create multiple backend API views to implement a GPUTextureView?

[beaufortfrancois]

@greggman Any thoughts on @kainino0x's latest comment? Shall we ask the WG to weigh in?

[Kangz]

It seems that the three possibilities are:

1. Add validation in BeginRenderPass and other places that look at the swizzle and require that it is RGBA.
2. During view creation, automatically subset the usages based on swizzle, format etc. This is in effect similar to 1) for developers but the extra logic is done at view creation.
3. Add validation at CreateView.

I'd really like to avoid 1) because it adds validation in command recording code. 2) feels a bit too magic as we'll have to explain to developers why we removed a usage on the view, which is why 3) seems better. We can also always loosen to 2) or similar later.

[greggman]

I'd really like to avoid 1) because it adds validation in command recording code.

We're already validating in beginRenderPass for each target,

renderViewDescriptor.format must be a color renderable format.

this.view must be a renderable texture view.

If renderViewDescriptor.dimension is "3d":

this.depthSlice must be provided and must be < the depthOrArrayLayers of the logical miplevel-specific texture extent of the renderTexture subresource at mipmap level renderViewDescriptor.baseMipLevel.

Otherwise:

this.depthSlice must not be provided.

(1) adds one bit to check: this.view swizzle must be the identity.

If there is some other objection to (1) that's fine but please don't exaggerate how expensive the check is. Whether a swizzle is the identity or not is a single bit that can be set at creation time. In Dawn it would be one line added about here, something to the effect of DAWN_INVALID_IF(!attachment->IsSwizzleIdentity(), "swizzle must be identity"); with a similar single line check for resolveTarget and both on depth/depthResolve. It's a tiny check among all the things already being checked.

[greggman]

Actually after looking at the implementation more, I think the deciding factor here might be: would doing this require any backend to create multiple texture view objects for one GPUTextureView?

For example, for a TEXTURE_BINDING | RENDER_ATTACHMENT view with rrrr swizzle, would we need to create:

• a backend view with the equivalent of TEXTURE_BINDING and rrrr swizzle
• a backend view with the equivalent of RENDER_ATTANCHMENT and no swizzle

or can we just create one backend view and have it behave in that way?

Is this valid in all backends, and do all backends disregard the swizzle in the same cases? For example do read-only storage attachments consistently ignore the swizzle?

OR do we want to say that we're OK with backends needing to create multiple backend API views to implement a GPUTextureView?

2 views are never needed to be created implicitly by the backend in the three proposals. Rather, in all 3 cases, 2 views are required to be created explicitly by the user.

1. validation at createView|

   The user has to create the correct view for each situation. They'll often have to manually remove RENDER_ATTACHMENT usage to use a swizzle

   • at setBindGroup - createView({ swizzle, usage: GPUTextureUsage.TEXTURE_BINDING })
   • at beginRenderPass - createView()

2. validation at setBindGroup/beginRenderPass

   The user has to create the correct view for each situation - it's just correct with less work.

   • at setBindGroup - createView({ swizzle })
   • at beginRenderPass - createView()

3. implicit usage removal at createView

   The user has to create the correct view for each situation - it's just correct by with less work in more cases.

   • at setBindGroup - createView({ swizzle })
   • at beginRenderPass - createView()

[kainino0x]

I agree that it is not an issue to add validation in beginRenderPass. It's already possible to prevalidate the entire renderable texture view check at createView, so that beginRenderPass only needs to check one bit - this is just one more thing to prevalidate.

2 views are never needed to be created implicitly by the backend in the three proposals. Rather, in all 3 cases, 2 views are required to be created explicitly by the user.

Oops, you're right. I confused myself about whether we were going to just ignore the swizzle for render attachments (as I believe Metal does) because the implementation-internal thing we were talking about was how to apply R001 swizzle to all depth/stencil attachments.