From 247fa84b98bbbc482e6f1f358eca0f0f5f20a61c Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Tue, 16 Nov 2021 14:28:14 +0000 Subject: [PATCH] only rename glyph element ids when clash actually occurs --- Lib/fontTools/subset/svg.py | 5 +++-- Tests/subset/svg_test.py | 30 +++++++++++++++++++----------- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/Lib/fontTools/subset/svg.py b/Lib/fontTools/subset/svg.py index b18e99d4d..e6cf53963 100644 --- a/Lib/fontTools/subset/svg.py +++ b/Lib/fontTools/subset/svg.py @@ -121,10 +121,11 @@ def subset_elements(el: etree.Element, retained_ids: Set[str]) -> bool: def remap_glyph_ids( - elements: Dict[str, etree.Element], glyph_index_map: Dict[int, int] + svg: etree.Element, glyph_index_map: Dict[int, int] ) -> Dict[str, str]: # Given {old_gid: new_gid} map, rename all elements containing id="glyph{gid}" # special attributes + elements = group_elements_by_id(svg) id_map = {} for el_id, el in elements.items(): m = GID_RE.match(el_id) @@ -230,7 +231,7 @@ def subset_glyphs(self, s) -> bool: continue if not s.options.retain_gids: - id_map = remap_glyph_ids(elements, glyph_index_map) + id_map = remap_glyph_ids(svg, glyph_index_map) update_glyph_href_links(svg, id_map) new_doc = etree.tostring(svg, pretty_print=s.options.pretty_svg).decode("utf-8") diff --git a/Tests/subset/svg_test.py b/Tests/subset/svg_test.py index cb8d16419..c5524c8d0 100644 --- a/Tests/subset/svg_test.py +++ b/Tests/subset/svg_test.py @@ -168,12 +168,15 @@ COMPLEX_SVG = """\ + + + - - + + @@ -304,27 +307,32 @@ COMPLEX_SVG = """\ ), ), ( - # 'glyph9' uses a path 'p2' defined inside 'glyph8', the latter is excluded - # from our subset, as such its id is renamed with a '.' prefix; but (edge - # case!) there already is an id=".glyph8" in the doc (for whatever reason, - # it's still a valid id), so we append a .{digit} suffix to disambiguate. - "7,9", + # 'glyph9' uses a path 'p2' defined inside 'glyph7', the latter is excluded + # from our subset, thus gets renamed '.glyph7'; an unrelated element with + # same id=".glyph7" doesn't clash because it was dropped. + # Similarly 'glyph10' uses path 'p3' defined inside 'glyph8', also excluded + # from subset and prefixed with '.'. But since an id=".glyph8" is already + # used in the doc, we append a .{digit} suffix to disambiguate. + "9,10", _lines( """\ - - + + - + - + + + + ]]>