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

Wrong array lookup for uniform floats on generated HLSL #74

Open
MoritzBrueckner opened this issue Sep 3, 2020 · 8 comments
Open

Wrong array lookup for uniform floats on generated HLSL #74

MoritzBrueckner opened this issue Sep 3, 2020 · 8 comments

Comments

@MoritzBrueckner
Copy link

MoritzBrueckner commented Sep 3, 2020

Minimal GLSL example:

#version 450

#define BUFFER_SIZE 16

uniform float radius[BUFFER_SIZE];

out vec4 fragColor;

void main() {
	fragColor = vec4(radius[1], radius[1], radius[1], 1.0);
}

The radius array looks like the following in the Visual Studio graphics debugger:
ConstantBufferFloat

So the shader should output the value at the second position of the array, which is 4. But instead the output is 0, it is read from the fifth position. If I change the index in the glsl source to 2, the value from the 9th position is used. In OpenGL, everything works as expected, so there is some problem with cross-compiling to HLSL.

Full HLSL assembly:

//
// Generated by Microsoft (R) HLSL Shader Compiler 10.1
//
//
// Buffer Definitions: 
//
// cbuffer $Globals
// {
//
//   float radius[16];                  // Offset:    0 Size:   244
// }
//
//
// Resource Bindings:
//
// Name                                 Type  Format         Dim      HLSL Bind  Count
// ------------------------------ ---------- ------- ----------- -------------- ------
// $Globals                          cbuffer      NA          NA            cb0      1 
//
//
//
// Input signature:
//
// Name                 Index   Mask Register SysValue  Format   Used
// -------------------- ----- ------ -------- -------- ------- ------
// TEXCOORD                 0   xyzw        0     NONE   float       
// TEXCOORD                 1   x           1     NONE     int       
// SV_Position              0   xyzw        2      POS   float       
//
//
// Output signature:
//
// Name                 Index   Mask Register SysValue  Format   Used
// -------------------- ----- ------ -------- -------- ------- ------
// SV_Target                0   xyzw        0   TARGET   float   xyzw
//
      ps_4_0
      dcl_constantbuffer CB0[2], immediateIndexed
      dcl_output o0.xyzw
   0: mov o0.xyz, cb0[1].xxxx
   1: mov o0.w, l(1.000000)
   2: ret 
// Approximately 3 instruction slots used

I think the problematic line is this:

0: mov o0.xyz, cb0[1].xxxx

It looks as if the values were interpreted as float4 instead of float and thus the pointer offset for the array access is 4 floats big instead of one.

The array interpreted as float4s:
ConstantBufferFloat4

Thanks!

@MoritzBrueckner
Copy link
Author

Those could be the problematic lines:

else if (utype == floatarraytype) offset += arraySizes[floatarraytype] * 4;
else if (utype == vec2arraytype) offset += arraySizes[vec2arraytype] * 4 * 2;
else if (utype == vec3arraytype) offset += arraySizes[vec3arraytype] * 4 * 3;
else if (utype == vec4arraytype) offset += arraySizes[vec4arraytype] * 4 * 4;

I'm not really sure about the contents of arraySizes but it looks like this might be the issue here. @RobDangerous does this look like a bug to you or am I getting something wrong maybe?

@RobDangerous
Copy link
Member

That code is unrelated to HLSL output.

@MoritzBrueckner
Copy link
Author

I don't know anything about Krafix so if you can point me to where to look that would be awesome (it's like finding a needle in a haystack to me). Also, is there a way to look into the generated .hlsl other than its assembly code? The generated shaders are in .d3d11 format.

@luboslenco
Copy link
Contributor

@MoritzBrueckner To look into the generated .hlsl shaders, check the build/temp folder (instead of the build/$target-resources folder where the .d3d11 files are).

@RobDangerous
Copy link
Member

And hlsl is generated in SPIRV-Cross/spirv_hlsl.cpp

@MoritzBrueckner
Copy link
Author

MoritzBrueckner commented Sep 14, 2020

Thanks :)

@MoritzBrueckner
Copy link
Author

MoritzBrueckner commented Sep 14, 2020

This looks like it could be the cause: https://docs.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-packing-rules. It says:

Arrays are not packed in HLSL by default. To avoid forcing the shader to take on ALU overhead for offset computations, every element in an array is stored in a four-component vector. Note that you can achieve packing for arrays (and incur the addressing calculations) by using casting.

Unfortunately I'm by far not capable to see if this is a Kha, Krafix, SPIR-V or a user issue. But I can certainly say that there are visible differences between OpenGl and DirectX which likely should not exist. This currently keeps me from working on a project because I can't really build for DirectX.

Here is the fragment .hlsl of the example above, maybe it helps you:

uniform float radius[16];

static float4 fragColor;
static float4 color;
static int instanceID;

struct SPIRV_Cross_Input
{
    float4 color : TEXCOORD0;
    nointerpolation int instanceID : TEXCOORD1;
};

struct SPIRV_Cross_Output
{
    float4 fragColor : SV_Target0;
};

void frag_main()
{
    // I filled the radius array with values > 1, only the fifth (index 4) element is 0.
    // On OpenGL, the output is white, on directX it is black.
    fragColor = float4(radius[1], radius[1], radius[1], 1.0f);
}

SPIRV_Cross_Output main(SPIRV_Cross_Input stage_input)
{
    color = stage_input.color;
    instanceID = stage_input.instanceID;
    frag_main();
    SPIRV_Cross_Output stage_output;
    stage_output.fragColor = fragColor;
    return stage_output;
}

@MoritzBrueckner
Copy link
Author

If someone else stumbles on this issue: as a workaround, you can pass the float array uniforms as packed vec4 arrays, even if some entries might be unused. The lookup will be correct then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants