Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(179)

Unified Diff: src/core/SkRecordDraw.cpp

Issue 680363003: Use SkTypeface::getBounds() in bounding-box calculations. (Closed) Base URL: https://skia.googlesource.com/skia.git@master
Patch Set: fudge differently Created 6 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | tests/RecordDrawTest.cpp » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/core/SkRecordDraw.cpp
diff --git a/src/core/SkRecordDraw.cpp b/src/core/SkRecordDraw.cpp
index 5981245c018700e8ed969e7abc75e1d76c63b7ba..6dcb9b8a4976d633b93b37cc82da12f477b02a00 100644
--- a/src/core/SkRecordDraw.cpp
+++ b/src/core/SkRecordDraw.cpp
@@ -7,6 +7,7 @@
#include "SkRecordDraw.h"
#include "SkPatchUtils.h"
+#include "SkTypeface.h"
void SkRecordDraw(const SkRecord& record,
SkCanvas* canvas,
@@ -444,15 +445,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 aroudn 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);
}
@@ -464,26 +463,39 @@ 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 typeface.
+ SkAutoTUnref<SkTypeface> au;
+ SkTypeface* tf = paint.getTypeface();
+ if (!tf) {
+ au.reset(SkTypeface::RefDefault());
+ tf = au.get();
+ }
+ const SkScalar size = paint.getTextSize();
+ const SkRect tb = tf->getBounds();
+ rect->fLeft += size * tb.fLeft;
+ rect->fRight += size * tb.fRight;
+ rect->fTop += size * tb.fTop;
+ rect->fBottom += size * tb.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;
+ // Our numbers are so close to correct now that we're sometimes _slightly_ under. Add
+ // a little fudge factor here to make sure the assert just catches really bad failures.
+ const SkScalar kFudge = 0.0625f;
+ SkRect fudged = {
+ size * (tb.fLeft - kFudge),
+ size * (tb.fTop - kFudge),
+ size * (tb.fRight + kFudge),
+ size * (tb.fBottom + kFudge),
+ };
+ 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),
- "%f %f %f %f vs. %f %f %f %f\n",
- -xPad, -yPad, +xPad, +yPad,
- metrics.fXMin, metrics.fTop, metrics.fXMax, metrics.fBottom);
+ SkASSERTF(paint.getTextSize() < 0.001f || fudged.contains(correct),
+ "%f %f %f %f vs. %f %f %f %f, text size %f\n",
+ size*tb.fLeft, size*tb.fTop, size*tb.fRight, size*tb.fBottom,
+ metrics.fXMin, metrics.fTop, metrics.fXMax, metrics.fBottom,
+ paint.getTextSize());
#endif
}
« no previous file with comments | « no previous file | tests/RecordDrawTest.cpp » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698