From 9d9ecb3ba8773b4e5176d40ebe61571ffe0ccad6 Mon Sep 17 00:00:00 2001 From: Wojtek Figat Date: Tue, 8 Jun 2021 14:12:31 +0200 Subject: [PATCH] Fix D3D12 resource state transitions barriers --- .../DirectX/DX12/GPUContextDX12.cpp | 113 ++++++++++-------- .../DirectX/DX12/GPUTextureDX12.cpp | 3 + .../DirectX/DX12/GPUTextureDX12.h | 1 + .../DirectX/DX12/ResourceOwnerDX12.h | 17 ++- 4 files changed, 78 insertions(+), 56 deletions(-) diff --git a/Source/Engine/GraphicsDevice/DirectX/DX12/GPUContextDX12.cpp b/Source/Engine/GraphicsDevice/DirectX/DX12/GPUContextDX12.cpp index c99340496..3fd85fad8 100644 --- a/Source/Engine/GraphicsDevice/DirectX/DX12/GPUContextDX12.cpp +++ b/Source/Engine/GraphicsDevice/DirectX/DX12/GPUContextDX12.cpp @@ -159,7 +159,7 @@ void GPUContextDX12::SetResourceState(ResourceOwnerDX12* resource, D3D12_RESOURC if (ResourceStateDX12::IsTransitionNeeded(before, after)) { AddTransitionBarrier(resource, before, after, D3D12_RESOURCE_BARRIER_ALL_SUBRESOURCES); - state.SetSubresourceState(subresourceIndex, after); + state.SetResourceState(after); } } else @@ -175,8 +175,8 @@ void GPUContextDX12::SetResourceState(ResourceOwnerDX12* resource, D3D12_RESOURC } } ASSERT(state.CheckResourceState(after)); + state.SetResourceState(after); } - state.SetResourceState(after); } else { @@ -286,7 +286,7 @@ void GPUContextDX12::flushSRVs() return; // Bind all required slots and mark them as not dirty - _srMaskDirtyCompute &= ~srMask; + //_srMaskDirtyCompute &= ~srMask; // TODO: this causes visual artifacts sometimes, maybe use binary SR-dirty flag for all slots? } else { @@ -295,7 +295,7 @@ void GPUContextDX12::flushSRVs() return; // Bind all required slots and mark them as not dirty - _srMaskDirtyGraphics &= ~srMask; + //_srMaskDirtyGraphics &= ~srMask; // TODO: this causes visual artifacts sometimes, maybe use binary SR-dirty flag for all slots? } // Count SRVs required to be bind to the pipeline (the index of the most significant bit that's set) @@ -309,7 +309,7 @@ void GPUContextDX12::flushSRVs() { const auto handle = _srHandles[i]; const auto dimensions = (D3D12_SRV_DIMENSION)header.SrDimensions[i]; - if (handle != nullptr && dimensions) + if (srMask & (1 << i) && handle != nullptr && dimensions) { ASSERT(handle->SrvDimension == dimensions); srcDescriptorRangeStarts[i] = handle->SRV(); @@ -360,7 +360,8 @@ void GPUContextDX12::flushRTVs() if (_rtDepth) { depthBuffer = _rtDepth->DSV(); - SetResourceState(_rtDepth->GetResourceOwner(), D3D12_RESOURCE_STATE_DEPTH_WRITE, _rtDepth->SubresourceIndex); + auto states = _rtDepth->ReadOnlyDepthView ? D3D12_RESOURCE_STATE_DEPTH_READ : D3D12_RESOURCE_STATE_DEPTH_WRITE; + SetResourceState(_rtDepth->GetResourceOwner(), states, _rtDepth->SubresourceIndex); } else { @@ -507,28 +508,33 @@ void GPUContextDX12::onDrawCall() SetResourceState(_ibHandle, D3D12_RESOURCE_STATE_INDEX_BUFFER); } - // If SRV resource is not binded to RTV then transition it to the whole state (GPU-BASED VALIDATION complains about it) - for (uint32 i = 0; i < GPU_MAX_SR_BINDED; i++) + if (_currentState) { - const auto handle = _srHandles[i]; - if (handle != nullptr && handle->GetResourceOwner()) + // If SRV resource is not binded to RTV then transition it to the whole state (GPU-BASED VALIDATION complains about it) + const uint32 srMask = _currentState->GetUsedSRsMask(); + const uint32 srCount = Math::FloorLog2(srMask) + 1; + for (uint32 i = 0; i < srCount; i++) { - const auto resourceOwner = handle->GetResourceOwner(); - bool isRtv = false; - for (int32 j = 0; j < _rtCount; j++) + const auto handle = _srHandles[i]; + if (srMask & (1 << i) && handle != nullptr && handle->GetResourceOwner()) { - if (_rtHandles[j] && _rtHandles[j]->GetResourceOwner() == resourceOwner) + const auto resourceOwner = handle->GetResourceOwner(); + bool isRtv = false; + for (int32 j = 0; j < _rtCount; j++) { - isRtv = true; - break; + if (_rtHandles[j] && _rtHandles[j]->GetResourceOwner() == resourceOwner) + { + isRtv = true; + break; + } + } + if (!isRtv) + { + D3D12_RESOURCE_STATES states = D3D12_RESOURCE_STATE_NON_PIXEL_SHADER_RESOURCE | D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE; + if (handle->IsDepthStencilResource()) + states |= D3D12_RESOURCE_STATE_DEPTH_READ; + SetResourceState(handle->GetResourceOwner(), states); } - } - if (!isRtv) - { - D3D12_RESOURCE_STATES states = D3D12_RESOURCE_STATE_NON_PIXEL_SHADER_RESOURCE | D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE; - if (handle->IsDepthStencilResource()) - states |= D3D12_RESOURCE_STATE_DEPTH_READ; - SetResourceState(handle->GetResourceOwner(), states); } } } @@ -543,36 +549,39 @@ void GPUContextDX12::onDrawCall() #if BUILD_DEBUG // Additional verification of the state - for (int32 i = 0; i < _rtCount; i++) + if (_currentState) { - const auto handle = _rtHandles[i]; - if (handle != nullptr && handle->GetResourceOwner()) + for (int32 i = 0; i < _rtCount; i++) { - const auto& state = handle->GetResourceOwner()->State; - ASSERT((state.GetSubresourceState(handle->SubresourceIndex) & D3D12_RESOURCE_STATE_RENDER_TARGET) != 0); - } - } - const uint32 srMask = _currentState->GetUsedSRsMask(); - const uint32 srCount = Math::FloorLog2(srMask) + 1; - for (uint32 i = 0; i < srCount; i++) - { - const auto handle = _srHandles[i]; - if (handle != nullptr && handle->GetResourceOwner()) - { - const auto& state = handle->GetResourceOwner()->State; - bool isRtv = false; - for (int32 j = 0; j < _rtCount; j++) + const auto handle = _rtHandles[i]; + if (handle != nullptr && handle->GetResourceOwner()) { - if (_rtHandles[j] && _rtHandles[j]->GetResourceOwner() == handle->GetResourceOwner()) - { - isRtv = true; - break; - } + const auto& state = handle->GetResourceOwner()->State; + ASSERT((state.GetSubresourceState(handle->SubresourceIndex) & D3D12_RESOURCE_STATE_RENDER_TARGET) != 0); } - ASSERT((state.GetSubresourceState(handle->SubresourceIndex) & D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE) != 0); - if (!isRtv) + } + const uint32 srMask = _currentState->GetUsedSRsMask(); + const uint32 srCount = Math::FloorLog2(srMask) + 1; + for (uint32 i = 0; i < srCount; i++) + { + const auto handle = _srHandles[i]; + if (srMask & (1 << i) && handle != nullptr && handle->GetResourceOwner()) { - ASSERT(state.AreAllSubresourcesSame()); + const auto& state = handle->GetResourceOwner()->State; + bool isRtv = false; + for (int32 j = 0; j < _rtCount; j++) + { + if (_rtHandles[j] && _rtHandles[j]->GetResourceOwner() == handle->GetResourceOwner()) + { + isRtv = true; + break; + } + } + ASSERT((state.GetSubresourceState(handle->SubresourceIndex) & D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE) != 0); + if (!isRtv) + { + ASSERT(state.AreAllSubresourcesSame()); + } } } } @@ -1069,10 +1078,10 @@ void GPUContextDX12::ClearState() void GPUContextDX12::FlushState() { // Flush - flushCBs(); - flushSRVs(); - flushRTVs(); - flushUAVs(); + //flushCBs(); + //flushSRVs(); + //flushRTVs(); + //flushUAVs(); flushRBs(); //flushPS(); } diff --git a/Source/Engine/GraphicsDevice/DirectX/DX12/GPUTextureDX12.cpp b/Source/Engine/GraphicsDevice/DirectX/DX12/GPUTextureDX12.cpp index 265023fa8..37058884a 100644 --- a/Source/Engine/GraphicsDevice/DirectX/DX12/GPUTextureDX12.cpp +++ b/Source/Engine/GraphicsDevice/DirectX/DX12/GPUTextureDX12.cpp @@ -450,6 +450,8 @@ void GPUTextureDX12::initHandles() srDesc.Texture2DArray.ResourceMinLODClamp = 0; } _handlesPerSlice[arrayIndex].SetSRV(srDesc); + if (isCubeMap) + _handlesPerSlice[arrayIndex].SrvDimension = D3D12_SRV_DIMENSION_TEXTURE2D; // Hack xD (to reproduce the problem comment this line and use Spot Light with a shadow) } if (useUAV) { @@ -688,6 +690,7 @@ void GPUTextureDX12::initHandles() if (_desc.Flags & GPUTextureFlags::ReadOnlyDepthView) { _handleReadOnlyDepth.Init(this, _device, this, format, msaa); + _handleReadOnlyDepth.ReadOnlyDepthView = true; if (useDSV) { if (isCubeMap) diff --git a/Source/Engine/GraphicsDevice/DirectX/DX12/GPUTextureDX12.h b/Source/Engine/GraphicsDevice/DirectX/DX12/GPUTextureDX12.h index c13df1756..b93828ed5 100644 --- a/Source/Engine/GraphicsDevice/DirectX/DX12/GPUTextureDX12.h +++ b/Source/Engine/GraphicsDevice/DirectX/DX12/GPUTextureDX12.h @@ -73,6 +73,7 @@ public: public: + bool ReadOnlyDepthView = false; void SetRTV(D3D12_RENDER_TARGET_VIEW_DESC& rtvDesc); void SetSRV(D3D12_SHADER_RESOURCE_VIEW_DESC& srvDesc); void SetDSV(D3D12_DEPTH_STENCIL_VIEW_DESC& dsvDesc); diff --git a/Source/Engine/GraphicsDevice/DirectX/DX12/ResourceOwnerDX12.h b/Source/Engine/GraphicsDevice/DirectX/DX12/ResourceOwnerDX12.h index acca85e3d..f797c84c0 100644 --- a/Source/Engine/GraphicsDevice/DirectX/DX12/ResourceOwnerDX12.h +++ b/Source/Engine/GraphicsDevice/DirectX/DX12/ResourceOwnerDX12.h @@ -32,12 +32,21 @@ public: /// /// Returns true if resource state transition is needed in order to use resource in given state. /// - /// The current resource state. - /// the destination resource state. + /// The current resource state. + /// the destination resource state. /// True if need to perform a transition, otherwise false. - FORCE_INLINE static bool IsTransitionNeeded(D3D12_RESOURCE_STATES currentState, D3D12_RESOURCE_STATES targetState) + FORCE_INLINE static bool IsTransitionNeeded(D3D12_RESOURCE_STATES before, D3D12_RESOURCE_STATES& after) { - return currentState != targetState && ((currentState | targetState) != currentState || targetState == D3D12_RESOURCE_STATE_COMMON); + if (before == D3D12_RESOURCE_STATE_DEPTH_WRITE && after == D3D12_RESOURCE_STATE_DEPTH_READ) + return false; + if (after == D3D12_RESOURCE_STATE_COMMON) + return before != D3D12_RESOURCE_STATE_COMMON; + if (after == D3D12_RESOURCE_STATE_DEPTH_READ) + return ~(before & (D3D12_RESOURCE_STATE_NON_PIXEL_SHADER_RESOURCE | D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE)) == 0; + const D3D12_RESOURCE_STATES combined = before | after; + if ((combined & (D3D12_RESOURCE_STATE_GENERIC_READ | D3D12_RESOURCE_STATE_INDIRECT_ARGUMENT)) == combined) + after = combined; + return before != after; } };