floooh / sokol-tools

Command line tools for use with sokol headers
MIT License
229 stars 57 forks source link

sokol-shdc doesn't generate ATTR macros #141

Closed danielchasehooper closed 2 months ago

danielchasehooper commented 2 months ago

I'm using the binary from here (version numbers would be great!) to compile my shaders and it isn't ouputting ATTR_ macros for all the attributes. In the last version of sokol-shdc that I used (from floooh/sokol-tools-bin/ commit 8573f3ccbb2a8d05ff5a97295f8d2c7bdc2f6065), it would generate all the macros

run the following command on the below glsl to reproduce the issue.

./sokol-shdc-osx_arm64-20c5c5c0c6c1162a2432d84d18a6ce025b2476f7 --ifdef -l glsl300es -i shaders.glsl -o shaders.h     
click to expand shaders.glsl source ```glsl @ctype mat4 Mat4 @ctype vec4 Float4 @ctype vec3 Float3 @ctype vec2 Float2 // // Mark: Layer // @vs layer_vs in mat2 aRotateScale; in mat2 aUVRotateScale; in vec4 aColor; // premultiplied alpha in vec4 aBorderColor; // premultiplied alpha in vec2 aUVTranslate; in vec2 aVertexPosition; in vec2 aTranslate; in vec2 aSize; in float aCornerRadius; in float aParam; in float aSamplerIndex; #define aBorderWidthOrShadowRadius aParam // sokol doesn't allow attributes names longer than 16 bytes out vec4 color; out vec4 borderColor; // premultiplied alpha out vec2 coord; out vec2 uv; out vec2 half_size; out float cornerRadius; out float borderWidthOrShadowRadius; out float samplerIndex; void main(void) { color = aColor; half_size = aSize/2.0; cornerRadius = aCornerRadius; borderWidthOrShadowRadius = aBorderWidthOrShadowRadius; borderColor = aBorderColor; samplerIndex = aSamplerIndex; gl_Position = vec4(aRotateScale * aVertexPosition + aTranslate, 1, 1); coord = (aVertexPosition - 0.5) * aSize; uv = aUVRotateScale * aVertexPosition + aUVTranslate; } @end @fs layer_fs #define MAX_SUPPORTED_SAMPLERS 12 uniform texture2D texture0; uniform texture2D texture1; uniform texture2D texture2; uniform texture2D texture3; uniform texture2D texture4; uniform texture2D texture5; uniform texture2D texture6; uniform texture2D texture7; uniform texture2D texture8; uniform texture2D texture9; uniform texture2D texture10; uniform texture2D texture11; uniform sampler layer_sampler; in vec4 color; // must be premultiplied alpha in vec4 borderColor; // must be premultiplied alpha in vec2 coord; in vec2 uv; in vec2 half_size; in float cornerRadius; in float borderWidthOrShadowRadius; in float samplerIndex; out vec4 FragColor; float round_rect_sdf(vec2 p, vec2 rect_half_size, float radius){ vec2 d = abs(p) - rect_half_size + radius; return min(max(d.x, d.y), 0.0) + length(max(d, 0.0)) - radius; } vec4 source_over(vec4 upper,vec4 lower) { return upper + lower*(1.0 - upper.a); } void main(void) { float b = round_rect_sdf( coord, half_size, cornerRadius ); vec4 image_color = vec4(0); if (samplerIndex < 0.5) {image_color = texture(sampler2D(texture0, layer_sampler), uv);} else if (samplerIndex < 1.5) {image_color = texture(sampler2D(texture1, layer_sampler), uv);} else if (samplerIndex < 2.5) {image_color = texture(sampler2D(texture2, layer_sampler), uv);} else if (samplerIndex < 3.5) {image_color = texture(sampler2D(texture3, layer_sampler), uv);} else if (samplerIndex < 4.5) {image_color = texture(sampler2D(texture4, layer_sampler), uv);} else if (samplerIndex < 5.5) {image_color = texture(sampler2D(texture5, layer_sampler), uv);} else if (samplerIndex < 6.5) {image_color = texture(sampler2D(texture6, layer_sampler), uv);} else if (samplerIndex < 7.5) {image_color = texture(sampler2D(texture7, layer_sampler), uv);} else if (samplerIndex < 8.5) {image_color = texture(sampler2D(texture8, layer_sampler), uv);} else if (samplerIndex < 9.5) {image_color = texture(sampler2D(texture9, layer_sampler), uv);} else if (samplerIndex < 10.5) {image_color = texture(sampler2D(texture10, layer_sampler), uv);} else if (samplerIndex < 11.5) {image_color = texture(sampler2D(texture11, layer_sampler), uv);} vec4 composite_color = source_over(image_color,color); vec4 color_with_border = composite_color; if (b+borderWidthOrShadowRadius > -1.5) { vec4 border = borderColor * (clamp(b+borderWidthOrShadowRadius+1.5,0.0,1.0)); color_with_border = source_over(border, composite_color); } // multiplying by clamp instead of discarding works around an AMD graphics driver bug caused by // using "discard" or "return", it seems any early exit from the shader triggers the bug // the bug requires referencing more than one sampler in the shader and at least one of the samplers is mipmapped float alpha = (1.-clamp(b+1.5,0.,1.)); FragColor = color_with_border * alpha; } @end @program layer layer_vs layer_fs ```

