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

user32: do more faithful emulation of GetSystemMetricsForDpi for Win<10 on HiDPI #99

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zx2c4
Copy link
Contributor

@zx2c4 zx2c4 commented Oct 31, 2019

This is a replacement for lxn/walk#641

@rozmansi
Copy link
Contributor

rozmansi commented Nov 1, 2019

LGTM... 👍

Initially, I was convinced you need to apply DPI conversion selectively, as not all the GetSystemMetrics()s metrics are screen related. E.g. SM_CLEANBOOT.

However, one should be extra dimwit to call the GetSystemMetricsForDpi(SM_CLEANBOOT, 96), right?

@zx2c4
Copy link
Contributor Author

zx2c4 commented Nov 1, 2019

Btw, do we actually need the floating point and rounding here? We multiply before dividing, which makes me think we probably won't actually loose precision doing it with boring integers.

Same thing applies for a few other call sites of yours I've.

Or do you actually think some circumstances it will be meaningful to round up on .5 instead of down?

@rozmansi
Copy link
Contributor

rozmansi commented Nov 1, 2019

Drat, GetDpiForWindow() is also a Win 10+ function. When there's no GetSystemMetricsForDpi(), there's no GetDpiForWindow() either. https://github.com/lxn/walk/blob/ae73dacae894e1c38913c5d2bfd90a8815ac50fe/font.go#L218 would be a viable way to get DPI on Win 7 and 8.

Btw, do we actually need the floating point and rounding here? We multiply before dividing, which makes me think we probably won't actually loose precision doing it with boring integers.

Windows have MulDiv() for this very purpose. Is there a Go equivalent?

Same thing applies for a few other call sites of yours I've.

I noticed lxn/walk is mostly doing it this way and I followed the pattern.

Or do you actually think some circumstances it will be meaningful to round up on .5 instead of down?

As long as the rounding is consistent across entire lxn/walk and its clients code, the truncation can do just fine. On the contrary, if one function calculates margins by rounding to nearest integer and another by truncating, we'll get 1px quirks here and there.

@zx2c4
Copy link
Contributor Author

zx2c4 commented Feb 24, 2021

GetDpiForWindow is GetDeviceCaps(LOGPIXELSY) on <win10. That makes this commit amount to GetSystemMetrics*dpi/GetDeviceCaps there, which seems fine. We've been shipping this for months now in WireGuard with good results. Time to merge this here?

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