From f0e8e721872989a375db59bb02c783740d9e448e Mon Sep 17 00:00:00 2001
From: Rod Sheeter <rsheeter@google.com>
Date: Thu, 7 Feb 2019 19:00:26 -0800
Subject: [PATCH 1/9] Test wiring for <rect> import

---
 Lib/fontTools/svgLib/path/shapes.py | 78 +++++++++++++++++++++++++++++
 Tests/svgLib/path/shapes_test.py    | 23 +++++++++
 2 files changed, 101 insertions(+)
 create mode 100644 Lib/fontTools/svgLib/path/shapes.py
 create mode 100644 Tests/svgLib/path/shapes_test.py

diff --git a/Lib/fontTools/svgLib/path/shapes.py b/Lib/fontTools/svgLib/path/shapes.py
new file mode 100644
index 000000000..041f59c56
--- /dev/null
+++ b/Lib/fontTools/svgLib/path/shapes.py
@@ -0,0 +1,78 @@
+def _PreferNonZero(*args):
+  for arg  in args:
+    if arg != 0:
+      return arg
+  return 0
+
+# TODO float movement
+class PathBuilder(object):
+  def __init__(self):
+    self.pathes = []
+
+  def StartPath(self):
+    self.pathes.append('')
+
+  def _Add(self, path_snippet):
+    path = self.pathes[-1]
+    if path:
+      path += ' ' + path_snippet
+    else:
+      path = path_snippet
+    self.pathes[-1] =  path
+
+  def _move(self, c, x, y):
+    self._Add('%s%d,%d' % (c, x, y))
+
+  def M(self, x, y):
+    self._move('M', x, y)
+
+  def m(self, x, y):
+    self._move('m', x, y)
+
+  def _arc(self, c, rx, ry, x, y):
+    self._Add('%s%d %d 0 0 1 %d %d' % (c, rx, ry, x, y))
+
+  def A(self, rx, ry, x, y):
+    self._arc('A', rx, ry, x, y)
+
+  def a(self, rx, ry, x, y):
+    self._arc('a', rx, ry, x, y)
+
+  def _vhline(self, c, x):
+    self._Add('%s%d' % (c, x))
+
+  def H(self, x):
+    self._vhline('H', x)
+
+  def h(self, x):
+    self._vhline('h', x)
+
+  def V(self, y):
+    self._vhline('V', y)
+
+  def v(self, x):
+    self._vhline('v', y)
+
+  def Rect(self, rect):
+    # TODO what format(s) do these #s come in?
+    x = float(rect.attrib.get('x', 0))
+    y = float(rect.attrib.get('x', 0))
+    w = float(rect.attrib.get('width'))
+    h = float(rect.attrib.get('height'))
+    rx = float(rect.attrib.get('rx', 0))
+    ry = float(rect.attrib.get('ry', 0))
+    rx = _PreferNonZero(rx, ry)
+    ry = _PreferNonZero(ry, rx)
+    # TODO there are more rules for adjusting rx, ry
+    self.StartPath()
+    self.M(x + rx, y)
+    self.H(x + w -rx)
+    if rx > 0:
+      self.A(rx, ry, x + w, y + ry)
+    self.V(x + h -ry)
+    if rx > 0:
+      self.A(rx, ry, x + w, y + ry)
+    self.H(x + rx)
+    if rx > 0:
+      self.A(rx, ry, x, y + h - ry)
+    self.V(y + ry)
diff --git a/Tests/svgLib/path/shapes_test.py b/Tests/svgLib/path/shapes_test.py
new file mode 100644
index 000000000..a9386152a
--- /dev/null
+++ b/Tests/svgLib/path/shapes_test.py
@@ -0,0 +1,23 @@
+from __future__ import print_function, absolute_import, division
+
+from fontTools.misc.py23 import *
+from fontTools.pens.recordingPen import RecordingPen
+from fontTools.svgLib.path import shapes
+from fontTools.misc import etree
+import pytest
+
+
+@pytest.mark.parametrize(
+    "rect, expected_path",
+    [
+        # minimal valid example
+        (
+            "<rect width='1' height='1'/>",
+            "M0,0 H1 V1 H0 V0",
+        )
+    ]
+)
+def test_rect_to_path(rect, expected_path):
+    pb = shapes.PathBuilder()
+    pb.Rect(etree.fromstring(rect))
+    assert pb.pathes == [expected_path]

From 1292029be289856946a8c38862b66f8ed2a9bb34 Mon Sep 17 00:00:00 2001
From: Rod Sheeter <rsheeter@google.com>
Date: Fri, 8 Feb 2019 10:50:22 -0800
Subject: [PATCH 2/9] test for basic rounded rect

