Merge pull request #237 from googlefonts/clip_bug

Fix bug with clippath attrs

src/picosvg/svg_meta.py

@@ -192,3 +192,34 @@ def __new__(cls, attr_name):
 
     def __call__(self, data_obj):
         return getattr(data_obj, self.attr_name)
+
+
+_ATTRIB_DEFAULTS = {
+    "clip-path": "",
+    "clip-rule": "nonzero",
+    "fill": "black",
+    "fill-opacity": 1.0,
+    "fill-rule": "nonzero",
+    "stroke": "none",
+    "stroke-width": 1.0,
+    "stroke-linecap": "butt",
+    "stroke-linejoin": "miter",
+    "stroke-miterlimit": 4,
+    "stroke-dasharray": "none",
+    "stroke-dashoffset": 0.0,
+    "stroke-opacity": 1.0,
+    "opacity": 1.0,
+    "transform": "",
+    "style": "",
+    "display": "inline",
+    "d": "",
+    "id": "",
+}
+
+
+def attrib_default(name: str, default: Any = ()) -> Any:
+    if name in _ATTRIB_DEFAULTS:
+        return _ATTRIB_DEFAULTS[name]
+    if default == ():
+        raise ValueError(f"No entry for '{name}' and no default given")
+    return default

src/picosvg/svg_transform.py

@@ -32,6 +32,7 @@
 )
 from picosvg.svg_meta import ntos
 
