lxn / walk

A Windows GUI toolkit for the Go Programming Language
Other
6.8k stars 885 forks source link

Icon: Assume default icon size is always 16x16 #641

Closed rozmansi closed 4 years ago

rozmansi commented 4 years ago

win.GetSystemMetricsForDpi() ignores dpi parameter prior Windows 10. It returns 16x16, or 24x24, or 32x32 depending on "zoom" set by user. Assume the default small icon size is always 16x16 at 96dpi.

zx2c4 commented 4 years ago

Is this a safe assumption to make?

Isn't the point of the system metrics to query it from the OS?

rozmansi commented 4 years ago

Isn't the point of the system metrics to query it from the OS?

While Windows 10 have the GetSystemMetricsForDPI() that returns accurate icon size for 96dpi, the Windows 7 and 8 will have the DPI embedded into GetSystemMetrics() result.

We can't use win.GetSystemMetricsForDPI() as it fallbacks to GetSystemMetrics() and we wouldn't know if the value returned is the icon size with or without DPI conversion.

The best way to get this metrics reliably is to do the GetSystemMetricsForDPI() and GetSystemMetrics() dance ourselves:

if GetSystemMetricsForDPI() is available {
   // This is Windows 10.
   return GetSystemMetricsForDPI(SM_CXSMICONSIZE, 96);
}
// This is Windows 7 or 8: GetSystemMetrics() returns values in screen DPI.
return GetSystemMetrics(SM_CXSMICONSIZE) / screenDPI * 96
zx2c4 commented 4 years ago

Can you just hack lxn/win so that its fallback implementation successfully /screenDPI*96?

zx2c4 commented 4 years ago

This seems better: https://github.com/lxn/win/pull/99

rozmansi commented 4 years ago

Indeed, the better approach is to fix GetSystemMetricsForDpi() in lxn/win instead.