---
 Lib/fontTools/svgLib/path/shapes.py | 14 ++++++++++----
 Tests/svgLib/path/shapes_test.py    | 15 +++++++++++++--
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/Lib/fontTools/svgLib/path/shapes.py b/Lib/fontTools/svgLib/path/shapes.py
index 041f59c56..ba77a0312 100644
--- a/Lib/fontTools/svgLib/path/shapes.py
+++ b/Lib/fontTools/svgLib/path/shapes.py
@@ -12,6 +12,9 @@ class PathBuilder(object):
   def StartPath(self):
     self.pathes.append('')
 
+  def EndPath(self):
+    self._Add('z')
+
   def _Add(self, path_snippet):
     path = self.pathes[-1]
     if path:
@@ -30,7 +33,7 @@ class PathBuilder(object):
     self._move('m', x, y)
 
   def _arc(self, c, rx, ry, x, y):
-    self._Add('%s%d %d 0 0 1 %d %d' % (c, rx, ry, x, y))
+    self._Add('%s%d,%d 0 0 1 %d,%d' % (c, rx, ry, x, y))
 
   def A(self, rx, ry, x, y):
     self._arc('A', rx, ry, x, y)
@@ -56,7 +59,7 @@ class PathBuilder(object):
   def Rect(self, rect):
     # TODO what format(s) do these #s come in?
     x = float(rect.attrib.get('x', 0))
-    y = float(rect.attrib.get('x', 0))
+    y = float(rect.attrib.get('y', 0))
     w = float(rect.attrib.get('width'))
     h = float(rect.attrib.get('height'))
     rx = float(rect.attrib.get('rx', 0))
@@ -69,10 +72,13 @@ class PathBuilder(object):
     self.H(x + w -rx)
     if rx > 0:
       self.A(rx, ry, x + w, y + ry)
-    self.V(x + h -ry)
+    self.V(y + h -ry)
     if rx > 0:
-      self.A(rx, ry, x + w, y + ry)
+      self.A(rx, ry, x + w - rx, y + h)
     self.H(x + rx)
     if rx > 0:
       self.A(rx, ry, x, y + h - ry)
     self.V(y + ry)
+    if rx > 0:
+      self.A(rx, ry, x + rx, y)
+    self.EndPath()
diff --git a/Tests/svgLib/path/shapes_test.py b/Tests/svgLib/path/shapes_test.py
index a9386152a..7611f58f0 100644
--- a/Tests/svgLib/path/shapes_test.py
+++ b/Tests/svgLib/path/shapes_test.py
@@ -13,11 +13,22 @@ import pytest
         # minimal valid example
         (
             "<rect width='1' height='1'/>",
-            "M0,0 H1 V1 H0 V0",
+            "M0,0 H1 V1 H0 V0 z",
+        ),
+        # sharp corners
+        (
+            "<rect x='10' y='11' width='17' height='11'/>",
+            "M10,11 H27 V22 H10 V11 z",
+        ),
+        # round corners
+        (
+            "<rect x='9' y='9' width='11' height='7' rx='2'/>",
+            "M11,9 H18 A2,2 0 0 1 20,11 V14 A2,2 0 0 1 18,16 H11"
+            " A2,2 0 0 1 9,14 V11 A2,2 0 0 1 11,9 z",
         )
     ]
 )
 def test_rect_to_path(rect, expected_path):
     pb = shapes.PathBuilder()
     pb.Rect(etree.fromstring(rect))
-    assert pb.pathes == [expected_path]
+    assert [expected_path] == pb.pathes

From d910ba371b5bc52c79648071da7972b4472e3e54 Mon Sep 17 00:00:00 2001
From: Rod Sheeter <rsheeter@google.com>
Date: Fri, 8 Feb 2019 11:37:00 -0800
Subject: [PATCH 3/9] Wire SVGPath to import other shapes

---
 Lib/fontTools/svgLib/path/__init__.py | 10 +++++++--
 Lib/fontTools/svgLib/path/shapes.py   | 18 +++++++++++++---
 Tests/svgLib/path/shapes_test.py      | 30 ++++++++++++++++++++-------
 3 files changed, 45 insertions(+), 13 deletions(-)

diff --git a/Lib/fontTools/svgLib/path/__init__.py b/Lib/fontTools/svgLib/path/__init__.py
index 690475f28..311329798 100644
--- a/Lib/fontTools/svgLib/path/__init__.py
+++ b/Lib/fontTools/svgLib/path/__init__.py
@@ -5,6 +5,7 @@ from fontTools.misc.py23 import *
 from fontTools.pens.transformPen import TransformPen
 from fontTools.misc import etree
 from .parser import parse_path
+from .shapes import PathBuilder
 
 
 __all__ = [tostr(s) for s in ("SVGPath", "parse_path")]
@@ -50,5 +51,10 @@ class SVGPath(object):
     def draw(self, pen):
         if self.transform:
             pen = TransformPen(pen, self.transform)