+DECOMPOSITION_ALMOST_EQUAL_TOLERANCE = 1e-6
 
 _SVG_ARG_FIXUPS = collections.defaultdict(
     lambda: lambda _: None,
@@ -110,19 +111,6 @@ def __matmul__(self, other: "Affine2D") -> "Affine2D":
 
     __imatmul__ = __matmul__
 
-    @staticmethod
-    def product(first: "Affine2D", second: "Affine2D") -> "Affine2D":
-        """Returns the product of second x first. DEPRECATED - use '@' infix operator"""
-        import warnings
-
-        warnings.warn(
-            "'product' is deprecated; use '@' infix operator",
-            DeprecationWarning,
-            stacklevel=2,
-        )
-
-        return second @ first
-
     def matrix(self, a, b, c, d, e, f):
         return self @ Affine2D(a, b, c, d, e, f)
 
@@ -290,6 +278,10 @@ def decompose_scale(self) -> Tuple["Affine2D", "Affine2D"]:
         sy = hypot(self.c, self.d)
         scale = Affine2D(sx, 0, 0, sy, 0, 0)
         remaining = Affine2D.compose_ltr((scale.inverse(), self))
+        test_compose = Affine2D.compose_ltr((scale, remaining))
+        assert self.almost_equals(
+            test_compose, DECOMPOSITION_ALMOST_EQUAL_TOLERANCE
+        ), f"Failed to extract scale from {self}, parts compose back to {test_compose}"
         return scale, remaining
 
     def decompose_translation(self) -> Tuple["Affine2D", "Affine2D"]:
@@ -341,7 +333,10 @@ def decompose_translation(self) -> Tuple["Affine2D", "Affine2D"]:
         # (with the translation zeroed) it'll land me in the same place?"
         translation = Affine2D.identity().translate(x_prime, y_prime)
         # sanity check that combining the two affines gives back self
-        assert self.almost_equals(Affine2D.compose_ltr((translation, affine_prime)))
+        test_compose = Affine2D.compose_ltr((translation, affine_prime))
+        assert self.almost_equals(
+            test_compose, DECOMPOSITION_ALMOST_EQUAL_TOLERANCE
+        ), f"Failed to extract translation from {self}, parts compose back to {test_compose}"
         return translation, affine_prime
 
 

src/picosvg/svg_types.py

@@ -16,12 +16,15 @@
 import dataclasses
 from itertools import zip_longest
 import math
+import numbers
 import re
 from picosvg.geometric_types import Point, Rect
 from picosvg.svg_meta import (
+    attrib_default,
     check_cmd,
     cmd_coords,
     number_or_percentage,
+    ntos,
     parse_css_declarations,
     path_segment,
     strip_ns,
@@ -33,7 +36,7 @@
 from picosvg.arc_to_cubic import arc_to_cubic
 from picosvg.svg_path_iter import parse_svg_path
 from picosvg.svg_transform import Affine2D
-from typing import Generator, Iterable, Mapping, MutableMapping, Tuple
+from typing import ClassVar, Generator, Iterable, Mapping, MutableMapping, Tuple
 
 
 def _round_multiple(f: float, of: float) -> float:
@@ -127,24 +130,25 @@ def _move_endpoint(curr_pos, cmd, cmd_args, new_endpoint):
 # Subset of https://www.w3.org/TR/SVG11/painting.html
 @dataclasses.dataclass
 class SVGShape:
+    tag: ClassVar[str] = "unknown"
     id: str = ""
-    clip_path: str = ""
-    clip_rule: str = "nonzero"
-    fill: str = "black"
-    fill_opacity: float = 1.0
-    fill_rule: str = "nonzero"
-    stroke: str = "none"
-    stroke_width: float = 1.0
-    stroke_linecap: str = "butt"
-    stroke_linejoin: str = "miter"
-    stroke_miterlimit: float = 4
-    stroke_dasharray: str = "none"
-    stroke_dashoffset: float = 0.0
-    stroke_opacity: float = 1.0
-    opacity: float = 1.0
-    transform: str = ""
-    style: str = ""
-    display: str = "inline"
+    clip_path: str = attrib_default("clip-path")
+    clip_rule: str = attrib_default("clip-rule")
+    fill: str = attrib_default("fill")
+    fill_opacity: float = attrib_default("fill-opacity")
+    fill_rule: str = attrib_default("fill-rule")
+    stroke: str = attrib_default("stroke")
+    stroke_width: float = attrib_default("stroke-width")
+    stroke_linecap: str = attrib_default("stroke-linecap")
+    stroke_linejoin: str = attrib_default("stroke-linejoin")
+    stroke_miterlimit: float = attrib_default("stroke-miterlimit")
+    stroke_dasharray: str = attrib_default("stroke-dasharray")
+    stroke_dashoffset: float = attrib_default("stroke-dashoffset")
+    stroke_opacity: float = attrib_default("stroke-opacity")
+    opacity: float = attrib_default("opacity")
+    transform: str = attrib_default("transform")
+    style: str = attrib_default("style")
+    display: str = attrib_default("display")
 
     def _copy_common_fields(
         self,
@@ -186,6 +190,28 @@ def _copy_common_fields(
         self.style = style
         self.display = display
 
+    def __str__(self) -> str:
+        parts = ["<", self.tag]
+        for field in dataclasses.fields(self):
+            attr_name = field.name.replace("_", "-")
+            value = getattr(self, field.name)
+            default_value = attrib_default(attr_name)
+            if isinstance(default_value, float):
+                value = float(value)
+            if value == default_value:
+                continue
+            parts.append(" ")
+            parts.append(attr_name)
+            parts.append("=")
+            parts.append('"')
+            if isinstance(value, numbers.Number):
+                parts.append(ntos(value))
+            else:
+                parts.append(str(value))
+            parts.append('"')
+        parts.append("/>")
+        return "".join(parts)
+
     def might_paint(self) -> bool:
         """False if we're sure this shape will not paint. True if it *might* paint."""
 
@@ -358,6 +384,7 @@ def normalize_opacity(self, inplace=False):
 # https://www.w3.org/TR/SVG11/paths.html#PathElement
 @dataclasses.dataclass
 class SVGPath(SVGShape, SVGCommandSeq):
+    tag: ClassVar[str] = "path"
     d: str = ""
 
     def __init__(self, **kwargs):
@@ -649,6 +676,7 @@ def round_multiple(self, multiple_of: float, inplace=False) -> "SVGPath":
 # https://www.w3.org/TR/SVG11/shapes.html#CircleElement
 @dataclasses.dataclass
 class SVGCircle(SVGShape):
+    tag: ClassVar[str] = "circle"
     r: float = 0
     cx: float = 0
     cy: float = 0
@@ -663,6 +691,7 @@ def as_path(self) -> SVGPath:
 # https://www.w3.org/TR/SVG11/shapes.html#EllipseElement
 @dataclasses.dataclass
 class SVGEllipse(SVGShape):
+    tag: ClassVar[str] = "ellipse"
     rx: float = 0
     ry: float = 0
     cx: float = 0
@@ -685,6 +714,7 @@ def as_path(self) -> SVGPath:
 # https://www.w3.org/TR/SVG11/shapes.html#LineElement
 @dataclasses.dataclass
 class SVGLine(SVGShape):
+    tag: ClassVar[str] = "line"
     x1: float = 0
     y1: float = 0
     x2: float = 0
@@ -702,6 +732,7 @@ def as_path(self) -> SVGPath:
 # https://www.w3.org/TR/SVG11/shapes.html#PolygonElement
 @dataclasses.dataclass
 class SVGPolygon(SVGShape):
+    tag: ClassVar[str] = "polygon"
     points: str = ""
 
     def as_path(self) -> SVGPath:
@@ -717,6 +748,7 @@ def as_path(self) -> SVGPath:
 # https://www.w3.org/TR/SVG11/shapes.html#PolylineElement
 @dataclasses.dataclass
 class SVGPolyline(SVGShape):
+    tag: ClassVar[str] = "polyline"
     points: str = ""
 
     def as_path(self) -> SVGPath:
@@ -732,6 +764,7 @@ def as_path(self) -> SVGPath:
 # https://www.w3.org/TR/SVG11/shapes.html#RectElement
 @dataclasses.dataclass
 class SVGRect(SVGShape):
+    tag: ClassVar[str] = "rect"
     x: float = 0
     y: float = 0
     width: float = 0
@@ -773,6 +806,7 @@ def as_path(self) -> SVGPath:
 
 
 class _SVGGradient:
+    tag: ClassVar[str] = "some_gradient"
     id: str
     gradientTransform: Affine2D
     gradientUnits: str
@@ -828,6 +862,7 @@ def from_element(cls, el, view_box: Rect) -> "_SVGGradient":
 # Should be parsed with from_element
 @dataclasses.dataclass
 class SVGLinearGradient(_SVGGradient):
+    tag: ClassVar[str] = "linearGradient"
     id: str
     x1: float
     y1: float
@@ -857,6 +892,7 @@ def from_element(cls, el, view_box) -> "SVGLinearGradient":
 # Should be parsed with from_element
 @dataclasses.dataclass
 class SVGRadialGradient(_SVGGradient):
+    tag: ClassVar[str] = "radialGradient"
     id: str
     cx: float
     cy: float