From 73af7100416e1627d1de0aaa94983f56406a9d49 Mon Sep 17 00:00:00 2001 From: Tim Foley Date: Tue, 12 Feb 2019 15:13:00 -0800 Subject: Track stage for varying sub-fields (#842) Fixes #841 This reverts a small change made in #815 that seemed innocent at the time: we stopped tracking an explicit `Stage` to go with every `VarLayout` that is part of an entry-point varying parameter, and instead only associated the stage with the top-level parameter. That change ended up breaking the logic to emit the `flat` modifier automatically for integer type fragment-shader inputs for GLSL, but we didn't have a regression test to catch that case. This change adds a regression test to cover this case, and adds the small number of lines that were removed from `parameter-binding.cpp`. A few other test outputs had to be updated for the change (these are outputs that were changed in #815 for the same reason). --- source/slang/parameter-binding.cpp | 13 ++++++++ tests/bugs/gh-841.slang | 19 ++++++++++++ tests/bugs/gh-841.slang.glsl | 35 ++++++++++++++++++++++ tests/reflection/sample-index-input.hlsl.expected | 1 + tests/reflection/sample-rate-input.hlsl.expected | 2 ++ .../vertex-input-semantics.hlsl.expected | 8 +++++ 6 files changed, 78 insertions(+) create mode 100644 tests/bugs/gh-841.slang create mode 100644 tests/bugs/gh-841.slang.glsl diff --git a/source/slang/parameter-binding.cpp b/source/slang/parameter-binding.cpp index 89226cdd9..f0abfe31c 100644 --- a/source/slang/parameter-binding.cpp +++ b/source/slang/parameter-binding.cpp @@ -2016,6 +2016,19 @@ static RefPtr processEntryPointVaryingParameter( EntryPointParameterState const& state, RefPtr varLayout) { + // Make sure to associate a stage with every + // varying parameter (including sub-fields of + // `struct`-type parameters), since downstream + // code generation will need to look at the + // stage (possibly on individual leaf fields) to + // decide when to emit things like the `flat` + // interpolation modifier. + // + if( varLayout ) + { + varLayout->stage = state.stage; + } + // The default handling of varying parameters should not apply // to geometry shader output streams; they have their own special rules. if( auto gsStreamType = as(type) ) diff --git a/tests/bugs/gh-841.slang b/tests/bugs/gh-841.slang new file mode 100644 index 000000000..becf741c1 --- /dev/null +++ b/tests/bugs/gh-841.slang @@ -0,0 +1,19 @@ +// gh-841.slang + +//TEST:CROSS_COMPILE: -profile ps_5_0 -entry main -target spirv-assembly + +// GitHub issue #841: failing to emit `flat` modifier in output GLSL when required + +struct RasterVertex +{ + float4 c : COLOR; + uint u : FLAGS; +}; + +float4 main(RasterVertex v) : SV_Target +{ + float4 result = v.c; + if(v.u & 1) + result += 1.0; + return result; +} diff --git a/tests/bugs/gh-841.slang.glsl b/tests/bugs/gh-841.slang.glsl new file mode 100644 index 000000000..caee80928 --- /dev/null +++ b/tests/bugs/gh-841.slang.glsl @@ -0,0 +1,35 @@ +//TEST_IGNORE_FILE: +#version 450 + +layout(location = 0) +out vec4 _S1; + +layout(location = 0) +in vec4 _S2; + +flat layout(location = 1) +in uint _S3; + +struct RasterVertex_0 +{ + vec4 c_0; + uint u_0; +}; + +void main() +{ + vec4 result_0; + RasterVertex_0 _S4 = RasterVertex_0(_S2, _S3); + vec4 result_1 = _S4.c_0; + + if(bool(_S4.u_0 & uint(1))) + { + result_0 = result_1 + 1.0; + } + else + { + result_0 = result_1; + } + _S1 = result_0; + return; +} diff --git a/tests/reflection/sample-index-input.hlsl.expected b/tests/reflection/sample-index-input.hlsl.expected index c799f8f25..5bf5f297e 100644 --- a/tests/reflection/sample-index-input.hlsl.expected +++ b/tests/reflection/sample-index-input.hlsl.expected @@ -29,6 +29,7 @@ standard output = { "scalarType": "float32" } }, + "stage": "fragment", "binding": {"kind": "varyingInput", "index": 0}, "semanticName": "COLOR" }, diff --git a/tests/reflection/sample-rate-input.hlsl.expected b/tests/reflection/sample-rate-input.hlsl.expected index ec6cfca6e..0c86ebecb 100644 --- a/tests/reflection/sample-rate-input.hlsl.expected +++ b/tests/reflection/sample-rate-input.hlsl.expected @@ -29,6 +29,7 @@ standard output = { "scalarType": "float32" } }, + "stage": "fragment", "binding": {"kind": "varyingInput", "index": 0}, "semanticName": "EXTRA" }, @@ -42,6 +43,7 @@ standard output = { "scalarType": "float32" } }, + "stage": "fragment", "binding": {"kind": "varyingInput", "index": 1}, "semanticName": "COLOR" } diff --git a/tests/reflection/vertex-input-semantics.hlsl.expected b/tests/reflection/vertex-input-semantics.hlsl.expected index 2ff8d7847..06b7bc95a 100644 --- a/tests/reflection/vertex-input-semantics.hlsl.expected +++ b/tests/reflection/vertex-input-semantics.hlsl.expected @@ -44,6 +44,7 @@ standard output = { "scalarType": "int32" } }, + "stage": "vertex", "binding": {"kind": "varyingInput", "index": 0}, "semanticName": "B" }, @@ -63,6 +64,7 @@ standard output = { "scalarType": "float32" } }, + "stage": "vertex", "binding": {"kind": "varyingInput", "index": 0}, "semanticName": "B", "semanticIndex": 1 @@ -77,12 +79,14 @@ standard output = { "scalarType": "float32" } }, + "stage": "vertex", "binding": {"kind": "varyingInput", "index": 1}, "semanticName": "B", "semanticIndex": 2 } ] }, + "stage": "vertex", "binding": {"kind": "varyingInput", "index": 1, "count": 2}, "semanticName": "B", "semanticIndex": 1 @@ -114,6 +118,7 @@ standard output = { "scalarType": "float32" } }, + "stage": "vertex", "binding": {"kind": "varyingInput", "index": 0}, "semanticName": "CX" }, @@ -127,12 +132,14 @@ standard output = { "scalarType": "float32" } }, + "stage": "vertex", "binding": {"kind": "varyingInput", "index": 1}, "semanticName": "CX", "semanticIndex": 1 } ] }, + "stage": "vertex", "binding": {"kind": "varyingInput", "index": 0, "count": 2}, "semanticName": "CX" }, @@ -146,6 +153,7 @@ standard output = { "scalarType": "int32" } }, + "stage": "vertex", "binding": {"kind": "varyingInput", "index": 2}, "semanticName": "CY" } -- cgit v1.2.3