From 5eb41fdf94187d6cc22702444622ed7897c8039a Mon Sep 17 00:00:00 2001 From: bsalomon Date: Tue, 6 Sep 2016 13:49:32 -0700 Subject: [PATCH] Revert of Restructure flushing relationship between GrContext, GrDrawingManager, and GrResourceCache. (patchset #4 id:60001 of https://codereview.chromium.org/2307053002/ ) Reason for revert: Causing assertions on bots Original issue's description: > Restructure flushing relationship between GrContext, GrDrawingManager, and GrResourceCache. > > Consolidates all flush actions into GrDrawingManager and makes GrContext::flush a passthrough. > > Removes the unused and untested discard flush variation. > > Replaces the indirect overbudget callback mechanism of GrResourceCache with a flag set by resource cache when it wants to flush that is checked after each draw by GrDrawContext. > > Modifies GrResourceCache::notifyFlushOccurred() to take a param indicating whether it triggered the > flush that just occurred. > > GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2307053002 > > Committed: https://skia.googlesource.com/skia/+/1dbb207babecdae8f1f74ed9d9900c73064df744 TBR=robertphillips@google.com # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Review-Url: https://codereview.chromium.org/2312123003 --- include/gpu/GrContext.h | 31 +++++++++++++++++++++++++++++- src/gpu/GrContext.cpp | 35 +++++++++++++++++++++++++++------- src/gpu/GrDrawContext.cpp | 2 +- src/gpu/GrDrawingManager.cpp | 13 ++++--------- src/gpu/GrDrawingManager.h | 17 +++-------------- src/gpu/GrResourceCache.cpp | 37 +++++++++++++++--------------------- src/gpu/GrResourceCache.h | 29 +++++++++++++++++----------- src/gpu/SkGpuDevice.cpp | 1 + tests/ResourceCacheTest.cpp | 10 +++++----- 9 files changed, 105 insertions(+), 70 deletions(-) diff --git a/include/gpu/GrContext.h b/include/gpu/GrContext.h index 996b77f2db..f14c36e35c 100644 --- a/include/gpu/GrContext.h +++ b/include/gpu/GrContext.h @@ -213,11 +213,32 @@ public: /////////////////////////////////////////////////////////////////////////// // Misc. + /** + * Flags that affect flush() behavior. + */ + enum FlushBits { + /** + * A client may reach a point where it has partially rendered a frame + * through a GrContext that it knows the user will never see. This flag + * causes the flush to skip submission of deferred content to the 3D API + * during the flush. + */ + kDiscard_FlushBit = 0x2, + }; + /** * Call to ensure all drawing to the context has been issued to the * underlying 3D API. + * @param flagsBitfield flags that control the flushing behavior. See + * FlushBits. */ - void flush(); + void flush(int flagsBitfield = 0); + + void flushIfNecessary() { + if (fFlushToReduceCacheSize || this->caps()->immediateFlush()) { + this->flush(); + } + } /** * These flags can be used with the read/write pixels functions below. @@ -388,6 +409,8 @@ private: GrBatchFontCache* fBatchFontCache; SkAutoTDelete fTextBlobCache; + // Set by OverbudgetCB() to request that GrContext flush before exiting a draw. + bool fFlushToReduceCacheSize; bool fDidTestPMConversions; int fPMToUPMConversion; int fUPMToPMConversion; @@ -448,6 +471,12 @@ private: will fail. In such cases fall back to SW conversion. */ bool didFailPMUPMConversionTest() const; + /** + * This callback allows the resource cache to callback into the GrContext + * when the cache is still over budget after a purge. + */ + static void OverBudgetCB(void* data); + /** * A callback similar to the above for use by the TextBlobCache * TODO move textblob draw calls below context so we can use the call above. diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp index 357f58e6e3..3f4d2fa959 100644 --- a/src/gpu/GrContext.cpp +++ b/src/gpu/GrContext.cpp @@ -66,6 +66,7 @@ GrContext::GrContext() : fUniqueID(next_id()) { fResourceCache = nullptr; fResourceProvider = nullptr; fBatchFontCache = nullptr; + fFlushToReduceCacheSize = false; } bool GrContext::init(GrBackend backend, GrBackendContext backendContext, @@ -86,6 +87,7 @@ void GrContext::initCommon(const GrContextOptions& options) { fCaps = SkRef(fGpu->caps()); fResourceCache = new GrResourceCache(fCaps); + fResourceCache->setOverBudgetCallback(OverBudgetCB, this); fResourceProvider = new GrResourceProvider(fGpu, fResourceCache, &fSingleOwner); fDidTestPMConversions = false; @@ -95,8 +97,7 @@ void GrContext::initCommon(const GrContextOptions& options) { dtOptions.fDrawBatchBounds = options.fDrawBatchBounds; dtOptions.fMaxBatchLookback = options.fMaxBatchLookback; dtOptions.fMaxBatchLookahead = options.fMaxBatchLookahead; - fDrawingManager.reset(new GrDrawingManager(this, dtOptions, options.fImmediateMode, - &fSingleOwner)); + fDrawingManager.reset(new GrDrawingManager(this, dtOptions, &fSingleOwner)); // GrBatchFontCache will eventually replace GrFontCache fBatchFontCache = new GrBatchFontCache(this); @@ -202,21 +203,41 @@ void GrContext::getResourceCacheUsage(int* resourceCount, size_t* resourceBytes) //////////////////////////////////////////////////////////////////////////////// +void GrContext::OverBudgetCB(void* data) { + SkASSERT(data); + + GrContext* context = reinterpret_cast(data); + + // Flush the GrBufferedDrawTarget to possibly free up some textures + context->fFlushToReduceCacheSize = true; +} + void GrContext::TextBlobCacheOverBudgetCB(void* data) { SkASSERT(data); - // TextBlobs are drawn at the SkGpuDevice level, therefore they cannot rely on GrDrawContext - // to perform a necessary flush. The solution is to move drawText calls to below the GrContext - // level, but this is not trivial because they call drawPath on SkGpuDevice. + + // Unlike the GrResourceCache, TextBlobs are drawn at the SkGpuDevice level, therefore they + // cannot use fFlushTorReduceCacheSize because it uses AutoCheckFlush. The solution is to move + // drawText calls to below the GrContext level, but this is not trivial because they call + // drawPath on SkGpuDevice GrContext* context = reinterpret_cast(data); context->flush(); } //////////////////////////////////////////////////////////////////////////////// -void GrContext::flush() { +void GrContext::flush(int flagsBitfield) { ASSERT_SINGLE_OWNER RETURN_IF_ABANDONED - fDrawingManager->flush(); + bool flushed = false; + if (kDiscard_FlushBit & flagsBitfield) { + fDrawingManager->reset(); + } else { + flushed = fDrawingManager->flush(); + } + if (flushed) { + fResourceCache->notifyFlushOccurred(); + } + fFlushToReduceCacheSize = false; } bool sw_convert_to_premul(GrPixelConfig srcConfig, int width, int height, size_t inRowBytes, diff --git a/src/gpu/GrDrawContext.cpp b/src/gpu/GrDrawContext.cpp index ed02602499..87252f3979 100644 --- a/src/gpu/GrDrawContext.cpp +++ b/src/gpu/GrDrawContext.cpp @@ -59,7 +59,7 @@ public: AutoCheckFlush(GrDrawingManager* drawingManager) : fDrawingManager(drawingManager) { SkASSERT(fDrawingManager); } - ~AutoCheckFlush() { fDrawingManager->flushIfNecessary(); } + ~AutoCheckFlush() { fDrawingManager->getContext()->flushIfNecessary(); } private: GrDrawingManager* fDrawingManager; diff --git a/src/gpu/GrDrawingManager.cpp b/src/gpu/GrDrawingManager.cpp index 4642e1a3fe..6c75c0d147 100644 --- a/src/gpu/GrDrawingManager.cpp +++ b/src/gpu/GrDrawingManager.cpp @@ -5,10 +5,8 @@ * found in the LICENSE file. */ -#include "GrDrawingManager.h" - -#include "GrContext.h" #include "GrDrawContext.h" +#include "GrDrawingManager.h" #include "GrDrawTarget.h" #include "GrPathRenderingDrawContext.h" #include "GrResourceProvider.h" @@ -76,9 +74,9 @@ void GrDrawingManager::reset() { fFlushState.reset(); } -void GrDrawingManager::internalFlush(GrResourceCache::FlushType type) { +bool GrDrawingManager::flush() { if (fFlushing || this->wasAbandoned()) { - return; + return false; } fFlushing = true; bool flushed = false; @@ -128,11 +126,8 @@ void GrDrawingManager::internalFlush(GrResourceCache::FlushType type) { #endif fFlushState.reset(); - // We always have to notify the cache when it requested a flush so it can reset its state. - if (flushed || type == GrResourceCache::FlushType::kCacheRequested) { - fContext->getResourceCache()->notifyFlushOccurred(type); - } fFlushing = false; + return flushed; } GrDrawTarget* GrDrawingManager::newDrawTarget(GrRenderTarget* rt) { diff --git a/src/gpu/GrDrawingManager.h b/src/gpu/GrDrawingManager.h index bc6f7a361a..745820f439 100644 --- a/src/gpu/GrDrawingManager.h +++ b/src/gpu/GrDrawingManager.h @@ -13,7 +13,6 @@ #include "GrBatchFlushState.h" #include "GrPathRendererChain.h" #include "GrPathRenderer.h" -#include "GrResourceCache.h" #include "SkTDArray.h" class GrContext; @@ -50,19 +49,11 @@ public: GrPathRendererChain::DrawType drawType, GrPathRenderer::StencilSupport* stencilSupport = NULL); - void flushIfNecessary() { - if (fContext->getResourceCache()->requestsFlush()) { - this->internalFlush(GrResourceCache::kCacheRequested); - } else if (fIsImmediateMode) { - this->internalFlush(GrResourceCache::kImmediateMode); - } - } - static bool ProgramUnitTest(GrContext* context, int maxStages); private: GrDrawingManager(GrContext* context, const GrDrawTarget::Options& optionsForDrawTargets, - bool isImmediateMode, GrSingleOwner* singleOwner) + GrSingleOwner* singleOwner) : fContext(context) , fOptionsForDrawTargets(optionsForDrawTargets) , fSingleOwner(singleOwner) @@ -77,8 +68,8 @@ private: void abandon(); void cleanup(); void reset(); - void flush() { this->internalFlush(GrResourceCache::FlushType::kExternal); } - void internalFlush(GrResourceCache::FlushType); + /** Returns true if there was anything to flush and false otherwise */ + bool flush(); friend class GrContext; // for access to: ctor, abandon, reset & flush @@ -101,8 +92,6 @@ private: GrBatchFlushState fFlushState; bool fFlushing; - - bool fIsImmediateMode; }; #endif diff --git a/src/gpu/GrResourceCache.cpp b/src/gpu/GrResourceCache.cpp index e3f4f9ff50..62360ed535 100644 --- a/src/gpu/GrResourceCache.cpp +++ b/src/gpu/GrResourceCache.cpp @@ -73,6 +73,8 @@ GrResourceCache::GrResourceCache(const GrCaps* caps) , fBytes(0) , fBudgetedCount(0) , fBudgetedBytes(0) + , fOverBudgetCB(nullptr) + , fOverBudgetData(nullptr) , fFlushTimestamps(nullptr) , fLastFlushTimestampIndex(0) , fPreferVRAMUseOverFlushes(caps->preferVRAMUseOverFlushes()) { @@ -501,9 +503,10 @@ void GrResourceCache::purgeAsNeeded() { this->validate(); if (stillOverbudget) { - // Set this so that GrDrawingManager will issue a flush to free up resources with pending - // IO that we were unable to purge in this pass. - fRequestFlush = true; + // Despite the purge we're still over budget. Call our over budget callback. If this frees + // any resources then we'll get notified and take appropriate action. + (*fOverBudgetCB)(fOverBudgetData); + this->validate(); } } @@ -618,26 +621,16 @@ uint32_t GrResourceCache::getNextTimestamp() { return fTimestamp++; } -void GrResourceCache::notifyFlushOccurred(FlushType type) { - switch (type) { - case FlushType::kImmediateMode: - break; - case FlushType::kCacheRequested: - SkASSERT(fRequestFlush); - fRequestFlush = false; - break; - case FlushType::kExternal: - if (fFlushTimestamps) { - SkASSERT(SkIsPow2(fMaxUnusedFlushes)); - fLastFlushTimestampIndex = (fLastFlushTimestampIndex + 1) & (fMaxUnusedFlushes - 1); - // get the timestamp before accessing fFlushTimestamps because getNextTimestamp will - // reallocate fFlushTimestamps on timestamp overflow. - uint32_t timestamp = this->getNextTimestamp(); - fFlushTimestamps[fLastFlushTimestampIndex] = timestamp; - } - break; +void GrResourceCache::notifyFlushOccurred() { + if (fFlushTimestamps) { + SkASSERT(SkIsPow2(fMaxUnusedFlushes)); + fLastFlushTimestampIndex = (fLastFlushTimestampIndex + 1) & (fMaxUnusedFlushes - 1); + // get the timestamp before accessing fFlushTimestamps because getNextTimestamp will + // reallocate fFlushTimestamps on timestamp overflow. + uint32_t timestamp = this->getNextTimestamp(); + fFlushTimestamps[fLastFlushTimestampIndex] = timestamp; + this->purgeAsNeeded(); } - this->purgeAsNeeded(); } void GrResourceCache::dumpMemoryStatistics(SkTraceMemoryDump* traceMemoryDump) const { diff --git a/src/gpu/GrResourceCache.h b/src/gpu/GrResourceCache.h index bf7b237006..6c64ddc933 100644 --- a/src/gpu/GrResourceCache.h +++ b/src/gpu/GrResourceCache.h @@ -11,7 +11,6 @@ #include "GrGpuResource.h" #include "GrGpuResourceCacheAccess.h" #include "GrGpuResourcePriv.h" -#include "GrResourceCache.h" #include "GrResourceKey.h" #include "SkMessageBus.h" #include "SkRefCnt.h" @@ -164,16 +163,23 @@ public: /** Purges all resources that don't have external owners. */ void purgeAllUnlocked(); - /** Returns true if the cache would like a flush to occur in order to make more resources - purgeable. */ - bool requestsFlush() const { return fRequestFlush; } + /** + * The callback function used by the cache when it is still over budget after a purge. The + * passed in 'data' is the same 'data' handed to setOverbudgetCallback. + */ + typedef void (*PFOverBudgetCB)(void* data); - enum FlushType { - kExternal, - kImmediateMode, - kCacheRequested, - }; - void notifyFlushOccurred(FlushType); + /** + * Set the callback the cache should use when it is still over budget after a purge. The 'data' + * provided here will be passed back to the callback. Note that the cache will attempt to purge + * any resources newly freed by the callback. + */ + void setOverBudgetCallback(PFOverBudgetCB overBudgetCB, void* data) { + fOverBudgetCB = overBudgetCB; + fOverBudgetData = data; + } + + void notifyFlushOccurred(); #if GR_CACHE_STATS struct Stats { @@ -320,7 +326,8 @@ private: int fBudgetedCount; size_t fBudgetedBytes; - bool fRequestFlush; + PFOverBudgetCB fOverBudgetCB; + void* fOverBudgetData; // We keep track of the "timestamps" of the last n flushes. If a resource hasn't been used in // that time then we well preemptively purge it to reduce memory usage. diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp index 790a19a506..64df998e2f 100644 --- a/src/gpu/SkGpuDevice.cpp +++ b/src/gpu/SkGpuDevice.cpp @@ -1008,6 +1008,7 @@ void SkGpuDevice::drawBitmapTile(const SkBitmap& bitmap, if (nullptr == texture) { return; } + sk_sp colorSpaceXform = GrColorSpaceXform::Make(bitmap.colorSpace(), fDrawContext->getColorSpace()); diff --git a/tests/ResourceCacheTest.cpp b/tests/ResourceCacheTest.cpp index b568485e4b..2cbf81c231 100644 --- a/tests/ResourceCacheTest.cpp +++ b/tests/ResourceCacheTest.cpp @@ -1133,7 +1133,7 @@ static void test_flush(skiatest::Reporter* reporter) { make_unique_key<1>(&k, i); r->resourcePriv().setUniqueKey(k); r->unref(); - cache->notifyFlushOccurred(GrResourceCache::kExternal); + cache->notifyFlushOccurred(); } // Send flush notifications to the cache. Each flush should purge the oldest resource. @@ -1147,7 +1147,7 @@ static void test_flush(skiatest::Reporter* reporter) { REPORTER_ASSERT(reporter, !SkToBool(r)); SkSafeUnref(r); } - cache->notifyFlushOccurred(GrResourceCache::kExternal); + cache->notifyFlushOccurred(); } REPORTER_ASSERT(reporter, 0 == cache->getResourceCount()); @@ -1169,13 +1169,13 @@ static void test_flush(skiatest::Reporter* reporter) { } else { r->unref(); } - cache->notifyFlushOccurred(GrResourceCache::kExternal); + cache->notifyFlushOccurred(); } for (int i = 0; i < kFlushCount; ++i) { // Should get a resource purged every other flush. REPORTER_ASSERT(reporter, kFlushCount - i/2 - 1 == cache->getResourceCount()); - cache->notifyFlushOccurred(GrResourceCache::kExternal); + cache->notifyFlushOccurred(); } // Unref all the resources that we kept refs on in the first loop. @@ -1187,7 +1187,7 @@ static void test_flush(skiatest::Reporter* reporter) { // get kFlushCount additional flushes. Then everything should be purged. for (int i = 0; i < kFlushCount; ++i) { REPORTER_ASSERT(reporter, kFlushCount >> 1 == cache->getResourceCount()); - cache->notifyFlushOccurred(GrResourceCache::kExternal); + cache->notifyFlushOccurred(); } REPORTER_ASSERT(reporter, 0 == cache->getResourceCount());