Handle SVG loading exceptions and path patching issues in IJ 233 EAP 4 (

#191)

* WIP

* Bump IJ 233 version to 233.9802.14-EAP-SNAPSHOT (EAP 4)

* Remove unused IconsPathPatching code

* Update code after changes in main

* Make loading fallback consistent for all resource types

Also document the behaviour in the KDoc

* Prevent icon loading from crashing

Plus, some IDE sample code cleanup

---------

Co-authored-by: Lamberto Basti <[email protected]>

buildSrc/src/main/kotlin/jewel.gradle.kts

@@ -37,6 +37,7 @@ kotlin {
                 optIn("androidx.compose.ui.ExperimentalComposeUiApi")
                 optIn("androidx.compose.foundation.ExperimentalFoundationApi")
                 optIn("org.jetbrains.jewel.ExperimentalJewelApi")
+                optIn("org.jetbrains.jewel.InternalJewelApi")
             }
         }
     }

core/src/main/kotlin/org/jetbrains/jewel/painter/PainterProvider.kt

@@ -6,16 +6,25 @@ import androidx.compose.runtime.State
 import androidx.compose.ui.graphics.painter.Painter
 
 /**
- * Implementations of this interface should handle the passed [PainterHint]s correctly.
- * For now, this means calling [PainterPathHint.patch] and [PainterSvgPatchHint.patch].
- * Most likely, a [PainterProvider] should also hold the resource path and [ClassLoader]
+ * Provides a [Painter] for an image, which may be transformed by the
+ * provided hints.
+ *
+ * Note: implementations of this interface should handle the passed
+ * [PainterHint]s correctly. For now, this means calling
+ * [PainterPathHint.patch] and [PainterSvgPatchHint.patch]. Most likely, a
+ * [PainterProvider] should also hold the resource path and [ClassLoader]
  * references.
  */
 @Immutable
 interface PainterProvider {
 
     /**
      * Provides a [Painter] using the specified [PainterHint]s.
+     * The painters are [remember][androidx.compose.runtime.remember]ed
+     * and this function can be called multiple times for the same data.
+     *
+     * Depending on the implementation, errors may be suppressed and a
+     * replacement painter provided.
      */
     @Composable
     fun getPainter(vararg hints: PainterHint): State<Painter>

core/src/main/kotlin/org/jetbrains/jewel/painter/ResourcePainterProvider.kt

@@ -5,13 +5,16 @@ import androidx.compose.runtime.Immutable
 import androidx.compose.runtime.State
 import androidx.compose.runtime.remember
 import androidx.compose.runtime.rememberUpdatedState
+import androidx.compose.ui.graphics.Color
 import androidx.compose.ui.graphics.painter.BitmapPainter
+import androidx.compose.ui.graphics.painter.ColorPainter
 import androidx.compose.ui.graphics.painter.Painter
 import androidx.compose.ui.graphics.vector.rememberVectorPainter
 import androidx.compose.ui.platform.LocalDensity
 import androidx.compose.ui.res.loadImageBitmap
 import androidx.compose.ui.res.loadSvgPainter
 import androidx.compose.ui.res.loadXmlImageVector
+import androidx.compose.ui.unit.Density
 import org.jetbrains.jewel.util.inDebugMode
 import org.w3c.dom.Document
 import org.xml.sax.InputSource
@@ -29,10 +32,18 @@ import javax.xml.transform.TransformerFactory
 import javax.xml.transform.dom.DOMSource
 import javax.xml.transform.stream.StreamResult
 
+private val errorPainter = ColorPainter(Color.Magenta)
+
 /**
- * Provide [Painter] by resources in the module and jars, it use the ResourceResolver to load resources.
+ * Provide [Painter] by resources in the module and jars, it use the
+ * ResourceResolver to load resources.
+ *
+ * It will cache the painter by [PainterHint]s, so it is safe to call
+ * [getPainter] multiple times.
  *
- * It will cache the painter by [PainterHint]s, so it is safe to call [getPainter] multiple times.
+ * If a resource fails to load, it will be silently replaced by a
+ * magenta color painter, and the exception logged. If Jewel is in
+ * [debug mode][inDebugMode], however, exceptions will not be suppressed.
  */
 @Immutable
 class ResourcePainterProvider(
@@ -70,56 +81,40 @@ class ResourcePainterProvider(
 
     @Composable
     private fun loadPainter(hints: List<PainterHint>): Painter {
-        val format = basePath.substringAfterLast(".").lowercase()
-
         val pathStack = buildSet {
             var path = basePath
 
             add(path)
 
-            hints.forEach {
-                path = when (it) {
-                    is PainterResourcePathHint -> it.patch(path, contextClassLoaders)
-                    is PainterPathHint -> it.patch(path)
-                    else -> return@forEach
+            for (hint in hints) {
+                path = when (hint) {
+                    is PainterResourcePathHint -> hint.patch(path, contextClassLoaders)
+                    is PainterPathHint -> hint.patch(path)
+                    else -> continue
                 }
                 add(path)
             }
         }.reversed()
 
-        val url = pathStack.firstNotNullOfOrNull {
-            resolveResource(it)
-        } ?: error("Resource '$basePath(${hints.joinToString()})' not found")
-
-        val density = LocalDensity.current
+        val url = pathStack.firstNotNullOfOrNull { resolveResource(it) }
 
-        return when (format) {
-            "svg" -> {
-                remember(url, density, hints) {
-                    patchSvg(url.openStream(), hints).use {
-                        if (inDebugMode) {
-                            println("Load icon $basePath(${hints.joinToString()}) from $url")
-                        }
-                        loadSvgPainter(it, density)
-                    }
-                }
+        @Suppress("UrlHashCode") // It's ok when comparing a URL to null
+        if (url == null) {
+            if (inDebugMode) {
+                error("Resource '$basePath(${hints.joinToString()})' not found")
+            } else {
+                return errorPainter
             }
+        }
 
-            "xml" -> {
-                val vector = url.openStream().use {
-                    loadXmlImageVector(InputSource(it), density)
-                }
-                rememberVectorPainter(vector)
-            }
+        val density = LocalDensity.current
 
-            else -> {
-                remember(url, density) {
-                    val bitmap = url.openStream().use {
-                        loadImageBitmap(it)
-                    }
-                    BitmapPainter(bitmap)
-                }
-            }
+        val extension = basePath.substringAfterLast(".").lowercase()
+
+        return when (extension) {
+            "svg" -> createSvgPainter(url, density, hints)
+            "xml" -> createVectorDrawablePainter(url, density)
+            else -> createBitmapPainter(url, density)
         }
     }
 
@@ -137,6 +132,21 @@ class ResourcePainterProvider(
         return null
     }
 
+    @Composable
+    private fun createSvgPainter(url: URL, density: Density, hints: List<PainterHint>): Painter =
+        tryLoadingResource(
+            url = url,
+            loadingAction = { resourceUrl ->
+                patchSvg(url.openStream(), hints).use { inputStream ->
+                    if (inDebugMode) {
+                        println("Loading icon $basePath(${hints.joinToString()}) from $resourceUrl")
+                    }
+                    loadSvgPainter(inputStream, density)
+                }
+            },
+            rememberAction = { remember(url, density, hints) { it } },
+        )
+
     private fun patchSvg(inputStream: InputStream, hints: List<PainterHint>): InputStream {
         if (hints.all { it !is PainterSvgPatchHint }) {
             return inputStream
@@ -155,6 +165,31 @@ class ResourcePainterProvider(
         }
     }
 
+    @Composable
+    private fun createVectorDrawablePainter(url: URL, density: Density): Painter =
+        tryLoadingResource(
+            url = url,
+            loadingAction = { resourceUrl ->
+                resourceUrl.openStream().use {
+                    loadXmlImageVector(InputSource(it), density)
+                }
+            },
+            rememberAction = { rememberVectorPainter(it) },
+        )
+
+    @Composable
+    private fun createBitmapPainter(url: URL, density: Density) =
+        tryLoadingResource(
+            url = url,
+            loadingAction = { resourceUrl ->
+                val bitmap = resourceUrl.openStream().use {
+                    loadImageBitmap(it)
+                }
+                BitmapPainter(bitmap)
+            },
+            rememberAction = { remember(url, density) { it } },
+        )
+
     private fun Document.writeToString(): String {
         val tf = TransformerFactory.newInstance()
         val transformer: Transformer
@@ -172,14 +207,32 @@ class ResourcePainterProvider(
             error("Unable to render XML document to string: ${e.message}")
         }
     }
+
+    @Composable
+    private fun <T> tryLoadingResource(
+        url: URL,
+        loadingAction: (URL) -> T,
+        rememberAction: @Composable (T) -> Painter,
+    ): Painter {
+        @Suppress("TooGenericExceptionCaught") // This is a last-resort fallback when icons fail to load
+        val painter = try {
+            loadingAction(url)
+        } catch (e: RuntimeException) {
+            val message = "Unable to load SVG resource from $url\n${e.stackTraceToString()}"
+            if (inDebugMode) {
+                error(message)
+            }
+
+            System.err.println(message)
+            return errorPainter
+        }
+
+        return rememberAction(painter)
+    }
 }
 
 @Composable
-fun rememberResourcePainterProvider(
-    path: String,
-    iconClass: Class<*>,
-): PainterProvider {
-    return remember(path, iconClass.classLoader) {
+fun rememberResourcePainterProvider(path: String, iconClass: Class<*>): PainterProvider =
+    remember(path, iconClass.classLoader) {
         ResourcePainterProvider(path, iconClass.classLoader)
     }
-}

core/src/main/kotlin/org/jetbrains/jewel/util/Debug.kt

@@ -2,6 +2,10 @@ package org.jetbrains.jewel.util
 
 import org.jetbrains.jewel.InternalJewelApi
 
+/**
+ * Determines whether we're in debug mode. This should not be used
+ * in the bridge for logging; instead, you should use the IDE logger.
+ */
 @InternalJewelApi
 val inDebugMode by lazy {
     System.getProperty("org.jetbrains.jewel.debug")?.toBoolean() ?: false

gradle/libs.versions.toml

@@ -3,7 +3,7 @@ composeDesktop = "1.5.2"
 coroutines = "1.7.3"
 detekt = "1.23.1"
 idea232 = "232.8660.185"
-idea233 = "233.9102.97-EAP-SNAPSHOT"
+idea233 = "233.9802.14-EAP-SNAPSHOT"
 ideaGradlePlugin = "1.15.0"
 javaSarif = "2.0"
 kotlinSarif = "0.4.0"

ide-laf-bridge/ide-laf-bridge-233/src/main/kotlin/org/jetbrains/jewel/bridge/BridgeOverride.kt

@@ -1,5 +1,6 @@
 package org.jetbrains.jewel.bridge
 
+import com.intellij.openapi.diagnostic.Logger
 import com.intellij.ui.icons.patchIconPath
 import com.intellij.util.ui.DirProvider
 import org.jetbrains.jewel.painter.PainterResourcePathHint
@@ -8,6 +9,7 @@ internal object BridgeOverride : PainterResourcePathHint {
 
     private val dirProvider = DirProvider()
 
+    @Suppress("UnstableApiUsage") // patchIconPath() is explicitly open to us
     override fun patch(path: String, classLoaders: List<ClassLoader>): String {
         // For all provided classloaders, we try to get the patched path, both using
         // the original path, and an "abridged" path that has gotten the icon path prefix
@@ -19,12 +21,45 @@ internal object BridgeOverride : PainterResourcePathHint {
             val patchedPath = patchIconPath(path.removePrefix("/"), classLoader)?.first
                 ?: patchIconPath(fallbackPath, classLoader)?.first
 
+            // 233 EAP 4 broke path patching horribly; now it can return a
+            // "reflective path", which is a FQN to an ExpUIIcons entry.
+            // As a (hopefully) temporary solution, we undo this transformation
+            // back into the original path.
+            if (patchedPath?.startsWith("com.intellij.icons.ExpUiIcons") == true) {
+                return inferActualPathFromReflectivePath(patchedPath)
+            }
+
             if (patchedPath != null) {
                 return patchedPath
             }
         }
         return path
     }
 
+    private fun inferActualPathFromReflectivePath(patchedPath: String): String {
+        val iconPath = patchedPath.removePrefix("com.intellij.icons.ExpUiIcons.")
+
+        return buildString {
+            append("expui/")
+            iconPath.split('.')
+                .map { it.trim() }
+                .filter { it.isNotEmpty() }
+                .forEach {
+                    append(it.first().lowercaseChar())
+                    append(it.drop(1))
+                    append('/')
+                }
+            replace(length - 1, length, "")
+            if (iconPath.contains("_dark")) append("_dark")
+            append(".svg")
+
+            Logger.getInstance("IconsPathPatching")
+                .warn(
+                    "IntelliJ returned a reflective path: $patchedPath for $iconPath." +
+                        " We reverted that to a plausible-looking resource path: ${toString()}",
+                )
+        }
+    }
+
     override fun toString(): String = "BridgeOverride"
 }

samples/ide-plugin/build.gradle.kts

@@ -45,6 +45,6 @@ tasks {
     }
 
     runIde {
-        this.systemProperties["org.jetbrains.jewel.debug"] = "true"
+        systemProperties["org.jetbrains.jewel.debug"] = "true"
     }
 }