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
Var store optimize more #3127
Var store optimize more #3127
Conversation
It's very unlikely that this path is needed, but it's correct to have it.
See comments. Produces better results.
Produces better results even.
09e7845
to
f12f3d8
Compare
5f64f1a
to
9cbde09
Compare
6fa38e2
to
d8fabfa
Compare
I'm happy with this code now. It was due for a refresh! |
38a93e4
to
f5e62a1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay I mostly justed used this as an excuse to familiarize myself with ItemVariationStore.
Out of curiosity, do we have a sense of the savings provided this optimization?
Lib/fontTools/varLib/varStore.py
Outdated
best_idx = i | ||
best_gain = combined_gain - separate_gain | ||
heap = [] | ||
for i, other_encoding in enumerate(todo): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm confused here, are we ever using other_encoding
? it looks like we rebind it in the next loop without having touched it. Maybe it was supposed to be encoding
, and then you wouldn't need to manually bind that, below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are correct.
Harmless thinko.
Honestly not much. In the few kilobytes range. For NotoSans-VF it's 10kb; Roboto-Flex it's 14kb. |
No description provided.