-        for el in self.root.findall(".//{http://www.w3.org/2000/svg}path[@d]"):
-            parse_path(el.get("d"), pen)
+        pb = PathBuilder()
+        # xpath | doesn't seem to reliable work so just walk it
+        for el in self.root.iter():
+            pb.AddPathFromElement(el)
+        for path in pb.pathes:
+            parse_path(path, pen)
+
diff --git a/Lib/fontTools/svgLib/path/shapes.py b/Lib/fontTools/svgLib/path/shapes.py
index ba77a0312..bb4f93acf 100644
--- a/Lib/fontTools/svgLib/path/shapes.py
+++ b/Lib/fontTools/svgLib/path/shapes.py
@@ -9,8 +9,8 @@ class PathBuilder(object):
   def __init__(self):
     self.pathes = []
 
-  def StartPath(self):
-    self.pathes.append('')
+  def StartPath(self, initial_path=''):
+    self.pathes.append(initial_path)
 
   def EndPath(self):
     self._Add('z')
@@ -56,7 +56,7 @@ class PathBuilder(object):
   def v(self, x):
     self._vhline('v', y)
 
-  def Rect(self, rect):
+  def _ParseRect(self, rect):
     # TODO what format(s) do these #s come in?
     x = float(rect.attrib.get('x', 0))
     y = float(rect.attrib.get('y', 0))
@@ -82,3 +82,15 @@ class PathBuilder(object):
     if rx > 0:
       self.A(rx, ry, x + rx, y)
     self.EndPath()
+
+  def _ParsePath(self, path):
+    if 'd' in path.attrib:
+      self.StartPath(initial_path=path.attrib['d'])
+
+  def AddPathFromElement(self, el):
+    tag = el.tag
+    if '}' in el.tag:
+      tag = el.tag.split('}', 1)[1]  # from https://bugs.python.org/issue18304
+    parse_fn = getattr(self, '_Parse%s' % tag.lower().capitalize(), None)
+    if callable(parse_fn):
+      parse_fn(el)
diff --git a/Tests/svgLib/path/shapes_test.py b/Tests/svgLib/path/shapes_test.py
index 7611f58f0..b1ae7e0bd 100644
--- a/Tests/svgLib/path/shapes_test.py
+++ b/Tests/svgLib/path/shapes_test.py
@@ -8,27 +8,41 @@ import pytest
 
 
 @pytest.mark.parametrize(
-    "rect, expected_path",
+    "svg_xml, expected_path",
     [
-        # minimal valid example
+        # path: direct passthrough
+        (
+          "<path d='I love kittens'/>",
+          "I love kittens"
+        ),
+        # path no @d
+        (
+          "<path duck='Mallard'/>",
+          None
+        ),
+        # rect: minimal valid example
         (
             "<rect width='1' height='1'/>",
             "M0,0 H1 V1 H0 V0 z",
         ),
-        # sharp corners
+        # rect: sharp corners
         (
             "<rect x='10' y='11' width='17' height='11'/>",
             "M10,11 H27 V22 H10 V11 z",
         ),
-        # round corners
+        # rect: round corners
         (
             "<rect x='9' y='9' width='11' height='7' rx='2'/>",
             "M11,9 H18 A2,2 0 0 1 20,11 V14 A2,2 0 0 1 18,16 H11"
             " A2,2 0 0 1 9,14 V11 A2,2 0 0 1 11,9 z",
-        )
+        ),
     ]
 )
-def test_rect_to_path(rect, expected_path):
+def test_el_to_path(svg_xml, expected_path):
     pb = shapes.PathBuilder()
-    pb.Rect(etree.fromstring(rect))
-    assert [expected_path] == pb.pathes
+    pb.AddPathFromElement(etree.fromstring(svg_xml))
+    if expected_path:
+      expected = [expected_path]
+    else:
+      expected = []
+    assert pb.pathes == expected

From 1e70458679eb2058d69e9c675755dbd3700ac765 Mon Sep 17 00:00:00 2001
From: Rod Sheeter <rsheeter@google.com>
Date: Fri, 8 Feb 2019 13:11:33 -0800
Subject: [PATCH 4/9] More simple shape examples

---
 Lib/fontTools/svgLib/path/shapes.py | 28 ++++++++++++++++++++++------
 Tests/svgLib/path/shapes_test.py    | 15 +++++++++++++++
 2 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/Lib/fontTools/svgLib/path/shapes.py b/Lib/fontTools/svgLib/path/shapes.py
index bb4f93acf..c58d19cee 100644
--- a/Lib/fontTools/svgLib/path/shapes.py
+++ b/Lib/fontTools/svgLib/path/shapes.py
@@ -32,14 +32,14 @@ class PathBuilder(object):
   def m(self, x, y):
     self._move('m', x, y)
 
-  def _arc(self, c, rx, ry, x, y):
-    self._Add('%s%d,%d 0 0 1 %d,%d' % (c, rx, ry, x, y))
+  def _arc(self, c, rx, ry, x, y, large_arc):
+    self._Add('%s%d,%d 0 %d 1 %d,%d' % (c, rx, ry, large_arc, x, y))
 
