Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JEWEL-746] Load inline markdown images using Coil3 #2924

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[JEWEL-746] Load inline markdown images using Coil3 #2924

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions platform/jewel/gradle/libs.versions.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[versions]
coil = "3.1.0"
commonmark = "0.24.0"
composeDesktop = "1.7.1"
detekt = "1.23.6"
Expand All @@ -16,6 +17,11 @@ kotlinxBinaryCompat = "0.16.3"
ktfmtGradlePlugin = "0.20.1"

[libraries]
coil-compose = { module = "io.coil-kt.coil3:coil-compose", version.ref = "coil" }
# network is only needed in a standalone non-ide version
coil-network-ktor3 = { module = "io.coil-kt.coil3:coil-network-ktor3", version.ref = "coil" }
coil-svg = { module = "io.coil-kt.coil3:coil-svg", version.ref = "coil" }

commonmark-core = { module = "org.commonmark:commonmark", version.ref = "commonmark" }
commonmark-ext-autolink = { module = "org.commonmark:commonmark-ext-autolink", version.ref = "commonmark" }
commonmark-ext-gfm-strikethrough = { module = "org.commonmark:commonmark-ext-gfm-strikethrough", version.ref = "commonmark" }
Expand All @@ -26,6 +32,7 @@ filePicker = { module = "com.darkrockstudios:mpfilepicker", version = "3.1.0" }
kotlinSarif = { module = "io.github.detekt.sarif4k:sarif4k", version.ref = "kotlinSarif" }
kotlin-reflect = { module = "org.jetbrains.kotlin:kotlin-reflect", version.ref = "kotlin" }
kotlinx-serialization-json = { module = "org.jetbrains.kotlinx:kotlinx-serialization-json", version.ref = "kotlinxSerialization" }
ktor-client-java = { module = "io.ktor:ktor-client-java", version = "3.0.3" }

jna-core = { module = "net.java.dev.jna:jna", version.ref = "jna" }

