fix building hybrid vector+bitmap COLRv1+SVG+CBDT font

src/nanoemoji/bitmap_tables.py

@@ -99,11 +99,6 @@ def _advance(ttfont: ttLib.TTFont, color_glyphs: Sequence[ColorGlyph]) -> int:
     return next(iter(advances))
 
 
-def _image_bytes(color_glyph: ColorGlyph) -> bytes:
-    with open(color_glyph.filename, "rb") as f:
-        return f.read()
-
-
 def _cbdt_record_size(image_format: int, image_data: bytes) -> int:
     assert image_format == _CBDT_SMALL_METRIC_PNGS, "Unrecognized format"
     return _CBDT_SMALL_METRIC_PNG_HEADER_SIZE + len(image_data)
@@ -117,7 +112,7 @@ def _cbdt_bitmapdata_offsets(
     offset = _CBDT_HEADER_SIZE
     for color_glyph in color_glyphs:
         offsets.append(offset)
-        offset += _cbdt_record_size(image_format, _image_bytes(color_glyph))
+        offset += _cbdt_record_size(image_format, color_glyph.bitmap)
     offsets.append(offset)  # capture end of stream
     return list(zip(offsets, offsets[1:]))
 
@@ -158,7 +153,7 @@ def make_sbix_table(
 
     for color_glyph in color_glyphs:
         # TODO: if we've seen these bytes before set graphicType "dupe", referenceGlyphName <name of glyph>
-        image_data = _image_bytes(color_glyph)
+        image_data = color_glyph.bitmap
 
         glyph_name = ttfont.getGlyphName(color_glyph.glyph_id)
         glyph = SbixGlyph(
@@ -258,7 +253,7 @@ def make_cbdt_table(
     cbdt.strikeData = [
         {
             ttfont.getGlyphName(c.glyph_id): _cbdt_bitmap_data(
-                config, metrics, _image_bytes(c)
+                config, metrics, c.bitmap
             )
             for c in color_glyphs
         }

src/nanoemoji/color_glyph.py

@@ -395,27 +395,28 @@ def _mutating_traverse(paint, mutator):
 
 class ColorGlyph(NamedTuple):
     ufo: ufoLib2.Font
-    filename: str
+    filenames: Tuple[str]
     ufo_glyph_name: str  # only if config has keep_glyph_names will this match in font binary
     glyph_id: int
     codepoints: Tuple[int, ...]
     painted_layers: Optional[Tuple[Paint, ...]]  # None for untouched and bitmap formats
-    svg: Optional[SVG]  # picosvg except for untouched and bitmap formats
+    svg: Optional[SVG]  # None for bitmap formats
     user_transform: Affine2D
+    bitmap: Optional[bytes]  # None for vector formats
 
     @staticmethod
     def create(
         font_config: FontConfig,
         ufo: ufoLib2.Font,
-        filename: str,
+        filenames: Sequence[str],
         glyph_id: int,
         ufo_glyph_name: str,
         codepoints: Tuple[int, ...],
         svg: Optional[SVG],
+        bitmap: Optional[bytes] = None,
     ) -> "ColorGlyph":
-        logging.debug(" ColorGlyph for %s (%s)", filename, codepoints)
+        logging.debug(" ColorGlyph for %s (%s)", ", ".join(filenames), codepoints)
         base_glyph = ufo.newGlyph(ufo_glyph_name)
-
         # non-square aspect ratio == proportional width; square == monospace
         view_box = None
         if svg:
@@ -435,23 +436,19 @@ def create(
         if not font_config.transform.is_degenerate():
             if font_config.has_picosvgs:
                 painted_layers = tuple(
-                    _painted_layers(
-                        filename,
-                        font_config,
-                        svg,
-                        base_glyph.width,
-                    )
+                    _painted_layers(filenames[0], font_config, svg, base_glyph.width)
                 )
 
         return ColorGlyph(
             ufo,
-            filename,
+            tuple(filenames),
             ufo_glyph_name,
             glyph_id,
             codepoints,
             painted_layers,
             svg,
             font_config.transform,
+            bitmap,
         )
 
     def _has_viewbox_for_transform(self) -> bool:

src/nanoemoji/config.py

@@ -19,6 +19,7 @@
 except ImportError:
     import importlib_resources as resources  # pytype: disable=import-error
 
+import itertools
 from pathlib import Path
 from picosvg.svg_transform import Affine2D
 import toml
@@ -163,23 +164,39 @@ class FontConfig(NamedTuple):
     masters: Tuple[MasterConfig, ...] = ()
     source_names: Tuple[str, ...] = ()
 
+    def _has_any(self, *color_formats) -> bool:
+        return bool(set(color_formats).intersection(self.color_format.split("_")))
+
     @property
     def output_format(self):
         return Path(self.output_file).suffix
 
     @property
-    def has_bitmaps(self):
-        return self.color_format.startswith("sbix") or self.color_format.startswith(
-            "cbdt"
-        )
+    def has_bitmaps(self) -> bool:
+        return self._has_any("sbix", "cbdt")
+
+    @property
+    def has_picosvgs(self) -> bool:
+        return self._has_any("glyf", "colr", "picosvg", "picosvgz")
+
+    @property
+    def has_untouchedsvgs(self) -> bool:
+        return self._has_any("untouchedsvg", "untouchedsvgz")
 
     @property
-    def has_picosvgs(self):
-        return not (self.color_format.startswith("untouchedsvg") or self.has_bitmaps)
+    def has_svgs(self) -> bool:
+        return self.has_picosvgs or self.has_untouchedsvgs
 
     @property
-    def has_svgs(self):
-        return not self.has_bitmaps
+    def is_vf(self) -> bool:
+        return len(self.masters) > 1
+
+    @property
+    def is_ot_svg(self) -> bool:
+        return self._has_any(
+            "".join(p)
+            for p in itertools.product(("picosvg", "untouchedsvg"), ("", "z"))
+        )
 
     def validate(self):
         for attr_name in (
@@ -200,6 +217,15 @@ def validate(self):
         if self.clipbox_quantization is not None and self.clipbox_quantization < 1:
             raise ValueError("If set, 'clipbox_quantization' must be 1 or positive")
 
+        # sanity check
+        assert self.has_svgs or self.has_bitmaps
+
+        if self.is_vf:
+            if self.has_bitmaps:
+                raise ValueError("bitmap formats cannot have multiple masters")
+            if self.is_ot_svg:
+                raise ValueError("OT-SVG formats cannot have multiple masters")
+
         return self
 
     def default(self) -> MasterConfig:

src/nanoemoji/glyphmap.py

@@ -13,24 +13,30 @@
 # limitations under the License.
 
 import csv
-from nanoemoji import codepoints
+from io import StringIO
 from pathlib import Path
-from typing import NamedTuple, Optional, Tuple
+from typing import NamedTuple, Tuple
 
 
 class GlyphMapping(NamedTuple):
     input_file: Path
     codepoints: Tuple[int, ...]
     glyph_name: str
 
-    def csv_line(self):
-        cp_str = ""
+    def csv_line(self) -> str:
+        row = [self.input_file, self.glyph_name]
         if self.codepoints:
-            cp_str = codepoints.string(self.codepoints)
-        return f"{self.input_file}, {self.glyph_name}, {cp_str}"
+            row.extend(f"{c:04x}" for c in self.codepoints)
+        else:
+            row.append("")
+        # Use a csv.writer instead of ",".join() so we escape commas in file/glyph names
+        f = StringIO()
+        writer = csv.writer(f, lineterminator="")
+        writer.writerow(row)
+        return f.getvalue()
 
 
-def load_from(file):
+def load_from(file) -> Tuple[GlyphMapping]:
     results = []
     reader = csv.reader(file, skipinitialspace=True)
     for row in reader:
@@ -50,6 +56,6 @@ def load_from(file):
     return tuple(results)
 
 
-def parse_csv(filename):
+def parse_csv(filename) -> Tuple[GlyphMapping]:
     with open(filename) as f:
         return load_from(f)

src/nanoemoji/nanoemoji.py

@@ -134,7 +134,7 @@ def _fea_file(font_config: FontConfig) -> Path:
 
 def _font_rule(font_config: FontConfig) -> str:
     suffix = "_font"
-    if _is_vf(font_config):
+    if font_config.is_vf:
         suffix = "_vfont"
     return Path(font_config.output_file).stem + suffix
 
@@ -155,14 +155,14 @@ def _ufo_config(font_config: FontConfig, master: MasterConfig) -> Path:
 
 def _glyphmap_rule(font_config: FontConfig, master: MasterConfig) -> str:
     master_part = ""
-    if _is_vf(font_config):
+    if font_config.is_vf:
         master_part = "_" + master.style_name.lower()
     return "write_" + Path(font_config.output_file).stem + master_part + "_glyphmap"
 
 
 def _glyphmap_file(font_config: FontConfig, master: MasterConfig) -> Path:
     master_part = ""
-    if _is_vf(font_config):
+    if font_config.is_vf:
         master_part = "." + master.output_ufo
     return _per_config_file(font_config, master_part + ".glyphmap")
 
@@ -189,7 +189,7 @@ def module_rule(
 
 
 def write_font_rule(nw, font_config: FontConfig, master: MasterConfig):
-    if _is_vf(font_config):
+    if font_config.is_vf:
         rule_name = _ufo_rule(font_config, master)
         config_file = _ufo_config(font_config, master)
     else:
@@ -282,7 +282,7 @@ def write_config_preamble(nw, font_config: FontConfig):
 
     for master in font_config.masters:
         write_font_rule(nw, font_config, master)
-    if _is_vf(font_config):
+    if font_config.is_vf:
         module_rule(
             nw,
             "write_variable_font",
@@ -479,15 +479,15 @@ def write_svg_font_diff_build(
 
 
 def _input_files(font_config: FontConfig, master: MasterConfig) -> List[Path]:
-    if font_config.has_bitmaps:
-        input_files = [bitmap_dest(f) for f in master.sources]
-    elif font_config.has_picosvgs:
-        input_files = [
+    input_files = []
+    if font_config.has_picosvgs:
+        input_files.extend(
             picosvg_dest(font_config.clip_to_viewbox, f) for f in master.sources
-        ]
-    else:
-
-        input_files = [abspath(f) for f in master.sources]
+        )
+    if font_config.has_untouchedsvgs:
+        input_files.extend(rel_build(f) for f in master.sources)
+    if font_config.has_bitmaps:
+        input_files.extend(bitmap_dest(f) for f in master.sources)
     return input_files
 
 
@@ -515,17 +515,23 @@ def write_glyphmap_build(
     nw.build(
         rel_build(_glyphmap_file(font_config, master)),
         _glyphmap_rule(font_config, master),
-        _input_files(font_config, master),
+        [rel_build(f) for f in master.sources],
     )
     nw.newline()
 
 
-def _inputs_to_font_build(font_config: FontConfig, master: MasterConfig) -> List[Path]:
+def _implicit_inputs_to_font_build(
+    font_config: FontConfig, master: MasterConfig
+) -> List[Path]:
+    # these inputs are not passed in as positional argv to write_font; unlike explicit
+    # inputs (i.e., .svg or .png files), these are generated files pulled in via CLI
+    # flags and hardcoded in the write_font ninja rule; thus we add them only as
+    # implicit dependencies
     return [
         rel_build(_config_file(font_config)),
         rel_build(_fea_file(font_config)),
         rel_build(_glyphmap_file(font_config, master)),
-    ] + _input_files(font_config, master)
+    ]
 
 
 def write_ufo_build(nw: NinjaWriter, font_config: FontConfig, master: MasterConfig):
@@ -535,17 +541,20 @@ def write_ufo_build(nw: NinjaWriter, font_config: FontConfig, master: MasterConf
     nw.build(
         master.output_ufo,
         _ufo_rule(font_config, master),
-        _inputs_to_font_build(font_config, master),
+        _input_files(font_config, master),
+        _implicit_inputs_to_font_build(font_config, master),
     )
     nw.newline()
 
 
 def write_static_font_build(nw: NinjaWriter, font_config: FontConfig):
     assert len(font_config.masters) == 1
+    master = font_config.default()
     nw.build(
         font_config.output_file,
         _font_rule(font_config),
-        _inputs_to_font_build(font_config, font_config.default()),
+        _input_files(font_config, master),
+        _implicit_inputs_to_font_build(font_config, master),
     )
     nw.newline()
 
@@ -569,16 +578,6 @@ def _write_config_for_build(font_config: FontConfig):
     logging.info(f"Wrote {config_file.relative_to(build_dir().parent)}")
 
 
-def _is_vf(font_config: FontConfig) -> bool:
-    return len(font_config.masters) > 1
-
-
-def _is_svg(font_config: FontConfig) -> bool:
-    return font_config.color_format.endswith(
-        "svg"
-    ) or font_config.color_format.endswith("svgz")
-
-
 def _run(argv):
     additional_srcs = tuple(Path(f) for f in argv if f.endswith(".svg"))
     font_configs = config.load_configs(
@@ -606,13 +605,11 @@ def _run(argv):
     ), "Can only generate diffs for one font at a time"
 
     if len(font_configs) > 1:
-        assert all(not _is_vf(c) for c in font_configs)
+        assert all(not c.is_vf for c in font_configs)
 
     logging.info(f"Proceeding with {len(font_configs)} config(s)")
 
     for font_config in font_configs:
-        if _is_vf(font_config) and _is_svg(font_config):
-            raise ValueError("svg formats cannot have multiple masters")
         _write_config_for_build(font_config)
         write_source_names(font_config)
 
@@ -658,17 +655,17 @@ def _run(argv):
 
             for font_config in font_configs:
                 if FLAGS.gen_svg_font_diffs:
-                    assert not _is_vf(font_config)
+                    assert not font_config.is_vf
                     write_svg_font_diff_build(
                         nw, font_config.output_file, font_config.masters[0].sources
                     )
 
                 for master in font_config.masters:
-                    if _is_vf(font_config):
+                    if font_config.is_vf:
                         write_ufo_build(nw, font_config, master)
 
             for font_config in font_configs:
-                if _is_vf(font_config):
+                if font_config.is_vf:
                     write_variable_font_build(nw, font_config)
                 else:
                     write_static_font_build(nw, font_config)

src/nanoemoji/svg.py

@@ -434,7 +434,7 @@ def _add_glyph(svg: SVG, color_glyph: ColorGlyph, reuse_cache: ReuseCache):
 
     view_box = color_glyph.svg.view_box()
     if view_box is None:
-        raise ValueError(f"{color_glyph.filename} must declare view box")
+        raise ValueError(f"{color_glyph.filenames[0]} must declare view box")
 
     # https://github.com/googlefonts/nanoemoji/issues/58: group needs transform
     svg_g.attrib["transform"] = _svg_matrix(color_glyph.transform_for_otsvg_space())