Skip to content

Commit

Permalink
When binding resources for write, unbind all other write uses. Refs #503
Browse files Browse the repository at this point in the history 


* The D3D11 hazard tracking rules are then as follows:
  - When binding a resource for read, if it is bound for write anywhere
    then NULL is bound instead. If it's not, then any other read
    references are preserved.
  - When binding a resource for write, all other binds for read AND
    write are forced to zero.
  - The exception to the above: If a DSV is bound with depth read-only
    then any read binds reading the depth channel remain bound. Likewise
    for stencil read-only and stencil channel read binds.
  - Special case: When binding RTVs and UAVs at the same time, if there
    is a write overlap within that bind, the entire bind gets discarded
    (including any non-overlapping binds) and state remains unchanged.
  - When considering if a bind overlaps, it only overlaps if the view
    in question covers an overlapping subresource range. In other words,
    it is valid to have a write bind of mip 1 and a read bind of mip 0
    at the same time (the classic case being mip generation).
* This commit fixes the second point, other write binds weren't being
  NULL'd out. So binding a UAV to slot 0 then slot 1 in that order will
  cause slot 0 to become unbound. Or binding to an RTV will unbind a
  UAV or vice-versa.
  • Loading branch information
@baldurk
baldurk committed Feb 1, 2017
1 parent 477df04 commit 9d6acd1
Show file tree
Hide file tree
Showing 3 changed files with 220 additions and 12 deletions.
49 changes: 39 additions & 10 deletions renderdoc/driver/d3d11/d3d11_context_wrap.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3131,7 +3131,7 @@ bool WrappedID3D11DeviceContext::Serialise_OMSetRenderTargets(
pDepthStencilView = (ID3D11DepthStencilView *)m_pDevice->GetResourceManager()->GetLiveResource(
DepthStencilView);

if(m_CurrentPipelineState->ValidOutputMerger(RenderTargetViews, pDepthStencilView))
if(m_CurrentPipelineState->ValidOutputMerger(RenderTargetViews, pDepthStencilView, NULL))
{
m_CurrentPipelineState->ChangeRefWrite(m_CurrentPipelineState->OM.RenderTargets,
RenderTargetViews, 0,
Expand Down Expand Up @@ -3183,7 +3183,7 @@ void WrappedID3D11DeviceContext::OMSetRenderTargets(UINT NumViews,
RTs[i] = ppRenderTargetViews[i];

// this function always sets all render targets
if(m_CurrentPipelineState->ValidOutputMerger(RTs, pDepthStencilView))
if(m_CurrentPipelineState->ValidOutputMerger(RTs, pDepthStencilView, NULL))
{
m_CurrentPipelineState->ChangeRefWrite(m_CurrentPipelineState->OM.RenderTargets, RTs, 0,
D3D11_SIMULTANEOUS_RENDER_TARGET_COUNT);
Expand Down Expand Up @@ -3320,7 +3320,11 @@ bool WrappedID3D11DeviceContext::Serialise_OMSetRenderTargetsAndUnorderedAccessV

if(NumRTVs != D3D11_KEEP_RENDER_TARGETS_AND_DEPTH_STENCIL)
{
if(m_CurrentPipelineState->ValidOutputMerger(RenderTargetViews, pDepthStencilView))
ID3D11UnorderedAccessView **srcUAVs = NumUAVs != D3D11_KEEP_UNORDERED_ACCESS_VIEWS
? UnorderedAccessViews
: m_CurrentPipelineState->OM.UAVs;

if(m_CurrentPipelineState->ValidOutputMerger(RenderTargetViews, pDepthStencilView, srcUAVs))
{
m_CurrentPipelineState->ChangeRefWrite(m_CurrentPipelineState->OM.RenderTargets,
RenderTargetViews, 0,
Expand All @@ -3332,9 +3336,21 @@ bool WrappedID3D11DeviceContext::Serialise_OMSetRenderTargetsAndUnorderedAccessV

if(NumUAVs != D3D11_KEEP_UNORDERED_ACCESS_VIEWS)
{
m_CurrentPipelineState->ChangeRefWrite(m_CurrentPipelineState->OM.UAVs, UnorderedAccessViews,
0, D3D11_1_UAV_SLOT_COUNT);
m_CurrentPipelineState->Change(m_CurrentPipelineState->OM.UAVStartSlot, UAVStartSlot);
bool valid = false;
if(NumRTVs != D3D11_KEEP_RENDER_TARGETS_AND_DEPTH_STENCIL)
valid = m_CurrentPipelineState->ValidOutputMerger(RenderTargetViews, pDepthStencilView,
UnorderedAccessViews);
else
valid = m_CurrentPipelineState->ValidOutputMerger(m_CurrentPipelineState->OM.RenderTargets,
m_CurrentPipelineState->OM.DepthView,
UnorderedAccessViews);

if(valid)
{
m_CurrentPipelineState->ChangeRefWrite(m_CurrentPipelineState->OM.UAVs,
UnorderedAccessViews, 0, D3D11_1_UAV_SLOT_COUNT);
m_CurrentPipelineState->Change(m_CurrentPipelineState->OM.UAVStartSlot, UAVStartSlot);
}
}

for(UINT i = 0; i < NumRTVs && NumRTVs != D3D11_KEEP_RENDER_TARGETS_AND_DEPTH_STENCIL; i++)
Expand Down Expand Up @@ -3403,7 +3419,10 @@ void WrappedID3D11DeviceContext::OMSetRenderTargetsAndUnorderedAccessViews(

if(NumRTVs != D3D11_KEEP_RENDER_TARGETS_AND_DEPTH_STENCIL)
{
if(m_CurrentPipelineState->ValidOutputMerger(RTs, pDepthStencilView))
ID3D11UnorderedAccessView **srcUAVs =
NumUAVs != D3D11_KEEP_UNORDERED_ACCESS_VIEWS ? UAVs : m_CurrentPipelineState->OM.UAVs;

if(m_CurrentPipelineState->ValidOutputMerger(RTs, pDepthStencilView, srcUAVs))
{
m_CurrentPipelineState->ChangeRefWrite(m_CurrentPipelineState->OM.RenderTargets, RTs, 0,
D3D11_SIMULTANEOUS_RENDER_TARGET_COUNT);
Expand Down Expand Up @@ -3439,9 +3458,19 @@ void WrappedID3D11DeviceContext::OMSetRenderTargetsAndUnorderedAccessViews(

if(NumUAVs != D3D11_KEEP_UNORDERED_ACCESS_VIEWS)
{
m_CurrentPipelineState->ChangeRefWrite(m_CurrentPipelineState->OM.UAVs, UAVs, 0,
D3D11_1_UAV_SLOT_COUNT);
m_CurrentPipelineState->Change(m_CurrentPipelineState->OM.UAVStartSlot, UAVStartSlot);
bool valid = false;
if(NumRTVs != D3D11_KEEP_RENDER_TARGETS_AND_DEPTH_STENCIL)
valid = m_CurrentPipelineState->ValidOutputMerger(RTs, pDepthStencilView, UAVs);
else
valid = m_CurrentPipelineState->ValidOutputMerger(m_CurrentPipelineState->OM.RenderTargets,
m_CurrentPipelineState->OM.DepthView, UAVs);

if(valid)
{
m_CurrentPipelineState->ChangeRefWrite(m_CurrentPipelineState->OM.UAVs, UAVs, 0,
D3D11_1_UAV_SLOT_COUNT);
m_CurrentPipelineState->Change(m_CurrentPipelineState->OM.UAVStartSlot, UAVStartSlot);
}
}

// invalid case where UAV/RTV overlap, UAV seems to take precedence
Expand Down
135 changes: 134 additions & 1 deletion renderdoc/driver/d3d11/d3d11_renderstate.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1212,7 +1212,8 @@ void D3D11RenderState::UnbindIUnknownForRead(const ResourceRange &range, bool al
}
}

bool D3D11RenderState::ValidOutputMerger(ID3D11RenderTargetView **RTs, ID3D11DepthStencilView *depth)
bool D3D11RenderState::ValidOutputMerger(ID3D11RenderTargetView **RTs, ID3D11DepthStencilView *depth,
ID3D11UnorderedAccessView **uavs)
{
D3D11_RENDER_TARGET_VIEW_DESC RTDescs[D3D11_SIMULTANEOUS_RENDER_TARGET_COUNT];
D3D11_DEPTH_STENCIL_VIEW_DESC DepthDesc;
Expand Down Expand Up @@ -1246,6 +1247,135 @@ bool D3D11RenderState::ValidOutputMerger(ID3D11RenderTargetView **RTs, ID3D11Dep

bool valid = true;

// check for duplicates and mark as invalid
{
ResourceRange rtvRanges[D3D11_SIMULTANEOUS_RENDER_TARGET_COUNT] = {
ResourceRange::Null, ResourceRange::Null, ResourceRange::Null, ResourceRange::Null,
ResourceRange::Null, ResourceRange::Null, ResourceRange::Null, ResourceRange::Null,
};
ResourceRange depthRange(depth);
ResourceRange uavRanges[D3D11_1_UAV_SLOT_COUNT] = {
ResourceRange::Null, ResourceRange::Null, ResourceRange::Null, ResourceRange::Null,
ResourceRange::Null, ResourceRange::Null, ResourceRange::Null, ResourceRange::Null,
ResourceRange::Null, ResourceRange::Null, ResourceRange::Null, ResourceRange::Null,
ResourceRange::Null, ResourceRange::Null, ResourceRange::Null, ResourceRange::Null,
ResourceRange::Null, ResourceRange::Null, ResourceRange::Null, ResourceRange::Null,
ResourceRange::Null, ResourceRange::Null, ResourceRange::Null, ResourceRange::Null,
ResourceRange::Null, ResourceRange::Null, ResourceRange::Null, ResourceRange::Null,
ResourceRange::Null, ResourceRange::Null, ResourceRange::Null, ResourceRange::Null,
ResourceRange::Null, ResourceRange::Null, ResourceRange::Null, ResourceRange::Null,
ResourceRange::Null, ResourceRange::Null, ResourceRange::Null, ResourceRange::Null,
ResourceRange::Null, ResourceRange::Null, ResourceRange::Null, ResourceRange::Null,
ResourceRange::Null, ResourceRange::Null, ResourceRange::Null, ResourceRange::Null,
ResourceRange::Null, ResourceRange::Null, ResourceRange::Null, ResourceRange::Null,
ResourceRange::Null, ResourceRange::Null, ResourceRange::Null, ResourceRange::Null,
ResourceRange::Null, ResourceRange::Null, ResourceRange::Null, ResourceRange::Null,
ResourceRange::Null, ResourceRange::Null, ResourceRange::Null, ResourceRange::Null,
};

for(int i = 0; RTs && i < D3D11_SIMULTANEOUS_RENDER_TARGET_COUNT; i++)
{
if(RTs[i])
rtvRanges[i] = ResourceRange(RTs[i]);
else
break;
}

if(depth)
depthRange = ResourceRange(depth);

int numUAVs = 0;

for(int i = 0; uavs && i < D3D11_1_UAV_SLOT_COUNT; i++)
{
if(uavs[i])
{
uavRanges[i] = ResourceRange(uavs[i]);
numUAVs = i + 1;
}
}

// since constants are low, just do naive check for any intersecting ranges

for(int i = 0; i < D3D11_SIMULTANEOUS_RENDER_TARGET_COUNT; i++)
{
if(rtvRanges[i].IsNull())
continue;

// does it match any other RTV?
for(int j = 0; j < D3D11_SIMULTANEOUS_RENDER_TARGET_COUNT; j++)
{
if(i == j)
continue;

if(rtvRanges[i].Intersects(rtvRanges[j]))
{
valid = false;
m_pDevice->AddDebugMessage(
eDbgCategory_State_Setting, eDbgSeverity_High, eDbgSource_IncorrectAPIUse,
StringFormat::Fmt("Invalid output merger - Render targets %d and %d overlap", i, j));
break;
}
}

// or depth?
if(rtvRanges[i].Intersects(depthRange))
{
valid = false;
m_pDevice->AddDebugMessage(
eDbgCategory_State_Setting, eDbgSeverity_High, eDbgSource_IncorrectAPIUse,
StringFormat::Fmt("Invalid output merger - Render target %d and depth overlap", i));
break;
}

// or a UAV?
for(int j = 0; j < numUAVs; j++)
{
if(rtvRanges[i].Intersects(uavRanges[j]))
{
valid = false;
m_pDevice->AddDebugMessage(
eDbgCategory_State_Setting, eDbgSeverity_High, eDbgSource_IncorrectAPIUse,
StringFormat::Fmt("Invalid output merger - Render target %d and UAV %d overlap", i, j));
break;
}
}
}

for(int i = 0; valid && i < numUAVs; i++)
{
// don't have to check RTVs, that's the reflection of the above check

// does it match depth?
if(uavRanges[i].Intersects(depthRange))
{
valid = false;
m_pDevice->AddDebugMessage(
eDbgCategory_State_Setting, eDbgSeverity_High, eDbgSource_IncorrectAPIUse,
StringFormat::Fmt("Invalid output merger - UAV %d and depth overlap", i));
break;
}

// or another UAV?
for(int j = 0; j < numUAVs; j++)
{
if(i == j)
continue;

if(uavRanges[i].Intersects(uavRanges[j]))
{
valid = false;
m_pDevice->AddDebugMessage(
eDbgCategory_State_Setting, eDbgSeverity_High, eDbgSource_IncorrectAPIUse,
StringFormat::Fmt("Invalid output merger - UAVs %d and %d overlap", i, j));
break;
}
}
}

// don't have to check depth - it was checked against all RTs and UAVs above
}

//////////////////////////////////////////////////////////////////////////
// Resource dimensions of all views must be the same

Expand Down Expand Up @@ -1570,6 +1700,9 @@ D3D11RenderStateTracker::~D3D11RenderStateTracker()
m_RS.ApplyState(m_pContext);
}

D3D11RenderState::ResourceRange D3D11RenderState::ResourceRange::Null =
D3D11RenderState::ResourceRange(NULL, 0, 0);

D3D11RenderState::ResourceRange::ResourceRange(ID3D11ShaderResourceView *srv)
{
minMip = minSlice = 0;
Expand Down
48 changes: 47 additions & 1 deletion renderdoc/driver/d3d11/d3d11_renderstate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ struct D3D11RenderState

struct ResourceRange
{
static ResourceRange Null;

ResourceRange(ID3D11Buffer *res)
{
resource = res;
Expand Down Expand Up @@ -116,6 +118,7 @@ struct D3D11RenderState
return false;
}

bool IsNull() const { return resource == NULL; }
private:
ResourceRange();

Expand Down Expand Up @@ -339,6 +342,44 @@ struct D3D11RenderState
UnbindIUnknownForRead(ResourceRange(uav), false, false);
}

template <typename T>
void UnbindForWrite(T *resource);

template <>
void UnbindForWrite(ID3D11Buffer *buffer)
{
if(buffer == NULL)
return;
UnbindIUnknownForWrite(ResourceRange(buffer));
}

template <>
void UnbindForWrite(ID3D11RenderTargetView *rtv)
{
if(rtv == NULL)
return;

UnbindIUnknownForWrite(ResourceRange(rtv));
}

template <>
void UnbindForWrite(ID3D11DepthStencilView *dsv)
{
if(dsv == NULL)
return;

UnbindIUnknownForWrite(ResourceRange(dsv));
}

template <>
void UnbindForWrite(ID3D11UnorderedAccessView *uav)
{
if(uav == NULL)
return;

UnbindIUnknownForWrite(ResourceRange(uav));
}

/////////////////////////////////////////////////////////////////////////
// Utility functions to swap resources around, removing and adding refs

Expand Down Expand Up @@ -382,7 +423,11 @@ struct D3D11RenderState
ReleaseRef(old);

if(newArray[i])
{
UnbindForRead(newArray[i]);
// when binding something for write, all other write slots are NULL'd too
UnbindForWrite(newArray[i]);
}
stateArray[offset + i] = newArray[i];
TakeRef(stateArray[offset + i]);
}
Expand Down Expand Up @@ -442,7 +487,8 @@ struct D3D11RenderState
// that might not be obvious/intended.

// validate an output merger combination of render targets and depth view
bool ValidOutputMerger(ID3D11RenderTargetView **RTs, ID3D11DepthStencilView *depth);
bool ValidOutputMerger(ID3D11RenderTargetView **RTs, ID3D11DepthStencilView *depth,
ID3D11UnorderedAccessView **uavs);

struct inputassembler
{
Expand Down

0 comments on commit 9d6acd1

Please sign in to comment.