Fix SkClassifyCubic for near-quadratics
Fixes the threshold logic for "0 ~= d1 && 0 ~= d2".
Previously, if d1 and d2 were both near zero, but on opposite sides
of the threshold, the curve could be misclassified as kCuspAtInfinity
and drawn incorrectly.
Bug: skia:
Change-Id: I65f30ddebf0a4a0b933610d8cc1a2f489efc99e4
Reviewed-on: https://skia-review.googlesource.com/22400
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Cary Clark <caryclark@google.com>
diff --git a/src/core/SkGeometry.cpp b/src/core/SkGeometry.cpp
index 46fd683..a1e8419 100644
--- a/src/core/SkGeometry.cpp
+++ b/src/core/SkGeometry.cpp
@@ -601,62 +601,63 @@
// d1 = d2 = 0, d3 != 0 Quadratic
// d1 = d2 = d3 = 0 Line or Point
static SkCubicType classify_cubic(const double d[4], double t[2], double s[2]) {
- double tolerance = SkTMax(fabs(d[1]), fabs(d[2]));
- tolerance = SkTMax(tolerance, fabs(d[3]));
- tolerance = tolerance * 1e-4;
- if (fabs(d[1]) > tolerance) {
- const double discr = 3 * d[2] * d[2] - 4 * d[1] * d[3];
- if (discr > 0) {
- if (t && s) {
- const double q = 3 * d[2] + copysign(sqrt(3 * discr), d[2]);
- t[0] = q;
- s[0] = 6 * d[1];
- t[1] = 2 * d[3];
- s[1] = q;
- normalize_t_s(t, s, 2);
- sort_and_orient_t_s(t, s);
- }
- return SkCubicType::kSerpentine;
- } else if (discr < 0) {
- if (t && s) {
- const double q = d[2] + copysign(sqrt(-discr), d[2]);
- t[0] = q;
- s[0] = 2 * d[1];
- t[1] = 2 * (d[2] * d[2] - d[3] * d[1]);
- s[1] = d[1] * q;
- normalize_t_s(t, s, 2);
- sort_and_orient_t_s(t, s);
- }
- return SkCubicType::kLoop;
- } else {
- SkASSERT(0 == discr); // Detect NaN.
- if (t && s) {
- t[0] = d[2];
- s[0] = 2 * d[1];
- normalize_t_s(t, s, 1);
- t[1] = t[0];
- s[1] = s[0];
- sort_and_orient_t_s(t, s);
- }
- return SkCubicType::kLocalCusp;
+ // Check for degenerate cubics (quadratics, lines, and points).
+ // This also attempts to detect near-quadratics in a resolution independent fashion, however it
+ // is still up to the caller to check for almost-linear curves if needed.
+ if (fabs(d[1]) + fabs(d[2]) <= fabs(d[3]) * 1e-3) {
+ if (t && s) {
+ t[0] = t[1] = 1;
+ s[0] = s[1] = 0; // infinity
}
+ return 0 == d[3] ? SkCubicType::kLineOrPoint : SkCubicType::kQuadratic;
+ }
+
+ if (0 == d[1]) {
+ SkASSERT(0 != d[2]); // captured in check for degeneracy above.
+ if (t && s) {
+ t[0] = d[3];
+ s[0] = 3 * d[2];
+ normalize_t_s(t, s, 1);
+ t[1] = 1;
+ s[1] = 0; // infinity
+ }
+ return SkCubicType::kCuspAtInfinity;
+ }
+
+ const double discr = 3 * d[2] * d[2] - 4 * d[1] * d[3];
+ if (discr > 0) {
+ if (t && s) {
+ const double q = 3 * d[2] + copysign(sqrt(3 * discr), d[2]);
+ t[0] = q;
+ s[0] = 6 * d[1];
+ t[1] = 2 * d[3];
+ s[1] = q;
+ normalize_t_s(t, s, 2);
+ sort_and_orient_t_s(t, s);
+ }
+ return SkCubicType::kSerpentine;
+ } else if (discr < 0) {
+ if (t && s) {
+ const double q = d[2] + copysign(sqrt(-discr), d[2]);
+ t[0] = q;
+ s[0] = 2 * d[1];
+ t[1] = 2 * (d[2] * d[2] - d[3] * d[1]);
+ s[1] = d[1] * q;
+ normalize_t_s(t, s, 2);
+ sort_and_orient_t_s(t, s);
+ }
+ return SkCubicType::kLoop;
} else {
- if (fabs(d[2]) > tolerance) {
- if (t && s) {
- t[0] = d[3];
- s[0] = 3 * d[2];
- normalize_t_s(t, s, 1);
- t[1] = 1;
- s[1] = 0; // infinity
- }
- return SkCubicType::kCuspAtInfinity;
- } else {
- if (t && s) {
- t[0] = t[1] = 1;
- s[0] = s[1] = 0; // infinity
- }
- return fabs(d[3]) > tolerance ? SkCubicType::kQuadratic : SkCubicType::kLineOrPoint;
+ SkASSERT(0 == discr); // Detect NaN.
+ if (t && s) {
+ t[0] = d[2];
+ s[0] = 2 * d[1];
+ normalize_t_s(t, s, 1);
+ t[1] = t[0];
+ s[1] = s[0];
+ sort_and_orient_t_s(t, s);
}
+ return SkCubicType::kLocalCusp;
}
}
diff --git a/tests/GeometryTest.cpp b/tests/GeometryTest.cpp
index 91136e0..4d955b5 100644
--- a/tests/GeometryTest.cpp
+++ b/tests/GeometryTest.cpp
@@ -8,6 +8,7 @@
#include "SkGeometry.h"
#include "Test.h"
#include "SkRandom.h"
+#include <array>
static bool nearly_equal(const SkPoint& a, const SkPoint& b) {
return SkScalarNearlyEqual(a.fX, b.fX) && SkScalarNearlyEqual(a.fY, b.fY);
@@ -205,6 +206,24 @@
}
}
+static void check_cubic_type(skiatest::Reporter* reporter,
+ const std::array<SkPoint, 4>& bezierPoints, SkCubicType expectedType) {
+ SkCubicType actualType = SkClassifyCubic(bezierPoints.data());
+ REPORTER_ASSERT(reporter, actualType == expectedType);
+}
+
+static void test_classify_cubic(skiatest::Reporter* reporter) {
+ check_cubic_type(reporter, {{{149.325f, 107.705f}, {149.325f, 103.783f},
+ {151.638f, 100.127f}, {156.263f, 96.736f}}},
+ SkCubicType::kQuadratic);
+ check_cubic_type(reporter, {{{225.694f, 223.15f}, {209.831f, 224.837f},
+ {195.994f, 230.237f}, {184.181f, 239.35f}}},
+ SkCubicType::kQuadratic);
+ check_cubic_type(reporter, {{{4.873f, 5.581f}, {5.083f, 5.2783f},
+ {5.182f, 4.8593f}, {5.177f, 4.3242f}}},
+ SkCubicType::kSerpentine);
+}
+
DEF_TEST(Geometry, reporter) {
SkPoint pts[3], dst[5];
@@ -233,4 +252,5 @@
test_quad_tangents(reporter);
test_conic_tangents(reporter);
test_conic_to_quads(reporter);
+ test_classify_cubic(reporter);
}
