Fix bug where SkPath will convert an arc to an oval and change the starting point.

tests/PathTest.cpp

 /*
  * Copyright 2011 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
+#include <cmath>
 #include "SkCanvas.h"
 #include "SkGeometry.h"
 #include "SkPaint.h"
 #include "SkParse.h"
 #include "SkParsePath.h"
 #include "SkPathPriv.h"
 #include "SkPathEffect.h"
 #include "SkRRect.h"
 #include "SkRandom.h"
 #include "SkReader32.h"
@@ skipping 3281 matching lines @@
     REPORTER_ASSERT(reporter, emptyOval.isEmpty());
     p.addArc(emptyOval, 1, 2);
     REPORTER_ASSERT(reporter, p.isEmpty());
     p.reset();
     SkRect oval = {10, 20, 30, 40};
     p.addArc(oval, 1, 0);
     REPORTER_ASSERT(reporter, p.isEmpty());
     p.reset();
     SkPath cwOval;
     cwOval.addOval(oval);
-    p.addArc(oval, 1, 360);
+    p.addArc(oval, 0, 360);
     REPORTER_ASSERT(reporter, p == cwOval);
     p.reset();
     SkPath ccwOval;
     ccwOval.addOval(oval, SkPath::kCCW_Direction);
-    p.addArc(oval, 1, -360);
+    p.addArc(oval, 0, -360);
     REPORTER_ASSERT(reporter, p == ccwOval);
     p.reset();
     p.addArc(oval, 1, 180);
     REPORTER_ASSERT(reporter, p.isConvex());
     REPORTER_ASSERT(reporter, SkPathPriv::CheapIsFirstDirection(p, SkPathPriv::kCW_FirstDirection));
     p.setConvexity(SkPath::kUnknown_Convexity);
     REPORTER_ASSERT(reporter, p.isConvex());
 }
 
+static inline SkScalar oval_start_index_to_angle(unsigned start) {
+    switch (start) {
+        case 0:
+            return 270.f;
+        case 1:
+            return 0.f;
+        case 2:
+            return 90.f;
+        case 3:
+            return 180.f;
+        default:
+            return -1.f;
+    }
+}
+
+static inline SkScalar canonical_start_angle(float angle) {
+    while (angle < 0.f) {
+        angle += 360.f;
+    }
+    while (angle >= 360.f) {
+        angle -= 360.f;
+    }
+    return angle;
+}
+
+static void test_arc_ovals(skiatest::Reporter* reporter) {
+    SkRect oval = SkRect::MakeWH(10, 20);
+    for (SkScalar sweep : {-720.f, -540.f, -360.f, 360.f, 432.f, 720.f}) {
+        for (SkScalar start = -360.f; start <= 360.f; start += 1.f) {
+            SkPath path;
+            path.addArc(oval, start, sweep);
+            SkRect r = SkRect::MakeEmpty();
+            SkPath::Direction d = SkPath::kCCW_Direction;
+            unsigned s = ~0U;
+            bool isOval = path.isOval(&r, &d, &s);
+            // SkPath's interfaces for inserting and extracting ovals only allow contours
+            // to start at multiples of 90 degrees.
+            if (std::fmod(start, 90.f) == 0) {
+                REPORTER_ASSERT(reporter, isOval);
+                SkPath recreatedPath;
+                recreatedPath.addOval(r, d, s);
+                REPORTER_ASSERT(reporter, path == recreatedPath);
+                REPORTER_ASSERT(reporter, oval_start_index_to_angle(s) ==
+                                          canonical_start_angle(start));
+                REPORTER_ASSERT(reporter, (SkPath::kCW_Direction == d) ==
+                                          (sweep > 0.f));
+            } else {
+                REPORTER_ASSERT(reporter, !isOval);
+            }
+        }
+        // Test start angles that are nearly at valid oval start angles.
+        for (float start : {-180.f, -90.f, 90.f, 180.f}) {
+            for (float delta : {-SK_ScalarNearlyZero, SK_ScalarNearlyZero}) {

[robertphillips, 2016/05/27 15:24:59]

Can we have a shared check_oval helper that takes and betterBeAnOval boolean?

[bsalomon, 2016/06/10 15:04:33]

Done.

+                SkPath path;
+                path.addArc(oval, start + delta, sweep);
+                SkRect r = SkRect::MakeEmpty();
+                SkPath::Direction d = SkPath::kCCW_Direction;
+                unsigned s = ~0U;
+                bool isOval = path.isOval(&r, &d, &s);
+                REPORTER_ASSERT(reporter, isOval);
+                SkPath recreatedPath;
+                recreatedPath.addOval(r, d, s);
+                REPORTER_ASSERT(reporter, path == recreatedPath);
+                REPORTER_ASSERT(reporter, oval_start_index_to_angle(s) ==
+                                          canonical_start_angle(start));
+                REPORTER_ASSERT(reporter, (SkPath::kCW_Direction == d) ==
+                                          (sweep > 0.f));
+            }
+        }
+    }
+}
+
 static void check_move(skiatest::Reporter* reporter, SkPath::RawIter* iter,
                        SkScalar x0, SkScalar y0) {
     SkPoint pts[4];
     SkPath::Verb v = iter->next(pts);
     REPORTER_ASSERT(reporter, v == SkPath::kMove_Verb);
     REPORTER_ASSERT(reporter, pts[0].fX == x0);
     REPORTER_ASSERT(reporter, pts[0].fY == y0);
 }
 
 static void check_line(skiatest::Reporter* reporter, SkPath::RawIter* iter,
@@ skipping 780 matching lines @@
     test_crbug_170666();
     test_crbug_493450(reporter);
     test_crbug_495894(reporter);
     test_bad_cubic_crbug229478();
     test_bad_cubic_crbug234190();
     test_gen_id(reporter);
     test_path_close_issue1474(reporter);
     test_path_to_region(reporter);
     test_rrect(reporter);
     test_arc(reporter);
+    test_arc_ovals(reporter);
     test_arcTo(reporter);
     test_addPath(reporter);
     test_addPathMode(reporter, false, false);
     test_addPathMode(reporter, true, false);
     test_addPathMode(reporter, false, true);
     test_addPathMode(reporter, true, true);
     test_extendClosedPath(reporter);
     test_addEmptyPath(reporter, SkPath::kExtend_AddPathMode);
     test_addEmptyPath(reporter, SkPath::kAppend_AddPathMode);
     test_conicTo_special_case(reporter);
     test_get_point(reporter);
     test_contains(reporter);
     PathTest_Private::TestPathTo(reporter);
     PathRefTest_Private::TestPathRef(reporter);
     PathTest_Private::TestPathrefListeners(reporter);
     test_dump(reporter);
     test_path_crbug389050(reporter);
     test_path_crbugskia2820(reporter);
     test_skbug_3469(reporter);
     test_skbug_3239(reporter);
     test_bounds_crbug_513799(reporter);
 }