search

lxn / win

A Windows API wrapper package for the Go Programming Language
Other
1.19k stars 312 forks source link

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

Open zx2c4 opened 4 years ago

zx2c4 commented 4 years ago

This is a replacement for https://github.com/lxn/walk/pull/641

rozmansi commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 3 years ago

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?