[JEWEL] IJPL-176416 Compose icon infrastructure #3217

jakub-senohrabek · 2025-09-03T19:05:03Z

No description provided.

…e demo, icon id and state, reuse texture for scaling

rock3r

I have a bunch of notes, but at a high level, I don't think this design actually achieves the goals of interoperability and unifying processing between Swing and Compose. It's also missing a Compose standalone implementation, which is where things probably will break down even further.

rock3r · 2025-09-10T07:25:29Z

.idea/runConfigurations/IDEA.xml

    <module name="intellij.idea.community.main" />
    <shortenClasspath name="ARGS_FILE" />
-    <option name="VM_PARAMETERS" value="-Xmx2g -XX:ReservedCodeCacheSize=240m -XX:SoftRefLRUPolicyMSPerMB=50 -XX:MaxJavaStackTraceDepth=10000 -ea -Dsun.io.useCanonCaches=false -Dapple.laf.useScreenMenuBar=true -Dsun.awt.disablegrab=true -Didea.jre.check=true -Didea.is.internal=true -Didea.debug.mode=true -Djdk.attach.allowAttachSelf -Dfus.internal.test.mode=true -Dkotlinx.coroutines.debug=off -Djdk.module.illegalAccess.silent=true -Didea.config.path=../config/idea -Didea.system.path=../system/idea -Didea.initially.ask.config=true --add-opens=java.base/java.io=ALL-UNNAMED --add-opens=java.base/java.lang.reflect=ALL-UNNAMED --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.nio.charset=ALL-UNNAMED --add-opens=java.base/java.text=ALL-UNNAMED --add-opens=java.base/java.time=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens=java.base/java.util.concurrent=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.base/jdk.internal.vm=ALL-UNNAMED --add-opens=java.base/sun.nio.ch=ALL-UNNAMED --add-opens=java.desktop/com.apple.eawt.event=ALL-UNNAMED --add-opens=java.desktop/com.apple.eawt=ALL-UNNAMED --add-opens=java.desktop/com.apple.laf=ALL-UNNAMED --add-opens=java.desktop/com.sun.java.swing.plaf.gtk=ALL-UNNAMED --add-opens=java.desktop/java.awt.dnd.peer=ALL-UNNAMED --add-opens=java.desktop/java.awt.event=ALL-UNNAMED --add-opens=java.desktop/java.awt.image=ALL-UNNAMED --add-opens=java.desktop/java.awt.peer=ALL-UNNAMED --add-opens=java.desktop/java.awt=ALL-UNNAMED --add-opens=java.desktop/javax.swing.plaf.basic=ALL-UNNAMED --add-opens=java.desktop/javax.swing.text=ALL-UNNAMED --add-opens=java.desktop/javax.swing.text.html=ALL-UNNAMED  --add-opens=java.desktop/javax.swing.text.html.parser=ALL-UNNAMED --add-opens=java.desktop/javax.swing=ALL-UNNAMED --add-opens=java.desktop/sun.awt.X11=ALL-UNNAMED --add-opens=java.desktop/sun.awt.datatransfer=ALL-UNNAMED --add-opens=java.desktop/sun.awt.image=ALL-UNNAMED --add-opens=java.desktop/sun.awt.windows=ALL-UNNAMED --add-opens=java.desktop/sun.awt=ALL-UNNAMED --add-opens=java.desktop/sun.font=ALL-UNNAMED --add-opens=java.desktop/sun.java2d=ALL-UNNAMED --add-opens=java.desktop/sun.lwawt.macosx=ALL-UNNAMED --add-opens=java.desktop/sun.lwawt=ALL-UNNAMED --add-opens=java.desktop/sun.swing=ALL-UNNAMED --add-opens=jdk.attach/sun.tools.attach=ALL-UNNAMED --add-opens=jdk.internal.jvmstat/sun.jvmstat.monitor=ALL-UNNAMED --add-opens=jdk.jdi/com.sun.tools.jdi=ALL-UNNAMED --add-opens=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED -Didea.platform.prefix=Idea -Djava.system.class.loader=com.intellij.util.lang.PathClassLoader -Djavax.xml.parsers.SAXParserFactory=com.sun.org.apache.xerces.internal.jaxp.SAXParserFactoryImpl" />
+    <option name="VM_PARAMETERS" value="-Xmx2g -XX:ReservedCodeCacheSize=240m -XX:SoftRefLRUPolicyMSPerMB=50 -XX:MaxJavaStackTraceDepth=10000 -ea -Dsun.io.useCanonCaches=false -Dapple.laf.useScreenMenuBar=true -Dsun.awt.disablegrab=true -Didea.jre.check=true -Didea.is.internal=true -Didea.debug.mode=true -Djdk.attach.allowAttachSelf -Dfus.internal.test.mode=true -Dkotlinx.coroutines.debug=off -Djdk.module.illegalAccess.silent=true -Didea.config.path=../config/idea -Didea.system.path=../system/idea -Didea.initially.ask.config=true --add-opens=java.base/java.io=ALL-UNNAMED --add-opens=java.base/java.lang.reflect=ALL-UNNAMED --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.nio.charset=ALL-UNNAMED --add-opens=java.base/java.text=ALL-UNNAMED --add-opens=java.base/java.time=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens=java.base/java.util.concurrent=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.base/jdk.internal.vm=ALL-UNNAMED --add-opens=java.base/sun.nio.ch=ALL-UNNAMED --add-opens=java.desktop/com.apple.eawt.event=ALL-UNNAMED --add-opens=java.desktop/com.apple.eawt=ALL-UNNAMED --add-opens=java.desktop/com.apple.laf=ALL-UNNAMED --add-opens=java.desktop/com.sun.java.swing.plaf.gtk=ALL-UNNAMED --add-opens=java.desktop/java.awt.dnd.peer=ALL-UNNAMED --add-opens=java.desktop/java.awt.event=ALL-UNNAMED --add-opens=java.desktop/java.awt.image=ALL-UNNAMED --add-opens=java.desktop/java.awt.peer=ALL-UNNAMED --add-opens=java.desktop/java.awt=ALL-UNNAMED --add-opens=java.desktop/javax.swing.plaf.basic=ALL-UNNAMED --add-opens=java.desktop/javax.swing.text=ALL-UNNAMED --add-opens=java.desktop/javax.swing.text.html=ALL-UNNAMED --add-opens=java.desktop/javax.swing.text.html.parser=ALL-UNNAMED --add-opens=java.desktop/javax.swing=ALL-UNNAMED --add-opens=java.desktop/sun.awt.X11=ALL-UNNAMED --add-opens=java.desktop/sun.awt.datatransfer=ALL-UNNAMED --add-opens=java.desktop/sun.awt.image=ALL-UNNAMED --add-opens=java.desktop/sun.awt.windows=ALL-UNNAMED --add-opens=java.desktop/sun.awt=ALL-UNNAMED --add-opens=java.desktop/sun.font=ALL-UNNAMED --add-opens=java.desktop/sun.java2d=ALL-UNNAMED --add-opens=java.desktop/sun.lwawt.macosx=ALL-UNNAMED --add-opens=java.desktop/sun.lwawt=ALL-UNNAMED --add-opens=java.desktop/sun.swing=ALL-UNNAMED --add-opens=jdk.attach/sun.tools.attach=ALL-UNNAMED --add-opens=jdk.internal.jvmstat/sun.jvmstat.monitor=ALL-UNNAMED --add-opens=jdk.jdi/com.sun.tools.jdi=ALL-UNNAMED --add-opens=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED -Didea.platform.prefix=Idea -Dadditional.modules=&quot;intellij.devkit&quot; -Djava.system.class.loader=com.intellij.util.lang.PathClassLoader -Djavax.xml.parsers.SAXParserFactory=com.sun.org.apache.xerces.internal.jaxp.SAXParserFactoryImpl" />


