Skip to content

Commit 5fba9ae

Browse files
jvanverthSkia Commit-Bot
jvanverth
authored and
Skia Commit-Bot
committed
Revert "Reland "Use Microsoft's ComPtr to wrap D3D12 objects.""
This reverts commit b8ae7fa. Reason for revert: Introduced a memory leak Original change's description: > Reland "Use Microsoft's ComPtr to wrap D3D12 objects." > > This is a reland of 0ef0491 > > Original change's description: > > Use Microsoft's ComPtr to wrap D3D12 objects. > > > > Change-Id: I4bd173428a2b65f0bc1994fb794ef9d4d68d5ba0 > > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/314957 > > Reviewed-by: Brian Salomon <bsalomon@google.com> > > Reviewed-by: Greg Daniel <egdaniel@google.com> > > Commit-Queue: Jim Van Verth <jvanverth@google.com> > > Change-Id: Id0199db4061c67ed53da35e74dc31a004744be95 > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/315655 > Reviewed-by: Brian Salomon <bsalomon@google.com> > Commit-Queue: Jim Van Verth <jvanverth@google.com> TBR=egdaniel@google.com,jvanverth@google.com,bsalomon@google.com # Not skipping CQ checks because original CL landed > 1 day ago. Change-Id: I3f8744668558f6b8f4f367eeeeff2f6aa2c36992 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/318209 Reviewed-by: Jim Van Verth <jvanverth@google.com> Commit-Queue: Jim Van Verth <jvanverth@google.com>
1 parent 2b788b1 commit 5fba9ae

28 files changed

+268
-148
lines changed

‎RELEASE_NOTES.txt‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,6 @@ Milestone 87
1515
the deprecated, expanding clip ops.
1616
https://review.skia.org/317209
1717

18-
* Switch to using Microsoft::WRL::ComPtr for wrapping D3D12 objects.
19-
https://review.skia.org/314957
20-
2118
* GPU backends now properly honor the SkFilterQuality when calling drawAtlas.
2219
https://review.skia.org/313081
2320

‎include/gpu/d3d/GrD3DBackendContext.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@
2525
// The BackendContext contains all of the base D3D objects needed by the GrD3DGpu. The assumption
2626
// is that the client will set these up and pass them to the GrD3DGpu constructor.
2727
struct SK_API GrD3DBackendContext {
28-
ComPtr<IDXGIAdapter1> fAdapter;
29-
ComPtr<ID3D12Device> fDevice;
30-
ComPtr<ID3D12CommandQueue> fQueue;
28+
gr_cp<IDXGIAdapter1> fAdapter;
29+
gr_cp<ID3D12Device> fDevice;
30+
gr_cp<ID3D12CommandQueue> fQueue;
3131
GrProtected fProtectedContext = GrProtected::kNo;
3232
};
3333

‎include/gpu/d3d/GrD3DTypes.h‎

Lines changed: 129 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,138 @@
2424
#include "include/gpu/d3d/GrD3DTypesMinimal.h"
2525
#include <d3d12.h>
2626
#include <dxgi1_4.h>
27-
#include <wrl/client.h>
2827

