Move fontmgr_bounds labels to tight bounds

The font bounds may be empty, so label the tight bounds instead,
especially since the labels are of the glyph id that touches that edge
of the tight bounds. Also rotate the labels so they dont' run into each
other.

This also fixes SkMetaData::set so that changing an existing value
doesn't cause strange issues with iterators or attempt to use data from
the previous rec after it's been freed. (Found by running viewer in a
asan build.)

Change-Id: Id255beff5d05310f098bd14baf0935e5fd349e7e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312494
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
This commit is contained in:
Ben Wagner 2020-08-21 16:52:48 -04:00 committed by Skia Commit-Bot
parent 584fe6230d
commit 1968473fea
3 changed files with 93 additions and 56 deletions

View File

@ -268,25 +268,28 @@ private:
if ((fm.fFlags & SkFontMetrics::kUnderlinePositionIsValid_Flag) &&
(fm.fFlags & SkFontMetrics::kUnderlineThicknessIsValid_Flag))
{
SkRect underline{ fontBounds.fLeft, fm.fUnderlinePosition,
fontBounds.fRight, fm.fUnderlinePosition + fm.fUnderlineThickness };
SkRect underline{ min.fLeft, fm.fUnderlinePosition,
min.fRight, fm.fUnderlinePosition + fm.fUnderlineThickness };
canvas->drawRect(underline, metricsPaint);
}
if ((fm.fFlags & SkFontMetrics::kStrikeoutPositionIsValid_Flag) &&
(fm.fFlags & SkFontMetrics::kStrikeoutThicknessIsValid_Flag))
{
SkRect strikeout{ fontBounds.fLeft, fm.fStrikeoutPosition - fm.fStrikeoutThickness,
fontBounds.fRight, fm.fStrikeoutPosition };
SkRect strikeout{ min.fLeft, fm.fStrikeoutPosition - fm.fStrikeoutThickness,
min.fRight, fm.fStrikeoutPosition };
canvas->drawRect(strikeout, metricsPaint);
}
SkGlyphID str[] = { left, right, top, bottom };
SkPoint location[] = {
{fontBounds.left(), fontBounds.centerY()},
{fontBounds.right(), fontBounds.centerY()},
{fontBounds.centerX(), fontBounds.top()},
{fontBounds.centerX(), fontBounds.bottom()}
struct GlyphToDraw {
SkGlyphID id;
SkPoint location;
SkScalar rotation;
} glyphsToDraw [] = {
{left, {min.left(), min.centerY()}, 270},
{right, {min.right(), min.centerY()}, 90},
{top, {min.centerX(), min.top() }, 0},
{bottom, {min.centerX(), min.bottom() }, 180},
};
SkFont labelFont;
@ -296,20 +299,24 @@ private:
if (labelBounds) {
SkString name;
font.getTypefaceOrDefault()->getFamilyName(&name);
canvas->drawString(name, fontBounds.fLeft, fontBounds.fBottom, labelFont, SkPaint());
canvas->drawString(name, min.fLeft, min.fBottom, labelFont, SkPaint());
}
for (size_t i = 0; i < SK_ARRAY_COUNT(str); ++i) {
for (const GlyphToDraw& glyphToDraw : glyphsToDraw) {
SkPath path;
font.getPath(str[i], &path);
font.getPath(glyphToDraw.id, &path);
SkPaint::Style style = path.isEmpty() ? SkPaint::kFill_Style : SkPaint::kStroke_Style;
SkPaint glyphPaint;
glyphPaint.setStyle(style);
canvas->drawSimpleText(&str[i], sizeof(str[0]), SkTextEncoding::kGlyphID, 0, 0, font, glyphPaint);
canvas->drawSimpleText(&glyphToDraw.id, sizeof(glyphToDraw.id),
SkTextEncoding::kGlyphID, 0, 0, font, glyphPaint);
if (labelBounds) {
SkAutoCanvasRestore acr(canvas, true);
canvas->translate(glyphToDraw.location.fX, glyphToDraw.location.fY);
canvas->rotate(glyphToDraw.rotation);
SkString glyphStr;
glyphStr.appendS32(str[i]);
canvas->drawString(glyphStr, location[i].fX, location[i].fY, labelFont, SkPaint());
glyphStr.appendS32(glyphToDraw.id);
canvas->drawString(glyphStr, 0, 0, labelFont, SkPaint());
}
}
@ -344,7 +351,8 @@ private:
// the glyphs which make up the maximum extent.
SkTypeface* typeface = font.getTypefaceOrDefault();
if (typeface && 0 < typeface->countGlyphs() && typeface->countGlyphs() < 1000) {
SkRect drawBounds = show_bounds(canvas, font, x, y, boundsColors[index & 1], fLabelBounds);
SkColor color = boundsColors[index & 1];
SkRect drawBounds = show_bounds(canvas, font, x, y, color, fLabelBounds);
x += drawBounds.width() + 20;
index += 1;
if (x > 900) {

View File

@ -54,24 +54,42 @@ void* SkMetaData::set(const char name[], const void* data, size_t dataSize, Type
SkASSERT(dataSize);
SkASSERT(count > 0);
(void)this->remove(name, type);
FindResult result = this->findWithPrev(name, type);
size_t len = strlen(name);
Rec* rec = Rec::Alloc(sizeof(Rec) + dataSize * count + len + 1);
Rec* rec;
bool reuseRec = result.rec &&
result.rec->fDataLen == dataSize &&
result.rec->fDataCount == count;
if (reuseRec) {
rec = result.rec;
} else {
size_t len = strlen(name);
rec = Rec::Alloc(sizeof(Rec) + dataSize * count + len + 1);
rec->fType = SkToU8(type);
rec->fDataLen = SkToU8(dataSize);
rec->fDataCount = SkToU16(count);
#ifndef SK_DEBUG
rec->fType = SkToU8(type);
#else
rec->fType = type;
#endif
rec->fDataLen = SkToU8(dataSize);
rec->fDataCount = SkToU16(count);
if (data)
memcpy(rec->name(), name, len + 1);
}
if (data) {
memcpy(rec->data(), data, dataSize * count);
memcpy(rec->name(), name, len + 1);
}
rec->fNext = fRec;
fRec = rec;
if (reuseRec) {
// Do nothing, reused
} else if (result.rec) {
// Had one, but had to create a new one. Invalidates iterators.
// Delayed removal since name or data may have been in the result.rec.
this->remove(result);
if (result.prev) {
rec->fNext = result.prev->fNext;
result.prev->fNext = rec;
}
} else {
// Adding a new one, stick it at head.
rec->fNext = fRec;
fRec = rec;
}
return rec->data();
}
@ -141,36 +159,39 @@ bool SkMetaData::findBool(const char name[], bool* value) const
return false;
}
const SkMetaData::Rec* SkMetaData::find(const char name[], Type type) const
{
const Rec* rec = fRec;
while (rec)
{
if (rec->fType == type && !strcmp(rec->name(), name))
return rec;
rec = rec->fNext;
SkMetaData::FindResult SkMetaData::findWithPrev(const char name[], Type type) const {
FindResult current { fRec, nullptr };
while (current.rec) {
if (current.rec->fType == type && !strcmp(current.rec->name(), name))
return current;
current.prev = current.rec;
current.rec = current.rec->fNext;
}
return nullptr;
return current;
}
const SkMetaData::Rec* SkMetaData::find(const char name[], Type type) const {
return this->findWithPrev(name, type).rec;
}
void SkMetaData::remove(FindResult result) {
SkASSERT(result.rec);
if (result.prev) {
result.prev->fNext = result.rec->fNext;
} else {
fRec = result.rec->fNext;
}
Rec::Free(result.rec);
}
bool SkMetaData::remove(const char name[], Type type) {
Rec* rec = fRec;
Rec* prev = nullptr;
while (rec) {
Rec* next = rec->fNext;
if (rec->fType == type && !strcmp(rec->name(), name)) {
if (prev) {
prev->fNext = next;
} else {
fRec = next;
}
Rec::Free(rec);
return true;
}
prev = rec;
rec = next;
FindResult result = this->findWithPrev(name, type);
if (!result.rec) {
return false;
}
return false;
this->remove(result);
return true;
}
bool SkMetaData::removeS32(const char name[])

View File

@ -110,6 +110,14 @@ public:
SkMetaData(const SkMetaData&) = delete;
SkMetaData& operator=(const SkMetaData&) = delete;
private:
struct FindResult {
SkMetaData::Rec* rec;
SkMetaData::Rec* prev;
};
FindResult findWithPrev(const char name[], Type type) const;
void remove(FindResult);
};
#endif