diff --git a/renderdoc/driver/d3d12/d3d12_command_list4_wrap.cpp b/renderdoc/driver/d3d12/d3d12_command_list4_wrap.cpp index 652cd7339..279213231 100644 --- a/renderdoc/driver/d3d12/d3d12_command_list4_wrap.cpp +++ b/renderdoc/driver/d3d12/d3d12_command_list4_wrap.cpp @@ -732,78 +732,52 @@ bool WrappedID3D12GraphicsCommandList::ProcessASBuildAfterSubmission(ResourceId UINT64 byteSize, ASBuildData *buildData) { - bool success = false; - D3D12ResourceManager *resManager = m_pDevice->GetResourceManager(); + D3D12ResourceManager *rm = m_pDevice->GetResourceManager(); + WrappedID3D12Resource *dstASB = rm->GetCurrentAs(destASBId); + + // unconditionally create a new AS at this location, never allow re-use even in the case of + // in-place update builds. This makes it easier to track ASs and we will not run out of + // ResourceIds. There is no case where we need to track an update (it may as well be a new build + // for our purposes), and when validating if a TLAS should be rebuilt based on the existing inputs + // it is easier if each build is new, as each build of a BLAS invalidates the TLAS. + // + // CreateAccStruct deletes any previous overlapping ASs on the ASB D3D12AccelerationStructure *accStructAtDestOffset = NULL; - - WrappedID3D12Resource *dstASB = resManager->GetCurrentAs(destASBId); - - // See if acc already exist at the given offset - bool accStructExistAtDestOffset = - dstASB->GetAccStructIfExist(destASBOffset, &accStructAtDestOffset); - - bool createAccStruct = false; - - if(accStructExistAtDestOffset) + if(dstASB->CreateAccStruct(destASBOffset, byteSize, &accStructAtDestOffset)) { - if(accStructAtDestOffset && accStructAtDestOffset->Size() != byteSize) + D3D12ResourceRecord *record = rm->AddResourceRecord(accStructAtDestOffset->GetResourceID()); + record->type = Resource_AccelerationStructure; + record->Length = 0; + accStructAtDestOffset->SetResourceRecord(record); + rm->MarkDirtyResource(accStructAtDestOffset->GetResourceID()); + + record->AddParent(rm->GetResourceRecord(accStructAtDestOffset->GetBackingBufferResourceId())); + + // register this AS so its resource can be created during replay + m_pDevice->CreateAS(dstASB, destASBOffset, byteSize, accStructAtDestOffset); + + m_pDevice->AddForcedReference(record); + // in case we're currently capturing, immediately consider the AS as referenced + GetResourceManager()->MarkResourceFrameReferenced(accStructAtDestOffset->GetResourceID(), + eFrameRef_Read); + + if(buildData) { - dstASB->DeleteAccStructAtOffset(destASBOffset); - createAccStruct = true; - } - else - { - // if the AS is being rebuilt in place, that's also successful - success = true; + // release any existing build data we had, this is a new version + SAFE_RELEASE(accStructAtDestOffset->buildData); + + // take ownership of the implicit ref + accStructAtDestOffset->buildData = buildData; } } else { - createAccStruct = true; + RDCERR("Unable to create acceleration structure"); + return false; } - if(createAccStruct) - { - // CreateAccStruct also deletes any previous overlapping ASs on the ASB - if(dstASB->CreateAccStruct(destASBOffset, byteSize, &accStructAtDestOffset)) - { - success = true; - D3D12ResourceRecord *record = - resManager->AddResourceRecord(accStructAtDestOffset->GetResourceID()); - record->type = Resource_AccelerationStructure; - record->Length = 0; - accStructAtDestOffset->SetResourceRecord(record); - resManager->MarkDirtyResource(accStructAtDestOffset->GetResourceID()); - - record->AddParent( - resManager->GetResourceRecord(accStructAtDestOffset->GetBackingBufferResourceId())); - - // register this AS so its resource can be created during replay - m_pDevice->CreateAS(dstASB, destASBOffset, byteSize, accStructAtDestOffset); - - m_pDevice->AddForcedReference(record); - // in case we're currently capturing, immediately consider the AS as referenced - GetResourceManager()->MarkResourceFrameReferenced(accStructAtDestOffset->GetResourceID(), - eFrameRef_Read); - } - else - { - RDCERR("Unable to create acceleration structure"); - success = false; - } - } - - if(success && buildData) - { - // release any existing build data we had, this is a new version - SAFE_RELEASE(accStructAtDestOffset->buildData); - - // take ownership of the implicit ref - accStructAtDestOffset->buildData = buildData; - } - - return success; + return true; } bool WrappedID3D12GraphicsCommandList::PatchAccStructBlasAddress( diff --git a/renderdoc/driver/d3d12/d3d12_replay.cpp b/renderdoc/driver/d3d12/d3d12_replay.cpp index 8aa9cce6d..e74a2f7bd 100644 --- a/renderdoc/driver/d3d12/d3d12_replay.cpp +++ b/renderdoc/driver/d3d12/d3d12_replay.cpp @@ -833,9 +833,7 @@ void D3D12Replay::FillDescriptor(Descriptor &dst, const D3D12Descriptor *src) { // we *should* get an AS here D3D12AccelerationStructure *as = NULL; - asRes->GetAccStructIfExist(dst.byteOffset, &as); - - if(as) + if(asRes->GetAccStructIfExist(dst.byteOffset, &as)) { dst.resource = rm->GetOriginalID(as->GetResourceID()); dst.byteOffset = 0; diff --git a/renderdoc/driver/d3d12/d3d12_resources.cpp b/renderdoc/driver/d3d12/d3d12_resources.cpp index f40177bda..698436bad 100644 --- a/renderdoc/driver/d3d12/d3d12_resources.cpp +++ b/renderdoc/driver/d3d12/d3d12_resources.cpp @@ -162,25 +162,23 @@ bool WrappedID3D12Resource::CreateAccStruct(D3D12BufferOffset bufferOffset, UINT D3D12AccelerationStructure **accStruct) { SCOPED_LOCK(m_accStructResourcesCS); - if(m_accelerationStructMap.find(bufferOffset) == m_accelerationStructMap.end()) + auto existing = m_accelerationStructMap.find(bufferOffset); + if(existing != m_accelerationStructMap.end()) { - m_accelerationStructMap[bufferOffset] = - new D3D12AccelerationStructure(m_pDevice, this, bufferOffset, byteSize); - - if(accStruct) - { - *accStruct = m_accelerationStructMap[bufferOffset]; - - if(IsCaptureMode(m_pDevice->GetState())) - { - DeleteOverlappingAccStructsInRangeAtOffset(bufferOffset); - } - } - - return true; + if(IsCaptureMode(m_pDevice->GetState())) + RDCASSERTEQUAL((uint32_t)existing->second->Release(), 0); + m_accelerationStructMap.erase(existing); } - return false; + m_accelerationStructMap[bufferOffset] = + new D3D12AccelerationStructure(m_pDevice, this, bufferOffset, byteSize); + + *accStruct = m_accelerationStructMap[bufferOffset]; + + if(IsCaptureMode(m_pDevice->GetState())) + DeleteOverlappingAccStructsInRangeAtOffset(bufferOffset); + + return true; } WrappedID3D12Resource::~WrappedID3D12Resource() @@ -434,10 +432,8 @@ bool WrappedID3D12Resource::DeleteAccStructAtOffset(D3D12BufferOffset bufferOffs D3D12AccelerationStructure *accStruct = NULL; if(GetAccStructIfExist(bufferOffset, &accStruct)) { - if(m_accelerationStructMap[bufferOffset]->Release() == 0) - { - m_accelerationStructMap.erase(bufferOffset); - } + RDCASSERTEQUAL((uint32_t)accStruct->Release(), 0); + m_accelerationStructMap.erase(bufferOffset); return true; }