icculus / mojoshader

Use Direct3D shaders with other 3D rendering APIs.
https://icculus.org/mojoshader/
zlib License
122 stars 36 forks source link

[spirv][glspirv] Const Arrays #35

Open kg opened 3 years ago

kg commented 3 years ago

Just a note for posterity that, like in every other backend before it, the spir-v backend seems to have a partially or completely broken implementation for local const arrays. static const arrays might also be broken but i haven't tested them.

Example problem code snippet:

#define DEFINE_QuadCorners const float2 QuadCorners[] = { \
    {0, 0}, \
    {1, 0}, \
    {1, 1}, \
    {0, 1} \
};

inline float2 ComputeCorner (
    in int2 cornerIndex : BLENDINDICES0,
    in float2 regionSize
) {
    DEFINE_QuadCorners
    float2 corner = QuadCorners[cornerIndex.x];
    return corner * regionSize;
}

in original XNA along with FNA3D d3d11 and FNA3D glsl, QuadCorners[1] will be {1, 0} but in spir-v right now, it seems to always be 0.

I'm just going to learn my lesson and never use arrays again but maybe someone else will run into this problem in the future.

flibitijibibo commented 3 years ago

Pretty sure this is the relevant block:

https://github.com/FNA-XNA/MojoShader/blob/upstream/profiles/mojoshader_profile_spirv.c#L2345-L2454

It should be emitting constants, vs ARB_gl_spirv where it emits constants as uniform data.

kg commented 3 years ago

More context for testing/troubleshooting, since I ran into another bit of code (not mine, this time) that uses them. Neither of these snippets work in Vulkan:

static const float thresholdMatrix[] = {
    1.0 / 17.0,  9.0 / 17.0,  3.0 / 17.0, 11.0 / 17.0,
    13.0 / 17.0,  5.0 / 17.0, 15.0 / 17.0,  7.0 / 17.0,
    4.0 / 17.0, 12.0 / 17.0,  2.0 / 17.0, 10.0 / 17.0,
    16.0 / 17.0,  8.0 / 17.0, 14.0 / 17.0,  6.0 / 17.0
};

bool StippleReject (float index, float stippleFactor) {
    float stippleThreshold = thresholdMatrix[index % 16];
    return (stippleFactor - stippleThreshold) <= 0;
}

nor this:

bool StippleReject (float index, float stippleFactor) {
    const float thresholdMatrix[] = {
        1.0 / 17.0,  9.0 / 17.0,  3.0 / 17.0, 11.0 / 17.0,
        13.0 / 17.0,  5.0 / 17.0, 15.0 / 17.0,  7.0 / 17.0,
        4.0 / 17.0, 12.0 / 17.0,  2.0 / 17.0, 10.0 / 17.0,
        16.0 / 17.0,  8.0 / 17.0, 14.0 / 17.0,  6.0 / 17.0
    };

    float stippleThreshold = thresholdMatrix[index % 16];
    return (stippleFactor - stippleThreshold) <= 0;
}
kg commented 3 years ago

Bad news is that /mojoshaderprofile:glspirv is not broken. The good news is that uniform const float thresholdMatrix[] does work, so I can use that until we figure this out. (Maybe I should just use that in general?)

flibitijibibo commented 3 years ago

The workaround seems fine, but this tells me the block I linked to is right - the only difference between the GL and VK modes are the VPOS fixup and the const array emit block.

krolli commented 3 years ago

It will still be a few days until I fully catch up with mojoshader and fna3d development after summer madness. I will try to build some test shaders when I get around to it to see what dxbc looks like for them. I don't see any obvious reason why it would work with glspirv (in GL), but not with vkspirv (in vulkan).

Is there difference between dxbc for local and global const array? Also, who fills in values? Should those be baked into the shader or is it done by preshader?

krolli commented 3 years ago

I've been experimenting with those arrays, but nothing obviously wrong comes to mind. Test shaders I used were compiled with fxc.exe /nologo /T ps_3_0 /O3 "hlsl\array-1-global.hlsl" /Fo "fxb\array-1-global.psa" /Fc "fxb\array-1-global.psh". Attaching them for reference:

