Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[spv-in] Sampling results for depth textures need to be changed to scalar floats #4551

Closed
expenses opened this issue May 25, 2023 · 3 comments · Fixed by #6384
Closed

[spv-in] Sampling results for depth textures need to be changed to scalar floats #4551

expenses opened this issue May 25, 2023 · 3 comments · Fixed by #6384
Labels
area: naga front-end lang: SPIR-V Vulkan's Shading Language naga Shader Translator type: enhancement New feature or request

Comments

@expenses
Copy link
Contributor

This is a followup to gfx-rs/naga#2341. Consider the following SPIR-V shader snippet:

<snip>
%type_2d_image = OpTypeImage %float 2D 1 0 0 1 Unknown
<snip>
         %68 = OpSampledImage %type_sampled_image %66 %67
         %69 = OpImageSampleExplicitLod %v4float %68 %65 Lod %float_0
         %70 = OpCompositeExtract %float %69 0
<snip>

OpTypeImage has a 1 after 2D to indicate that it is a depth texture. When it is sampled the sample typed is a 4-component vector as required by the spec.

This shader doesn't pass naga validation though because the wgsl spec requires that depth textures sample to scalar floats:

[2023-05-25T23:00:43Z ERROR naga::proc::typifier] Access index type Scalar { kind: Float, width: 4 }
Function [1] 'dof_downsample_with_coc' is invalid: 
        Expression [45] is invalid
        Type resolution failed
        Invalid access into expression [44], indexed: true
Generating wgsl output requires validation to succeed, and it failed in a previous step

If I manually mess with the the SPIR-V assembly to change the result type of the sample to a scalar float then naga processes it fine (note that this fails SPIR-V validation):

<snip>
%type_2d_image = OpTypeImage %float 2D 1 0 0 1 Unknown
<snip>
         %68 = OpSampledImage %type_sampled_image %66 %67
         %70 = OpImageSampleExplicitLod %float %68 %65 Lod %float_0
<snip>

What we need to do in the above case is, upon sampling a depth texture, we need to change the sample type to be a scalar float then insert a composite instruction that creates a 4-component vector with each field set to the scalar sample result (to not break later instructions).

@teoxoy teoxoy added lang: SPIR-V Vulkan's Shading Language area: naga front-end type: enhancement New feature or request labels May 31, 2023
@cwfitzgerald cwfitzgerald transferred this issue from gfx-rs/naga Oct 25, 2023
@cwfitzgerald cwfitzgerald added the naga Shader Translator label Oct 25, 2023
@schell
Copy link
Contributor

schell commented Oct 7, 2024

I just got bit by this one while attempting to use OpImageFetch to read a depth texture. @expenses is this still on your radar? I might try to tackle it as it's preventing me from finishing occlusion culling. I imagine @LPGhatguy would be interested in getting this one done, as well.

schell added a commit to schell/wgpu that referenced this issue Oct 8, 2024
@schell
Copy link
Contributor

schell commented Oct 8, 2024

In the case of OpImageSampleDref[Explicit | Implicit]Lod the result type is a scalar, we have to account for that.

@schell
Copy link
Contributor

schell commented Oct 8, 2024

Actually it's any of the OpImage variants that use depth references.

schell added a commit to schell/wgpu that referenced this issue Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: naga front-end lang: SPIR-V Vulkan's Shading Language naga Shader Translator type: enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants