Fix ID registration when false-duplication happens on replay

* We normally deduplicate during capture, but replay can introduce new
  duplication against internal objects. Using ReplaceResource() would fix this
  but updating the wrapper map is necessary for D3D11 to query current state
  properly.
This commit is contained in:
baldurk
2026-04-16 11:21:08 +01:00
parent c610f14a0c
commit 0b9a36b581
18 changed files with 98 additions and 252 deletions
+27 -6
View File
@@ -654,7 +654,8 @@ public:
bool AddWrapper(WrappedResourceType wrap, RealResourceType real);
bool HasWrapper(RealResourceType real);
WrappedResourceType GetWrapper(RealResourceType real);
void RemoveWrapper(RealResourceType real);
void RemoveWrapper(WrappedResourceType wrapped, RealResourceType real);
void OverrideWrapper(RealResourceType real);
void ResetLastWriteTimes();
void ResetCaptureStartTime();
@@ -1784,18 +1785,38 @@ bool ResourceManager<Configuration>::AddWrapper(WrappedResourceType wrap, RealRe
}
template <typename Configuration>
void ResourceManager<Configuration>::RemoveWrapper(RealResourceType real)
void ResourceManager<Configuration>::RemoveWrapper(WrappedResourceType wrapped, RealResourceType real)
{
SCOPED_LOCK_OPTIONAL(m_Lock, m_Capturing);
if(real == (RealResourceType)RecordType::NullResource || !HasWrapper(real))
if(real == (RealResourceType)RecordType::NullResource)
{
RDCERR(
"Invalid state removing resource wrapper - real resource is NULL or doesn't have wrapper");
RDCERR("Invalid state removing resource wrapper - real resource is NULL");
return;
}
m_WrapperMap.erase(m_WrapperMap.find(real));
auto it = m_WrapperMap.find(real);
// silently ignore/drop removals of non-canonical wrappers. This handles the case where we have
// multiple wrappers for the same object on replay, due to API deduplication between an internal
// object and an application-created object. Backends are expected to deduplicate during capture
// (See OverrideWrapper below)
if(it == m_WrapperMap.end() || it->second == wrapped)
m_WrapperMap.erase(it);
}
template <typename Configuration>
void ResourceManager<Configuration>::OverrideWrapper(RealResourceType real)
{
SCOPED_LOCK_OPTIONAL(m_Lock, m_Capturing);
// during replay we may find that we have extra duplicate wrappers because of internal resources,
// even though we trid to deduplicate on capture. For this case we remove the old wrapper and
// allow the new wrapper to become canonical.
auto it = m_WrapperMap.find(real);
if(it != m_WrapperMap.end())
m_WrapperMap.erase(it);
}
template <typename Configuration>
+1 -1
View File
@@ -1952,7 +1952,7 @@ void WrappedID3D11Device::DestroyDeadObject(ID3D11DeviceChild *child)
m_pImmediateContext->RemoveAnnotations(id);
// clean up book-keeping
rm->RemoveWrapper(wrapped->GetReal());
rm->RemoveWrapper(wrapped, wrapped->GetReal());
rm->ReleaseResource(id);
D3D11ResourceRecord *record = GetResourceManager()->GetResourceRecord(id);
if(record)
+6 -20
View File
@@ -102,16 +102,9 @@ bool WrappedID3D11Device::Serialise_CreateBlendState1(SerialiserType &ser,
}
else
{
if(GetResourceManager()->HasWrapper(ret))
{
ret->Release();
ret = (ID3D11BlendState1 *)GetResourceManager()->GetWrapper(ret);
ret->AddRef();
}
else
{
ret = new WrappedID3D11BlendState1(pState, ret, this);
}
GetResourceManager()->OverrideWrapper(ret);
ret = new WrappedID3D11BlendState1(pState, ret, this);
}
AddResource(pState, ResourceType::StateObject, "Blend State");
@@ -216,16 +209,9 @@ bool WrappedID3D11Device::Serialise_CreateRasterizerState1(
}
else
{
if(GetResourceManager()->HasWrapper(ret))
{
ret->Release();
ret = (ID3D11RasterizerState1 *)GetResourceManager()->GetWrapper(ret);
ret->AddRef();
}
else
{
ret = new WrappedID3D11RasterizerState2(pState, ret, this);
}
GetResourceManager()->OverrideWrapper(ret);
ret = new WrappedID3D11RasterizerState2(pState, ret, this);
}
AddResource(pState, ResourceType::StateObject, "Rasterizer State");
+3 -10
View File
@@ -734,16 +734,9 @@ bool WrappedID3D11Device::Serialise_CreateRasterizerState2(
}
else
{
if(GetResourceManager()->HasWrapper(ret))
{
ret->Release();
ret = (ID3D11RasterizerState2 *)GetResourceManager()->GetWrapper(ret);
ret->AddRef();
}
else
{
ret = new WrappedID3D11RasterizerState2(pState, ret, this);
}
GetResourceManager()->OverrideWrapper(ret);
ret = new WrappedID3D11RasterizerState2(pState, ret, this);
}
AddResource(pState, ResourceType::StateObject, "Rasterizer State");
+12 -40
View File
@@ -2629,16 +2629,9 @@ bool WrappedID3D11Device::Serialise_CreateBlendState(SerialiserType &ser,
}
else
{
if(GetResourceManager()->HasWrapper(ret))
{
ret->Release();
ret = (ID3D11BlendState *)GetResourceManager()->GetWrapper(ret);
ret->AddRef();
}
else
{
ret = new WrappedID3D11BlendState1(pState, ret, this);
}
GetResourceManager()->OverrideWrapper(ret);
ret = new WrappedID3D11BlendState1(pState, ret, this);
}
AddResource(pState, ResourceType::StateObject, "Blend State");
@@ -2740,16 +2733,9 @@ bool WrappedID3D11Device::Serialise_CreateDepthStencilState(
}
else
{
if(GetResourceManager()->HasWrapper(ret))
{
ret->Release();
ret = (ID3D11DepthStencilState *)GetResourceManager()->GetWrapper(ret);
ret->AddRef();
}
else
{
ret = new WrappedID3D11DepthStencilState(pState, ret, this);
}
GetResourceManager()->OverrideWrapper(ret);
ret = new WrappedID3D11DepthStencilState(pState, ret, this);
}
AddResource(pState, ResourceType::StateObject, "Depth-Stencil State");
@@ -2849,16 +2835,9 @@ bool WrappedID3D11Device::Serialise_CreateRasterizerState(SerialiserType &ser,
}
else
{
if(GetResourceManager()->HasWrapper(ret))
{
ret->Release();
ret = (ID3D11RasterizerState *)GetResourceManager()->GetWrapper(ret);
ret->AddRef();
}
else
{
ret = new WrappedID3D11RasterizerState2(pState, ret, this);
}
GetResourceManager()->OverrideWrapper(ret);
ret = new WrappedID3D11RasterizerState2(pState, ret, this);
}
AddResource(pState, ResourceType::StateObject, "Rasterizer State");
@@ -2958,16 +2937,9 @@ bool WrappedID3D11Device::Serialise_CreateSamplerState(SerialiserType &ser,
}
else
{
if(GetResourceManager()->HasWrapper(ret))
{
ret->Release();
ret = (ID3D11SamplerState *)GetResourceManager()->GetWrapper(ret);
ret->AddRef();
}
else
{
ret = new WrappedID3D11SamplerState(pState, ret, this);
}
GetResourceManager()->OverrideWrapper(ret);
ret = new WrappedID3D11SamplerState(pState, ret, this);
}
AddResource(pState, ResourceType::Sampler, "Sampler State");
+1 -1
View File
@@ -1458,7 +1458,7 @@ WrappedID3D12GraphicsCommandList::~WrappedID3D12GraphicsCommandList()
m_UnusedCleanupCallbacks.clear();
if(m_pList)
m_pDevice->GetResourceManager()->RemoveWrapper(m_pList);
m_pDevice->GetResourceManager()->RemoveWrapper(this, m_pList);
if(m_CreationRecord)
m_CreationRecord->Delete(m_pDevice->GetResourceManager());
+3 -7
View File
@@ -1211,14 +1211,8 @@ bool WrappedID3D12Device::Serialise_CreateRootSignature(SerialiserType &ser, UIN
else
{
// we deduplicated during capture but this could alias one of ours in theory
if(GetResourceManager()->HasWrapper(ret))
{
ret->Release();
ret = (ID3D12RootSignature *)GetResourceManager()->GetWrapper(ret);
GetResourceManager()->OverrideWrapper(ret);
GetResourceManager()->ReplaceResource(pRootSignature, GetResID(ret));
}
else
{
ret = new WrappedID3D12RootSignature(pRootSignature, ret, this);
@@ -1946,6 +1940,8 @@ bool WrappedID3D12Device::Serialise_CreateCommandSignature(SerialiserType &ser,
}
else
{
GetResourceManager()->OverrideWrapper(ret);
WrappedID3D12CommandSignature *wrapped =
new WrappedID3D12CommandSignature(pCommandSignature, ret, this, Descriptor);
+3 -10
View File
@@ -108,16 +108,9 @@ bool WrappedID3D12Device::Serialise_CreateRootSignatureFromSubobjectInLibrary(
}
else
{
if(GetResourceManager()->HasWrapper(ret))
{
ret->Release();
ret = (ID3D12RootSignature *)GetResourceManager()->GetWrapper(ret);
ret->AddRef();
}
else
{
ret = new WrappedID3D12RootSignature(pRootSignature, ret, this);
}
GetResourceManager()->OverrideWrapper(ret);
ret = new WrappedID3D12RootSignature(pRootSignature, ret, this);
WrappedID3D12RootSignature *wrapped = (WrappedID3D12RootSignature *)ret;
+1 -1
View File
@@ -102,7 +102,7 @@ protected:
void Shutdown()
{
if(m_pReal)
m_pDevice->GetResourceManager()->RemoveWrapper(m_pReal);
m_pDevice->GetResourceManager()->RemoveWrapper(this, m_pReal);
m_pDevice->GetResourceManager()->ReleaseResource(GetResourceID());
m_pDevice->ReleaseResource((NestedType *)this);
+1 -1
View File
@@ -376,7 +376,7 @@ public:
if(IsReplayMode(m_State))
{
ResourceManager::RemoveWrapper(ToTypedHandle(Unwrap(obj)));
ResourceManager::RemoveWrapper(GetWrapped(obj), ToTypedHandle(Unwrap(obj)));
}
ResourceManager::ReleaseResource(id);
@@ -1784,18 +1784,8 @@ bool WrappedVulkan::Serialise_vkCreateDescriptorSetLayout(
{
ResourceId live;
if(GetResourceManager()->HasWrapper(ToTypedHandle(layout)))
{
live = GetResourceManager()->GetNonDispWrapper(layout)->id;
GetResourceManager()->OverrideWrapper(ToTypedHandle(layout));
// destroy this instance of the duplicate, as we must have matching create/destroy
// calls and there won't be a wrapped resource hanging around to destroy this one.
ObjDisp(device)->DestroyDescriptorSetLayout(Unwrap(device), layout, NULL);
// whenever the new ID is requested, return the old ID, via replacements.
GetResourceManager()->ReplaceResource(SetLayout, live);
}
else
{
live = GetResourceManager()->WrapResource(SetLayout, Unwrap(device), layout);
@@ -534,14 +534,14 @@ bool WrappedVulkan::ReleaseResource(WrappedVkRes *res)
if(IsReplayMode(m_State))
{
GetResourceManager()->ReleaseResource(disp->id);
GetResourceManager()->RemoveWrapper(ToTypedHandle(disp->real.As<VkDevice>()));
GetResourceManager()->RemoveWrapper(res, ToTypedHandle(disp->real.As<VkDevice>()));
}
break;
case eResInstance:
if(IsReplayMode(m_State))
{
GetResourceManager()->ReleaseResource(disp->id);
GetResourceManager()->RemoveWrapper(ToTypedHandle(disp->real.As<VkInstance>()));
GetResourceManager()->RemoveWrapper(res, ToTypedHandle(disp->real.As<VkInstance>()));
}
break;
@@ -750,18 +750,8 @@ bool WrappedVulkan::Serialise_vkCreateSampler(SerialiserType &ser, VkDevice devi
{
ResourceId live;
if(GetResourceManager()->HasWrapper(ToTypedHandle(samp)))
{
live = GetResourceManager()->GetNonDispWrapper(samp)->id;
GetResourceManager()->OverrideWrapper(ToTypedHandle(samp));
// destroy this instance of the duplicate, as we must have matching create/destroy
// calls and there won't be a wrapped resource hanging around to destroy this one.
ObjDisp(device)->DestroySampler(Unwrap(device), samp, NULL);
// whenever the new ID is requested, return the old ID, via replacements.
GetResourceManager()->ReplaceResource(Sampler, live);
}
else
{
live = GetResourceManager()->WrapResource(Sampler, Unwrap(device), samp);
@@ -966,18 +956,8 @@ bool WrappedVulkan::Serialise_vkCreateFramebuffer(SerialiserType &ser, VkDevice
{
ResourceId live;
if(GetResourceManager()->HasWrapper(ToTypedHandle(fb)))
{
live = GetResourceManager()->GetNonDispWrapper(fb)->id;
GetResourceManager()->OverrideWrapper(ToTypedHandle(fb));
// destroy this instance of the duplicate, as we must have matching create/destroy
// calls and there won't be a wrapped resource hanging around to destroy this one.
ObjDisp(device)->DestroyFramebuffer(Unwrap(device), fb, NULL);
// whenever the new ID is requested, return the old ID, via replacements.
GetResourceManager()->ReplaceResource(Framebuffer, live);
}
else
{
live = GetResourceManager()->WrapResource(Framebuffer, Unwrap(device), fb);
@@ -1255,18 +1235,8 @@ bool WrappedVulkan::Serialise_vkCreateRenderPass(SerialiserType &ser, VkDevice d
{
ResourceId live;
if(GetResourceManager()->HasWrapper(ToTypedHandle(rp)))
{
live = GetResourceManager()->GetNonDispWrapper(rp)->id;
GetResourceManager()->OverrideWrapper(ToTypedHandle(rp));
// destroy this instance of the duplicate, as we must have matching create/destroy
// calls and there won't be a wrapped resource hanging around to destroy this one.
ObjDisp(device)->DestroyRenderPass(Unwrap(device), rp, NULL);
// whenever the new ID is requested, return the old ID, via replacements.
GetResourceManager()->ReplaceResource(RenderPass, live);
}
else
{
live = GetResourceManager()->WrapResource(RenderPass, Unwrap(device), rp);
@@ -1554,18 +1524,8 @@ bool WrappedVulkan::Serialise_vkCreateRenderPass2(SerialiserType &ser, VkDevice
{
ResourceId live;
if(GetResourceManager()->HasWrapper(ToTypedHandle(rp)))
{
live = GetResourceManager()->GetNonDispWrapper(rp)->id;
GetResourceManager()->OverrideWrapper(ToTypedHandle(rp));
// destroy this instance of the duplicate, as we must have matching create/destroy
// calls and there won't be a wrapped resource hanging around to destroy this one.
ObjDisp(device)->DestroyRenderPass(Unwrap(device), rp, NULL);
// whenever the new ID is requested, return the old ID, via replacements.
GetResourceManager()->ReplaceResource(RenderPass, live);
}
else
{
live = GetResourceManager()->WrapResource(RenderPass, Unwrap(device), rp);
@@ -2284,18 +2244,8 @@ bool WrappedVulkan::Serialise_vkCreateSamplerYcbcrConversion(
{
ResourceId live;
if(GetResourceManager()->HasWrapper(ToTypedHandle(conv)))
{
live = GetResourceManager()->GetNonDispWrapper(conv)->id;
GetResourceManager()->OverrideWrapper(ToTypedHandle(conv));
// destroy this instance of the duplicate, as we must have matching create/destroy
// calls and there won't be a wrapped resource hanging around to destroy this one.
ObjDisp(device)->DestroySamplerYcbcrConversion(Unwrap(device), conv, NULL);
// whenever the new ID is requested, return the old ID, via replacements.
GetResourceManager()->ReplaceResource(ycbcrConversion, live);
}
else
{
live = GetResourceManager()->WrapResource(ycbcrConversion, Unwrap(device), conv);
@@ -55,14 +55,8 @@ bool WrappedVulkan::Serialise_vkGetDeviceQueue(SerialiserType &ser, VkDevice dev
ObjDisp(device)->GetDeviceQueue(Unwrap(device), remapFamily, remapIndex, &queue);
if(GetResourceManager()->HasWrapper(ToTypedHandle(queue)))
{
ResourceId live = GetResourceManager()->GetDispWrapper(queue)->id;
GetResourceManager()->OverrideWrapper(ToTypedHandle(queue));
// whenever the new ID is requested, return the old ID, via replacements.
GetResourceManager()->ReplaceResource(Queue, live);
}
else
{
GetResourceManager()->WrapResource(Queue, Unwrap(device), queue);
}
@@ -2393,18 +2393,8 @@ bool WrappedVulkan::Serialise_vkCreateBufferView(SerialiserType &ser, VkDevice d
{
ResourceId live;
if(GetResourceManager()->HasWrapper(ToTypedHandle(view)))
{
live = GetResourceManager()->GetNonDispWrapper(view)->id;
GetResourceManager()->OverrideWrapper(ToTypedHandle(view));
// destroy this instance of the duplicate, as we must have matching create/destroy
// calls and there won't be a wrapped resource hanging around to destroy this one.
ObjDisp(device)->DestroyBufferView(Unwrap(device), view, NULL);
// whenever the new ID is requested, return the old ID, via replacements.
GetResourceManager()->ReplaceResource(View, live);
}
else
{
live = GetResourceManager()->WrapResource(View, Unwrap(device), view);
@@ -3326,18 +3316,8 @@ bool WrappedVulkan::Serialise_vkCreateImageView(SerialiserType &ser, VkDevice de
{
ResourceId live;
if(GetResourceManager()->HasWrapper(ToTypedHandle(view)))
{
live = GetResourceManager()->GetNonDispWrapper(view)->id;
GetResourceManager()->OverrideWrapper(ToTypedHandle(view));
// destroy this instance of the duplicate, as we must have matching create/destroy
// calls and there won't be a wrapped resource hanging around to destroy this one.
ObjDisp(device)->DestroyImageView(Unwrap(device), view, NULL);
// whenever the new ID is requested, return the old ID, via replacements.
GetResourceManager()->ReplaceResource(View, live);
}
else
{
live = GetResourceManager()->WrapResource(View, Unwrap(device), view);
@@ -3976,18 +3956,8 @@ bool WrappedVulkan::Serialise_vkCreateAccelerationStructureKHR(
{
ResourceId live;
if(GetResourceManager()->HasWrapper(ToTypedHandle(acc)))
{
live = GetResourceManager()->GetNonDispWrapper(acc)->id;
GetResourceManager()->OverrideWrapper(ToTypedHandle(acc));
// destroy this instance of the duplicate, as we must have matching create/destroy
// calls and there won't be a wrapped resource hanging around to destroy this one.
ObjDisp(device)->DestroyAccelerationStructureKHR(Unwrap(device), acc, NULL);
// whenever the new ID is requested, return the old ID, via replacements.
GetResourceManager()->ReplaceResource(AccelerationStructure, live);
}
else
{
live = GetResourceManager()->WrapResource(AccelerationStructure, Unwrap(device), acc);
@@ -295,18 +295,8 @@ bool WrappedVulkan::Serialise_vkCreatePipelineLayout(SerialiserType &ser, VkDevi
{
ResourceId live;
if(GetResourceManager()->HasWrapper(ToTypedHandle(layout)))
{
live = GetResourceManager()->GetNonDispWrapper(layout)->id;
GetResourceManager()->OverrideWrapper(ToTypedHandle(layout));
// destroy this instance of the duplicate, as we must have matching create/destroy
// calls and there won't be a wrapped resource hanging around to destroy this one.
ObjDisp(device)->DestroyPipelineLayout(Unwrap(device), layout, NULL);
// whenever the new ID is requested, return the old ID, via replacements.
GetResourceManager()->ReplaceResource(PipelineLayout, live);
}
else
{
live = GetResourceManager()->WrapResource(PipelineLayout, Unwrap(device), layout);
@@ -420,18 +410,8 @@ bool WrappedVulkan::Serialise_vkCreateShaderModule(SerialiserType &ser, VkDevice
{
ResourceId live;
if(GetResourceManager()->HasWrapper(ToTypedHandle(sh)))
{
live = GetResourceManager()->GetNonDispWrapper(sh)->id;
GetResourceManager()->OverrideWrapper(ToTypedHandle(sh));
// destroy this instance of the duplicate, as we must have matching create/destroy
// calls and there won't be a wrapped resource hanging around to destroy this one.
ObjDisp(device)->DestroyShaderModule(Unwrap(device), sh, NULL);
// whenever the new ID is requested, return the old ID, via replacements.
GetResourceManager()->ReplaceResource(ShaderModule, live);
}
else
{
live = GetResourceManager()->WrapResource(ShaderModule, Unwrap(device), sh);
@@ -520,18 +500,9 @@ bool WrappedVulkan::Serialise_vkCreateShadersEXT(SerialiserType &ser, VkDevice d
else
{
ResourceId live;
if(GetResourceManager()->HasWrapper(ToTypedHandle(sh)))
{
live = GetResourceManager()->GetNonDispWrapper(sh)->id;
// destroy this instance of the duplicate, as we must have matching create/destroy
// calls and there won't be a wrapped resource hanging around to destroy this one.
ObjDisp(device)->DestroyShaderEXT(Unwrap(device), sh, NULL);
GetResourceManager()->OverrideWrapper(ToTypedHandle(sh));
// whenever the new ID is requested, return the old ID, via replacements.
GetResourceManager()->ReplaceResource(Shader, live);
}
else
{
live = GetResourceManager()->WrapResource(Shader, Unwrap(device), sh);
@@ -555,22 +555,8 @@ bool WrappedVulkan::Serialise_vkCreateSemaphore(SerialiserType &ser, VkDevice de
{
ResourceId live;
if(GetResourceManager()->HasWrapper(ToTypedHandle(sem)))
{
live = GetResourceManager()->GetNonDispWrapper(sem)->id;
GetResourceManager()->OverrideWrapper(ToTypedHandle(sem));
RDCWARN(
"On replay, semaphore got a duplicate handle - maybe a bug, or it could be an "
"indication of an implementation that doesn't use semaphores");
// destroy this instance of the duplicate, as we must have matching create/destroy
// calls and there won't be a wrapped resource hanging around to destroy this one.
ObjDisp(device)->DestroySemaphore(Unwrap(device), sem, NULL);
// whenever the new ID is requested, return the old ID, via replacements.
GetResourceManager()->ReplaceResource(Semaphore, live);
}
else
{
live = GetResourceManager()->WrapResource(Semaphore, Unwrap(device), sem);
}
@@ -84,6 +84,15 @@ float4 main(v2f IN) : SV_Target0
ctx1->SwapDeviceContextState(ctxstate_off, NULL);
D3D11_RASTERIZER_DESC rastDesc = {
D3D11_FILL_SOLID, D3D11_CULL_NONE, FALSE, 0, 0.00, 0.00, FALSE, TRUE, FALSE, FALSE,
};
// this is expected to alias a renderdoc-internal state
ID3D11RasterizerStatePtr rastStateObj;
dev->CreateRasterizerState(&rastDesc, &rastStateObj);
SetDebugName(rastStateObj, "RastState");
std::string features1_tiled_resources("Features1: D3D11_TILED_RESOURCES_SUPPORTED");
std::string features2_tiled_resources("Features2: D3D11_TILED_RESOURCES_SUPPORTED");
std::string create_tiled_buffer("CreateTiledBuffer: Passed");
@@ -199,6 +208,11 @@ float4 main(v2f IN) : SV_Target0
setMarker(create_tiled_texture2D);
setMarker(create_tiled_texture2D1);
ctx->RSSetState(rastStateObj);
setMarker("RastState");
ctx->Draw(3, 0);
Present();
// get back to how we should be with a handle to ctxstate and ctxstate_off bound
@@ -65,4 +65,14 @@ class D3D11_Parameter_Zoo(rdtest.TestCase):
out.Shutdown()
action = self.find_action("RastState")
self.check(action is not None)
self.controller.SetFrameEvent(action.eventId, False)
pipe11 = self.controller.GetD3D11PipelineState()
self.check(pipe11.rasterizer.state.resourceId != rd.ResourceId())
self.check(self.get_resource(pipe11.rasterizer.state.resourceId).name == "RastState")
rdtest.log.success("Overlay color is as expected")