Hard to tell what changed here

The IDE shows it in the diff 🎉

-Dadditional.modules="intellij.devkit" was added

rock3r · 2025-09-10T07:25:49Z

.idea/markdown.xml

@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8"?>


Did you mean to commit this?

rock3r · 2025-09-10T07:26:39Z

platform/core-ui/src/ui/icons/IconWithOverlay.java

 */
 public abstract class IconWithOverlay extends LayeredIcon {
-  public IconWithOverlay(@NotNull Icon main, @NotNull Icon overlay) {
+  public IconWithOverlay(@NotNull javax.swing.Icon main, @NotNull javax.swing.Icon overlay) {


I'd revert changes to this file

rock3r · 2025-09-10T07:26:50Z

platform/core-ui/src/ui/icons/IconWrapperWithToolTipComposite.java

 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;

+import javax.swing.Icon;


rock3r · 2025-09-10T07:27:05Z

platform/core-ui/src/util/IconUtil.kt

 import java.util.concurrent.ConcurrentHashMap
 import java.util.function.Supplier
 import java.util.function.ToIntFunction
+import javax.swing.Icon


rock3r · 2025-09-10T13:24:05Z

plugins/devkit/intellij.devkit.compose/src/showcase/ComposePerformanceDemoAction.kt

+    )
+
+    Column {
+      val fps = frameTimes.size


This fps counter is completely wrong. It's using the size of the linked list as fps value. That makes no sense; the fps is the number of frames rendered in the last second — or in this case, recomposed in the last second.

Likewise, the min and max fps values below make no sense. minFps will always be 0 as the linked list starts empty, and maxFps is going to grow on each recomposition.

rock3r · 2025-09-10T13:28:20Z

plugins/devkit/intellij.devkit.compose/src/showcase/ComposeShowcase.kt

+    250f,
+    infiniteRepeatable(tween(durationMillis = 1000, easing = EaseInOut), repeatMode = RepeatMode.Reverse),
+  )
+  Box {


No need for these boxes

rock3r · 2025-09-10T13:28:41Z

plugins/devkit/intellij.devkit.compose/src/showcase/ComposeShowcase.kt

+  }
+}
+
+private val deferedIcon = IconDeferrer.getInstance().deferAsync(


Same typo: deferredIcon

rock3r · 2025-09-10T13:31:40Z

platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/icon/IconPainterProvider.kt

+import androidx.compose.ui.graphics.painter.Painter
+import org.jetbrains.icons.api.Icon
+
+public interface IconPainterProvider {


No implementation in Standalone?!?

rock3r · 2025-09-10T13:33:33Z

platform/util/ui/src/com/intellij/ui/icons/FallbackIconIdentifier.kt

+
+import org.jetbrains.icons.api.IconIdentifier
+
+class FallbackIconIdentifier(


Is this fallback or nested?

chashnikov · 2025-09-19T13:07:25Z

platform/icons-api/src/main/kotlin/org/jetbrains/icons/api/IconManager.kt

+  fun registerIcon(id: IconIdentifier, icon: Icon)
+}
+
+interface IconIdentifier


One of the things we can solve with new icons API is getting rid of dependency on javax.swing from *.psi modules. It seems that javax.swing.Icon is the only class from javax.swing which is used there (PsiElement interface extends Iconable with getIcon method). So if we introduce IconIdentifier, we can use it in PsiElement instead of javax.swing.Icon and eventually get rid of the latter.

However, to do that we probably need to put IconIdentifier in a different module, so it can be added to dependencies of *.psi modules without having any actual rendering API there.

chashnikov · 2025-09-19T13:08:54Z

platform/icons-api/src/main/kotlin/org/jetbrains/icons/api/IconManager.kt

+
+interface IconManager {
+  fun loadIcon(id: IconIdentifier, path: String, aClass: Class<*>): Icon?
+  fun registerIcon(id: IconIdentifier, icon: Icon)


registerIcon method is supposed to be used to register all available icons in advance, right? I think it's better to avoid such a requirement and allow locating icons by demand only.

chashnikov · 2025-09-19T13:49:50Z

platform/icons-api/src/main/kotlin/org/jetbrains/icons/api/Icon.kt

+  val identifier: IconIdentifier
+  val state: IconState
+
+  fun render(api: PaintingApi)


Do we really need to have fully abstract painting api?
It seems (please correct me if I'm wrong) the vast majority of icons go through the following stages:

An element (PsiElement, action, tool window) determines how it should be presented. Often it just returns a field from one of *Icons classes which wraps a path to the svg file. For PsiElement the presentation may be composed of multiple layers. Sometimes calculating the proper presentation requires some time, so it shouldn't be done synchronously.

The presentation is adjusted according to device-specific configuration: light or dark theme, scaling.

The resulting image is actually painted.

The problem with the current implementation is that the same interface javax.swing.Icon is used during all these stages.
And code which determines presentation during the first stage shouldn't know specify how the icon should be rendered.
We would need this if we want to have absolute flexibility here and allow elements to generate bitmaps on the fly, for example. But do we really need it? If in 99% of cases the icons are svg files with few transformations applied, painting them all via such a generic API may be not efficient.

[jewel,platform] IJPL-176416 Compose icon infrastructure

a40d698

jakub-senohrabek requested a review from rock3r September 3, 2025 19:06

jakub-senohrabek marked this pull request as draft September 3, 2025 19:06

[jewel,platform] IJPL-176416 Compose icon infrastructure - performanc…

8d00dbb

…e demo, icon id and state, reuse texture for scaling

rock3r assigned jakub-senohrabek Sep 10, 2025

rock3r added the Jewel label Sep 10, 2025

rock3r suggested changes Sep 10, 2025

View reviewed changes

chashnikov reviewed Sep 19, 2025

View reviewed changes


		import org.jetbrains.icons.api.IconIdentifier

		class FallbackIconIdentifier(

[JEWEL] IJPL-176416 Compose icon infrastructure #3217

Are you sure you want to change the base?

[JEWEL] IJPL-176416 Compose icon infrastructure #3217

Conversation

jakub-senohrabek commented Sep 3, 2025

Uh oh!

rock3r left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants