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

sokol-shdc doesn't generate ATTR macros #141

Closed
danielchasehooper opened this issue Aug 12, 2024 · 5 comments
Closed

sokol-shdc doesn't generate ATTR macros #141

danielchasehooper opened this issue Aug 12, 2024 · 5 comments
Assignees

Comments

@danielchasehooper
Copy link

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
@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
Copy link
Owner

floooh commented Aug 12, 2024

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

for (const StageAttr& attr: prog.vs().inputs) {
if (attr.slot >= 0) {
cbl("{} => {}\n", vertex_attr_name(attr), attr.slot);
}
}

...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:

for (const StageAttr& attr: gen.refl.unique_vs_inputs) {
if (attr.slot >= 0) {
l("{}\n", vertex_attr_definition(attr));
}
}

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

std::vector<StageAttr> Reflection::merge_vs_inputs(const std::vector<ProgramReflection>& progs, ErrMsg& out_error) {
std::vector<StageAttr> out_attrs;
out_error = ErrMsg();
for (const ProgramReflection& prog: progs) {
for (const StageAttr& attr: prog.vs().inputs) {
if (attr.slot == -1) {
break;
}
const StageAttr* other_attr = find_attr_by_name(out_attrs, attr.snippet_name, attr.name);
if (other_attr) {
// take snippet-name into account for equality check
if (!attr.equals(*other_attr, true)) {
out_error = ErrMsg::error(fmt::format("conflicting vertex shader attributes found for '{}/{}'", attr.snippet_name, attr.name));
return std::vector<StageAttr>{};
}
} else {
out_attrs.push_back(attr);
}
}
}
return out_attrs;
}

I'll try to investigate further tomorrow.

@floooh
Copy link
Owner

floooh commented Aug 12, 2024

...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?

@floooh floooh self-assigned this Aug 12, 2024
floooh added a commit that referenced this issue Aug 12, 2024
@danielchasehooper
Copy link
Author

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
Copy link
Owner

floooh commented Aug 13, 2024

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

@floooh
Copy link
Owner

floooh commented Aug 13, 2024

This should now be fixed via PR #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 :)

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

2 participants