Revert "Hold sk_sp<GrAtlasTextBlob> refs in GrTextBlobCache instead of raw ptrs"
This reverts commit db3ceb86421fb9da86bb920e3a1f0957beec08d9.
Reason for revert: observing some strange budget behavior w/ instrumented Chromium builds; need to investigate.
Original change's description:
> Hold sk_sp<GrAtlasTextBlob> refs in GrTextBlobCache instead of raw ptrs
>
> Refactor to store sk_sps, and minimize explicit ref manipulation.
>
> Change-Id: Ie3d18e5fe1cefbbc5c2f3c4941287a24038522a6
> Reviewed-on: https://skia-review.googlesource.com/9490
> Commit-Queue: Florin Malita <fmalita@chromium.org>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
>
TBR=bsalomon@google.com,robertphillips@google.com,fmalita@chromium.org,reviews@skia.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Change-Id: I8ca9862ad1519a9ec69ad1ce8e4d129b0dae7b0a
Reviewed-on: https://skia-review.googlesource.com/9524
Reviewed-by: Florin Malita <fmalita@google.com>
Commit-Queue: Florin Malita <fmalita@google.com>
diff --git a/src/gpu/text/GrAtlasTextBlob.cpp b/src/gpu/text/GrAtlasTextBlob.cpp
index 5427855..accdf6b 100644
--- a/src/gpu/text/GrAtlasTextBlob.cpp
+++ b/src/gpu/text/GrAtlasTextBlob.cpp
@@ -17,7 +17,7 @@
#include "SkTextBlobRunIterator.h"
#include "ops/GrAtlasTextOp.h"
-sk_sp<GrAtlasTextBlob> GrAtlasTextBlob::Make(GrMemoryPool* pool, int glyphCount, int runCount) {
+GrAtlasTextBlob* GrAtlasTextBlob::Create(GrMemoryPool* pool, int glyphCount, int runCount) {
// We allocate size for the GrAtlasTextBlob itself, plus size for the vertices array,
// and size for the glyphIds array.
size_t verticesCount = glyphCount * kVerticesPerGlyph * kMaxVASize;
@@ -31,12 +31,11 @@
sk_bzero(allocation, size);
}
- sk_sp<GrAtlasTextBlob> cacheBlob(new (allocation) GrAtlasTextBlob);
+ GrAtlasTextBlob* cacheBlob = new (allocation) GrAtlasTextBlob;
cacheBlob->fSize = size;
// setup offsets for vertices / glyphs
- cacheBlob->fVertices = sizeof(GrAtlasTextBlob) +
- reinterpret_cast<unsigned char*>(cacheBlob.get());
+ cacheBlob->fVertices = sizeof(GrAtlasTextBlob) + reinterpret_cast<unsigned char*>(cacheBlob);
cacheBlob->fGlyphs = reinterpret_cast<GrGlyph**>(cacheBlob->fVertices + verticesCount);
cacheBlob->fRuns = reinterpret_cast<GrAtlasTextBlob::Run*>(cacheBlob->fGlyphs + glyphCount);
diff --git a/src/gpu/text/GrAtlasTextBlob.h b/src/gpu/text/GrAtlasTextBlob.h
index 47dd1db..7d686a9 100644
--- a/src/gpu/text/GrAtlasTextBlob.h
+++ b/src/gpu/text/GrAtlasTextBlob.h
@@ -51,7 +51,7 @@
public:
SK_DECLARE_INTERNAL_LLIST_INTERFACE(GrAtlasTextBlob);
- static sk_sp<GrAtlasTextBlob> Make(GrMemoryPool* pool, int glyphCount, int runCount);
+ static GrAtlasTextBlob* Create(GrMemoryPool* pool, int glyphCount, int runCount);
struct Key {
Key() {
diff --git a/src/gpu/text/GrAtlasTextContext.cpp b/src/gpu/text/GrAtlasTextContext.cpp
index 4e377c9..e15281e 100644
--- a/src/gpu/text/GrAtlasTextContext.cpp
+++ b/src/gpu/text/GrAtlasTextContext.cpp
@@ -115,7 +115,7 @@
key.fHasBlur = SkToBool(mf);
key.fCanonicalColor = canonicalColor;
key.fScalerContextFlags = scalerContextFlags;
- cacheBlob = cache->find(key);
+ cacheBlob.reset(SkSafeRef(cache->find(key)));
}
GrTextUtils::Paint paint(&skPaint);
@@ -125,7 +125,7 @@
// TODO we could probably get away reuse most of the time if the pointer is unique,
// but we'd have to clear the subrun information
cache->remove(cacheBlob.get());
- cacheBlob = cache->makeCachedBlob(blob, key, blurRec, skPaint);
+ cacheBlob.reset(SkRef(cache->createCachedBlob(blob, key, blurRec, skPaint)));
RegenerateTextBlob(cacheBlob.get(), context->getAtlasGlyphCache(),
*context->caps()->shaderCaps(), paint, scalerContextFlags,
viewMatrix, props, blob, x, y, drawFilter);
@@ -136,7 +136,7 @@
int glyphCount = 0;
int runCount = 0;
GrTextBlobCache::BlobGlyphCount(&glyphCount, &runCount, blob);
- sk_sp<GrAtlasTextBlob> sanityBlob(cache->makeBlob(glyphCount, runCount));
+ sk_sp<GrAtlasTextBlob> sanityBlob(cache->createBlob(glyphCount, runCount));
sanityBlob->setupKey(key, blurRec, skPaint);
RegenerateTextBlob(sanityBlob.get(), context->getAtlasGlyphCache(),
*context->caps()->shaderCaps(), paint, scalerContextFlags,
@@ -146,9 +146,9 @@
}
} else {
if (canCache) {
- cacheBlob = cache->makeCachedBlob(blob, key, blurRec, skPaint);
+ cacheBlob.reset(SkRef(cache->createCachedBlob(blob, key, blurRec, skPaint)));
} else {
- cacheBlob = cache->makeBlob(blob);
+ cacheBlob.reset(cache->createBlob(blob));
}
RegenerateTextBlob(cacheBlob.get(), context->getAtlasGlyphCache(),
*context->caps()->shaderCaps(), paint, scalerContextFlags, viewMatrix,
@@ -230,52 +230,52 @@
}
}
-inline sk_sp<GrAtlasTextBlob>
-GrAtlasTextContext::MakeDrawTextBlob(GrTextBlobCache* blobCache,
- GrAtlasGlyphCache* fontCache,
- const GrShaderCaps& shaderCaps,
- const GrTextUtils::Paint& paint,
- uint32_t scalerContextFlags,
- const SkMatrix& viewMatrix,
- const SkSurfaceProps& props,
- const char text[], size_t byteLength,
- SkScalar x, SkScalar y) {
+inline GrAtlasTextBlob*
+GrAtlasTextContext::CreateDrawTextBlob(GrTextBlobCache* blobCache,
+ GrAtlasGlyphCache* fontCache,
+ const GrShaderCaps& shaderCaps,
+ const GrTextUtils::Paint& paint,
+ uint32_t scalerContextFlags,
+ const SkMatrix& viewMatrix,
+ const SkSurfaceProps& props,
+ const char text[], size_t byteLength,
+ SkScalar x, SkScalar y) {
int glyphCount = paint.skPaint().countText(text, byteLength);
- sk_sp<GrAtlasTextBlob> blob = blobCache->makeBlob(glyphCount, 1);
+ GrAtlasTextBlob* blob = blobCache->createBlob(glyphCount, 1);
blob->initThrowawayBlob(viewMatrix, x, y);
if (GrTextUtils::CanDrawAsDistanceFields(paint, viewMatrix, props, shaderCaps)) {
- GrTextUtils::DrawDFText(blob.get(), 0, fontCache, props, paint, scalerContextFlags,
- viewMatrix, text, byteLength, x, y);
+ GrTextUtils::DrawDFText(blob, 0, fontCache, props, paint, scalerContextFlags, viewMatrix,
+ text, byteLength, x, y);
} else {
- GrTextUtils::DrawBmpText(blob.get(), 0, fontCache, props, paint, scalerContextFlags,
- viewMatrix, text, byteLength, x, y);
+ GrTextUtils::DrawBmpText(blob, 0, fontCache, props, paint, scalerContextFlags, viewMatrix,
+ text, byteLength, x, y);
}
return blob;
}
-inline sk_sp<GrAtlasTextBlob>
-GrAtlasTextContext::MakeDrawPosTextBlob(GrTextBlobCache* blobCache,
- GrAtlasGlyphCache* fontCache,
- const GrShaderCaps& shaderCaps,
- const GrTextUtils::Paint& paint,
- uint32_t scalerContextFlags,
- const SkMatrix& viewMatrix,
- const SkSurfaceProps& props,
- const char text[], size_t byteLength,
- const SkScalar pos[], int scalarsPerPosition, const
- SkPoint& offset) {
+inline GrAtlasTextBlob*
+GrAtlasTextContext::CreateDrawPosTextBlob(GrTextBlobCache* blobCache,
+ GrAtlasGlyphCache* fontCache,
+ const GrShaderCaps& shaderCaps,
+ const GrTextUtils::Paint& paint,
+ uint32_t scalerContextFlags,
+ const SkMatrix& viewMatrix,
+ const SkSurfaceProps& props,
+ const char text[], size_t byteLength,
+ const SkScalar pos[], int scalarsPerPosition, const
+ SkPoint& offset) {
int glyphCount = paint.skPaint().countText(text, byteLength);
- sk_sp<GrAtlasTextBlob> blob = blobCache->makeBlob(glyphCount, 1);
+ GrAtlasTextBlob* blob = blobCache->createBlob(glyphCount, 1);
blob->initThrowawayBlob(viewMatrix, offset.x(), offset.y());
if (GrTextUtils::CanDrawAsDistanceFields(paint, viewMatrix, props, shaderCaps)) {
- GrTextUtils::DrawDFPosText(blob.get(), 0, fontCache, props, paint, scalerContextFlags,
- viewMatrix, text, byteLength, pos, scalarsPerPosition, offset);
+ GrTextUtils::DrawDFPosText(blob, 0, fontCache, props, paint, scalerContextFlags, viewMatrix,
+ text, byteLength, pos, scalarsPerPosition, offset);
} else {
- GrTextUtils::DrawBmpPosText(blob.get(), 0, fontCache, props, paint, scalerContextFlags,
+ GrTextUtils::DrawBmpPosText(blob, 0, fontCache, props, paint, scalerContextFlags,
viewMatrix, text, byteLength, pos, scalarsPerPosition, offset);
}
return blob;
@@ -292,11 +292,11 @@
GrTextUtils::Paint paint(&skPaint);
if (this->canDraw(skPaint, viewMatrix, props, *context->caps()->shaderCaps())) {
sk_sp<GrAtlasTextBlob> blob(
- MakeDrawTextBlob(context->getTextBlobCache(), context->getAtlasGlyphCache(),
- *context->caps()->shaderCaps(),
- paint, ComputeScalerContextFlags(rtc),
- viewMatrix, props,
- text, byteLength, x, y));
+ CreateDrawTextBlob(context->getTextBlobCache(), context->getAtlasGlyphCache(),
+ *context->caps()->shaderCaps(),
+ paint, ComputeScalerContextFlags(rtc),
+ viewMatrix, props,
+ text, byteLength, x, y));
blob->flushThrowaway(context, rtc, props, fDistanceAdjustTable.get(), paint, clip,
viewMatrix, regionClipBounds, x, y);
return;
@@ -318,13 +318,13 @@
return;
} else if (this->canDraw(skPaint, viewMatrix, props, *context->caps()->shaderCaps())) {
sk_sp<GrAtlasTextBlob> blob(
- MakeDrawPosTextBlob(context->getTextBlobCache(), context->getAtlasGlyphCache(),
- *context->caps()->shaderCaps(),
- paint, ComputeScalerContextFlags(rtc),
- viewMatrix, props,
- text, byteLength,
- pos, scalarsPerPosition,
- offset));
+ CreateDrawPosTextBlob(context->getTextBlobCache(), context->getAtlasGlyphCache(),
+ *context->caps()->shaderCaps(),
+ paint, ComputeScalerContextFlags(rtc),
+ viewMatrix, props,
+ text, byteLength,
+ pos, scalarsPerPosition,
+ offset));
blob->flushThrowaway(context, rtc, props, fDistanceAdjustTable.get(), paint, clip,
viewMatrix, regionClipBounds, offset.fX, offset.fY);
return;
@@ -377,7 +377,7 @@
GrTextUtils::Paint paint(&skPaint);
// right now we don't handle textblobs, nor do we handle drawPosText. Since we only intend to
// test the text op with this unit test, that is okay.
- sk_sp<GrAtlasTextBlob> blob(GrAtlasTextContext::MakeDrawTextBlob(
+ sk_sp<GrAtlasTextBlob> blob(GrAtlasTextContext::CreateDrawTextBlob(
context->getTextBlobCache(), context->getAtlasGlyphCache(),
*context->caps()->shaderCaps(), paint,
GrAtlasTextContext::kTextBlobOpScalerContextFlags, viewMatrix, gSurfaceProps, text,
diff --git a/src/gpu/text/GrAtlasTextContext.h b/src/gpu/text/GrAtlasTextContext.h
index 7438b64..20a7368 100644
--- a/src/gpu/text/GrAtlasTextContext.h
+++ b/src/gpu/text/GrAtlasTextContext.h
@@ -64,24 +64,24 @@
SkDrawFilter* drawFilter);
inline static bool HasLCD(const SkTextBlob*);
- static inline sk_sp<GrAtlasTextBlob> MakeDrawTextBlob(GrTextBlobCache*, GrAtlasGlyphCache*,
- const GrShaderCaps&,
- const GrTextUtils::Paint&,
- uint32_t scalerContextFlags,
- const SkMatrix& viewMatrix,
- const SkSurfaceProps&,
- const char text[], size_t byteLength,
- SkScalar x, SkScalar y);
- static inline sk_sp<GrAtlasTextBlob> MakeDrawPosTextBlob(GrTextBlobCache*, GrAtlasGlyphCache*,
- const GrShaderCaps&,
- const GrTextUtils::Paint&,
- uint32_t scalerContextFlags,
- const SkMatrix& viewMatrix,
- const SkSurfaceProps&,
- const char text[], size_t byteLength,
- const SkScalar pos[],
- int scalarsPerPosition,
- const SkPoint& offset);
+ static inline GrAtlasTextBlob* CreateDrawTextBlob(GrTextBlobCache*, GrAtlasGlyphCache*,
+ const GrShaderCaps&,
+ const GrTextUtils::Paint&,
+ uint32_t scalerContextFlags,
+ const SkMatrix& viewMatrix,
+ const SkSurfaceProps&,
+ const char text[], size_t byteLength,
+ SkScalar x, SkScalar y);
+ static inline GrAtlasTextBlob* CreateDrawPosTextBlob(GrTextBlobCache*, GrAtlasGlyphCache*,
+ const GrShaderCaps&,
+ const GrTextUtils::Paint&,
+ uint32_t scalerContextFlags,
+ const SkMatrix& viewMatrix,
+ const SkSurfaceProps&,
+ const char text[], size_t byteLength,
+ const SkScalar pos[],
+ int scalarsPerPosition,
+ const SkPoint& offset);
const GrDistanceFieldAdjustTable* dfAdjustTable() const { return fDistanceAdjustTable.get(); }
sk_sp<const GrDistanceFieldAdjustTable> fDistanceAdjustTable;
diff --git a/src/gpu/text/GrTextBlobCache.cpp b/src/gpu/text/GrTextBlobCache.cpp
index c53fcb0..f6dac69 100644
--- a/src/gpu/text/GrTextBlobCache.cpp
+++ b/src/gpu/text/GrTextBlobCache.cpp
@@ -8,13 +8,14 @@
#include "GrTextBlobCache.h"
GrTextBlobCache::~GrTextBlobCache() {
- SkDEBUGCODE(this->freeAll();)
+ this->freeAll();
}
void GrTextBlobCache::freeAll() {
fBlobIDCache.foreach([this](uint32_t, BlobIDCacheEntry* entry) {
- for (const auto& blob : entry->fBlobs) {
- fBlobList.remove(blob.get());
+ for (auto* blob : entry->fBlobs) {
+ fBlobList.remove(blob);
+ blob->unref();
}
});
diff --git a/src/gpu/text/GrTextBlobCache.h b/src/gpu/text/GrTextBlobCache.h
index af6c792..886a091 100644
--- a/src/gpu/text/GrTextBlobCache.h
+++ b/src/gpu/text/GrTextBlobCache.h
@@ -9,7 +9,6 @@
#define GrTextBlobCache_DEFINED
#include "GrAtlasTextContext.h"
-#include "SkRefCnt.h"
#include "SkTArray.h"
#include "SkTextBlobRunIterator.h"
#include "SkTHash.h"
@@ -32,28 +31,31 @@
~GrTextBlobCache();
// creates an uncached blob
- sk_sp<GrAtlasTextBlob> makeBlob(int glyphCount, int runCount) {
- return GrAtlasTextBlob::Make(&fPool, glyphCount, runCount);
+ GrAtlasTextBlob* createBlob(int glyphCount, int runCount) {
+ return GrAtlasTextBlob::Create(&fPool, glyphCount, runCount);
}
-
- sk_sp<GrAtlasTextBlob> makeBlob(const SkTextBlob* blob) {
+ GrAtlasTextBlob* createBlob(const SkTextBlob* blob) {
int glyphCount = 0;
int runCount = 0;
BlobGlyphCount(&glyphCount, &runCount, blob);
- return GrAtlasTextBlob::Make(&fPool, glyphCount, runCount);
+ GrAtlasTextBlob* cacheBlob = GrAtlasTextBlob::Create(&fPool, glyphCount, runCount);
+ return cacheBlob;
}
- sk_sp<GrAtlasTextBlob> makeCachedBlob(const SkTextBlob* blob,
- const GrAtlasTextBlob::Key& key,
- const SkMaskFilter::BlurRec& blurRec,
- const SkPaint& paint) {
- sk_sp<GrAtlasTextBlob> cacheBlob(this->makeBlob(blob));
+ GrAtlasTextBlob* createCachedBlob(const SkTextBlob* blob,
+ const GrAtlasTextBlob::Key& key,
+ const SkMaskFilter::BlurRec& blurRec,
+ const SkPaint& paint) {
+ int glyphCount = 0;
+ int runCount = 0;
+ BlobGlyphCount(&glyphCount, &runCount, blob);
+ GrAtlasTextBlob* cacheBlob = GrAtlasTextBlob::Create(&fPool, glyphCount, runCount);
cacheBlob->setupKey(key, blurRec, paint);
this->add(cacheBlob);
return cacheBlob;
}
- sk_sp<GrAtlasTextBlob> find(const GrAtlasTextBlob::Key& key) const {
+ GrAtlasTextBlob* find(const GrAtlasTextBlob::Key& key) const {
const auto* idEntry = fBlobIDCache.find(key.fUniqueID);
return idEntry ? idEntry->find(key) : nullptr;
}
@@ -63,11 +65,26 @@
auto* idEntry = fBlobIDCache.find(id);
SkASSERT(idEntry);
- fBlobList.remove(blob);
idEntry->removeBlob(blob);
if (idEntry->fBlobs.empty()) {
fBlobIDCache.remove(id);
}
+
+ fBlobList.remove(blob);
+ blob->unref();
+ }
+
+ void add(GrAtlasTextBlob* blob) {
+ auto id = GrAtlasTextBlob::GetKey(*blob).fUniqueID;
+ auto* idEntry = fBlobIDCache.find(id);
+ if (!idEntry) {
+ idEntry = fBlobIDCache.set(id, BlobIDCacheEntry(id));
+ }
+
+ idEntry->addBlob(blob);
+ fBlobList.addToHead(blob);
+
+ this->checkPurge(blob);
}
void makeMRU(GrAtlasTextBlob* blob) {
@@ -105,12 +122,12 @@
return entry.fID;
}
- void addBlob(sk_sp<GrAtlasTextBlob> blob) {
+ void addBlob(GrAtlasTextBlob* blob) {
SkASSERT(blob);
SkASSERT(GrAtlasTextBlob::GetKey(*blob).fUniqueID == fID);
SkASSERT(!this->find(GrAtlasTextBlob::GetKey(*blob)));
- fBlobs.emplace_back(std::move(blob));
+ fBlobs.push_back(blob);
}
void removeBlob(GrAtlasTextBlob* blob) {
@@ -123,7 +140,7 @@
fBlobs.removeShuffle(index);
}
- sk_sp<GrAtlasTextBlob> find(const GrAtlasTextBlob::Key& key) const {
+ GrAtlasTextBlob* find(const GrAtlasTextBlob::Key& key) const {
auto index = this->findBlobIndex(key);
return index < 0 ? nullptr : fBlobs[index];
}
@@ -140,24 +157,9 @@
uint32_t fID;
// Current clients don't generate multiple GrAtlasTextBlobs per SkTextBlob, so an array w/
// linear search is acceptable. If usage changes, we should re-evaluate this structure.
- SkSTArray<1, sk_sp<GrAtlasTextBlob>, true> fBlobs;
+ SkSTArray<1, GrAtlasTextBlob*, true> fBlobs;
};
- void add(sk_sp<GrAtlasTextBlob> blob) {
- auto id = GrAtlasTextBlob::GetKey(*blob).fUniqueID;
- auto* idEntry = fBlobIDCache.find(id);
- if (!idEntry) {
- idEntry = fBlobIDCache.set(id, BlobIDCacheEntry(id));
- }
-
- // Safe to retain a raw ptr temporarily here, because the cache will hold a ref.
- GrAtlasTextBlob* rawBlobPtr = blob.get();
- fBlobList.addToHead(rawBlobPtr);
- idEntry->addBlob(std::move(blob));
-
- this->checkPurge(rawBlobPtr);
- }
-
void checkPurge(GrAtlasTextBlob* blob = nullptr) {
// If we are overbudget, then unref until we are below budget again
if (fPool.size() > fBudget) {
@@ -191,9 +193,9 @@
static const int kPreAllocSize = 1 << 17;
static const int kMinGrowthSize = 1 << 17;
static const int kDefaultBudget = 1 << 22;
- GrMemoryPool fPool;
BitmapBlobList fBlobList;
SkTHashMap<uint32_t, BlobIDCacheEntry> fBlobIDCache;
+ GrMemoryPool fPool;
PFOverBudgetCB fCallback;
void* fData;
size_t fBudget;