shader-slang / slang

Making it easier to work with shaders
http://shader-slang.com
Other
2.81k stars 207 forks source link

Detect and deduplicate system semantic input/output. #5530

Open wijiler opened 2 weeks ago

wijiler commented 2 weeks ago

using this fragment shader

static const int scaleFactor = 10;

struct Light
{
    float3 position;
    float4 color;
    float radius;
}
///
struct Transform
{
    float3 position;
    float2 scale;
    float rotation;
}
static const float4 vpositions[6] =
    {
        float4(1.0f, 1.0f, 0.0f, 1.0f),
        float4(1.0f, -1.0f, 0.0f, 1.0f),
        float4(-1.0f, -1.0f, 0.0f, 1.0f),
        float4(-1.0f, -1.0f, 0.0f, 1.0f),
        float4(-1.0f, 1.0f, 0.0f, 1.0f),
        float4(1.0f, 1.0f, 0.0f, 1.0f),
    };

float3 rot(float3 p, float3 o, float a)
{
    return float3((p.x - o.x) * cos(a) - (p.y - o.y) * sin(a) + o.x, (p.x - o.x) * sin(a) + (p.y - o.y) * cos(a) + o.y, p.z);
}

float3 scale(float3 p, float3 o, float sf)
{
    return float3(o.x + (p.x / sf), o.y + (p.y / sf), p.z);
}

float3 transform(Transform vert, uint32_t id)
{
    float3 rPos = (float3(vpositions[id].xyz) + vert.position) / scaleFactor;
    rPos = rot(rPos, vert.position, vert.rotation);
    rPos.x *= vert.scale.x;
    rPos.y *= vert.scale.y;

    return rPos;
}
///
struct pushConstants
{
    uint lightCount;
    Light *lights;
    Transform *instances;
}

struct VertOutput
{
    float4 pos : SV_Position;
}

[vk_binding(0, 0)]
Texture2D<float4> gBuffer[];

static const Texture2D<float4> albedo = gBuffer[0];

[vk_push_constant]
pushConstants pc;
[shader("vertex")]
VertOutput vertMain(uint vertID: SV_VertexID, uint instID: SV_InstanceID)
{
    VertOutput output;
    output.pos = float4(transform(pc.instances[instID], vertID), 1.0f);
    return output;
}

[shader("fragment")]
float4 fragMain(VertOutput input, uint2 fragPos: SV_Position)
    : SV_Target
{
    float4 color = albedo[fragPos];
    float3 diffuse = float3(0, 0, 0);
    for (uint i = 0; i < pc.lightCount; i++)
    {
        Light light = pc.lights[i];
        float3 ambient = light.color.xyz * 0.1f;

        float3 lightDir = light.position - input.pos.xyz;
        float atten = 1.0 / dot(lightDir, lightDir);
        float3 lightColor = light.color.xyz * (light.color.w * atten);
        diffuse = lightColor;
    }
    return float4(diffuse, 1.0f) * color;
}

We are met with these fun validation errors

VUID-VkShaderCreateInfoEXT-pCode-08737(ERROR / SPEC): msgNum: -1600984977 - Validation Error: [ VUID-VkShaderCreateInfoEXT-pCode-08737 ] | MessageID = 0xa092e86f | vkCreateShadersEXT(): pCreateInfos[0].pCode (spirv-val produced an error):
Non-unique OpEntryPoint interface '150[%150]' is disallowed
  %gl_FragCoord = OpVariable %_ptr_Input_v4float Input
. The Vulkan spec states: If codeType is VK_SHADER_CODE_TYPE_SPIRV_EXT, pCode must adhere to the validation rules described by the Validation Rules within a Module section of the SPIR-V Environment appendix (https://vulkan.lunarg.com/doc/view/1.3.290.0/windows/1.3-extensions/vkspec.html#VUID-VkShaderCreateInfoEXT-pCode-08737)
    Objects: 0
VUID-VkShaderCreateInfoEXT-pCode-08737(ERROR / SPEC): msgNum: -1600984977 - Validation Error: [ VUID-VkShaderCreateInfoEXT-pCode-08737 ] | MessageID = 0xa092e86f | vkCreateShadersEXT(): pCreateInfos[0].pCode (spirv-val produced an error):
Non-unique OpEntryPoint interface '150[%150]' is disallowed
  %gl_FragCoord = OpVariable %_ptr_Input_v4float Input
. The Vulkan spec states: If codeType is VK_SHADER_CODE_TYPE_SPIRV_EXT, pCode must adhere to the validation rules described by the Validation Rules within a Module section of the SPIR-V Environment appendix (https://vulkan.lunarg.com/doc/view/1.3.290.0/windows/1.3-extensions/vkspec.html#VUID-VkShaderCreateInfoEXT-pCode-08737)
    Objects: 0

But if we just return color or just return diffuse, there is no validation errors, hence why this confuses me, on latest release btw

wijiler commented 1 week ago

https://github.com/gfx-rs/naga/issues/2036

csyonghe commented 1 week ago

The problem is that SV_Position is declared twice in the fragment shader parameter list, once in VertOutput::pos and once in fragMain::fragPos, and Slang doesn't current deduplicate these things. A workaround is to remove one declaration.

wijiler commented 1 week ago

The problem is that SV_Position is declared twice in the fragment shader parameter list, once in VertOutput::pos and once in fragMain::fragPos, and Slang doesn't current deduplicate these things. A workaround is to remove one declaration.

Ah, it might be nice to have some warning for this, thank you for the information

wijiler commented 1 week ago

@csyonghe It might be nice to have some syntax like


struct VertOutput
{
    float4 pos : SV_Position, REAL_POS;
}

so that if I need to pass the vertex position into the fragment shader I dont have to do this


struct VertOutput
{
    float4 pos : SV_Position;
    float3 realPos : REAL_POS;
}

although im not sure its really that big of an issue

pdjonov commented 3 days ago

I hit this validation error with SV_ViewID because I linked together two different modules that bound to it (one a using private file-scope variable, the other in shader params). I worked around it, but modules being incompatible (even though the shader compile succeeds) because of something that's not even in the modules' public interface is not great.