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

[colorLib] Sort paints by decreasing number of layers when encoding #3251

Closed
wants to merge 1 commit into from

Conversation

behdad
Copy link
Member

@behdad behdad commented Aug 9, 2023

Something like this. Untested. Trying to test now.

Fixes #3250

@behdad
Copy link
Member Author

behdad commented Aug 9, 2023

Wait. My measurement might be flawed.

@behdad
Copy link
Member Author

behdad commented Aug 9, 2023

In my testing with NotoColorEmoji-Regular, it saved a meager 824 bytes. Still, it's the right thing to do.

@anthrotype
Copy link
Member

The BaseGlyphList must be sorted by glyphID to be able to do the binary search, remember?

https://learn.microsoft.com/en-us/typography/opentype/spec/colr#baseglyph-and-layer-records

The BaseGlyph records must be sorted in increasing glyphID order. It is assumed that a binary search can be used to find a matching BaseGlyph record for a specific glyphID.

If you really want to build the LayerList in that decreasing length order, you can do that but then you need to make sure that the baseGlyphs are sorted by glyphOrder at the end

@behdad
Copy link
Member Author

behdad commented Aug 10, 2023

You are right. I'll fix up.

@behdad
Copy link
Member Author

behdad commented Aug 14, 2023

@anthrotype PTAL.

@anthrotype
Copy link
Member

would also be nice to have a test that exercise this if not much trouble

@behdad
Copy link
Member Author

behdad commented Aug 14, 2023

would also be nice to have a test that exercise this if not much trouble

Good thing I did. Implementation was wrong. Building noto color emoji now to get new numbers.

@behdad
Copy link
Member Author

behdad commented Aug 14, 2023

Need to update test expectations now.

@behdad behdad force-pushed the optimize-LayerList branch 2 times, most recently from 5d7bb5f to 5d12e1a Compare August 14, 2023 18:32
@behdad
Copy link
Member Author

behdad commented Aug 14, 2023

@anthrotype PTAL again. Sorry about that earlier.

@behdad
Copy link
Member Author

behdad commented Aug 14, 2023

Humm. I don't see this making any difference in NotoColorEmoji. Investigating.

@behdad
Copy link
Member Author

behdad commented Aug 14, 2023

Humm. I don't see this making any difference in NotoColorEmoji. Investigating.

I fixed the culprit. However, now the font grows by a few hundred bytes. Maybe status quo is good enough.

@behdad behdad closed this Aug 14, 2023
@khaledhosny khaledhosny deleted the optimize-LayerList branch May 6, 2024 22:11
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.

[colorLib] More optimized LayerList builder
2 participants