[JEWEL-1076, IJPL-176416] New Icons API

[jakub-senohrabek]

This PR adds new icons api that allows cross-api icons (compose, swing), and icons serialization. The implementation is separated from the API (and the rendering part from data part) to allow creation of Icons without having to depend on UI code. (useful when creating icons on backend)

Make sure to start with intellij.platform.icons.api module, which contains the core api. (also check the rendering submodule). Afterwards, check the impl module (start with intellij submodule) and jewel implementation. (changed jewel modules)

⚠️ Do not merge this in usual way, this is already in ultimate branch ⚠️

GitOrigin-RevId: 32b987b2a7bba316208aefd998a523f6a74ace30

[develar]

We do not have configured codex bot here to review, right ;)?

[jreznot]

Do we expect that every plugin requiring it will need to add explicit dependency? I believe it must be exposed transitively from some platform modules

[jreznot]

Does it require constant opt-ins in every usage? May be plain @Experimental instead?

[develar]

Why does this change have status DRAFT? Is it ready for review?

[rock3r]

As we talked about offline, I’m adding this here mostly to document that conversation in the review.

A broader architectural concern I still have is how this is supposed to work cleanly in Jewel standalone, which is a real target for this API and not optional.

Right now it still feels like some important parts of icon resource resolution are effectively trapped inside the IntelliJ Platform icon-loading stack (IconLoader / util-ui/icons), especially around path transformation / mapping / variant selection. That makes me nervous, because I don’t think we want either of these outcomes:

• Jewel standalone depending directly on the full IJPL icon-loading implementation
• or duplicating the same path-mapping logic in a separate standalone stack

Given that standalone support is a hard requirement, would it make sense to explicitly extract the resource-path resolution part into a smaller backend-neutral module that both IJPL and Jewel standalone can depend on?

By “path resolution” I mean the logic that takes a logical icon path and environment/context and produces the effective resource candidates / transformations (e.g. theme/UI variants, size-specialized variants, any other canonical path mapping rules). The actual loading/caching/rendering backends can still remain platform-specific, but the mapping rules themselves seem like they want to be shared.

Otherwise I worry we end up with an awkward split where:

• the icon model is portable
• the rendering backends are separate
• but the resource resolution semantics are still IntelliJ-specific and therefore either unavailable or reimplemented differently in standalone

That feels like exactly the sort of thing that will cause subtle divergence over time. Since standalone support is non-negotiable here, extracting that path-resolution layer seems like the cleaner long-term shape.

And to be explicit: I do think this is part of the implementation work required for this feature, and it will need to be addressed in this PR before merge.

[rock3r]

I think this can break:

• InPlaceDeferredIconResolver.resolve() returns immediately when resolvedIcon != null
• But DefaultDeferredIconRenderer only flips from placeholder/null to the resolved renderer in whenDone()
• New renderers start with isDone = false, renderer = icon.placeholder?.createRenderer(...)

So if a renderer is created after the deferred icon was already resolved, it never gets whenDone(), and resolve() now short-circuits without re-notifying listeners. That means the new renderer can stay stuck on the placeholder/ null forever.

There’s a second broken path here too:
• DeferredIconResolverService.getOrCreateDeferredIcon() returns resolver?.deferredIcon?.get() ?: DefaultDeferredIcon(identifier, placeholder)
• If the resolver is still in the map but its weak ref was cleared, callers get a fresh DefaultDeferredIcon that is not connected to the existing resolver state

[rock3r]

This seems unused, is it intended?

[rock3r]

This is only computed once, is it intended? I would imagine this could break with animated or deferred icons if the dimensions change (e.g., the placeholder has a different size from the final icon)

[rock3r]

Just noting — this TODO will need to be addressed, or it'll break rendering animated/deferred icons in Swing

[rock3r]

That’s not how javax.swing.Icon usually behaves. A Swing icon should paint into its own icon bounds, not expand to the host component’s full size. In things like buttons/labels with text, this risks stretching the icon to the component size instead of respecting getIconWidth()/getIconHeight().

