From 1c190f0eb54b076fccb19a4e36f0e836892864a8 Mon Sep 17 00:00:00 2001 From: baldurk Date: Fri, 12 May 2017 16:45:06 +0100 Subject: [PATCH] Don't change values of m_ImmediatePipeline/m_pDevice when assigning * This fixes an issue where applying a ID3DDeviceContextState by copying the state it has saved would trash these members on the immediate context's state tracker, and lead to missing refcounting and possibly a crash if an error was later encountered. * Since this would be confusing to do in an operator=, we instead remove the operator and explicitly call a CopyState() function --- renderdoc/driver/d3d11/d3d11_context.cpp | 4 +-- .../driver/d3d11/d3d11_context1_wrap.cpp | 31 ++++++++++++------- renderdoc/driver/d3d11/d3d11_debug.cpp | 2 +- renderdoc/driver/d3d11/d3d11_device1_wrap.cpp | 2 +- renderdoc/driver/d3d11/d3d11_renderstate.cpp | 10 ++---- renderdoc/driver/d3d11/d3d11_renderstate.h | 7 ++++- 6 files changed, 32 insertions(+), 24 deletions(-) diff --git a/renderdoc/driver/d3d11/d3d11_context.cpp b/renderdoc/driver/d3d11/d3d11_context.cpp index 6485aace1..5873b43b0 100644 --- a/renderdoc/driver/d3d11/d3d11_context.cpp +++ b/renderdoc/driver/d3d11/d3d11_context.cpp @@ -230,7 +230,7 @@ bool WrappedID3D11DeviceContext::Serialise_BeginCaptureFrame(bool applyInitialSt if(m_State >= WRITING) { - state = *m_CurrentPipelineState; + state.CopyState(*m_CurrentPipelineState); state.SetSerialiser(m_pSerialiser); @@ -243,7 +243,7 @@ bool WrappedID3D11DeviceContext::Serialise_BeginCaptureFrame(bool applyInitialSt { m_DoStateVerify = false; { - *m_CurrentPipelineState = state; + m_CurrentPipelineState->CopyState(state); m_CurrentPipelineState->SetDevice(m_pDevice); state.ApplyState(this); } diff --git a/renderdoc/driver/d3d11/d3d11_context1_wrap.cpp b/renderdoc/driver/d3d11/d3d11_context1_wrap.cpp index c4b2a64d6..79d76eaca 100644 --- a/renderdoc/driver/d3d11/d3d11_context1_wrap.cpp +++ b/renderdoc/driver/d3d11/d3d11_context1_wrap.cpp @@ -2012,7 +2012,8 @@ bool WrappedID3D11DeviceContext::Serialise_SwapDeviceContextState( if(m_State >= WRITING) { - state = *((WrappedID3DDeviceContextState *)pState)->state; + WrappedID3DDeviceContextState *wrapped = (WrappedID3DDeviceContextState *)pState; + state.CopyState(*wrapped->state); state.SetSerialiser(m_pSerialiser); @@ -2025,7 +2026,7 @@ bool WrappedID3D11DeviceContext::Serialise_SwapDeviceContextState( { m_DoStateVerify = false; { - *m_CurrentPipelineState = state; + m_CurrentPipelineState->CopyState(state); m_CurrentPipelineState->SetDevice(m_pDevice); state.ApplyState(this); } @@ -2046,20 +2047,26 @@ void WrappedID3D11DeviceContext::SwapDeviceContextState(ID3DDeviceContextState * m_pRealContext1->SwapDeviceContextState(UNWRAP(WrappedID3DDeviceContextState, pState), &prev); - WrappedID3DDeviceContextState *wrapped = NULL; + { + WrappedID3DDeviceContextState *wrapped = NULL; - if(m_pDevice->GetResourceManager()->HasWrapper(prev)) - wrapped = (WrappedID3DDeviceContextState *)m_pDevice->GetResourceManager()->GetWrapper(prev); - else if(prev) - wrapped = new WrappedID3DDeviceContextState(prev, m_pDevice); + if(m_pDevice->GetResourceManager()->HasWrapper(prev)) + wrapped = (WrappedID3DDeviceContextState *)m_pDevice->GetResourceManager()->GetWrapper(prev); + else if(prev) + wrapped = new WrappedID3DDeviceContextState(prev, m_pDevice); - if(wrapped) - *wrapped->state = *m_CurrentPipelineState; + if(wrapped) + wrapped->state->CopyState(*m_CurrentPipelineState); - if(ppPreviousState) - *ppPreviousState = wrapped; + if(ppPreviousState) + *ppPreviousState = wrapped; + } - *m_CurrentPipelineState = *((WrappedID3DDeviceContextState *)pState)->state; + { + WrappedID3DDeviceContextState *wrapped = (WrappedID3DDeviceContextState *)pState; + + m_CurrentPipelineState->CopyState(*wrapped->state); + } DrainAnnotationQueue(); diff --git a/renderdoc/driver/d3d11/d3d11_debug.cpp b/renderdoc/driver/d3d11/d3d11_debug.cpp index bd57dd797..b990d4e5f 100644 --- a/renderdoc/driver/d3d11/d3d11_debug.cpp +++ b/renderdoc/driver/d3d11/d3d11_debug.cpp @@ -1698,7 +1698,7 @@ void D3D11DebugManager::BindOutputWindow(uint64_t id, bool depth) RDCERR("Trashing RealState! Mismatched use of BindOutputWindow / FlipOutputWindow"); m_RealState.active = true; - m_RealState.state = *m_WrappedContext->GetCurrentPipelineState(); + m_RealState.state.CopyState(*m_WrappedContext->GetCurrentPipelineState()); m_WrappedContext->OMSetRenderTargets( 1, &m_OutputWindows[id].rtv, depth && m_OutputWindows[id].dsv ? m_OutputWindows[id].dsv : NULL); diff --git a/renderdoc/driver/d3d11/d3d11_device1_wrap.cpp b/renderdoc/driver/d3d11/d3d11_device1_wrap.cpp index ed7f2def0..98740c73a 100644 --- a/renderdoc/driver/d3d11/d3d11_device1_wrap.cpp +++ b/renderdoc/driver/d3d11/d3d11_device1_wrap.cpp @@ -251,7 +251,7 @@ HRESULT WrappedID3D11Device::CreateDeviceContextState(UINT Flags, WrappedID3DDeviceContextState *wrapped = new WrappedID3DDeviceContextState(real, this); - *wrapped->state = *m_pImmediateContext->GetCurrentPipelineState(); + wrapped->state->CopyState(*m_pImmediateContext->GetCurrentPipelineState()); *ppContextState = wrapped; } diff --git a/renderdoc/driver/d3d11/d3d11_renderstate.cpp b/renderdoc/driver/d3d11/d3d11_renderstate.cpp index 9fa383b3c..1bc096865 100644 --- a/renderdoc/driver/d3d11/d3d11_renderstate.cpp +++ b/renderdoc/driver/d3d11/d3d11_renderstate.cpp @@ -61,13 +61,14 @@ D3D11RenderState::D3D11RenderState(const D3D11RenderState &other) RDCEraseEl(OM); RDCEraseEl(CS); RDCEraseEl(CSUAVs); - *this = other; m_ImmediatePipeline = false; m_pDevice = NULL; + + CopyState(other); } -D3D11RenderState &D3D11RenderState::operator=(const D3D11RenderState &other) +void D3D11RenderState::CopyState(const D3D11RenderState &other) { ReleaseRefs(); @@ -83,12 +84,7 @@ D3D11RenderState &D3D11RenderState::operator=(const D3D11RenderState &other) memcpy(&CS, &other.CS, sizeof(CS)); memcpy(&CSUAVs, &other.CSUAVs, sizeof(CSUAVs)); - m_ImmediatePipeline = false; - m_pDevice = NULL; - AddRefs(); - - return *this; } D3D11RenderState::~D3D11RenderState() diff --git a/renderdoc/driver/d3d11/d3d11_renderstate.h b/renderdoc/driver/d3d11/d3d11_renderstate.h index 5909b8a3f..d8b7c56b6 100644 --- a/renderdoc/driver/d3d11/d3d11_renderstate.h +++ b/renderdoc/driver/d3d11/d3d11_renderstate.h @@ -41,7 +41,12 @@ struct D3D11RenderState D3D11RenderState(const D3D11RenderState &other); ~D3D11RenderState(); - D3D11RenderState &operator=(const D3D11RenderState &other); + // we don't allow operator = since we want to preserve some properties. + // Instead use CopyState() which copies all of the state contained without + // modifying the device pointer or immediate pipeline flag. + D3D11RenderState &operator=(const D3D11RenderState &other) = delete; + + void CopyState(const D3D11RenderState &other); void ApplyState(WrappedID3D11DeviceContext *context); void Clear();