-  def A(self, rx, ry, x, y):
-    self._arc('A', rx, ry, x, y)
+  def A(self, rx, ry, x, y, large_arc = 0):
+    self._arc('A', rx, ry, x, y, large_arc)
 
-  def a(self, rx, ry, x, y):
-    self._arc('a', rx, ry, x, y)
+  def a(self, rx, ry, x, y, large_arc = 0):
+    self._arc('a', rx, ry, x, y, large_arc)
 
   def _vhline(self, c, x):
     self._Add('%s%d' % (c, x))
@@ -87,6 +87,22 @@ class PathBuilder(object):
     if 'd' in path.attrib:
       self.StartPath(initial_path=path.attrib['d'])
 
+  def _ParsePolygon(self, poly):
+    if 'points' in poly.attrib:
+      self.StartPath('M' + poly.attrib['points'])
+      self.EndPath()
+
+  def _ParseCircle(self, circle):
+    cx = float(circle.attrib.get('cx', 0))
+    cy = float(circle.attrib.get('cy', 0))
+    r = float(circle.attrib.get('r'))
+
+    # arc doesn't seem to like being a complete shape, draw two halves
+    self.StartPath()
+    self.M(cx - r, cy)
+    self.A(r, r, cx + r, cy, large_arc=1)
+    self.A(r, r, cx - r, cy, large_arc=1)
+
   def AddPathFromElement(self, el):
     tag = el.tag
     if '}' in el.tag:
diff --git a/Tests/svgLib/path/shapes_test.py b/Tests/svgLib/path/shapes_test.py
index b1ae7e0bd..27c7d5a8b 100644
--- a/Tests/svgLib/path/shapes_test.py
+++ b/Tests/svgLib/path/shapes_test.py
@@ -36,6 +36,21 @@ import pytest
             "M11,9 H18 A2,2 0 0 1 20,11 V14 A2,2 0 0 1 18,16 H11"
             " A2,2 0 0 1 9,14 V11 A2,2 0 0 1 11,9 z",
         ),
+        # polygon
+        (
+            "<polygon points='30,10 50,30 10,30'/>",
+            "M30,10 50,30 10,30 z"
+        ),
+        # circle, minimal valid example
+        (
+            "<circle r='1'/>",
+            "M-1,0 A1,1 0 1 1 1,0 A1,1 0 1 1 -1,0"
+        ),
+        # circle
+        (
+            "<circle cx='600' cy='200' r='100'/>",
+            "M500,200 A100,100 0 1 1 700,200 A100,100 0 1 1 500,200"
+        )
     ]
 )
 def test_el_to_path(svg_xml, expected_path):

From 199aa9e24b32dd4fd6003697bcd505a3db53f15b Mon Sep 17 00:00:00 2001
From: Rod Sheeter <rsheeter@google.com>
Date: Fri, 8 Feb 2019 14:57:10 -0800
Subject: [PATCH 5/9] Rudimentary decimal support

---
 Lib/fontTools/svgLib/path/shapes.py | 24 ++++++++++++++++--------
 Tests/svgLib/path/shapes_test.py    |  5 +++++
 2 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/Lib/fontTools/svgLib/path/shapes.py b/Lib/fontTools/svgLib/path/shapes.py
index c58d19cee..e9dbf0f01 100644
--- a/Lib/fontTools/svgLib/path/shapes.py
+++ b/Lib/fontTools/svgLib/path/shapes.py
@@ -2,9 +2,12 @@ def _PreferNonZero(*args):
   for arg  in args:
     if arg != 0:
       return arg
-  return 0
+  return 0.
+
+def _ntos(n):
+  # %f likes to add unnecessary 0's, %g isn't consistent about # decimals
+  return ('%.3f' % n).rstrip('0').rstrip('.')
 
-# TODO float movement
 class PathBuilder(object):
   def __init__(self):
     self.pathes = []
@@ -24,7 +27,7 @@ class PathBuilder(object):
     self.pathes[-1] =  path
 
   def _move(self, c, x, y):
-    self._Add('%s%d,%d' % (c, x, y))
+    self._Add('%s%s,%s' % (c, _ntos(x), _ntos(y)))
 
   def M(self, x, y):
     self._move('M', x, y)
@@ -33,7 +36,8 @@ class PathBuilder(object):
     self._move('m', x, y)
 
   def _arc(self, c, rx, ry, x, y, large_arc):
-    self._Add('%s%d,%d 0 %d 1 %d,%d' % (c, rx, ry, large_arc, x, y))
+    self._Add('%s%s,%s 0 %d 1 %s,%s' % (c, _ntos(rx), _ntos(ry), large_arc,
+                                        _ntos(x), _ntos(y)))
 
   def A(self, rx, ry, x, y, large_arc = 0):
     self._arc('A', rx, ry, x, y, large_arc)
@@ -42,7 +46,7 @@ class PathBuilder(object):
     self._arc('a', rx, ry, x, y, large_arc)
 
   def _vhline(self, c, x):
-    self._Add('%s%d' % (c, x))
+    self._Add('%s%s' % (c, _ntos(x)))
 
   def H(self, x):
     self._vhline('H', x)
@@ -57,16 +61,17 @@ class PathBuilder(object):
     self._vhline('v', y)
 
   def _ParseRect(self, rect):
-    # TODO what format(s) do these #s come in?
     x = float(rect.attrib.get('x', 0))
     y = float(rect.attrib.get('y', 0))
     w = float(rect.attrib.get('width'))
     h = float(rect.attrib.get('height'))
     rx = float(rect.attrib.get('rx', 0))
     ry = float(rect.attrib.get('ry', 0))
+
     rx = _PreferNonZero(rx, ry)
     ry = _PreferNonZero(ry, rx)
     # TODO there are more rules for adjusting rx, ry
+
     self.StartPath()
     self.M(x + rx, y)
     self.H(x + w -rx)
@@ -108,5 +113,8 @@ class PathBuilder(object):
     if '}' in el.tag:
       tag = el.tag.split('}', 1)[1]  # from https://bugs.python.org/issue18304
     parse_fn = getattr(self, '_Parse%s' % tag.lower().capitalize(), None)
-    if callable(parse_fn):
-      parse_fn(el)
+    if not callable(parse_fn):
+      return False
+    parse_fn(el)
+    return True
+
diff --git a/Tests/svgLib/path/shapes_test.py b/Tests/svgLib/path/shapes_test.py
index 27c7d5a8b..14dcc9c71 100644
--- a/Tests/svgLib/path/shapes_test.py
+++ b/Tests/svgLib/path/shapes_test.py
@@ -50,6 +50,11 @@ import pytest
         (
             "<circle cx='600' cy='200' r='100'/>",
             "M500,200 A100,100 0 1 1 700,200 A100,100 0 1 1 500,200"
+        ),
+        # circle, decimal positioning
+        (
+            "<circle cx='12' cy='6.5' r='1.5'></circle>",
+            "M10.5,6.5 A1.5,1.5 0 1 1 13.5,6.5 A1.5,1.5 0 1 1 10.5,6.5"
         )
     ]
 )

From 416da67fdd8222fa75c65442863d987d8b6bb2de Mon Sep 17 00:00:00 2001
From: Rod Sheeter <rsheeter@google.com>
Date: Tue, 12 Feb 2019 11:53:27 -0800
Subject: [PATCH 6/9] Try to follow FT coding style

---
 Lib/fontTools/svgLib/path/__init__.py |   2 +-
 Lib/fontTools/svgLib/path/shapes.py   | 184 +++++++++++++-------------
 Tests/svgLib/path/shapes_test.py      |   6 +-
 3 files changed, 96 insertions(+), 96 deletions(-)

diff --git a/Lib/fontTools/svgLib/path/__init__.py b/Lib/fontTools/svgLib/path/__init__.py
index 311329798..187381f69 100644
--- a/Lib/fontTools/svgLib/path/__init__.py
+++ b/Lib/fontTools/svgLib/path/__init__.py
@@ -54,7 +54,7 @@ class SVGPath(object):
         pb = PathBuilder()
         # xpath | doesn't seem to reliable work so just walk it
         for el in self.root.iter():
-            pb.AddPathFromElement(el)
+            pb.add_path_from_element(el)
         for path in pb.pathes:
             parse_path(path, pen)
 
diff --git a/Lib/fontTools/svgLib/path/shapes.py b/Lib/fontTools/svgLib/path/shapes.py
index e9dbf0f01..04e9ef9f7 100644
--- a/Lib/fontTools/svgLib/path/shapes.py
+++ b/Lib/fontTools/svgLib/path/shapes.py
@@ -1,120 +1,120 @@
-def _PreferNonZero(*args):
-  for arg  in args:
-    if arg != 0:
-      return arg
-  return 0.
+def _prefer_non_zero(*args):
+    for arg  in args:
+      if arg != 0:
+        return arg
+    return 0.
 
 def _ntos(n):
   # %f likes to add unnecessary 0's, %g isn't consistent about # decimals
-  return ('%.3f' % n).rstrip('0').rstrip('.')
+    return ('%.3f' % n).rstrip('0').rstrip('.')
 
 class PathBuilder(object):
-  def __init__(self):
-    self.pathes = []
+    def __init__(self):
+        self.pathes = []
 
-  def StartPath(self, initial_path=''):
-    self.pathes.append(initial_path)
+    def _start_path(self, initial_path=''):
+        self.pathes.append(initial_path)
 
-  def EndPath(self):
-    self._Add('z')
+    def _end_path(self):
+        self._add('z')
 
-  def _Add(self, path_snippet):
-    path = self.pathes[-1]
-    if path:
-      path += ' ' + path_snippet
-    else:
-      path = path_snippet
-    self.pathes[-1] =  path
+    def _add(self, path_snippet):
+        path = self.pathes[-1]
+        if path:
+            path += ' ' + path_snippet
+        else:
+            path = path_snippet
+        self.pathes[-1] =  path
 
