-
Notifications
You must be signed in to change notification settings - Fork 187
[win32] Unify up-scaling bounds using Rectangle #1783
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
[win32] Unify up-scaling bounds using Rectangle #1783
Conversation
HeikoKlare
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes looks sound to me. I was only wondering about whether scaling destination coordinates independently is correct, but found that to be fine (in particular since source x/y are only used for "correcting" width/height).
Like posted for related PRs: This is part of completing a fix in 2016 (3b55f60) improving the initial HiDPI support (e02d49a) for fractional scale values, which was only applied to according calculations in DPIUtil but not to all consumers that contained the same inaccuracy.
| width = DPIUtil.scaleUp(width, zoom); | ||
| height = DPIUtil.scaleUp(height, zoom); | ||
| scrollInPixels(destX, destY, x, y, width, height, all); | ||
| Rectangle rect = DPIUtil.scaleUp(new Rectangle(x, y, width, height), zoom); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: at all other places the variable is called rectangle
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adapted it 👍
This commit unifies scale up results in different places. With zoom values not dividable by 100 scaling up via rectangle or scaling up all values separately can give different result. Scaling always as rectangle solves this limitation.
a1abc24 to
9b4e9ba
Compare
Yes, from the usages I found this the correct/best solution |
This commit unifies scale up results in different places. With zoom values not dividable by 100 scaling up via rectangle or scaling up all values separately can give different result. Scaling always as rectangle solves this limitation.