8000 [varLib/glyf] Rework phantom-points calculations by behdad · Pull Request #2266 · fonttools/fonttools · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[varLib/glyf] Rework phantom-points calculations #2266

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 15 commits into from
Apr 15, 2021
Merged

[varLib/glyf] Rework phantom-points calculations #2266

merged 15 commits into from
Apr 15, 2021

Conversation

behdad
Copy link
Member
@behdad behdad commented Apr 13, 2021

No description provided.

@behdad
[varLib/glyf] Remove fallback vertical phantom-points code
ee5ecd1 
I suggested Cosimo add that code in
#1528

Upon further inspection now, that code is unnecessary and even wrong in
cases where different masters have different ascent, because that would
result in encoding variations for vertical phantom points, which will
be then either be unused or double-apply on top of ascent etc.
@behdad behdad marked this pull request as draft April 13, 2021 00:29
@behdad
Copy link
Member Author
behdad commented Apr 13, 2021

Calls to ttFont.__getitem__ go down from over 200,000, to 70k so far.

@behdad
Copy link
Member Author
behdad commented Apr 13, 2021

And following timing goes from 5s to 4.6s:

$ time fonttools varLib  master_ttf_interpolatable/NotoSans-MM.designspace -x GSUB -x GPOS -x GDEF --disable-iup

@behdad behdad mentioned this pull request Apr 13, 2021
Closed
@behdad
Copy link
Member Author
behdad commented Apr 13, 2021

Calls to ttFont.__getitem__ go down from over 200,000, to 70k so far.

And now down to just 170.

This is ready for review now.

@behdad behdad marked this pull request as ready for review April 13, 2021 21:09
@behdad behdad requested review from anthrotype and madig April 13, 2021 21:09
@behdad
[glyf] Remove unused code
d12d7fb 
Also simplify bounds check.
@behdad behdad requested a review from simoncozens April 13, 2021 21:13
justvanrossum
justvanrossum reviewed Apr 14, 2021
View reviewed changes
anthrotype
anthrotype reviewed Apr 14, 2021
View reviewed changes
@behdad behdad force-pushed the varLib-phantomPoints branch from 0206cf8 to b65e83a Compare April 14, 2021 20:42
@behdad behdad force-pushed the varLib-phantomPoints branch from b65e83a to a43690e Compare April 14, 2021 20:43
@behdad
Copy link
Member Author
behdad commented Apr 14, 2021

Okay. Thanks everyone. I put back the old names. Didn't quite deprecate them. Am not sure where to go from here.

I think these were added as public API a bit hastily. We can deprecate and remove, while we figure out what to expose as public API now.

@behdad
Copy link
Member Author
behdad commented Apr 14, 2021

I think this can be merged if we open a tracker issue for the deprecation / new API part.

madig
madig reviewed Apr 14, 2021
View reviewed changes
madig
madig reviewed Apr 14, 2021
View reviewed changes
@behdad @madig
Update Lib/fontTools/varLib/instancer/__init__.py
438dcb5 
Co-authored-by: Nikolaus Waxweiler <madigens@gmail.com>
@behdad behdad merged commit 8ab6af0 into main Apr 15, 2021
@khaledhosny khaledhosny deleted the varLib-phantomPoints branch April 16, 2021 07:18
@anthrotype anthrotype mentioned this pull request May 7, 2021
Closed
simoncozens pushed a commit to simoncozens/fonttools that referenced this pull request Oct 22, 2021
@behdad @simoncozens
[glyf] Add old setCoordinates, getCoordinatesAndControls, getPhantomP…
48e3761 
…oints

fonttools#2266
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@justvanrossum justvanrossum justvanrossum left review comments

@madig madig madig approved these changes

@anthrotype anthrotype anthrotype approved these changes

@simoncozens simoncozens Awaiting requested review from simoncozens

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@behdad @madig @justvanrossum @anthrotype
0