-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Resolve #21494: Automatically set window scaling based on DPI #21907
base: develop
Are you sure you want to change the base?
Conversation
Check DPI at UiContext load if not previously checked, then set to reasonable division of 0.25.
if (!SDL_GetDisplayDPI(0, &ddpi, &hdpi, &vdpi)) | ||
{ | ||
// If DPI can be read, divide DPI by regular DPI (96.0f) and round to nearest 0.25 | ||
gConfigGeneral.WindowScale = std::round(ddpi / 96.0f * 4.0) / 4.0; |
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.
gConfigGeneral.WindowScale = std::round(ddpi / 96.0f * 4.0) / 4.0; | |
constexpr auto regularDPI = 96.0f; | |
gConfigGeneral.WindowScale = std::round((ddpi / regularDPI) * 4.0) / 4.0; |
That also makes the comment redundant. (The general rule is to try and make the code speak, if that is not feasible, a comment is fine)
if (gConfigGeneral.RefreshDPIScaling) | ||
{ | ||
float ddpi, hdpi, vdpi; | ||
if (!SDL_GetDisplayDPI(0, &ddpi, &hdpi, &vdpi)) |
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.
if (!SDL_GetDisplayDPI(0, &ddpi, &hdpi, &vdpi)) | |
if (SDL_GetDisplayDPI(0, &ddpi, &hdpi, &vdpi) == 0) |
! implies a bool.
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.
It returns an error code, 0 in this case means success.
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.
I know. What I was saying is that an explicit check for that code makes it clearer it is a code and not a bool.
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.
For some reason I thought the suggestion is the actual diff, my bad.
@karstenvanf Could you apply the requested changes? |
I used a similar to solution to #6991, but modified it to include a config option that flags whether the DPI has been checked instead of setting an invalid scaling factor. If the DPI check returns an error, it does not change from the default of 1.