Skip to content

Commit

Permalink
[SVG] Make sure colored .notdef never gets reused and stays gid0
Browse files Browse the repository at this point in the history
Fixes #429
  • Loading branch information
anthrotype committed Sep 1, 2022
1 parent dfbb1e7 commit 1ead965
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 15 deletions.
34 changes: 20 additions & 14 deletions src/nanoemoji/svg.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
from picosvg.svg_types import SVGPath
from typing import (
cast,
ClassVar,
Mapping,
MutableMapping,
NamedTuple,
Expand Down Expand Up @@ -84,20 +85,24 @@ class ReuseCache:
gradient_ids: MutableMapping[GradientReuseKey, str] = dataclasses.field(
default_factory=dict
)
# sentinel value to forcibly skip reuse of a glyph (e.g. .notdef), distinct
# from None, which means "reuse not found".
SKIP_REUSE: ClassVar[ReuseResult] = ReuseResult("", Affine2D.identity())

def add_glyph(
self,
glyph_name: str,
reuse_result: Optional[ReuseResult],
context: SVGTraverseContext,
reuse_result: Optional[ReuseResult] = SKIP_REUSE,
):
assert glyph_name not in self.glyph_elements, f"Second addition of {glyph_name}"
if not isinstance(context.paint, PaintGlyph):
raise ValueError(f"Not a PaintGlyph {context}")
if not reuse_result:
self.glyph_cache.add_glyph(glyph_name, context.paint.glyph)
else:
self.reuse_results[glyph_name] = reuse_result
if reuse_result is not self.SKIP_REUSE:
if reuse_result is None:
self.glyph_cache.add_glyph(glyph_name, context.paint.glyph)
else:
self.reuse_results[glyph_name] = reuse_result
self.glyph_elements[glyph_name] = to_element(SVGPath(d=context.paint.glyph))


Expand Down Expand Up @@ -150,18 +155,19 @@ def _glyph_groups(

# we still need to add the .notdef layers to reuse_cache even if not reused,
# as later code expects all glyph elements to be there.
reuse_result = None
if color_glyph.ufo_glyph_name != ".notdef":
if color_glyph.ufo_glyph_name == ".notdef":
reuse_cache.add_glyph(glyph_name, context)
else:
reuse_result = reuse_cache.glyph_cache.try_reuse(
context.paint.glyph # pytype: disable=attribute-error
)
reuse_cache.add_glyph(glyph_name, reuse_result, context)
if reuse_result:
# This entire color glyph and the one we share a shape with go in one svg doc
reuse_groups.union(
color_glyph.ufo_glyph_name,
_color_glyph_name(reuse_result.glyph_name),
)
reuse_cache.add_glyph(glyph_name, context, reuse_result)
if reuse_result:
# This entire color glyph and the one we share a shape with go in one svg doc
reuse_groups.union(
color_glyph.ufo_glyph_name,
_color_glyph_name(reuse_result.glyph_name),
)

nth_paint_glyph += 1

Expand Down
18 changes: 17 additions & 1 deletion tests/maximum_color_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,9 +200,25 @@ def test_foreground_colr_to_svg_currentColor(tmp_path):

def test_colr_to_svg_with_colored_notdef(tmp_path):
initial_font = ttLib.TTFont()
# this subset of Nabla only contains notdef, space and numbersign
initial_font.importXML(locate_test_file("fonts/Nabla.subset.ttx"))
initial_font_file = tmp_path / "Nabla.subset.ttf"
initial_font.save(initial_font_file)

maxmium_font_file = _maximize_color(initial_font_file, ())
# TODO

maximum_font = ttLib.TTFont(maxmium_font_file)

# check .notdef glyph is still the first glyph and that space character
# follows it and has its codepoint assigned in cmap
assert maximum_font.getGlyphOrder()[:3] == [".notdef", "space", "numbersign"]
assert maximum_font["cmap"].getBestCmap() == {0x20: "space", ord("#"): "numbersign"}

# check that SVG table contains a .notdef glyph as GID=0 in a distinct SVG document
# from the one containing the numbersign
assert "SVG " in maximum_font
assert len(maximum_font["SVG "].docList) == 2
assert maximum_font["SVG "].docList[0].startGlyphID == 0
assert maximum_font["SVG "].docList[0].startGlyphID == 0
assert maximum_font["SVG "].docList[1].endGlyphID == 2
assert maximum_font["SVG "].docList[1].endGlyphID == 2

0 comments on commit 1ead965

Please sign in to comment.