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 drawString and setClipRect with 90/270-degree rotation #2343

Merged
merged 2 commits into from Mar 10, 2023

Conversation

dzhu
Copy link
Contributor

@dzhu dzhu commented Mar 10, 2023

Previously, the calculation of minX, minY, maxX, and maxY inside jswrap_graphics_drawString was what graphicsToDeviceCoordinates does, just written out explicitly. With 90- or 270-degree rotations, that meant that the final values ended up rotated 180 degrees relative to what they should have been. As a result, characters could end up effectively clipped twice and hence incorrectly not being drawn. What we actually want instead is to do the reverse transformation of graphicsToDeviceCoordinates, which fortunately already exists as deviceToGraphicsCoordinates.

The PR includes a test that would fail before and is fixed by this change.

dzhu added 2 commits March 9, 2023 20:08
Previously, the calculation of `minX`, `minY`, `maxX`, and `maxY` inside
`jswrap_graphics_drawString` was what `graphicsToDeviceCoordinates`
does, just written out explicitly. With 90- or 270-degree rotations,
that meant that the final values ended up rotated 180 degrees relative
to what they should have been. As a result, characters could end up
effectively clipped twice and hence incorrectly not being drawn.  What
we actually want instead is to do the _reverse_ transformation of
`graphicsToDeviceCoordinates`, which fortunately already exists as
`deviceToGraphicsCoordinates`.
@gfwilliams
Copy link
Member

Thanks - this looks great! I was sure there was a reason it was done that way, but I can't remember now - maybe it's just that deviceToGraphicsCoordinates didn't exist at the time or I'd forgotten about it...

@gfwilliams gfwilliams merged commit e433226 into espruino:master Mar 10, 2023
16 checks passed
@dzhu dzhu deleted the fix-rotate-clip-string branch March 10, 2023 18:43
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