Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit ed58654

Browse files
drWulfSkia Commit-Bot
drWulf
authored and
Skia Commit-Bot
committed
Revert "Detach op memory pool from recording context"
This reverts commit 6b95516. Reason for revert: breaking some Win10 bots Original change's description: > Detach op memory pool from recording context > > This changes GrOpMemoryPool to no longer extend SkRefCnt, and all usages > either are std::unique_ptr for owners, or GrOpMemoryPool* when ownership > is held somewhere else. The culmination of this is that DDLs explicitly > detach the memory pool from the recording context instead of the GrOpsTask > maintaining a strong ref that preserved the memory somewhat sneakily. > > Change-Id: I33e2caebea70cebe8fd7681207c631feeaf2c703 > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/259424 > Commit-Queue: Michael Ludwig <michaelludwig@google.com> > Reviewed-by: Robert Phillips <robertphillips@google.com> > Reviewed-by: Brian Salomon <bsalomon@google.com> TBR=bsalomon@google.com,robertphillips@google.com,michaelludwig@google.com Change-Id: I942ae1e07fdc63d9311f6ee482bd71beca090502 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://skia-review.googlesource.com/c/skia/+/259696 Reviewed-by: Derek Sollenberger <djsollen@google.com> Commit-Queue: Derek Sollenberger <djsollen@google.com>
1 parent 77d3694 commit ed58654

13 files changed

+45
-44
lines changed

