diff --git a/renderdoc/core/replay_proxy.cpp b/renderdoc/core/replay_proxy.cpp index 74f21584e..47567376b 100644 --- a/renderdoc/core/replay_proxy.cpp +++ b/renderdoc/core/replay_proxy.cpp @@ -1246,11 +1246,6 @@ const ShaderReflection *ReplayProxy::GetShader(ResourceId pipeline, ResourceId s PROXY_FUNCTION(GetShader, pipeline, shader, entry); } -const ShaderReflection *ReplayProxy::GetShaderReflectionByPointer(uint64_t reflectionPointer) -{ - PROXY_FUNCTION(GetShaderReflectionByPointer, reflectionPointer); -} - template const ShaderReflection *ReplayProxy::Proxied_GetShaderReflectionByPointer(ParamSerialiser ¶mser, ReturnSerialiser &retser, @@ -1292,6 +1287,9 @@ const ShaderReflection *ReplayProxy::Proxied_GetShaderReflectionByPointer(ParamS SERIALISE_ELEMENT(packet); ser.EndChunk(); + // The remote pointer is only a lookup token, so the reflection is serialised to create a local + // copy. The serialiser owns this allocation; setting the pointer to NULL effectively steals it + // into the cache, which takes ownership. if(ser.IsReading()) { m_PointerReflectionCache[reflectionPointer] = reflection; @@ -1304,6 +1302,11 @@ const ShaderReflection *ReplayProxy::Proxied_GetShaderReflectionByPointer(ParamS return m_PointerReflectionCache[reflectionPointer]; } +const ShaderReflection *ReplayProxy::GetShaderReflectionByPointer(uint64_t reflectionPointer) +{ + PROXY_FUNCTION(GetShaderReflectionByPointer, reflectionPointer); +} + template rdcstr ReplayProxy::Proxied_DisassembleShader(ParamSerialiser ¶mser, ReturnSerialiser &retser, ResourceId pipeline, const ShaderReflection *refl, @@ -1859,15 +1862,6 @@ void ReplayProxy::Proxied_SavePipelineState(ParamSerialiser ¶mser, ReturnSer { const ReplayProxyPacket expectedPacket = eReplayProxy_SavePipelineState; ReplayProxyPacket packet = eReplayProxy_SavePipelineState; - rdcarray reflectionPointers; - - const auto fillReflectionPointers = [&reflectionPointers](const auto &stages, - size_t extraCount = 0) { - reflectionPointers.resize(ARRAY_COUNT(stages) + extraCount); - - for(size_t i = 0; i < ARRAY_COUNT(stages); i++) - reflectionPointers[i] = (uint64_t)(uintptr_t)stages[i]->reflection; - }; { BEGIN_PARAMS(); @@ -1888,60 +1882,20 @@ void ReplayProxy::Proxied_SavePipelineState(ParamSerialiser ¶mser, ReturnSer PACKET_HEADER(packet); if(m_APIProps.pipelineType == GraphicsAPI::D3D11) { - D3D11Pipe::Shader *stages[] = { - &m_D3D11PipelineState->vertexShader, &m_D3D11PipelineState->hullShader, - &m_D3D11PipelineState->domainShader, &m_D3D11PipelineState->geometryShader, - &m_D3D11PipelineState->pixelShader, &m_D3D11PipelineState->computeShader}; - - fillReflectionPointers(stages, 1); - - if(m_D3D11PipelineState->inputAssembly.resourceId != ResourceId()) - { - reflectionPointers[ARRAY_COUNT(stages)] = - (uint64_t)(uintptr_t)m_D3D11PipelineState->inputAssembly.bytecode; - } - SERIALISE_ELEMENT(*m_D3D11PipelineState); } else if(m_APIProps.pipelineType == GraphicsAPI::D3D12) { - D3D12Pipe::Shader *stages[] = { - &m_D3D12PipelineState->vertexShader, &m_D3D12PipelineState->hullShader, - &m_D3D12PipelineState->domainShader, &m_D3D12PipelineState->geometryShader, - &m_D3D12PipelineState->pixelShader, &m_D3D12PipelineState->computeShader, - &m_D3D12PipelineState->ampShader, &m_D3D12PipelineState->meshShader, - }; - - fillReflectionPointers(stages); - SERIALISE_ELEMENT(*m_D3D12PipelineState); } else if(m_APIProps.pipelineType == GraphicsAPI::OpenGL) { - GLPipe::Shader *stages[] = { - &m_GLPipelineState->vertexShader, &m_GLPipelineState->tessControlShader, - &m_GLPipelineState->tessEvalShader, &m_GLPipelineState->geometryShader, - &m_GLPipelineState->fragmentShader, &m_GLPipelineState->computeShader, - }; - - fillReflectionPointers(stages); - SERIALISE_ELEMENT(*m_GLPipelineState); } else if(m_APIProps.pipelineType == GraphicsAPI::Vulkan) { - VKPipe::Shader *stages[] = { - &m_VulkanPipelineState->vertexShader, &m_VulkanPipelineState->tessControlShader, - &m_VulkanPipelineState->tessEvalShader, &m_VulkanPipelineState->geometryShader, - &m_VulkanPipelineState->fragmentShader, &m_VulkanPipelineState->computeShader, - &m_VulkanPipelineState->taskShader, &m_VulkanPipelineState->meshShader, - }; - - fillReflectionPointers(stages); - SERIALISE_ELEMENT(*m_VulkanPipelineState); } - SERIALISE_ELEMENT(reflectionPointers); SERIALISE_ELEMENT(packet); ser.EndChunk(); @@ -1956,16 +1910,16 @@ void ReplayProxy::Proxied_SavePipelineState(ParamSerialiser ¶mser, ReturnSer }; for(size_t i = 0; i < ARRAY_COUNT(stages); i++) - if(reflectionPointers[i] != 0) + if(stages[i]->reflection != 0) { - stages[i]->reflection = GetShaderReflectionByPointer(reflectionPointers[i]); + stages[i]->reflection = + GetShaderReflectionByPointer((uint64_t)(uintptr_t)stages[i]->reflection); } - if(m_D3D11PipelineState->inputAssembly.resourceId != ResourceId()) + if(m_D3D11PipelineState->inputAssembly.bytecode != NULL) { - const size_t inputAssemblyByteCodeIndex = ARRAY_COUNT(stages); - m_D3D11PipelineState->inputAssembly.bytecode = - GetShaderReflectionByPointer(reflectionPointers[inputAssemblyByteCodeIndex]); + m_D3D11PipelineState->inputAssembly.bytecode = GetShaderReflectionByPointer( + (uint64_t)(uintptr_t)m_D3D11PipelineState->inputAssembly.bytecode); } } else if(m_APIProps.pipelineType == GraphicsAPI::D3D12 && m_D3D12PipelineState) @@ -1978,8 +1932,9 @@ void ReplayProxy::Proxied_SavePipelineState(ParamSerialiser ¶mser, ReturnSer }; for(size_t i = 0; i < ARRAY_COUNT(stages); i++) - if(reflectionPointers[i] != 0) - stages[i]->reflection = GetShaderReflectionByPointer(reflectionPointers[i]); + if(stages[i]->reflection != 0) + stages[i]->reflection = + GetShaderReflectionByPointer((uint64_t)(uintptr_t)stages[i]->reflection); } else if(m_APIProps.pipelineType == GraphicsAPI::OpenGL && m_GLPipelineState) { @@ -1990,8 +1945,9 @@ void ReplayProxy::Proxied_SavePipelineState(ParamSerialiser ¶mser, ReturnSer }; for(size_t i = 0; i < ARRAY_COUNT(stages); i++) - if(reflectionPointers[i] != 0) - stages[i]->reflection = GetShaderReflectionByPointer(reflectionPointers[i]); + if(stages[i]->reflection != 0) + stages[i]->reflection = + GetShaderReflectionByPointer((uint64_t)(uintptr_t)stages[i]->reflection); } else if(m_APIProps.pipelineType == GraphicsAPI::Vulkan && m_VulkanPipelineState) { @@ -2004,8 +1960,9 @@ void ReplayProxy::Proxied_SavePipelineState(ParamSerialiser ¶mser, ReturnSer for(size_t i = 0; i < ARRAY_COUNT(stages); i++) { - if(reflectionPointers[i] != 0) - stages[i]->reflection = GetShaderReflectionByPointer(reflectionPointers[i]); + if(stages[i]->reflection != 0) + stages[i]->reflection = + GetShaderReflectionByPointer((uint64_t)(uintptr_t)stages[i]->reflection); } } } diff --git a/renderdoc/replay/renderdoc_serialise.inl b/renderdoc/replay/renderdoc_serialise.inl index 6f16ed191..1c054b8a2 100644 --- a/renderdoc/replay/renderdoc_serialise.inl +++ b/renderdoc/replay/renderdoc_serialise.inl @@ -53,6 +53,14 @@ class undersized #define SIZE_CHECK(expected) #endif +// Serialise the remote reflection pointer as an opaque lookup token. It must not be dereferenced +// locally and is later replaced with a cached local reflection. +#define SERIALISE_SHADER_REFLECTION(member) \ + uint64_t refl = (uint64_t)(uintptr_t)el.member; \ + ser.Serialise(STRING_LITERAL(#member), refl); \ + if(ser.IsReading()) \ + el.member = (ShaderReflection *)(uintptr_t)refl; + template void DoSerialise(SerialiserType &ser, PathEntry &el) { @@ -1278,8 +1286,9 @@ void DoSerialise(SerialiserType &ser, D3D11Pipe::InputAssembly &el) { SERIALISE_MEMBER(layouts); SERIALISE_MEMBER(resourceId); - // don't serialise bytecode, just set it to NULL. See the definition of SERIALISE_MEMBER_DUMMY - SERIALISE_MEMBER_OPT_EMPTY(bytecode); + // Serialise the bytecode pointer as an opaque remote lookup token. It is resolved to a valid + // local reflection after the pipeline state is deserialised. + SERIALISE_SHADER_REFLECTION(bytecode); SERIALISE_MEMBER(vertexBuffers); SERIALISE_MEMBER(indexBuffer); SERIALISE_MEMBER(topology); @@ -1291,8 +1300,9 @@ template void DoSerialise(SerialiserType &ser, D3D11Pipe::Shader &el) { SERIALISE_MEMBER(resourceId); - // don't serialise reflection, just set it to NULL. See the definition of SERIALISE_MEMBER_DUMMY - SERIALISE_MEMBER_OPT_EMPTY(reflection); + // Serialise the reflection pointer as an opaque remote lookup token. It is resolved to a valid + // local reflection after the pipeline state is deserialised. + SERIALISE_SHADER_REFLECTION(reflection); SERIALISE_MEMBER(stage); SERIALISE_MEMBER(classInstances); @@ -1481,8 +1491,9 @@ template void DoSerialise(SerialiserType &ser, D3D12Pipe::Shader &el) { SERIALISE_MEMBER(resourceId); - // don't serialise reflection, just set it to NULL. See the definition of SERIALISE_MEMBER_DUMMY - SERIALISE_MEMBER_OPT_EMPTY(reflection); + // Serialise the reflection pointer as an opaque remote lookup token. It is resolved to a valid + // local reflection after the pipeline state is deserialised. + SERIALISE_SHADER_REFLECTION(reflection); SERIALISE_MEMBER(stage); SIZE_CHECK(24); @@ -1737,8 +1748,9 @@ void DoSerialise(SerialiserType &ser, GLPipe::Shader &el) SERIALISE_MEMBER(shaderResourceId); SERIALISE_MEMBER(programResourceId); - // don't serialise reflection, just set it to NULL. See the definition of SERIALISE_MEMBER_DUMMY - SERIALISE_MEMBER_OPT_EMPTY(reflection); + // Serialise the reflection pointer as an opaque remote lookup token. It is resolved to a valid + // local reflection after the pipeline state is deserialised. + SERIALISE_SHADER_REFLECTION(reflection); SERIALISE_MEMBER(stage); SERIALISE_MEMBER(subroutines); @@ -2055,8 +2067,9 @@ void DoSerialise(SerialiserType &ser, VKPipe::Shader &el) SERIALISE_MEMBER(resourceId); SERIALISE_MEMBER(entryPoint); - // don't serialise reflection, just set it to NULL. See the definition of SERIALISE_MEMBER_DUMMY - SERIALISE_MEMBER_OPT_EMPTY(reflection); + // Serialise the reflection pointer as an opaque remote lookup token. It is resolved to a valid + // local reflection after the pipeline state is deserialised. + SERIALISE_SHADER_REFLECTION(reflection); SERIALISE_MEMBER(stage); SERIALISE_MEMBER(pushConstantRangeByteOffset);