diff --git a/renderdoc/driver/shaders/dxbc/dxbc_debug.cpp b/renderdoc/driver/shaders/dxbc/dxbc_debug.cpp index 2a3a5328c..cb50c8ba9 100644 --- a/renderdoc/driver/shaders/dxbc/dxbc_debug.cpp +++ b/renderdoc/driver/shaders/dxbc/dxbc_debug.cpp @@ -4666,6 +4666,47 @@ void LookupSRVFormatFromShaderReflection(const DXBC::Reflection &reflection, } } +DXBCBytecode::InterpolationMode GetInterpolationModeForInputParam(const SigParameter &sig, + const DXBC::Reflection &psDxbc, + const DXBCBytecode::Program *program) +{ + if(sig.varType == VarType::SInt || sig.varType == VarType::UInt) + return DXBCBytecode::InterpolationMode::INTERPOLATION_CONSTANT; + + if(sig.varType == VarType::Float) + { + // if we're packed with ints on either side, we must be nointerpolation + size_t numInputs = psDxbc.InputSig.size(); + for(size_t j = 0; j < numInputs; j++) + { + if(sig.regIndex == psDxbc.InputSig[j].regIndex && psDxbc.InputSig[j].varType != VarType::Float) + return DXBCBytecode::InterpolationMode::INTERPOLATION_CONSTANT; + } + + DXBCBytecode::InterpolationMode interpolation = DXBCBytecode::INTERPOLATION_UNDEFINED; + + if(program) + { + for(size_t d = 0; d < program->GetNumDeclarations(); d++) + { + const DXBCBytecode::Declaration &decl = program->GetDeclaration(d); + + if(decl.declaration == DXBCBytecode::OPCODE_DCL_INPUT_PS && + decl.operand.indices[0].absolute && decl.operand.indices[0].index == sig.regIndex) + { + interpolation = decl.interpolation; + break; + } + } + } + + return interpolation; + } + + RDCERR("Unexpected input signature type: %s", ToStr(sig.varType).c_str()); + return DXBCBytecode::InterpolationMode::INTERPOLATION_UNDEFINED; +} + void GatherPSInputDataForInitialValues(const DXBC::DXBCContainer *dxbc, const DXBC::Reflection &prevStageDxbc, rdcarray &initialValues, @@ -4793,50 +4834,11 @@ void GatherPSInputDataForInitialValues(const DXBC::DXBCContainer *dxbc, nextreg = sig.regIndex + 1; - if(sig.varType == VarType::Float) - { - // if we're packed with ints on either side, we must be nointerpolation - bool nointerp = false; - for(size_t j = 0; j < numInputs; j++) - { - if(sig.regIndex == psDxbc.InputSig[j].regIndex && psDxbc.InputSig[j].varType != VarType::Float) - { - nointerp = true; - break; - } - } - - DXBCBytecode::InterpolationMode interpolation = DXBCBytecode::INTERPOLATION_UNDEFINED; - - if(program) - { - for(size_t d = 0; d < program->GetNumDeclarations(); d++) - { - const DXBCBytecode::Declaration &decl = program->GetDeclaration(d); - - if(decl.declaration == DXBCBytecode::OPCODE_DCL_INPUT_PS && - decl.operand.indices[0].absolute && decl.operand.indices[0].index == sig.regIndex) - { - interpolation = decl.interpolation; - break; - } - } - } - - if(nointerp) - psInputDefinition += "nointerpolation "; - else if(interpolation != DXBCBytecode::INTERPOLATION_UNDEFINED && - interpolation != DXBCBytecode::INTERPOLATION_CONSTANT) - psInputDefinition += ToStr(interpolation) + " "; - - psInputDefinition += "float"; - } - else if(sig.varType == VarType::SInt) - psInputDefinition += "nointerpolation int"; - else if(sig.varType == VarType::UInt) - psInputDefinition += "nointerpolation uint"; - else - RDCERR("Unexpected input signature type: %s", ToStr(sig.varType).c_str()); + DXBCBytecode::InterpolationMode interpolation = + GetInterpolationModeForInputParam(sig, psDxbc, program); + if(interpolation != DXBCBytecode::INTERPOLATION_UNDEFINED) + psInputDefinition += ToStr(interpolation) + " "; + psInputDefinition += ToStr(sig.varType); int numCols = (sig.regChannelMask & 0x1 ? 1 : 0) + (sig.regChannelMask & 0x2 ? 1 : 0) + (sig.regChannelMask & 0x4 ? 1 : 0) + (sig.regChannelMask & 0x8 ? 1 : 0); @@ -4869,17 +4871,22 @@ void GatherPSInputDataForInitialValues(const DXBC::DXBCContainer *dxbc, for(size_t j = i + 1; j < numInputs; j++) { - // if we've found the 'next' semantic - if(sig.semanticName == psDxbc.InputSig[j].semanticName && - nextIdx == psDxbc.InputSig[j].semanticIndex) - { - int jNumCols = (psDxbc.InputSig[j].regChannelMask & 0x1 ? 1 : 0) + - (psDxbc.InputSig[j].regChannelMask & 0x2 ? 1 : 0) + - (psDxbc.InputSig[j].regChannelMask & 0x4 ? 1 : 0) + - (psDxbc.InputSig[j].regChannelMask & 0x8 ? 1 : 0); + const SigParameter &jSig = psDxbc.InputSig[j]; - // if it's the same size, and it's at the start of the next register - if(jNumCols == numCols && psDxbc.InputSig[j].regChannelMask <= 0x3) + // if we've found the 'next' semantic + if(sig.semanticName == jSig.semanticName && nextIdx == jSig.semanticIndex) + { + int jNumCols = (jSig.regChannelMask & 0x1 ? 1 : 0) + (jSig.regChannelMask & 0x2 ? 1 : 0) + + (jSig.regChannelMask & 0x4 ? 1 : 0) + (jSig.regChannelMask & 0x8 ? 1 : 0); + + DXBCBytecode::InterpolationMode jInterp = + GetInterpolationModeForInputParam(jSig, psDxbc, program); + + // if it's the same size, type, and interpolation mode, then it could potentially be + // packed into an array. Check if it's using the first channel component to tell whether + // it's tightly packed with another semantic. + if(jNumCols == numCols && interpolation == jInterp && sig.varType == jSig.varType && + jSig.regChannelMask & 0x1) { if(arrayLength == 0) arrayLength = 2; @@ -4908,13 +4915,13 @@ void GatherPSInputDataForInitialValues(const DXBC::DXBCContainer *dxbc, // detected but it WILL be put in its own register (not packed together), so detect this // case too. // Note we have to search *backwards* because we need to know if this register should have - // been packed into the previous register, but wasn't. float/float2 can be packed after an - // array just fine. - if(included && i > 0 && arrayLength == 0 && numCols <= 2 && sig.regChannelMask <= 0x3) + // been packed into the previous register, but wasn't. float/float2/float3 can be packed after + // an array just fine, so long as the sum of their components doesn't exceed a register width + if(included && i > 0 && arrayLength == 0) { const SigParameter &prev = psDxbc.InputSig[i - 1]; - if(prev.regIndex != sig.regIndex && prev.compCount <= 2 && prev.regChannelMask <= 0x3) + if(prev.regIndex != sig.regIndex && prev.compCount + sig.compCount <= 4) arrayLength = 1; } diff --git a/util/test/demos/d3d11/d3d11_shader_linkage_zoo.cpp b/util/test/demos/d3d11/d3d11_shader_linkage_zoo.cpp index 4b88758f0..f3eef81da 100644 --- a/util/test/demos/d3d11/d3d11_shader_linkage_zoo.cpp +++ b/util/test/demos/d3d11/d3d11_shader_linkage_zoo.cpp @@ -264,6 +264,11 @@ float4 main(v2f IN) : SV_Target0 {false, VarType::Float, 3, 0, "TEXCOORD1", true}, {false, VarType::Float, 2, 0, "TEXCOORD2", true}})); + tests.push_back(BuildTestCase({{false, VarType::Float, 2, 1, "TEXCOORD0", true}, + {false, VarType::Float, 2, 1, "TEXCOORD1", true}, + {false, VarType::Float, 3, 2, "TEXCOORD2", true}, + {false, VarType::Float, 2, 0, "TEXCOORD4", true}})); + // Semantics that don't pack together due to being arrays tests.push_back(BuildTestCase({{false, VarType::Float, 1, 2, "TEXCOORD0", true}})); tests.push_back(BuildTestCase({{false, VarType::Float, 2, 1, "TEXCOORD0", true}, @@ -276,26 +281,25 @@ float4 main(v2f IN) : SV_Target0 // Tests focusing on different interpolation modes tests.push_back(BuildTestCase({{false, VarType::Float, 2, 0, "TEXCOORD0", true}, {true, VarType::Float, 2, 0, "TEXCOORD1", true}})); - // The following test is currently broken: the semantics live in v1.x and v1.y, but during - // debugging our initial inputs shader places them in an array[2], resulting in v1.x and v2.x + // These semantics are placed in v1.x and v1.y since they share interpolation modes and types + // (all int semantics are nointerpolation). Test that they don't get placed in v1.x and v2.x tests.push_back(BuildTestCase({{false, VarType::UInt, 1, 0, "TEXCOORD0", true}, {true, VarType::UInt, 1, 0, "TEXCOORD1", true}})); - // The following test is currently broken: the semantics live in v1.x and v2.x, but during - // debugging our initial inputs shader places them in an array[2], resulting in the correct - // register placement, but incorrect interpolation modes since uints are always nointerpolation + // These semantics are placed in v1.x and v2.x since their interpolation modes differ. Test that + // they don't turn into an array[2] which would result in an erroneous interpolation mode for + // one semantic or the other tests.push_back(BuildTestCase({{false, VarType::Float, 1, 0, "TEXCOORD0", true}, {false, VarType::UInt, 1, 0, "TEXCOORD1", true}})); - // The following test is currently broken: the semantics live in v1.x and v1.y, despite having - // different types since the interpolation mode is the same. During debugging our initial inputs - // shader places them in an array[2], resulting in the wrong register placement + // These semantics are placed in v1.x and v1.y despite having different types since the + // interpolation mode is the same. Test that they don't turn into an array[2] which would place + // them in the wrong registers tests.push_back(BuildTestCase({{true, VarType::Float, 1, 0, "TEXCOORD0", true}, {false, VarType::UInt, 1, 0, "TEXCOORD1", true}})); // Bespoke tests for broken scenarios discovered through bug reports: - // The following test is currently broken: the semantics live in v1.xy, v2.x, and v3.xyz due - // to each being an array. During debugging, out initial input shader defines the last semantic - // without an array size, so FXC packs it into v2.yzw + // These semantics live in v1.xy, v2.x, and v3.xyz due to each being an array. If any of them + // are not treated as an array[1], they will incorrectly pack together with a previous semantic tests.push_back(BuildTestCase({{false, VarType::Float, 2, 1, "TEXCOORD0", true}, {false, VarType::Float, 1, 1, "TEXCOORD1", false}, {false, VarType::Float, 3, 1, "TEXCOORD2", true}})); diff --git a/util/test/demos/d3d12/d3d12_shader_linkage_zoo.cpp b/util/test/demos/d3d12/d3d12_shader_linkage_zoo.cpp index ba877f837..3d6b62511 100644 --- a/util/test/demos/d3d12/d3d12_shader_linkage_zoo.cpp +++ b/util/test/demos/d3d12/d3d12_shader_linkage_zoo.cpp @@ -250,6 +250,11 @@ float4 main(v2f IN) : SV_Target0 {false, VarType::Float, 3, 0, "TEXCOORD1", true}, {false, VarType::Float, 2, 0, "TEXCOORD2", true}})); + psos.push_back(BuildPSO(sig, {{false, VarType::Float, 2, 1, "TEXCOORD0", true}, + {false, VarType::Float, 2, 1, "TEXCOORD1", true}, + {false, VarType::Float, 3, 2, "TEXCOORD2", true}, + {false, VarType::Float, 2, 0, "TEXCOORD4", true}})); + // Semantics that don't pack together due to being arrays psos.push_back(BuildPSO(sig, {{false, VarType::Float, 1, 2, "TEXCOORD0", true}})); psos.push_back(BuildPSO(sig, {{false, VarType::Float, 2, 1, "TEXCOORD0", true}, @@ -262,26 +267,25 @@ float4 main(v2f IN) : SV_Target0 // Tests focusing on different interpolation modes psos.push_back(BuildPSO(sig, {{false, VarType::Float, 2, 0, "TEXCOORD0", true}, {true, VarType::Float, 2, 0, "TEXCOORD1", true}})); - // The following test is currently broken: the semantics live in v1.x and v1.y, but during - // debugging our initial inputs shader places them in an array[2], resulting in v1.x and v2.x + // These semantics are placed in v1.x and v1.y since they share interpolation modes and types + // (all int semantics are nointerpolation). Test that they don't get placed in v1.x and v2.x psos.push_back(BuildPSO(sig, {{false, VarType::UInt, 1, 0, "TEXCOORD0", true}, {true, VarType::UInt, 1, 0, "TEXCOORD1", true}})); - // The following test is currently broken: the semantics live in v1.x and v2.x, but during - // debugging our initial inputs shader places them in an array[2], resulting in the correct - // register placement, but incorrect interpolation modes since uints are always nointerpolation + // These semantics are placed in v1.x and v2.x since their interpolation modes differ. Test that + // they don't turn into an array[2] which would result in an erroneous interpolation mode for + // one semantic or the other psos.push_back(BuildPSO(sig, {{false, VarType::Float, 1, 0, "TEXCOORD0", true}, {false, VarType::UInt, 1, 0, "TEXCOORD1", true}})); - // The following test is currently broken: the semantics live in v1.x and v1.y, despite having - // different types since the interpolation mode is the same. During debugging our initial inputs - // shader places them in an array[2], resulting in the wrong register placement + // These semantics are placed in v1.x and v1.y despite having different types since the + // interpolation mode is the same. Test that they don't turn into an array[2] which would place + // them in the wrong registers psos.push_back(BuildPSO(sig, {{true, VarType::Float, 1, 0, "TEXCOORD0", true}, {false, VarType::UInt, 1, 0, "TEXCOORD1", true}})); // Bespoke tests for broken scenarios discovered through bug reports: - // The following test is currently broken: the semantics live in v1.xy, v2.x, and v3.xyz due - // to each being an array. During debugging, out initial input shader defines the last semantic - // without an array size, so FXC packs it into v2.yzw + // These semantics live in v1.xy, v2.x, and v3.xyz due to each being an array. If any of them + // are not treated as an array[1], they will incorrectly pack together with a previous semantic psos.push_back(BuildPSO(sig, {{false, VarType::Float, 2, 1, "TEXCOORD0", true}, {false, VarType::Float, 1, 1, "TEXCOORD1", false}, {false, VarType::Float, 3, 1, "TEXCOORD2", true}}));