Update to support group and thus the upcoming picosvg improvements

setup.py

@@ -39,7 +39,7 @@
         "importlib_resources>=3.3.0; python_version < '3.9'",
         "lxml>=4.0",
         "ninja>=1.10.0.post1",
-        "picosvg==0.16.0",  # TEMPORARY VERSION LOCK
+        "picosvg>=0.17.0",
         "pillow>=7.2.0",
         "regex>=2020.4.4",
         "toml>=0.10.1",

src/nanoemoji/color_glyph.py

@@ -14,6 +14,7 @@
 
 import math
 from absl import logging
+import dataclasses
 from itertools import chain, groupby, combinations
 from lxml import etree  # type: ignore
 from nanoemoji.colors import Color
@@ -22,7 +23,11 @@
 from nanoemoji.paint import (
     Extend,
     ColorStop,
+    CompositeMode,
     Paint,
+    PaintColrLayers,
+    PaintComposite,
+    PaintGlyph,
     PaintLinearGradient,
     PaintRadialGradient,
     PaintSolid,
@@ -32,7 +37,7 @@
 from picosvg.svg_meta import number_or_percentage
 from picosvg.svg_reuse import normalize, affine_between
 from picosvg.svg_transform import Affine2D
-from picosvg.svg import SVG
+from picosvg.svg import SVG, SVGTraverseContext
 from picosvg.svg_types import (
     SVGPath,
     SVGLinearGradient,
@@ -249,16 +254,6 @@ def _common_gradient_parts(el, shape_opacity=1.0):
     }
 
 
-class PaintedLayer(NamedTuple):
-    paint: Paint
-    path: str  # path.d
-    reuses: Tuple[Affine2D, ...] = ()
-
-    def shape_cache_key(self):
-        # a hashable cache key ignoring paint
-        return (self.path, self.reuses)
-
-
 def _paint(
     debug_hint: str, config: FontConfig, picosvg: SVG, shape: SVGPath, glyph_width: int
 ) -> Paint:
@@ -281,15 +276,36 @@ def _paint(
     return PaintSolid(color=Color.fromstring(shape.fill, alpha=shape.opacity))
 
 
-def _in_glyph_reuse_key(
-    debug_hint: str, config: FontConfig, picosvg: SVG, shape: SVGPath, glyph_width: int
-) -> Tuple[Paint, SVGPath]:
-    """Within a glyph reuse shapes only when painted consistently.
-    paint+normalized shape ensures this."""
-    return (
-        _paint(debug_hint, config, picosvg, shape, glyph_width),
-        normalize(shape, config.reuse_tolerance),
-    )
+def _paint_glyph(
+    debug_hint: str,
+    config: FontConfig,
+    picosvg: SVG,
+    context: SVGTraverseContext,
+    glyph_width: int,
+) -> Paint:
+    shape = context.shape()
+
+    if shape.fill.startswith("url("):
+        fill_el = picosvg.resolve_url(shape.fill, "*")
+        try:
+            glyph_paint = _GRADIENT_INFO[etree.QName(fill_el).localname](
+                config,
+                fill_el,
+                shape.bounding_box(),
+                picosvg.view_box(),
+                glyph_width,
+                shape.opacity,
+            )
+        except ValueError as e:
+            raise ValueError(
+                f"parse failed for {debug_hint}, {etree.tostring(fill_el)[:128]}"
+            ) from e
+    else:
+        glyph_paint = PaintSolid(
+            color=Color.fromstring(shape.fill, alpha=shape.opacity)
+        )
+
+    return PaintGlyph(glyph=shape.as_path().d, paint=glyph_paint)
 
 
 def _intersect(path1: SVGPath, path2: SVGPath) -> bool:
@@ -332,55 +348,50 @@ def _painted_layers(
     config: FontConfig,
     picosvg: SVG,
     glyph_width: int,
-) -> Generator[PaintedLayer, None, None]:
-    if config.reuse_tolerance < 0:
-        # shape reuse disabled
-        for path in picosvg.shapes():
-            yield PaintedLayer(
-                _paint(debug_hint, config, picosvg, path, glyph_width), path.d
-            )
-        return
-
-    # Don't sort; we only want to find groups that are consecutive in the picosvg
-    # to ensure we don't mess up layer order
-    for (paint, normalized), paths in groupby(
-        picosvg.shapes(),
-        key=lambda s: _in_glyph_reuse_key(debug_hint, config, picosvg, s, glyph_width),
-    ):
-        paths = list(paths)
-        transforms = [Affine2D.identity()]
-        if len(paths) > 1:
-            transforms.extend(
-                affine_between(paths[0], p, config.reuse_tolerance) for p in paths[1:]
+) -> Tuple[Paint, ...]:
+
+    defs_seen = False
+    layers = []
+
+    # Reverse to get leaves first because that makes building Paint's easier
+    # shapes *must* be leaves per picosvg
+    nodes = []
+    for context in reversed(tuple(picosvg.breadth_first())):
+        if context.depth() == 0:
+            continue  # svg root
+
+        # picosvg will deliver us exactly one defs and it will be the first child of svg
+        if context.path == "/svg[0]/defs[0]":
+            assert not defs_seen
+            defs_seen = True
+            continue  # defs are pulled in by the consuming paints
+
+        if context.is_shape():
+            nodes.append(
+                _paint_glyph(debug_hint, config, picosvg, context, glyph_width)
             )
 
-        success = True
-        for path, transform in zip(paths[1:], transforms[1:]):
-            if transform is None:
-                success = False
-                error_msg = (
-                    f"{debug_hint} grouped the following paths but no affine_between "
-                    f"could be computed:\n  {paths[0]}\n  {path}"
-                )
-                if config.ignore_reuse_error:
-                    logging.warning(error_msg)
-                else:
-                    raise ValueError(error_msg)
-
-        if success and len(paths) > 1:
-            # Don't create composite glyphs if components intersect each other and
-            # they have a transform which reverses their winding direction, or else this
-            # creates holes where they overlap as nonzero fill rule is applied by
-            # TrueType renderer: https://github.com/googlefonts/nanoemoji/issues/287
-            success = not _any_overlap_with_reversing_transform(
-                debug_hint, paths, transforms
+        if context.is_group():
+            # flush the current shapes into a new group
+            opacity = float(context.element.get("opacity"))
+            assert 0.0 < opacity < 1.0, f"{context.path} should be transparent"
+            assert len(nodes) > 1, f"{context.path} should have 2+ children"
+            assert {"opacity"} == set(
+                context.element.attrib.keys()
+            ), f"{context.path} only attribute should be opacity. Found {context.element.attrib.keys()}"
+            paint = PaintComposite(
+                mode=CompositeMode.SRC_IN,
+                source=PaintColrLayers(tuple(nodes)),
+                backdrop=PaintSolid(Color(0, 0, 0, opacity)),
             )
+            nodes = [paint]
 
-        if success:
-            yield PaintedLayer(paint, paths[0].d, tuple(transforms[1:]))
-        else:
-            for path in paths:
-                yield PaintedLayer(paint, path.d)
+        if context.depth() == 1:
+            # insert reversed to undo the reversed at the top of loop
+            layers.insert(0, nodes.pop())
+
+    assert defs_seen, "We never saw defs, what's up with that?!"
+    return tuple(layers)
 
 
 def _color_glyph_advance_width(view_box: Rect, config: FontConfig) -> int:
@@ -390,13 +401,38 @@ def _color_glyph_advance_width(view_box: Rect, config: FontConfig) -> int:
     return max(config.width, round(font_height * view_box.w / view_box.h))
 
 
+def _mutating_traverse(paint, mutator):
+    paint = mutator(paint)
+    assert paint is not None, "Return the input for no change, not None"
+
+    try:
+        fields = dataclasses.fields(paint)
+    except TypeError as e:
+        raise ValueError(f"{paint} is not a dataclass?") from e
+
+    changes = {}
+    for field in fields:
+        try:
+            is_paint = issubclass(field.type, Paint)
+        except TypeError:  # typing.Tuple and friends helpfully fail issubclass
+            is_paint = False
+        if is_paint:
+            current = getattr(paint, field.name)
+            modified = _mutating_traverse(current, mutator)
+            if current is not modified:
+                changes[field.name] = modified
+    if changes:
+        paint = dataclasses.replace(paint, **changes)
+    return paint
+
+
 class ColorGlyph(NamedTuple):
     ufo: ufoLib2.Font
     filename: str
     glyph_name: str
     glyph_id: int
     codepoints: Tuple[int, ...]
-    painted_layers: Optional[Tuple[PaintedLayer, ...]]  # None for untouched formats
+    painted_layers: Optional[Tuple[Paint, ...]]  # None for untouched formats
     svg: SVG  # picosvg except for untouched formats
     user_transform: Affine2D
 
@@ -408,7 +444,7 @@ def create(
         glyph_id: int,
         codepoints: Tuple[int],
         svg: SVG,
-    ):
+    ) -> "ColorGlyph":
         logging.debug(" ColorGlyph for %s (%s)", filename, codepoints)
         glyph_name = glyph.glyph_name(codepoints)
         base_glyph = ufo.newGlyph(glyph_name)
@@ -425,16 +461,18 @@ def create(
             base_glyph.unicode = next(iter(codepoints))
 
         # Grab the transform + (color, glyph) layers unless they aren't to be touched
-        painted_layers = None
-        if font_config.has_picosvgs:
-            painted_layers = tuple(
-                _painted_layers(
-                    filename,
-                    font_config,
-                    svg,
-                    base_glyph.width,
+        # or cannot possibly paint
+        painted_layers = ()
+        if not font_config.transform.is_degenerate():
+            if font_config.has_picosvgs:
+                painted_layers = tuple(
+                    _painted_layers(
+                        filename,
+                        font_config,
+                        svg,
+                        base_glyph.width,
+                    )
                 )
-            )
 
         return ColorGlyph(
             ufo,
@@ -472,10 +510,23 @@ def transform_for_otsvg_space(self):
     def transform_for_font_space(self):
         return self._transform(map_viewbox_to_font_space)
 
-    def paints(self):
-        """Set of Paint used by this glyph."""
-        return {l.paint for l in self.painted_layers}
-
     def colors(self):
         """Set of Color used by this glyph."""
-        return set(chain.from_iterable([p.colors() for p in self.paints()]))
+        all_colors = set()
+        self.traverse(lambda paint: all_colors.update(paint.colors()))
+        return all_colors
+
+    def traverse(self, visitor):
+        def _traverse_callback(paint):
+            visitor(paint)
+            return paint
+
+        for p in self.painted_layers:
+            _mutating_traverse(p, _traverse_callback)
+
+    def mutating_traverse(self, mutator) -> "ColorGlyph":
+        return self._replace(
+            painted_layers=tuple(
+                _mutating_traverse(p, mutator) for p in self.painted_layers
+            )
+        )

src/nanoemoji/config.py

@@ -87,6 +87,11 @@
     "Whether to fail or continue with a warning when picosvg cannot compute "
     "affine between paths that normalize the same.",
 )
+flags.DEFINE_bool(
+    "pretty_print",
+    None,
+    "Whether to prefer pretty printed content whenever possible (for testing).",
+)
 
 
 class Axis(NamedTuple):
@@ -128,6 +133,7 @@ class FontConfig(NamedTuple):
     output: str = "font"
     fea_file: str = "features.fea"
     codepointmap_file: str = "codepointmap.csv"
+    pretty_print: bool = False
     axes: Tuple[Axis, ...] = ()
     masters: Tuple[MasterConfig, ...] = ()
     source_names: Tuple[str, ...] = ()
@@ -176,6 +182,7 @@ def write(dest: Path, config: FontConfig):
         "ignore_reuse_error": config.ignore_reuse_error,
         "keep_glyph_names": config.keep_glyph_names,
         "clip_to_viewbox": config.clip_to_viewbox,
+        "pretty_print": config.pretty_print,
         "output": config.output,
         "axis": {
             a.axisTag: {
@@ -197,7 +204,7 @@ def write(dest: Path, config: FontConfig):
 
 
 def _resolve_config(
-    config_file: Path = None,
+    config_file: Optional[Path] = None,
 ) -> Tuple[Optional[Path], MutableMapping[str, Any]]:
     if config_file is None:
         if FLAGS.config is None:
@@ -236,7 +243,9 @@ def _pop_flag(config: MutableMapping[str, Any], name: str) -> Any:
     return flag_value if flag_value is not None else config_value
 
 
-def load(config_file: Path = None, additional_srcs: Tuple[Path] = None) -> FontConfig:
+def load(
+    config_file: Optional[Path] = None, additional_srcs: Optional[Tuple[Path]] = None
+) -> FontConfig:
     config_dir, config = _resolve_config(config_file)
 
     # CLI flags will take precedence over the config file
@@ -258,6 +267,7 @@ def load(config_file: Path = None, additional_srcs: Tuple[Path] = None) -> FontC
     ignore_reuse_error = _pop_flag(config, "ignore_reuse_error")
     keep_glyph_names = _pop_flag(config, "keep_glyph_names")
     clip_to_viewbox = _pop_flag(config, "clip_to_viewbox")
+    pretty_print = _pop_flag(config, "pretty_print")
     output = _pop_flag(config, "output")
 
     axes = []
@@ -333,6 +343,7 @@ def load(config_file: Path = None, additional_srcs: Tuple[Path] = None) -> FontC
         ignore_reuse_error=ignore_reuse_error,
         keep_glyph_names=keep_glyph_names,
         clip_to_viewbox=clip_to_viewbox,
+        pretty_print=pretty_print,
         output=output,
         fea_file=_DEFAULT_CONFIG.fea_file,
         codepointmap_file=_DEFAULT_CONFIG.codepointmap_file,