Remove duplicate point, add more tests

Lib/fontTools/svgLib/path/parser.py

@@ -127,9 +127,6 @@ def parse_path(pathdef, pen, current_pos=(0, 0)):
 
     If 'current_pos' (2-float tuple) is provided, the initial moveTo will
     be relative to that instead being absolute.
-
-    Arc segments (commands "A" or "a") are not currently supported, and raise
-    NotImplementedError.
     """
     # In the SVG specs, initial movetos are absolute, even if
     # specified as 'm'. This is the default behavior here as well.
@@ -310,21 +307,19 @@ def parse_path(pathdef, pen, current_pos=(0, 0)):
             end = float(elements.pop()) + float(elements.pop()) * 1j
 
             if not absolute:
-                if end == 0:
-                    # Guard against a situation where arc start and end being same.
-                    # That results division by zero issues in Arc parameterization.
-                    end = 0.00009
                 end += current_pos
 
+            if end == current_pos:
+                # Guard against a situation where arc start and end being same.
+                # That results division by zero issues in Arc parameterization.
+                end += 0.00009
             svg_arc = Arc(current_pos, radius, rotation, arc, sweep, end)
-            arc_points = [(current_pos.real, current_pos.imag)]
+            arc_points = []
-            for x in range(1, 5):
+            for point in [0.2, 0.4, 0.6, 0.8, 1]:
                 # There are infinite points in an arc, but for our context,
-                # define the arc using 5 points(0.2, 0.4, 0.6...)
+                # define the arc using 5 points.
-                arc_point = svg_arc.point(x*0.2)
+                arc_point = svg_arc.point(point)
                 arc_points.append((arc_point.real, arc_point.imag))
-            arc_points.append((end.real, end.imag))
-
             pen.qCurveTo(*arc_points)
             current_pos = end
 

Tests/svgLib/path/parser_test.py

@@ -226,17 +226,57 @@ import pytest
                 ("closePath", ()),
             ]
         ),
-        # absolute A command
+        # absolute A command, arc 1
         (
             "M 100 100 A 150 150 0 1 0 150 -150 z",
             [
                 ('moveTo', ((100.0, 100.0),)),
-                ('qCurveTo', ((100.0, 100.0),
+                ('qCurveTo', ((217.17583, 139.78681),
-                    (217.17583, 139.78681),
+                              (324.37829, 77.97418),
-                    (324.37829, 77.97418),
+                              (348.64695, -43.36913),
-                    (348.64695, -43.36913),
+                              (273.46493, -141.65865),
-                    (273.46493, -141.65865),
+                              (150.0, -150.0))),
-                    (150.0, -150.0))),
+                ('lineTo', ((100.0, 100.0),)),
+                ('closePath', ()),
+            ]
+        ),
+        # relative A command
+        (
+            "M 100 100 a 150 150 0 1 0 150 -150",
+            [
+                ('moveTo', ((100.0, 100.0),)),
+                ('qCurveTo', ((161.832212, 221.352549),
+                              (296.3525491, 242.6584774),
+                              (392.6584774, 146.35254915),
+                              (371.3525491, 11.83221215),
+                              (250.0, -50.0))),
+                ('endPath', ())
+            ]
+        ),
+        # absolute A command, arc 1, sweap 1, rotation 30
+        (
+            "M 100 100 A 150 150 30 1 1 150 -150 z",
+            [
+                ('moveTo', ((100.0, 100.0),)),
+                ('qCurveTo', ((-23.46493, 91.65865),
+                              (-98.6469560, -6.63086811),
+                              (-74.3782932, -127.97418174),
+                              (32.8241612, -189.786813),
+                              (150.0, -150.0))),
+                ('lineTo', ((100.0, 100.0),)),
+                ('closePath', ()),
+            ]
+        ),
+        # absolute A command, arc 1, sweap 1, rotation 30, end == start
+        (
+            "M 100 100 A 150 150 30 1 1 100 100 z",
+            [
+                ('moveTo', ((100.0, 100.0),)),
+                ('qCurveTo', ((-42.6584408, -3.64747653),
+                              (11.832264448, -171.3525544),
+                              (188.16782558, -171.352554),
+                              (242.65853078, -3.647476),
+                              (100.0, 100.0))),
                 ('lineTo', ((100.0, 100.0),)),
                 ('closePath', ()),
             ]
@@ -249,11 +289,11 @@ def test_parse_path(pathdef, expected):
     parse_path(pathdef, pen)
 
     assert len(pen.value) == len(expected)
-    for i, (instr, coords) in enumerate(pen.value):
+    for (instr, coords), (exp_instr, exp_coords) in zip(pen.value, expected):
-        # instr are 'moveTo', 'qCurveTo', 'closePath' etc.
+        assert instr == exp_instr
-        assert instr == expected[i][0]
+        assert len(coords) == len(exp_coords)
-        for j, (real, imag) in enumerate(coords):
+        for c, e in zip(coords, exp_coords):
-            assert pytest.approx((real, imag)) == expected[i][1][j]
+            assert c == pytest.approx(e)
 
 @pytest.mark.parametrize(
     "pathdef1, pathdef2",