diff --git a/gm/patharcto.cpp b/gm/patharcto.cpp new file mode 100644 index 0000000000..603bf82945 --- /dev/null +++ b/gm/patharcto.cpp @@ -0,0 +1,40 @@ +/* + * Copyright 2019 Google LLC + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#include "gm/gm.h" +#include "include/core/SkCanvas.h" +#include "include/core/SkPath.h" +#include "include/core/SkPaint.h" + +// crbug.com/982968 +// Intended to draw a curvy triangle. +// With the bug, we extend the left side of the triangle with lines, a bit like a flag pole. +// The bug/fix is related to precision of the calculation. The original impl used all floats, +// but the very shallow angle causes our sin/cos and other calcs to lose too many bits. +// The fix was to use doubles for this part of the calc in SkPath::arcTo(). +// +DEF_SIMPLE_GM(shallow_angle_path_arcto, canvas, 300, 300) { + SkPath path; + SkPaint paint; + paint.setStyle(SkPaint::kStroke_Style); + + path.moveTo(313.44189096331155f, 106.6009423589212f); + path.arcTo(284.3113082008462f, 207.1407719157063f, + 255.15053777129728f, 307.6718505416374f, + 697212.0011054524f); + path.lineTo(255.15053777129728f, 307.6718505416374f); + path.arcTo(340.4737465981018f, 252.6907319346971f, + 433.54333477716153f, 212.18116363345337f, + 1251.2484277907251f); + path.lineTo(433.54333477716153f, 212.18116363345337f); + path.arcTo(350.19513833839466f, 185.89280014838369f, + 313.44189096331155f, 106.6009423589212f, + 198.03116885327813f); + + canvas->translate(-200, -50); + canvas->drawPath(path, paint); +}; diff --git a/gn/gm.gni b/gn/gm.gni index 124a61671e..a49b656ba9 100644 --- a/gn/gm.gni +++ b/gn/gm.gni @@ -255,6 +255,7 @@ gm_sources = [ "$_gm/p3.cpp", "$_gm/patch.cpp", "$_gm/path_stroke_with_zero_length.cpp", + "$_gm/patharcto.cpp", "$_gm/pathcontourstart.cpp", "$_gm/patheffects.cpp", "$_gm/pathfill.cpp", diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp index 98b2a51547..a3eeb0b948 100644 --- a/src/core/SkPath.cpp +++ b/src/core/SkPath.cpp @@ -21,6 +21,8 @@ #include "src/core/SkPointPriv.h" #include "src/core/SkSafeMath.h" #include "src/core/SkTLazy.h" +// need SkDVector +#include "src/pathops/SkPathOpsPoint.h" #include #include @@ -1572,32 +1574,30 @@ SkPath& SkPath::arcTo(SkScalar x1, SkScalar y1, SkScalar x2, SkScalar y2, SkScal return this->lineTo(x1, y1); } - SkVector before, after; - // need to know our prev pt so we can construct tangent vectors - { - SkPoint start; - this->getLastPt(&start); - // Handle degenerate cases by adding a line to the first point and - // bailing out. - before.setNormalize(x1 - start.fX, y1 - start.fY); - after.setNormalize(x2 - x1, y2 - y1); - } + SkPoint start; + this->getLastPt(&start); - SkScalar cosh = SkPoint::DotProduct(before, after); - SkScalar sinh = SkPoint::CrossProduct(before, after); + // need double precision for these calcs. + SkDVector befored, afterd; + befored.set({x1 - start.fX, y1 - start.fY}).normalize(); + afterd.set({x2 - x1, y2 - y1}).normalize(); + double cosh = befored.dot(afterd); + double sinh = befored.cross(afterd); - if (SkScalarNearlyZero(sinh)) { // angle is too tight + if (!befored.isFinite() || !afterd.isFinite() || SkScalarNearlyZero(SkDoubleToScalar(sinh))) { return this->lineTo(x1, y1); } - SkScalar dist = SkScalarAbs(radius * (1 - cosh) / sinh); - + // safe to convert back to floats now + SkVector before = befored.asSkVector(); + SkVector after = afterd.asSkVector(); + SkScalar dist = SkScalarAbs(SkDoubleToScalar(radius * (1 - cosh) / sinh)); SkScalar xx = x1 - dist * before.fX; SkScalar yy = y1 - dist * before.fY; after.setLength(dist); this->lineTo(xx, yy); - SkScalar weight = SkScalarSqrt(SK_ScalarHalf + cosh * SK_ScalarHalf); + SkScalar weight = SkScalarSqrt(SkDoubleToScalar(SK_ScalarHalf + cosh * 0.5)); return this->conicTo(x1, y1, x1 + after.fX, y1 + after.fY, weight); } @@ -2337,11 +2337,6 @@ static bool almost_equal(SkScalar compA, SkScalar compB) { return aBits < bBits + epsilon && bBits < aBits + epsilon; } -static bool approximately_zero_when_compared_to(double x, double y) { - return x == 0 || fabs(x) < fabs(y * FLT_EPSILON); -} - - // only valid for a single contour struct Convexicator { Convexicator() diff --git a/src/pathops/SkPathOpsPoint.h b/src/pathops/SkPathOpsPoint.h index 86515a4fac..bca0530fed 100644 --- a/src/pathops/SkPathOpsPoint.h +++ b/src/pathops/SkPathOpsPoint.h @@ -18,9 +18,10 @@ struct SkDVector { double fX; double fY; - void set(const SkVector& pt) { + SkDVector& set(const SkVector& pt) { fX = pt.fX; fY = pt.fY; + return *this; } // only used by testing @@ -84,10 +85,15 @@ struct SkDVector { return fX * fX + fY * fY; } - void normalize() { - double inverseLength = 1 / this->length(); + SkDVector& normalize() { + double inverseLength = sk_ieee_double_divide(1, this->length()); fX *= inverseLength; fY *= inverseLength; + return *this; + } + + bool isFinite() const { + return std::isfinite(fX) && std::isfinite(fY); } };