Allow rewriting image URLs via an IImageURLModifier strategy

[vogella]

Icon packs and RCP applications can currently replace platform icons only through the Equinox transforms hook, which needs a framework extension enabled on the command line. This adds IImageURLModifier, a small JFace strategy that URLImageDescriptor asks for the URL to load, while the substitution policy itself stays outside JFace. The E4 workbench installs the highest ranked OSGi service of that type, so an icon pack bundle needs no startup code in the IDE or in E4 RCP applications, and plain JFace applications can call ImageURLModifiers.setURLModifier directly. HiDPI variants are derived from the rewritten URL, so a pack only supplies the base name. Images are not reloaded, so a new pack takes effect after a restart. The types live in the x-friends package org.eclipse.jface.internal.provisional.resource rather than in public API, so they can still change if a better solution comes up.

[github-actions]

Test Results

   864 files  ± 0     864 suites  ±0   53m 2s ⏱️ + 5m 54s
 8 414 tests +11   8 172 ✅ +11  242 💤 ±0  0 ❌ ±0 
21 111 runs  +33  20 434 ✅ +33  677 💤 ±0  0 ❌ ±0 

Results for commit 5737088. ± Comparison against base commit 0074e63.

♻️ This comment has been updated with latest results.

[HannesWell]

Support for Icon Packs is indeed something we need, especially for the great work on Dual Tone Icons happening in

• https://github.com/eclipse-platform/ui-best-practices

It has been discussed for quiet some time and was also again topic at the Eclipse Community Meetup in Sankt-Leon Rot last week. I couldn't participate in that discussion, but as far as I know a proper complete solution was considered relatively complex. AFAIK @HeikoKlare also wanted to write down the greater plan with the complete list of requirements and the discussed approaches to fulfill them.
And I assume @BeckerWdf is also interested in this.

What comes into my mind immediately is that this solution requires icons to use ImageDescriptor. While this is probably the case for many/the most icons, I'm not sure if it's true for all.
This approach is also static and if the pack is changed, e.g. through a preference introduced later, it would require a restart to take effect.
Furthermore it identifies 'original' images based on their URL, but it's not defined which format that URL has (e.g. which schema it uses) and it would make it impossible/very hard to move icon images within a Plug-in because it would break all existing themes.

Maybe, just as an idea, it would be better to introduce a dedicated 'Icon' class that assigns the icon a unique ID and receives the path/URL of the original image (but never exposes it)?
The substitution of icon images could then be based on that ID, which has a semantic. That could also help to better share icons among bundles and would allow us to define icons as API through ordinary code.
Such an Icon class could be designed in a way so that it can track the Widgets it's used in (e.g. by having just an 'applyTo' method and not exposing the actual image), which allow immediate updates of the Icons if the pack is changed at runtime.
Of course that would require to add corresponding constants for all existing icons.

[BeckerWdf]

We wrote down quite a lot in #2870.
@HeikoKlare wanted to add more details to it. AFAIK he's currently out of office this week.

[vogella]

Maybe, just as an idea, it would be better to introduce a dedicated 'Icon' class that assigns the icon a unique ID and receives the path/URL of the original image (but never exposes it)?

If I understand @HannesWell's idea correctly, it would require moving all icon usages in Platform, JDT, PDE, EGit, etc. to a new Icon class, and plugin.xml icon= attributes and E4 model iconURIs would need a new reference form or a bridge. That may be a good long-term goal, but I don't see it happening in the next few years.

Almost every file-based icon in the platform already goes through URLImageDescriptor, including ImageDescriptor.createFromFile, E4 iconURIs and CSS url(). The icons that bypass it are mostly drawn in code, which neither approach would cover.

Regarding restart: #2870 lists a restart as acceptable, and this PR is Layer A of the concept I posted there(#2870 (comment)). Stable icon IDs would also avoid the problem of moved files breaking packs, so an ID-based approach could be layered on top of this hook later.

I suggest marking IImageURLModifier as experimental in the Javadoc. That way we can start using icon sets in the IDE and in RCP applications without being tied to this API if we find a better solution.

[HannesWell]

Maybe, just as an idea, it would be better to introduce a dedicated 'Icon' class that assigns the icon a unique ID and receives the path/URL of the original image (but never exposes it)?