‎include/private/GrRecordingContext.h‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,8 @@ class GrRecordingContext : public GrImageContext {
5454

5555
GrDrawingManager* drawingManager();
5656

57+
sk_sp<GrOpMemoryPool> refOpMemoryPool();
5758
GrOpMemoryPool* opMemoryPool();
58-
// This entry point should only be used for DDL creation where we want the ops' lifetime to
59-
// match that of the DDL.
60-
std::unique_ptr<GrOpMemoryPool> detachOpMemoryPool();
6159

6260
SkArenaAlloc* recordTimeAllocator();
6361
// This entry point should only be used for DDL creation where we want the ops' data's lifetime
@@ -150,7 +148,7 @@ class GrRecordingContext : public GrImageContext {
150148
private:
151149
std::unique_ptr<GrDrawingManager> fDrawingManager;
152150
// All the GrOp-derived classes use this pool.
153-
std::unique_ptr<GrOpMemoryPool> fOpMemoryPool;
151+
sk_sp<GrOpMemoryPool> fOpMemoryPool;
154152
std::unique_ptr<SkArenaAlloc> fRecordTimeAllocator;
155153

156154
std::unique_ptr<GrStrikeCache> fStrikeCache;

‎include/private/SkDeferredDisplayList.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ class SkDeferredDisplayListPriv;
1717
#if SK_SUPPORT_GPU
1818
#include "include/private/SkTArray.h"
1919
#include <map>
20-
class GrOpMemoryPool;
2120
class GrRenderTask;
2221
class GrRenderTargetProxy;
2322
struct GrCCPerOpsTaskPaths;
@@ -80,7 +79,6 @@ class SkDeferredDisplayList {
8079

8180
SkTArray<sk_sp<GrRenderTask>> fRenderTasks;
8281
PendingPathsMap fPendingPaths; // This is the path data from CCPR.
83-
std::unique_ptr<GrOpMemoryPool> fOpMemoryPool;
8482
std::unique_ptr<SkArenaAlloc> fRecordTimeData;
8583

8684
// The program infos should be stored in 'fRecordTimeData' so do not need to be ref counted

‎src/atlastext/SkAtlasTextTarget.cpp‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,9 @@ class SkInternalAtlasTextTarget : public GrTextTarget, public SkAtlasTextTarget
8585
void* handle)
8686
: GrTextTarget(width, height, kColorInfo)
8787
, SkAtlasTextTarget(std::move(context), width, height, handle)
88-
, fGlyphPainter(kProps, kColorInfo) {}
88+
, fGlyphPainter(kProps, kColorInfo) {
89+
fOpMemoryPool = fContext->internal().grContext()->priv().refOpMemoryPool();
90+
}
8991

9092
~SkInternalAtlasTextTarget() override {
9193
this->deleteOps();
@@ -122,14 +124,11 @@ class SkInternalAtlasTextTarget : public GrTextTarget, public SkAtlasTextTarget
122124
private:
123125
void deleteOps();
124126

125-
GrOpMemoryPool* opMemoryPool() {
126-
return fContext->internal().grContext()->priv().opMemoryPool();
127-
}
128-
129127
uint32_t fColor;
130128
using SkAtlasTextTarget::fWidth;
131129
using SkAtlasTextTarget::fHeight;
132130
SkTArray<std::unique_ptr<GrAtlasTextOp>, true> fOps;
131+
sk_sp<GrOpMemoryPool> fOpMemoryPool;
133132
SkGlyphRunListPainter fGlyphPainter;
134133
};
135134

@@ -176,12 +175,10 @@ void SkInternalAtlasTextTarget::addDrawOp(const GrClip& clip, std::unique_ptr<Gr
176175
const GrCaps& caps = *this->context()->internal().grContext()->priv().caps();
177176
op->finalizeForTextTarget(fColor, caps);
178177
int n = SkTMin(kMaxBatchLookBack, fOps.count());
179-
180-
GrOpMemoryPool* pool = this->opMemoryPool();
181178
for (int i = 0; i < n; ++i) {
182179
GrAtlasTextOp* other = fOps.fromBack(i).get();
183180
if (other->combineIfPossible(op.get(), caps) == GrOp::CombineResult::kMerged) {
184-
pool->release(std::move(op));
181+
fOpMemoryPool->release(std::move(op));
185182
return;
186183
}
187184
if (GrRectsOverlap(op->bounds(), other->bounds())) {
@@ -192,10 +189,9 @@ void SkInternalAtlasTextTarget::addDrawOp(const GrClip& clip, std::unique_ptr<Gr
192189
}
193190

194191
void SkInternalAtlasTextTarget::deleteOps() {
195-
GrOpMemoryPool* pool = this->opMemoryPool();
196192
for (int i = 0; i < fOps.count(); ++i) {
197193
if (fOps[i]) {
198-
pool->release(std::move(fOps[i]));
194+
fOpMemoryPool->release(std::move(fOps[i]));
199195
}
200196
}
201197
fOps.reset();

‎src/core/SkDeferredDisplayList.cpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
class SkSurfaceCharacterization;
1414

1515
#if SK_SUPPORT_GPU
16-
#include "src/gpu/GrMemoryPool.h"
1716
#include "src/gpu/GrRenderTask.h"
1817
#include "src/gpu/ccpr/GrCCPerOpsTaskPaths.h"
1918
#endif

‎src/gpu/GrContextPriv.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,10 @@ sk_sp<GrSkSLFPFactoryCache> GrContextPriv::fpFactoryCache() {
4141
return fContext->fpFactoryCache();
4242
}
4343

44+
sk_sp<GrOpMemoryPool> GrContextPriv::refOpMemoryPool() {
45+
return fContext->refOpMemoryPool();
46+
}
47+
4448
void GrContextPriv::addOnFlushCallbackObject(GrOnFlushCallbackObject* onFlushCBObject) {
4549
fContext->addOnFlushCallbackObject(onFlushCBObject);
4650
}

‎src/gpu/GrContextPriv.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ class GrContextPriv {
5959
// from GrRecordingContext
6060
GrDrawingManager* drawingManager() { return fContext->drawingManager(); }
6161

62+
sk_sp<GrOpMemoryPool> refOpMemoryPool();
6263
GrOpMemoryPool* opMemoryPool() { return fContext->opMemoryPool(); }
6364

6465
GrStrikeCache* getGrStrikeCache() { return fContext->getGrStrikeCache(); }

‎src/gpu/GrDrawingManager.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -581,7 +581,6 @@ void GrDrawingManager::moveRenderTasksToDDL(SkDeferredDisplayList* ddl) {
581581
renderTask->prePrepare(fContext);
582582
}
583583

584-
ddl->fOpMemoryPool = fContext->priv().detachOpMemoryPool();
585584
ddl->fRecordTimeData = fContext->priv().detachRecordTimeAllocator();
586585

587586
fContext->priv().detachProgramInfos(&ddl->fProgramInfos);
@@ -681,7 +680,7 @@ sk_sp<GrOpsTask> GrDrawingManager::newOpsTask(GrSurfaceProxyView surfaceView,
681680
GrSurfaceProxy* proxy = surfaceView.proxy();
682681
this->closeRenderTasksForNewRenderTask(proxy);
683682

684-
sk_sp<GrOpsTask> opsTask(new GrOpsTask(fContext->priv().opMemoryPool(),
683+
sk_sp<GrOpsTask> opsTask(new GrOpsTask(fContext->priv().refOpMemoryPool(),
685684
std::move(surfaceView), fContext->priv().auditTrail()));
686685
SkASSERT(proxy->getLastRenderTask() == opsTask.get());
687686

‎src/gpu/GrMemoryPool.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,9 @@ class GrMemoryPool {
126126

127127
class GrOp;
128128

129-
class GrOpMemoryPool {
129+
// DDL TODO: for the DLL use case this could probably be the non-intrinsic-based style of
130+
// ref counting
131+
class GrOpMemoryPool : public SkRefCnt {
130132
public:
131133
GrOpMemoryPool(size_t preallocSize, size_t minAllocSize)
132134
: fMemoryPool(preallocSize, minAllocSize) {

‎src/gpu/GrOpsTask.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -352,11 +352,11 @@ inline void GrOpsTask::OpChain::validate() const {
352352

353353
////////////////////////////////////////////////////////////////////////////////
354354

355-
GrOpsTask::GrOpsTask(GrOpMemoryPool* opMemoryPool,
355+
GrOpsTask::GrOpsTask(sk_sp<GrOpMemoryPool> opMemoryPool,
356356
GrSurfaceProxyView view,
357357
GrAuditTrail* auditTrail)
358358
: GrRenderTask(std::move(view))
359-
, fOpMemoryPool(opMemoryPool)
359+
, fOpMemoryPool(std::move(opMemoryPool))
360360
, fAuditTrail(auditTrail)
361361
, fLastClipStackGenID(SK_InvalidUniqueID)
362362
SkDEBUGCODE(, fNumClips(0)) {
@@ -366,7 +366,7 @@ GrOpsTask::GrOpsTask(GrOpMemoryPool* opMemoryPool,
366366

367367
void GrOpsTask::deleteOps() {
368368
for (auto& chain : fOpChains) {
369-
chain.deleteOps(fOpMemoryPool);
369+
chain.deleteOps(fOpMemoryPool.get());
370370
}
371371
fOpChains.reset();
372372
}
@@ -815,7 +815,7 @@ void GrOpsTask::recordOp(
815815
while (true) {
816816
OpChain& candidate = fOpChains.fromBack(i);
817817
op = candidate.appendOp(std::move(op), processorAnalysis, dstProxyView, clip, caps,
818-
fOpMemoryPool, fAuditTrail);
818+
fOpMemoryPool.get(), fAuditTrail);
819819
if (!op) {
820820
return;
821821
}
@@ -850,7 +850,7 @@ void GrOpsTask::forwardCombine(const GrCaps& caps) {
850850
int j = i + 1;
851851
while (true) {
852852
OpChain& candidate = fOpChains[j];
853-
if (candidate.prependChain(&chain, caps, fOpMemoryPool, fAuditTrail)) {
853+
if (candidate.prependChain(&chain, caps, fOpMemoryPool.get(), fAuditTrail)) {
854854
break;
855855
}
856856
// Stop traversing if we would cause a painter's order violation.

‎src/gpu/GrOpsTask.h‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,7 @@ class GrOpsTask : public GrRenderTask {
3838
using DstProxyView = GrXferProcessor::DstProxyView;
3939

4040
public:
41-
// The GrOpMemoryPool must outlive the GrOpsTask, either by preserving the context that owns
42-
// the pool, or by moving the pool to the DDL that takes over the GrOpsTask.
43-
GrOpsTask(GrOpMemoryPool*, GrSurfaceProxyView, GrAuditTrail*);
41+
GrOpsTask(sk_sp<GrOpMemoryPool>, GrSurfaceProxyView, GrAuditTrail*);
4442
~GrOpsTask() override;
4543

4644
GrOpsTask* asOpsTask() override { return this; }
@@ -277,10 +275,10 @@ class GrOpsTask : public GrRenderTask {
277275
friend class GrRenderTargetContext;
278276

279277
// This is a backpointer to the GrOpMemoryPool that holds the memory for this GrOpsTask's ops.
280-
// In the DDL case, the GrOpMemoryPool must have been detached from the original recording
281-
// context and moved into the owning DDL.
282-
GrOpMemoryPool* fOpMemoryPool;
283-
GrAuditTrail* fAuditTrail;
278+
// In the DDL case, these back pointers keep the DDL's GrOpMemoryPool alive as long as its
279+
// constituent GrOpsTask survives.
280+
sk_sp<GrOpMemoryPool> fOpMemoryPool;
281+
GrAuditTrail* fAuditTrail;
284282

285283
GrLoadOp fColorLoadOp = GrLoadOp::kLoad;
286284
SkPMColor4f fLoadClearColor = SK_PMColor4fTRANSPARENT;

0 commit comments

Comments
 (0)