googlefonts / ufo2ft

A bridge from UFOs to FontTools objects (and therefore, OTFs and TTFs).
MIT License
151 stars 43 forks source link

Filters (shallow-)copy glyph set by default #708

Open madig opened 1 year ago

madig commented 1 year ago

The filter seems to not draw a dottedCircle into a UFO, unless you pass in the glyphset/layer expliticly:

from ufoLib2 import Font
from ufo2ft.filters.dottedCircleFilter import DottedCircleFilter
ufo = Font()
ufo.info.xHeight = 500
philter = DottedCircleFilter()
philter(ufo)
assert "dottedCircle" in ufo or "uni25CC" in ufo

Fails on the assert. It passes if you call philter(ufo, ufo.layers.defaultLayer).

Is this intended behavior?

anthrotype commented 1 year ago

definitely not intended behaviour. is this only happening with DottedCircleFilter or with any other filter?

madig commented 1 year ago

As Cosimo found: https://github.com/googlefonts/ufo2ft/blob/5a5f6dfdebeaaaa50de0b927f122776f3783e1d9/Lib/ufo2ft/util.py#L61

This shallow copy is fine if you mod existing glyphs, but here we add a glyph, landing it in nirvana. @anthrotype says we should maybe change all instances of glyphSet = _GlyphSet.from_layer(font) to glyphSet = font.layers.defaultLayer in filters. The problem doesn't typically come up because ufo2ft's preprocessor code makes and uses its own glyphsets and passes them in explicitly, whereas here we implicitly want the current UFO to be modified.