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

obask · 2025-01-21T17:12:18Z

It supports every image as an inline node

obask · 2025-01-24T13:21:44Z

currently there are 2 differences with a JCEF render: JCEF renders SVG as a giant image while we keep it small(I guess it just depends on a default size parameter), and the text in build|failing is not shown for some reason...

rock3r · 2025-01-24T13:34:09Z

They look like entirely different images... Are you sure it's the same image it's being displayed, and that it's loading correctly in the first case? That to me looks like a "broken image" placeholder (which also is way too big)

SVG files have an intrinsic size declared in their root node, and any renderer should adhere to that

obask · 2025-01-24T15:29:12Z

images I posted earlier are examples of how JCEF renders my "README.md" which this is how compose shows the same things:

README.md

rock3r · 2025-01-27T09:37:00Z

I see. I had misinterpreted the original message, sorry 🙏

The first image (the broken square) has a declared size of 800x800 px, encoded as width="800px" height="800px", which browsers seem to accept but maybe the Compose renderer doesn't like, and falls back to the viewBox size of 16x16. That would explain why it's larger on JCEF.

As for the latter, I think it's because the text for whatever reason just doesn't get rendered:

<g text-anchor="middle" font-family="DejaVu Sans,Verdana,Geneva,sans-serif" font-size="11">
    <use x="5.5" y="2" fill="#010101" fill-opacity=".3" href="#logo" />
    <use x="5.5" y="1" fill="#fff" href="#logo" />
  <text x="40.5" y="15" fill="#010101" fill-opacity=".3">build</text>
  <text x="40.5" y="14" fill="#fff">build</text>
  <text x="83.5" y="15" fill="#010101" fill-opacity=".3">passing</text>
  <text x="83.5" y="14" fill="#fff">passing</text>
</g>

Again, possibly some unsupported value in there that Skia ignores.

That's sort of weird since Skia can interpret image sizes with units, and render text — it does in Chrome! But maybe there is some setup that is missing, or Chrome does some magic stuff. Either way, these are either CMP or Skiko bugs :)

obask · 2025-01-31T13:25:25Z

This fixes both:

obask · 2025-01-31T13:27:25Z

The first image (the broken square) has a declared size of 800x800 px, encoded as width="800px" height="800px", which browsers seem to accept but maybe the Compose renderer doesn't like, and falls back to the viewBox size of 16x16. That would explain why it's larger on JCEF.

As for the latter, I think it's because the text for whatever reason just doesn't get rendered:

Again, possibly some unsupported value in there that Skia ignores.

I'm going to create a separate issue for SVG regressions. Should it go to youtrack?

.../core/src/main/kotlin/org/jetbrains/jewel/markdown/rendering/DefaultMarkdownBlockRenderer.kt

rock3r · 2025-01-31T14:15:13Z

Should it go to youtrack?

Please, no. Stick with GH until further notice. You'll know together with everyone else when YT is usable. We're still waiting.

rock3r · 2025-02-06T15:33:17Z

platform/jewel/markdown/core/src/main/kotlin/org/jetbrains/jewel/markdown/Markdown.kt