If I understand @HannesWell's idea correctly, it would require moving all icon usages in Platform, JDT, PDE, EGit, etc. to a new Icon class, and plugin.xml icon= attributes and E4 model iconURIs would need a new reference form or a bridge. That may be a good long-term goal, but I don't see it happening in the next few years.

That's correct. But I'm not concerned about any of the SDK projects , projects in the SimRel or in general active projects.
In times of AI such task shouldn't be too difficult. Since it's more or less a complex search and replace and an AI should be able to do that.
What I'm more concerned about is about projects that re not actively maintained anymore or are not willing to embrace a new API.
So eventually we probably need a fall-back that is just based on bundle-name+path or URL/URIs that works like it's done here.
But to me it's not clear how this then fits exactly in the bigger picture. And I think that should be clarified before and we should at least have a plan before starting to work on it.

What's for example missing here is also the ability and a concept for settings so that users can select between different (installed) Icon packs in the preferences.
Furthermore modularization or aggregation of icon-packs from multiple sources (maybe somebody wants to contribute more icons for a 'theme'/Pack) is missing too.
Of course that would also be doable with this approach. But each icon-pack provider would have to find own solutions.
IMO the Eclipse Platform or better Jface should provide that framework and pack providers should really only provide the icons and define what they replace. This then leads also to the question what APIs are necessary and should be available. This currently adds a way accessible to more or less everyone, which might lead to unpleasant results if used incorrectly.
And IIRC JFace, like SWT, can also run in plain Java (non OSGi applications). So we should check if that's a relevant use-case for icon packs too.

I've pushed my current ideas to a branch in my fork, in case you or anybody else wants to have a look:
https://github.com/HannesWell/eclipse.platform.ui/tree/icon-images
But it's more or less just a class with partial implementation, non ready APIs and a lot of notes. So not even worth a draft PR (and I spent less than an hour with it).

Regarding restart: #2870 lists a restart as acceptable, and this PR is Layer A of the concept I posted there(#2870 (comment)).

For example for the case of non cooperative original bundles a restart would probably be inevitable (or at least very difficult or with too much overhead to implement). But with cooperating bundles, I think it's possible and then it would IMO be good to have.

Stable icon IDs would also avoid the problem of moved files breaking packs

Yes, plus representing icons as POJO instances also would make their API status more clear and easier to maintain and thus would also allow reliable reuse.
If we only use instances and they are only used from Java, we could maybe even avoid explicit IDs, but that would probably be problematic for 'textual' references, e.g. in E4 models.

I suggest marking IImageURLModifier as experimental in the Javadoc. That way we can start using icon sets in the IDE and in RCP applications without being tied to this API if we find a better solution.

I don't think this is a good idea. Either it's not used, then it doesn't help to have it now or it's used extensivly and becomes a de-facto API that's again hard to replace.

I suggest we await the fully summary of requirements @HeikoKlare and discuss it also at the next Dev-Call scheduled at October 8th:

• https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/wiki/Eclipse-IDE-%E2%80%90-Developers-Community-Call

Once it's settled can then execute it step by step.

[vogella]

What I'm more concerned about is about projects that re not actively maintained anymore or are not willing to
embrace a new API.

Just to add to this valid feedback that another blocker for consist usage of new API is that most actively maintained Eclipse plug-ins support older releases, e.g. WindowsBuilder currently 2024-06, and Egit also 2 years IIRC.

[ptziegler]

Just to add to this valid feedback that another blocker for consist usage of new API is that most actively maintained Eclipse plug-ins support older releases, e.g. WindowsBuilder currently 2024-06, and Egit also 2 years IIRC.

Just my two cents: Adding new API is fine, as long as the Platform adheres to the two year deprecation policy. WindowBuilder for example would continue to use the "legacy" API for at least the next couple of years and then eventually migrate (together with dropping support for older Eclipse releases) to the new classes.

But as already mentioned, my bigger concern are projects which already on life-support. I've seen it with GEF in the past, where the changes required for proper High-DPI caused a minor disaster for e.g. Graphiti and all projects depending on it and which are always frustrating, for all parties involved.

In times of AI such task shouldn't be too difficult. Since it's more or less a complex search and replace and an AI should be able to do that.

