diff options
| -rw-r--r-- | src/core/SkRecordDraw.cpp | 37 | ||||
| -rw-r--r-- | tests/RecordDrawTest.cpp | 10 |
2 files changed, 27 insertions, 20 deletions
diff --git a/src/core/SkRecordDraw.cpp b/src/core/SkRecordDraw.cpp index ba401005fa..86621f2170 100644 --- a/src/core/SkRecordDraw.cpp +++ b/src/core/SkRecordDraw.cpp @@ -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 } diff --git a/tests/RecordDrawTest.cpp b/tests/RecordDrawTest.cpp index fb5e9cf3dd..e830af4b43 100644 --- a/tests/RecordDrawTest.cpp +++ b/tests/RecordDrawTest.cpp @@ -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 |