From 60dd62ba20fad508eb6786195b40ca5c576965a0 Mon Sep 17 00:00:00 2001 From: Robert Phillips Date: Tue, 12 Mar 2019 16:28:59 +0000 Subject: [PATCH] Revert "Always try to reduce opList splitting in DDL contexts/drawingManagers" This reverts commit 2cce805f1f8216ebdc56b247389d8aa05a51df11. Reason for revert: Chrome Original change's description: > Always try to reduce opList splitting in DDL contexts/drawingManagers > > This may get us in trouble w/ local DDL testing (since we run all our GMs through DDLs). For Chrome this shouldn't yet be a problem (since they are only using DDLs for compositing). > > This does mean we're on a tight timeline to land predictive intermediate flushes before Chrome starts using DDLs for rasterization. > > Change-Id: I0bb95c075cff3ee49498ff267d76c3a61d16373e > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/199722 > Reviewed-by: Brian Salomon > Commit-Queue: Robert Phillips TBR=bsalomon@google.com,robertphillips@google.com Change-Id: Idb2dbda1a41844b2541526d504b117fd4cd628cc No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://skia-review.googlesource.com/c/skia/+/200505 Reviewed-by: Robert Phillips Commit-Queue: Robert Phillips --- include/private/GrRecordingContext.h | 2 +- src/gpu/GrDDLContext.cpp | 7 +++---- src/gpu/GrDrawingManager.cpp | 14 +++++++++++--- src/gpu/GrDrawingManager.h | 2 +- src/gpu/GrLegacyDirectContext.cpp | 13 +------------ src/gpu/GrRecordingContext.cpp | 18 ++++++++++-------- 6 files changed, 27 insertions(+), 29 deletions(-) diff --git a/include/private/GrRecordingContext.h b/include/private/GrRecordingContext.h index 2ce46d30e0..48e673f213 100644 --- a/include/private/GrRecordingContext.h +++ b/include/private/GrRecordingContext.h @@ -32,7 +32,7 @@ protected: GrRecordingContext(GrBackendApi, const GrContextOptions&, uint32_t contextID); bool init(sk_sp, sk_sp) override; - void setupDrawingManager(bool explicitlyAllocate, bool sortOpLists, bool reduceOpListSplitting); + void setupDrawingManager(bool explicitlyAllocate, bool sortOpLists); void abandonContext() override; diff --git a/src/gpu/GrDDLContext.cpp b/src/gpu/GrDDLContext.cpp index c10659dcd5..6836e4634f 100644 --- a/src/gpu/GrDDLContext.cpp +++ b/src/gpu/GrDDLContext.cpp @@ -52,10 +52,9 @@ protected: return false; } - // DDL contexts/drawing managers always sort the oplists and reduce opList splitting. - // This, in turn, implies that explicit resource allocation is always on (regardless - // of how Skia is compiled). - this->setupDrawingManager(true, true, true); + // DDL contexts/drawing managers always sort the oplists. This, in turn, implies that + // explicit resource allocation is always on (regardless of how Skia is compiled). + this->setupDrawingManager(true, true); SkASSERT(this->caps()); diff --git a/src/gpu/GrDrawingManager.cpp b/src/gpu/GrDrawingManager.cpp index fc3866d8a9..b6d45ff0d4 100644 --- a/src/gpu/GrDrawingManager.cpp +++ b/src/gpu/GrDrawingManager.cpp @@ -145,7 +145,7 @@ GrDrawingManager::GrDrawingManager(GrRecordingContext* context, const GrTextContext::Options& optionsForTextContext, bool explicitlyAllocating, bool sortOpLists, - bool reduceOpListSplitting) + GrContextOptions::Enable reduceOpListSplitting) : fContext(context) , fOptionsForPathRendererChain(optionsForPathRendererChain) , fOptionsForTextContext(optionsForTextContext) @@ -153,8 +153,16 @@ GrDrawingManager::GrDrawingManager(GrRecordingContext* context, , fTextContext(nullptr) , fPathRendererChain(nullptr) , fSoftwarePathRenderer(nullptr) - , fFlushing(false) - , fReduceOpListSplitting(reduceOpListSplitting) { + , fFlushing(false) { + if (GrContextOptions::Enable::kNo == reduceOpListSplitting) { + fReduceOpListSplitting = false; + } else if (GrContextOptions::Enable::kYes == reduceOpListSplitting) { + fReduceOpListSplitting = true; + } else { + // For now, this is only turned on when explicitly enabled. Once mini-flushes are + // implemented it should be enabled whenever sorting is enabled. + fReduceOpListSplitting = false; // sortOpLists + } } void GrDrawingManager::cleanup() { diff --git a/src/gpu/GrDrawingManager.h b/src/gpu/GrDrawingManager.h index 0f5f0df129..3f2245e241 100644 --- a/src/gpu/GrDrawingManager.h +++ b/src/gpu/GrDrawingManager.h @@ -140,7 +140,7 @@ private: const GrTextContext::Options&, bool explicitlyAllocating, bool sortOpLists, - bool reduceOpListSplitting); + GrContextOptions::Enable reduceOpListSplitting); bool wasAbandoned() const; diff --git a/src/gpu/GrLegacyDirectContext.cpp b/src/gpu/GrLegacyDirectContext.cpp index 7a5fad0e59..d5bce1b693 100644 --- a/src/gpu/GrLegacyDirectContext.cpp +++ b/src/gpu/GrLegacyDirectContext.cpp @@ -86,18 +86,7 @@ protected: sortOpLists = true; } - // For now, this is only turned on for direct rendering when explicitly enabled. - // Once predictive intermediate flushes are implemented it should be enabled whenever - // sorting is enabled. - bool reduceOpListSplitting = false; // sortOpLists - if (GrContextOptions::Enable::kNo == this->options().fReduceOpListSplitting) { - reduceOpListSplitting = false; - } else if (GrContextOptions::Enable::kYes == this->options().fReduceOpListSplitting) { - reduceOpListSplitting = true; - } - - this->setupDrawingManager(this->explicitlyAllocateGPUResources(), - sortOpLists, reduceOpListSplitting); + this->setupDrawingManager(this->explicitlyAllocateGPUResources(), sortOpLists); SkASSERT(this->caps()); diff --git a/src/gpu/GrRecordingContext.cpp b/src/gpu/GrRecordingContext.cpp index 088aebeac0..5d907bfded 100644 --- a/src/gpu/GrRecordingContext.cpp +++ b/src/gpu/GrRecordingContext.cpp @@ -64,9 +64,7 @@ bool GrRecordingContext::init(sk_sp caps, sk_spoptions().fAllowPathMaskCaching; #if GR_TEST_UTILS @@ -96,12 +94,16 @@ void GrRecordingContext::setupDrawingManager(bool explicitlyAllocate, } #endif + // SHORT TERM TODO: until intermediate flushes at allocation time are added we need to obey the + // reduceOpListSplitting flag. Once that lands we should always reduce opList splitting in + // DDL contexts/drawing managers. We should still obey the options for non-DDL drawing managers + // until predictive intermediate flushes are added (i.e., we can't reorder forever). fDrawingManager.reset(new GrDrawingManager(this, - prcOptions, - textContextOptions, - explicitlyAllocate, - sortOpLists, - reduceOpListSplitting)); + prcOptions, + textContextOptions, + explicitlyAllocate, + sortOpLists, + this->options().fReduceOpListSplitting)); } void GrRecordingContext::abandonContext() {