static const float thresholdMatrix[] = {
    1.0 / 17.0,  9.0 / 17.0,  3.0 / 17.0, 11.0 / 17.0,
    13.0 / 17.0,  5.0 / 17.0, 15.0 / 17.0,  7.0 / 17.0,
    4.0 / 17.0, 12.0 / 17.0,  2.0 / 17.0, 10.0 / 17.0,
    16.0 / 17.0,  8.0 / 17.0, 14.0 / 17.0,  6.0 / 17.0
};

bool StippleReject (float index, float stippleFactor) {
    float stippleThreshold = thresholdMatrix[index % 16];
    return (stippleFactor - stippleThreshold) <= 0;
}

void main(
    in  float4 in0  : TEXCOORD0,
    out float4 out0 : COLOR0
)
{
    out0 = StippleReject(in0.x, in0.y) ? float4(0, 0, 0, 0) : float4(1, 1, 1, 1);
}
bool StippleReject (float index, float stippleFactor) {
    const float thresholdMatrix[] = {
        1.0 / 17.0,  9.0 / 17.0,  3.0 / 17.0, 11.0 / 17.0,
        13.0 / 17.0,  5.0 / 17.0, 15.0 / 17.0,  7.0 / 17.0,
        4.0 / 17.0, 12.0 / 17.0,  2.0 / 17.0, 10.0 / 17.0,
        16.0 / 17.0,  8.0 / 17.0, 14.0 / 17.0,  6.0 / 17.0
    };

    float stippleThreshold = thresholdMatrix[index % 16];
    return (stippleFactor - stippleThreshold) <= 0;
}

void main(
    in  float4 in0  : TEXCOORD0,
    out float4 out0 : COLOR0
)
{
    out0 = StippleReject(in0.x, in0.y) ? float4(0, 0, 0, 0) : float4(1, 1, 1, 1);
}
#define DEFINE_QuadCorners const float2 QuadCorners[] = { \
    {0, 0}, \
    {1, 0}, \
    {1, 1}, \
    {0, 1} \
};

inline float2 ComputeCorner (
    in int2 cornerIndex : BLENDINDICES0,
    in float2 regionSize
) {
    DEFINE_QuadCorners
    float2 corner = QuadCorners[cornerIndex.x];
    return corner * regionSize;
}

void main(
    in  float2 in0  : TEXCOORD0,
    in  int2   in1  : BLENDINDICES0,
    out float4 out0 : COLOR0
)
{
    out0 = float4(0,0,0,1);
    out0.xy = ComputeCorner(in1, in0);
}

Unfortunately, fxc spits out exactly same binary for global and local array in StippleReject. Also, glspirv and vkspirv results are exactly the same for all three shaders.

@kg, can you provide compiled versions of any of those problematic shaders? Ideally version that works and one that doesn't so I can see what is different between them.

It is possible that the way constants are emitted for SPIR-V is only compatible with OpenGL and not with Vulkan, but that would probably show up elsewhere as well, so I'm not convinced that is the case.

Ethan might be right that the problem stems from uniform handling, though I'd like to be sure before making changes in it.

kg commented 3 years ago

The global and local array should produce the same binary, the thing that works is uniform const float x[] at the top level.

krolli commented 3 years ago

Ah, right, you said that neither of them works. In any case, those shaders I posted compile into what looks like perfectly normal binary. Unfortunately, fxc replaced array access in these simple shaders with a bunch of comparison tricks and simple constants (not uniforms).

krolli commented 3 years ago

I found some minor difference between SPIR-V output of glslangValidator and mojoshader. While mojoshader uses initializer operand of OpVariable instruction to set constant data, glslangValidator emits separate OpStore instruction in main entrypoint. It's a shot in the dark, but maybe it will fix the problem. I'm not sure whether this is bug in the driver or our generated code though. If it fixes the problem, I'll try to go over the spec in more detail to see if there is some mention this.

Here's the commit, if you could give it a try and see if it helps: https://github.com/krolli/MojoShader/commit/06f924f1e0efe625b508a5cd1be7a26777785970

krolli commented 3 years ago

@kg, have you tried those changes to see if they help with the problem?

kg commented 3 years ago

Missed the previous comment, will try to take a look soon