-  def _move(self, c, x, y):
-    self._Add('%s%s,%s' % (c, _ntos(x), _ntos(y)))
+    def _move(self, c, x, y):
+        self._add('%s%s,%s' % (c, _ntos(x), _ntos(y)))
 
-  def M(self, x, y):
-    self._move('M', x, y)
+    def M(self, x, y):
+        self._move('M', x, y)
 
-  def m(self, x, y):
-    self._move('m', x, y)
+    def m(self, x, y):
+        self._move('m', x, y)
 
-  def _arc(self, c, rx, ry, x, y, large_arc):
-    self._Add('%s%s,%s 0 %d 1 %s,%s' % (c, _ntos(rx), _ntos(ry), large_arc,
-                                        _ntos(x), _ntos(y)))
+    def _arc(self, c, rx, ry, x, y, large_arc):
+        self._add('%s%s,%s 0 %d 1 %s,%s' % (c, _ntos(rx), _ntos(ry), large_arc,
+                                            _ntos(x), _ntos(y)))
 
-  def A(self, rx, ry, x, y, large_arc = 0):
-    self._arc('A', rx, ry, x, y, large_arc)
+    def A(self, rx, ry, x, y, large_arc = 0):
+        self._arc('A', rx, ry, x, y, large_arc)
 
-  def a(self, rx, ry, x, y, large_arc = 0):
-    self._arc('a', rx, ry, x, y, large_arc)
+    def a(self, rx, ry, x, y, large_arc = 0):
+        self._arc('a', rx, ry, x, y, large_arc)
 
-  def _vhline(self, c, x):
-    self._Add('%s%s' % (c, _ntos(x)))
+    def _vhline(self, c, x):
+        self._add('%s%s' % (c, _ntos(x)))
 
-  def H(self, x):
-    self._vhline('H', x)
+    def H(self, x):
+        self._vhline('H', x)
 
-  def h(self, x):
-    self._vhline('h', x)
+    def h(self, x):
+        self._vhline('h', x)
 
-  def V(self, y):
-    self._vhline('V', y)
+    def V(self, y):
+        self._vhline('V', y)
 
-  def v(self, x):
-    self._vhline('v', y)
+    def v(self, x):
+        self._vhline('v', y)
 
-  def _ParseRect(self, rect):
-    x = float(rect.attrib.get('x', 0))
-    y = float(rect.attrib.get('y', 0))
-    w = float(rect.attrib.get('width'))
-    h = float(rect.attrib.get('height'))
-    rx = float(rect.attrib.get('rx', 0))
-    ry = float(rect.attrib.get('ry', 0))
+    def _parse_rect(self, rect):
+        x = float(rect.attrib.get('x', 0))
+        y = float(rect.attrib.get('y', 0))
+        w = float(rect.attrib.get('width'))
+        h = float(rect.attrib.get('height'))
+        rx = float(rect.attrib.get('rx', 0))
+        ry = float(rect.attrib.get('ry', 0))
 
-    rx = _PreferNonZero(rx, ry)
-    ry = _PreferNonZero(ry, rx)
-    # TODO there are more rules for adjusting rx, ry
+        rx = _prefer_non_zero(rx, ry)
+        ry = _prefer_non_zero(ry, rx)
+        # TODO there are more rules for adjusting rx, ry
 
-    self.StartPath()
-    self.M(x + rx, y)
-    self.H(x + w -rx)
-    if rx > 0:
-      self.A(rx, ry, x + w, y + ry)
-    self.V(y + h -ry)
-    if rx > 0:
-      self.A(rx, ry, x + w - rx, y + h)
-    self.H(x + rx)
-    if rx > 0:
-      self.A(rx, ry, x, y + h - ry)
-    self.V(y + ry)
-    if rx > 0:
-      self.A(rx, ry, x + rx, y)
-    self.EndPath()
+        self._start_path()
+        self.M(x + rx, y)
+        self.H(x + w -rx)
+        if rx > 0:
+            self.A(rx, ry, x + w, y + ry)
+        self.V(y + h -ry)
+        if rx > 0:
+            self.A(rx, ry, x + w - rx, y + h)
+        self.H(x + rx)
+        if rx > 0:
+            self.A(rx, ry, x, y + h - ry)
+        self.V(y + ry)
+        if rx > 0:
+            self.A(rx, ry, x + rx, y)
+        self._end_path()
 
-  def _ParsePath(self, path):
-    if 'd' in path.attrib:
-      self.StartPath(initial_path=path.attrib['d'])
+    def _parse_path(self, path):
+        if 'd' in path.attrib:
+            self._start_path(initial_path=path.attrib['d'])
 
-  def _ParsePolygon(self, poly):
-    if 'points' in poly.attrib:
-      self.StartPath('M' + poly.attrib['points'])
-      self.EndPath()
+    def _parse_polygon(self, poly):
+        if 'points' in poly.attrib:
+            self._start_path('M' + poly.attrib['points'])
+            self._end_path()
 
