Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

set-overlap-bits added #918

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions Lib/gftools/scripts/set_overlap_bits.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import argparse
from defcon import Font
from pathops import Path
from ufo2ft.filters.decomposeComponents import DecomposeComponentsFilter
from ufo2ft.preProcessor import TTFPreProcessor
import math
from fontTools.designspaceLib import DesignSpaceDocument


def set_overlap_bits(ufo):
# Skip setting bits if ufo
if any(g.lib.get("public.truetype.overlap") for g in ufo):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

safe to skip if flags are present (might have been added manually), but perhaps you could also add a CLI flag to overwrite existing overlap flags, as they might have been mechanically generated and out of sync

return
# Decompose components first because some component glyphs may have
# components that overlap each other
outline_glyphset = TTFPreProcessor(
ufo, filters=[DecomposeComponentsFilter()]
).process()

overlaps = set()
for glyph in outline_glyphset.values():
skia_path = Path()
pen = skia_path.getPen()
for contour in glyph:
contour.draw(pen)
area = skia_path.area
# rm overlaps
skia_path.simplify()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can fail sometimes (pathops has some tricky unfixable bugs), I think it'd be better to default to setting the flag for all glyphs except those whose area does not change after simplifying

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which means you'll have to guard against pathops errors here and pass (with a warning maybe) in case it occurs

simplified_area = skia_path.area
if not math.isclose(area, simplified_area, abs_tol=0.1):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's unusual to set abs_tol so much larger than rel_tol (default rel_tol is 1e-09), it'd take precedence most of the time, whereas in fact it's supposed to help with comparisons close 0 (where rel_tol tends to be too small), see https://docs.python.org/3/library/math.html#math.isclose
If you only care about comparing against an absolute value (and not do any relative comparisons) just do abs(a-b) <= abs_tol and you don't need to bother using isclose.

But actually, perhaps it would make more sense to use a tolernce that is relative to the font's unitsPerEm?
I don't know, something like font.info.unitsPerEm / 10_000 (you'd still get 0.1 for fonts with 1000 UPEM, but if would increase/decrease accordingly)

ufo[glyph.name].lib["public.truetype.overlap"] = True
overlaps.add(glyph.name)
return overlaps


def main(args=None):
parser = argparse.ArgumentParser(description="Set the overlap bits of a ufo/ds")
parser.add_argument("input", help="Input UFO or Designspace file", nargs="+")
args = parser.parse_args(args)

ufos = []
for fp in args.input:
if fp.endswith(".ufo"):
ufos.append(Font(fp))
elif fp.endswith(".designspace"):
ds = DesignSpaceDocument.fromfile(fp)
for src in ds.sources:
ufos.append(Font(src.path))
Comment on lines +45 to +48
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doing this for each master and then for each individual glyph is not sufficient; you want to set the flag on all the masters (or if you like only on the default master, which is the one that matters and which will contribute the actual glyf table to the final VF) if any of them needs overlap. If you only set on a non-default master, but not on the default one, then it will just be ignored and not present in the VF.

else:
raise NotImplementedError(f"Not supported file type: {fp}")

for ufo in ufos:
overlapping_glyphs = set_overlap_bits(ufo)
if overlapping_glyphs:
print(f"Overlap flags set for {len(overlapping_glyphs)} glyphs")
ufo.save()


if __name__ == "__main__":
main()