Resolve issue with flush-time opsTask management
Despite what the bug says, I believe the only issue here was that
all the flush-time opsTasks weren't guaranteed to be in
'fOnFlushRenderTasks'. Since the users of flush-time opsTasks
use GrRenderTargetContext::addDrawOp, if the opsTask were to be
split, new ops would be added to the correct/new opsTask.
Bug: skia:9357
Change-Id: I90577bcc852419a9e0c31d858f71cda9f6f6b6a3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336435
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
diff --git a/src/gpu/GrDrawingManager.cpp b/src/gpu/GrDrawingManager.cpp
index ff3885e..def3dbd 100644
--- a/src/gpu/GrDrawingManager.cpp
+++ b/src/gpu/GrDrawingManager.cpp
@@ -676,7 +676,7 @@
}
sk_sp<GrOpsTask> GrDrawingManager::newOpsTask(GrSurfaceProxyView surfaceView,
- bool managedOpsTask) {
+ bool flushTimeOpsTask) {
SkDEBUGCODE(this->validate());
SkASSERT(fContext);
@@ -688,7 +688,9 @@
fContext->priv().auditTrail()));
SkASSERT(this->getLastRenderTask(proxy) == opsTask.get());
- if (managedOpsTask) {
+ if (flushTimeOpsTask) {
+ fOnFlushRenderTasks.push_back(opsTask);
+ } else {
this->appendTask(opsTask);
if (!fReduceOpsTaskSplitting) {
diff --git a/src/gpu/GrDrawingManager.h b/src/gpu/GrDrawingManager.h
index 7006be0..da5040b 100644
--- a/src/gpu/GrDrawingManager.h
+++ b/src/gpu/GrDrawingManager.h
@@ -45,9 +45,8 @@
void freeGpuResources();
- // A managed opsTask is controlled by the drawing manager (i.e., sorted & flushed with the
- // others). An unmanaged one is created and used by the onFlushCallback.
- sk_sp<GrOpsTask> newOpsTask(GrSurfaceProxyView, bool managedOpsTask);
+ // OpsTasks created at flush time are stored and handled different from the others.
+ sk_sp<GrOpsTask> newOpsTask(GrSurfaceProxyView, 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/GrOnFlushResourceProvider.cpp b/src/gpu/GrOnFlushResourceProvider.cpp
index 4ac8538..dff56a3 100644
--- a/src/gpu/GrOnFlushResourceProvider.cpp
+++ b/src/gpu/GrOnFlushResourceProvider.cpp
@@ -34,7 +34,7 @@
auto renderTargetContext = GrRenderTargetContext::Make(
context, colorType, std::move(colorSpace), std::move(proxy),
- origin, props, false);
+ origin, props, true);
if (!renderTargetContext) {
return nullptr;
@@ -42,9 +42,6 @@
renderTargetContext->discard();
- // FIXME: http://skbug.com/9357: This breaks if the renderTargetContext splits its opsTask.
- fDrawingMgr->fOnFlushRenderTasks.push_back(sk_ref_sp(renderTargetContext->getOpsTask()));
-
return renderTargetContext;
}
diff --git a/src/gpu/GrRenderTargetContext.cpp b/src/gpu/GrRenderTargetContext.cpp
index 3a54015..57e6c29 100644
--- a/src/gpu/GrRenderTargetContext.cpp
+++ b/src/gpu/GrRenderTargetContext.cpp
@@ -99,7 +99,7 @@
sk_sp<GrSurfaceProxy> proxy,
GrSurfaceOrigin origin,
const SkSurfaceProps* surfaceProps,
- bool managedOps) {
+ bool flushTimeOpsTask) {
if (!proxy) {
return nullptr;
}
@@ -116,7 +116,8 @@
return std::make_unique<GrRenderTargetContext>(context, std::move(readView),
std::move(writeView), colorType,
- std::move(colorSpace), surfaceProps, managedOps);
+ std::move(colorSpace), surfaceProps,
+ flushTimeOpsTask);
}
std::unique_ptr<GrRenderTargetContext> GrRenderTargetContext::Make(
@@ -148,7 +149,7 @@
}
auto rtc = GrRenderTargetContext::Make(context, colorType, std::move(colorSpace),
- std::move(proxy), origin, surfaceProps, true);
+ std::move(proxy), origin, surfaceProps);
if (!rtc) {
return nullptr;
}
@@ -301,12 +302,12 @@
GrColorType colorType,
sk_sp<SkColorSpace> colorSpace,
const SkSurfaceProps* surfaceProps,
- bool managedOpsTask)
+ bool flushTimeOpsTask)
: GrSurfaceContext(context, std::move(readView), colorType, kPremul_SkAlphaType,
std::move(colorSpace))
, fWriteView(std::move(writeView))
, fSurfaceProps(SkSurfacePropsCopyOrDefault(surfaceProps))
- , fManagedOpsTask(managedOpsTask)
+ , fFlushTimeOpsTask(flushTimeOpsTask)
, fGlyphPainter(*this) {
fOpsTask = sk_ref_sp(context->priv().drawingManager()->getLastOpsTask(this->asSurfaceProxy()));
SkASSERT(this->asSurfaceProxy() == fWriteView.proxy());
@@ -351,8 +352,8 @@
SkDEBUGCODE(this->validate();)
if (!fOpsTask || fOpsTask->isClosed()) {
- sk_sp<GrOpsTask> newOpsTask =
- this->drawingManager()->newOpsTask(this->writeSurfaceView(), fManagedOpsTask);
+ sk_sp<GrOpsTask> newOpsTask = this->drawingManager()->newOpsTask(this->writeSurfaceView(),
+ fFlushTimeOpsTask);
if (fOpsTask && fNumStencilSamples > 0) {
// Store the stencil values in memory upon completion of fOpsTask.
fOpsTask->setMustPreserveStencil();
diff --git a/src/gpu/GrRenderTargetContext.h b/src/gpu/GrRenderTargetContext.h
index 2a1f2ef..0b48f19 100644
--- a/src/gpu/GrRenderTargetContext.h
+++ b/src/gpu/GrRenderTargetContext.h
@@ -63,7 +63,7 @@
public:
static std::unique_ptr<GrRenderTargetContext> Make(
GrRecordingContext*, GrColorType, sk_sp<SkColorSpace>, sk_sp<GrSurfaceProxy>,
- GrSurfaceOrigin, const SkSurfaceProps*, bool managedOps = true);
+ GrSurfaceOrigin, const SkSurfaceProps*, bool flushTimeOpsTask = false);
static std::unique_ptr<GrRenderTargetContext> Make(GrRecordingContext*,
GrColorType,
@@ -138,7 +138,7 @@
GrRenderTargetContext(GrRecordingContext*, GrSurfaceProxyView readView,
GrSurfaceProxyView writeView, GrColorType, sk_sp<SkColorSpace>,
- const SkSurfaceProps*, bool managedOpsTask = true);
+ const SkSurfaceProps*, bool flushTimeOpsTask = false);
~GrRenderTargetContext() override;
@@ -599,7 +599,6 @@
friend class GrClipStackClip; // for access to getOpsTask
friend class GrClipStack; // ""
- friend class GrOnFlushResourceProvider; // for access to getOpsTask (http://skbug.com/9357)
friend class GrRenderTargetContextPriv;
@@ -713,7 +712,7 @@
sk_sp<GrOpsTask> fOpsTask;
SkSurfaceProps fSurfaceProps;
- bool fManagedOpsTask;
+ bool fFlushTimeOpsTask;
int fNumStencilSamples = 0;