Improve computeMatrices singular matrix handling.
The existing singular matrix detection in computeMatrices is sufficient
but not necessary. Since compute matrices is already doing a QR
decomposition, use that to determine singularity instead. This is both
faster and more accurate than the previous method for the common case.
BUG=https://bugzilla.mozilla.org/show_bug.cgi?id=1305085
Change-Id: Iccef8368527b45e4eb565eddbebbbcf41ca66a2c
Reviewed-on: https://skia-review.googlesource.com/20054
Reviewed-by: Lee Salzman <lsalzman@mozilla.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
diff --git a/src/core/SkScalerContext.cpp b/src/core/SkScalerContext.cpp
index f310011..b2860e8 100644
--- a/src/core/SkScalerContext.cpp
+++ b/src/core/SkScalerContext.cpp
@@ -703,37 +703,11 @@
*A_out = A;
}
- // If the 'total' matrix is singular, set the 'scale' to something finite and zero the matrices.
- // All underlying ports have issues with zero text size, so use the matricies to zero.
-
- // Map the vectors [0,1], [1,0], [1,1] and [1,-1] (the EM) through the 'total' matrix.
- // If the length of one of these vectors is less than 1/256 then an EM filling square will
- // never affect any pixels.
- SkVector diag[4] = { { A.getScaleX() , A.getSkewY() },
- { A.getSkewX(), A.getScaleY() },
- { A.getScaleX() + A.getSkewX(), A.getScaleY() + A.getSkewY() },
- { A.getScaleX() - A.getSkewX(), A.getScaleY() - A.getSkewY() }, };
- if (diag[0].lengthSqd() <= SK_ScalarNearlyZero * SK_ScalarNearlyZero ||
- diag[1].lengthSqd() <= SK_ScalarNearlyZero * SK_ScalarNearlyZero ||
- diag[2].lengthSqd() <= SK_ScalarNearlyZero * SK_ScalarNearlyZero ||
- diag[3].lengthSqd() <= SK_ScalarNearlyZero * SK_ScalarNearlyZero)
- {
- s->fX = SK_Scalar1;
- s->fY = SK_Scalar1;
- sA->setScale(0, 0);
- if (GsA) {
- GsA->setScale(0, 0);
- }
- if (G_inv) {
- G_inv->reset();
- }
- return false;
- }
-
// GA is the matrix A with rotation removed.
SkMatrix GA;
bool skewedOrFlipped = A.getSkewX() || A.getSkewY() || A.getScaleX() < 0 || A.getScaleY() < 0;
if (skewedOrFlipped) {
+ // QR by Givens rotations. G is Q^T and GA is R. G is rotational (no reflections).
// h is where A maps the horizontal baseline.
SkPoint h = SkPoint::Make(SK_Scalar1, 0);
A.mapPoints(&h, 1);
@@ -759,6 +733,25 @@
}
}
+ // If the 'total' matrix is singular, set the 'scale' to something finite and zero the matrices.
+ // All underlying ports have issues with zero text size, so use the matricies to zero.
+ // If one of the scale factors is less than 1/256 then an EM filling square will
+ // never affect any pixels.
+ if (SkScalarAbs(GA.get(SkMatrix::kMScaleX)) <= SK_ScalarNearlyZero ||
+ SkScalarAbs(GA.get(SkMatrix::kMScaleY)) <= SK_ScalarNearlyZero)
+ {
+ s->fX = SK_Scalar1;
+ s->fY = SK_Scalar1;
+ sA->setScale(0, 0);
+ if (GsA) {
+ GsA->setScale(0, 0);
+ }
+ if (G_inv) {
+ G_inv->reset();
+ }
+ return false;
+ }
+
// At this point, given GA, create s.
switch (preMatrixScale) {
case kFull_PreMatrixScale:
diff --git a/tests/DrawTextTest.cpp b/tests/DrawTextTest.cpp
index 2f8fe05..0134d05 100644
--- a/tests/DrawTextTest.cpp
+++ b/tests/DrawTextTest.cpp
@@ -128,3 +128,41 @@
canvas->drawString("a", 0.0f, -y, SkPaint());
}
}
+
+// Test drawing text with some unusual matricies.
+// We measure success by not crashing or asserting.
+DEF_TEST(DrawText_weirdMatricies, r) {
+ auto surface = SkSurface::MakeRasterN32Premul(100,100);
+ auto canvas = surface->getCanvas();
+
+ SkPaint paint;
+ paint.setAntiAlias(true);
+ paint.setLCDRenderText(true);
+
+ struct {
+ SkScalar textSize;
+ SkScalar matrix[9];
+ } testCases[] = {
+ // 2x2 singular
+ {10, { 0, 0, 0, 0, 0, 0, 0, 0, 1}},
+ {10, { 0, 0, 0, 0, 1, 0, 0, 0, 1}},
+ {10, { 0, 0, 0, 1, 0, 0, 0, 0, 1}},
+ {10, { 0, 0, 0, 1, 1, 0, 0, 0, 1}},
+ {10, { 0, 1, 0, 0, 1, 0, 0, 0, 1}},
+ {10, { 1, 0, 0, 0, 0, 0, 0, 0, 1}},
+ {10, { 1, 0, 0, 1, 0, 0, 0, 0, 1}},
+ {10, { 1, 1, 0, 0, 0, 0, 0, 0, 1}},
+ {10, { 1, 1, 0, 1, 1, 0, 0, 0, 1}},
+ // See https://bugzilla.mozilla.org/show_bug.cgi?id=1305085 .
+ { 1, {10, 20, 0, 20, 40, 0, 0, 0, 1}},
+ };
+
+ for (const auto& testCase : testCases) {
+ paint.setTextSize(testCase.textSize);
+ const SkScalar(&m)[9] = testCase.matrix;
+ SkMatrix mat;
+ mat.setAll(m[0], m[1], m[2], m[3], m[4], m[5], m[6], m[7], m[8]);
+ canvas->setMatrix(mat);
+ canvas->drawString("Hamburgefons", 10, 10, paint);
+ }
+}