minor refactorings following review comment

https://github.com/fonttools/fonttools/pull/2288#discussion_r627907922
This commit is contained in:
Cosimo Lupo 2021-05-07 09:49:33 +01:00
parent 80598d3c2c
commit b0e24384c2
2 changed files with 32 additions and 25 deletions

View File

@ -5,7 +5,7 @@ Requires https://github.com/fonttools/skia-pathops
import itertools import itertools
import logging import logging
from typing import Iterable, Optional, Mapping from typing import Callable, Iterable, Optional, Mapping
from fontTools.misc.roundTools import otRound from fontTools.misc.roundTools import otRound
from fontTools.ttLib import ttFont from fontTools.ttLib import ttFont
@ -81,6 +81,15 @@ def ttfGlyphFromSkPath(path: pathops.Path) -> _g_l_y_f.Glyph:
return glyph return glyph
def _round_path(
path: pathops.Path, round: Callable[[float], float] = otRound
) -> pathops.Path:
rounded_path = pathops.Path()
for verb, points in path:
rounded_path.add(verb, *((round(p[0]), round(p[1])) for p in points))
return rounded_path
def _simplify(path: pathops.Path, debugGlyphName: str) -> pathops.Path: def _simplify(path: pathops.Path, debugGlyphName: str) -> pathops.Path:
# skia-pathops has a bug where it sometimes fails to simplify paths when there # skia-pathops has a bug where it sometimes fails to simplify paths when there
# are float coordinates and control points are very close to one another. # are float coordinates and control points are very close to one another.
@ -90,26 +99,28 @@ def _simplify(path: pathops.Path, debugGlyphName: str) -> pathops.Path:
# for the entire font. # for the entire font.
# https://bugs.chromium.org/p/skia/issues/detail?id=11958 # https://bugs.chromium.org/p/skia/issues/detail?id=11958
# https://github.com/google/fonts/issues/3365 # https://github.com/google/fonts/issues/3365
# TODO(anthrotype): remove once this Skia bug is fixed
try: try:
path2 = pathops.simplify(path, clockwise=path.clockwise) return pathops.simplify(path, clockwise=path.clockwise)
except pathops.PathOpsError: except pathops.PathOpsError:
path2 = pathops.Path() pass
for verb, points in path:
path2.add(verb, *((otRound(p[0]), otRound(p[1])) for p in points)) path = _round_path(path)
try: try:
path2.simplify(clockwise=path.clockwise) path = pathops.simplify(path, clockwise=path.clockwise)
except pathops.PathOpsError as e:
path.dump()
raise RemoveOverlapsError(
f"Failed to remove overlaps from glyph {debugGlyphName!r}"
) from e
else:
log.debug( log.debug(
"skia-pathops failed to simplify '%s' with float coordinates, " "skia-pathops failed to simplify '%s' with float coordinates, "
"but succeded using rounded integer coordinates", "but succeded using rounded integer coordinates",
debugGlyphName, debugGlyphName,
) )
return path2 return path
except pathops.PathOpsError as e:
path.dump()
raise RemoveOverlapsError(
f"Failed to remove overlaps from glyph {debugGlyphName!r}"
) from e
raise AssertionError("Unreachable")
def removeTTGlyphOverlaps( def removeTTGlyphOverlaps(

View File

@ -3,7 +3,7 @@ import pytest
pathops = pytest.importorskip("pathops") pathops = pytest.importorskip("pathops")
from fontTools.ttLib.removeOverlaps import _simplify from fontTools.ttLib.removeOverlaps import _simplify, _round_path
def test_pathops_simplify_bug_workaround(caplog): def test_pathops_simplify_bug_workaround(caplog):
@ -48,8 +48,4 @@ def test_pathops_simplify_bug_workaround(caplog):
expected.lineTo(713, 0) expected.lineTo(713, 0)
expected.close() expected.close()
rounded_result = pathops.Path() assert expected == _round_path(result, round=lambda v: round(v, 3))
for verb, pts in result:
rounded_result.add(verb, *((round(p[0], 3), round(p[1], 3)) for p in pts))
assert expected == rounded_result