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

Generated instances/fonts lose stacked nodes #35

Closed
kateliev opened this issue Mar 28, 2021 · 5 comments
Closed

Generated instances/fonts lose stacked nodes #35

kateliev opened this issue Mar 28, 2021 · 5 comments

Comments

@kateliev
Copy link

Hi, I am using your great library into several build tools for a project that i am working on. And i love it :)

I am facing the following problem: my project has many layers/masters (only three are shown below) where nodes that shape curves on some masters (soft/round corner) are kept stacked on others (sharp corner).
image

I am using your library for some CLI tools to generate "synthetic" masters from predefined instances and then use the resulting UFOs for VF generation (utilizing FontMake).

It seems that generating new instances removes the off-curve points between stacked nodes, thus the newly made masters lose compatibility. Is there a workaround or solution to avoid this. This all looks as optimization of some sort, but by looking at your code i see no option for turning that on/off... or actually anything that could cause this "problem"... Any suggestions are welcome.

Thank you in advance!

@LettError
Copy link
Owner

LettError commented Mar 28, 2021

Yes, it was an optimisation in mathGlyph from long ago. The filtering can be switched off, but it is quite deep in the stack in fontMath.mathGlyph. The switch is not exposed in fontParts. I explored making that accessible in ufoProcessor. It would mean some work for fontParts, adding tests, etc. I have not had time (or reason) to revisit.

If you want to tinker, this is where to dig:

font[glyphName].fromMathGlyph(glyphInstanceObject)

This calls fontParts:
https://github.com/robotools/fontParts/blob/4a55d73d049e8122fb43fd32ce53b0dcbde10340/Lib/fontParts/base/glyph.py#L1646

You see mathGlyph can be called with a filterRedundantPoints=True flag. You could, if you know how to do such things, change that to False in your own setup.

Mind you, this is untested and I may have overlooked something. Also, it may expose new compatibility issues in your geometry that are hidden by mathGlyph doing this.

Ideally, it should be abstracted as a single flag that would switch this behaviour off. But that means changes to ufoProcessor as well as fontParts.

@kateliev
Copy link
Author

kateliev commented Mar 29, 2021

Hey, thank you for your quick and detailed replay!
It seems that our project requires diving even deeper into your code (and others). Our glyphs fall into following condition:
https://github.com/LettError/ufoProcessor/blob/cb253029153d8399f1264a2e256409c646c93f2b/Lib/ufoProcessor/__init__.py
image

which leads to:
https://github.com/robotools/fontMath/blob/99e7dfb5fc0c3bc63a932cbee5e48f9f5ce9d7cc/Lib/fontMath/mathGlyph.py
image

and the FilterRedundantPointPen. But regretfully fiddling with the pen leads to even worse results... So I guess I would have to search for some alternate route to get the job done!

Thank you, if you come to any solution please let me know! So far I will leave the issue open, if you see that this is not going to be fixed in some foreseeable future, please close it :)

@kateliev
Copy link
Author

Just being curious... If this is a known problem or desired effect - what other solution (library) would you recommend me for generating my masters with stacked nodes intact so i keep compatibility. I understand that .makeInstance() is targeted more or less for instances :) ....otherwise it would have been called `.makeMaster() :)

@LettError
Copy link
Owner

You're right to raise the issue. It's a bit of a hassle as it touches a couple of packages. But it needs to get done somehow.

I can't guarantee this will get a response that fits with your release schedule though..

@kateliev
Copy link
Author

OK, i have solved this situation for me, by adding names to the temporary generated offcurve points in MathGlyphPen's _flushContour():
image

Then i let the FilterRedundantPointPen clean up only those stacked nodes that were named beforehand:
image

To me this seems like a very reasonable solution if they want to keep the current practice of adding new nodes to cover compatibility problems.... Like discussed here. Will offer it as solution, who knows they might like it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants