Use large program icons on Windows

[HeikoKlare]

Icons for files associated with an external program area always loaded in small size using the Windows API. The API also provides an option to load large icons, which have twice the size of the small ones.

In order to improve the icon sharpness when retrieving the icon for (monitor) scale value of more than 100%, this change ensures that the higher resolution icon is used and scaled appropriately.

This is a follow-up to #409, in which proper scaling support for icons was first added. The missing retrieval of large icons has been identified there (#409 (comment)) and deferred to follow-up work (#409 (comment)).

Example at 150% scaling (swt.autoScale=quarter) before (left) and after (right) this PR.

Example at 225% scaling (swt.autoScale=quarter) before (left) and after (right) this PR.

[HeikoKlare]

@deepika-u fyi, since you first pointed to the missing usage of large icons in #409 (comment).

[github-actions]

Test Results

   299 files  ±0     299 suites  ±0   7m 13s ⏱️ + 1m 17s
 4 100 tests ±0   4 092 ✅ ±0   8 💤 ±0  0 ❌ ±0 
12 212 runs  ±0  12 137 ✅ ±0  75 💤 ±0  0 ❌ ±0 

Results for commit b1ddf05. ± Comparison against base commit 91b51cb.

♻️ This comment has been updated with latest results.

[deepika-u]

@HeikoKlare
Let me check and update you.

[deepika-u]

@HeikoKlare Let me check and update you.

@HeikoKlare
Sorry for my late update but i am not able to test your patch as such.

My master as of now is upto date and swt workspace works fine.
Without your pr, i am able to launch the child eclipse and see icons with 100% and 125% like this ->

But after applying your pr, my workspace starts reporting all these problems

Child workspace doesnt launch atall and creates a .log file which i have attached for reference here ->
.log

Now to resume old state of workspace, i need to delete whole windows fragment and reimport again, set .classpath for windows and do a clean. I am good to go again(sometimes though some errors are there i am able to proceed sometimes no errors atall. Behavior all the times is not the same). Not sure what is the problem behind this.

Can anyone else test this pr please? are they successful?

[HeikoKlare]

Thanks for trying @deepika-u!
This PR was still based on the state without #1008, which changed the way SWT fragments are built. I guess that produced the problems in your workspace. I have updated the PR to be based on latest master commit. If you pull the PR again, you should not have the problems you had before. Maybe you want to try again?

[deepika-u]

Thanks for trying @deepika-u! This PR was still based on the state without #1008, which changed the way SWT fragments are built. I guess that produced the problems in your workspace. I have updated the PR to be based on latest master commit. If you pull the PR again, you should not have the problems you had before. Maybe you want to try again?

@HeikoKlare
Now at 125%, after your new commit is added my child workspace is launching. Let me test and update you, thanks.

[deepika-u]

Thanks for trying @deepika-u! This PR was still based on the state without #1008, which changed the way SWT fragments are built. I guess that produced the problems in your workspace. I have updated the PR to be based on latest master commit. If you pull the PR again, you should not have the problems you had before. Maybe you want to try again?

@HeikoKlare
I would say NO for this patch because i see the icons are broken at 200%. I have tried 100%, 125%, 150%, 175% and 200%.

Without the patch

After the pr applied

The icons at 200% look broken (but yeah icons at 175% look good after the patch are more clear) and they are better without the patch so.

The overall impact at 200% is making me say a "NO" to this fix. But yeah please do let me know if you have further comments on my observations.

[HeikoKlare]

Thanks for that verification, @deepika-u! I agree that based your observations we should not merge this PR as is.
In order to improve the PR or to persist the knowledge why it should never be applied, I investigated the reasons for the observation:

The Windows operation ExtractIconEx, which is used to retrieve the icon, delivers the icon in a size according to the primary monitor scale factor. So for a usual icon size of 16x16 px, having primary monitor scaling at 150% will make the method return a 24x24 px image. This already applies some kind of "proper" scaling rather than simply upscaling a lower-resolution image. Take a look at the following image. The first icon is the 100% (16x16) icon scaled up to 200%. The second is the one the OS returns as a "small" icon when primary monitor scaling is set to 200%. That is definitely not a scaled-up version of the 16x16 px icon, in particular because in that case the edge of the paper within the icon would have a line width of 2 px rather than 1 px.

The large icon retrieved via ExtractIconEx is 32x32 px scaled up to the primary monitor scaling. So for primary monitor scaling of 200%, we receive a 64x64 px image. This will then be scaled down to the required 32x32 px again, which results in the loss of information, such as the top and bottom edge of the paper in the PPT icon. This can also be seen when setting the SWT autoscale method to "smooth", as then the border of the paper in the PPT icon becomes half gray.

So in summary, retrieving the large icon is not necessary or even bad when the small one will already fit the needs without rescaling. Still, it is relevant when the icon is required with higher scaling than the native one of the primary monitor. This will be the case once we support adapting the window scaling when moving it to a monitor with a different scale value. For example, starting the application on the primary monitor with 100% scaling and then moving the window to a monitor with 200% scaling, it makes sense to then use the large icon as the one delivered by ExtractIconEx is properly scaled for the 100% primary monitor, whereas the large one will be appropriate for the other HiDPI monitor. But even now, the package explorer retrieves a 200% icon (and then performs a custom downscaling) when the primary monitor is, e.g., at 150%. That's why the results with 125%--175% look better with this PR. That why I adapted the PR in c3e52f3 to only use large icons in case the requested zoom exceeds the one of the primary monitor. This will particularly resolve the issue with 200% scaling, which now looks like this:

[HeikoKlare]

I close this PR for now as I did not find a proper way to consider large program icons when it makes sense to. The reason is that at those places where program icons are used (within the Eclipse IDE product), they are always retrieved at 200% scaling and then scaled to the required value by the consumer (in particular, Java element image descriptors). So unless the consumers retrieve the icon in the scaling they really require, there will be two many cases in which the icon is badly, see, e.g., #1014 (comment)

Still, I updated the PR to reflect the recent changes of #1138 to be able to revive the PR if required, e.g., for providing proper/sharp icons in multi-monitor HiDPI setups (like introduced with #1064 and follow-ups).