Local repro from the playground: this is the same row(spacing = 25.percent) { ... } icon rendered through toSwingIcon() under two different host sizes.

What’s striking is that the icon’s internal layout changes based on the available host width: instead of behaving like a small icon composed of two children plus spacing, it gets horizontally redistributed/squeezed as the host gets wider. That strongly suggests the renderer is resolving layout against the surrounding component bounds rather than a stable icon canvas / getIconWidth() × getIconHeight().

So this does not just look theoretically “non-Swing-like” — it produces visibly wrong results for bounds-relative/layout-based icons in practice.

[rock3r]

This can produce negative width / height when the margins exceed the available parent bounds.

Do we have a defined contract for that? If negative bounds are allowed to leak further down the pipeline, Compose can crash on them during draw. It seems like this path should either clamp to zero or validate/reject impossible layouts earlier.

[rock3r]

I think this may be missing an update/repaint notification.

When the wrapped legacy DeferredIcon finishes, this path updates its cached image, but I don't see it notifying the new icon renderer/frontend via renderingContext.updateFlow.triggerUpdate() (or equivalent). If so, the visual update would depend on some unrelated repaint happening later.

That seems especially risky for the migration/interop path, because javax.swing.Icon.toNewIcon() can wrap exactly these old deferred icons.

[rock3r]

I don't think x / y should be affecting the backing-layer dimensions here.

In Swing, paintIcon(c, g, x, y) uses x/y as the paint offset, not as a reduction in the icon's available size. Painting the same icon at (0, 0) vs (10, 10) should translate it, not shrink/crop the backing buffer by 10 pixels in each dimension.

As written, this seems likely to produce clipped/incorrect output for icons painted at non-zero coordinates.

[rock3r]

Is this a bug? The extension point says custom providers are supposed to implement createRenderer(...), but this path checks extension.handles(layer) and then unconditionally returns SwingIconLayerRenderer(...) instead of delegating to extension.createRenderer(...).

If I am reading this correctly, custom renderer providers can never actually provide their own renderer implementation here.

[rock3r]

spacing seems to be missing from both equals() and hashCode() here.

That means two layout layers that differ only by spacing compare equal, even though spacing materially affects rendering. Given how much this new icon model seems to rely on value-like model objects, caching/reuse, and caller-side remembering, this looks like a real correctness bug rather than just an implementation detail.

[rock3r]

I think this PR still needs much stronger test coverage before merge.

The new icon system is no longer just "paint an image"; it is now a compositional tree with layout, sizing, modifiers, deferred resolution, animation, and toolkit-specific update/repaint behavior. That means a lot of correctness now lives in the alignment between measurement and rendering, and that is exactly where I would expect subtle regressions.

I only found a very basic smoke test that renders a new icon to a BufferedImage, but I could not find dedicated coverage for the cases that seem most failure-prone:

• bounds propagation through nested icon/layout trees
• percentage-based sizing relative to parent bounds
• measurement vs rendering consistency for row / column / box
• spacing / alignment / margin interactions in nested layouts
• placeholder vs resolved deferred-icon size changes
• animation frame sizing / bounds stability
• nested deferred + animated update propagation
• Compose-side invalidation/repaint behavior for deferred/animated icons
• Swing-hosted legacy/custom layers participating correctly in sizing and repaint

Even if some of these behaviors are intentionally unsupported, I think that contract still needs to be made explicit and then locked down with tests. Right now the absence of coverage here makes me pretty nervous, because these are exactly the kinds of issues that tend to look fine in simple demos and then break in real UI usage.

My strong suggestion would be:

• add unit tests for layout/measurement/modifier semantics wherever possible
• add Compose UI tests for repaint/invalidation behavior on the Jewel side
• add at least a few Swing integration tests for toSwingIcon() / legacy-layer interop / deferred repaint behavior

[rock3r]

PS I have a few tweaks/samples to propose that I referred to in the review public/jakub.senohrabek/new-icons-api...rock3r:sebp/new-icons-tweaks