-  def _ParseCircle(self, circle):
-    cx = float(circle.attrib.get('cx', 0))
-    cy = float(circle.attrib.get('cy', 0))
-    r = float(circle.attrib.get('r'))
+    def _parse_circle(self, circle):
+        cx = float(circle.attrib.get('cx', 0))
+        cy = float(circle.attrib.get('cy', 0))
+        r = float(circle.attrib.get('r'))
 
-    # arc doesn't seem to like being a complete shape, draw two halves
-    self.StartPath()
-    self.M(cx - r, cy)
-    self.A(r, r, cx + r, cy, large_arc=1)
-    self.A(r, r, cx - r, cy, large_arc=1)
+        # arc doesn't seem to like being a complete shape, draw two halves
+        self._start_path()
+        self.M(cx - r, cy)
+        self.A(r, r, cx + r, cy, large_arc=1)
+        self.A(r, r, cx - r, cy, large_arc=1)
 
-  def AddPathFromElement(self, el):
-    tag = el.tag
-    if '}' in el.tag:
-      tag = el.tag.split('}', 1)[1]  # from https://bugs.python.org/issue18304
-    parse_fn = getattr(self, '_Parse%s' % tag.lower().capitalize(), None)
-    if not callable(parse_fn):
-      return False
-    parse_fn(el)
-    return True
+    def add_path_from_element(self, el):
+        tag = el.tag
+        if '}' in el.tag:
+            tag = el.tag.split('}', 1)[1]  # from https://bugs.python.org/issue18304
+        parse_fn = getattr(self, '_parse_%s' % tag.lower(), None)
+        if not callable(parse_fn):
+            return False
+        parse_fn(el)
+        return True
 
diff --git a/Tests/svgLib/path/shapes_test.py b/Tests/svgLib/path/shapes_test.py
index 14dcc9c71..e56c1ac51 100644
--- a/Tests/svgLib/path/shapes_test.py
+++ b/Tests/svgLib/path/shapes_test.py
@@ -60,9 +60,9 @@ import pytest
 )
 def test_el_to_path(svg_xml, expected_path):
     pb = shapes.PathBuilder()
-    pb.AddPathFromElement(etree.fromstring(svg_xml))
+    pb.add_path_from_element(etree.fromstring(svg_xml))
     if expected_path:
-      expected = [expected_path]
+        expected = [expected_path]
     else:
-      expected = []
+        expected = []
     assert pb.pathes == expected

From a4ed057dd1419f304a392c50411232f9d7839020 Mon Sep 17 00:00:00 2001
From: Cosimo Lupo <cosimo@anthrotype.com>
Date: Thu, 14 Feb 2019 17:16:26 +0000
Subject: [PATCH 7/9] minor whitespace

---
 Lib/fontTools/svgLib/path/shapes.py | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/Lib/fontTools/svgLib/path/shapes.py b/Lib/fontTools/svgLib/path/shapes.py
index 04e9ef9f7..1b6174aea 100644
--- a/Lib/fontTools/svgLib/path/shapes.py
+++ b/Lib/fontTools/svgLib/path/shapes.py
@@ -1,13 +1,15 @@
 def _prefer_non_zero(*args):
-    for arg  in args:
-      if arg != 0:
-        return arg
+    for arg in args:
+        if arg != 0:
+            return arg
     return 0.
 
+
 def _ntos(n):
-  # %f likes to add unnecessary 0's, %g isn't consistent about # decimals
+    # %f likes to add unnecessary 0's, %g isn't consistent about # decimals
     return ('%.3f' % n).rstrip('0').rstrip('.')
 
+
 class PathBuilder(object):
     def __init__(self):
         self.pathes = []
@@ -24,7 +26,7 @@ class PathBuilder(object):
             path += ' ' + path_snippet
         else:
             path = path_snippet
-        self.pathes[-1] =  path
+        self.pathes[-1] = path
 
     def _move(self, c, x, y):
         self._add('%s%s,%s' % (c, _ntos(x), _ntos(y)))
@@ -39,10 +41,10 @@ class PathBuilder(object):
         self._add('%s%s,%s 0 %d 1 %s,%s' % (c, _ntos(rx), _ntos(ry), large_arc,
                                             _ntos(x), _ntos(y)))
 
-    def A(self, rx, ry, x, y, large_arc = 0):
+    def A(self, rx, ry, x, y, large_arc=0):
         self._arc('A', rx, ry, x, y, large_arc)
 
-    def a(self, rx, ry, x, y, large_arc = 0):
+    def a(self, rx, ry, x, y, large_arc=0):
         self._arc('a', rx, ry, x, y, large_arc)
 
     def _vhline(self, c, x):
@@ -57,7 +59,7 @@ class PathBuilder(object):
     def V(self, y):
         self._vhline('V', y)
 
-    def v(self, x):
+    def v(self, y):
         self._vhline('v', y)
 
     def _parse_rect(self, rect):
