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
diff --git a/include/gpu/GrContext.h b/include/gpu/GrContext.h
index 996b77f..f14c36e 100644
--- a/include/gpu/GrContext.h
+++ b/include/gpu/GrContext.h
@@ -214,10 +214,31 @@
// 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 @@
GrBatchFontCache* fBatchFontCache;
SkAutoTDelete<GrTextBlobCache> fTextBlobCache;
+ // Set by OverbudgetCB() to request that GrContext flush before exiting a draw.
+ bool fFlushToReduceCacheSize;
bool fDidTestPMConversions;
int fPMToUPMConversion;
int fUPMToPMConversion;
@@ -449,6 +472,12 @@
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 357f58e..3f4d2fa 100644
--- a/src/gpu/GrContext.cpp
+++ b/src/gpu/GrContext.cpp
@@ -66,6 +66,7 @@
fResourceCache = nullptr;
fResourceProvider = nullptr;
fBatchFontCache = nullptr;
+ fFlushToReduceCacheSize = false;
}
bool GrContext::init(GrBackend backend, GrBackendContext backendContext,
@@ -86,6 +87,7 @@
fCaps = SkRef(fGpu->caps());
fResourceCache = new GrResourceCache(fCaps);
+ fResourceCache->setOverBudgetCallback(OverBudgetCB, this);
fResourceProvider = new GrResourceProvider(fGpu, fResourceCache, &fSingleOwner);
fDidTestPMConversions = false;
@@ -95,8 +97,7 @@
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::OverBudgetCB(void* data) {
+ SkASSERT(data);
+
+ GrContext* context = reinterpret_cast<GrContext*>(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<GrContext*>(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 ed02602..87252f3 100644
--- a/src/gpu/GrDrawContext.cpp
+++ b/src/gpu/GrDrawContext.cpp
@@ -59,7 +59,7 @@
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 4642e1a..6c75c0d 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 @@
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 @@
#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 bc6f7a3..745820f 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 @@
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 @@
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 @@
GrBatchFlushState fFlushState;
bool fFlushing;
-
- bool fIsImmediateMode;
};
#endif
diff --git a/src/gpu/GrResourceCache.cpp b/src/gpu/GrResourceCache.cpp
index e3f4f9f..62360ed 100644
--- a/src/gpu/GrResourceCache.cpp
+++ b/src/gpu/GrResourceCache.cpp
@@ -73,6 +73,8 @@
, fBytes(0)
, fBudgetedCount(0)
, fBudgetedBytes(0)
+ , fOverBudgetCB(nullptr)
+ , fOverBudgetData(nullptr)
, fFlushTimestamps(nullptr)
, fLastFlushTimestampIndex(0)
, fPreferVRAMUseOverFlushes(caps->preferVRAMUseOverFlushes()) {
@@ -501,9 +503,10 @@
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 @@
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 bf7b237..6c64ddc 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 @@
/** 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 @@
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 790a19a..64df998 100644
--- a/src/gpu/SkGpuDevice.cpp
+++ b/src/gpu/SkGpuDevice.cpp
@@ -1008,6 +1008,7 @@
if (nullptr == texture) {
return;
}
+
sk_sp<GrColorSpaceXform> colorSpaceXform =
GrColorSpaceXform::Make(bitmap.colorSpace(), fDrawContext->getColorSpace());
diff --git a/tests/ResourceCacheTest.cpp b/tests/ResourceCacheTest.cpp
index b568485..2cbf81c 100644
--- a/tests/ResourceCacheTest.cpp
+++ b/tests/ResourceCacheTest.cpp
@@ -1133,7 +1133,7 @@
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 @@
REPORTER_ASSERT(reporter, !SkToBool(r));
SkSafeUnref(r);
}
- cache->notifyFlushOccurred(GrResourceCache::kExternal);
+ cache->notifyFlushOccurred();
}
REPORTER_ASSERT(reporter, 0 == cache->getResourceCount());
@@ -1169,13 +1169,13 @@
} 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 @@
// 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());