Program icon scaling using DPI #1804
Conversation
There was a problem hiding this comment.
As we have discussed, this seem to be the "right" short-term solution. I want to point out that we plan to work on replacing the call to SHGetFileInfo in order to get rid of the returned icons being scaled according to initial primary monitor zoom.
Can you please adapt the commit message to state that this will actually fix
#1759 (as the usage of display causing the reported issue is completely removed)?
bundles/org.eclipse.swt/Eclipse SWT Program/win32/org/eclipse/swt/program/Program.java
Outdated
Show resolved
Hide resolved
amartya4256
force-pushed
the
amartya4256/program_icon_scaling_using_dpi
branch
from
6297c78 to
869fa19
Compare
Adapted both in the PR description and the commit message |
amartya4256
force-pushed
the
amartya4256/program_icon_scaling_using_dpi
branch
from
869fa19 to
5ad58b9
Compare
HeikoKlare
force-pushed
the
amartya4256/program_icon_scaling_using_dpi
branch
from
5ad58b9 to
a1ac77d
Compare
| @@ -427,6 +427,13 @@ public ImageData getImageData (int zoom) { | |||
| return imageData; | |||
| } | |||
|
|
|||
| private int getSystemWideDPI() { | |||
There was a problem hiding this comment.
..AtStartup ? This value doesn't change. Thats why we need to use it
There was a problem hiding this comment.
I thought so but SystemWideDPI is, I reckon implicable for System DPI-aware value. At least that's what I found by definition here: https://learn.microsoft.com/en-us/windows/win32/hidpi/high-dpi-desktop-application-development-on-windows#system-dpi-awareness
There was a problem hiding this comment.
My understanding is that the value we use here is equivalent to the DPI value when using "system DPI awareness", but we need to be aware that the we use here is the same throughout application lifecycle even when not using system DPI awareness, isn't it? I usually execute my runtime application directly with PerMonitorV2 and still this value is static.
Maybe we should not refer to any such predefined name but could instead also put the DPIUtil.mapDpiToZoom() call inside this method an make the method do some getPrimaryMonitorZoomAtStartup() and keep the documentation that SHGetFileInfo returns the icon scaled according to primary monitor zoom at startup? Then at least the given information is consistent and can be easily validated by anyone trying to understand the code.
There was a problem hiding this comment.
Fair enough. On it.
There was a problem hiding this comment.
Do you agree with this, @akoch-yatta?
There was a problem hiding this comment.
Yes, sure. I think, that should give the right expectation, what the method returns
amartya4256
force-pushed
the
amartya4256/program_icon_scaling_using_dpi
branch
from
a1ac77d to
2756354
Compare
HeikoKlare
force-pushed
the
amartya4256/program_icon_scaling_using_dpi
branch
from
2756354 to
d684b15
Compare
This commit contributes to proper scaling of Program icons. Since these icons are retrieved by OS.SHGetFileInfo which is System DPI-Aware, it returns the icon with zoom level of the primary monitor at application startup. Hence, for initialNativeZoom of the Image object to be created for the icon, the zoom retrieved from the DPI is considered. Contributes to eclipse-platform#62 and eclipse-platform#127 Fixes eclipse-platform#1759
HeikoKlare
force-pushed
the
amartya4256/program_icon_scaling_using_dpi
branch
from
d684b15 to
d7120e1
Compare
This PR contributes to proper scaling of Program icon. Since, these icons are retrieved by OS.SHGetFileInfo which is System DPI-Aware, it returns the icon with zoom level of the primary monitor at application startup. Hence, for initialNativeZoom of the Image object to be created for the icon, the zoom retrieved from the DPI is considered.
contributes to #62 and #127
fixes #1759