@@ -74,10 +76,10 @@ class PathBuilder(object):
 
         self._start_path()
         self.M(x + rx, y)
-        self.H(x + w -rx)
+        self.H(x + w - rx)
         if rx > 0:
             self.A(rx, ry, x + w, y + ry)
-        self.V(y + h -ry)
+        self.V(y + h - ry)
         if rx > 0:
             self.A(rx, ry, x + w - rx, y + h)
         self.H(x + rx)
@@ -117,4 +119,3 @@ class PathBuilder(object):
             return False
         parse_fn(el)
         return True
-

From dda4c1a41eb85717b286789305545ae2da31d17f Mon Sep 17 00:00:00 2001
From: Cosimo Lupo <cosimo@anthrotype.com>
Date: Thu, 14 Feb 2019 17:18:33 +0000
Subject: [PATCH 8/9] svgLib: rename PathBuilder's 'pathes' attribute to
 'paths'

---
 Lib/fontTools/svgLib/path/__init__.py | 2 +-
 Lib/fontTools/svgLib/path/shapes.py   | 8 ++++----
 Tests/svgLib/path/shapes_test.py      | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/Lib/fontTools/svgLib/path/__init__.py b/Lib/fontTools/svgLib/path/__init__.py
index 187381f69..017ff57e6 100644
--- a/Lib/fontTools/svgLib/path/__init__.py
+++ b/Lib/fontTools/svgLib/path/__init__.py
@@ -55,6 +55,6 @@ class SVGPath(object):
         # xpath | doesn't seem to reliable work so just walk it
         for el in self.root.iter():
             pb.add_path_from_element(el)
-        for path in pb.pathes:
+        for path in pb.paths:
             parse_path(path, pen)
 
diff --git a/Lib/fontTools/svgLib/path/shapes.py b/Lib/fontTools/svgLib/path/shapes.py
index 1b6174aea..05255f459 100644
--- a/Lib/fontTools/svgLib/path/shapes.py
+++ b/Lib/fontTools/svgLib/path/shapes.py
@@ -12,21 +12,21 @@ def _ntos(n):
 
 class PathBuilder(object):
     def __init__(self):
-        self.pathes = []
+        self.paths = []
 
     def _start_path(self, initial_path=''):
-        self.pathes.append(initial_path)
+        self.paths.append(initial_path)
 
     def _end_path(self):
         self._add('z')
 
     def _add(self, path_snippet):
-        path = self.pathes[-1]
+        path = self.paths[-1]
         if path:
             path += ' ' + path_snippet
         else:
             path = path_snippet
-        self.pathes[-1] = path
+        self.paths[-1] = path
 
     def _move(self, c, x, y):
         self._add('%s%s,%s' % (c, _ntos(x), _ntos(y)))
diff --git a/Tests/svgLib/path/shapes_test.py b/Tests/svgLib/path/shapes_test.py
index e56c1ac51..ee9ddeae6 100644
--- a/Tests/svgLib/path/shapes_test.py
+++ b/Tests/svgLib/path/shapes_test.py
@@ -65,4 +65,4 @@ def test_el_to_path(svg_xml, expected_path):
         expected = [expected_path]
     else:
         expected = []
-    assert pb.pathes == expected
+    assert pb.paths == expected

From 7a25b3a4e133c660a6a9f9a4f5cf3101209ec6de Mon Sep 17 00:00:00 2001
From: Cosimo Lupo <cosimo@anthrotype.com>
Date: Thu, 14 Feb 2019 17:43:44 +0000
Subject: [PATCH 9/9] factor out _strip_xml_ns into its own function

---
 Lib/fontTools/svgLib/path/shapes.py | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/Lib/fontTools/svgLib/path/shapes.py b/Lib/fontTools/svgLib/path/shapes.py
index 05255f459..a83274e4b 100644
--- a/Lib/fontTools/svgLib/path/shapes.py
+++ b/Lib/fontTools/svgLib/path/shapes.py
@@ -10,6 +10,12 @@ def _ntos(n):
     return ('%.3f' % n).rstrip('0').rstrip('.')
 
 
+def _strip_xml_ns(tag):
+    # ElementTree API doesn't provide a way to ignore XML namespaces in tags
+    # so we here strip them ourselves: cf. https://bugs.python.org/issue18304
+    return tag.split('}', 1)[1] if '}' in tag else tag
+
+
 class PathBuilder(object):
     def __init__(self):
         self.paths = []
@@ -111,9 +117,7 @@ class PathBuilder(object):
         self.A(r, r, cx - r, cy, large_arc=1)
 
     def add_path_from_element(self, el):
-        tag = el.tag
-        if '}' in el.tag:
-            tag = el.tag.split('}', 1)[1]  # from https://bugs.python.org/issue18304
+        tag = _strip_xml_ns(el.tag)
         parse_fn = getattr(self, '_parse_%s' % tag.lower(), None)
         if not callable(parse_fn):
             return False