put an arena on GrRenderTargetProxy
This is a reland of 5a2de5e72f24d1bfbdc532252be3dcdefa7b75a2
This is part one of two CLs. In this CL, I put a
sk_sp<GrArenas> on GrRenderTargetProxy where GrArenas wraps
a SkArenaAlloc to add ref counting. Creating
a GrOpsTask shares the GrArenas with the ops task. When an
GrOpsTask is destroyed, it nulls out the fArenas sk_sp on
the GrRenderTargetProxy to limit the life span of the arenas.
New plumbing was added to GR_DRAW_OP_TEST_DEFINE to allow a
proper GrSurfaceDrawContext to be passed to GrAtlasTextOp's
GR_DRAW_OP_TEST_DEFINE so the arena will have a proper lifetime.
The second CL will work on replacing GrOpsTask's fAllocators
system with the shared arena.
Change-Id: Ieb568e4533c17e31b3b015e7781365d7d898c483
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396818
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Herb Derby <herb@google.com>
diff --git a/src/gpu/GrDrawOpTest.h b/src/gpu/GrDrawOpTest.h
index 23f744e..a0d0b6c 100644
--- a/src/gpu/GrDrawOpTest.h
+++ b/src/gpu/GrDrawOpTest.h
@@ -26,11 +26,12 @@
/** GrDrawOp subclasses should define test factory functions using this macro. */
#define GR_DRAW_OP_TEST_DEFINE(Op) \
- GrOp::Owner Op##__Test(GrPaint&& paint, SkRandom* random, \
- GrRecordingContext* context, int numSamples)
+ GrOp::Owner Op##__Test(GrPaint&& paint, SkRandom* random, \
+ GrRecordingContext* context, \
+ GrSurfaceDrawContext* sdc, int numSamples)
#define GR_DRAW_OP_TEST_FRIEND(Op) \
- friend GrOp::OpOwner Op##__Test(GrPaint&& paint, SkRandom* random, \
- GrRecordingContext* context, int numSamples)
+ friend GrOp::OpOwner Op##__Test(GrPaint&&, SkRandom*, \
+ GrRecordingContext*, GrSurfaceDrawContext*, int)
/** Helper for op test factories to pick a random stencil state. */
const GrUserStencilSettings* GrGetRandomStencil(SkRandom* random, GrContext_Base*);
diff --git a/src/gpu/GrDrawingManager.cpp b/src/gpu/GrDrawingManager.cpp
index abf9c94..a85436a 100644
--- a/src/gpu/GrDrawingManager.cpp
+++ b/src/gpu/GrDrawingManager.cpp
@@ -667,14 +667,17 @@
}
sk_sp<GrOpsTask> GrDrawingManager::newOpsTask(GrSurfaceProxyView surfaceView,
+ sk_sp<GrArenas> arenas,
bool flushTimeOpsTask) {
SkDEBUGCODE(this->validate());
SkASSERT(fContext);
this->closeActiveOpsTask();
- sk_sp<GrOpsTask> opsTask(new GrOpsTask(this, std::move(surfaceView),
- fContext->priv().auditTrail()));
+ sk_sp<GrOpsTask> opsTask(new GrOpsTask(this,
+ std::move(surfaceView),
+ fContext->priv().auditTrail(),
+ std::move(arenas)));
SkASSERT(this->getLastRenderTask(opsTask->target(0)) == opsTask.get());
if (flushTimeOpsTask) {
diff --git a/src/gpu/GrDrawingManager.h b/src/gpu/GrDrawingManager.h
index a289422..ce7308c 100644
--- a/src/gpu/GrDrawingManager.h
+++ b/src/gpu/GrDrawingManager.h
@@ -23,6 +23,7 @@
// Enabling this will print out which path renderers are being chosen
#define GR_PATH_RENDERER_SPEW 0
+class GrArenas;
class GrCoverageCountingPathRenderer;
class GrGpuBuffer;
class GrOnFlushCallbackObject;
@@ -46,7 +47,9 @@
void freeGpuResources();
// OpsTasks created at flush time are stored and handled different from the others.
- sk_sp<GrOpsTask> newOpsTask(GrSurfaceProxyView, bool flushTimeOpsTask);
+ sk_sp<GrOpsTask> newOpsTask(GrSurfaceProxyView,
+ sk_sp<GrArenas> arenas,
+ bool flushTimeOpsTask);
// Create a render task that can resolve MSAA and/or regenerate mipmap levels on proxies. This
// method will only add the new render task to the list. It is up to the caller to call
diff --git a/src/gpu/GrOpsTask.cpp b/src/gpu/GrOpsTask.cpp
index f853c8e..7f0077d 100644
--- a/src/gpu/GrOpsTask.cpp
+++ b/src/gpu/GrOpsTask.cpp
@@ -355,12 +355,14 @@
GrOpsTask::GrOpsTask(GrDrawingManager* drawingMgr,
GrSurfaceProxyView view,
- GrAuditTrail* auditTrail)
+ GrAuditTrail* auditTrail,
+ sk_sp<GrArenas> arenas)
: GrRenderTask()
, fAuditTrail(auditTrail)
, fUsesMSAASurface(view.asRenderTargetProxy()->numSamples() > 1)
, fTargetSwizzle(view.swizzle())
, fTargetOrigin(view.origin())
+ , fArenas{std::move(arenas)}
SkDEBUGCODE(, fNumClips(0)) {
fAllocators.push_back(std::make_unique<SkArenaAlloc>(4096));
this->addTarget(drawingMgr, view.detachProxy());
@@ -375,6 +377,7 @@
GrOpsTask::~GrOpsTask() {
this->deleteOps();
+ this->target(0)->asRenderTargetProxy()->clearArenas();
}
void GrOpsTask::addOp(GrDrawingManager* drawingMgr, GrOp::Owner op,
diff --git a/src/gpu/GrOpsTask.h b/src/gpu/GrOpsTask.h
index aedcfc2..e90ff8b 100644
--- a/src/gpu/GrOpsTask.h
+++ b/src/gpu/GrOpsTask.h
@@ -37,9 +37,8 @@
using DstProxyView = GrXferProcessor::DstProxyView;
public:
- // The Arenas must outlive the GrOpsTask, either by preserving the context that owns
- // the pool, or by moving the pool to the DDL that takes over the GrOpsTask.
- GrOpsTask(GrDrawingManager*, GrSurfaceProxyView, GrAuditTrail*);
+ // Manage the arenas life time by maintaining are reference to it.
+ GrOpsTask(GrDrawingManager*, GrSurfaceProxyView, GrAuditTrail*, sk_sp<GrArenas>);
~GrOpsTask() override;
GrOpsTask* asOpsTask() override { return this; }
@@ -269,6 +268,7 @@
// MDB TODO: 4096 for the first allocation may be huge overkill. Gather statistics to determine
// the correct size.
SkSTArray<1, std::unique_ptr<SkArenaAlloc>> fAllocators;
+ sk_sp<GrArenas> fArenas;
SkDEBUGCODE(int fNumClips;)
// TODO: We could look into this being a set if we find we're adding a lot of duplicates that is
diff --git a/src/gpu/GrRenderTargetProxy.h b/src/gpu/GrRenderTargetProxy.h
index 98040fc..ca6ad08 100644
--- a/src/gpu/GrRenderTargetProxy.h
+++ b/src/gpu/GrRenderTargetProxy.h
@@ -9,6 +9,7 @@
#define GrRenderTargetProxy_DEFINED
#include "include/private/GrTypesPriv.h"
+#include "src/core/SkArenaAlloc.h"
#include "src/gpu/GrCaps.h"
#include "src/gpu/GrNativeRect.h"
#include "src/gpu/GrSurfaceProxy.h"
@@ -16,6 +17,14 @@
class GrResourceProvider;
+class GrArenas : public SkNVRefCnt<GrArenas> {
+public:
+ SkArenaAlloc* arenaAlloc() { return &fArenaAlloc; }
+
+private:
+ SkArenaAlloc fArenaAlloc{1024};
+};
+
// This class delays the acquisition of RenderTargets until they are actually
// required
// Beware: the uniqueID of the RenderTargetProxy will usually be different than
@@ -85,6 +94,17 @@
// TODO: move this to a priv class!
bool refsWrappedObjects() const;
+ sk_sp<GrArenas> arenas() {
+ if (fArenas == nullptr) {
+ fArenas = sk_make_sp<GrArenas>();
+ }
+ return fArenas;
+ }
+
+ void clearArenas() {
+ fArenas = nullptr;
+ }
+
protected:
friend class GrProxyProvider; // for ctors
friend class GrRenderTargetProxyPriv;
@@ -149,6 +169,8 @@
int8_t fNumStencilSamples = 0;
WrapsVkSecondaryCB fWrapsVkSecondaryCB;
SkIRect fMSAADirtyRect = SkIRect::MakeEmpty();
+ sk_sp<GrArenas> fArenas{nullptr};
+
// This is to fix issue in large comment above. Without the padding we can end up with the
// GrTextureProxy starting 8 byte aligned by not 16. This happens when the RT ends at bytes 1-8.
// Note: with the virtual inheritance an 8 byte pointer is at the start of GrRenderTargetProxy.
diff --git a/src/gpu/GrSurfaceFillContext.cpp b/src/gpu/GrSurfaceFillContext.cpp
index 49bf84d..bc34587 100644
--- a/src/gpu/GrSurfaceFillContext.cpp
+++ b/src/gpu/GrSurfaceFillContext.cpp
@@ -342,8 +342,8 @@
SkDEBUGCODE(this->validate();)
if (!fOpsTask || fOpsTask->isClosed()) {
- sk_sp<GrOpsTask> newOpsTask = this->drawingManager()->newOpsTask(this->writeSurfaceView(),
- fFlushTimeOpsTask);
+ sk_sp<GrOpsTask> newOpsTask = this->drawingManager()->newOpsTask(
+ this->writeSurfaceView(), this->arenas(), fFlushTimeOpsTask);
if (fOpsTask) {
this->willReplaceOpsTask(fOpsTask.get(), newOpsTask.get());
}
diff --git a/src/gpu/GrSurfaceFillContext.h b/src/gpu/GrSurfaceFillContext.h
index d7e4b5c..7e89f77 100644
--- a/src/gpu/GrSurfaceFillContext.h
+++ b/src/gpu/GrSurfaceFillContext.h
@@ -195,6 +195,8 @@
bool wrapsVkSecondaryCB() const { return this->asRenderTargetProxy()->wrapsVkSecondaryCB(); }
GrMipmapped mipmapped() const;
+ SkArenaAlloc* arenaAlloc() { return this->arenas()->arenaAlloc(); }
+
#if GR_TEST_UTILS
GrOpsTask* testingOnly_PeekLastOpsTask() { return fOpsTask.get(); }
#endif
@@ -210,6 +212,8 @@
void addOp(GrOp::Owner);
private:
+ sk_sp<GrArenas> arenas() { return fWriteView.proxy()->asRenderTargetProxy()->arenas(); }
+
template <SkAlphaType AlphaType>
static std::array<float, 4> ConvertColor(SkRGBA4f<AlphaType> color);
diff --git a/src/gpu/ops/GrAtlasTextOp.cpp b/src/gpu/ops/GrAtlasTextOp.cpp
index 2553c95..4efb410 100644
--- a/src/gpu/ops/GrAtlasTextOp.cpp
+++ b/src/gpu/ops/GrAtlasTextOp.cpp
@@ -112,14 +112,18 @@
SkPoint drawOrigin,
SkIRect clipRect,
sk_sp<GrTextBlob> blob,
- const SkPMColor4f& color) -> Geometry* {
- return new Geometry{subRun,
- drawMatrix,
- drawOrigin,
- clipRect,
- std::move(blob),
- nullptr,
- color};
+ const SkPMColor4f& color,
+ SkArenaAlloc* alloc) -> Geometry* {
+ // Bypass the automatic dtor behavior in SkArenaAlloc. I'm leaving this up to the Op to run
+ // all geometry dtors for now.
+ void* geo = alloc->makeBytesAlignedTo(sizeof(Geometry), alignof(Geometry));
+ return new(geo) Geometry{subRun,
+ drawMatrix,
+ drawOrigin,
+ clipRect,
+ std::move(blob),
+ nullptr,
+ color};
}
void GrAtlasTextOp::Geometry::fillVertexData(void *dst, int offset, int count) const {
@@ -522,10 +526,6 @@
}
GR_DRAW_OP_TEST_DEFINE(GrAtlasTextOp) {
- // Setup dummy SkPaint / GrPaint / GrSurfaceDrawContext
- auto rtc = GrSurfaceDrawContext::Make(
- context, GrColorType::kRGBA_8888, nullptr, SkBackingFit::kApprox, {1024, 1024});
-
SkSimpleMatrixProvider matrixProvider(GrTest::TestMatrixInvertible(random));
SkPaint skPaint;
@@ -548,8 +548,7 @@
int xInt = (random->nextU() % kMaxTrans) * xPos;
int yInt = (random->nextU() % kMaxTrans) * yPos;
- return GrAtlasTextOp::CreateOpTestingOnly(
- rtc.get(), skPaint, font, matrixProvider, text, xInt, yInt);
+ return GrAtlasTextOp::CreateOpTestingOnly(sdc, skPaint, font, matrixProvider, text, xInt, yInt);
}
#endif
diff --git a/src/gpu/ops/GrAtlasTextOp.h b/src/gpu/ops/GrAtlasTextOp.h
index 17e5a97..0a8710b 100644
--- a/src/gpu/ops/GrAtlasTextOp.h
+++ b/src/gpu/ops/GrAtlasTextOp.h
@@ -27,7 +27,7 @@
~GrAtlasTextOp() override {
for (const Geometry* g = fHead; g != nullptr;) {
const Geometry* next = g->fNext;
- delete g;
+ g->~Geometry();
g = next;
}
}
@@ -67,7 +67,8 @@
SkPoint drawOrigin,
SkIRect clipRect,
sk_sp<GrTextBlob> blob,
- const SkPMColor4f& color);
+ const SkPMColor4f& color,
+ SkArenaAlloc* alloc);
void fillVertexData(void* dst, int offset, int count) const;
diff --git a/src/gpu/text/GrTextBlob.cpp b/src/gpu/text/GrTextBlob.cpp
index a272251..92d5071 100644
--- a/src/gpu/text/GrTextBlob.cpp
+++ b/src/gpu/text/GrTextBlob.cpp
@@ -677,7 +677,8 @@
drawOrigin,
clipRect,
sk_ref_sp<GrTextBlob>(fBlob),
- drawingColor);
+ drawingColor,
+ sdc->arenaAlloc());
GrRecordingContext* const rContext = sdc->recordingContext();
GrOp::Owner op = GrOp::Make<GrAtlasTextOp>(rContext,
@@ -970,7 +971,8 @@
drawOrigin,
SkIRect::MakeEmpty(),
sk_ref_sp<GrTextBlob>(fBlob),
- drawingColor);
+ drawingColor,
+ sdc->arenaAlloc());
GrRecordingContext* const rContext = sdc->recordingContext();
GrOp::Owner op = GrOp::Make<GrAtlasTextOp>(
@@ -1252,7 +1254,8 @@
drawOrigin,
SkIRect::MakeEmpty(),
sk_ref_sp<GrTextBlob>(fBlob),
- drawingColor);
+ drawingColor,
+ sdc->arenaAlloc());
GrRecordingContext* const rContext = sdc->recordingContext();
GrOp::Owner op = GrOp::Make<GrAtlasTextOp>(