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

Flatten components: handle contour+component glyphs in nested components #448

Merged
merged 3 commits into from
Jan 18, 2021

Conversation

moyogo
Copy link
Collaborator

@moyogo moyogo commented Jan 17, 2021

At the moment FlattenComponentsFilter breaks glyphs that mix contours and components when used as nested component in other glyphs.
Only the components as copied when flattening and the contours are not carried over.
However since the target is TTF which doesn't handle mixing contours and components, FlattenComponentsFilter should just leave the nested component glyphs that use mixed glyphs

Any contour will cause components to be decomposed in TTF.

But more importantly, avoid losing contours in mixed contour-component glyphs when using nested components.
@moyogo moyogo requested a review from madig January 17, 2021 21:35
@anthrotype
Copy link
Member

Only the components are copied when flattening and the contours are not carried over.

then maybe the fix should be to carry over those contours?

It seems to me that this is relying on the decomposeComponents filter be run afterwards, which should not be necessarily the case (even if it will in the end in 99.999% of the cases). I would prefer if the filters were as much as possible independent of one another as well as from the order of execution.

@madig
Copy link
Collaborator

madig commented Jan 18, 2021

This strikes me as an annoying edge case. As Cosimo says, ideally filters are 100% independent of each other. Maybe decomposition of mixed glyphs needs to happen beforehand and this filter should actually raise an error upon encountering mixed glyphs?

@madig
Copy link
Collaborator

madig commented Jan 18, 2021

Cosimo says:

anthrotype: if each filter does its own thing everything works. one should be able to run a filter independently of the others. unless here i'm missing something crucial, like it's impossible to at the same time flattenComponents in mixed glyphs and carry over the contours.
anthrotype: yes i think it's possible. the leftover contours in mixed glyphs should be returned and drawn with the pen and the given component's transform
currently they are being discarded. which basically means mixed glyphs will be decomposed but only for the part where they are mixed.
madigens: you mean the filter should return leftover contours?
anthrotype: no, that helper _flattenComponents. or we just say run this as a post-filter and be done with it. post-decomposition-of-mixed-glyphs
madigens: and then the main filter function draws the contours?
anthrotype: yes to keep the mixedness

@moyogo
Copy link
Collaborator Author

moyogo commented Jan 18, 2021

We don't want the contours to be carried over.
That's a mistake (which I initially made as well).
It creates more glyphs mixing contours and components than necessary.

Say /Aring is a mixed /A component and contour for ring (instead of component), and /Aringacute is a /Aring component with /acutecomb.cap component.
In the TTF /Aring will be a basic contour glyph, so it can be used in /Aringacute as a component.
Carrying over the contours would mean /Aringacute is made of /A component, a contour for ring, /acutecomb.cap component, which would make /Aringacute a basic glyph as well in the TTF. The flattening would be a decomposition in the end.

What the FlattenComponentFilter does is ensure the target TTFs don't have nested components but flat components instead.
It's job is not to replace nested components by contour glyphs.

If you want the filter to be used for a different target than TTF, say for UFO in some other process, we can add a parameter to carry over contours but I'm not sure what that is useful for. FlattenComponentFilter exists because many TTF implementations are broken and can't handle nested components.

@moyogo
Copy link
Collaborator Author

moyogo commented Jan 18, 2021

If you prefer we can rename FlattenComponentsFilter to DontBreakMyTTFImplementationWithNestedComponentsFilter ;-)

@madig
Copy link
Collaborator

madig commented Jan 18, 2021

If I understand correctly, your change does what you describe as bad and carries over contours, but implicitly (relying on the decomposition filter) rather than explicitly (drawing stuff into the resulting glyph)? It sounds to me that the real fix is changing the sources to not mix outlines and components?

@moyogo
Copy link
Collaborator Author

moyogo commented Jan 18, 2021

If I understand correctly, your change does what you describe as bad and carries over contours, but implicitly (relying on the decomposition filter) rather than explicitly (drawing stuff into the resulting glyph)?

No. It doesn't carry over contours. It keeps components to one level, relying on the decomposition filter (required to produce TTFs). With this change, glyphs mixing contours and components are treated as glyphs with no components, because they will not have components in TTFs. TTF either has basic contour glyphs or component glyphs, it cannot mix both contour and components.

Saying the filter relies on the decomposition filter is true, but we are always relying on the decomposition filter, we cannot generate TTFs without decomposing glyphs mixing contours and components.

In fact, even the other solution, explicitly drawing the contours when replacing nested components by flat components, would also rely on the decomposition filter, because TTFs cannot mix contours and components.

Whatever we do, we are relying on the decomposition filter, it is absolutely required for TTFs.

It sounds to me that the real fix is changing the sources to not mix outlines and components?

The point of filters is to have design-friendly sources used or modified at build time.

@moyogo
Copy link
Collaborator Author

moyogo commented Jan 18, 2021

If this helps: running DecomposeComponentsFilter before FlattenComponentsFilter or after will produce the same result, with the current fix.

Mixed glyphs will either be decomposed first and become contour glyphs, treated like contour glyphs by FlattenComponentsFilter,
or
Mixed glyphs with be treated like contour glyphs by FlattenComponentsFilter and decomposed later by DecomposeComponentsFilter.

@madig
Copy link
Collaborator

madig commented Jan 18, 2021

Hm!!!! So while I'm not sure how to proceed and defer to Cosimo, I can just modify a certain font project to keep a list of mixed glyphs to decompose first thing because that's actually in my power 🤠

@anthrotype
Copy link
Member

anthrotype commented Jan 18, 2021

OK yes, Denis is right, mixed glyphs will be decomposed anyway (by TTGlyphPen, if ufo2ft hasn't already done that) when creating truetype glyphs. I'm OK with this change. Basically flatten components only flattens "pure" composite glyphs and stops the recursion whenever any contour is found, which means that subtree will have to be decomposed.

@moyogo
Copy link
Collaborator Author

moyogo commented Jan 18, 2021

Thanks @madig and @anthrotype !

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

Successfully merging this pull request may close these issues.

3 participants