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

Instances created from some Noto VFs fail remove overlaps #3365

Open
rsheeter opened this issue Apr 30, 2021 · 13 comments
Open

Instances created from some Noto VFs fail remove overlaps #3365

rsheeter opened this issue Apr 30, 2021 · 13 comments
Assignees

Comments

@rsheeter
Copy link
Collaborator

fonttools varLib.instancer ofl/notosansethiopic/NotoSansEthiopic\[wdth\,wght\].ttf wdth=100 wght=800 --remove-overlaps

Restricting axes: {'wdth': 100.0, 'wght': 800.0}
Loading variable font
Normalized limits: {'wght': 0.78997802734375, 'wdth': 0.0}
Instantiating glyf/gvar tables
Dropping HVAR table
Instantiating GDEF and GPOS tables
Dropping avar table
Dropping fvar table
Pruning name table
Removing overlaps from glyf table
Traceback (most recent call last):
  File "/tmp/venv/bin/fonttools", line 8, in <module>
    sys.exit(main())
  File "/private/tmp/venv/lib/python3.9/site-packages/fontTools/__main__.py", line 30, in main
    runpy.run_module(mod, run_name='__main__')
  File "/usr/local/Cellar/python@3.9/3.9.4/Frameworks/Python.framework/Versions/3.9/lib/python3.9/runpy.py", line 213, in run_module
    return _run_code(code, {}, init_globals, run_name, mod_spec)
  File "/usr/local/Cellar/python@3.9/3.9.4/Frameworks/Python.framework/Versions/3.9/lib/python3.9/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/private/tmp/venv/lib/python3.9/site-packages/fontTools/varLib/instancer/__main__.py", line 5, in <module>
    sys.exit(main())
  File "/private/tmp/venv/lib/python3.9/site-packages/fontTools/varLib/instancer/__init__.py", line 1408, in main
    instantiateVariableFont(
  File "/private/tmp/venv/lib/python3.9/site-packages/fontTools/varLib/instancer/__init__.py", line 1250, in instantiateVariableFont
    removeOverlaps(varfont)
  File "/private/tmp/venv/lib/python3.9/site-packages/fontTools/ttLib/removeOverlaps.py", line 162, in removeOverlaps
    if removeTTGlyphOverlaps(
  File "/private/tmp/venv/lib/python3.9/site-packages/fontTools/ttLib/removeOverlaps.py", line 96, in removeTTGlyphOverlaps
    path2 = pathops.simplify(path, clockwise=path.clockwise)
  File "src/python/pathops/_pathops.pyx", line 1476, in pathops._pathops.simplify
  File "src/python/pathops/_pathops.pyx", line 1487, in pathops._pathops.simplify
pathops._pathops.PathOpsError: operation did not succeed
@rsheeter
Copy link
Collaborator Author

Not sure if this is something to do with the font or an issue with simplify itself

@rsheeter
Copy link
Collaborator Author

@davelab6 perhaps instancing a few common locations would be a good bakery test for VFs?

@rsheeter
Copy link
Collaborator Author

rsheeter commented May 1, 2021

This occurs for Telugu as well:

fonttools varLib.instancer ofl/notosanstelugu/NotoSansTelugu\[wdth\,wght\].ttf wdth=75 wght=100 --remove-overlaps
fonttools varLib.instancer ofl/notosansteluguui/NotoSansTeluguUI\[wdth\,wght\].ttf wdth=75 wght=100 --remove-overlaps

Instances create fine at many other locations, something about the outlines at these positions seems to upset simplify.

@rsheeter rsheeter changed the title Noto Sans Ethiopic fails remove overlaps Instances created from some Noto VFs fail remove overlaps May 1, 2021
@davelab6
Copy link
Member

davelab6 commented May 1, 2021

Agreed, good test fail cases. @moyogo could you take a look at the Fonts

anthrotype added a commit to fonttools/fonttools that referenced this issue May 4, 2021
Sometimes skia-pathops simplify may fail (for unknown reasons which I'm still trying to debug).
It's a good idea to know the name of the offending glyph
google/fonts#3365
@anthrotype
Copy link
Member

If I print the name of the glyph that fails with PathOpsError, I get 'cche.eth' and 'che.eth'. This is for NotoSansEthiopic[wdth,wght].ttf wdth=100 wght=800. I don't know why skia-pathops fails like that with those specific glyphs. I don't think there's anything wrong with the glyphs per se. I will try to reproduce using only skia and file an issue with upstream.

@thlinard
Copy link
Contributor

thlinard commented May 4, 2021

If the Noto VFs are going to be corrected upstream, it would be nice to raise #3267 too: I haven't seen a single Noto VF with a correct STAT table.

@rsheeter
Copy link
Collaborator Author

rsheeter commented May 4, 2021

@marekjez86 is fixing Noto STATs feasible?

@moyogo
Copy link
Collaborator

moyogo commented May 4, 2021

For NotoSansTelugu, the PathOpsError is raise when removeOverlaps is run on ddauvoweltelu and ddhauvoweltelu.

I couldn't see anything wrong with these nor the ones in NotoSansEthiopic.

@rsheeter @thlinard We can fix the STAT tables.

@rsheeter
Copy link
Collaborator Author

rsheeter commented May 4, 2021

Anything unusual about them? Any pattern present in all the kerploding glyphs?

@anthrotype
Copy link
Member

I filed a bug upstream https://bugs.chromium.org/p/skia/issues/detail?id=11958

@rsheeter
Copy link
Collaborator Author

@anthrotype heads up, I'm observing the problem even after the rounding fix

@rsheeter
Copy link
Collaborator Author

Our automation reported fontTools.ttLib.removeOverlaps.RemoveOverlapsError: Failed to remove overlaps from glyph 'uni1328' at width 87.5, weight 800. Reproduces, albeit with different glyph reported, using fonttools cli:

fonttools varLib.instancer ofl/notosansethiopic/NotoSansEthiopic\[wdth\,wght\].ttf wdth=87.5 wght=800 --remove-overlaps
...blah blah blah...
Traceback (most recent call last):
  File ".../fontTools/ttLib/removeOverlaps.py", line 110, in _simplify
    path = pathops.simplify(path, clockwise=path.clockwise)
  File "src/python/pathops/_pathops.pyx", line 1476, in pathops._pathops.simplify
  File "src/python/pathops/_pathops.pyx", line 1487, in pathops._pathops.simplify
pathops._pathops.PathOpsError: operation did not succeed

The above exception was the direct cause of the following exception:

  ...snip...
  File ".../fontTools/varLib/instancer/__main__.py", line 5, in <module>
    sys.exit(main())
  File ".../fontTools/varLib/instancer/__init__.py", line 1409, in main
    instantiateVariableFont(
  File ".../fontTools/varLib/instancer/__init__.py", line 1251, in instantiateVariableFont
    removeOverlaps(varfont)
  File ".../fontTools/ttLib/removeOverlaps.py", line 209, in removeOverlaps
    if removeTTGlyphOverlaps(
  File ".../fontTools/ttLib/removeOverlaps.py", line 143, in removeTTGlyphOverlaps
    path2 = _simplify(path, glyphName)
  File ".../fontTools/ttLib/removeOverlaps.py", line 119, in _simplify
    raise RemoveOverlapsError(
fontTools.ttLib.removeOverlaps.RemoveOverlapsError: Failed to remove overlaps from glyph 'cchee.eth'

@anthrotype
Copy link
Member

cc @nathan-williams

simoncozens pushed a commit to simoncozens/fonttools that referenced this issue Oct 22, 2021
Sometimes skia-pathops simplify may fail (for unknown reasons which I'm still trying to debug).
It's a good idea to know the name of the offending glyph
google/fonts#3365
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants