Skip to content

Conversation

killingerm
Copy link
Contributor

@killingerm killingerm commented Jan 20, 2025

This draft includes Snippet384 to inspect the behavior of the current icon-disabling algorithms implemented in the constructor of org.eclipse.swt.graphics.Image(Device, Image, int) when called with the SWT.IMAGE_DISABLE flag.

Currently, there are two different algorithms depending on whether Windows/MacOS or Linux is used. The Windows/MacOS implementation is outdated, and the generated disabled icons often look unappealing.

The snippet includes two new algorithms that can serve as a foundation for deriving a new algorithm, with adjustable parameters. This unified approach could also be applied to the Eclipse icons provided in disabled state, ensuring consistent appearance across all disabled icons, whether included in Eclipse JARs or generated dynamically using the Image constructor.

How to Use

  1. Select a folder containing images.
  2. Adjust the sliders to modify the parameters of the new algorithms.

Example Images

I have included example images that demonstrate the weaknesses of the current algorithms and created a resource folder:
resources/Snippet384/Example Images.

For detailed information and to participate in discussions about changes to the algorithm, please refer to the accompanying discussion thread.

Copy link
Contributor

github-actions bot commented Apr 23, 2025

Test Results

   545 files  + 6     545 suites  +6   27m 10s ⏱️ - 9m 7s
 4 377 tests +37   4 359 ✅ +35   18 💤 +3  0 ❌  - 1 
16 647 runs  +37  16 506 ✅ +35  141 💤 +3  0 ❌  - 1 

Results for commit 6463e78. ± Comparison against base commit 995c9b5.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@HeikoKlare HeikoKlare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@killingerm I think it would be nice to have this snippet in the repository, even though we already have multiple algorithms implemented productively now, as it allows for easier testing of alternatives and reconfigurations.

To do so, would you please squash your commits into one and sign the Eclipse Contributor Agreement (ECA)?

In addition, we need to link the new snippet in the Snippets.md file at the root of the org.eclipse.swt.snippets bundle. I would propose to add this snippet to the Image section of that markdown file.

@akurtakov
Copy link
Member

@killingerm You have to sign Eclipse contributor agreement in order for your PR to be reviewed - https://www.eclipse.org/legal/eca/

@killingerm
Copy link
Contributor Author

@akurtakov I signed the ECA long ago but then changed my GitHub username to killigerm (to better reflect my name). I tried to relink it to my eclipse account but got a notification to send a mail to correct it, so far I sent two of them. I've also tried the "Eclipse ECA Validation" plugin. Is there anything else I can try?

@akurtakov
Copy link
Member

I'm deferring to @eclipsewebmaster as I can't help with this one.

@killingerm
Copy link
Contributor Author

Thank you @akurtakov, @HeikoKlare and @eclipsewebmaster, it should work now.

@HeikoKlare
Copy link
Contributor

Thank you, @killingerm! The ECA validation is fine now. Can you please address the review comments so that we can merge afterwards?

@killingerm
Copy link
Contributor Author

Hi again,
@HeikoKlare I have now applied the requested changes:

  • Linked the Disabled Icon Viewer Snippet (Snippet 348) in Snippets.md under the image section
  • Added Javadoc describing the Snippets functionality
  • Squashed everything into a single commit

I believe it has worked, though I'm not entirely sure whether the .prefs and .project files are supposed to be in the commit.

Also, since a new icon-disablement algorithm was selected I made a few adjustments:

  • Added the new disabling algorithm directly underneith the original images
  • Changed the order of algorithms with priority current -> old -> experimental
  • Updated sliders default values to the ones agreed on in the discussion thread
  • Replaced the preview image:

Snippet384

Please have a look at it and let me know if everything is in order this way or further changes are needed!

Lastly I'm really glad the snippet was helpful and is added into the repository, thank you!

@HeikoKlare HeikoKlare marked this pull request as ready for review May 9, 2025 13:48
Copy link
Contributor

@HeikoKlare HeikoKlare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the update! I did some minor changes. With them the PR is ready for me:

  • Removed faulty additions of .project and preferences files
  • Adapted snippet headline to be shown in SnippetExplorer
  • Replaced the squashed commit message with a proper one.

This adds Snippet384 to inspect the behavior of different different
algorithms for the transformations of images to be used as disabled
versions.
@HeikoKlare HeikoKlare merged commit f498ca3 into eclipse-platform:master May 12, 2025
17 checks passed
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.

3 participants