From dcd7ad0c8a68fe8cdd7415bbd20a4ec2dc7950d9 Mon Sep 17 00:00:00 2001 From: Steve Karolewics Date: Thu, 5 Mar 2020 19:18:08 -0800 Subject: [PATCH] Fix incorrect logical identifiers for constant buffers with D3D12 The assignment of logical identifiers is not guaranteed to match the order of appearance in the shader bytecode. Adjusted D3D12_CBuffer_Zoo to exercise this for testing. --- .../driver/shaders/dxbc/dxbc_bytecode.cpp | 3 --- .../driver/shaders/dxbc/dxbc_container.cpp | 8 +++----- renderdoc/driver/shaders/dxbc/dxbc_debug.cpp | 19 ++++++++++++++++++- util/test/demos/d3d12/d3d12_cbuffer_zoo.cpp | 4 ++-- .../tests/D3D11/D3D11_Shader_Debug_Zoo.py | 17 +++++++---------- util/test/tests/D3D12/D3D12_CBuffer_Zoo.py | 18 +++++++++++++++--- .../tests/D3D12/D3D12_Shader_Debug_Zoo.py | 19 ++++++++----------- 7 files changed, 53 insertions(+), 35 deletions(-) diff --git a/renderdoc/driver/shaders/dxbc/dxbc_bytecode.cpp b/renderdoc/driver/shaders/dxbc/dxbc_bytecode.cpp index b42f60803..115f92aca 100644 --- a/renderdoc/driver/shaders/dxbc/dxbc_bytecode.cpp +++ b/renderdoc/driver/shaders/dxbc/dxbc_bytecode.cpp @@ -350,9 +350,6 @@ DXBC::Reflection *Program::GuessReflection() cb.name = desc.name; - // In addition to the register, store the identifier that we'll use to lookup during - // debugging. For SM5.1, this is the logical identifier that correlates to the CB order in - // the bytecode. For SM5 and earlier, it's the CB register. cb.identifier = idx; cb.space = dcl.space; cb.reg = reg; diff --git a/renderdoc/driver/shaders/dxbc/dxbc_container.cpp b/renderdoc/driver/shaders/dxbc/dxbc_container.cpp index 3e732a094..beeee12fa 100644 --- a/renderdoc/driver/shaders/dxbc/dxbc_container.cpp +++ b/renderdoc/driver/shaders/dxbc/dxbc_container.cpp @@ -725,7 +725,7 @@ DXBCContainer::DXBCContainer(const void *ByteCode, size_t ByteCodeLength) struct CBufferBind { - uint32_t reg, space, bindCount; + uint32_t reg, space, bindCount, identifier; }; std::map cbufferbinds; @@ -776,6 +776,7 @@ DXBCContainer::DXBCContainer(const void *ByteCode, size_t ByteCodeLength) cb.space = desc.space; cb.reg = desc.reg; cb.bindCount = desc.bindCount; + cb.identifier = h->targetVersion >= 0x501 ? res->ID : desc.reg; cbufferbinds[cname] = cb; } else if(desc.IsSampler()) @@ -926,10 +927,7 @@ DXBCContainer::DXBCContainer(const void *ByteCode, size_t ByteCodeLength) cbuffernames.insert(cname); - // In addition to the register, store the identifier that we'll use to lookup during - // debugging. For SM5.1, this is the logical identifier that correlates to the CB order in - // the bytecode. For SM5 and earlier, it's the CB register. - cb.identifier = (h->targetVersion < 0x501) ? cbufferbinds[cname].reg : i; + cb.identifier = cbufferbinds[cname].identifier; cb.space = cbufferbinds[cname].space; cb.reg = cbufferbinds[cname].reg; cb.bindCount = cbufferbinds[cname].bindCount; diff --git a/renderdoc/driver/shaders/dxbc/dxbc_debug.cpp b/renderdoc/driver/shaders/dxbc/dxbc_debug.cpp index cc88c62e5..928542092 100644 --- a/renderdoc/driver/shaders/dxbc/dxbc_debug.cpp +++ b/renderdoc/driver/shaders/dxbc/dxbc_debug.cpp @@ -4232,7 +4232,24 @@ void AddCBufferToGlobalState(const DXBCBytecode::Program &program, GlobalState & : global.constantBlocks[i].members; RDCASSERTMSG("Reassigning previously filled cbuffer", targetVars.empty()); - uint32_t cbufferIndex = program.IsShaderModel51() ? (uint32_t)i : slot.shaderRegister; + uint32_t cbufferIndex = slot.shaderRegister; + if(program.IsShaderModel51()) + { + // Need to lookup the logical identifier from the declarations + size_t numDeclarations = program.GetNumDeclarations(); + for(size_t d = 0; d < numDeclarations; ++d) + { + const DXBCBytecode::Declaration &decl = program.GetDeclaration(d); + if(decl.operand.type == DXBCBytecode::TYPE_CONSTANT_BUFFER && + decl.space == slot.registerSpace && + decl.operand.indices[1].index <= slot.shaderRegister && + decl.operand.indices[2].index >= slot.shaderRegister) + { + cbufferIndex = (uint32_t)decl.operand.indices[0].index; + break; + } + } + } global.constantBlocks[i].name = program.GetRegisterName(DXBCBytecode::TYPE_CONSTANT_BUFFER, cbufferIndex); diff --git a/util/test/demos/d3d12/d3d12_cbuffer_zoo.cpp b/util/test/demos/d3d12/d3d12_cbuffer_zoo.cpp index a89eb78b6..cd6c421a8 100644 --- a/util/test/demos/d3d12/d3d12_cbuffer_zoo.cpp +++ b/util/test/demos/d3d12/d3d12_cbuffer_zoo.cpp @@ -49,7 +49,7 @@ struct nested_with_padding // [3]: {24, 25, 26}, <27> }; -cbuffer consts : register(b0) +cbuffer consts : register(b7) { // dummy* entries are just to 'reset' packing to avoid pollution between tests @@ -329,7 +329,7 @@ float4 main() : SV_Target0 ID3D12ResourcePtr cb = MakeBuffer().Data(cbufferdata); ID3D12RootSignaturePtr sig = MakeSig({ - cbvParam(D3D12_SHADER_VISIBILITY_PIXEL, 0, 0), + cbvParam(D3D12_SHADER_VISIBILITY_PIXEL, 0, 7), constParam(D3D12_SHADER_VISIBILITY_PIXEL, 0, 1, sizeof(rootData) / sizeof(uint32_t)), cbvParam(D3D12_SHADER_VISIBILITY_PIXEL, 999999999, 0), }); diff --git a/util/test/tests/D3D11/D3D11_Shader_Debug_Zoo.py b/util/test/tests/D3D11/D3D11_Shader_Debug_Zoo.py index bac89d88d..ce040a724 100644 --- a/util/test/tests/D3D11/D3D11_Shader_Debug_Zoo.py +++ b/util/test/tests/D3D11/D3D11_Shader_Debug_Zoo.py @@ -17,8 +17,7 @@ class D3D11_Shader_Debug_Zoo(rdtest.TestCase): failed = False # Loop over every test - rdtest.log.print("Performing general tests:") - rdtest.log.indent() + rdtest.log.begin_section("General tests") for test in range(draw.numInstances): # Debug the shader trace: rd.ShaderDebugTrace = self.controller.DebugPixel(4 * test, 0, rd.ReplayController.NoPreference, @@ -31,7 +30,7 @@ class D3D11_Shader_Debug_Zoo(rdtest.TestCase): debugged = self.evaluate_source_var(output, variables) try: - self.check_pixel_value(pipe.GetOutputTargets()[0].resourceId, 4 * test, 0, debugged.value.fv[0:4], 0.0) + self.check_pixel_value(pipe.GetOutputTargets()[0].resourceId, 4 * test, 0, debugged.value.fv[0:4]) except rdtest.TestFailureException as ex: failed = True rdtest.log.error("Test {} did not match. {}".format(test, str(ex))) @@ -40,10 +39,9 @@ class D3D11_Shader_Debug_Zoo(rdtest.TestCase): self.controller.FreeTrace(trace) rdtest.log.success("Test {} matched as expected".format(test)) - rdtest.log.dedent() + rdtest.log.end_section("General tests") - rdtest.log.print("Performing MSAA tests:") - rdtest.log.indent() + rdtest.log.begin_section("MSAA tests") draw = draw.next self.controller.SetFrameEvent(draw.eventId, False) pipe: rd.PipeState = self.controller.GetPipelineState() @@ -53,9 +51,8 @@ class D3D11_Shader_Debug_Zoo(rdtest.TestCase): rd.ReplayController.NoPreference) # Validate that the correct sample index was debugged - inputs: List[rd.ShaderVariable] = list(trace.inputs) sampRegister = self.find_input_source_var(trace, rd.ShaderBuiltin.MSAASampleIndex) - sampInput = [var for var in inputs if var.name == sampRegister.variables[0].name][0] + sampInput = [var for var in trace.inputs if var.name == sampRegister.variables[0].name][0] if sampInput.value.uv[0] != test: rdtest.log.error("Test {} did not pick the correct sample.".format(test)) @@ -67,13 +64,13 @@ class D3D11_Shader_Debug_Zoo(rdtest.TestCase): # Validate the debug output result try: - self.check_pixel_value(pipe.GetOutputTargets()[0].resourceId, 4, 4, debugged.value.fv[0:4], 0.0, sub=rd.Subresource(0, 0, test)) + self.check_pixel_value(pipe.GetOutputTargets()[0].resourceId, 4, 4, debugged.value.fv[0:4], sub=rd.Subresource(0, 0, test)) except rdtest.TestFailureException as ex: failed = True rdtest.log.error("Test {} did not match. {}".format(test, str(ex))) continue - rdtest.log.dedent() + rdtest.log.end_section("MSAA tests") if failed: raise rdtest.TestFailureException("Some tests were not as expected") diff --git a/util/test/tests/D3D12/D3D12_CBuffer_Zoo.py b/util/test/tests/D3D12/D3D12_CBuffer_Zoo.py index 76ff9071f..1055fa873 100644 --- a/util/test/tests/D3D12/D3D12_CBuffer_Zoo.py +++ b/util/test/tests/D3D12/D3D12_CBuffer_Zoo.py @@ -1,7 +1,6 @@ import rdtest import renderdoc as rd - class D3D12_CBuffer_Zoo(rdtest.TestCase): demos_test_name = 'D3D12_CBuffer_Zoo' @@ -19,9 +18,9 @@ class D3D12_CBuffer_Zoo(rdtest.TestCase): refl: rd.ShaderReflection = pipe.GetShaderReflection(stage) mapping: rd.ShaderBindpointMapping = pipe.GetBindpointMapping(stage) - # Make sure we have three constant buffers - b0 normal, b1 root constants, and space9999999:b0 + # Make sure we have three constant buffers - b7 normal, b1 root constants, and space9999999:b0 binds = [ - (0, 0), + (0, 7), (0, 1), (999999999, 0), ] @@ -105,6 +104,19 @@ class D3D12_CBuffer_Zoo(rdtest.TestCase): rdtest.log.success("Debugged CBuffer variables are as expected") + cycles, variables = self.process_trace(trace) + + output = self.find_output_source_var(trace, rd.ShaderBuiltin.ColorOutput, 0) + + debugged = self.evaluate_source_var(output, variables) + + if not rdtest.util.value_compare(debugged.value.fv[0:4], [512.1, 513.0, 514.0, 515.0]): + raise rdtest.TestFailureException( + "Debugged output {} did not match expected {}".format( + debugged.value.fv[0:4], [512.1, 513.0, 514.0, 515.0])) + + rdtest.log.success("Debugged output matched as expected") + self.controller.FreeTrace(trace) self.check_pixel_value(pipe.GetOutputTargets()[0].resourceId, 0.5, 0.5, [512.1, 513.0, 514.0, 515.0]) diff --git a/util/test/tests/D3D12/D3D12_Shader_Debug_Zoo.py b/util/test/tests/D3D12/D3D12_Shader_Debug_Zoo.py index d0e6d6103..711cafe64 100644 --- a/util/test/tests/D3D12/D3D12_Shader_Debug_Zoo.py +++ b/util/test/tests/D3D12/D3D12_Shader_Debug_Zoo.py @@ -15,8 +15,7 @@ class D3D12_Shader_Debug_Zoo(rdtest.TestCase): shaderModels = ["sm_5_0", "sm_5_1"] for sm in range(len(shaderModels)): - rdtest.log.print("Beginning " + shaderModels[sm] + " tests...") - rdtest.log.indent() + rdtest.log.begin_section(shaderModels[sm] + " tests") # Jump to the draw test_marker: rd.DrawcallDescription = self.find_draw(shaderModels[sm]) @@ -38,7 +37,7 @@ class D3D12_Shader_Debug_Zoo(rdtest.TestCase): debugged = self.evaluate_source_var(output, variables) try: - self.check_pixel_value(pipe.GetOutputTargets()[0].resourceId, 4 * test, 0, debugged.value.fv[0:4], 0.0) + self.check_pixel_value(pipe.GetOutputTargets()[0].resourceId, 4 * test, 0, debugged.value.fv[0:4]) except rdtest.TestFailureException as ex: failed = True rdtest.log.error("Test {} did not match. {}".format(test, str(ex))) @@ -47,11 +46,10 @@ class D3D12_Shader_Debug_Zoo(rdtest.TestCase): self.controller.FreeTrace(trace) rdtest.log.success("Test {} matched as expected".format(test)) + + rdtest.log.end_section(shaderModels[sm] + " tests") - rdtest.log.dedent() - - rdtest.log.print("Performing MSAA tests:") - rdtest.log.indent() + rdtest.log.begin_section("MSAA tests") test_marker: rd.DrawcallDescription = self.find_draw("MSAA") draw = test_marker.next self.controller.SetFrameEvent(draw.eventId, False) @@ -62,9 +60,8 @@ class D3D12_Shader_Debug_Zoo(rdtest.TestCase): rd.ReplayController.NoPreference) # Validate that the correct sample index was debugged - inputs: List[rd.ShaderVariable] = list(trace.inputs) sampRegister = self.find_input_source_var(trace, rd.ShaderBuiltin.MSAASampleIndex) - sampInput = [var for var in inputs if var.name == sampRegister.variables[0].name][0] + sampInput = [var for var in trace.inputs if var.name == sampRegister.variables[0].name][0] if sampInput.value.uv[0] != test: rdtest.log.error("Test {} did not pick the correct sample.".format(test)) @@ -76,13 +73,13 @@ class D3D12_Shader_Debug_Zoo(rdtest.TestCase): # Validate the debug output result try: - self.check_pixel_value(pipe.GetOutputTargets()[0].resourceId, 4, 4, debugged.value.fv[0:4], 0.0, sub=rd.Subresource(0, 0, test)) + self.check_pixel_value(pipe.GetOutputTargets()[0].resourceId, 4, 4, debugged.value.fv[0:4], sub=rd.Subresource(0, 0, test)) except rdtest.TestFailureException as ex: failed = True rdtest.log.error("Test {} did not match. {}".format(test, str(ex))) continue - rdtest.log.dedent() + rdtest.log.end_section("MSAA tests") if failed: raise rdtest.TestFailureException("Some tests were not as expected")