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

Fix font matrix handling for an edge case #2

Merged
merged 2 commits into from
Aug 15, 2024
Merged

Conversation

LaurenzV
Copy link
Collaborator

@LaurenzV LaurenzV commented Aug 5, 2024

By pure chance, I found that Typst main fails to properly render some text with the new subsetter:

#set text(font: "PingFang SC", fallback: false)

你

After debuging, I realized that this is because, if the TOP DICT matrix is empty and the FONT DICT matrix is not, then in the subsetted font we would write 0.001 for the TOP DICT matrix and the existing FONT DICT matrix, which means that we are applying two transforms now. After rereading the comment in the ghostscript tracker, what ghostscript does is initialize TOP DICT matrix to 0.001 like we were doing before, but scales the FONT DICT matrix by x1000.

The way it is in the PR is not entirely correct because I don't think you can just scale the matrix like that, but I haven't seen any font matrix that has a skew/translation part, so I hope this should be fine for now.

Before (current Typst main): Only Apple Preview would render the PDF:
typst-0

After: All viewers render it, including Acrobat:
typst-0

Having no TOP DICT matrix but a FONT DICT matrix seems to be somewhat of an edge case, which is also why no tests are affected by this. So hopefully this doesn't cause any unwanted regressions.

@LaurenzV
Copy link
Collaborator Author

Alright, so this PR is now based on #3, so it shouldn't be merged before that.

I've created a "synthetic" font now with two crazy matrices and compared the output to what ghostscript would produce with such an embedded font, and they match (apart from the fact that our 0.799995 turns to 0.8 for them). And I also manually verified that the original font works in Typst-created PDFs. So hopefully this implementation is better now.

@LaurenzV LaurenzV marked this pull request as ready for review August 15, 2024 18:43
@LaurenzV LaurenzV merged commit 67bde74 into typst:main Aug 15, 2024
2 checks passed
@LaurenzV LaurenzV deleted the fix-2 branch August 15, 2024 19:42
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.

1 participant