You'll notice that the resulting shaders.h file doesn't have a definition for all the attributes it lists in the comments:

comment:

Attributes:
    ATTR_layer_vs_aRotateScale => 0
    ATTR_layer_vs_aUVRotateScale => 2
    ATTR_layer_vs_aColor => 4
    ATTR_layer_vs_aBorderColor => 5
    ATTR_layer_vs_aUVTranslate => 6
    ATTR_layer_vs_aVertexPosition => 7
    ATTR_layer_vs_aTranslate => 8
    ATTR_layer_vs_aSize => 9
    ATTR_layer_vs_aCornerRadius => 10
    ATTR_layer_vs_aParam => 11
    ATTR_layer_vs_aSamplerIndex => 12

macros:

#define ATTR_layer_vs_aRotateScale (0)
#define SLOT_texture0 (0)
#define SLOT_texture1 (1)
... etc ...
floooh commented 2 months ago

The list in the comment is generated from the inputs array directly on the vertex shader, so that seems to be correct:

https://github.com/floooh/sokol-tools/blob/9300010ad8c6ecbf68ed50c00ad2036591f8459c/src/shdc/generators/generator.cc#L95-L99

...but the actual macro definitions are generated from a 'unique list' which is supposed to contain all unique vertex shader inputs over all vertex shader snippets:

https://github.com/floooh/sokol-tools/blob/9300010ad8c6ecbf68ed50c00ad2036591f8459c/src/shdc/generators/generator.cc#L147-L151

...so the problem must be in the merge_vs_inputs function here:

https://github.com/floooh/sokol-tools/blob/9300010ad8c6ecbf68ed50c00ad2036591f8459c/src/shdc/reflection.cc#L287-L308

I'll try to investigate further tomorrow.

floooh commented 2 months ago

...looking at the vertex shader inputs in the debugger, there are unexpected gaps for the mat2 attributes.

E.g. after the array item for aRotateScale there's an empty slot (with slot index -1), followed by the next mat2 vertex attribute, which is also followed by another empty slot. The remaining vertex attributes then look normal.

image

Those empty slots cause the merge_vs_inputs() function to stop iterating which explains the problem.

So it's something about the mat2 vertex attributes (which are not 'officially supported' though). I suspect that GLSL or SPIRV splits mat2 vertex attributes into two vec2 attributes, but I need to investigate this further and also need to check what the GLSL and SPIRV spec has to say about matrix vertex attributes.

In any case it would be tricky to support matrix vertex attributes properly if they are indeed split into two regular vector attributes. It would be better to write them either as two explicit vec2 attributes, or a single vec4 attribute and then construct a matrix value in the shader from those.

I'm tempted to turn this problem into a 'proper' error to avoid confusion. But if it worked without problems before then maybe we can also implement a 'relaxed workaround'. Did it actually also work with HLSL and Metal outputs, or did you only test with GLSL output?

danielchasehooper commented 2 months ago

It has worked with metal. I don't target HLSL.

For what it's worth, this is how I was providing the data on the CPU side:

sg_make_pipeline(&(sg_pipeline_desc){

       // lines removed for clarity

        .layout.attrs = {
                [ATTR_layer_vs_aUVRotateScale+0] = {
                    .offset = offsetof(DrawInfo,uvRotateScaleMatrix),
                    .format = SG_VERTEXFORMAT_FLOAT2,
                    .buffer_index = 1,
                },
                [ATTR_layer_vs_aUVRotateScale+1] = {
                    .offset = offsetof(DrawInfo,uvRotateScaleMatrix) +sizeof(float)*2,
                    .format = SG_VERTEXFORMAT_FLOAT2,
                    .buffer_index = 1,
                },
       }
});
floooh commented 2 months ago

Ah interesting, that should work yeah. Ok I'll see if I can restore the old behaviour.

floooh commented 2 months ago

This should now be fixed via PR https://github.com/floooh/sokol-tools/pull/142.

Binaries will be updated after this CI pipeline goes green: https://github.com/floooh/sokol-tools/actions/runs/10373572552

Please let me know if it works for you :)