diff options
| -rw-r--r-- | src/core/SkRecordDraw.cpp | 37 | ||||
| -rw-r--r-- | tests/RecordDrawTest.cpp | 10 |
2 files changed, 20 insertions, 27 deletions
diff --git a/src/core/SkRecordDraw.cpp b/src/core/SkRecordDraw.cpp index 86621f2170..ba401005fa 100644 --- a/src/core/SkRecordDraw.cpp +++ b/src/core/SkRecordDraw.cpp @@ -487,16 +487,13 @@ private: Bounds bounds(const DrawTextOnPath& op) const { SkRect dst = op.path.getBounds(); - // Pad all sides by the maximum padding in any direction we'd normally apply. + // 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. SkRect pad = { 0, 0, 0, 0}; AdjustTextForFontMetrics(&pad, op.paint); - - // 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); - + SkScalar max = SkTMax(SkTMax(-pad.fLeft, pad.fRight), + SkTMax(-pad.fTop, pad.fBottom)); + dst.outset(max, max); return this->adjustAndMap(dst, &op.paint); } @@ -511,25 +508,21 @@ private: } static void AdjustTextForFontMetrics(SkRect* rect, const SkPaint& paint) { -#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); + // 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 SkPaint::FontMetrics metrics; paint.getFontMetrics(&metrics); - correct.fLeft += metrics.fXMin; - correct.fTop += metrics.fTop; - correct.fRight += metrics.fXMax; - correct.fBottom += metrics.fBottom; + const SkRect correct = { metrics.fXMin, metrics.fTop, metrics.fXMax, metrics.fBottom }; // See skia:2862 for why we ignore small text sizes. - SkASSERTF(paint.getTextSize() < 0.001f || rect->contains(correct), + SkASSERTF(paint.getTextSize() < 0.001f || bounds.contains(correct), "%f %f %f %f vs. %f %f %f %f\n", - -xPad, -yPad, +xPad, +yPad, + bounds.fLeft, bounds.fTop, bounds.fRight, bounds.fBottom, metrics.fXMin, metrics.fTop, metrics.fXMax, metrics.fBottom); #endif } diff --git a/tests/RecordDrawTest.cpp b/tests/RecordDrawTest.cpp index e830af4b43..fb5e9cf3dd 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); - // 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))); + // 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)); } // Base test to ensure start/stop range is respected |