-
Notifications
You must be signed in to change notification settings - Fork 567
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 Scale by removing DPI. #1042
Conversation
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.
Trying out my new reviewing power... just one little nit in some old code.
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.
Looks like a good simplification!
I think it is useful to always link the reason for changes: #913 (comment)
Thanks, this is ready for another look. 👀 |
This PR removes any notion of DPI from
Scale
and makes it strictly scale factor based.Previously
Scale
always considered 96 as the target DPI but with this change that decision is in the platform code's hands. This will allow for easier implementation of future platforms that have a different target DPI like Android with 160.This also means that all the DPI-related stuff can be an implementation detail and it doesn't need to be surfaced to developers using druid. Especially as DPI doesn't really match any actual inches etc.