Revert of Stop using SkScalerContext::getAdvance() in SkGlyphCache. (patchset #2 id:20001 of https://codereview.chromium.org/1321243004/ )

Reason for revert:
Suspect this is the cause of regressions in crbug.com/527445.  It's triggering on Windows and Linux, where getting advances does less than getting full metrics.  It's not triggering on Mac, where this CL was a no-op.

Original issue's description:
> Stop using SkScalerContext::getAdvance() in SkGlyphCache.
>
> We think it'll simplify things to just always get the full metrics.
> On most platforms, it's no different, and we think the platforms that
> do differ (FreeType) will be nearly just as cheap.
>
> Removing this distinction helps us make SkGlyphCaches concurrent by
> removing a state (we-have-only-advances) from its logical state machine.
>
> We see no significant changes running SKPs before and after this CL.
> That makes sense, of course, because the SKPs bake some of this into drawPosText.
>
> BUG=skia:
>
> Committed: https://skia.googlesource.com/skia/+/518a2923f11b819fa44ed5cff54155326959540f

TBR=reed@google.com,bungeman@google.com,herb@google.com,mtklein@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:

Review URL: https://codereview.chromium.org/1383403003
This commit is contained in:
mtklein 2015-10-06 07:00:45 -07:00 committed by Commit bot
parent d1201058bf
commit 8d57faf318
2 changed files with 35 additions and 17 deletions

View File

@ -118,40 +118,40 @@ int SkGlyphCache::countCachedGlyphs() const {
const SkGlyph& SkGlyphCache::getUnicharAdvance(SkUnichar charCode) {
VALIDATE();
return *this->lookupByChar(charCode);
return *this->lookupByChar(charCode, kJustAdvance_MetricsType);
}
const SkGlyph& SkGlyphCache::getGlyphIDAdvance(uint16_t glyphID) {
VALIDATE();
PackedGlyphID packedGlyphID = SkGlyph::MakeID(glyphID);
return *this->lookupByPackedGlyphID(packedGlyphID);
return *this->lookupByPackedGlyphID(packedGlyphID, kJustAdvance_MetricsType);
}
///////////////////////////////////////////////////////////////////////////////
const SkGlyph& SkGlyphCache::getUnicharMetrics(SkUnichar charCode) {
VALIDATE();
return *this->lookupByChar(charCode);
return *this->lookupByChar(charCode, kFull_MetricsType);
}
const SkGlyph& SkGlyphCache::getUnicharMetrics(SkUnichar charCode, SkFixed x, SkFixed y) {
VALIDATE();
return *this->lookupByChar(charCode, x, y);
return *this->lookupByChar(charCode, kFull_MetricsType, x, y);
}
const SkGlyph& SkGlyphCache::getGlyphIDMetrics(uint16_t glyphID) {
VALIDATE();
PackedGlyphID packedGlyphID = SkGlyph::MakeID(glyphID);
return *this->lookupByPackedGlyphID(packedGlyphID);
return *this->lookupByPackedGlyphID(packedGlyphID, kFull_MetricsType);
}
const SkGlyph& SkGlyphCache::getGlyphIDMetrics(uint16_t glyphID, SkFixed x, SkFixed y) {
VALIDATE();
PackedGlyphID packedGlyphID = SkGlyph::MakeID(glyphID, x, y);
return *this->lookupByPackedGlyphID(packedGlyphID);
return *this->lookupByPackedGlyphID(packedGlyphID, kFull_MetricsType);
}
SkGlyph* SkGlyphCache::lookupByChar(SkUnichar charCode, SkFixed x, SkFixed y) {
SkGlyph* SkGlyphCache::lookupByChar(SkUnichar charCode, MetricsType type, SkFixed x, SkFixed y) {
PackedUnicharID id = SkGlyph::MakeID(charCode, x, y);
CharGlyphRec* rec = this->getCharGlyphRec(id);
if (rec->fPackedUnicharID != id) {
@ -160,21 +160,26 @@ SkGlyph* SkGlyphCache::lookupByChar(SkUnichar charCode, SkFixed x, SkFixed y) {
// this ID is based on the glyph index
PackedGlyphID combinedID = SkGlyph::MakeID(fScalerContext->charToGlyphID(charCode), x, y);
rec->fPackedGlyphID = combinedID;
return this->lookupByPackedGlyphID(combinedID);
return this->lookupByPackedGlyphID(combinedID, type);
} else {
return this->lookupByPackedGlyphID(rec->fPackedGlyphID);
return this->lookupByPackedGlyphID(rec->fPackedGlyphID, type);
}
}
SkGlyph* SkGlyphCache::lookupByPackedGlyphID(PackedGlyphID packedGlyphID) {
SkGlyph* SkGlyphCache::lookupByPackedGlyphID(PackedGlyphID packedGlyphID, MetricsType type) {
SkGlyph* glyph = fGlyphMap.find(packedGlyphID);
if (nullptr == glyph) {
glyph = this->allocateNewGlyph(packedGlyphID);
glyph = this->allocateNewGlyph(packedGlyphID, type);
} else {
if (type == kFull_MetricsType && glyph->isJustAdvance()) {
fScalerContext->getMetrics(glyph);
}
}
return glyph;
}
SkGlyph* SkGlyphCache::allocateNewGlyph(PackedGlyphID packedGlyphID) {
SkGlyph* SkGlyphCache::allocateNewGlyph(PackedGlyphID packedGlyphID, MetricsType mtype) {
fMemoryUsed += sizeof(SkGlyph);
SkGlyph* glyphPtr;
@ -183,7 +188,13 @@ SkGlyph* SkGlyphCache::allocateNewGlyph(PackedGlyphID packedGlyphID) {
glyph.initGlyphFromCombinedID(packedGlyphID);
glyphPtr = fGlyphMap.set(glyph);
}
fScalerContext->getMetrics(glyphPtr);
if (kJustAdvance_MetricsType == mtype) {
fScalerContext->getAdvance(glyphPtr);
} else {
SkASSERT(kFull_MetricsType == mtype);
fScalerContext->getMetrics(glyphPtr);
}
SkASSERT(glyphPtr->fID != SkGlyph::kImpossibleID);
return glyphPtr;

View File

@ -181,6 +181,11 @@ public:
private:
friend class SkGlyphCache_Globals;
enum MetricsType {
kJustAdvance_MetricsType,
kFull_MetricsType
};
enum {
kHashBits = 8,
kHashCount = 1 << kHashBits,
@ -208,13 +213,15 @@ private:
// Return the SkGlyph* associated with MakeID. The id parameter is the
// combined glyph/x/y id generated by MakeID. If it is just a glyph id
// then x and y are assumed to be zero.
SkGlyph* lookupByPackedGlyphID(PackedGlyphID packedGlyphID);
SkGlyph* lookupByPackedGlyphID(PackedGlyphID packedGlyphID, MetricsType type);
// Return a SkGlyph* associated with unicode id and position x and y.
SkGlyph* lookupByChar(SkUnichar id, SkFixed x = 0, SkFixed y = 0);
SkGlyph* lookupByChar(SkUnichar id, MetricsType type, SkFixed x = 0, SkFixed y = 0);
// Return a new SkGlyph for the glyph ID and subpixel position id.
SkGlyph* allocateNewGlyph(PackedGlyphID packedGlyphID);
// Return a new SkGlyph for the glyph ID and subpixel position id. Limit the amount
// of work
// using type.
SkGlyph* allocateNewGlyph(PackedGlyphID packedGlyphID, MetricsType type);
static bool DetachProc(const SkGlyphCache*, void*) { return true; }