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

Fix the order of BE kern gathering #678

Merged
merged 1 commit into from
Jan 13, 2024
Merged

Fix the order of BE kern gathering #678

merged 1 commit into from
Jan 13, 2024

Conversation

rsheeter
Copy link
Contributor

@rsheeter rsheeter commented Jan 12, 2024

I believe this resolves the issue described at #647 (comment) by applying the same fix as #655 to the second scatter/gather we do on kerning.

I have filed #677 to track making this simpler as it's definitely harder than it shoudl be.

The following produces observably unstable output on main, usually in 2-4 builds, and is stable through 20 cycles after the fix:

$ rm -f *.ttxl threads*.svg && for i in {0..19}; do echo "CYCLE $i" && rm -rf build/ && cargo run ../googlesans/source/GoogleSans/GoogleSans.designspace --emit-timing && cp build/threads.svg ./threads_$i.svg && ttx -l build/font.ttf > run$i.ttxl && grep GPOS *.ttxl; done

# Unstable: GPOS occasionally dramatically smaller
# because gather ran before the tasks that produce what it gathers
CYCLE 2
    Finished dev [unoptimized + debuginfo] target(s) in 0.19s
     Running `target/debug/fontc ../googlesans/source/GoogleSans/GoogleSans.designspace --emit-timing`
run0.ttxl:    GPOS  0x19131F6A   4364690     19000
run1.ttxl:    GPOS  0x19131F6A   4364690     19000
run2.ttxl:    GPOS  0x4F3B316D    106928     15264  # MUCH BADNESS

JMM if happy.

Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

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

if you're happy, I'm happy.

@cmyr cmyr added this pull request to the merge queue Jan 13, 2024
Merged via the queue into main with commit 8ef01ff Jan 13, 2024
10 checks passed
@cmyr cmyr deleted the stabby branch January 13, 2024 20:21
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.

2 participants