29-
using Microsoft::WRL::ComPtr;
28+
/** Check if the argument is non-null, and if so, call obj->AddRef() and return obj.
29+
*/
30+
template <typename T> static inline T* GrSafeComAddRef(T* obj) {
31+
if (obj) {
32+
obj->AddRef();
33+
}
34+
return obj;
35+
}
36+
37+
/** Check if the argument is non-null, and if so, call obj->Release()
38+
*/
39+
template <typename T> static inline void GrSafeComRelease(T* obj) {
40+
if (obj) {
41+
obj->Release();
42+
}
43+
}
44+
45+
template <typename T> class gr_cp {
46+
public:
47+
using element_type = T;
48+
49+
constexpr gr_cp() : fObject(nullptr) {}
50+
constexpr gr_cp(std::nullptr_t) : fObject(nullptr) {}
51+
52+
/**
53+
* Shares the underlying object by calling AddRef(), so that both the argument and the newly
54+
* created gr_cp both have a reference to it.
55+
*/
56+
gr_cp(const gr_cp<T>& that) : fObject(GrSafeComAddRef(that.get())) {}
57+
58+
/**
59+
* Move the underlying object from the argument to the newly created gr_cp. Afterwards only
60+
* the new gr_cp will have a reference to the object, and the argument will point to null.
61+
* No call to AddRef() or Release() will be made.
62+
*/
63+
gr_cp(gr_cp<T>&& that) : fObject(that.release()) {}
64+
65+
/**
66+
* Adopt the bare object into the newly created gr_cp.
67+
* No call to AddRef() or Release() will be made.
68+
*/
69+
explicit gr_cp(T* obj) {
70+
fObject = obj;
71+
}
72+
73+
/**
74+
* Calls Release() on the underlying object pointer.
75+
*/
76+
~gr_cp() {
77+
GrSafeComRelease(fObject);
78+
SkDEBUGCODE(fObject = nullptr);
79+
}
80+
81+
/**
82+
* Shares the underlying object referenced by the argument by calling AddRef() on it. If this
83+
* gr_cp previously had a reference to an object (i.e. not null) it will call Release()
84+
* on that object.
85+
*/
86+
gr_cp<T>& operator=(const gr_cp<T>& that) {
87+
if (this != &that) {
88+
this->reset(GrSafeComAddRef(that.get()));
89+
}
90+
return *this;
91+
}
92+
93+
/**
94+
* Move the underlying object from the argument to the gr_cp. If the gr_cp
95+
* previously held a reference to another object, Release() will be called on that object.
96+
* No call to AddRef() will be made.
97+
*/
98+
gr_cp<T>& operator=(gr_cp<T>&& that) {
99+
this->reset(that.release());
100+
return *this;
101+
}
102+
103+
explicit operator bool() const { return this->get() != nullptr; }
104+
105+
T* get() const { return fObject; }
106+
T* operator->() const { return fObject; }
107+
T** operator&() { return &fObject; }
108+
109+
/**
110+
* Adopt the new object, and call Release() on any previously held object (if not null).
111+
* No call to AddRef() will be made.
112+
*/
113+
void reset(T* object = nullptr) {
114+
T* oldObject = fObject;
115+
fObject = object;
116+
GrSafeComRelease(oldObject);
117+
}
118+
119+
/**
120+
* Shares the new object by calling AddRef() on it. If this gr_cp previously had a
121+
* reference to an object (i.e. not null) it will call Release() on that object.
122+
*/
123+
void retain(T* object) {
124+
if (this->fObject != object) {
125+
this->reset(GrSafeComAddRef(object));
126+
}
127+
}
128+
129+
/**
130+
* Return the original object, and set the internal object to nullptr.
131+
* The caller must assume ownership of the object, and manage its reference count directly.
132+
* No call to Release() will be made.
133+
*/
134+
T* SK_WARN_UNUSED_RESULT release() {
135+
T* obj = fObject;
136+
fObject = nullptr;
137+
return obj;
138+
}
139+
140+
private:
141+
T* fObject;
142+
};
143+
144+
template <typename T> inline bool operator==(const gr_cp<T>& a,
145+
const gr_cp<T>& b) {
146+
return a.get() == b.get();
147+
}
148+
149+
template <typename T> inline bool operator!=(const gr_cp<T>& a,
150+
const gr_cp<T>& b) {
151+
return a.get() != b.get();
152+
}
30153

31154
// Note: there is no notion of Borrowed or Adopted resources in the D3D backend,
32155
// so Ganesh will ref fResource once it's asked to wrap it.
33156
// Clients are responsible for releasing their own ref to avoid memory leaks.
34157
struct GrD3DTextureResourceInfo {
35-
ComPtr<ID3D12Resource> fResource;
158+
gr_cp<ID3D12Resource> fResource;
36159
D3D12_RESOURCE_STATES fResourceState;
37160
DXGI_FORMAT fFormat;
38161
uint32_t fLevelCount;
@@ -47,7 +170,7 @@ struct GrD3DTextureResourceInfo {
47170
, fSampleQualityPattern(DXGI_STANDARD_MULTISAMPLE_QUALITY_PATTERN)
48171
, fProtected(GrProtected::kNo) {}
49172

50-
GrD3DTextureResourceInfo(const ComPtr<ID3D12Resource>& resource,
173+
GrD3DTextureResourceInfo(ID3D12Resource* resource,
51174
D3D12_RESOURCE_STATES resourceState,
52175
DXGI_FORMAT format,
53176
uint32_t levelCount,
@@ -84,8 +207,8 @@ struct GrD3DFenceInfo {
84207
, fValue(0) {
85208
}
86209

87-
ComPtr<ID3D12Fence> fFence;
88-
uint64_t fValue; // signal value for the fence
210+
gr_cp<ID3D12Fence> fFence;
211+
uint64_t fValue; // signal value for the fence
89212
};
90213

91214
#endif

‎src/gpu/d3d/GrD3DBuffer.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
#define VALIDATE() do {} while(false)
1717
#endif
1818

19-
static ComPtr<ID3D12Resource> make_d3d_buffer(GrD3DGpu* gpu,
19+
static gr_cp<ID3D12Resource> make_d3d_buffer(GrD3DGpu* gpu,
2020
size_t size,
2121
GrGpuBufferType intendedType,
2222
GrAccessPattern accessPattern,
@@ -61,7 +61,7 @@ static ComPtr<ID3D12Resource> make_d3d_buffer(GrD3DGpu* gpu,
6161
bufferDesc.Layout = D3D12_TEXTURE_LAYOUT_ROW_MAJOR;
6262
bufferDesc.Flags = D3D12_RESOURCE_FLAG_NONE;
6363

64-
ComPtr<ID3D12Resource> resource;
64+
ID3D12Resource* resource;
6565
HRESULT hr = gpu->device()->CreateCommittedResource(
6666
&heapProperties,
6767
D3D12_HEAP_FLAG_NONE,
@@ -73,7 +73,7 @@ static ComPtr<ID3D12Resource> make_d3d_buffer(GrD3DGpu* gpu,
7373
return nullptr;
7474
}
7575

76-
return resource;
76+
return gr_cp<ID3D12Resource>(resource);
7777
}
7878

7979
sk_sp<GrD3DBuffer> GrD3DBuffer::Make(GrD3DGpu* gpu, size_t size, GrGpuBufferType intendedType,
@@ -82,8 +82,8 @@ sk_sp<GrD3DBuffer> GrD3DBuffer::Make(GrD3DGpu* gpu, size_t size, GrGpuBufferType
8282
D3D12_RESOURCE_STATES resourceState;
8383

8484

85-
ComPtr<ID3D12Resource> resource = make_d3d_buffer(gpu, size, intendedType, accessPattern,
86-
&resourceState);
85+
gr_cp<ID3D12Resource> resource = make_d3d_buffer(gpu, size, intendedType, accessPattern,
86+
&resourceState);
8787
if (!resource) {
8888
return nullptr;
8989
}
@@ -93,7 +93,7 @@ sk_sp<GrD3DBuffer> GrD3DBuffer::Make(GrD3DGpu* gpu, size_t size, GrGpuBufferType
9393
}
9494

9595
GrD3DBuffer::GrD3DBuffer(GrD3DGpu* gpu, size_t size, GrGpuBufferType intendedType,
96-
GrAccessPattern accessPattern, ComPtr<ID3D12Resource> bufferResource,
96+
GrAccessPattern accessPattern, gr_cp<ID3D12Resource> bufferResource,
9797
D3D12_RESOURCE_STATES resourceState)
9898
: INHERITED(gpu, size, intendedType, accessPattern)
9999
, fResourceState(resourceState)

‎src/gpu/d3d/GrD3DBuffer.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,13 @@ class GrD3DBuffer : public GrGpuBuffer {
2323

2424
ID3D12Resource* d3dResource() const {
2525
SkASSERT(fD3DResource);
26-
return fD3DResource.Get();
26+
return fD3DResource.get();
2727
}
2828

2929
void setResourceState(const GrD3DGpu* gpu, D3D12_RESOURCE_STATES newResourceState);
3030

3131
protected:
32-
GrD3DBuffer(GrD3DGpu*, size_t size, GrGpuBufferType, GrAccessPattern, ComPtr<ID3D12Resource>,
32+
GrD3DBuffer(GrD3DGpu*, size_t size, GrGpuBufferType, GrAccessPattern, gr_cp<ID3D12Resource>,
3333
D3D12_RESOURCE_STATES);
3434

3535
void onAbandon() override;
@@ -52,7 +52,7 @@ class GrD3DBuffer : public GrGpuBuffer {
5252
return reinterpret_cast<GrD3DGpu*>(this->getGpu());
5353
}
5454

55-
ComPtr<ID3D12Resource> fD3DResource;
55+
gr_cp<ID3D12Resource> fD3DResource;
5656
ID3D12Resource* fStagingBuffer = nullptr;
5757
size_t fStagingOffset = 0;
5858

‎src/gpu/d3d/GrD3DCommandList.cpp‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@
1818
#include "src/gpu/d3d/GrD3DTextureResource.h"
1919
#include "src/gpu/d3d/GrD3DUtil.h"
2020

21-
GrD3DCommandList::GrD3DCommandList(ComPtr<ID3D12CommandAllocator> allocator,
22-
ComPtr<ID3D12GraphicsCommandList> commandList)
21+
GrD3DCommandList::GrD3DCommandList(gr_cp<ID3D12CommandAllocator> allocator,
22+
gr_cp<ID3D12GraphicsCommandList> commandList)
2323
: fCommandList(std::move(commandList))
2424
, fAllocator(std::move(allocator)) {
2525
}
@@ -43,7 +43,7 @@ GrD3DCommandList::SubmitResult GrD3DCommandList::submit(ID3D12CommandQueue* queu
4343
return SubmitResult::kFailure;
4444
}
4545
SkASSERT(!fIsActive);
46-
ID3D12CommandList* ppCommandLists[] = { fCommandList.Get() };
46+
ID3D12CommandList* ppCommandLists[] = { fCommandList.get() };
4747
queue->ExecuteCommandLists(1, ppCommandLists);
4848

4949
return SubmitResult::kSuccess;
@@ -52,7 +52,7 @@ GrD3DCommandList::SubmitResult GrD3DCommandList::submit(ID3D12CommandQueue* queu
5252
void GrD3DCommandList::reset() {
5353
SkASSERT(!fIsActive);
5454
GR_D3D_CALL_ERRCHECK(fAllocator->Reset());
55-
GR_D3D_CALL_ERRCHECK(fCommandList->Reset(fAllocator.Get(), nullptr));
55+
GR_D3D_CALL_ERRCHECK(fCommandList->Reset(fAllocator.get(), nullptr));
5656
this->onReset();
5757

5858
this->releaseResources();
@@ -200,21 +200,21 @@ void GrD3DCommandList::addingWork() {
200200
////////////////////////////////////////////////////////////////////////////////////////////////////
201201

202202
std::unique_ptr<GrD3DDirectCommandList> GrD3DDirectCommandList::Make(ID3D12Device* device) {
203-
ComPtr<ID3D12CommandAllocator> allocator;
203+
gr_cp<ID3D12CommandAllocator> allocator;
204204
GR_D3D_CALL_ERRCHECK(device->CreateCommandAllocator(
205205
D3D12_COMMAND_LIST_TYPE_DIRECT, IID_PPV_ARGS(&allocator)));
206206

207-
ComPtr<ID3D12GraphicsCommandList> commandList;
207+
gr_cp<ID3D12GraphicsCommandList> commandList;
208208
GR_D3D_CALL_ERRCHECK(device->CreateCommandList(0, D3D12_COMMAND_LIST_TYPE_DIRECT,
209-
allocator.Get(), nullptr,
209+
allocator.get(), nullptr,
210210
IID_PPV_ARGS(&commandList)));
211211

212212
auto grCL = new GrD3DDirectCommandList(std::move(allocator), std::move(commandList));
213213
return std::unique_ptr<GrD3DDirectCommandList>(grCL);
214214
}
215215

216-
GrD3DDirectCommandList::GrD3DDirectCommandList(ComPtr<ID3D12CommandAllocator> allocator,
217-
ComPtr<ID3D12GraphicsCommandList> commandList)
216+
GrD3DDirectCommandList::GrD3DDirectCommandList(gr_cp<ID3D12CommandAllocator> allocator,
217+
gr_cp<ID3D12GraphicsCommandList> commandList)
218218
: GrD3DCommandList(std::move(allocator), std::move(commandList))
219219
, fCurrentPipelineState(nullptr)
220220
, fCurrentRootSignature(nullptr)
@@ -281,7 +281,7 @@ void GrD3DDirectCommandList::setViewports(unsigned int numViewports,
281281

282282
void GrD3DDirectCommandList::setCenteredSamplePositions(unsigned int numSamples) {
283283
if (!fUsingCenteredSamples && numSamples > 1) {
284-
ComPtr<ID3D12GraphicsCommandList1> commandList1;
284+
gr_cp<ID3D12GraphicsCommandList1> commandList1;
285285
GR_D3D_CALL_ERRCHECK(fCommandList->QueryInterface(IID_PPV_ARGS(&commandList1)));
286286
static D3D12_SAMPLE_POSITION kCenteredSampleLocations[16] = {};
287287
commandList1->SetSamplePositions(numSamples, 1, kCenteredSampleLocations);
@@ -291,7 +291,7 @@ void GrD3DDirectCommandList::setCenteredSamplePositions(unsigned int numSamples)
291291

292292
void GrD3DDirectCommandList::setDefaultSamplePositions() {
293293
if (fUsingCenteredSamples) {
294-
ComPtr<ID3D12GraphicsCommandList1> commandList1;
294+
gr_cp<ID3D12GraphicsCommandList1> commandList1;
295295
GR_D3D_CALL_ERRCHECK(fCommandList->QueryInterface(IID_PPV_ARGS(&commandList1)));
296296
commandList1->SetSamplePositions(0, 0, nullptr);
297297
fUsingCenteredSamples = false;
@@ -444,7 +444,7 @@ void GrD3DDirectCommandList::resolveSubresourceRegion(const GrD3DTextureResource
444444
this->addResource(dstTexture->resource());
445445
this->addResource(srcTexture->resource());
446446

447-
ComPtr<ID3D12GraphicsCommandList1> commandList1;
447+
gr_cp<ID3D12GraphicsCommandList1> commandList1;
448448
HRESULT result = fCommandList->QueryInterface(IID_PPV_ARGS(&commandList1));
449449
if (SUCCEEDED(result)) {
450450
commandList1->ResolveSubresourceRegion(dstTexture->d3dResource(), 0, dstX, dstY,
@@ -505,18 +505,18 @@ void GrD3DDirectCommandList::addSampledTextureRef(GrD3DTexture* texture) {
505505
////////////////////////////////////////////////////////////////////////////////////////////////////
506506

507507
std::unique_ptr<GrD3DCopyCommandList> GrD3DCopyCommandList::Make(ID3D12Device* device) {
508-
ComPtr<ID3D12CommandAllocator> allocator;
508+
gr_cp<ID3D12CommandAllocator> allocator;
509509
GR_D3D_CALL_ERRCHECK(device->CreateCommandAllocator(D3D12_COMMAND_LIST_TYPE_DIRECT,
510510
IID_PPV_ARGS(&allocator)));
511511

512-
ComPtr<ID3D12GraphicsCommandList> commandList;
513-
GR_D3D_CALL_ERRCHECK(device->CreateCommandList(0, D3D12_COMMAND_LIST_TYPE_COPY, allocator.Get(),
512+
gr_cp<ID3D12GraphicsCommandList> commandList;
513+
GR_D3D_CALL_ERRCHECK(device->CreateCommandList(0, D3D12_COMMAND_LIST_TYPE_COPY, allocator.get(),
514514
nullptr, IID_PPV_ARGS(&commandList)));
515515
auto grCL = new GrD3DCopyCommandList(std::move(allocator), std::move(commandList));
516516
return std::unique_ptr<GrD3DCopyCommandList>(grCL);
517517
}
518518

519-
GrD3DCopyCommandList::GrD3DCopyCommandList(ComPtr<ID3D12CommandAllocator> allocator,
520-
ComPtr<ID3D12GraphicsCommandList> commandList)
519+
GrD3DCopyCommandList::GrD3DCopyCommandList(gr_cp<ID3D12CommandAllocator> allocator,
520+
gr_cp<ID3D12GraphicsCommandList> commandList)
521521
: GrD3DCommandList(std::move(allocator), std::move(commandList)) {
522522
}

0 commit comments

Comments
 (0)