googlefonts / nanoemoji

A wee tool to build color fonts.
Apache License 2.0
239 stars 19 forks source link

fix maximum_color'ing 0-width glyphs #424

Closed anthrotype closed 2 years ago

anthrotype commented 2 years ago

trying to reproduce issues converting COLRv1 font to OT-SVG when the former contains glyphs with 0 advance width (https://github.com/googlefonts/nanoemoji/issues/421)

anthrotype commented 2 years ago

oh right, empty glyf glyphs (numContours == 0) have no attribute xMax in fonttools. Currently glyph_region expects xMax to be there when a glyph's width == 0 (so that it can make up some advance width to prevent bitmaps disappear..)

https://github.com/googlefonts/nanoemoji/runs/7863175783?check_suite_focus=true#step:5:210

glyph.recalcBounds(glyfTable) would set bbox attributes to 0 for empty glyphs anyway, so we can simply assume if not hasattr(glyph, "xMax") then xMax is going to be 0 (provided that glyf glyph has been "expanded" [fully decompiled], which is the case as soon as you __getitem__ it from glyf).

anthrotype commented 2 years ago

ok, i'm now hitting the first assert width > 0

  File "/home/runner/work/nanoemoji/nanoemoji/.tox/py/lib/python3.7/site-packages/nanoemoji/generate_svgs_from_colr.py", line 41, in _view_box
    assert region.w > 0, f"0-width region for {glyph_name}"

https://github.com/googlefonts/nanoemoji/runs/7863539114?check_suite_focus=true#step:5:207

anthrotype commented 2 years ago

I do wonder if the 0 width check shouldn't still exist somewhere for bitmaps only.

I am not sure where to place that check though.. On the one hand, when going COLR=>SVG, I want to be able in generate_svgs_from_colr.py to write svg files that have viewbox.w=0, so that they get placed correctly in font space and there's no viewbox clipping for vector color formats; on the other hand, when coing COLR=>SVG=CBDT I want to prevent resvg from outputting empty PNGs when the viewbox.w = 0 (and use the current hack that takes as width the glyph.xMax), but then I can't reuse the same svgs generated from COLR as we currently do, but i'd have to generate a different sets of svgs where viewbox widths have not been zeroed. But even so, the xMax we take comes from the fallback b/w glyf glyph, which nanoemoji itself keeps empty when building COLRv1 from .svg input.. and empty glyphs implicitly have xMax=0 so we're back to invisible PNGs :,(

I think we should remove all the hacks that tamper with the glyphs advance width and that assert it's > 0 and live with the fact that 0-width bitmap glyphs will disappear until #402 is properly dealt with

rsheeter commented 2 years ago

I think we should remove all the hacks that tamper with the glyphs advance width and that assert it's > 0 and live with the fact that 0-width bitmap glyphs will disappear until https://github.com/googlefonts/nanoemoji/issues/402 is properly dealt with

SGTM, it's moving us forward

anthrotype commented 2 years ago

@rsheeter I think this is ready now