I'm a little careful with statements like that. AI tools (and probably also non-AI tools like OpenRewrite) are definitiely capable of migrating to the new API. But I would still expect a noticable cost, not just from the code change itself but also all the organizational change that comes with such a migration.

Out of curiosity regarding the ImageIcon approach: How would this interact with the ISharedImages? There IDs are also used as substitute for the image URL and it feels like both try to solve the same underlying problem.

[vogella]

I'm already using the icon replacement since a few weeks in my custom IDE build and promissed @BeckerWdf and @HeikoKlare some screenshots in an email conversation, so here there are:

Compare my different example icon sets:

Dualtone:

IntelliJ:

Codicons (VsCode)

[vogella]

See also the summary from @HeikoKlare from the community which IMHO fits well to my PR. #2870

Planning to merge this end of the week so that I can provide a draft for the concept of replacing icons. I use the existing icon replacement implementation from me since a week (in a bit different implementation as @HeikoKlare described, so I need to do some adjustments) and it is very pleasant and so far stable.

[HannesWell]

In times of AI such task shouldn't be too difficult. Since it's more or less a complex search and replace and an AI should be able to do that.

I'm a little careful with statements like that. AI tools (and probably also non-AI tools like OpenRewrite) are definitiely capable of migrating to the new API. But I would still expect a noticable cost, not just from the code change itself but also all the organizational change that comes with such a migration.

Yes it's not free, but I think it's manageable. At least it's simpler instead when being done just manually.

Out of curiosity regarding the ImageIcon approach: How would this interact with the ISharedImages? There IDs are also used as substitute for the image URL and it feels like both try to solve the same underlying problem.

There seems indeed to be some overlap. At this early stage I haven't thought about that yet, but it should be investigated if that path is followed further.

I'm already using the icon replacement since a few weeks in my custom IDE build and promissed some screenshots in an email conversation

That looks nice.

Planning to merge this end of the week so that I can provide a draft for the concept of replacing icons. I use the existing icon replacement implementation from me since a week (in a bit different implementation as @HeikoKlare described, so I need to do some adjustments) and it is very pleasant and so far stable.

Why does this have to be merged before you can propose that concept? Can't this be done in one PR (if suitable with multiple commits), so that others can see the bigger picture too and discuss it?

As said before, with the knowledge I have currently I think the low level API you are proposing now is not suitable!
And we should avoid back and forth.
Therefore I think this should not be submitted in the current state.

For the dev-call tomorrow afternoon (CET), I plan to discuss the Icon-Pack topic too, it would be nice if you'd join.

[vogella]

Why does this have to be merged before you can propose that concept? Can't this be done in one PR (if suitable with multiple commits), so that others can see the bigger picture too and discuss it?

The concept is proposed since 10.04.2026 in #2870 and regular adjusted to the adjusted requirements. It did not see a lot of feedback. Doing the full blown implementation before agreeing on the underlying approach is IMHO not efficient. But I can later provide a code snippet or demo DRAFT to show my solution in action. It is relatively simple to use with my proposal.

As said before, with the knowledge I have currently I think the low level API you are proposing now is not suitable! And we should avoid back and forth. Therefore I think this should not be submitted in the current state.

I looked at your proposal and think is relatively complex and also hard to use in a plan SWT / JFace application without OSGi, which IMHO is also a very important scenario.

For the dev-call tomorrow afternoon (CET), I plan to discuss the Icon-Pack topic too, it would be nice if you'd join.

I try to join. I can also demo or discuss my solution proposal if desired.

[vogella]

Here is a snippet to use my solution proposal:

@Component
public class MyIconPack implements IImageURLModifier {
      public URL modifyURL(URL url) {
              return getClass().getResource("/pack" + url.getPath().substring(url.getPath().lastIndexOf('/')));
      }
}

"pack" would be a folder in your RCP application using the same image names as the "real"images. Obviously that is hard coded in the above example, but I assume it demos the usage quite nicely. Let me know if you have questions.

The logic of course would need to be extended to support the desired scenarios from #2870

Outside OSGi you call setURLModifier yourself in main(), before the first image is created, and put the icon folder on the classpath:

public class MyApp {
      public static void main(String[] args) {
              ImageDescriptor.setURLModifier(url -> MyApp.class.getResource(
                              "/pack" + url.getPath().substring(url.getPath().lastIndexOf('/'))));

              Display display = new Display();
              ApplicationWindow window = new ApplicationWindow(null);
              window.setBlockOnOpen(true);
              window.open();
              display.dispose();
      }
}

[vogella]

See #4456 for a demo

[vogella]

My notes from the community discussion: We should make the IImageURLModifier x-internal or x-friends before continuing and @HannesWell additional / alternative approach fits on top of it. @HeikoKlare plans to test in his IDE with a simple icon replacement rule.

@HannesWell and @HeikoKlare please correct the above if I misphrased or misunderstood.

[laeubi]

I like to leave some general remarks here, see code comments. I think the idea is a good starting-point but some details and architectural points need to be refined.

[laeubi]

This javadoc seems misleading it is not necessarily set form an OSGi service event.

[laeubi]

I think @ruspl-afed would say we should not add any (provisional) API that returns null for missing and instead use

Suggested change

	public static IImageURLModifier getURLModifier() {
	public static Optional<IImageURLModifier> getURLModifier() {

[laeubi]

Should we allow this to change anytime during the lifecycle or go a similar way like URL Protocol factories does that is you can only set a value if it is currently unset / null and get an exception otherwise. In the current from it can happen that any plugin is messing around and you get mixed results depending on when it is called.

[laeubi]

In general I wonder if this whole logic is not better placed at URLImageDecriptor and marking the methods as @Provisional - this would even avoid having a getter at all and it seems to logically belongs there.

[laeubi]

This is already heavy nested call site here and the null case fall through effectively becomes cumbersome here. I would suggest to simply do

Suggested change

	URL result = toURL(urlString);
	URL result;
	try {
	result = new URL(urlString);
	} catch (MalformedURLException e) {
	Policy.logException(e);
	return null;
	}

[laeubi]

Similar here

Suggested change

	if (modified != null) {
	result = modified;
	}
	if (modified != null) {
	return modified;
	}

[laeubi]

This approach looks fragile to me and misplaced, so we have a way to set IImageURLModifier that then calls a simple static setting that a contributor can set directly anyways. It also seems not very suitable to either agree on "Tracked because the contributing bundle may start after the workbench" and on the other hand then possibly icons are already loaded and you get a mixture. Further if even the bundle goes away and then "best" ist installed the same issue arise.

Also I see some issues here as installBest does not check for null return of getService so the "best" might already be gone at this point and one has to chose another "better" one. Because of such subtile problems, its usually better not trying to be smart and implement such OSGi low-level things and better use ServiceTracker instead (what has getService() to always return the highest ranked tracked service). Additionally, this will blindly overwrite any possibly set modifier (e.g. by client code) - see my comments above for making this only allow one over the lifespan of the application.

So I would propose the following:

1. If we track an service use ServiceTracker and install a proxy IImageURLModifier that delegates the actual calls. This would then even allow a kind of delegation model e.g. if one IImageURLModifier can not supply an icon just ask the next (lower ranked) one.
2. In the context of icon-packs, I would more see this as a dedicated bundle (e.g. org.eclipse.e4.ui.workbench.swt.iconpacks) that then do not trac bare IImageURLModifier but offers a higher level concept (e.g. using an extension point where I can declare icon packs that are then possibly static (e.g. simple rewrite pattern) or dynamic (e.g. possibly what @HannesWell has proposed with a dedicated class) and carry additional metadata (e.g. a pack-name so multiple bundles are grouped together and can be referenced in css or alike)
3. Then if there is a demand for not using the platform approach at all, one can simply omit this bundle (or did not start it or ...) and implement an own full replacement.

This would prevent the ambiguity of "started later" as extension points do not require a service to be registered and they can be lazy (e.g. an not choose pack will never gets loaded / activated) and prevent people fiddle around with the raw JFace API, further one even don't need to install this new bundle at the beginning, an icon-pack can then just declare a dependency (like we did for svg already) to an iconpack capability and the necessary bundle will then automatically installed once I install the first icon pack - it would even allow a third party to supply an alternative implementation for even this part of the system.