From 1d4f01eeb0e55df0bb78088a4d7fa1d5245e3f0c Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 28 Apr 2022 18:37:29 +0100 Subject: [PATCH 1/2] glue_together must rebuild glyphID map after adding glyphs to glyf Otherwise we hit https://github.com/fonttools/fonttools/issues/2605 --- src/nanoemoji/glue_together.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/nanoemoji/glue_together.py b/src/nanoemoji/glue_together.py index 8c214c84..426de0dc 100644 --- a/src/nanoemoji/glue_together.py +++ b/src/nanoemoji/glue_together.py @@ -64,6 +64,13 @@ def _copy_colr(target: ttLib.TTFont, donor: ttLib.TTFont): target["CPAL"] = donor["CPAL"] target["COLR"] = donor["COLR"] + # Adding new glyphs to glyf table automatically appends to the TTFont's + # glyphOrder list, however the cached map from names to glyph IDs is not + # updated, so TTFont.getGlyphID will return incorrect result until + # we force it to be rebuilt, e.g. like so: + # https://github.com/fonttools/fonttools/issues/2605 + target.getReverseGlyphMap(rebuild=True) + def _svg_glyphs(font: ttLib.TTFont) -> Iterable[Tuple[int, str]]: for _, min_gid, max_gid in font["SVG "].docList: From d56f1b2e1d7f0e9a567461d8f190442a9612d6a9 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Fri, 29 Apr 2022 11:18:35 +0100 Subject: [PATCH 2/2] avoid using glyf __setitem__, call TTFont.setGlyphOrder at the end to rebuild gid cache --- src/nanoemoji/glue_together.py | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/src/nanoemoji/glue_together.py b/src/nanoemoji/glue_together.py index 426de0dc..635481f5 100644 --- a/src/nanoemoji/glue_together.py +++ b/src/nanoemoji/glue_together.py @@ -46,12 +46,20 @@ class CbdtGlyphInfo(NamedTuple): def _copy_colr(target: ttLib.TTFont, donor: ttLib.TTFont): # Copy all glyphs used by COLR over - _glyphs_to_copy = { - p.Glyph for p in paints_of_type(donor, ot.PaintFormat.PaintGlyph) - } - - for glyph_name in sorted(_glyphs_to_copy): - target["glyf"][glyph_name] = donor["glyf"][glyph_name] + glyphs_to_copy = sorted( + {p.Glyph for p in paints_of_type(donor, ot.PaintFormat.PaintGlyph)} + ) + + # We avoid using the glyf table's `__setitem__` for it appends to the TTFont's + # glyphOrder list without also invalidating the {glyphNames:glyphID} cache, + # which means TTFont.getGlyphID could return incorrect result. + # Instead we set the new glyphs directly inside the glyf's `glyphs` dict and + # call TTFont.setGlyphOrder at the end, which automatically triggers a rebuild + # of glyphID cache. + # https://github.com/fonttools/fonttools/issues/2605 + target_glyphs = target["glyf"].glyphs + for glyph_name in glyphs_to_copy: + target_glyphs[glyph_name] = donor["glyf"][glyph_name] if glyph_name in target["hmtx"].metrics: assert ( @@ -61,16 +69,11 @@ def _copy_colr(target: ttLib.TTFont, donor: ttLib.TTFont): # new glyph, new metrics target["hmtx"][glyph_name] = donor["hmtx"][glyph_name] + target.setGlyphOrder(target.getGlyphOrder() + glyphs_to_copy) + target["CPAL"] = donor["CPAL"] target["COLR"] = donor["COLR"] - # Adding new glyphs to glyf table automatically appends to the TTFont's - # glyphOrder list, however the cached map from names to glyph IDs is not - # updated, so TTFont.getGlyphID will return incorrect result until - # we force it to be rebuilt, e.g. like so: - # https://github.com/fonttools/fonttools/issues/2605 - target.getReverseGlyphMap(rebuild=True) - def _svg_glyphs(font: ttLib.TTFont) -> Iterable[Tuple[int, str]]: for _, min_gid, max_gid in font["SVG "].docList: