Revert of Make the glyph array entries inline. (patchset #10 id:170001 of https://codereview.chromium.org/885903002/)

Reason for revert:
I suspect this is causing the off-by-one character issues that show up in gold.skia.org as of now. All the errors I've seen are on a Win7 bot. Example:

good:
https://gold.skia.org/img/images/35396bb0d299b81c0031dc0632a019d4.png

bad:
https://gold.skia.org/img/images/484e511f9e696d95031cd25aeae59da0.png

Original issue's description:
> Make the glyph array entries inline.
> BUG=skia:
>
> Committed: https://skia.googlesource.com/skia/+/4c08f16b252a55e438a61f26e5581394ed177da1

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

Review URL: https://codereview.chromium.org/897463004
This commit is contained in:
bsalomon 2015-02-02 21:06:23 -08:00 committed by Commit bot
parent b9eb4ac0f1
commit 9bf4e5bbf6
3 changed files with 140 additions and 142 deletions

View File

@ -104,10 +104,8 @@ struct SkGlyph {
kSubShiftY = 0
};
// The code is increased by one in MakeID. Adjust back. This allows the zero
// id to be the invalid id.
static unsigned ID2Code(uint32_t id) {
return (id & kCodeMask) - 1;
return id & kCodeMask;
}
static unsigned ID2SubX(uint32_t id) {
@ -127,21 +125,17 @@ struct SkGlyph {
return sub << (16 - kSubBits);
}
// This and the MakeID below must not return an id of zero. Zero is used as
// the invalid id.
static uint32_t MakeID(unsigned code) {
SkASSERT(code + 1 <= kCodeMask);
return code + 1;
return code;
}
// See comment for MakeID above.
static uint32_t MakeID(unsigned code, SkFixed x, SkFixed y) {
SkASSERT(code + 1 <= kCodeMask);
SkASSERT(code <= kCodeMask);
x = FixedToSub(x);
y = FixedToSub(y);
return (x << (kSubShift + kSubShiftX)) |
(y << (kSubShift + kSubShiftY)) |
(code + 1);
code;
}
void toMask(SkMask* mask) const;

View File

@ -66,14 +66,10 @@ SkGlyphCache::SkGlyphCache(SkTypeface* typeface, const SkDescriptor* desc, SkSca
fDesc = desc->copy();
fScalerContext->getFontMetrics(&fFontMetrics);
// Create the sentinel SkGlyph.
SkGlyph* sentinel = fGlyphArray.insert(kSentinelGlyphID);
sentinel->init(0);
// Initialize all index to zero which points to the sentinel SkGlyph.
memset(fGlyphHash, 0x00, sizeof(fGlyphHash));
// init to 0 so that all of the pointers will be null
memset(fGlyphHash, 0, sizeof(fGlyphHash));
fMemoryUsed = sizeof(*this);
fGlyphArray.setReserve(kMinGlyphCount);
@ -110,10 +106,10 @@ SkGlyphCache::~SkGlyphCache() {
}
#endif
SkGlyph* gptr = fGlyphArray.begin();
SkGlyph* stop = fGlyphArray.end();
SkGlyph** gptr = fGlyphArray.begin();
SkGlyph** stop = fGlyphArray.end();
while (gptr < stop) {
SkPath* path = gptr->fPath;
SkPath* path = (*gptr)->fPath;
if (path) {
SkDELETE(path);
}
@ -126,31 +122,15 @@ SkGlyphCache::~SkGlyphCache() {
SkGlyphCache::CharGlyphRec* SkGlyphCache::getCharGlyphRec(uint32_t id) {
if (NULL == fCharToGlyphHash.get()) {
// Allocate the array.
fCharToGlyphHash.reset(kHashCount);
// Initialize entries of fCharToGlyphHash to index the sentinel glyph.
memset(fCharToGlyphHash.get(), 0x00,
// init with 0xFF so that the charCode field will be -1, which is invalid
memset(fCharToGlyphHash.get(), 0xFF,
sizeof(CharGlyphRec) * kHashCount);
}
return &fCharToGlyphHash[ID2HashIndex(id)];
}
void SkGlyphCache::adjustCaches(int insertion_index) {
for (int i = 0; i < kHashCount; ++i) {
if (fGlyphHash[i] >= SkToU16(insertion_index)) {
fGlyphHash[i] += 1;
}
}
if (fCharToGlyphHash.get() != NULL) {
for (int i = 0; i < kHashCount; ++i) {
if (fCharToGlyphHash[i].fGlyphIndex >= SkToU16(insertion_index)) {
fCharToGlyphHash[i].fGlyphIndex += 1;
}
}
}
}
///////////////////////////////////////////////////////////////////////////////
#ifdef SK_DEBUG
@ -165,7 +145,7 @@ uint16_t SkGlyphCache::unicharToGlyph(SkUnichar charCode) {
const CharGlyphRec& rec = *this->getCharGlyphRec(id);
if (rec.fID == id) {
return fGlyphArray[rec.fGlyphIndex].getGlyphID();
return rec.fGlyph->getGlyphID();
} else {
return fScalerContext->charToGlyphID(charCode);
}
@ -183,118 +163,159 @@ unsigned SkGlyphCache::getGlyphCount() {
const SkGlyph& SkGlyphCache::getUnicharAdvance(SkUnichar charCode) {
VALIDATE();
return *this->lookupByChar(charCode, kJustAdvance_MetricsType);
uint32_t id = SkGlyph::MakeID(charCode);
CharGlyphRec* rec = this->getCharGlyphRec(id);
if (rec->fID != id) {
// this ID is based on the UniChar
rec->fID = id;
// this ID is based on the glyph index
id = SkGlyph::MakeID(fScalerContext->charToGlyphID(charCode));
rec->fGlyph = this->lookupMetrics(id, kJustAdvance_MetricsType);
}
return *rec->fGlyph;
}
const SkGlyph& SkGlyphCache::getGlyphIDAdvance(uint16_t glyphID) {
VALIDATE();
uint32_t id = SkGlyph::MakeID(glyphID);
return *this->lookupByChar(id, kJustAdvance_MetricsType);
unsigned index = ID2HashIndex(id);
SkGlyph* glyph = fGlyphHash[index];
if (NULL == glyph || glyph->fID != id) {
glyph = this->lookupMetrics(glyphID, kJustAdvance_MetricsType);
fGlyphHash[index] = glyph;
}
return *glyph;
}
///////////////////////////////////////////////////////////////////////////////
const SkGlyph& SkGlyphCache::getUnicharMetrics(SkUnichar charCode) {
VALIDATE();
return *this->lookupByChar(charCode, kFull_MetricsType);
uint32_t id = SkGlyph::MakeID(charCode);
CharGlyphRec* rec = this->getCharGlyphRec(id);
if (rec->fID != id) {
RecordHashCollisionIf(rec->fGlyph != NULL);
// this ID is based on the UniChar
rec->fID = id;
// this ID is based on the glyph index
id = SkGlyph::MakeID(fScalerContext->charToGlyphID(charCode));
rec->fGlyph = this->lookupMetrics(id, kFull_MetricsType);
} else {
RecordHashSuccess();
if (rec->fGlyph->isJustAdvance()) {
fScalerContext->getMetrics(rec->fGlyph);
}
}
SkASSERT(rec->fGlyph->isFullMetrics());
return *rec->fGlyph;
}
const SkGlyph& SkGlyphCache::getUnicharMetrics(SkUnichar charCode,
SkFixed x, SkFixed y) {
VALIDATE();
return *this->lookupByChar(charCode, kFull_MetricsType, x, y);
uint32_t id = SkGlyph::MakeID(charCode, x, y);
CharGlyphRec* rec = this->getCharGlyphRec(id);
if (rec->fID != id) {
RecordHashCollisionIf(rec->fGlyph != NULL);
// this ID is based on the UniChar
rec->fID = id;
// this ID is based on the glyph index
id = SkGlyph::MakeID(fScalerContext->charToGlyphID(charCode), x, y);
rec->fGlyph = this->lookupMetrics(id, kFull_MetricsType);
} else {
RecordHashSuccess();
if (rec->fGlyph->isJustAdvance()) {
fScalerContext->getMetrics(rec->fGlyph);
}
}
SkASSERT(rec->fGlyph->isFullMetrics());
return *rec->fGlyph;
}
const SkGlyph& SkGlyphCache::getGlyphIDMetrics(uint16_t glyphID) {
VALIDATE();
uint32_t id = SkGlyph::MakeID(glyphID);
return *this->lookupByCombinedID(id, kFull_MetricsType);
unsigned index = ID2HashIndex(id);
SkGlyph* glyph = fGlyphHash[index];
if (NULL == glyph || glyph->fID != id) {
RecordHashCollisionIf(glyph != NULL);
glyph = this->lookupMetrics(glyphID, kFull_MetricsType);
fGlyphHash[index] = glyph;
} else {
RecordHashSuccess();
if (glyph->isJustAdvance()) {
fScalerContext->getMetrics(glyph);
}
}
SkASSERT(glyph->isFullMetrics());
return *glyph;
}
const SkGlyph& SkGlyphCache::getGlyphIDMetrics(uint16_t glyphID, SkFixed x, SkFixed y) {
VALIDATE();
uint32_t id = SkGlyph::MakeID(glyphID, x, y);
return *this->lookupByCombinedID(id, kFull_MetricsType);
unsigned index = ID2HashIndex(id);
SkGlyph* glyph = fGlyphHash[index];
if (NULL == glyph || glyph->fID != id) {
RecordHashCollisionIf(glyph != NULL);
glyph = this->lookupMetrics(id, kFull_MetricsType);
fGlyphHash[index] = glyph;
} else {
RecordHashSuccess();
if (glyph->isJustAdvance()) {
fScalerContext->getMetrics(glyph);
}
}
SkASSERT(glyph->isFullMetrics());
return *glyph;
}
SkGlyph* SkGlyphCache::lookupByChar(SkUnichar charCode, MetricsType type, SkFixed x, SkFixed y) {
uint32_t id = SkGlyph::MakeID(charCode, x, y);
CharGlyphRec* rec = this->getCharGlyphRec(id);
SkGlyph* SkGlyphCache::lookupMetrics(uint32_t id, MetricsType mtype) {
SkGlyph* glyph;
if (rec->fID != id) {
RecordHashCollisionIf(glyph_index != kSentinelGlyphIndex);
// this ID is based on the UniChar
rec->fID = id;
// this ID is based on the glyph index
id = SkGlyph::MakeID(fScalerContext->charToGlyphID(charCode), x, y);
rec->fGlyphIndex = this->lookupMetrics(id, type);
glyph = &fGlyphArray[rec->fGlyphIndex];
} else {
RecordHashSuccess();
glyph = &fGlyphArray[rec->fGlyphIndex];
if (type == kFull_MetricsType && glyph->isJustAdvance()) {
fScalerContext->getMetrics(glyph);
}
}
return glyph;
}
SkGlyph* SkGlyphCache::lookupByCombinedID(uint32_t id, MetricsType type) {
uint32_t hash_index = ID2HashIndex(id);
uint16_t glyph_index = fGlyphHash[hash_index];
SkGlyph* glyph = &fGlyphArray[glyph_index];
if (glyph->fID != id) {
RecordHashCollisionIf(glyph_index != kSentinelGlyphIndex);
glyph_index = this->lookupMetrics(id, type);
fGlyphHash[hash_index] = glyph_index;
glyph = &fGlyphArray[glyph_index];
} else {
RecordHashSuccess();
if (type == kFull_MetricsType && glyph->isJustAdvance()) {
fScalerContext->getMetrics(glyph);
}
}
return glyph;
}
int hi = 0;
int count = fGlyphArray.count();
uint16_t SkGlyphCache::lookupMetrics(uint32_t id, MetricsType mtype) {
// Count is always greater than 0 because of the sentinel.
SkGlyph* gptr = fGlyphArray.begin();
int lo = 0;
int hi = fGlyphArray.count() - 1;
while (lo < hi) {
int mid = (hi + lo) >> 1;
if (gptr[mid].fID < id) {
lo = mid + 1;
} else {
hi = mid;
if (count) {
SkGlyph** gptr = fGlyphArray.begin();
int lo = 0;
hi = count - 1;
while (lo < hi) {
int mid = (hi + lo) >> 1;
if (gptr[mid]->fID < id) {
lo = mid + 1;
} else {
hi = mid;
}
}
glyph = gptr[hi];
if (glyph->fID == id) {
if (kFull_MetricsType == mtype && glyph->isJustAdvance()) {
fScalerContext->getMetrics(glyph);
}
return glyph;
}
// check if we need to bump hi before falling though to the allocator
if (glyph->fID < id) {
hi += 1;
}
}
uint16_t glyph_index = hi;
SkGlyph* glyph = &gptr[glyph_index];
if (glyph->fID == id) {
if (kFull_MetricsType == mtype && glyph->isJustAdvance()) {
fScalerContext->getMetrics(glyph);
}
SkASSERT(glyph_index != kSentinelGlyphIndex);
return glyph_index;
}
// check if we need to bump hi before falling though to the allocator
if (glyph->fID < id) {
glyph_index += 1;
}
// not found, but hi tells us where to inser the new glyph
fMemoryUsed += sizeof(SkGlyph);
this->adjustCaches(glyph_index);
glyph = fGlyphArray.insert(glyph_index);
glyph = (SkGlyph*)fGlyphAlloc.alloc(sizeof(SkGlyph),
SkChunkAlloc::kThrow_AllocFailType);
glyph->init(id);
*fGlyphArray.insert(hi) = glyph;
if (kJustAdvance_MetricsType == mtype) {
fScalerContext->getAdvance(glyph);
@ -303,8 +324,7 @@ uint16_t SkGlyphCache::lookupMetrics(uint32_t id, MetricsType mtype) {
fScalerContext->getMetrics(glyph);
}
SkASSERT(glyph_index != kSentinelGlyphIndex);
return glyph_index;
return glyph;
}
const void* SkGlyphCache::findImage(const SkGlyph& glyph) {

View File

@ -187,48 +187,32 @@ private:
kFull_MetricsType
};
// 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 assuemd to be zero.
SkGlyph* lookupByCombinedID(uint32_t id, MetricsType type);
// Return a SkGlyph* associated with unicode id and position x and y.
SkGlyph* lookupByChar(SkUnichar id, MetricsType type, SkFixed x = 0, SkFixed y = 0);
// Return the index of id in the fGlyphArray. If it does
// not exist, create a new one using MaetricsType.
uint16_t lookupMetrics(uint32_t id, MetricsType type);
SkGlyph* lookupMetrics(uint32_t id, MetricsType);
static bool DetachProc(const SkGlyphCache*, void*) { return true; }
SkGlyphCache* fNext, *fPrev;
SkDescriptor* fDesc;
SkScalerContext* fScalerContext;
SkGlyphCache* fNext, *fPrev;
SkDescriptor* fDesc;
SkScalerContext* fScalerContext;
SkPaint::FontMetrics fFontMetrics;
enum {
kHashBits = 8,
kHashCount = 1 << kHashBits,
kHashMask = kHashCount - 1,
kSentinelGlyphIndex = 0,
kSentinelGlyphID = 0
kHashBits = 8,
kHashCount = 1 << kHashBits,
kHashMask = kHashCount - 1
};
// A quick lookup to avoid the binary search looking for glyphs in fGlyphArray.
uint16_t fGlyphHash[kHashCount];
SkTDArray<SkGlyph> fGlyphArray;
SkChunkAlloc fGlyphAlloc;
SkGlyph* fGlyphHash[kHashCount];
SkTDArray<SkGlyph*> fGlyphArray;
SkChunkAlloc fGlyphAlloc;
struct CharGlyphRec {
uint32_t fID; // unichar + subpixel
uint16_t fGlyphIndex;
SkGlyph* fGlyph;
};
// no reason to use the same kHashCount as fGlyphHash, but we do for now
// Dynamically allocated when chars are encountered.
SkAutoTArray<CharGlyphRec> fCharToGlyphHash;
// The id arg is a combined id generated by MakeID.
CharGlyphRec* getCharGlyphRec(uint32_t id);
void adjustCaches(int insertion_index);
static inline unsigned ID2HashIndex(uint32_t h) {
return SkChecksum::CheapMix(h) & kHashMask;