maximum_color COLR => SVG

[rsheeter]

First swing at COLR to SVG. Note that I rejigged svg generation to in general slightly, in particular we now always keep glyph names until after make_svg_table which enables svg.py::_ensure_groups_grouped_in_glyph_order to rely on the names.

[anthrotype]

I needed to merge #390 so you want to change the base branch from svg2colr to main now

[anthrotype]

When I try to run maximum_color and I pass a relative (non absolute) paths to a font, ninja fails because it can't find it since its current directory is the build directory. Something like this below fixes the problem:

--- a/src/nanoemoji/maximum_color.py
+++ b/src/nanoemoji/maximum_color.py
@@ -266,6 +266,7 @@ def _run(argv):

     if gen_ninja():
         logging.info(f"Generating {build_file.relative_to(build_dir())}")
+        input_font = rel_build(input_font)
         with open(build_file, "w") as f:
             nw = NinjaWriter(f)
             _write_preamble(nw, input_font)

[anthrotype]

maybe we should add a test that confirms that doing nanoemoji --color_format glyf_colr_1_and_picosvg gives the same (or functionally the same?) result as running nanoemoji --color_format glyt_colr_1 or (picosvg) and then maximum_color on the previous font?

[anthrotype]

what do we do with the tools/colr2svg.py script? Maybe we can delete it, since it's superseded by python -m nanoemoji.generate_svgs_from_colr? The colr2svg.py script has two additional flags, one to set a custom viewbox size, and another to set a given number of decimal digits for rounding floats. Maybe you can add equivalent to generate_svgs_from_colr to replicate that functionality, can be handy at times. By default we'd still want it to not round floats.

[anthrotype]

I found another bug also related to the glyphOrder -- in addition to the one I commented about earlier in nanoemoji.svg where you need to also set glyf_table.glyphOrder.

This one is in glue_together.py, where you are similarly reordering the glyphs of a live TTFont, in this case the target. Also in there, like we do in nanoemoji.svg, you need to ensure the TTFont is fully decompiled (otherwise tables like cmap or even COLR are messed up) and you need to set the glyf_table.glyphOrder.

It may make sense to define a reorder_glyphs method in nanoemoji.util and use it in both nanoemoji.svg and nanoemoji.glue_together, like I do in the patch below.

Only after applying that, I can do maximum_color COLR=>SVG fonts that work as intented when rendered in Chrome (the COLR part) as well as Firefox/Safari (for the OT-SVG part).

diff --git a/src/nanoemoji/glue_together.py b/src/nanoemoji/glue_together.py
index d0d1aa3..e0716bf 100644
--- a/src/nanoemoji/glue_together.py
+++ b/src/nanoemoji/glue_together.py
@@ -21,6 +21,7 @@ from absl import logging
 from fontTools.ttLib.tables import otTables as ot
 from fontTools import ttLib
 from nanoemoji.colr import paints_of_type
+from nanoemoji.util import reorder_glyphs
 import os
 from typing import Iterable, Tuple

@@ -77,7 +78,7 @@ def _copy_svg(target: ttLib.TTFont, donor: ttLib.TTFont):

     new_glyph_order.extend(non_svg_target_glyphs)  # any leftovers?

-    target.setGlyphOrder(new_glyph_order)
+    reorder_glyphs(target, new_glyph_order)
     target["SVG "] = donor["SVG "]

diff --git a/src/nanoemoji/svg.py b/src/nanoemoji/svg.py
index af6820d..0756985 100644
--- a/src/nanoemoji/svg.py
+++ b/src/nanoemoji/svg.py
@@ -39,6 +39,7 @@ from nanoemoji.paint import (
     PaintColrLayers,
     is_transform,
 )
+from nanoemoji.util import reorder_glyphs
 from picosvg.geometric_types import Rect
 from picosvg.svg import to_element, SVG, SVGTraverseContext
 from picosvg import svg_meta
@@ -556,24 +557,6 @@ def _add_glyph(svg: SVG, color_glyph: ColorGlyph, reuse_cache: ReuseCache):
                 raise ValueError(f"What do we do with {context}")

