Skip to content

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

Closed
rozmansi wants to merge 1 commit intolxn:masterfrom
rozmansi:pending/default-icon-size
Closed

Icon: Assume default icon size is always 16x16#641
rozmansi wants to merge 1 commit intolxn:masterfrom
rozmansi:pending/default-icon-size

Conversation

@rozmansi
Copy link
Contributor

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.

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.

Reverts: 8f6d762
See-also: lxn#636
Signed-off-by: Simon Rozman <simon@rozman.si>
@rozmansi rozmansi force-pushed the pending/default-icon-size branch from 62dbc90 to 16b7e24 Compare October 31, 2019 08:23
@zx2c4
Copy link
Contributor

zx2c4 commented Oct 31, 2019

Is this a safe assumption to make?

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

@rozmansi
Copy link
Contributor Author

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
Copy link
Contributor

zx2c4 commented Oct 31, 2019

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

@zx2c4
Copy link
Contributor

zx2c4 commented Oct 31, 2019

This seems better: lxn/win#99

@rozmansi
Copy link
Contributor Author

rozmansi commented Nov 1, 2019

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

@rozmansi rozmansi closed this Nov 1, 2019
@rozmansi rozmansi deleted the pending/default-icon-size branch November 1, 2019 06:22
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