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

Simplify transform logic for glyphs #797

Merged
merged 1 commit into from
Aug 1, 2024

Conversation

LaurenzV
Copy link
Contributor

This PR adds a method to get the transform of a glyph independent of the actual table that will be used to render the glyph. This allows user to perform their own logic for glyph selection (which is the case for me when rendering glyphs via PDF). The specific glyph transform methods use this new method as the base.

@RazrFalcon
Copy link
Collaborator

So you have changed the code you wrote before? Because I do not recognize it.
Can you explain why we had multiple transform methods before and now just one is ok?

@LaurenzV
Copy link
Contributor Author

LaurenzV commented Aug 1, 2024

Yes, that's the code I wrote before. We still have multiple transform methods but the difference is that I extracted the transforms common to all methods (i.e. span transform + cluster transform + glyph transform, you need to apply those regardless of whether you draw an outline or a bitmap glyph) into a separate method, and then each specific transform method calls those and applies additional transforms on top of that.

The reason I didn't do this the first time is that I was unable to disentangle the ts.pre_scale(1.0, -1.0) transform for outline methods from the other parts, as I got a test failure. But yesterday, after looking it again, I realized that this was because in the layout method, there was a transform that was applied wrongly (namely let ts = Transform::from_translate(x + glyph.dx as f32, glyph.dy as f32);, since in SVG coordinates and glyph coordinates the y axis is flipped, this should be -glyph.dy. And after making this change, I was able to disentangle it with all test cases still passing.

@RazrFalcon RazrFalcon merged commit 546df09 into linebender:master Aug 1, 2024
3 checks passed
@RazrFalcon
Copy link
Collaborator

Got it. Thanks!

@LaurenzV LaurenzV deleted the better_transforms branch August 1, 2024 10:33
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