Expand Down
4 changes: 4 additions & 0 deletions platform/jewel/markdown/core/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ plugins {
dependencies {
api(projects.ui)
api(libs.commonmark.core)
runtimeOnly(libs.ktor.client.java)
implementation(libs.coil.compose)
implementation(libs.coil.network.ktor3)
implementation(libs.coil.svg)

testImplementation(compose.desktop.uiTestJUnit4)
testImplementation(projects.ui)
Expand Down
130 changes: 127 additions & 3 deletions platform/jewel/markdown/core/intellij.platform.jewel.markdown.core.iml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@
</stringArguments>
<arrayArguments>
<arrayArg name="pluginClasspaths">
<args>
<arg>$MAVEN_REPOSITORY$/org/jetbrains/kotlin/kotlin-compose-compiler-plugin/2.1.10/kotlin-compose-compiler-plugin-2.1.10.jar</arg>
</args>
<args>$MAVEN_REPOSITORY$/org/jetbrains/kotlin/kotlin-compose-compiler-plugin/2.1.10/kotlin-compose-compiler-plugin-2.1.10.jar</args>
</arrayArg>
</arrayArguments>
</compilerArguments>
Expand Down Expand Up @@ -55,6 +53,132 @@
</SOURCES>
</library>
</orderEntry>
<orderEntry type="module-library">
<library name="io.coil.kt.coil3.core.jvm" type="repository">
<properties include-transitive-deps="false" maven-id="io.coil-kt.coil3:coil-core-jvm:3.1.0">
<verification>
<artifact url="file://$MAVEN_REPOSITORY$/io/coil-kt/coil3/coil-core-jvm/3.1.0/coil-core-jvm-3.1.0.jar">
<sha256sum>6d44a11188c53f4eea2e87ebadc6aad90069132c9b029fc1b35aeb0added5ef7</sha256sum>
</artifact>
</verification>
</properties>
<CLASSES>
<root url="jar://$MAVEN_REPOSITORY$/io/coil-kt/coil3/coil-core-jvm/3.1.0/coil-core-jvm-3.1.0.jar!/" />
</CLASSES>
<JAVADOC />
<SOURCES>
<root url="jar://$MAVEN_REPOSITORY$/io/coil-kt/coil3/coil-core-jvm/3.1.0/coil-core-jvm-3.1.0-sources.jar!/" />
</SOURCES>
</library>
</orderEntry>
<orderEntry type="module-library">
<library name="io.coil.kt.coil3.jvm" type="repository">
<properties include-transitive-deps="false" maven-id="io.coil-kt.coil3:coil-jvm:3.1.0">
<verification>
<artifact url="file://$MAVEN_REPOSITORY$/io/coil-kt/coil3/coil-jvm/3.1.0/coil-jvm-3.1.0.jar">
<sha256sum>7d0182c987052d6502135a6a6cc7cc68288ef48f990d75ad4c4ae03e25be786e</sha256sum>
</artifact>
</verification>
</properties>
<CLASSES>
<root url="jar://$MAVEN_REPOSITORY$/io/coil-kt/coil3/coil-jvm/3.1.0/coil-jvm-3.1.0.jar!/" />
</CLASSES>
<JAVADOC />
<SOURCES>
<root url="jar://$MAVEN_REPOSITORY$/io/coil-kt/coil3/coil-jvm/3.1.0/coil-jvm-3.1.0-sources.jar!/" />
</SOURCES>
</library>
</orderEntry>
<orderEntry type="module-library" exported="">
<library name="io.coil.kt.coil3.compose.core.jvm" type="repository">
<properties include-transitive-deps="false" maven-id="io.coil-kt.coil3:coil-compose-core-jvm:3.1.0">
<verification>
<artifact url="file://$MAVEN_REPOSITORY$/io/coil-kt/coil3/coil-compose-core-jvm/3.1.0/coil-compose-core-jvm-3.1.0.jar">
<sha256sum>8aa1d7ae1d11f969e8cdcc8fee42b7ee6a036e21f70567e3c3486edd9d7dc594</sha256sum>
</artifact>
</verification>
</properties>
<CLASSES>
<root url="jar://$MAVEN_REPOSITORY$/io/coil-kt/coil3/coil-compose-core-jvm/3.1.0/coil-compose-core-jvm-3.1.0.jar!/" />
</CLASSES>
<JAVADOC />
<SOURCES>
<root url="jar://$MAVEN_REPOSITORY$/io/coil-kt/coil3/coil-compose-core-jvm/3.1.0/coil-compose-core-jvm-3.1.0-sources.jar!/" />
</SOURCES>
</library>
</orderEntry>
<orderEntry type="module-library" exported="">
<library name="io.coil.kt.coil3.compose.jvm" type="repository">
<properties include-transitive-deps="false" maven-id="io.coil-kt.coil3:coil-compose-jvm:3.1.0">
<verification>
<artifact url="file://$MAVEN_REPOSITORY$/io/coil-kt/coil3/coil-compose-jvm/3.1.0/coil-compose-jvm-3.1.0.jar">
<sha256sum>d43e0ed4566d30f8ce8e023539240b494be6fa69a1b8c81b4c32f5f465a7cc06</sha256sum>
</artifact>
</verification>
</properties>
<CLASSES>
<root url="jar://$MAVEN_REPOSITORY$/io/coil-kt/coil3/coil-compose-jvm/3.1.0/coil-compose-jvm-3.1.0.jar!/" />
</CLASSES>
<JAVADOC />
<SOURCES>
<root url="jar://$MAVEN_REPOSITORY$/io/coil-kt/coil3/coil-compose-jvm/3.1.0/coil-compose-jvm-3.1.0-sources.jar!/" />
</SOURCES>
</library>
</orderEntry>
<orderEntry type="module-library">
<library name="io.coil.kt.coil3.network.core.jvm" type="repository">
<properties include-transitive-deps="false" maven-id="io.coil-kt.coil3:coil-network-core-jvm:3.1.0">
<verification>
<artifact url="file://$MAVEN_REPOSITORY$/io/coil-kt/coil3/coil-network-core-jvm/3.1.0/coil-network-core-jvm-3.1.0.jar">
<sha256sum>8332e45cf792cd24d9814744db84b0e6d33ec6eaf0724bd07ddc1fce7c55591f</sha256sum>
</artifact>
</verification>
</properties>
<CLASSES>
<root url="jar://$MAVEN_REPOSITORY$/io/coil-kt/coil3/coil-network-core-jvm/3.1.0/coil-network-core-jvm-3.1.0.jar!/" />
</CLASSES>
<JAVADOC />
<SOURCES>
<root url="jar://$MAVEN_REPOSITORY$/io/coil-kt/coil3/coil-network-core-jvm/3.1.0/coil-network-core-jvm-3.1.0-sources.jar!/" />
</SOURCES>
</library>
</orderEntry>
<orderEntry type="module-library">
<library name="io.coil.kt.coil3.network.ktor3.jvm" type="repository">
<properties include-transitive-deps="false" maven-id="io.coil-kt.coil3:coil-network-ktor3-jvm:3.1.0">
<verification>
<artifact url="file://$MAVEN_REPOSITORY$/io/coil-kt/coil3/coil-network-ktor3-jvm/3.1.0/coil-network-ktor3-jvm-3.1.0.jar">
<sha256sum>cc4f9f8d6d447e7559cb9717c3a45dfbd351ddd06a34724001322512160b0215</sha256sum>
</artifact>
</verification>
</properties>
<CLASSES>
<root url="jar://$MAVEN_REPOSITORY$/io/coil-kt/coil3/coil-network-ktor3-jvm/3.1.0/coil-network-ktor3-jvm-3.1.0.jar!/" />
</CLASSES>
<JAVADOC />
<SOURCES>
<root url="jar://$MAVEN_REPOSITORY$/io/coil-kt/coil3/coil-network-ktor3-jvm/3.1.0/coil-network-ktor3-jvm-3.1.0-sources.jar!/" />
</SOURCES>
</library>
</orderEntry>
<orderEntry type="module-library" exported="">
<library name="io.coil.kt.coil3.svg.jvm" type="repository">
<properties include-transitive-deps="false" maven-id="io.coil-kt.coil3:coil-svg-jvm:3.1.0">
<verification>
<artifact url="file://$MAVEN_REPOSITORY$/io/coil-kt/coil3/coil-svg-jvm/3.1.0/coil-svg-jvm-3.1.0.jar">
<sha256sum>413a53b4b6e0a40b851d1a99a01a4cd91ac0dc68facefddd7adfdb2d5456588f</sha256sum>
</artifact>
</verification>
</properties>
<CLASSES>
<root url="jar://$MAVEN_REPOSITORY$/io/coil-kt/coil3/coil-svg-jvm/3.1.0/coil-svg-jvm-3.1.0.jar!/" />
</CLASSES>
<JAVADOC />
<SOURCES>
<root url="jar://$MAVEN_REPOSITORY$/io/coil-kt/coil3/coil-svg-jvm/3.1.0/coil-svg-jvm-3.1.0-sources.jar!/" />
</SOURCES>
</library>
</orderEntry>
<orderEntry type="module-library" scope="TEST">
<library name="org.jetbrains.compose.ui.ui.test.junit4" type="repository">
<properties include-transitive-deps="false" maven-id="org.jetbrains.compose.ui:ui-test-junit4:1.7.1">
Expand Down
20 changes: 20 additions & 0 deletions platform/jewel/markdown/core/src/main/kotlin/org/jetbrains/jewel/markdown/Markdown.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ import androidx.compose.runtime.setValue
import androidx.compose.ui.Modifier
import androidx.compose.ui.semantics.semantics
import androidx.compose.ui.unit.dp
import coil3.ImageLoader
import coil3.PlatformContext
import coil3.compose.setSingletonImageLoaderFactory
import coil3.memory.MemoryCache
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.withContext
Expand Down Expand Up @@ -76,6 +80,7 @@ public fun Markdown(
markdownStyling: MarkdownStyling = JewelTheme.markdownStyling,
blockRenderer: MarkdownBlockRenderer = DefaultMarkdownBlockRenderer(markdownStyling),
) {
setSingletonImageLoaderFactory(::createImageLoader)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this belong here? It feels like it's init code, so having it once only (e.g., in ProvideMarkdownStyling?) would make more sense to me

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ProvideMarkdownStyling is in a different module and it's not guaranteed that a user will wrap Markdown into a style provider, so it may be confusing for debugging.
Will it attempt to invoke this code on each rerender or on each markdown usage in the app? I don't mind moving it somewhere, I just haven't figured out how exactly.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will it attempt to invoke this code on each rerender or on each markdown usage in the app?

It won't attempt to invoke, it will invoke it. That's why I'm saying it'd make more sense to do it elsewhere. I think we should even consider not using a singleton factory since users may want to use different ones; can we create one that we store in a composition local, and set it up in ProvideMarkdownStyling?

We can instruct users to use ProvideMarkdownStyling in the error message for when the composition local is not set up, like we do for other mandatory composition locals.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the message, I assume you're referring to invocation on each markdown usage?

The code itself is checking if an image loader already set before using compareAndSet to invoke a factory. I've set a debugger to see what's going on there. I assume one defererence on each markdown frame is not bad and I still don't completely follow the ProvideMarkdownStyling approach.

Does this sound reasonable or should I move this call out?
Screenshot 2025-02-18 at 4 13 37 PM

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume one defererence on each markdown frame is not bad

Yeah, that's ok, thanks for clarifying. But it's not my main concern. What happens if this is used in a context where the user has set their own loader? Seems to me like setting up a singleton image loader sets some expectations on our part that could be broken if the user had already done the same, with a different loader.

It also feels like we're leaking the loader outside Jewel, which is not very good either. We might be causing problems for our users because they expect to be able to set their own singleton IL, and it doesn't do anything if any Markdown composable just happened to be composed before then.

Is there no way to use composition locals to have a Jewel-only image loader that does not leak externally, and that we know we can trust? Of course, users can override it, but it's fine if they do it knowingly. It's the magical, undocumented side effect that is not acceptable.

if (selectable) {
SelectionContainer(modifier.semantics { rawMarkdown = markdown }) {
Column(verticalArrangement = Arrangement.spacedBy(markdownStyling.blockVerticalSpacing)) {
Expand Down Expand Up @@ -110,6 +115,7 @@ public fun LazyMarkdown(
markdownStyling: MarkdownStyling = JewelTheme.markdownStyling,
blockRenderer: MarkdownBlockRenderer = JewelTheme.markdownBlockRenderer,
) {
setSingletonImageLoaderFactory(::createImageLoader)
if (selectable) {
SelectionContainer(modifier) {
LazyColumn(
Expand All @@ -131,3 +137,17 @@ public fun LazyMarkdown(
}
}
}

private const val IMAGES_MEMORY_CACHE_SIZE = 24L * 1024 * 1024 // 24mb

/**
* This method sets up an image loader with a memory cache but disables the disk cache. Disabling the disk cache is
* necessary because Coil crashes when attempting to use the file system cache with IDEA platform.
*
* Otherwise, Coil3 will throw java.lang.NoSuchMethodError: kotlinx.coroutines.CoroutineDispatcher.limitedParallelism
*/
private fun createImageLoader(context: PlatformContext) =
ImageLoader.Builder(context)
.memoryCache { MemoryCache.Builder().maxSizeBytes(IMAGES_MEMORY_CACHE_SIZE).build() }
.diskCache(null)
.build()
3 changes: 2 additions & 1 deletion ...e/src/main/kotlin/org/jetbrains/jewel/markdown/rendering/DefaultInlineMarkdownRenderer.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.jetbrains.jewel.markdown.rendering

import androidx.compose.foundation.text.appendInlineContent
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.text.AnnotatedString
import androidx.compose.ui.text.AnnotatedString.Builder
Expand Down Expand Up @@ -80,7 +81,7 @@ public open class DefaultInlineMarkdownRenderer(rendererExtensions: List<Markdow
}

is InlineMarkdown.Image -> {
// TODO not supported yet — see JEWEL-746
appendInlineContent(child.source, "![${child.title}](${child.source})")
}

is InlineMarkdown.CustomDelimitedNode -> {
Expand Down
77 changes: 77 additions & 0 deletions ...re/src/main/kotlin/org/jetbrains/jewel/markdown/rendering/DefaultMarkdownBlockRenderer.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@ import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.layout.width
import androidx.compose.foundation.layout.widthIn
import androidx.compose.foundation.text.InlineTextContent
import androidx.compose.foundation.text.selection.DisableSelection
import androidx.compose.runtime.Composable
import androidx.compose.runtime.CompositionLocalProvider
import androidx.compose.runtime.collectAsState
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
Expand All @@ -30,15 +32,25 @@ import androidx.compose.ui.graphics.isSpecified
import androidx.compose.ui.graphics.takeOrElse
import androidx.compose.ui.input.pointer.PointerIcon
import androidx.compose.ui.input.pointer.pointerHoverIcon
import androidx.compose.ui.platform.LocalDensity
import androidx.compose.ui.text.AnnotatedString
import androidx.compose.ui.text.Placeholder
import androidx.compose.ui.text.PlaceholderVerticalAlign
import androidx.compose.ui.text.TextStyle
import androidx.compose.ui.unit.Dp
import androidx.compose.ui.unit.LayoutDirection.Ltr
import androidx.compose.ui.unit.dp
import androidx.compose.ui.unit.sp
import coil3.compose.AsyncImage
import coil3.compose.LocalPlatformContext
import coil3.request.ImageRequest
import coil3.size.Size
import org.jetbrains.jewel.foundation.ExperimentalJewelApi
import org.jetbrains.jewel.foundation.code.MimeType
import org.jetbrains.jewel.foundation.code.highlighting.LocalCodeHighlighter
import org.jetbrains.jewel.foundation.theme.LocalContentColor
import org.jetbrains.jewel.foundation.util.JewelLogger
import org.jetbrains.jewel.markdown.InlineMarkdown
import org.jetbrains.jewel.markdown.MarkdownBlock
import org.jetbrains.jewel.markdown.MarkdownBlock.BlockQuote
import org.jetbrains.jewel.markdown.MarkdownBlock.CodeBlock
Expand Down Expand Up @@ -106,6 +118,8 @@ public open class DefaultMarkdownBlockRenderer(
}
}

private data class ImageSize(val width: Int, val height: Int)

@Composable
override fun render(
block: Paragraph,
Expand All @@ -128,6 +142,7 @@ public open class DefaultMarkdownBlockRenderer(
.clickable(interactionSource = interactionSource, indication = null, onClick = onTextClick),
text = renderedContent,
style = mergedStyle,
inlineContent = renderedImages(block),
)
}

Expand Down Expand Up @@ -447,6 +462,50 @@ public open class DefaultMarkdownBlockRenderer(
inlineRenderer.renderAsAnnotatedString(block.inlineContent, styling, enabled, onUrlClick)
}

@Composable
private fun renderedImages(blockInlineContent: WithInlineMarkdown): Map<String, InlineTextContent> =
getImages(blockInlineContent).associate { image -> image.source to imageContent(image) }

@Composable
private fun imageContent(image: InlineMarkdown.Image): InlineTextContent {
val knownSize = remember(image.source) { mutableStateOf<ImageSize?>(null) }
return InlineTextContent(
with(LocalDensity.current) {
// `toSp` ensures that the placeholder size matches the original image size in
// pixels.
// This approach doesn't allow images from appearing larger with different screen
// scaling,
// but simply maintains behavior consistent with standalone AsyncImage rendering.
Placeholder(
width = knownSize.value?.width?.dp?.toSp() ?: 0.sp,
height = knownSize.value?.height?.dp?.toSp() ?: 1.sp,
placeholderVerticalAlign = PlaceholderVerticalAlign.Bottom,
)
}
) {
AsyncImage(
model =
ImageRequest.Builder(LocalPlatformContext.current)
.data(image.source)
// make sure image doesn't get downscaled to the placeholder size
.size(Size.ORIGINAL)
.build(),
contentDescription = image.title,
onSuccess = { state ->
// onSuccess should only be called once, but adding additional protection from
// unnecessary rerender
if (knownSize.value == null) {
knownSize.value = state.result.image.let { ImageSize(it.width, it.height) }
}
},
onError = { error ->
JewelLogger.getInstance("Jewel").warn("AsyncImage loading: ${error.result.throwable}")
},
modifier = knownSize.value?.let { Modifier.height(it.height.dp).width(it.width.dp) } ?: Modifier,
)
}
}

@Composable
protected fun MaybeScrollingContainer(
isScrollable: Boolean,
Expand Down Expand Up @@ -475,3 +534,21 @@ public open class DefaultMarkdownBlockRenderer(
override operator fun plus(extension: MarkdownRendererExtension): MarkdownBlockRenderer =
DefaultMarkdownBlockRenderer(rootStyling, rendererExtensions = rendererExtensions + extension, inlineRenderer)
}

private fun getImages(input: WithInlineMarkdown): List<InlineMarkdown.Image> = buildList {
fun collectImagesRecursively(items: List<InlineMarkdown>) {
for (item in items) {
when (item) {
is InlineMarkdown.Image -> {
if (item.source.isNotBlank()) add(item)
}
is WithInlineMarkdown -> {
collectImagesRecursively(item.inlineContent)
}

else -> {}
}
}
}
collectImagesRecursively(input.inlineContent)
}
2 changes: 2 additions & 0 deletions .../ide-plugin/src/main/kotlin/org/jetbrains/jewel/samples/ideplugin/ComponentShowcaseTab.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,8 @@ private fun MarkdownExample(project: Project) {
| * Tables
| * And more — I am running out of random things to say 😆
|
|![logo](https://avatars.githubusercontent.com/u/878437?s=48)
|
|```kotlin
|fun hello() = "World"
|```
Expand Down
2 changes: 2 additions & 0 deletions ...standalone/src/main/kotlin/org/jetbrains/jewel/samples/standalone/markdown/JewelReadme.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ desktop-optimized theme and set of components.
>
> Use at your own risk!

![logo](https://avatars.githubusercontent.com/u/878437?s=48)

Jewel provides an implementation of the IntelliJ Platform themes that can be used in any Compose for Desktop
application. Additionally, it has a Swing LaF Bridge that only works in the IntelliJ Platform (i.e., used to create IDE
plugins), but automatically mirrors the current Swing LaF into Compose for a native-looking, consistent UI.
Expand Down