-def _ensure_ttfont_fully_decompiled(ttfont: ttLib.TTFont):
-    # A TTFont might be opened lazily and some tables only partially decompiled.
-    # So for this to work on any TTFont, we first compile everything to a temporary
-    # stream then reload with lazy=False. Input font is modified in-place.
-    tmp = BytesIO()
-    ttfont.save(tmp)
-    tmp.seek(0)
-    ttfont2 = ttLib.TTFont(tmp, lazy=False)
-    for tag in ttfont2.keys():
-        table = ttfont2[tag]
-        # cmap is exceptional in that it always loads subtables lazily upon getting
-        # their attributes, no matter the value of TTFont.lazy option.
-        # TODO: remove this hack once fixed in fonttools upstream
-        if tag == "cmap":
-            _ = [st.cmap for st in table.tables]
-        ttfont[tag] = table
-
-
 def _ensure_groups_grouped_in_glyph_order(
     color_glyphs: MutableMapping[str, ColorGlyph],
     color_glyph_order: Sequence[str],
@@ -582,11 +565,6 @@ def _ensure_groups_grouped_in_glyph_order(
 ):
     # svg requires glyphs in same doc have sequential gids; reshuffle to make this true.

-    # Changing the order of glyphs in a TTFont requires that all tables that use
-    # glyph indexes have been fully decompiled (loaded with lazy=False).
-    # Cf. https://github.com/fonttools/fonttools/issues/2060
-    _ensure_ttfont_fully_decompiled(ttfont)
-
     # We kept glyph names stable when saving a font for svg so it's safe to match on
     assert ttfont["post"].formatType == 2, "glyph names need to be stable"

@@ -609,7 +587,8 @@ def _ensure_groups_grouped_in_glyph_order(
     ) == set(
         glyph_order
     ), f"lhs only {set(ttfont.getGlyphOrder()) - set(glyph_order)} rhs only {set(glyph_order) - set(ttfont.getGlyphOrder())}"
-    ttfont.setGlyphOrder(glyph_order)
+
+    reorder_glyphs(ttfont, glyph_order)

 def _use_href(use_el):
diff --git a/src/nanoemoji/util.py b/src/nanoemoji/util.py
index 51076fe..a82ed5e 100644
--- a/src/nanoemoji/util.py
+++ b/src/nanoemoji/util.py
@@ -17,9 +17,11 @@
 import os
 import contextlib
 from functools import partial
+from io import BytesIO
 from pathlib import Path
 import shlex
 from typing import List
+from fontTools import ttLib

 def only(filter_fn, iterable):
@@ -81,3 +83,32 @@ def file_printer(filename):
     else:
         with open(filename, "w") as f:
             yield partial(print, file=f)
+
+
+def ensure_ttfont_fully_decompiled(ttfont: ttLib.TTFont):
+    # A TTFont might be opened lazily and some tables only partially decompiled.
+    # So for this to work on any TTFont, we first compile everything to a temporary
+    # stream then reload with lazy=False. Input font is modified in-place.
+    tmp = BytesIO()
+    ttfont.save(tmp)
+    tmp.seek(0)
+    ttfont2 = ttLib.TTFont(tmp, lazy=False)
+    for tag in ttfont2.keys():
+        table = ttfont2[tag]
+        # cmap is exceptional in that it always loads subtables lazily upon getting
+        # their attributes, no matter the value of TTFont.lazy option.
+        # TODO: remove this hack once fixed in fonttools upstream
+        if tag == "cmap":
+            _ = [st.cmap for st in table.tables]
+        ttfont[tag] = table
+
+
+def reorder_glyphs(ttfont: ttLib.TTFont, glyph_order: List[str]):
+    # Changing the order of glyphs in a TTFont requires that all tables that use
+    # glyph indexes have been fully decompiled (loaded with lazy=False).
+    # Cf. https://github.com/fonttools/fonttools/issues/2060
+    ensure_ttfont_fully_decompiled(ttfont)
+    ttfont.setGlyphOrder(glyph_order)
+    # glyf table is special and needs its own glyphOrder...
+    assert ttfont.isLoaded("glyf")
+    ttfont["glyf"].glyphOrder = glyph_order

[rsheeter]

@anthrotype I see warnings for "GSUB/GPOS Coverage is not sorted by glyph ids." ... is there by any chance a convenient method that fixes that for me?

EDIT: also the resulting font fails OTS and thus won't work on the web

ERROR at ../src/layout.cc:402 (ParseCoverageFormat2)
ERROR: Layout: bad start coverage index.
ERROR at ../src/gpos.cc:450 (ParsePairAdjustment)
ERROR: GPOS: Failed to parse coverage table
ERROR at ../src/layout.cc:1178 (Parse)
ERROR: Layout: Failed to parse lookup subtable 1
ERROR at ../src/layout.cc:240 (ParseLookupTable)
ERROR: Layout: Failed to parse subtable 0
ERROR at ../src/layout.cc:1328 (ParseLookupListTable)
ERROR: Layout: Failed to parse lookup 0
ERROR: GPOS: Failed to parse lookup list table
ERROR at ../src/ots.cc:711 (ProcessGeneric)
ERROR: GPOS: Failed to parse table
Failed to sanitize file!

EDIT2

Apparently "not lazy" doesn't mean what I think it does

    # one would think lazy=False meant tags would report isLoaded true
    # one would be wrong
    font = ttLib.TTFont(str(font_location), lazy=False)

    print("LOAD NOT LAZY?")
    for tag in font.keys():
        print("  ", tag, font.isLoaded(tag))

    not_loaded = sorted(t for t in font.keys() if not font.isLoaded(t))
    assert not not_loaded, f"Everything should be loaded, following aren't: {not_loaded}"  # this will fail, nothing is loaded

EDIT3

I went to some length to ensure everything is loaded when creating the TTFonts and the coverage problem still occurs. Curiously it runs code that seems to aspire to fix the problem (ranges.sort(key=lambda a: a.StartID)) but the resulting font fails OTS.

[anthrotype]

If the GSUB has been fully decompiled before reordering glyphs, then upon recompile the Coverage will be ordered using the new glyph order, so it's weird you see that. I'll take a look next week (I'm off today)

[anthrotype]

if you'd like to run OTS on the CI to check fonts work for web, you can use the python wrapper that we maintain at https://github.com/googlefonts/ots-python, available from PyPI as pip install opentype-sanitizer. The CLI entry point (equivalent to the C++ ots-sanitize executable) is python -m ots --help

[anthrotype]

i marked "approved" but it's not ready yet until the issue with glyph reordering in #399 is fixed first

[rsheeter]

I think this is sufficiently far along to be worth merging. I expect follow-on PRs but I don't anticipate tossing the core approach at this point.