Add support for "Icon Packs"

AObuchow commented 2 years ago

This is a continuation/migration of Bug 562174 - Add support for "icon packs" in Eclipse IDE.

It'd be nice for to have an officially supported way to replace the icons used in Eclipse.

There is a lot of discussion to be held on how this should be handled (and there was already a lot of discussion on the original BZ thread), so I'm going to leave this relatively open-ended.

IIRC the approach I had initially tried was:

Allow custom/replacement icons to be provided by a plugin fragment, by placing them in a "theme" directory of the plugin fragment
- So in order to replace one of the "debug" icons, you must make a plugin fragment where the host plugin is "eclipse.debug.ui" and place the icon within a "theme" directory, with the same file path as the original icon
When loading an icon from a plugin, check if the plugin provides an alternative path to the icon that is prefixed by "/themes/"
If so, load it
Otherwise, use the default icon

There are probably some limitations to this approach, for instance:

The icon-provider must create a plugin fragment for each plugin which they wish to replace the icons for. This is very tedious.
How do we handle switching between different icon packs, if each plugin fragment uses the same "themes" directory?

It might be nicer to set some JFace preference which points to a specific plugin path (where the plugin is the icon pack provider plugin), and then alternative icons are checked from that plugin before loading the default icon.

I am going to submit a WIP PR that shows my current implementation, as well as an example plugin fragment that provides icon replacements.

laeubi commented 2 years ago

I think a preference would be a good idea in any case to prevent excessive lookups, even if they are fast they are not for free.

t might be nicer to set some JFace preference which points to a specific plugin path

There is already org.eclipse.e4.ui.css.swt.theme/themeid I think it would be good to reuse that, lets assume the themeid is 'my.custom.theme' jface could simply prefix all icons with that id eg icon/cross.png -> icon/my.custom.theme/cross.png with a fallback to the original icon as the "error icon".

mickaelistria commented 2 years ago

There is already org.eclipse.e4.ui.css.swt.theme/themeid

While it's good to reuse it if possible, I think it's important that this particular issue of "icon packs" can be built-in JFace without dependency of e4 or higher-level platform. Then the integration of icon packs in e4 themes can decide to rely on this themeid, but that discussion is IMO a step further.

laeubi commented 2 years ago

JFace without dependency of e4 or higher-level platform.

JFace don't need to depend on e4, just reading that preferences with that ID, I actually won't care much about that in contrast to inventing another "icon pack id" that then must match the themeId or similar ...

Actually it makes most sense to have a icon pack for a theme than having different icons with the standard theme.

mickaelistria commented 2 years ago

Actually it makes most sense to have a icon pack for a theme than having different icons with the standard theme.

I disagree here. Also SWT/JFace do not apply only to Eclipse IDE nor workbench, there are other apps that may want icon/image theme without dragging the e4 stuff. Let's fix the issue where it is (in JFace) without considering the legacy downstream (e4), that will lead to the best design.

laeubi commented 2 years ago

You don't need to drag in anything. This is just a preference and we can access it in any bundle we like without any dependency. As SWT does not allow for any theming, it might then even be better to not do this in swt/jface but in the eclipse theming framework to handle that case.

BeckerWdf commented 2 years ago

and place the icon within a "theme" directory, with the same file path as the original icon

So this implies only icons that originally come from a file can be replaced? What out other icons (that are e.g. drawn via GC operations) like these:

laeubi commented 2 years ago

What out other icons (that are e.g. drawn via GC operations) like these:

These are obviously not Image-Icons :-)

BeckerWdf commented 2 years ago

Will this new feature allow plugin developers to use different icons for the light and the dark theme? Currently it's quite hard to design icons that have good contrast on light and dark background. So providing alternative icons for the dark theme is a much needed feature.

These are obviously not Image-Icons :-)

So if the answer to the above question is yes: We might also want to provide different versions of these "none-image-icons". What would you propose as as solution for this? Having IFs in the drawing code that check for theme-names or brightness of the background color?

laeubi commented 2 years ago

Will this new feature allow plugin developers to use different icons for the light and the dark theme?

Not in its current form, thats why I requested cahnges and suggested to better integrate this with the theming framework instead of an additional soloution just for SWT.

So if the answer to the above question is yes: We might also want to provide different versions of these "none-image-icons".

none-image-icons are custom painted components as as such are best supporting CSS styling instead of loading alternative images. see: https://wiki.eclipse.org/E4/CSS/SWT_Mapping https://wiki.eclipse.org/Eclipse4/RCP/CSS#Extending_CSS_to_handle_new_widgets

BeckerWdf commented 2 years ago

Not in its current form, thats why I requested changes and suggested to better integrate this with the theming framework instead of an additional solution just for SWT.

👍

mickaelistria commented 2 years ago

Will this new feature allow plugin developers to use different icons for the light and the dark theme?

Ultimately yes, but it will also require change in the theme support once we're done with SWT part. The theme would be able to define an icon set to use and configure SWT; so SWT would then load different images.

becph01 commented 1 month ago

@AObuchow what would it take to get this supported for icon packs? I can look for budget to sponsor this. It's for an RCP application to have a sane light mode/dark mode color scheme.

laeubi commented 1 month ago

See also here for some thoughts how to have "icon theming support":

https://github.com/eclipse-platform/eclipse.platform.images/issues/81

AObuchow commented 1 month ago

@AObuchow what would it take to get this supported for icon packs? I can look for budget to sponsor this. It's for an RCP application to have a sane light mode/dark mode color scheme.

@becph01 I actually think @laeubi would be a much better candidate to sponsor for developing this feature :)

@laeubi hopefully this is something you're allowed to do? I'm not sure if your work has some conflict-of-interest clause regarding sponsored development for the Eclipse IDE?

laeubi commented 1 month ago

@laeubi hopefully this is something you're allowed to do? I'm not sure if your work has some conflict-of-interest clause regarding sponsored development for the Eclipse IDE?

Eclipse Foundation does not limit me regarding other agreements, especially if its in the best interest for the Eclipse IDE itself :-)

eclipse-platform / eclipse.platform.ui

Add support for "Icon Packs" #13