Add zoom level support for program image data

[ptief]

Add zoom level support for Program image data.

[akurtakov]

There is no gtk implementation.

[lshanmug]

Changes look good for Windows and Mac.

On Linux, the zoom passed in the new API is not used, so behavior is same as Program.getImageData(). I'm unable to test on Linux, so not sure if the Program.getImageData() already returns ImageData based on the system scaling. If that's the case, I think we can merge the change as Program.getImageData(zoom) is working as expected. Behavior of Program.getImageData() would be different compared to other platforms, but that would be a different issue. @akurtakov Can you please verify the behavior of Program.getImageData on Linux?

[lshanmug]

Changes look good for Windows and Mac.

On Linux, the zoom passed in the new API is not used, so behavior is same as Program.getImageData(). I'm unable to test on Linux, so not sure if the Program.getImageData() already returns ImageData based on the system scaling. If that's the case, I think we can merge the change as Program.getImageData(zoom) is working as expected. Behavior of Program.getImageData() would be different compared to other platforms, but that would be a different issue. @akurtakov Can you please verify the behavior of Program.getImageData on Linux?

@ptief targeting this for 4.27M1. From the code looks like no changes are required for Linux, but I couldn't confirm it. If required let's track it in a separate issue.

[github-actions]

Test Results

   299 files  ±0     299 suites  ±0   6m 34s ⏱️ + 1m 2s
 4 098 tests ±0   4 090 ✅ ±0   8 💤 ±0  0 ❌ ±0 
12 206 runs  ±0  12 133 ✅ ±0  73 💤 ±0  0 ❌ ±0 

Results for commit 19145ae. ± Comparison against base commit 9fc4460.

♻️ This comment has been updated with latest results.

[lshanmug]

I've verified this with SWT and Platform UI patches on Mac and it fixes the issue.
@deepika-u Can you please test the PR on Windows? Associated Platform UI PR - eclipse-platform/eclipse.platform.ui#295

[deepika-u]

@ptief @lshanmug
I have tried pulling both SWT and Platform UI patches on windows and seeing the behavior as below with my observations summarized::

Observation 1 ::
On windows - 100%, 125%(recommended), 150% and 175% captured section of images.

1. 125% and 150% icons are having some problem(not center/not complete if we observe keenly).
2. browser icon say mozilla icon and zip file kind of icons are getting some black background irrespective of zoom size.

Observation 2 ::
When i checked on the file changes and went through below link => https://learn.microsoft.com/en-us/windows/win32/api/shellapi/nf-shellapi-extracticonexa
They are providing both phiconLarge and phiconSmall but in the fix proposed now with zoom being passed, you are only using phiconSmall. Any specific reason why you are always using phiconSmall?

[iloveeclipse]

Please retarget this for the appropriate milestone in 4.28.

[HeikoKlare]

I have pushed a slight adaptation in c5998de. The program icons delivered by Windows are scaled according to the scale factor of the primary monitor. This is different from other images that are loaded in their original scaling. This has to be represented in zoom factor passed to the getImageData() method in order to retrieve data at proper scaling. With the added change, the program icons are scaled in the same way as other images are scaled, both with "proper" scaling (swt.autoScale=quarter) and with default scaling (swt.autoScale=integer200). This is how it looks like (when applied together with eclipse-platform/eclipse.platform.ui#295

Note that the only difference when not applying eclipse-platform/eclipse.platform.ui/pull/295 is that the images will become more blurry, but they are still present with right scaling.

[HeikoKlare]

@deepika-u could you please check whether your concerns in #409 (comment) have been addressed with the recent changes? The icons should now be sized correctly and visible completely, no matter which scale factor.

Regarding your question concerning usage of phIconLarge instead of phIconSmall: I did some experiments with that but found the "large" icons to even be less sharp than the small ones on 200%, as the "small" ones were already sharp and the larger ones had to be scaled down. My impression was that the Windows API already delivers the "large" image in case the primary monitor has a scale factor of more than 100%. But it may also be that there is still something broken in how the icon is loaded and then rescaled. I would propose to first process this PR (together with the related Platform UI PR) and then investigate the usage of phIconLarge again afterwards.

Since recent changes did only affect the Windows implementation, there is no need to re-verify the effects on Mac.

[deepika-u]

@HeikoKlare

Thanks alot for driving this pr in.

First of all i am happy with this pr now and my vote goes for this fix 👍 , yeah but sorry for my late reply since i tried multiple times across resolutions to be sure of my observations.

Please find my observations at 100%, 125%, 150%, 175%, 200% as below

Without any pr ->

With 409 pr alone, the images are not clear as you said ->

With both prs 409 and eclipse-platform/eclipse.platform.ui#295 applied, images are PRETTY clear and complete ->

So along with this pr expecting eclipse-platform/eclipse.platform.ui#295 to be merged ASAP. Thanks once again.

Least but not last i have a small observation here in the last image when both pr 409 and pr 295 applied case =>
The images from 100%, 125%, 150% of the various icons almost look similar size if you keenly observe(150% icons look slightly shrunk than 125% but definitely look better after the fix). Where as in 175%, 200% icons sizes increase proportionately. Yeah but may be of less importance as of now i feel.

[HeikoKlare]

Thanks a lot for that detailed verification, @deepika-u!

Least but not last i have a small observation here in the last image when both pr 409 and pr 295 applied case =>
The images from 100%, 125%, 150% of the various icons almost look similar size if you keenly observe(150% icons look slightly shrunk than 125% but definitely look better after the fix). Where as in 175%, 200% icons sizes increase proportionately. Yeah but may be of less importance as of now i feel.

That's a nice observation. Unfortunately, that is something we cannot change. The reason is the following: When retrieving an icon via the Windows API, it returns that icon scaled according to the primary monitor scaling, so in your example in 100% (16x16 px), 125% (20x20 px), 150% (24x24 px) and so on. SWT is currently configured to only use 100% and 200% images (due to
swt.autoScale=integer200), so it scales the icons down to 16x16 px or up to 32x32 px. The performed rescaling is what produces the results that differ in quality for different scale values.
In my opinion, the "correct" scaling mode is swt.autoScale=quarter, because then images have the "correct" size (i.e., when scaling down a screenshot of a 150% UI to 100% and compare it to a native 100% UI, the images have the same size). In that case, icons do not need to be resized as they can be drawn with the scaling in which the Windows API provides them. You can see how that looks like in my screenshots in #409 (comment).
I am not sure why swt.autoScale=quarter isn't the default on Windows (maybe it's because there have been several problems with image scaling, but they should have been addressed recently and with this PR). I appreciate input on that and will also start a discussion about that soon.

[merks]

I'm really impressed not only by the work here but also by the collaborative teamwork!

[HeikoKlare]

eclipse-platform/eclipse.platform.ui#295 should be merged right afterwards.