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

RenderPass.setPushConstant broken on Dx12 #6558

Open
fyellin opened this issue Nov 16, 2024 · 3 comments
Open

RenderPass.setPushConstant broken on Dx12 #6558

fyellin opened this issue Nov 16, 2024 · 3 comments
Labels
area: naga back-end Outputs of naga shader conversion lang: HLSL D3D Shading Language naga Shader Translator type: bug Something isn't working

Comments

@fyellin
Copy link
Contributor

fyellin commented Nov 16, 2024

set_push_constant seems to be broken on Dx12. I have a small test that runs fine on GL, Vulkan, and Metal, but breaks on Dx12.

The test is simple. My push constants are an array of sixteen i32s, (the squares from 0 to 255). In either the vertex shader or the fragment shader I copy the constants to a buffer. I then read the contents of the buffer and compare with the values that were originally pushed.

assert_eq(result, expected_result);

Panic: assertion `left == right` failed
      left: [0, 16, 64, 144, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]
     right: [0, 1, 4, 9, 16, 25, 36, 49, 64, 81, 100, 121, 144, 169, 196, 225]

It seems strange that I'm seeing every fourth constant. And I'm only seeing this on Dx12.

bug_report.zip contains a single file bug_report.rs, which is a wgpu test.

bug_report.zip

@teoxoy
Copy link
Member

teoxoy commented Nov 18, 2024

I think this is because HLSL doesn't support arrays for root constants.

See: https://learn.microsoft.com/en-us/windows/win32/direct3d12/using-constants-directly-in-the-root-signature

I would have thought we'd get an error but it seems the shader is being miscompiled and we are indexing the internal vec4s and getting the first element?

@teoxoy teoxoy added the area: validation Issues related to validation, diagnostics, and error handling label Nov 18, 2024
@fyellin
Copy link
Contributor Author

fyellin commented Nov 18, 2024

I changed the code so that `let count = 8, and modified the shaders to be:

    struct PushConstants {
        value1: vec4i,
        value2: vec4i,
    }

and

    fn write(ix: u32) {
        let value = select(push_constants.value1, push_constants.value2, ix >= 4u);
        data[ix] = value[ix % 4];
    }

Everything worked fine on all platforms.

I'm not sure what to do with this bug. Is it a validator bug, and arrays shouldn't be allowed in push constants? Can we have platform-specific validator bugs, where arrays are just not allowed in DX12 push constants?

In any case, it's good to have a better understanding of what's going on.

@teoxoy teoxoy added type: bug Something isn't working area: naga back-end Outputs of naga shader conversion naga Shader Translator lang: HLSL D3D Shading Language and removed area: validation Issues related to validation, diagnostics, and error handling labels Nov 19, 2024
@teoxoy
Copy link
Member

teoxoy commented Nov 19, 2024

I think this is actually a bug in our HLSL backend, we should work around the HLSL limitation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: naga back-end Outputs of naga shader conversion lang: HLSL D3D Shading Language naga Shader Translator type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants