Skip to content

Commit a4a132c

Browse files
egdanielSkia Commit-Bot
egdaniel
authored and
Skia Commit-Bot
committed
Fix tracking of d3d cpu descriptor heaps.
The way you access individual descriptors in a heap for d3d is that you get a handle to the start of the heap, get the stride between descriptors, and then you can do start + index * stride. Previously we were assuming that the ptr handles for different heaps would not overlap. Thus when recycling an individual descriptor we were using its handle ptr to decide which heap it belonged to. However, the spec doesn't require that these handles act like actual pointer to memory. On a least some machines it was noticed that the start handle for different heaps just incremented by 1 for each new heap even though each used 32 as their stride. Long story short it is not valid to do any sort of comparison of handles between different heaps Thus we have to explicitly track which handles come from which heaps so we know of to correctly recycle them. This fixes the main/most common crash we were seeing on d3d viewer. Change-Id: I5107104b245dc13a3b6b21e9e04b88ed8f55c80b Reviewed-on: https://skia-review.googlesource.com/c/skia/+/296448 Reviewed-by: Brian Osman <brianosman@google.com> Commit-Queue: Greg Daniel <egdaniel@google.com>
1 parent a08bde6 commit a4a132c

13 files changed

+187
-160
lines changed

‎src/gpu/d3d/GrD3DCpuDescriptorManager.cpp‎

Lines changed: 32 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -15,57 +15,58 @@ GrD3DCpuDescriptorManager::GrD3DCpuDescriptorManager(GrD3DGpu* gpu)
1515
, fCBVSRVDescriptorPool(gpu, D3D12_DESCRIPTOR_HEAP_TYPE_CBV_SRV_UAV)
1616
, fSamplerDescriptorPool(gpu, D3D12_DESCRIPTOR_HEAP_TYPE_SAMPLER) {}
1717

18-
D3D12_CPU_DESCRIPTOR_HANDLE GrD3DCpuDescriptorManager::createRenderTargetView(
18+
GrD3DDescriptorHeap::CPUHandle GrD3DCpuDescriptorManager::createRenderTargetView(
1919
GrD3DGpu* gpu, ID3D12Resource* textureResource) {
20-
D3D12_CPU_DESCRIPTOR_HANDLE descriptor = fRTVDescriptorPool.allocateHandle(gpu);
21-
gpu->device()->CreateRenderTargetView(textureResource, nullptr, descriptor);
20+
const GrD3DDescriptorHeap::CPUHandle& descriptor = fRTVDescriptorPool.allocateHandle(gpu);
21+
gpu->device()->CreateRenderTargetView(textureResource, nullptr, descriptor.fHandle);
2222
return descriptor;
2323
}
2424

2525
void GrD3DCpuDescriptorManager::recycleRenderTargetView(
26-
D3D12_CPU_DESCRIPTOR_HANDLE* rtvDescriptor) {
26+
const GrD3DDescriptorHeap::CPUHandle& rtvDescriptor) {
2727
fRTVDescriptorPool.releaseHandle(rtvDescriptor);
2828
}
2929

30-
D3D12_CPU_DESCRIPTOR_HANDLE GrD3DCpuDescriptorManager::createDepthStencilView(
30+
GrD3DDescriptorHeap::CPUHandle GrD3DCpuDescriptorManager::createDepthStencilView(
3131
GrD3DGpu* gpu, ID3D12Resource* textureResource) {
32-
D3D12_CPU_DESCRIPTOR_HANDLE descriptor = fDSVDescriptorPool.allocateHandle(gpu);
33-
gpu->device()->CreateDepthStencilView(textureResource, nullptr, descriptor);
32+
const GrD3DDescriptorHeap::CPUHandle& descriptor = fDSVDescriptorPool.allocateHandle(gpu);
33+
gpu->device()->CreateDepthStencilView(textureResource, nullptr, descriptor.fHandle);
3434
return descriptor;
3535
}
3636

3737
void GrD3DCpuDescriptorManager::recycleDepthStencilView(
38-
D3D12_CPU_DESCRIPTOR_HANDLE* dsvDescriptor) {
38+
const GrD3DDescriptorHeap::CPUHandle& dsvDescriptor) {
3939
fDSVDescriptorPool.releaseHandle(dsvDescriptor);
4040
}
4141

42-
D3D12_CPU_DESCRIPTOR_HANDLE GrD3DCpuDescriptorManager::createConstantBufferView(
42+
GrD3DDescriptorHeap::CPUHandle GrD3DCpuDescriptorManager::createConstantBufferView(
4343
GrD3DGpu* gpu, ID3D12Resource* bufferResource, size_t offset, size_t size) {
44-
D3D12_CPU_DESCRIPTOR_HANDLE descriptor = fCBVSRVDescriptorPool.allocateHandle(gpu);
44+
const GrD3DDescriptorHeap::CPUHandle& descriptor = fCBVSRVDescriptorPool.allocateHandle(gpu);
4545
D3D12_CONSTANT_BUFFER_VIEW_DESC desc = {};
4646
desc.BufferLocation = bufferResource->GetGPUVirtualAddress() + offset;
4747
desc.SizeInBytes = size;
48-
gpu->device()->CreateConstantBufferView(&desc, descriptor);
48+
gpu->device()->CreateConstantBufferView(&desc, descriptor.fHandle);
4949
return descriptor;
5050
}
5151

52-
D3D12_CPU_DESCRIPTOR_HANDLE GrD3DCpuDescriptorManager::createShaderResourceView(
52+
GrD3DDescriptorHeap::CPUHandle GrD3DCpuDescriptorManager::createShaderResourceView(
5353
GrD3DGpu* gpu, ID3D12Resource* resource) {
54-
D3D12_CPU_DESCRIPTOR_HANDLE descriptor = fCBVSRVDescriptorPool.allocateHandle(gpu);
54+
const GrD3DDescriptorHeap::CPUHandle& descriptor = fCBVSRVDescriptorPool.allocateHandle(gpu);
5555
// TODO: for 4:2:0 YUV formats we'll need to map two different views, one for Y and one for UV.
5656
// For now map the entire resource.
57-
gpu->device()->CreateShaderResourceView(resource, nullptr, descriptor);
57+
gpu->device()->CreateShaderResourceView(resource, nullptr, descriptor.fHandle);
5858
return descriptor;
5959
}
6060

61-
void GrD3DCpuDescriptorManager::recycleConstantOrShaderView(D3D12_CPU_DESCRIPTOR_HANDLE* view) {
61+
void GrD3DCpuDescriptorManager::recycleConstantOrShaderView(
62+
const GrD3DDescriptorHeap::CPUHandle& view) {
6263
fCBVSRVDescriptorPool.releaseHandle(view);
6364
}
6465

65-
D3D12_CPU_DESCRIPTOR_HANDLE GrD3DCpuDescriptorManager::createSampler(
66+
GrD3DDescriptorHeap::CPUHandle GrD3DCpuDescriptorManager::createSampler(
6667
GrD3DGpu* gpu, D3D12_FILTER filter, D3D12_TEXTURE_ADDRESS_MODE addressModeU,
6768
D3D12_TEXTURE_ADDRESS_MODE addressModeV) {
68-
D3D12_CPU_DESCRIPTOR_HANDLE descriptor = fSamplerDescriptorPool.allocateHandle(gpu);
69+
const GrD3DDescriptorHeap::CPUHandle& descriptor = fSamplerDescriptorPool.allocateHandle(gpu);
6970
D3D12_SAMPLER_DESC desc = {};
7071
desc.Filter = filter;
7172
desc.AddressU = addressModeU;
@@ -78,11 +79,12 @@ D3D12_CPU_DESCRIPTOR_HANDLE GrD3DCpuDescriptorManager::createSampler(
7879
desc.MinLOD = 0;
7980
desc.MaxLOD = SK_ScalarMax;
8081

81-
gpu->device()->CreateSampler(&desc, descriptor);
82+
gpu->device()->CreateSampler(&desc, descriptor.fHandle);
8283
return descriptor;
8384
}
8485

85-
void GrD3DCpuDescriptorManager::recycleSampler(D3D12_CPU_DESCRIPTOR_HANDLE* samplerDescriptor) {
86+
void GrD3DCpuDescriptorManager::recycleSampler(
87+
const GrD3DDescriptorHeap::CPUHandle& samplerDescriptor) {
8688
fSamplerDescriptorPool.releaseHandle(samplerDescriptor);
8789
}
8890

@@ -99,23 +101,19 @@ std::unique_ptr<GrD3DCpuDescriptorManager::Heap> GrD3DCpuDescriptorManager::Heap
99101
return std::unique_ptr<Heap>(new Heap(heap, numDescriptors));
100102
}
101103

102-
D3D12_CPU_DESCRIPTOR_HANDLE GrD3DCpuDescriptorManager::Heap::allocateCPUHandle() {
104+
GrD3DDescriptorHeap::CPUHandle GrD3DCpuDescriptorManager::Heap::allocateCPUHandle() {
103105
SkBitSet::OptionalIndex freeBlock = fFreeBlocks.findFirst();
104106
SkASSERT(freeBlock);
105107
fFreeBlocks.reset(*freeBlock);
106108
--fFreeCount;
107109
return fHeap->getCPUHandle(*freeBlock);
108110
}
109111

110-
bool GrD3DCpuDescriptorManager::Heap::freeCPUHandle(D3D12_CPU_DESCRIPTOR_HANDLE* handle) {
111-
size_t index;
112-
if (!fHeap->getIndex(*handle, &index)) {
113-
return false;
114-
}
112+
void GrD3DCpuDescriptorManager::Heap::freeCPUHandle(const GrD3DDescriptorHeap::CPUHandle& handle) {
113+
SkASSERT(this->ownsHandle(handle));
114+
size_t index = fHeap->getIndex(handle);
115115
fFreeBlocks.set(index);
116116
++fFreeCount;
117-
handle->ptr = 0;
118-
return true;
119117
}
120118

121119
////////////////////////////////////////////////////////////////////////////////////////////////
@@ -128,10 +126,11 @@ GrD3DCpuDescriptorManager::HeapPool::HeapPool(GrD3DGpu* gpu, D3D12_DESCRIPTOR_HE
128126
fDescriptorHeaps.push_back(std::move(heap));
129127
}
130128

131-
D3D12_CPU_DESCRIPTOR_HANDLE GrD3DCpuDescriptorManager::HeapPool::allocateHandle(GrD3DGpu* gpu) {
129+
GrD3DDescriptorHeap::CPUHandle GrD3DCpuDescriptorManager::HeapPool::allocateHandle(
130+
GrD3DGpu* gpu) {
132131
for (unsigned int i = 0; i < fDescriptorHeaps.size(); ++i) {
133132
if (fDescriptorHeaps[i]->canAllocate()) {
134-
D3D12_CPU_DESCRIPTOR_HANDLE handle = fDescriptorHeaps[i]->allocateCPUHandle();
133+
GrD3DDescriptorHeap::CPUHandle handle = fDescriptorHeaps[i]->allocateCPUHandle();
135134
return handle;
136135
}
137136
}
@@ -142,15 +141,16 @@ D3D12_CPU_DESCRIPTOR_HANDLE GrD3DCpuDescriptorManager::HeapPool::allocateHandle(
142141

143142
fDescriptorHeaps.push_back(std::move(heap));
144143
fMaxAvailableDescriptors *= 2;
145-
D3D12_CPU_DESCRIPTOR_HANDLE handle =
144+
GrD3DDescriptorHeap::CPUHandle handle =
146145
fDescriptorHeaps[fDescriptorHeaps.size() - 1]->allocateCPUHandle();
147146
return handle;
148147
}
149148

150149
void GrD3DCpuDescriptorManager::HeapPool::releaseHandle(
151-
D3D12_CPU_DESCRIPTOR_HANDLE* dsvDescriptor) {
150+
const GrD3DDescriptorHeap::CPUHandle& dsvDescriptor) {
152151
for (unsigned int i = 0; i < fDescriptorHeaps.size(); ++i) {
153-
if (fDescriptorHeaps[i]->freeCPUHandle(dsvDescriptor)) {
152+
if (fDescriptorHeaps[i]->ownsHandle(dsvDescriptor)) {
153+
fDescriptorHeaps[i]->freeCPUHandle(dsvDescriptor);
154154
return;
155155
}
156156
}

‎src/gpu/d3d/GrD3DCpuDescriptorManager.h‎

Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -16,33 +16,39 @@ class GrD3DCpuDescriptorManager {
1616
public:
1717
GrD3DCpuDescriptorManager(GrD3DGpu*);
1818

19-
D3D12_CPU_DESCRIPTOR_HANDLE createRenderTargetView(GrD3DGpu*, ID3D12Resource* textureResource);
20-
void recycleRenderTargetView(D3D12_CPU_DESCRIPTOR_HANDLE*);
21-
22-
D3D12_CPU_DESCRIPTOR_HANDLE createDepthStencilView(GrD3DGpu*, ID3D12Resource* textureResource);
23-
void recycleDepthStencilView(D3D12_CPU_DESCRIPTOR_HANDLE*);
24-
25-
D3D12_CPU_DESCRIPTOR_HANDLE createConstantBufferView(GrD3DGpu*,
26-
ID3D12Resource* bufferResource,
27-
size_t offset,
28-
size_t size);
29-
D3D12_CPU_DESCRIPTOR_HANDLE createShaderResourceView(GrD3DGpu*,
30-
ID3D12Resource* resource);
31-
void recycleConstantOrShaderView(D3D12_CPU_DESCRIPTOR_HANDLE*);
32-
33-
D3D12_CPU_DESCRIPTOR_HANDLE createSampler(GrD3DGpu*, D3D12_FILTER filter,
34-
D3D12_TEXTURE_ADDRESS_MODE addressModeU,
35-
D3D12_TEXTURE_ADDRESS_MODE addressModeV);
36-
void recycleSampler(D3D12_CPU_DESCRIPTOR_HANDLE*);
19+
GrD3DDescriptorHeap::CPUHandle createRenderTargetView(GrD3DGpu*,
20+
ID3D12Resource* textureResource);
21+
void recycleRenderTargetView(const GrD3DDescriptorHeap::CPUHandle&);
22+
23+
GrD3DDescriptorHeap::CPUHandle createDepthStencilView(GrD3DGpu*,
24+
ID3D12Resource* textureResource);
25+
void recycleDepthStencilView(const GrD3DDescriptorHeap::CPUHandle&);
26+
27+
GrD3DDescriptorHeap::CPUHandle createConstantBufferView(GrD3DGpu*,
28+
ID3D12Resource* bufferResource,
29+
size_t offset,
30+
size_t size);
31+
GrD3DDescriptorHeap::CPUHandle createShaderResourceView(GrD3DGpu*,
32+
ID3D12Resource* resource);
33+
void recycleConstantOrShaderView(const GrD3DDescriptorHeap::CPUHandle&);
34+
35+
GrD3DDescriptorHeap::CPUHandle createSampler(GrD3DGpu*,
36+
D3D12_FILTER filter,
37+
D3D12_TEXTURE_ADDRESS_MODE addressModeU,
38+
D3D12_TEXTURE_ADDRESS_MODE addressModeV);
39+
void recycleSampler(const GrD3DDescriptorHeap::CPUHandle&);
3740

3841
private:
3942
class Heap {
4043
public:
4144
static std::unique_ptr<Heap> Make(GrD3DGpu* gpu, D3D12_DESCRIPTOR_HEAP_TYPE type,
4245
unsigned int numDescriptors);
4346

44-
D3D12_CPU_DESCRIPTOR_HANDLE allocateCPUHandle();
45-
bool freeCPUHandle(D3D12_CPU_DESCRIPTOR_HANDLE*);
47+
GrD3DDescriptorHeap::CPUHandle allocateCPUHandle();
48+
void freeCPUHandle(const GrD3DDescriptorHeap::CPUHandle&);
49+
bool ownsHandle(const GrD3DDescriptorHeap::CPUHandle& handle) {
50+
return handle.fHeapID == fHeap->uniqueID();
51+
}
4652

4753
bool canAllocate() { return fFreeCount > 0; }
4854

@@ -65,8 +71,8 @@ class GrD3DCpuDescriptorManager {
6571
public:
6672
HeapPool(GrD3DGpu*, D3D12_DESCRIPTOR_HEAP_TYPE);
6773

68-
D3D12_CPU_DESCRIPTOR_HANDLE allocateHandle(GrD3DGpu*);
69-
void releaseHandle(D3D12_CPU_DESCRIPTOR_HANDLE*);
74+
GrD3DDescriptorHeap::CPUHandle allocateHandle(GrD3DGpu*);
75+
void releaseHandle(const GrD3DDescriptorHeap::CPUHandle&);
7076

7177
private:
7278
std::vector<std::unique_ptr<Heap>> fDescriptorHeaps;

‎src/gpu/d3d/GrD3DDescriptorHeap.cpp‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,23 +28,24 @@ std::unique_ptr<GrD3DDescriptorHeap> GrD3DDescriptorHeap::Make(GrD3DGpu* gpu,
2828
GrD3DDescriptorHeap::GrD3DDescriptorHeap(const gr_cp<ID3D12DescriptorHeap>& heap,
2929
unsigned int handleIncrementSize)
3030
: fHeap(heap)
31-
, fHandleIncrementSize(handleIncrementSize) {
31+
, fHandleIncrementSize(handleIncrementSize)
32+
, fUniqueID(GenID()) {
3233
fCPUHeapStart = fHeap->GetCPUDescriptorHandleForHeapStart();
3334
fGPUHeapStart = fHeap->GetGPUDescriptorHandleForHeapStart();
3435
}
3536

36-
D3D12_CPU_DESCRIPTOR_HANDLE GrD3DDescriptorHeap::getCPUHandle(unsigned int index) {
37+
GrD3DDescriptorHeap::CPUHandle GrD3DDescriptorHeap::getCPUHandle(unsigned int index) {
3738
SkASSERT(index < fHeap->GetDesc().NumDescriptors);
3839
D3D12_CPU_DESCRIPTOR_HANDLE handle = fCPUHeapStart;
3940
handle.ptr += index * fHandleIncrementSize;
40-
return handle;
41+
return {handle, fUniqueID};
4142
}
4243

43-
D3D12_GPU_DESCRIPTOR_HANDLE GrD3DDescriptorHeap::getGPUHandle(unsigned int index) {
44+
GrD3DDescriptorHeap::GPUHandle GrD3DDescriptorHeap::getGPUHandle(unsigned int index) {
4445
SkASSERT(index < fHeap->GetDesc().NumDescriptors);
4546
D3D12_GPU_DESCRIPTOR_HANDLE handle = fGPUHeapStart;
4647
handle.ptr += index * fHandleIncrementSize;
47-
return handle;
48+
return {handle, fUniqueID};
4849
}
4950

5051

‎src/gpu/d3d/GrD3DDescriptorHeap.h‎

Lines changed: 36 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -22,44 +22,56 @@ class GrD3DDescriptorHeap {
2222

2323
~GrD3DDescriptorHeap() = default;
2424

25-
D3D12_CPU_DESCRIPTOR_HANDLE getCPUHandle(unsigned int index); // write-only if shader-visible
26-
D3D12_GPU_DESCRIPTOR_HANDLE getGPUHandle(unsigned int index);
25+
uint32_t uniqueID() const { return fUniqueID; }
26+
27+
struct CPUHandle {
28+
D3D12_CPU_DESCRIPTOR_HANDLE fHandle;
29+
uint32_t fHeapID;
30+
};
31+
32+
struct GPUHandle {
33+
D3D12_GPU_DESCRIPTOR_HANDLE fHandle;
34+
uint32_t fHeapID;
35+
};
36+
37+
CPUHandle getCPUHandle(unsigned int index); // write-only if shader-visible
38+
GPUHandle getGPUHandle(unsigned int index);
2739
ID3D12DescriptorHeap* descriptorHeap() const { return fHeap.get(); }
2840
size_t handleIncrementSize() { return fHandleIncrementSize; }
2941

30-
bool getIndex(D3D12_CPU_DESCRIPTOR_HANDLE handle, size_t* indexPtr) {
31-
if (handle.ptr < fCPUHeapStart.ptr) {
32-
return false;
33-
}
34-
size_t index = (handle.ptr - fCPUHeapStart.ptr) / fHandleIncrementSize;
35-
if (index >= fHeap->GetDesc().NumDescriptors) {
36-
return false;
37-
}
38-
SkASSERT(handle.ptr == fCPUHeapStart.ptr + index * fHandleIncrementSize);
39-
*indexPtr = index;
40-
return true;
42+
size_t getIndex(const CPUHandle& handle) {
43+
SkASSERT(handle.fHeapID == fUniqueID);
44+
size_t index = (handle.fHandle.ptr - fCPUHeapStart.ptr) / fHandleIncrementSize;
45+
SkASSERT(index < fHeap->GetDesc().NumDescriptors);
46+
SkASSERT(handle.fHandle.ptr == fCPUHeapStart.ptr + index * fHandleIncrementSize);
47+
return index;
4148
}
4249

43-
bool getIndex(D3D12_GPU_DESCRIPTOR_HANDLE handle, size_t* indexPtr) {
44-
if (handle.ptr < fGPUHeapStart.ptr) {
45-
return false;
46-
}
47-
size_t index = (handle.ptr - fGPUHeapStart.ptr) / fHandleIncrementSize;
48-
if (index >= fHeap->GetDesc().NumDescriptors) {
49-
return false;
50-
}
51-
SkASSERT(handle.ptr == fGPUHeapStart.ptr + index * fHandleIncrementSize);
52-
*indexPtr = index;
53-
return true;
50+
size_t getIndex(const GPUHandle& handle) {
51+
SkASSERT(handle.fHeapID == fUniqueID);
52+
size_t index = (handle.fHandle.ptr - fCPUHeapStart.ptr) / fHandleIncrementSize;
53+
SkASSERT(index < fHeap->GetDesc().NumDescriptors);
54+
SkASSERT(handle.fHandle.ptr == fCPUHeapStart.ptr + index * fHandleIncrementSize);
55+
return index;
5456
}
5557

5658
protected:
5759
GrD3DDescriptorHeap(const gr_cp<ID3D12DescriptorHeap>&, unsigned int handleIncrementSize);
5860

61+
static uint32_t GenID() {
62+
static std::atomic<uint32_t> nextID{1};
63+
uint32_t id;
64+
do {
65+
id = nextID++;
66+
} while (id == SK_InvalidUniqueID);
67+
return id;
68+
}
69+
5970
gr_cp<ID3D12DescriptorHeap> fHeap;
6071
size_t fHandleIncrementSize;
6172
D3D12_CPU_DESCRIPTOR_HANDLE fCPUHeapStart;
6273
D3D12_GPU_DESCRIPTOR_HANDLE fGPUHeapStart;
74+
uint32_t fUniqueID;
6375
};
6476

6577
#endif

‎src/gpu/d3d/GrD3DDescriptorTableManager.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,8 @@ std::unique_ptr<GrD3DDescriptorTable> GrD3DDescriptorTableManager::Heap::allocat
8080
unsigned int startIndex = fNextAvailable;
8181
fNextAvailable += count;
8282
return std::unique_ptr<GrD3DDescriptorTable>(
83-
new GrD3DDescriptorTable(fHeap->getCPUHandle(startIndex),
84-
fHeap->getGPUHandle(startIndex), fType));
83+
new GrD3DDescriptorTable(fHeap->getCPUHandle(startIndex).fHandle,
84+
fHeap->getGPUHandle(startIndex).fHandle, fType));
8585
}
8686

8787
void GrD3DDescriptorTableManager::Heap::onRecycle() const {

0 commit comments

Comments
 (0)