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

Improve subsetter #1

Merged
merged 272 commits into from
Jun 16, 2024
Merged

Improve subsetter #1

merged 272 commits into from
Jun 16, 2024

Conversation

LaurenzV
Copy link
Sponsor Contributor

Still in-progress and far from being done. Just opening this so people know that it is being worked on.

@LaurenzV
Copy link
Sponsor Contributor Author

LaurenzV commented Jun 1, 2024

@laurmaedje so basically, I think this is ready to be reviewed now. I won't write much more here in the comment about the implementation itself, as I've tried to add README's/comments in the code wherever possible, but let me know if something is unclear.

I've let the fuzzer run for 20 minutes now and no issues have arised, so I'm (cautiously) feeling pretty good about it.

We do need to test more fonts for PDF (right now I've just tested 3 different ones to make sure the integration works), but I would prefer to do this with the PR in Typst, to make sure that you are okay with the current design of the crate, before I invest more time in testing that. And then we can also test it with some printers if we can find volunteers. So I think this shouldn't block this PR.

Let me know what you think (but no rush, as I know this is a lot, and I also don't expect you to try to understand every detail of how it works 😄)!

@LaurenzV LaurenzV marked this pull request as ready for review June 1, 2024 22:37
@laurmaedje
Copy link
Member

Great, I'll try to review it soon!

src/lib.rs Outdated Show resolved Hide resolved
@LaurenzV
Copy link
Sponsor Contributor Author

LaurenzV commented Jun 5, 2024

So I just tried changing the skrifa fuzzer to use hinting and seems like there are often small differences... Will try to investigate this.

@LaurenzV
Copy link
Sponsor Contributor Author

LaurenzV commented Jun 5, 2024

Alright, I did end up finding two issues, but there are still other issues with the hinted drawing mode... However, I get the same problem when using fonttools subset, so I don't think this should be a blocker. I also asked about it at googlefonts/fontations#936

@laurmaedje laurmaedje merged commit 4e0058b into typst:main Jun 16, 2024
2 checks passed
@laurmaedje
Copy link
Member

Thank you so much! 🚀🚀

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.

None yet

2 participants