Skip to content
New issue

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

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

Already on GitHub? # to your account

Deterministic resolving of mixed component/contour glyphs #1150

Merged
merged 5 commits into from
Dec 3, 2024

Conversation

cmyr
Copy link
Member

@cmyr cmyr commented Nov 28, 2024

If a glyph has mixed components & contours, we need to convert it to use all of one or the other.

If such a glyph has a component that also has mixed components & contours, we need to make sure we handle the child component first.

We handles this by first collecting the set of such glyphs, and then determining an appropriate ordering.

I tried to keep this as simple as possible; in particular I'm defensive against cycles but am not reporting those errors here, since I assume we catch them elsewhere.

cmyr added 2 commits November 28, 2024 17:43
This will be useful for debugging in fontir.
When converting components to contours we need to make sure we consider
dependencies between glyphs.
@rsheeter
Copy link
Contributor

I'm defensive against cycles but am not reporting those errors here

Making sure we handle cycles is tracked as #1062

@rsheeter
Copy link
Contributor

IIUC the root problem is:

When converting components to contours we need to make sure to perform this operation in order, i.e. if glyph A has component B has component C we need to make sure we convert B before A.

This PR says:

We handles this by first collecting the set of such glyphs, and then determining an appropriate ordering.

Maybe I'm missing something, isn't this resolved by traversing components depth-first? - would make watching for cycles easier too.

@cmyr
Copy link
Member Author

cmyr commented Nov 28, 2024

maybe just unclear PR text? I'm using the same approach to determining the ordering as in the anchor propogation (it is depth first).

The reason this isn't useful as general-purpose cycle detection is that we're only operating on the subset of glyphs which have mixed contours/components.


for name in component_ordering_within_glyphs(items.values().map(|(_, g)| g.clone())) {
let (op, glyph) = items.get(&name).unwrap();
match op {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I feel like op.execute(context, glyph) might be nicer than matching on op to call a fn?

Exploratory fix for non-deterministic processing of mixed contours and paths
@cmyr cmyr added this pull request to the merge queue Dec 3, 2024
Merged via the queue into main with commit f036d57 Dec 3, 2024
10 checks passed
@cmyr cmyr deleted the glyph-determinism branch December 3, 2024 00:32
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

non-deterministic interpolation error
2 participants