Skip to content

Improve subsetter#1

Merged
laurmaedje merged 272 commits intotypst:mainfrom
LaurenzV:rewrite
Jun 16, 2024
Merged

Improve subsetter#1
laurmaedje merged 272 commits intotypst:mainfrom
LaurenzV:rewrite

Conversation

@LaurenzV
Copy link
Collaborator

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

@LaurenzV
Copy link
Collaborator 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!

@LaurenzV
Copy link
Collaborator 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
Collaborator 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
@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.

2 participants