From 4f867cc201a204b25aea3a45729863deba7fee74 Mon Sep 17 00:00:00 2001 From: Jim Van Verth Date: Mon, 20 Sep 2021 10:59:56 -0400 Subject: [PATCH] Reland "Direct3D: Be sure to set correct heaps for current descriptor tables." This is a reland of 7e33d95f4f881f7d304ea81db39d20be4dab4416 Original change's description: > Direct3D: Be sure to set correct heaps for current descriptor tables. > > When binding descriptor tables, their associated heaps need to be bound > as well. Previously we would bind those heaps when allocating from them. > However, if we re-use a descriptor table later, its heap may no longer > be bound. So we need to be sure to bind heaps for the current set. > > To avoid unnecessary refs, rather than store a > sk_sp in each descriptor table, we > only store its ID3D12DescriptorHeap pointer. The Heap only needs to be > added to the command list once, when it is first used to allocate for > the current submit. > > Bug: skia:12359 > Change-Id: I70018368e4f08bf7757969b9e878b0ff42059486 > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/448836 > Commit-Queue: Jim Van Verth > Reviewed-by: Greg Daniel Bug: skia:12359 Change-Id: Ifdd6c76cb5f00c82f9e4206ae31ea2f1df6ef37b Reviewed-on: https://skia-review.googlesource.com/c/skia/+/449916 Commit-Queue: Jim Van Verth Reviewed-by: Brian Salomon --- src/gpu/d3d/GrD3DCommandList.cpp | 8 +++---- src/gpu/d3d/GrD3DCommandList.h | 16 ++++++-------- src/gpu/d3d/GrD3DDescriptorTableManager.cpp | 24 ++++++++------------- src/gpu/d3d/GrD3DDescriptorTableManager.h | 6 ++++-- src/gpu/d3d/GrD3DGpu.cpp | 11 ++++++---- src/gpu/d3d/GrD3DPipelineState.cpp | 11 ++++++---- 6 files changed, 37 insertions(+), 39 deletions(-) diff --git a/src/gpu/d3d/GrD3DCommandList.cpp b/src/gpu/d3d/GrD3DCommandList.cpp index 327566140f..5187446179 100644 --- a/src/gpu/d3d/GrD3DCommandList.cpp +++ b/src/gpu/d3d/GrD3DCommandList.cpp @@ -572,9 +572,9 @@ void GrD3DDirectCommandList::setComputeRootDescriptorTable( } } -void GrD3DDirectCommandList::setDescriptorHeaps(sk_sp srvCrvHeapResource, - ID3D12DescriptorHeap* srvCrvDescriptorHeap, - sk_sp samplerHeapResource, +// We don't need to add these resources to the command list. +// They're added when we first allocate from a heap in a given submit. +void GrD3DDirectCommandList::setDescriptorHeaps(ID3D12DescriptorHeap* srvCrvDescriptorHeap, ID3D12DescriptorHeap* samplerDescriptorHeap) { if (srvCrvDescriptorHeap != fCurrentSRVCRVDescriptorHeap || samplerDescriptorHeap != fCurrentSamplerDescriptorHeap) { @@ -584,8 +584,6 @@ void GrD3DDirectCommandList::setDescriptorHeaps(sk_sp srvCrv }; fCommandList->SetDescriptorHeaps(2, heaps); - this->addRecycledResource(std::move(srvCrvHeapResource)); - this->addRecycledResource(std::move(samplerHeapResource)); fCurrentSRVCRVDescriptorHeap = srvCrvDescriptorHeap; fCurrentSamplerDescriptorHeap = samplerDescriptorHeap; } diff --git a/src/gpu/d3d/GrD3DCommandList.h b/src/gpu/d3d/GrD3DCommandList.h index 715c52d7c7..3f08f6177c 100644 --- a/src/gpu/d3d/GrD3DCommandList.h +++ b/src/gpu/d3d/GrD3DCommandList.h @@ -100,6 +100,12 @@ public: fTrackedGpuBuffers.push_back(std::move(buffer)); } + // Add ref-counted resource that will be tracked and released when this command buffer finishes + // execution. When it is released, it will signal that the resource can be recycled for reuse. + void addRecycledResource(sk_sp resource) { + fTrackedRecycledResources.push_back(std::move(resource)); + } + void releaseResources(); bool hasWork() const { return fHasWork; } @@ -120,12 +126,6 @@ protected: fTrackedResources.push_back(std::move(resource)); } - // Add ref-counted resource that will be tracked and released when this command buffer finishes - // execution. When it is released, it will signal that the resource can be recycled for reuse. - void addRecycledResource(sk_sp resource) { - fTrackedRecycledResources.push_back(std::move(resource)); - } - void addingWork(); virtual void onReset() {} @@ -201,9 +201,7 @@ public: D3D12_GPU_VIRTUAL_ADDRESS bufferLocation); void setComputeRootDescriptorTable(unsigned int rootParameterIndex, D3D12_GPU_DESCRIPTOR_HANDLE bufferLocation); - void setDescriptorHeaps(sk_sp srvCrvHeapResource, - ID3D12DescriptorHeap* srvDescriptorHeap, - sk_sp samplerHeapResource, + void setDescriptorHeaps(ID3D12DescriptorHeap* srvDescriptorHeap, ID3D12DescriptorHeap* samplerDescriptorHeap); void addSampledTextureRef(GrD3DTexture*); diff --git a/src/gpu/d3d/GrD3DDescriptorTableManager.cpp b/src/gpu/d3d/GrD3DDescriptorTableManager.cpp index ec2ff195a3..56dcf6baa9 100644 --- a/src/gpu/d3d/GrD3DDescriptorTableManager.cpp +++ b/src/gpu/d3d/GrD3DDescriptorTableManager.cpp @@ -16,27 +16,15 @@ GrD3DDescriptorTableManager::GrD3DDescriptorTableManager(GrD3DGpu* gpu) sk_sp GrD3DDescriptorTableManager::createShaderViewTable(GrD3DGpu* gpu, unsigned int size) { sk_sp table = fShaderViewDescriptorPool.allocateTable(gpu, size); - this->setHeaps(gpu); return table; } sk_sp GrD3DDescriptorTableManager::createSamplerTable( GrD3DGpu* gpu, unsigned int size) { sk_sp table = fSamplerDescriptorPool.allocateTable(gpu, size); - this->setHeaps(gpu); return table; } -void GrD3DDescriptorTableManager::setHeaps(GrD3DGpu* gpu) { - sk_sp& currentCBVSRVHeap = fShaderViewDescriptorPool.currentDescriptorHeap(); - sk_sp& currentSamplerHeap = fSamplerDescriptorPool.currentDescriptorHeap(); - GrD3DDirectCommandList* commandList = gpu->currentCommandList(); - commandList->setDescriptorHeaps(currentCBVSRVHeap, - currentCBVSRVHeap->d3dDescriptorHeap(), - currentSamplerHeap, - currentSamplerHeap->d3dDescriptorHeap()); -} - void GrD3DDescriptorTableManager::prepForSubmit(GrD3DGpu* gpu) { fShaderViewDescriptorPool.prepForSubmit(gpu); fSamplerDescriptorPool.prepForSubmit(gpu); @@ -80,7 +68,8 @@ sk_sp GrD3DDescriptorTableManager::Heap::allocateTable( fNextAvailable += count; return sk_sp( new GrD3DDescriptorTable(fHeap->getCPUHandle(startIndex).fHandle, - fHeap->getGPUHandle(startIndex).fHandle, fType)); + fHeap->getGPUHandle(startIndex).fHandle, + fHeap->descriptorHeap(), fType)); } void GrD3DDescriptorTableManager::Heap::onRecycle() const { @@ -103,8 +92,12 @@ sk_sp GrD3DDescriptorTableManager::HeapPool::allocateTable // If it was already used, it will have been added to the commandlist, // and then later recycled back to us. while (fDescriptorHeaps.size() > 0) { - if (fDescriptorHeaps[fDescriptorHeaps.size() - 1]->canAllocate(count)) { - return fDescriptorHeaps[fDescriptorHeaps.size() - 1]->allocateTable(count); + auto& heap = fDescriptorHeaps[fDescriptorHeaps.size() - 1]; + if (heap->canAllocate(count)) { + if (!heap->used()) { + gpu->currentCommandList()->addRecycledResource(heap); + } + return heap->allocateTable(count); } // No space in current heap, pop off list fDescriptorHeaps.pop_back(); @@ -114,6 +107,7 @@ sk_sp GrD3DDescriptorTableManager::HeapPool::allocateTable fCurrentHeapDescriptorCount = std::min(2*fCurrentHeapDescriptorCount, 2048u); sk_sp heap = GrD3DDescriptorTableManager::Heap::Make(gpu, fHeapType, fCurrentHeapDescriptorCount); + gpu->currentCommandList()->addRecycledResource(heap); fDescriptorHeaps.push_back(heap); return fDescriptorHeaps[fDescriptorHeaps.size() - 1]->allocateTable(count); } diff --git a/src/gpu/d3d/GrD3DDescriptorTableManager.h b/src/gpu/d3d/GrD3DDescriptorTableManager.h index 253f03845f..fe54a1b022 100644 --- a/src/gpu/d3d/GrD3DDescriptorTableManager.h +++ b/src/gpu/d3d/GrD3DDescriptorTableManager.h @@ -17,9 +17,10 @@ class GrD3DGpu; class GrD3DDescriptorTable : public SkRefCnt { public: GrD3DDescriptorTable(D3D12_CPU_DESCRIPTOR_HANDLE baseCPU, D3D12_GPU_DESCRIPTOR_HANDLE baseGPU, - D3D12_DESCRIPTOR_HEAP_TYPE type) + ID3D12DescriptorHeap* heap, D3D12_DESCRIPTOR_HEAP_TYPE type) : fDescriptorTableCpuStart(baseCPU) , fDescriptorTableGpuStart(baseGPU) + , fHeap(heap) , fType(type) {} const D3D12_CPU_DESCRIPTOR_HANDLE* baseCpuDescriptorPtr() { @@ -30,11 +31,13 @@ public: return fDescriptorTableGpuStart; } + ID3D12DescriptorHeap* heap() const { return fHeap; } D3D12_DESCRIPTOR_HEAP_TYPE type() const { return fType; } private: D3D12_CPU_DESCRIPTOR_HANDLE fDescriptorTableCpuStart; D3D12_GPU_DESCRIPTOR_HANDLE fDescriptorTableGpuStart; + ID3D12DescriptorHeap* fHeap; D3D12_DESCRIPTOR_HEAP_TYPE fType; }; @@ -112,7 +115,6 @@ private: unsigned int fCurrentHeapDescriptorCount; }; - void setHeaps(GrD3DGpu*); void recycle(Heap*); HeapPool fShaderViewDescriptorPool; diff --git a/src/gpu/d3d/GrD3DGpu.cpp b/src/gpu/d3d/GrD3DGpu.cpp index 7163802d07..c9c8e5bbe7 100644 --- a/src/gpu/d3d/GrD3DGpu.cpp +++ b/src/gpu/d3d/GrD3DGpu.cpp @@ -1136,9 +1136,6 @@ bool GrD3DGpu::onRegenerateMipMapLevels(GrTexture * tex) { samplers[0] = fResourceProvider.findOrCreateCompatibleSampler(samplerState); this->currentCommandList()->addSampledTextureRef(uavTexture.get()); sk_sp samplerTable = fResourceProvider.findOrCreateSamplerTable(samplers); - this->currentCommandList()->setComputeRootDescriptorTable( - static_cast(GrD3DRootSignature::ParamIndex::kSamplerDescriptorTable), - samplerTable->baseGpuDescriptor()); // Transition the top subresource to be readable in the compute shader D3D12_RESOURCE_STATES currentResourceState = uavTexture->currentState(); @@ -1189,12 +1186,18 @@ bool GrD3DGpu::onRegenerateMipMapLevels(GrTexture * tex) { shaderViews.push_back(uavHandle.fHandle); fMipmapCPUDescriptors.push_back(uavHandle); - // set up and bind shaderView descriptor table + // set up shaderView descriptor table sk_sp srvTable = fResourceProvider.findOrCreateShaderViewTable(shaderViews); + + // bind both descriptor tables + this->currentCommandList()->setDescriptorHeaps(srvTable->heap(), samplerTable->heap()); this->currentCommandList()->setComputeRootDescriptorTable( (unsigned int)GrD3DRootSignature::ParamIndex::kShaderViewDescriptorTable, srvTable->baseGpuDescriptor()); + this->currentCommandList()->setComputeRootDescriptorTable( + static_cast(GrD3DRootSignature::ParamIndex::kSamplerDescriptorTable), + samplerTable->baseGpuDescriptor()); // Transition resource state of dstMip subresource so we can write to it barrier.Subresource = dstMip; diff --git a/src/gpu/d3d/GrD3DPipelineState.cpp b/src/gpu/d3d/GrD3DPipelineState.cpp index 9db5ddf488..916df8a2be 100644 --- a/src/gpu/d3d/GrD3DPipelineState.cpp +++ b/src/gpu/d3d/GrD3DPipelineState.cpp @@ -131,16 +131,19 @@ void GrD3DPipelineState::setAndBindTextures(GrD3DGpu* gpu, // fill in descriptor tables and bind to root signature if (fNumSamplers > 0) { - // set up and bind shader resource view table + // set up descriptor tables and bind heaps sk_sp srvTable = gpu->resourceProvider().findOrCreateShaderViewTable(shaderResourceViews); + sk_sp samplerTable = + gpu->resourceProvider().findOrCreateSamplerTable(samplers); + gpu->currentCommandList()->setDescriptorHeaps(srvTable->heap(), samplerTable->heap()); + + // bind shader resource view table gpu->currentCommandList()->setGraphicsRootDescriptorTable( (unsigned int)GrD3DRootSignature::ParamIndex::kShaderViewDescriptorTable, srvTable->baseGpuDescriptor()); - // set up and bind sampler table - sk_sp samplerTable = - gpu->resourceProvider().findOrCreateSamplerTable(samplers); + // bind sampler table gpu->currentCommandList()->setGraphicsRootDescriptorTable( (unsigned int)GrD3DRootSignature::ParamIndex::kSamplerDescriptorTable, samplerTable->baseGpuDescriptor());