From c43db5a64d985e1b426b3fa3defeebc0769d3c2d Mon Sep 17 00:00:00 2001 From: baldurk Date: Thu, 14 Nov 2024 15:57:52 +0000 Subject: [PATCH] Try to serialise ASs as their own IDs not as a buffer+offset * Preallocate IDs for builds and pass via a new serialise sideband key-value storage. It's ugly and warty but less ugly than trying to pack the data in directly. * Descriptor references to ASs may not be right as a descriptor could be written before a build is even known about, or at least before it has fully resolved on the CPU timeline. --- renderdoc/driver/d3d12/d3d12_command_list.h | 1 + .../driver/d3d12/d3d12_command_list4_wrap.cpp | 39 +++++--- renderdoc/driver/d3d12/d3d12_common.h | 24 +++++ renderdoc/driver/d3d12/d3d12_device.cpp | 3 +- renderdoc/driver/d3d12/d3d12_resources.cpp | 9 +- renderdoc/driver/d3d12/d3d12_resources.h | 17 ++-- renderdoc/driver/d3d12/d3d12_serialise.cpp | 97 ++++++++++++++++++- renderdoc/serialise/serialiser.cpp | 3 + renderdoc/serialise/serialiser.h | 31 ++++++ 9 files changed, 198 insertions(+), 26 deletions(-) diff --git a/renderdoc/driver/d3d12/d3d12_command_list.h b/renderdoc/driver/d3d12/d3d12_command_list.h index e7266bb62..7e0c0fdd9 100644 --- a/renderdoc/driver/d3d12/d3d12_command_list.h +++ b/renderdoc/driver/d3d12/d3d12_command_list.h @@ -600,6 +600,7 @@ public: bool ProcessASBuildAfterSubmission(ResourceId asbWrappedResourceId, D3D12BufferOffset asbWrappedResourceBufferOffset, + ResourceId dstASId, D3D12_RAYTRACING_ACCELERATION_STRUCTURE_TYPE type, UINT64 byteSize, ASBuildData *buildData); diff --git a/renderdoc/driver/d3d12/d3d12_command_list4_wrap.cpp b/renderdoc/driver/d3d12/d3d12_command_list4_wrap.cpp index 047afb5c0..35fcc47ef 100644 --- a/renderdoc/driver/d3d12/d3d12_command_list4_wrap.cpp +++ b/renderdoc/driver/d3d12/d3d12_command_list4_wrap.cpp @@ -728,7 +728,7 @@ void WrappedID3D12GraphicsCommandList::ExecuteMetaCommand( } bool WrappedID3D12GraphicsCommandList::ProcessASBuildAfterSubmission( - ResourceId destASBId, D3D12BufferOffset destASBOffset, + ResourceId destASBId, D3D12BufferOffset destASBOffset, ResourceId dstASId, D3D12_RAYTRACING_ACCELERATION_STRUCTURE_TYPE type, UINT64 byteSize, ASBuildData *buildData) { D3D12ResourceManager *rm = m_pDevice->GetResourceManager(); @@ -743,7 +743,7 @@ bool WrappedID3D12GraphicsCommandList::ProcessASBuildAfterSubmission( // // CreateAccStruct deletes any previous overlapping ASs on the ASB D3D12AccelerationStructure *accStructAtDestOffset = NULL; - if(dstASB->CreateAccStruct(destASBOffset, type, byteSize, &accStructAtDestOffset)) + if(dstASB->CreateAccStruct(destASBOffset, type, byteSize, dstASId, &accStructAtDestOffset)) { D3D12ResourceRecord *record = rm->AddResourceRecord(accStructAtDestOffset->GetResourceID()); record->type = Resource_AccelerationStructure; @@ -1142,6 +1142,9 @@ void WrappedID3D12GraphicsCommandList::BuildRaytracingAccelerationStructure( sizeof(D3D12_RAYTRACING_ACCELERATION_STRUCTURE_POSTBUILD_INFO_CURRENT_SIZE_DESC)); } + // pre-allocate the AS ID so it can be serialised before the resource is created later on after submission + ResourceId dstASId = ResourceIDGen::GetNewUniqueID(); + // Acceleration structure (AS) are created on buffer created with Acceleration structure init // state which helps them differentiate between non-Acceleration structure buffers (non-ASB). @@ -1153,6 +1156,10 @@ void WrappedID3D12GraphicsCommandList::BuildRaytracingAccelerationStructure( { CACHE_THREAD_SERIALISER(); SCOPED_SERIALISE_CHUNK(D3D12Chunk::List_BuildRaytracingAccelerationStructure); + + // pass in the new AS ID for the destination + ser.SetSidebandData(D3D12DestASLocation::SidebandGUID, dstASId); + Serialise_BuildRaytracingAccelerationStructure(ser, pDesc, NumPostbuildInfoDescs, pPostbuildInfoDescs); @@ -1199,9 +1206,10 @@ void WrappedID3D12GraphicsCommandList::BuildRaytracingAccelerationStructure( AddSubmissionASBuildCallback( false, - [this, asbWrappedResourceId, asbWrappedResourceBufferOffset, type, byteSize, buildData]() { + [this, asbWrappedResourceId, asbWrappedResourceBufferOffset, dstASId, type, byteSize, + buildData]() { return ProcessASBuildAfterSubmission(asbWrappedResourceId, asbWrappedResourceBufferOffset, - type, byteSize, buildData); + dstASId, type, byteSize, buildData); }, [buildData]() { if(buildData) @@ -1258,7 +1266,7 @@ bool WrappedID3D12GraphicsCommandList::Serialise_EmitRaytracingAccelerationStruc SERIALISE_ELEMENT(pCommandList); SERIALISE_ELEMENT_LOCAL(Desc, *pDesc).Named("pDesc"); SERIALISE_ELEMENT(NumSourceAccelerationStructures).Important(); - SERIALISE_ELEMENT_ARRAY_TYPED(D3D12BufferLocation, pSourceAccelerationStructureData, + SERIALISE_ELEMENT_ARRAY_TYPED(D3D12SrcASLocation, pSourceAccelerationStructureData, NumSourceAccelerationStructures); SERIALISE_CHECK_READ_ERRORS(); @@ -1363,10 +1371,10 @@ bool WrappedID3D12GraphicsCommandList::Serialise_CopyRaytracingAccelerationStruc { ID3D12GraphicsCommandList4 *pCommandList = this; SERIALISE_ELEMENT(pCommandList); - SERIALISE_ELEMENT_TYPED(D3D12BufferLocation, DestAccelerationStructureData) + SERIALISE_ELEMENT_TYPED(D3D12DestASLocation, DestAccelerationStructureData) .TypedAs("D3D12_GPU_VIRTUAL_ADDRESS"_lit) .Important(); - SERIALISE_ELEMENT_TYPED(D3D12BufferLocation, SourceAccelerationStructureData) + SERIALISE_ELEMENT_TYPED(D3D12SrcASLocation, SourceAccelerationStructureData) .TypedAs("D3D12_GPU_VIRTUAL_ADDRESS"_lit) .Important(); SERIALISE_ELEMENT(Mode); @@ -1421,9 +1429,16 @@ void WrappedID3D12GraphicsCommandList::CopyRaytracingAccelerationStructure( // invalidating occupying previous acceleration structure(s) in order of command list execution. // It can also be updated but there are many update constraints around it. + // pre-allocate the AS ID so it can be serialised before the resource is created later on after submission + ResourceId dstASId = ResourceIDGen::GetNewUniqueID(); + { CACHE_THREAD_SERIALISER(); SCOPED_SERIALISE_CHUNK(D3D12Chunk::List_CopyRaytracingAccelerationStructure); + + // pass in the new AS ID for the destination + ser.SetSidebandData(D3D12DestASLocation::SidebandGUID, dstASId); + Serialise_CopyRaytracingAccelerationStructure(ser, DestAccelerationStructureData, SourceAccelerationStructureData, Mode); @@ -1510,13 +1525,15 @@ void WrappedID3D12GraphicsCommandList::CopyRaytracingAccelerationStructure( type = accStructAtSrcOffset->Type(); } - auto PostBldExecute = [this, destASBId, destASBOffset, type, sizeBuffer, buildData]() -> bool { + auto PostBldExecute = [this, destASBId, destASBOffset, dstASId, type, sizeBuffer, + buildData]() -> bool { UINT64 *size = (UINT64 *)sizeBuffer->Map(); UINT64 destSize = *size; sizeBuffer->Unmap(); sizeBuffer->Release(); - return ProcessASBuildAfterSubmission(destASBId, destASBOffset, type, destSize, buildData); + return ProcessASBuildAfterSubmission(destASBId, destASBOffset, dstASId, type, destSize, + buildData); }; AddSubmissionASBuildCallback(true, PostBldExecute, [buildData]() { @@ -1540,7 +1557,7 @@ void WrappedID3D12GraphicsCommandList::CopyRaytracingAccelerationStructure( // up to date before any subsequent work that depends on it like beginning a capture. AddSubmissionASBuildCallback( true, - [this, destASBId, destASBOffset, srcASBId, srcASBOffset]() { + [this, destASBId, destASBOffset, dstASId, srcASBId, srcASBOffset]() { D3D12ResourceManager *resManager = m_pDevice->GetResourceManager(); D3D12AccelerationStructure *accStructAtSrcOffset = NULL; @@ -1558,7 +1575,7 @@ void WrappedID3D12GraphicsCommandList::CopyRaytracingAccelerationStructure( // is likely to be deleted and release its own ref) SAFE_ADDREF(accStructAtSrcOffset->buildData); return ProcessASBuildAfterSubmission( - destASBId, destASBOffset, accStructAtSrcOffset->Type(), + destASBId, destASBOffset, dstASId, accStructAtSrcOffset->Type(), accStructAtSrcOffset->Size(), accStructAtSrcOffset->buildData); }, NULL); diff --git a/renderdoc/driver/d3d12/d3d12_common.h b/renderdoc/driver/d3d12/d3d12_common.h index e31be7ddb..2ecbe94dd 100644 --- a/renderdoc/driver/d3d12/d3d12_common.h +++ b/renderdoc/driver/d3d12/d3d12_common.h @@ -667,7 +667,31 @@ struct D3D12BufferLocation UINT64 Location; }; +// thin utility aliases of a ResourceId so that we know we're serialising an AS - distinct from +// buffer locations above to ensure it's mapped naturally. We have dest/src as different types with +// a common root because dest tries to draw from sideband data for its Id and source doesn't - this +// distinction is needed since e.g. the build desc serialises two AS locations and we need to tell +// which one should pull from sideband. +struct D3D12ASLocation +{ + D3D12ASLocation() : Location(0) {} + D3D12ASLocation(UINT64 l) : Location(l) {} + operator UINT64() const { return Location; } + UINT64 Location; +}; + +struct D3D12SrcASLocation : public D3D12ASLocation +{ +}; + +struct D3D12DestASLocation : public D3D12ASLocation +{ + static const uint64_t SidebandGUID = 0x3FD533B58697ULL; +}; + DECLARE_REFLECTION_STRUCT(D3D12BufferLocation); +DECLARE_REFLECTION_STRUCT(D3D12SrcASLocation); +DECLARE_REFLECTION_STRUCT(D3D12DestASLocation); DECLARE_REFLECTION_STRUCT(D3D12_CPU_DESCRIPTOR_HANDLE); DECLARE_REFLECTION_STRUCT(D3D12_GPU_DESCRIPTOR_HANDLE); diff --git a/renderdoc/driver/d3d12/d3d12_device.cpp b/renderdoc/driver/d3d12/d3d12_device.cpp index edae1b2c7..c86b31f87 100644 --- a/renderdoc/driver/d3d12/d3d12_device.cpp +++ b/renderdoc/driver/d3d12/d3d12_device.cpp @@ -3947,7 +3947,8 @@ bool WrappedID3D12Device::Serialise_CreateAS(SerialiserType &ser, ID3D12Resource { WrappedID3D12Resource *asbWrappedResource = (WrappedID3D12Resource *)pResource; D3D12AccelerationStructure *accStructAtOffset = NULL; - if(asbWrappedResource->CreateAccStruct(resourceOffset, type, byteSize, &accStructAtOffset)) + if(asbWrappedResource->CreateAccStruct(resourceOffset, type, byteSize, ResourceId(), + &accStructAtOffset)) { GetResourceManager()->AddLiveResource(asId, accStructAtOffset); diff --git a/renderdoc/driver/d3d12/d3d12_resources.cpp b/renderdoc/driver/d3d12/d3d12_resources.cpp index 763a3a843..e624ff287 100644 --- a/renderdoc/driver/d3d12/d3d12_resources.cpp +++ b/renderdoc/driver/d3d12/d3d12_resources.cpp @@ -142,10 +142,10 @@ ID3D12DeviceChild *Unwrap(ID3D12DeviceChild *ptr) WRAPPED_POOL_INST(D3D12AccelerationStructure); D3D12AccelerationStructure::D3D12AccelerationStructure( - WrappedID3D12Device *wrappedDevice, WrappedID3D12Resource *bufferRes, + WrappedID3D12Device *wrappedDevice, ResourceId id, WrappedID3D12Resource *bufferRes, D3D12BufferOffset bufferOffset, D3D12_RAYTRACING_ACCELERATION_STRUCTURE_TYPE type, UINT64 byteSize) - : WrappedDeviceChild12(NULL, wrappedDevice), + : WrappedDeviceChild12(NULL, wrappedDevice, id), m_asbWrappedResource(bufferRes), m_asbWrappedResourceBufferOffset(bufferOffset), type(type), @@ -161,7 +161,8 @@ D3D12AccelerationStructure::~D3D12AccelerationStructure() bool WrappedID3D12Resource::CreateAccStruct(D3D12BufferOffset bufferOffset, D3D12_RAYTRACING_ACCELERATION_STRUCTURE_TYPE type, - UINT64 byteSize, D3D12AccelerationStructure **accStruct) + UINT64 byteSize, ResourceId id, + D3D12AccelerationStructure **accStruct) { SCOPED_LOCK(m_accStructResourcesCS); auto existing = m_accelerationStructMap.find(bufferOffset); @@ -173,7 +174,7 @@ bool WrappedID3D12Resource::CreateAccStruct(D3D12BufferOffset bufferOffset, } m_accelerationStructMap[bufferOffset] = - new D3D12AccelerationStructure(m_pDevice, this, bufferOffset, type, byteSize); + new D3D12AccelerationStructure(m_pDevice, id, this, bufferOffset, type, byteSize); *accStruct = m_accelerationStructMap[bufferOffset]; diff --git a/renderdoc/driver/d3d12/d3d12_resources.h b/renderdoc/driver/d3d12/d3d12_resources.h index e7ed38c84..ff09b6a93 100644 --- a/renderdoc/driver/d3d12/d3d12_resources.h +++ b/renderdoc/driver/d3d12/d3d12_resources.h @@ -55,9 +55,12 @@ D3D12_UNORDERED_ACCESS_VIEW_DESC MakeUAVDesc(const D3D12_RESOURCE_DESC &desc); class TrackedResource12 { public: - TrackedResource12() + TrackedResource12(ResourceId id = ResourceId()) { - m_ID = ResourceIDGen::GetNewUniqueID(); + if(id == ResourceId()) + m_ID = ResourceIDGen::GetNewUniqueID(); + else + m_ID = id; m_pRecord = NULL; } ResourceId GetResourceID() { return m_ID; } @@ -82,8 +85,8 @@ protected: WrappedID3D12Device *m_pDevice; int32_t m_Resident = 1; - WrappedDeviceChild12(NestedType *real, WrappedID3D12Device *device) - : RefCounter12(real), m_pDevice(device) + WrappedDeviceChild12(NestedType *real, WrappedID3D12Device *device, ResourceId id = ResourceId()) + : RefCounter12(real), TrackedResource12(id), m_pDevice(device) { m_pDevice->SoftRef(); @@ -1334,7 +1337,7 @@ public: bool CreateAccStruct(D3D12BufferOffset bufferOffset, D3D12_RAYTRACING_ACCELERATION_STRUCTURE_TYPE type, UINT64 byteSize, - D3D12AccelerationStructure **accStruct); + ResourceId id, D3D12AccelerationStructure **accStruct); bool GetAccStructIfExist(D3D12BufferOffset bufferOffset, D3D12AccelerationStructure **accStruct = NULL); @@ -1665,8 +1668,8 @@ class D3D12AccelerationStructure : public WrappedDeviceChild12 +void DoSerialise(SerialiserType &ser, D3D12ASLocation &el, bool useSideband) +{ + D3D12ResourceManager *rm = (D3D12ResourceManager *)ser.GetUserData(); + + ResourceId buffer; + UINT64 offs = 0; + ResourceId asId; + bool isZero = el.Location == 0; + + if(ser.IsWriting() || ser.IsStructurising()) + { + WrappedID3D12Resource::GetResIDFromAddrAllowOutOfBounds(el.Location, buffer, offs); + + // for ASs which haven't yet been created where we're serialising them, we allow the user to + // pass in the upcoming ResourceId via sideband data. + if(useSideband) + { + asId = ser.GetSidebandData(D3D12DestASLocation::SidebandGUID); + } + else + { + // otherwise query from the resource for the current AS there, if one exists + WrappedID3D12Resource *res = rm ? rm->GetCurrentAs(buffer) : NULL; + if(res) + { + D3D12AccelerationStructure *as = NULL; + if(res->GetAccStructIfExist(offs, &as)) + asId = as->GetResourceID(); + } + } + } + if(ser.IsStructurising() && rm) + { + buffer = rm->GetOriginalID(buffer); + asId = rm->GetOriginalID(asId); + } + + // we get a little dynamic with this. If we successfully got an AS (or it was a zero location) + // then we only display the AS's ID since no offset is needed. If for some reason the AS didn't + // come through, we display it as a buffer location with buffer+offset + + ser.Serialise("isZero"_lit, isZero).Hidden(); + ser.Serialise("AccStruct"_lit, asId); + + if(asId != ResourceId() || isZero) + ser.Important(); + else + ser.Hidden(); + + ser.Serialise("Buffer"_lit, buffer); + + if(asId != ResourceId() || isZero) + ser.Hidden(); + else + ser.Important(); + + ser.Serialise("Offset"_lit, offs).OffsetOrSize(); + + if(asId != ResourceId() || isZero) + ser.Hidden(); + + if(ser.IsReading() && !ser.IsStructurising()) + { + if(rm && asId != ResourceId() && rm->HasLiveResource(asId)) + el.Location = rm->GetLiveAs(asId)->GetVirtualAddress(); + else if(rm && buffer != ResourceId() && rm->HasLiveResource(buffer)) + el.Location = rm->GetLiveAs(buffer)->GetGPUVirtualAddress() + offs; + else + ser.ClearObj(el.Location); + } +} + +template +void DoSerialise(SerialiserType &ser, D3D12DestASLocation &el) +{ + return DoSerialise(ser, el, true); +} + +template +void DoSerialise(SerialiserType &ser, D3D12SrcASLocation &el) +{ + return DoSerialise(ser, el, false); +} + template void DoSerialise(SerialiserType &ser, D3D12Descriptor &el) { @@ -1075,7 +1160,13 @@ void DoSerialise(SerialiserType &ser, D3D12_TEXCUBE_ARRAY_SRV &el) template void DoSerialise(SerialiserType &ser, D3D12_RAYTRACING_ACCELERATION_STRUCTURE_SRV &el) { - SERIALISE_MEMBER_TYPED(D3D12BufferLocation, Location); + // because descriptor writes are on the CPU timeline and AS existence is effectively on the GPU + // timeline, this may come before the first build is submitted or even recorded. In that case the + // AS could be serialised as a buffer+offset instead of by its ID, or it could be serialised as + // the "wrong" AS ID - e.g. the previous version of an AS when this descriptor won't be used until + // after a rebuild. We assume in the case where we get the wrong AS ID that it will be a strict + // alias and aliasing it handled by any re-allocation of AS backing memory that happens + SERIALISE_MEMBER_TYPED(D3D12SrcASLocation, Location); } template @@ -1880,10 +1971,10 @@ void Deserialise(const D3D12_BUILD_RAYTRACING_ACCELERATION_STRUCTURE_DESC &el) template void DoSerialise(SerialiserType &ser, D3D12_BUILD_RAYTRACING_ACCELERATION_STRUCTURE_DESC &el) { - SERIALISE_MEMBER_TYPED(D3D12BufferLocation, DestAccelerationStructureData).Important(); + SERIALISE_MEMBER_TYPED(D3D12DestASLocation, DestAccelerationStructureData).Important(); SERIALISE_MEMBER(Inputs); - SERIALISE_MEMBER_TYPED(D3D12BufferLocation, SourceAccelerationStructureData); + SERIALISE_MEMBER_TYPED(D3D12SrcASLocation, SourceAccelerationStructureData); if(el.SourceAccelerationStructureData) ser.Important(); diff --git a/renderdoc/serialise/serialiser.cpp b/renderdoc/serialise/serialiser.cpp index 9bb81375a..5672cdfe4 100644 --- a/renderdoc/serialise/serialiser.cpp +++ b/renderdoc/serialise/serialiser.cpp @@ -353,6 +353,9 @@ uint32_t Serialiser::BeginChunk(uint32_t chunkID, uint6 RDCASSERTMSG("Beginning a chunk inside another chunk", m_ChunkMetadata.chunkID == 0, m_ChunkMetadata.chunkID); + // don't carry over any previous sideband data + m_SidebandKV.clear(); + { // chunk index needs to be valid RDCASSERT(chunkID > 0); diff --git a/renderdoc/serialise/serialiser.h b/renderdoc/serialise/serialiser.h index fd56a01fa..e88b5d541 100644 --- a/renderdoc/serialise/serialiser.h +++ b/renderdoc/serialise/serialiser.h @@ -166,6 +166,36 @@ public: // to flag if some context-sensitive members might be invalid void SetStructArg(uint64_t arg) { m_StructArg = arg; } uint64_t GetStructArg() { return m_StructArg; } + + // in rare cases it is necessary to pass side-band data even further than the small context + // allowed by SetStructArg, from serialisation users all the way down to child structs. We allow + // adding side-band data indexed by 'GUID' here for those rare cases. + template + T GetSidebandData(uint64_t guid) + { + RDCCOMPILE_ASSERT(sizeof(T) <= sizeof(uint64_t), "Side-band data is at most 64-bit"); + + T ret; + for(rdcpair &kv : m_SidebandKV) + { + if(kv.first == guid) + { + memcpy(&ret, &kv.second, sizeof(T)); + return ret; + } + } + + return ret; + } + template + void SetSidebandData(uint64_t guid, const T &t) + { + RDCCOMPILE_ASSERT(sizeof(T) <= sizeof(uint64_t), "Side-band data is at most 64-bit"); + uint64_t data = 0; + memcpy(&data, &t, sizeof(T)); + m_SidebandKV.push_back({guid, data}); + } + ////////////////////////////////////////// // Public serialisation interface @@ -1578,6 +1608,7 @@ private: uint64_t m_Version = 0; uint64_t m_StructArg = 0; + rdcarray> m_SidebandKV; StreamWriter *m_Write = NULL; StreamReader *m_Read = NULL;