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.

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

Use precomputed part files for shape reuse #414

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Use precomputed part files for shape reuse #414

wants to merge 1 commit into from

Conversation

rsheeter
Copy link
Collaborator

@rsheeter rsheeter commented Apr 29, 2022 •
edited
Loading

Fixes #415. Fixes #313. Makes identifying and troubleshooting reuse failure far far easier as things in build/parts-merged.json with donor null after a build are the ones that failed affine_between, you can look to see if you agree with how things grouped, etc.

TODO list:

anthrotype
anthrotype reviewed May 3, 2022
View reviewed changes
src/nanoemoji/parts.py Outdated Show resolved Hide resolved
anthrotype added a commit that referenced this pull request May 3, 2022
@anthrotype
Revert "Merge pull request #407 from googlefonts/parts"
4d3af6f 
This reverts commit ca02334, reversing
changes made to 5c73fa2.

This PR laid the groundworks for #414, however the latter needs more work before merge.
I want to make a release now with the changes to maximum_color, so I will revert-revert this immediately afterwards.
@anthrotype
Copy link
Member

anthrotype commented May 3, 2022 •
edited
Loading

heads up, I wanted to make a release that included the recent couple fixes to maximum_color (i.e. removed assert width > 0, and rebuilding the glyphID cache after glueing the COLR glyphs), so I thought it'd be best to temporarily revert #407 (since it's incomplete without this #414 one), then cut the release without it, and finally revert-revert to restore it to the main branch.
So you may want to rebase on main now that I released v0.13.2

@rsheeter
Copy link
Collaborator Author

rsheeter commented May 3, 2022 •
edited
Loading

It should be harmless to leave #407 in place, no?

Edit: also fine to roll out/back ofc. Edit2: rebased on main.

@rsheeter rsheeter force-pushed the parts2 branch 5 times, most recently from 3176f99 to b811c51 Compare May 13, 2022 04:04
@rsheeter rsheeter changed the title [WIP] Use part files to power reusable parts Use precomputed part files for shape reuse May 13, 2022
@rsheeter rsheeter marked this pull request as ready for review May 13, 2022 20:41
@rsheeter rsheeter mentioned this pull request Jun 10, 2022
Merged
anthrotype
anthrotype reviewed Jun 10, 2022
View reviewed changes
src/nanoemoji/svg.py Outdated
)


def _svg_units_to_font_units(view_box: Rect, config: FontConfig, glyph_width: int) -> Affine2D:
Copy link
Member

Choose a reason for hiding this comment

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

this _svg_units_to_font_units and _font_units_to_svg_units do the same thing (both call map_viewbox_to_otsvg_space with same parameters) but I'd expect them to be one the inverse of the other.. Am I missing anything?

Copy link
Member

@anthrotype anthrotype Jun 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

maybe you wanted to use map_viewbox_to_font_units below? 🤔

@rsheeter rsheeter force-pushed the parts2 branch 8 times, most recently from e5cb9f1 to 558eea2 Compare June 12, 2022 00:20
@rsheeter rsheeter mentioned this pull request Jan 22, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anthrotype anthrotype anthrotype left review comments

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

Successfully merging this pull request may close these issues.

Arcs don't normalize well Generation of bad affines
2 participants
@rsheeter @anthrotype