WIP

src/nanoemoji/color_glyph.py

@@ -475,29 +475,6 @@ def _has_viewbox_for_transform(self) -> bool:
             )
         return view_box is not None
 
-    def _transform(self, map_fn):
-        if not self._has_viewbox_for_transform():
-            return Affine2D.identity()
-        return map_fn(
-            self.svg.view_box(),
-            self.ufo.info.ascender,
-            self.ufo.info.descender,
-            self.ufo_glyph.width,
-            self.user_transform,
-        )
-
-    def transform_for_otsvg_space(self):
-        return self._transform(map_viewbox_to_otsvg_space)
-
-    def transform_for_font_space(self):
-        print()
-        print(
-            self.svg_filename,
-            self.svg.view_box(),
-            self._transform(map_viewbox_to_font_space),
-        )
-        return self._transform(map_viewbox_to_font_space)
-
     @property
     def ufo_glyph(self) -> UfoGlyph:
         return self.ufo[self.ufo_glyph_name]

src/nanoemoji/glyph_reuse.py

@@ -45,7 +45,6 @@ def try_reuse(self, path: str, path_view_box: Rect) -> ReuseResult:
         assert path[0].upper() == "M", path
 
         path = SVGPath(d=path)
-        path_view_box = ReusableParts.view_box_for((path_view_box,))
         if path_view_box != self._reusable_parts.view_box:
             print(path, path_view_box, self._reusable_parts.view_box)
             path = path.apply_transform(

src/nanoemoji/nanoemoji.py

@@ -261,7 +261,7 @@ def write_preamble(nw):
     module_rule(
         nw,
         "write_part_file",
-        f"--reuse_tolerance $reuse_tolerance --output_file $out $in",
+        f"--reuse_tolerance $reuse_tolerance --upem $upem --output_file $out $in",
     )
     nw.newline()
 
@@ -360,19 +360,18 @@ def master_part_file_dest() -> Path:
 def write_picosvg_builds(
     picosvg_builds: Set[Path],
     nw: NinjaWriter,
-    clipped: bool,
-    reuse_tolerance: float,
+    font_config: FontConfig,
     master: MasterConfig,
 ) -> Tuple[Set[Path], Set[Path]]:
     rule_name = "picosvg_unclipped"
-    if clipped:
+    if font_config.clip_to_viewbox:
         rule_name = "picosvg_clipped"
 
     picosvgs = set()
     part_files = set()
     for svg_file in master.sources:
         svg_file = abspath(svg_file)
-        dest = picosvg_dest(clipped, svg_file)
+        dest = picosvg_dest(font_config.clip_to_viewbox, svg_file)
         if svg_file in picosvg_builds:
             continue
         picosvg_builds.add(svg_file)
@@ -383,7 +382,7 @@ def write_picosvg_builds(
             part_dest,
             "write_part_file",
             dest,
-            variables={"reuse_tolerance": reuse_tolerance},
+            variables={"reuse_tolerance": font_config.reuse_tolerance, "upem": font_config.upem},
         )
 
         picosvgs.add(dest)
@@ -667,8 +666,7 @@ def _run(argv):
                         _, parts = write_picosvg_builds(
                             picosvg_builds,
                             nw,
-                            font_config.clip_to_viewbox,
-                            font_config.reuse_tolerance,
+                            font_config,
                             master,
                         )
                         part_files |= parts

src/nanoemoji/parts.py

@@ -14,15 +14,20 @@
 """A cache of reusable parts, esp paths, for whatever purpose you see fit.
 
 Intended to be used as a building block for glyph reuse.
+
+We always apply nop transforms to ensure any command type flips, such as arcs
+to cubics, occur. This ensures that if we merge a part file with no transform
+with one that has transformation the command types still align.
 """
 
 import dataclasses
-from functools import lru_cache
+from functools import lru_cache, partial, reduce
 import json
 from nanoemoji.config import FontConfig
 from pathlib import Path
 from picosvg.geometric_types import Rect
 from picosvg.svg import SVG
+from picosvg.svg_meta import cmd_coords
 from picosvg.svg_reuse import affine_between, normalize
 from picosvg.svg_transform import Affine2D
 from picosvg.svg_types import SVGPath, SVGShape
@@ -86,15 +91,17 @@ def _bbox_area(shape: Shape) -> float:
     return bbox.w * bbox.h
 
 
-def _round(path: SVGShape) -> SVGPath:
-    return path.as_path().round_floats(_DEFAULT_ROUND_NDIGITS)
+def _round(shape: SVGShape) -> SVGPath:
+    return shape.as_path().round_floats(_DEFAULT_ROUND_NDIGITS)
 
 
-def as_shape(shape: SVGPath) -> Shape:
-    return Shape(_round(shape).d)
+def as_shape(path: SVGPath) -> Shape:
+    # apply a nop transform because some things still change, like arcs to cubics
+    path = path.apply_transform(Affine2D.identity())
+    return Shape(_round(path).d)
 
 
-# TODO: create a parts builder and a frozen parts to more explicitly model the add/use cycle
+# TODO: create a parts builder and a frozen parts from compute_donors() to more explicitly model the add/use cycle
 
 
 @dataclasses.dataclass
@@ -112,7 +119,13 @@ class ReusableParts:
     def normalize(self, path: str) -> NormalizedShape:
         if self.reuse_tolerance != -1:
             # normalize handles it's own rounding
-            norm = NormalizedShape(normalize(SVGPath(d=path), self.reuse_tolerance).d)
+            # apply a nop transform because some things still change, like arcs to cubics
+            norm = NormalizedShape(
+                normalize(
+                    SVGPath(d=path).apply_transform(Affine2D.identity()),
+                    self.reuse_tolerance,
+                ).d
+            )
         else:
             norm = NormalizedShape(path)
         return norm
@@ -131,24 +144,23 @@ def add(self, source: PathSource):
         """Combine two sets of parts. Source shapes will be scaled to dest viewbox."""
         if isinstance(source, ReusableParts):
             transform = Affine2D.rect_to_rect(source.view_box, self.view_box)
-            for normalized, shape_set in source.shape_sets.items():
-                for shape in shape_set:
-                    if transform != Affine2D.identity():
-                        shape = as_shape(SVGPath(d=shape).apply_transform(transform))
-                    self._add_norm_path(normalized, shape)
+            shapes = tuple(
+                reduce(lambda a, c: a | c, source.shape_sets.values(), set())
+            )
         elif isinstance(source, SVG):
             source.checkpicosvg()
-            transform = Affine2D.rect_to_rect(
-                ReusableParts.view_box_for((source.view_box(),)), self.view_box
-            )
-            for svg_shape in source.shapes():
-                svg_shape = svg_shape.as_path()
-                if transform != Affine2D.identity():
-                    svg_shape = svg_shape.apply_transform(transform)
-                self._add(as_shape(svg_shape))
+            transform = Affine2D.rect_to_rect(source.view_box(), self.view_box)
+            shapes = tuple(s.as_path() for s in source.shapes())
         else:
             raise ValueError(f"Unknown part source: {type(source)}")
 
+        for shape in shapes:
+            if isinstance(shape, str):
+                shape = SVGPath(d=shape)
+            if transform != Affine2D.identity():
+                shape = shape.apply_transform(transform)
+            self._add(as_shape(shape))
+
     def _compute_donor(self, norm: NormalizedShape):
         self._donor_cache[norm] = None  # no solution
 
@@ -157,7 +169,7 @@ def _compute_donor(self, norm: NormalizedShape):
         # shrinking a big thing is more likely to result in small #s that fit into
         # more compact PaintTransform variants so try biggest first
 
-        # TODO there are cases where this picks a suboptimal transform, e.g. a 2x3
+        # NOTE there are cases where this picks a suboptimal transform, e.g. a 2x3
         # downscale be used when a scale uniform around center upscale might work
         # Ex SVGPath(d="M8,13 A3 3 0 1 1 2,13 A3 3 0 1 1 8,13 Z")
         #    SVGPath(d="M11,5 A2 2 0 1 1 7,5 A2 2 0 1 1 11,5 Z")
@@ -249,52 +261,38 @@ def to_json(self):
         return json.dumps(json_dict, indent=2)
 
     @classmethod
-    def fromstring(cls, string) -> "ReusableParts":
-        first = string.strip()[0]
-        parts = cls()
-        if first == "<":
-            svg = SVG.fromstring(string).topicosvg()
-            parts.view_box = ReusableParts.view_box_for((svg.view_box(),))
-            for svg_shape in svg.shapes():
-                parts._add(as_shape(svg_shape))
-        elif first == "{":
-            json_dict = json.loads(string)
-            parts.version = tuple(int(v) for v in json_dict.pop("version").split("."))
-            assert parts.version == (1, 0, 0), f"Bad version {parts.version}"
-            parts.view_box = Rect(
-                *(int(v) for v in json_dict.pop("view_box").split(" "))
-            )
-            assert parts.view_box[:2] == (
-                0,
-                0,
-            ), f"Must be a viewbox from 0,0 {parts.view_box}"
-            parts.reuse_tolerance = float(json_dict.pop("reuse_tolerance"))
-            for shape_set_json in json_dict.pop("shape_sets"):
-                norm = NormalizedShape(shape_set_json.pop("normalized"))
-                shapes = ShapeSet({Shape(s) for s in shape_set_json.pop("shapes")})
-                donor = shape_set_json.pop("donor")
-                if donor and donor not in shapes:
-                    raise ValueError("Donor must be in group")
-                if shape_set_json:
-                    raise ValueError(f"Unconsumed input {shape_set_json}")
-                parts.shape_sets[norm] = shapes
-                if donor != "":
-                    parts._donor_cache[norm] = donor
-            if json_dict:
-                raise ValueError(f"Unconsumed input {json_dict}")
-
-        else:
-            raise ValueError(f"Unrecognized start sequence {string[:16]}")
+    def from_json(cls, string: str) -> "ReusableParts":
+        json_dict = json.loads(string)
+        parts = ReusableParts()
+        parts.version = tuple(int(v) for v in json_dict.pop("version").split("."))
+        assert parts.version == (1, 0, 0), f"Bad version {parts.version}"
+        parts.view_box = Rect(
+            *(int(v) for v in json_dict.pop("view_box").split(" "))
+        )
+        assert parts.view_box[:2] == (
+            0,
+            0,
+        ), f"Must be a viewbox from 0,0 {parts.view_box}"
+        parts.reuse_tolerance = float(json_dict.pop("reuse_tolerance"))
+        for shape_set_json in json_dict.pop("shape_sets"):
+            norm = NormalizedShape(shape_set_json.pop("normalized"))
+            shapes = ShapeSet({Shape(s) for s in shape_set_json.pop("shapes")})
+            donor = shape_set_json.pop("donor")
+            if donor and donor not in shapes:
+                raise ValueError("Donor must be in group")
+            if shape_set_json:
+                raise ValueError(f"Unconsumed input {shape_set_json}")
+            parts.shape_sets[norm] = shapes
+            if donor != "":
+                parts._donor_cache[norm] = donor
+        if json_dict:
+            raise ValueError(f"Unconsumed input {json_dict}")
         return parts
 
     @classmethod
-    def load(cls, input_file: Path) -> "ReusableParts":
+    def loadjson(cls, input_file: Path) -> "ReusableParts":
         ext = input_file.suffix.lower()
-        if ext not in {".svg", ".json"}:
+        if ext != ".json":
             raise ValueError(f"Unknown format {input_file}")
-        return cls.fromstring(input_file.read_text(encoding="utf-8"))
+        return cls.from_json(input_file.read_text(encoding="utf-8"))
 
-    @staticmethod
-    def view_box_for(view_boxes: Iterable[Rect]) -> Rect:
-        max_h = max((v.h for v in view_boxes))
-        return Rect(0, 0, max_h, max_h)

src/nanoemoji/svg.py

@@ -21,7 +21,7 @@
 from functools import reduce
 from lxml import etree  # pytype: disable=import-error
 from nanoemoji.colors import Color
-from nanoemoji.color_glyph import ColorGlyph
+from nanoemoji.color_glyph import map_viewbox_to_otsvg_space, ColorGlyph
 from nanoemoji.config import FontConfig
 from nanoemoji.disjoint_set import DisjointSet
 from nanoemoji.glyph_reuse import GlyphReuseCache
@@ -457,7 +457,27 @@ def _create_use_element(
     return svg_use
 
 
-def _add_glyph(svg: SVG, color_glyph: ColorGlyph, reuse_cache: ReuseCache):
+def _font_units_to_svg_units(view_box: Rect, config: FontConfig, glyph_width: int) -> Affine2D:
+    return map_viewbox_to_otsvg_space(
+        view_box,
+        config.ascender,
+        config.descender,
+        glyph_width,
+        config.transform,
+    )
+
+
+def _svg_units_to_font_units(view_box: Rect, config: FontConfig, glyph_width: int) -> Affine2D:
+    return map_viewbox_to_otsvg_space(
+        view_box,
+        config.ascender,
+        config.descender,
+        glyph_width,
+        config.transform,
+    )
+
+
+def _add_glyph(config: FontConfig, svg: SVG, color_glyph: ColorGlyph, reuse_cache: ReuseCache):
     svg_defs = svg.xpath_one("//svg:defs")
 
     # each glyph gets a group of its very own
@@ -469,11 +489,11 @@ def _add_glyph(svg: SVG, color_glyph: ColorGlyph, reuse_cache: ReuseCache):
         raise ValueError(f"{color_glyph.svg_filename} must declare view box")
 
     # https://github.com/googlefonts/nanoemoji/issues/58: group needs transform
-    transform = color_glyph.transform_for_otsvg_space()
+    transform = _font_units_to_svg_units(reuse_cache.glyph_cache.view_box(), config, color_glyph.ufo_glyph.width)
     if not transform.almost_equals(Affine2D.identity()):
         svg_g.attrib["transform"] = _svg_matrix(transform)
 
-    vbox_to_upem = color_glyph.transform_for_font_space()
+    vbox_to_upem = _svg_units_to_font_units(reuse_cache.glyph_cache.view_box(), config, color_glyph.ufo_glyph.width)
     upem_to_vbox = vbox_to_upem.inverse()
 
     # copy the shapes into our svg
@@ -726,7 +746,7 @@ def _picosvg_docs(
 
         for color_glyph in (color_glyphs[g] for g in group):
             if color_glyph.painted_layers:
-                _add_glyph(svg, color_glyph, reuse_cache)
+                _add_glyph(config, svg, color_glyph, reuse_cache)
 
         # tidy use elements, they may emerge from _add_glyph with unnecessary attributes
         _tidy_use_elements(svg)
@@ -767,7 +787,7 @@ def _rawsvg_docs(
                 # Map gid => svg doc
                 "id": f"glyph{color_glyph.glyph_id}",
                 # map viewBox to OT-SVG space (+x,-y)
-                "transform": _svg_matrix(color_glyph.transform_for_otsvg_space()),
+                "transform": _svg_matrix(_font_units_to_svg_units(color_glyph.svg.view_box(), config, color_glyph.ufo_glyph.width)),
             },
         )
         # move all the elements under the new group

src/nanoemoji/write_combined_part_files.py

@@ -29,14 +29,14 @@ def main(argv):
     input_files = util.expand_ninja_response_files(argv[1:])
 
     combined_parts = ReusableParts()
-    individual_parts = [ReusableParts.load(Path(p)) for p in input_files]
+    individual_parts = [ReusableParts.loadjson(Path(p)) for p in input_files]
     if individual_parts:
         combined_parts.version = util.only({p.version for p in individual_parts})
         combined_parts.reuse_tolerance = util.only(
             {p.reuse_tolerance for p in individual_parts}
         )
-        combined_parts.view_box = ReusableParts.view_box_for(
-            (p.view_box for p in individual_parts)
+        combined_parts.view_box =  util.only(
+            {p.view_box for p in individual_parts}
         )
 
     for parts in individual_parts: