From 9eaeb0f23999e940ee1c4ffb7a0db8e54ab26da1 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Tue, 16 Aug 2022 18:51:29 +0200 Subject: [PATCH 1/7] add (failing) test maximum_color'ing 0-width glyph from COLRv1 to SVG trying to reproduce issues converting COLRv1 font to OT-SVG when the former contains glyphs with 0 advance width --- tests/emoji_u0301.svg | 3 +++ tests/maximum_color_test.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 tests/emoji_u0301.svg diff --git a/tests/emoji_u0301.svg b/tests/emoji_u0301.svg new file mode 100644 index 00000000..97c0931d --- /dev/null +++ b/tests/emoji_u0301.svg @@ -0,0 +1,3 @@ + + + diff --git a/tests/maximum_color_test.py b/tests/maximum_color_test.py index ee0349fc..ca454349 100644 --- a/tests/maximum_color_test.py +++ b/tests/maximum_color_test.py @@ -109,3 +109,34 @@ def test_keep_glyph_names(keep_names): assert all( not gn.startswith("duck_") for gn in maximum_font.getGlyphOrder() ), maximum_font.getGlyphOrder() + + +def test_zero_advance_width_colrv1_to_svg(): + tmp_dir = run_nanoemoji( + ( + "--color_format", + "glyf_colr_1", + # use proportional widths, based on viewBox.w + "--width=0", + # don't clip to viewBox else zero-width glyph disappears + "--noclip_to_viewbox", + locate_test_file("emoji_u42.svg"), + # this one has viewBox="0 0 0 10", i.e. zero width, like + # combining marks usually are (e.g. 'acutecomb') + locate_test_file("emoji_u0301.svg"), + ) + ) + + initial_font_file = tmp_dir / "Font.ttf" + assert initial_font_file.is_file() + + initial_font = ttLib.TTFont(initial_font_file) + # sanity check widhts are proportional and we have 2 colr glyphs + assert initial_font["hmtx"]["B"] == (1200, 0) + assert initial_font["hmtx"]["acutecomb"] == (0, 0) + assert initial_font["COLR"].table.BaseGlyphList.BaseGlyphCount == 2 + + maxmium_font_file = _maximize_color(initial_font_file, ()) + maximum_font = ttLib.TTFont(maxmium_font_file) + + # TODO From 843692c812cd5df0242f31dfccb0801bafa464ad Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Tue, 16 Aug 2022 19:02:30 +0200 Subject: [PATCH 2/7] empty glyf.Glyphs have no bbox attributes that's how it is.. We could alternatively do glyph.recalcBounds(ttfont['glyf']) and the empty glyph would gain xMin, yMin, etc. all set to 0 anyway. --- src/nanoemoji/colr_to_svg.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/nanoemoji/colr_to_svg.py b/src/nanoemoji/colr_to_svg.py index 9c514e9d..993028f3 100644 --- a/src/nanoemoji/colr_to_svg.py +++ b/src/nanoemoji/colr_to_svg.py @@ -313,7 +313,9 @@ def glyph_region(ttfont: ttLib.TTFont, glyph_name: str) -> Rect: map_font_space_to_viewbox handles font +y goes up => svg +y goes down.""" width = ttfont["hmtx"][glyph_name][0] if width == 0: - width = ttfont["glyf"][glyph_name].xMax + glyph = ttfont["glyf"][glyph_name] + if hasattr(glyph, "xMax"): # empty glyphs have no bbox attributes + width = glyph.xMax return Rect( 0, -ttfont["OS/2"].sTypoAscender, From b73209ba016bc653eb4b147517ea9eb17c3b517d Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Tue, 16 Aug 2022 19:14:57 +0200 Subject: [PATCH 3/7] svg.docList contains list of tuples, not lists now why was this test not failing before? I guess because we don't keep dependencies pinned for our testing and something happened upstream since the last time we run our tests.. --- tests/nanoemoji_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/nanoemoji_test.py b/tests/nanoemoji_test.py index 8ae1e81d..7e17ba45 100644 --- a/tests/nanoemoji_test.py +++ b/tests/nanoemoji_test.py @@ -408,7 +408,7 @@ def test_glyph_with_zero_advance_width(color_format, tmp_path): svg = font["SVG "] assert len(svg.docList) == 1 gid = font.getGlyphID(gname) - assert svg.docList[0][1:] == [gid, gid] # [start, end] + assert svg.docList[0][1:] == (gid, gid) # (start, end) if "CBDT" in font: cbdt = font["CBDT"] assert len(cbdt.strikeData) == 1 From 3f9e921750453a5081d4a17bdbe1c154f8925b09 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Tue, 16 Aug 2022 19:17:44 +0200 Subject: [PATCH 4/7] looks like we already have a zero-width test glyph --- tests/emoji_u0301.svg | 3 --- tests/maximum_color_test.py | 4 ++-- 2 files changed, 2 insertions(+), 5 deletions(-) delete mode 100644 tests/emoji_u0301.svg diff --git a/tests/emoji_u0301.svg b/tests/emoji_u0301.svg deleted file mode 100644 index 97c0931d..00000000 --- a/tests/emoji_u0301.svg +++ /dev/null @@ -1,3 +0,0 @@ - - - diff --git a/tests/maximum_color_test.py b/tests/maximum_color_test.py index ca454349..3494fef8 100644 --- a/tests/maximum_color_test.py +++ b/tests/maximum_color_test.py @@ -121,9 +121,9 @@ def test_zero_advance_width_colrv1_to_svg(): # don't clip to viewBox else zero-width glyph disappears "--noclip_to_viewbox", locate_test_file("emoji_u42.svg"), - # this one has viewBox="0 0 0 10", i.e. zero width, like + # this one has viewBox="0 0 0 1200", i.e. zero width, like # combining marks usually are (e.g. 'acutecomb') - locate_test_file("emoji_u0301.svg"), + locate_test_file("u0301.svg"), ) ) From 5406472426be3e3eadcec94855d2135a2f7336b3 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Wed, 17 Aug 2022 18:29:05 +0200 Subject: [PATCH 5/7] remove assertions that glyph advance width > 0 this will make zero-width glyphs disappear when rasterized via resvg to PNG, because anything outside viewbox gets clipped (https://github.com/googlefonts/nanoemoji/issues/402). Thus, running maximum_color --bitmaps with a font that has such zero-width glyphs will produce invisible empty CBDT/sbix glyphs until that issue is resolved. However, removing this assertions allows us to support zero-width glyphs in vector color formats like OT-SVG/COLR (cf. https://github.com/googlefonts/nanoemoji/issues/421) --- src/nanoemoji/colr_to_svg.py | 6 ------ src/nanoemoji/generate_svgs_from_colr.py | 4 +--- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/src/nanoemoji/colr_to_svg.py b/src/nanoemoji/colr_to_svg.py index 993028f3..700b6897 100644 --- a/src/nanoemoji/colr_to_svg.py +++ b/src/nanoemoji/colr_to_svg.py @@ -327,15 +327,9 @@ def glyph_region(ttfont: ttLib.TTFont, glyph_name: str) -> Rect: def _view_box_and_transform( ttfont: ttLib.TTFont, view_box_callback: ViewboxCallback, glyph_name: str ) -> Tuple[Rect, Affine2D]: - view_box = view_box_callback(glyph_name) - assert view_box.w > 0, f"0-width viewBox for {glyph_name}?!" - region = glyph_region(ttfont, glyph_name) - assert region.w > 0, f"0-width region for {glyph_name}?!" - font_to_vbox = map_font_space_to_viewbox(view_box, region) - return (view_box, font_to_vbox) diff --git a/src/nanoemoji/generate_svgs_from_colr.py b/src/nanoemoji/generate_svgs_from_colr.py index ce6178f2..72e425c7 100644 --- a/src/nanoemoji/generate_svgs_from_colr.py +++ b/src/nanoemoji/generate_svgs_from_colr.py @@ -37,9 +37,7 @@ def _view_box(font: ttLib.TTFont, glyph_name: str) -> Rect: # we want a viewbox that results in no scaling when translating from font-space - region = glyph_region(font, glyph_name) - assert region.w > 0, f"0-width region for {glyph_name}" - return region + return glyph_region(font, glyph_name) def main(argv): From d404e17687bf0dcaff86378d008417314db1617b Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Wed, 17 Aug 2022 18:32:03 +0200 Subject: [PATCH 6/7] remove hack that sets 0-width to glyph.xMax this messes up horizontal positioning of glyphs with zero advance width, was added to prevent clipping of bitmap glyphs exceeding their viewbox (#402). The latter issue will be dealt with separately. --- src/nanoemoji/colr_to_svg.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/nanoemoji/colr_to_svg.py b/src/nanoemoji/colr_to_svg.py index 700b6897..9bbccc14 100644 --- a/src/nanoemoji/colr_to_svg.py +++ b/src/nanoemoji/colr_to_svg.py @@ -312,10 +312,6 @@ def glyph_region(ttfont: ttLib.TTFont, glyph_name: str) -> Rect: map_font_space_to_viewbox handles font +y goes up => svg +y goes down.""" width = ttfont["hmtx"][glyph_name][0] - if width == 0: - glyph = ttfont["glyf"][glyph_name] - if hasattr(glyph, "xMax"): # empty glyphs have no bbox attributes - width = glyph.xMax return Rect( 0, -ttfont["OS/2"].sTypoAscender, From de3147edf76f85859a567e9e0da909f9186b5725 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 18 Aug 2022 16:11:42 +0200 Subject: [PATCH 7/7] assert svg glyph converted from COLR has same dimension/horizontal positioning --- tests/maximum_color_test.py | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/tests/maximum_color_test.py b/tests/maximum_color_test.py index 3494fef8..335378a0 100644 --- a/tests/maximum_color_test.py +++ b/tests/maximum_color_test.py @@ -17,6 +17,7 @@ import copy from fontTools import ttLib from nanoemoji.keep_glyph_names import keep_glyph_names +from picosvg.svg import SVG from pathlib import Path import pytest import sys @@ -131,7 +132,7 @@ def test_zero_advance_width_colrv1_to_svg(): assert initial_font_file.is_file() initial_font = ttLib.TTFont(initial_font_file) - # sanity check widhts are proportional and we have 2 colr glyphs + # sanity check widths are proportional and we have 2 colr glyphs assert initial_font["hmtx"]["B"] == (1200, 0) assert initial_font["hmtx"]["acutecomb"] == (0, 0) assert initial_font["COLR"].table.BaseGlyphList.BaseGlyphCount == 2 @@ -139,4 +140,25 @@ def test_zero_advance_width_colrv1_to_svg(): maxmium_font_file = _maximize_color(initial_font_file, ()) maximum_font = ttLib.TTFont(maxmium_font_file) - # TODO + assert "COLR" in maximum_font + assert maximum_font["COLR"].table.BaseGlyphList.BaseGlyphCount == 2 + assert "SVG " in maximum_font + assert len(maximum_font["SVG "].docList) == 2 + + # check that 'acutecomb' still has 0 advance width + assert initial_font["hmtx"]["acutecomb"] == (0, 0) + # it has not been cropped away (has a non-empty bounding box) + doc = maximum_font["SVG "].docList[1] + assert doc.startGlyphID == doc.endGlyphID == maximum_font.getGlyphID("acutecomb") + svg = SVG.fromstring(doc.data) + shapes = list(svg.shapes()) + assert len(shapes) == 1 + bbox = shapes[0].bounding_box() + assert bbox.w > 0 + assert bbox.h > 0 + # its bbox matches the respective COLR ClipBox dimensions (quantized to 10) + clipBox = maximum_font["COLR"].table.ClipList.clips["acutecomb"] + assert abs(bbox.w - (clipBox.xMax - clipBox.xMin)) <= 10 + assert abs(bbox.h - (clipBox.yMax - clipBox.yMin)) <= 10 + # the SVG shape's horizontal positioning also matches the respective COLR glyph + assert abs(bbox.x - clipBox.xMin) <= 10