Usage of depthReadOnly / stencilReadOnly is not clear in the specification

[mwyrzykowski]

It is fairly clear what depthReadOnly / stencilReadOnly mean from an API perspective: https://gpuweb.github.io/gpuweb/#dom-gpurenderpassdepthstencilattachment-depthreadonly

and https://gpuweb.github.io/gpuweb/#command-encoder-pass-encoding

If depthStencilAttachment.depthReadOnly and stencilReadOnly are true:

The subresources of depthStencilView are considered to be used as attachment-read for the duration of the render pass.

but how does one read from those attachments in the WGSL fragment shader? Is the web developer expected to pass a GPUTextureView instance created from the depth texture to a bind group? Or is @builtin(frag_depth) available for input and output in the WGSL fragment shader? The spec says it is currently output only https://gpuweb.github.io/gpuweb/wgsl/#builtin-values

Or is there some other mechanism as to which the depth stencil attachments are available for reading in the fragment shader?

Presumably depthReadOnly / stencilReadOnly is more than just depthWriteEnabled=false, right?

[litherum]

It would be great if we could hook up GPURenderPassDepthStencilAttachment.depthReadOnly to MTLDepthStencilDescriptor.isDepthWriteEnabled, because they appear to mean the same thing, but are exposed on different objects, so I'm not sure it's possible.

It would be unfortunate if the WebGPU developer gives the implementation a signal about what's read-only, which the implementation would use if it could, but the developer supplies this information too late for it to actually be useful, just because of an API impedance mismatch.

[Kangz]

It would be great if we could hook up GPURenderPassDepthStencilAttachment.depthReadOnly to MTLDepthStencilDescriptor.isDepthWriteEnabled, because they appear to mean the same thing, but are exposed on different objects, so I'm not sure it's possible.

setRenderPipeline forces the depthWriteEnabled to be false when setting a pipeline in a depthReadOnly render pass, so that should already be the case.

[Kangz]

Is the web developer expected to pass a GPUTextureView instance created from the depth texture to a bind group? Or is @builtin(frag_depth) available for input and output in the WGSL fragment shader?

You're supposed to bind the depth texture as a either a texture_2d<f32> or a texture_2d_depth. Accessing @builtin(frag_depth) is not possible, because that requires at least your GPU to be a tiler, and even then I'm not sure if they all support it.

[mwyrzykowski]

@Kangz to be clear binding the depth texture is done the same as all other textures via setBindGroup, correct?

In that case it seems like depthReadOnly could be achieved by creating two identical GPURenderPipeline instances, except one has its depthStencilState's depthWriteEnabled set to true and the other to false.

Or is there a difference? I read over what I think is the initial proposal from Dzmitry https://github.com/gpuweb/gpuweb/issues/514 but it is not obvious why a developer would need to use depthReadOnly as opposed to creating the depth texture with usage RENDER_ATTACHMENT | TEXTURE_BINDING

[Kangz]

@Kangz to be clear binding the depth texture is done the same as all other textures via setBindGroup, correct?

Yep

In that case it seems like depthReadOnly could be achieved by creating two identical GPURenderPipeline instances, except one has its depthStencilState's depthWriteEnabled set to true and the other to false.

There shouldn't be a need to create two different pipelines: the pipelines have to have depthWriteEnabled = false if they are going to be used with depthReadOnly render passes. So there should be a single pipelines, the depthWriteEnabled = false.

Or is there a difference? I read over what I think is the initial proposal from Dzmitry https://github.com/gpuweb/gpuweb/issues/514 but it is not obvious why a developer would need to use depthReadOnly as opposed to creating the depth texture with usage RENDER_ATTACHMENT | TEXTURE_BINDING

The reason why creating the depth texture with the RENDER_ATTACHMENT | TEXTURE_BINDING is not sufficient is because of the "usage scope validation rules" that prevent data races. It is not allowed to use a subresource as "attachment" and "constant" at the same time. But it is allowed to have "attachment-readonly" and "constant" at the same time. depthReadOnly = true is what changes "attachment" to "attachment-readonly", which allows also sampling the depth texture in the pass.

[mwyrzykowski]

Doesn't GPUCommandEncoder.beginRenderPass seem like the incorrect place in the pipeline to specify a flag that will modify the resource usage of texture from "attachment" to "attachment-readonly"?

I will do some additional investigation but it was not immediately obvious how to implement this in Metal without specifying a MTLTextureUsage of MTLTextureUsageShaderRead | MTLTextureUsageRenderTarget which maps to WebGPU texture usages of RENDER_ATTACHMENT | TEXTURE_BINDING. Changing the usage at runtime is not supported as far as I can tell, we could copy the texture when we see depthReadOnly=true in GPUCommandEncoder.beginRenderPass but that would go against the intent of this feature I think.

Do you know why a flag such as RENDER_ATTACHMENT_READ_ONLY was not considered in GPUTextureUsage? Are there certain restrictions of D3D or Vulkan that would not allow that to apply to color attachments?

[Kangz]

I'm not sure I understand, the depthStencilReadOnly doesn't change the usage flags of the texture at creation. It just changes it's usage in the "usage scope" which is just a memory barrier thing (and can be ignored on Metal). Both Dawn and wgpu have implemented this feature successfully without additional copies or changing the texture usage flags at runtime.

[mwyrzykowski]

It is not allowed in Metal to read from a texture whose usage does not specify MTLTextureUsageShaderRead. Metal API validation returns an error if the depth texture is not created with MTLTextureUsageShaderRead and read in the fragment shader:

Fragment Function(fragmentShader): Shader reads texture (texture[0]) whose usage (0x04) doesn't specify MTLTextureUsageShaderRead (0x01)

It seems to work if I disable API validation, but we should not ignore API validation. It can become non-functional at any update or may not work on certain devices / hardware.

I can also always create the depth texture with MTLTextureUsageShaderRead irrespective of WebGPU's texture usage GPUTextureUsage.TEXTURE_BINDING, however this has considerable performance and memory implications so that would not be preferable.

[kainino0x]

I will do some additional investigation but it was not immediately obvious how to implement this in Metal without specifying a MTLTextureUsage of MTLTextureUsageShaderRead | MTLTextureUsageRenderTarget which maps to WebGPU texture usages of RENDER_ATTACHMENT | TEXTURE_BINDING.

This is correct. You must always must create a texture with GPUTextureUsage.TEXTURE_BINDING (MTLTextureUsageShaderRead) in order to be allowed to read it from a shader. Specifying depthReadOnly/stencilReadOnly is necessary but not sufficient to read from a depth/stencil attachment while it is bound; you need TEXTURE_BINDING too.

[mwyrzykowski]

Ok great, I think that clarifies everything, thank you.

[kainino0x]

No problem! Do you have suggestions on what could be changed/added in the spec to make this clearer? I'm thinking we can explain the purpose of depthReadOnly/stencilReadOnly on the member definition for each.

I'll reopen this to track at least a small clarification.

[mwyrzykowski]

That's a good idea. Maybe expand on the definition to of depthReadOnly to note the depth view's texture should be created with RENDER_ATTACHMENT | TEXTURE_BINDING to allow sampling it?

Or should attachment-read also specify the TEXTURE_BINDING bit?

attachment-read Texture used as a read-only attachment in a render pass. Preserves the contents. Allowed by texture RENDER_ATTACHMENT.

It's not clear if there is benefit to having a resource as attachment-read without TEXTURE_BINDING.

[Kangz]

Or should attachment-read also specify the TEXTURE_BINDING bit?

"attachment-read" should not be allowed by TEXTURE_BINDING because that would require adding the "renderable" usage in at least some of the target APIs, which can cause the texture to be more expensive to allocate and/or use. We don't want to add it for all depth-stencil TEXTURE_BINDING, only the ones the user will use as attachments. But a note explaining why you might want to do this and also point out you might want to add the TEXTURE_BINDING usage would work.