AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Revert of Use SkPaint::getFontBounds() for text bounding boxes in pictures. (patchset #1 id:1 of https://codereview.chromium.org/805983003/)

Browse Source 
Reason for revert:
Test-Win7-ShuttleA-HD2000-x86-Debug-GDI

Original issue's description:
> Use SkPaint::getFontBounds() for text bounding boxes in pictures.
>
> Now that SkTextBlobs have landed, this is a perf no-op, but it at least lets us eliminate a bunch of questionable hacks.
>
> CQ_EXTRA_TRYBOTS=client.skia:Test-Win7-ShuttleA-HD2000-x86-Debug-Trybot,Test-Mac10.8-MacMini4.1-GeForce320M-x86_64-Debug-Trybot
>
>
> BUG=skia:
>
> Committed: https://skia.googlesource.com/skia/+/02d2b9831579173e783569530ab7bae08de907e9

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

Review URL: https://codereview.chromium.org/789793004
This commit is contained in:
mtklein 2014-12-16 06:31:01 -08:00 committed by Commit bot
parent bdd22a35bf
commit 9a5380dd35
2 changed files with 27 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
src/core/SkRecordDraw.cpp
View File

@ -487,13 +487,16 @@ private:
Bounds bounds(const DrawTextOnPath& op) const {
SkRect dst = op.path.getBounds();
// We don't know how the text will curve around the path, so
// pad all sides by the maximum padding in any direction we'd normally apply.
// Pad all sides by the maximum padding in any direction we'd normally apply.
SkRect pad = { 0, 0, 0, 0};
AdjustTextForFontMetrics(&pad, op.paint);
SkScalar max = SkTMax(SkTMax(-pad.fLeft, pad.fRight),
SkTMax(-pad.fTop, pad.fBottom));
dst.outset(max, max);
// That maximum padding happens to always be the right pad today.
SkASSERT(pad.fLeft == -pad.fRight);
SkASSERT(pad.fTop == -pad.fBottom);
SkASSERT(pad.fRight > pad.fBottom);
dst.outset(pad.fRight, pad.fRight);
return this->adjustAndMap(dst, &op.paint);
}
@ -508,21 +511,25 @@ private:
}
static void AdjustTextForFontMetrics(SkRect* rect, const SkPaint& paint) {
// rect was built from only the text's origin points, so we need to
// outset it by the worst-case bounds of the font.
const SkRect bounds = paint.getFontBounds();
rect->fLeft += bounds.fLeft;
rect->fRight += bounds.fRight;
rect->fTop += bounds.fTop;
rect->fBottom += bounds.fBottom;
#ifdef SK_DEBUG
SkRect correct = *rect;
#endif
// crbug.com/373785 ~~> xPad = 4x yPad
// crbug.com/424824 ~~> bump yPad from 2x text size to 2.5x
const SkScalar yPad = 2.5f * paint.getTextSize(),
xPad = 4.0f * yPad;
rect->outset(xPad, yPad);
#ifdef SK_DEBUG
SkPaint::FontMetrics metrics;
paint.getFontMetrics(&metrics);
const SkRect correct = { metrics.fXMin, metrics.fTop, metrics.fXMax, metrics.fBottom };
correct.fLeft += metrics.fXMin;
correct.fTop += metrics.fTop;
correct.fRight += metrics.fXMax;
correct.fBottom += metrics.fBottom;
// See skia:2862 for why we ignore small text sizes.
SkASSERTF(paint.getTextSize() < 0.001f || bounds.contains(correct),
SkASSERTF(paint.getTextSize() < 0.001f || rect->contains(correct),
"%f %f %f %f vs. %f %f %f %f\n",
bounds.fLeft, bounds.fTop, bounds.fRight, bounds.fBottom,
-xPad, -yPad, +xPad, +yPad,
metrics.fXMin, metrics.fTop, metrics.fXMax, metrics.fBottom);
#endif
}

10
tests/RecordDrawTest.cpp
View File

@ -191,11 +191,11 @@ DEF_TEST(RecordDraw_TextBounds, r) {
SkRecordFillBounds(SkRect::MakeWH(SkIntToScalar(W), SkIntToScalar(H)), record, &bbh);
REPORTER_ASSERT(r, bbh.fEntries.count() == 2);
// Font metrics are somewhat platform dependent, so these assertions may need to be adjusted.
// But these particular numbers are left over from the days when we used to wildly overestimate
// font metrics, so these assertions should actually be pretty safe.
REPORTER_ASSERT(r, SkRect::MakeLTRB(0, 0,140, 60).contains(bbh.fEntries[0].bounds));
REPORTER_ASSERT(r, SkRect::MakeLTRB(0,20,180,100).contains(bbh.fEntries[1].bounds));
// We can make these next assertions confidently because SkRecordFillBounds
// builds its bounds by overestimating font metrics in a platform-independent way.
// If that changes, these tests will need to be more flexible.
REPORTER_ASSERT(r, sloppy_rect_eq(bbh.fEntries[0].bounds, SkRect::MakeLTRB(0, 0, 140, 60)));
REPORTER_ASSERT(r, sloppy_rect_eq(bbh.fEntries[1].bounds, SkRect::MakeLTRB(0, 20, 180, 100)));
}
// Base test to ensure start/stop range is respected