Unify and improve image disablement calculation algorithm

[HeikoKlare]

Each OS-specific implementation of the Image class has its own implementation of an algorithm to calculate a disabled representation of a given image. In addition, the algorithm used on Windows and MacOS produces bad results, which is why usually pre-generated disabled icons are used.

This change centralizes the implementations of those algorithms in ImageColorTransformers. The implementations used for the pre-generated disabled icons is added as default algorithm with an enhanced and the existing GTK-conforming algorithms being provided as options. It is supposed to render the usage of pre-generated disabled version of icons obsolete in many cases.

This is based on the snippet for testing different algorithms proposed by @killingerm and the according discussion in #1741

How to test

Most icons Eclipse are provided in a pre-generated, disabled version. To completely disable this and enforce all icons to be generated on-the-fly via the disablement algorithm, you can change the value of IWorkbenchRegistryConstants#ATT_DISABLEDICON so that the according attribute defining the disabled icon path does not match.

The following screenshot shows how the three version of icons look like:

• Top: pre-generated icons and icons generated by proposed on-the-fly algorithm
• Middle: icons with the proposed enhanced on-the-fly algorithm
• Bottom: icons with the current on-the-fly algorithm (to be removed)

[github-actions]

Test Results

   545 files  ±0     545 suites  ±0   31m 34s ⏱️ + 4m 21s
 4 373 tests ±0   4 361 ✅ ±0   12 💤 ±0  0 ❌ ±0 
16 634 runs  ±0  16 520 ✅ ±0  114 💤 ±0  0 ❌ ±0 

Results for commit 677dd56. ± Comparison against base commit 7cc858e.

♻️ This comment has been updated with latest results.

[HannesWell]

Thanks for this PR, I really like the result of the proposed on-the-fly algorithm, i.e. the second version in your screenshots.

Below I have written some thoughts about the code.

[HannesWell]

As already written I really like the enhanced on-the-fly algorithm, i.e. the second version in your screenshots.
If I understand this PR correctly this is only applied for Windows and Mac. Wouldn't it make sense to use it on Linux as well for consistency?

Besides that this looks good generally. Thanks to everyone involved.

[HeikoKlare]

If I understand this PR correctly this is only applied for Windows and Mac. Wouldn't it make sense to use it on Linux as well for consistency?

We can of course do it for Linux as well. I didn't do this as Linux uses an adapted algorithm (fitting with what GTK does) anyway. But for the sake of being/becoming consistent again, we could unify the algorithm across all OSes, as we keep the previous algorithms in a legacy mode anyway. I have adapted the PR accordingly.

[HeikoKlare]

@HannesWell Can you please have another look at this?
I have now adapted the PR as we discussed: By default an image disablement algorithm will be used that conforms with the algorithm used for the existing pre-generated disabled icons. This will allow us to simply remove all the disabled icons and use the on-the-fly algorithm instead. Via a system property, the enhanced mode with the new proposed disabled icon styling can be activated and with gtk a mode conforming to the GTK-like disablement implemented for Linux will be used. These will make most sense when no using pre-generated disabled icons at all (i.e., if we introduced a mode in Platform UI that avoids the usage of explicit disabled icons), as otherwise there will be a mixture of differently styled disabled icons in the UI.

[HannesWell]

Thanks for the update. All what you have described makes sense to me and I'm generally fine with this change and look forward to it. I didn't exactly follow the algorithm changes in all three Image implementations, but I trust your change.

I just have a remark regarding the naming of the values.

[HeikoKlare]

All comments have been resolved. If no one objects on this, I plan to merge this by end of today.

[HannesWell]

Thank you, looks good to me.

[HeikoKlare]

Thank you, @HannesWell!
I have just finally tested all configurations on all OSes and found that the existing algorithm for Linux is kind of wrong. More precisely, it ignores the fact that alpha values are also encoded in the RGB values. This led to wrong results, in particular making the icon background (which is supposed to be transparent) non-transparent. I have adapted the algorithm to properly reflect that fact. It then produces the expected results for all configurations also on Linux.

[HannesWell]

Sounds good. Thanks for finding it.

[HeikoKlare]

There was a successful Jenkins build without any new isues for this PR yesterday: https://ci.eclipse.org/releng/job/eclipse.platform.swt/job/PR-1936/7/
And nothing but an added comment and rebases on master changed since back then.

Thus merging despite Jenkins build failing due to https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/issues/5893

[HeikoKlare]

Since Jenkins build failed for the PR: the subsequent master build after merging was successful has no new issues (https://ci.eclipse.org/releng/job/eclipse.platform.swt/job/master/1108/)