@@ -76,6 +80,7 @@ public fun Markdown(
    markdownStyling: MarkdownStyling = JewelTheme.markdownStyling,
    blockRenderer: MarkdownBlockRenderer = DefaultMarkdownBlockRenderer(markdownStyling),
 ) {
+    setSingletonImageLoaderFactory(::createImageLoader)


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

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.

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.

platform/jewel/markdown/core/src/main/kotlin/org/jetbrains/jewel/markdown/Markdown.kt

...l/markdown/core/src/main/kotlin/org/jetbrains/jewel/markdown/processing/MarkdownProcessor.kt

...core/src/main/kotlin/org/jetbrains/jewel/markdown/rendering/DefaultInlineMarkdownRenderer.kt

.../core/src/main/kotlin/org/jetbrains/jewel/markdown/rendering/DefaultMarkdownBlockRenderer.kt

rock3r · 2025-02-06T15:49:02Z

.../core/src/main/kotlin/org/jetbrains/jewel/markdown/rendering/DefaultMarkdownBlockRenderer.kt

+                        .build(),
+                contentDescription = image.title,
+                onSuccess = { state ->
+                    if (knownSize.value == null) {


Is the check needed? I'd imagine whatever we get in onSuccess we only get once anyway...

since changing knownSize.value could trigger re rendering, it's better to be safe than end up in an infinite loop. Here is serves the goal of changing the placeholder.

rock3r · 2025-02-06T16:03:17Z

Out of curiosity, have you filed the issues for SVG rendering?

I think the size one is caused by https://github.com/coil-kt/coil/blob/main/coil-svg/src/commonMain/kotlin/coil3/svg/SvgDecoder.kt#L39 this parameter being true by default. Ideally, users should be able to override the Coil loader with one they want, but we can keep it for a follow up (it'd be nice to have an issue tracking this in our YouTrack, if you can file it).

The text one is the more baffling one. If you can look into it, maybe you can figure out what is the cause, or at least where it should happen, and file an issue for that in the right place.

rock3r · 2025-02-11T16:20:25Z

@obask ping

obask · 2025-02-12T12:35:14Z

Out of curiosity, have you filed the issues for SVG rendering?

Thanks for doing initial investigation. I filed a tracking issue, but not the one in a coil repository yet.
https://youtrack.jetbrains.com/issue/JEWEL-762/Coil-image-rendering-issues-with-SVG

It supports every image as an inline node; Using built-in coroutine library and ktor2 from the platform; Added SVG support using a coil dependency.

rock3r self-requested a review January 21, 2025 17:18

obask mentioned this pull request Jan 21, 2025

Load markdown images with Coil3 JetBrains/jewel#327

Draft

obask force-pushed the images branch 3 times, most recently from b0ba3c3 to 7fdb5ac Compare January 23, 2025 15:31

obask force-pushed the images branch from 7fdb5ac to 762068f Compare January 31, 2025 13:08

obask changed the title ~~[JEWEL-TBD] Load markdown images using Coil3~~ [JEWEL-327] Load markdown images using Coil3 Jan 31, 2025

obask changed the title ~~[JEWEL-327] Load markdown images using Coil3~~ [JEWEL-746] Load inline markdown images using Coil3 Jan 31, 2025

obask marked this pull request as ready for review January 31, 2025 13:26

obask commented Jan 31, 2025

View reviewed changes

.../core/src/main/kotlin/org/jetbrains/jewel/markdown/rendering/DefaultMarkdownBlockRenderer.kt Outdated Show resolved Hide resolved

obask force-pushed the images branch from 762068f to e75e946 Compare January 31, 2025 13:31

obask self-assigned this Jan 31, 2025

rock3r reviewed Feb 6, 2025

View reviewed changes

rock3r added the jewel label Feb 11, 2025

obask force-pushed the images branch 3 times, most recently from 97a2f31 to 96b68a0 Compare February 12, 2025 17:14

[JEWEL-746] Load markdown images using Coil3

a415d3d

It supports every image as an inline node; Using built-in coroutine library and ktor2 from the platform; Added SVG support using a coil dependency.

obask force-pushed the images branch from 96b68a0 to a415d3d Compare February 13, 2025 10:56

obask requested a review from rock3r February 13, 2025 10:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

obask commented Jan 21, 2025

obask commented Jan 24, 2025

rock3r commented Jan 24, 2025

obask commented Jan 24, 2025

rock3r commented Jan 27, 2025 •

edited

Loading

obask commented Jan 31, 2025

obask commented Jan 31, 2025

rock3r commented Jan 31, 2025 •

edited

Loading

rock3r Feb 6, 2025

obask Feb 12, 2025

rock3r Feb 13, 2025

rock3r Feb 6, 2025

obask Feb 12, 2025

rock3r commented Feb 6, 2025

rock3r commented Feb 11, 2025

obask commented Feb 12, 2025

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

Are you sure you want to change the base?

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

Conversation

obask commented Jan 21, 2025

obask commented Jan 24, 2025

rock3r commented Jan 24, 2025

obask commented Jan 24, 2025

rock3r commented Jan 27, 2025 • edited Loading

obask commented Jan 31, 2025

obask commented Jan 31, 2025

rock3r commented Jan 31, 2025 • edited Loading

rock3r Feb 6, 2025

Choose a reason for hiding this comment

obask Feb 12, 2025

Choose a reason for hiding this comment

rock3r Feb 13, 2025

Choose a reason for hiding this comment

rock3r Feb 6, 2025

Choose a reason for hiding this comment

obask Feb 12, 2025

Choose a reason for hiding this comment

rock3r commented Feb 6, 2025

rock3r commented Feb 11, 2025

obask commented Feb 12, 2025

rock3r commented Jan 27, 2025 •

edited

Loading

rock3r commented Jan 31, 2025 •

edited

Loading