shader-slang / slang

Making it easier to work with shaders
MIT License
1.78k stars 159 forks source link

Metal: Slang does not legalize `[[color(n)]]`/`[[attribute(n)]]` attributes correctly #4487

Open ArielG-NV opened 2 days ago

ArielG-NV commented 2 days ago

breaks compile of (non exhaustive list, seems to be quite a number of tests): tests\compute\constexpr.slang tests\compute\compile-time-loop.slang tests\compute\discard-stmt.slang tests\compute\texture-sampling.slang

problem: the following metal code is allowed:

struct FragmentStageOutput_0
{
    float4 fragment_0 [[color(0)]];
};

[[fragment]] FragmentStageOutput_0 fragmentMain(float device* outputBuffer_0 [[buffer(0)]])
{
    thread FragmentStageOutput_0 output_0;
    (&output_0)->fragment_0 = float4(1.0, 1.0, 1.0, 1.0);
    *(outputBuffer_0+int(0)) = 1.0;
    return output_0;
}

the following metal code is not allowed (Slang generates the following code if struct wrapping a SV_Target):

struct float4Wrapper
{
    float4 v;
}
struct FragmentStageOutput_0
{
    float4Wrapper fragment_0 [[color(0)]];
};

[[fragment]] FragmentStageOutput_0 fragmentMain(float device* outputBuffer_0 [[buffer(0)]])
{
    thread FragmentStageOutput_0 output_0;
    (&output_0)->fragment_0.v = float4(1.0, 1.0, 1.0, 1.0);
    *(outputBuffer_0+int(0)) = 1.0;
    return output_0;
}

Specifically, metal allows [[color()]] and [[attribute()]] to only be annotated on PRIMITIVE and vector<PRIMITIVE,N>.

ArielG-NV commented 2 days ago

note: this is very similar to hoistEntryPointParameterFromStruct

jkwak-work commented 1 day ago

Is this duplicate to https://github.com/shader-slang/slang/issues/4375 ?

ArielG-NV commented 1 day ago

Is this duplicate to https://github.com/shader-slang/slang/issues/4375 ?

Looks like it is a partial duplicate.

[[color(n)]] is fragment shader, not vertex shader. I noticed the issue does discuss fragment shader inputs though.

ArielG-NV commented 16 hours ago

Plan to fix the following issues: 1. Allow n number "allowed types" instead of only two "allowed types" for when legalizing System Semantics for Metal 2. Move parts of legalizeEntryPointForMetal into EntryPointVaryingParamLegalizeContext so code compiles 3. Restructure so User Input Semantics legalization works 4. Restructure so User Output Semantics legalization works. 5. Restructure so System Semantic legalization works 6. Add logic to legalize Structs for attributes.