Fix SkPathStroker::lineTo() for line with length SK_ScalarNearlyZero
authorepoger@google.com <epoger@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81>
Wed, 11 Apr 2012 17:51:01 +0000 (17:51 +0000)
committerepoger@google.com <epoger@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81>
Wed, 11 Apr 2012 17:51:01 +0000 (17:51 +0000)
Review URL: https://codereview.appspot.com/5992077

git-svn-id: http://skia.googlecode.com/svn/trunk@3650 2bbb7eff-a529-9590-31e7-b0007b416f81

include/core/SkPath.h
include/core/SkPoint.h
src/core/SkPoint.cpp
tests/CanvasTest.cpp

index 957d50e..01b5488 100644 (file)
@@ -185,7 +185,7 @@ public:
         @return true if the line is of zero length; otherwise false.
     */
     static bool IsLineDegenerate(const SkPoint& p1, const SkPoint& p2) {
-        return p1.equalsWithinTolerance(p2, SK_ScalarNearlyZero);
+        return p1.equalsWithinTolerance(p2);
     }
 
     /** Test a quad for zero length
@@ -194,8 +194,8 @@ public:
     */
     static bool IsQuadDegenerate(const SkPoint& p1, const SkPoint& p2,
                                  const SkPoint& p3) {
-        return p1.equalsWithinTolerance(p2, SK_ScalarNearlyZero) &&
-               p2.equalsWithinTolerance(p3, SK_ScalarNearlyZero);
+        return p1.equalsWithinTolerance(p2) &&
+               p2.equalsWithinTolerance(p3);
     }
 
     /** Test a cubic curve for zero length
@@ -204,9 +204,9 @@ public:
     */
     static bool IsCubicDegenerate(const SkPoint& p1, const SkPoint& p2,
                                   const SkPoint& p3, const SkPoint& p4) {
-        return p1.equalsWithinTolerance(p2, SK_ScalarNearlyZero) &&
-               p2.equalsWithinTolerance(p3, SK_ScalarNearlyZero) &&
-               p3.equalsWithinTolerance(p4, SK_ScalarNearlyZero);
+        return p1.equalsWithinTolerance(p2) &&
+               p2.equalsWithinTolerance(p3) &&
+               p3.equalsWithinTolerance(p4);
     }
 
     /** Returns true if the path specifies a rectangle. If so, and if rect is
index d371e64..4570e25 100644 (file)
@@ -315,11 +315,29 @@ struct SK_API SkPoint {
         return a.fX != b.fX || a.fY != b.fY;
     }
 
-    /** Return true if this and the given point are componentwise within tol.
+    /** Return true if this point and the given point are far enough apart
+        such that a vector between them would be non-degenerate.
+
+        WARNING: Unlike the deprecated version of equalsWithinTolerance(),
+        this method does not use componentwise comparison.  Instead, it
+        uses a comparison designed to match judgments elsewhere regarding
+        degeneracy ("points A and B are so close that the vector between them
+        is essentially zero").
+    */
+    bool equalsWithinTolerance(const SkPoint& p) const {
+        return !CanNormalize(fX - p.fX, fY - p.fY);
+    }
+
+    /** DEPRECATED: Return true if this and the given point are componentwise
+        within tolerance "tol".
+
+        WARNING: There is no guarantee that the result will reflect judgments
+        elsewhere regarding degeneracy ("points A and B are so close that the
+        vector between them is essentially zero").
     */
-    bool equalsWithinTolerance(const SkPoint& v, SkScalar tol) const {
-        return SkScalarNearlyZero(fX - v.fX, tol)
-               && SkScalarNearlyZero(fY - v.fY, tol);
+    bool equalsWithinTolerance(const SkPoint& p, SkScalar tol) const {
+        return SkScalarNearlyZero(fX - p.fX, tol)
+               && SkScalarNearlyZero(fY - p.fY, tol);
     }
 
     /** Returns a new point whose coordinates are the difference between
index 5747504..a347b86 100644 (file)
@@ -87,31 +87,49 @@ bool SkPoint::setLength(SkScalar length) {
     return this->setLength(fX, fY, length);
 }
 
+#ifdef SK_SCALAR_IS_FLOAT
+
+// Returns the square of the Euclidian distance to (dx,dy).
+static inline float getLengthSquared(float dx, float dy) {
+    return dx * dx + dy * dy;
+}
+
+// Calculates the square of the Euclidian distance to (dx,dy) and stores it in
+// *lengthSquared.  Returns true if the distance is judged to be "nearly zero".
+//
+// This logic is encapsulated in a helper method to make it explicit that we
+// always perform this check in the same manner, to avoid inconsistencies
+// (see http://code.google.com/p/skia/issues/detail?id=560 ).
+static inline bool isLengthNearlyZero(float dx, float dy,
+                                      float *lengthSquared) {
+    *lengthSquared = getLengthSquared(dx, dy);
+    return *lengthSquared <= (SK_ScalarNearlyZero * SK_ScalarNearlyZero);
+}
+
 SkScalar SkPoint::Normalize(SkPoint* pt) {
-    SkScalar mag = SkPoint::Length(pt->fX, pt->fY);
-    if (mag > SK_ScalarNearlyZero) {
-        SkScalar scale = SkScalarInvert(mag);
-        pt->fX = SkScalarMul(pt->fX, scale);
-        pt->fY = SkScalarMul(pt->fY, scale);
+    float mag2;
+    if (!isLengthNearlyZero(pt->fX, pt->fY, &mag2)) {
+        float mag = sk_float_sqrt(mag2);
+        float scale = 1.0 / mag;
+        pt->fX = pt->fX * scale;
+        pt->fY = pt->fY * scale;
         return mag;
     }
     return 0;
 }
 
-#ifdef SK_SCALAR_IS_FLOAT
-
 bool SkPoint::CanNormalize(SkScalar dx, SkScalar dy) {
-    float mag2 = dx * dx + dy * dy;
-    return mag2 > SK_ScalarNearlyZero * SK_ScalarNearlyZero;
+    float mag2_unused;
+    return !isLengthNearlyZero(dx, dy, &mag2_unused);
 }
 
 SkScalar SkPoint::Length(SkScalar dx, SkScalar dy) {
-    return sk_float_sqrt(dx * dx + dy * dy);
+    return sk_float_sqrt(getLengthSquared(dx, dy));
 }
 
 bool SkPoint::setLength(float x, float y, float length) {
-    float mag2 = x * x + y * y;
-    if (mag2 > SK_ScalarNearlyZero * SK_ScalarNearlyZero) {
+    float mag2;
+    if (!isLengthNearlyZero(x, y, &mag2)) {
         float scale = length / sk_float_sqrt(mag2);
         fX = x * scale;
         fY = y * scale;
@@ -124,12 +142,25 @@ bool SkPoint::setLength(float x, float y, float length) {
 
 #include "Sk64.h"
 
-bool SkPoint::CanNormalize(SkScalar dx, SkScalar dy) {
-    Sk64    tmp1, tmp2, tolSqr;
-    
-    tmp1.setMul(dx, dx);
-    tmp2.setMul(dy, dy);
-    tmp1.add(tmp2);
+// Returns the square of the Euclidian distance to (dx,dy) in *result.
+static inline void getLengthSquared(SkScalar dx, SkScalar dy, Sk64 *result) {
+    Sk64    dySqr;
+
+    result->setMul(dx, dx);
+    dySqr.setMul(dy, dy);
+    result->add(dySqr);
+}
+
+// Calculates the square of the Euclidian distance to (dx,dy) and stores it in
+// *lengthSquared.  Returns true if the distance is judged to be "nearly zero".
+//
+// This logic is encapsulated in a helper method to make it explicit that we
+// always perform this check in the same manner, to avoid inconsistencies
+// (see http://code.google.com/p/skia/issues/detail?id=560 ).
+static inline bool isLengthNearlyZero(SkScalar dx, SkScalar dy,
+                                      Sk64 *lengthSquared) {
+    Sk64 tolSqr;
+    getLengthSquared(dx, dy, lengthSquared);
 
     // we want nearlyzero^2, but to compute it fast we want to just do a
     // 32bit multiply, so we require that it not exceed 31bits. That is true
@@ -138,17 +169,30 @@ bool SkPoint::CanNormalize(SkScalar dx, SkScalar dy) {
     SkASSERT(SK_ScalarNearlyZero <= 0xB504);
 
     tolSqr.set(0, SK_ScalarNearlyZero * SK_ScalarNearlyZero);
-    return tmp1 > tolSqr;
+    return *lengthSquared <= tolSqr;
 }
 
-SkScalar SkPoint::Length(SkScalar dx, SkScalar dy) {
-    Sk64    tmp1, tmp2;
+SkScalar SkPoint::Normalize(SkPoint* pt) {
+    Sk64 mag2;
+    if (!isLengthNearlyZero(pt->fX, pt->fY, &mag2)) {
+        SkScalar mag = mag2.getSqrt();
+        SkScalar scale = SkScalarInvert(mag);
+        pt->fX = SkScalarMul(pt->fX, scale);
+        pt->fY = SkScalarMul(pt->fY, scale);
+        return mag;
+    }
+    return 0;
+}
 
-    tmp1.setMul(dx, dx);
-    tmp2.setMul(dy, dy);
-    tmp1.add(tmp2);
+bool SkPoint::CanNormalize(SkScalar dx, SkScalar dy) {
+    Sk64 mag2_unused;
+    return !isLengthNearlyZero(dx, dy, &mag2_unused);
+}
 
-    return tmp1.getSqrt();
+SkScalar SkPoint::Length(SkScalar dx, SkScalar dy) {
+    Sk64    tmp;
+    getLengthSquared(dx, dy, &tmp);
+    return tmp.getSqrt();
 }
 
 #ifdef SK_DEBUGx
index 7cafda2..65a3494 100644 (file)
@@ -286,6 +286,29 @@ SIMPLE_TEST_STEP(DrawData, drawData(kTestText.c_str(), kTestText.size()));
 ///////////////////////////////////////////////////////////////////////////////
 // Complex test steps
 
+// exercise fix for http://code.google.com/p/skia/issues/detail?id=560
+// ('SkPathStroker::lineTo() fails for line with length SK_ScalarNearlyZero')
+static void DrawNearlyZeroLengthPathTestStep(SkCanvas* canvas, 
+                                             skiatest::Reporter* reporter,
+                                             CanvasTestStep* testStep) {
+    SkPaint paint;
+    paint.setStrokeWidth(SkIntToScalar(1));
+    paint.setStyle(SkPaint::kStroke_Style);
+
+    SkPath path;
+    SkPoint pt1 = { 0, 0 };
+    SkPoint pt2 = { 0, SK_ScalarNearlyZero };
+    SkPoint pt3 = { SkIntToScalar(1), 0 };
+    SkPoint pt4 = { SkIntToScalar(1), SK_ScalarNearlyZero/2 };
+    path.moveTo(pt1);
+    path.lineTo(pt2);
+    path.lineTo(pt3);
+    path.lineTo(pt4);
+
+    canvas->drawPath(path, paint);
+}
+TEST_STEP(DrawNearlyZeroLengthPath, DrawNearlyZeroLengthPathTestStep);
+
 static void DrawVerticesShaderTestStep(SkCanvas* canvas, 
                                        skiatest::Reporter* reporter,
                                        CanvasTestStep* testStep) {