Skip to content

Commit 2107ea7

Browse files
mikepenzmikepenz
committed
fix: resolve critical bugs in inline image placeholder sizing
Fix all P0 issues identified in PR #510 review: 1. Fix unit conversion bugs in ImageTransformer.placeholderConfig - Changed from mixed SP/PX units to consistent DP-based calculations - Convert all PX inputs (containerSize, imageSize) to DP first - Perform all calculations in DP units - Return Size in DP values for proper placeholder sizing - Rename constant: DEFAULT_IMAGE_SIDE_LENGTH_SP → DEFAULT_IMAGE_SIDE_LENGTH_DP 2. Fix typo: lowestRation → lowestRatio 3. Add node to remember keys in MarkdownText - Include node parameter in remember() keys to prevent stale node reuse - Remove imageSizeByLink from keys (keep snapshot only) 4. Fix dead callback mechanism for placeholder resizing - Create MarkdownInlineImageWithSize composable - Observe intrinsicSize when painter becomes available - Connect intrinsicSize detection to imageSizeChanged callback - Enable dynamic placeholder resizing when images load 5. Regenerate API dumps with correct signatures - Update placeholderConfig signature: 3 params → 5 params - Add containerSize, imageSize, and imageSizeChanged callback - Update all module API dumps (coil2, coil3, main, klib) All changes maintain backward compatibility through optional parameters and default implementations. API check and compilation verified.
1 parent cd8f8b8 commit 2107ea7

File tree

6 files changed

+92
-20
lines changed

6 files changed

+92
-20
lines changed

‎multiplatform-markdown-renderer-coil2/api/jvm/multiplatform-markdown-renderer-coil2.api‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ public final class com/mikepenz/markdown/coil2/Coil2ImageTransformerImpl : com/m
22
public static final field $stable I
33
public static final field INSTANCE Lcom/mikepenz/markdown/coil2/Coil2ImageTransformerImpl;
44
public fun intrinsicSize-bSu-EZI (Landroidx/compose/ui/graphics/painter/Painter;Landroidx/compose/runtime/Composer;I)J
5-
public fun placeholderConfig-cSwnlzA (Ljava/lang/String;Landroidx/compose/ui/unit/Density;J)Lcom/mikepenz/markdown/model/PlaceholderConfig;
5+
public fun placeholderConfig-M3cbxlo (Ljava/lang/String;Landroidx/compose/ui/unit/Density;JJLkotlin/jvm/functions/Function2;)Lcom/mikepenz/markdown/model/PlaceholderConfig;
66
public fun transform (Ljava/lang/String;Landroidx/compose/runtime/Composer;I)Lcom/mikepenz/markdown/model/ImageData;
77
}
88

‎multiplatform-markdown-renderer-coil3/api/jvm/multiplatform-markdown-renderer-coil3.api‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ public final class com/mikepenz/markdown/coil3/Coil3ImageTransformerImpl : com/m
22
public static final field $stable I
33
public static final field INSTANCE Lcom/mikepenz/markdown/coil3/Coil3ImageTransformerImpl;
44
public fun intrinsicSize-bSu-EZI (Landroidx/compose/ui/graphics/painter/Painter;Landroidx/compose/runtime/Composer;I)J
5-
public fun placeholderConfig-cSwnlzA (Ljava/lang/String;Landroidx/compose/ui/unit/Density;J)Lcom/mikepenz/markdown/model/PlaceholderConfig;
5+
public fun placeholderConfig-M3cbxlo (Ljava/lang/String;Landroidx/compose/ui/unit/Density;JJLkotlin/jvm/functions/Function2;)Lcom/mikepenz/markdown/model/PlaceholderConfig;
66
public fun transform (Ljava/lang/String;Landroidx/compose/runtime/Composer;I)Lcom/mikepenz/markdown/model/ImageData;
77
}
88

‎multiplatform-markdown-renderer/api/jvm/multiplatform-markdown-renderer.api‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -483,14 +483,22 @@ public final class com/mikepenz/markdown/model/ImageData {
483483
}
484484

485485
public abstract interface class com/mikepenz/markdown/model/ImageTransformer {
486+
public static final field Companion Lcom/mikepenz/markdown/model/ImageTransformer$Companion;
487+
public static final field DEFAULT_IMAGE_SIDE_LENGTH_DP F
486488
public fun intrinsicSize-bSu-EZI (Landroidx/compose/ui/graphics/painter/Painter;Landroidx/compose/runtime/Composer;I)J
487-
public fun placeholderConfig-cSwnlzA (Ljava/lang/String;Landroidx/compose/ui/unit/Density;J)Lcom/mikepenz/markdown/model/PlaceholderConfig;
489+
public fun placeholderConfig-M3cbxlo (Ljava/lang/String;Landroidx/compose/ui/unit/Density;JJLkotlin/jvm/functions/Function2;)Lcom/mikepenz/markdown/model/PlaceholderConfig;
490+
public static synthetic fun placeholderConfig-M3cbxlo$default (Lcom/mikepenz/markdown/model/ImageTransformer;Ljava/lang/String;Landroidx/compose/ui/unit/Density;JJLkotlin/jvm/functions/Function2;ILjava/lang/Object;)Lcom/mikepenz/markdown/model/PlaceholderConfig;
488491
public abstract fun transform (Ljava/lang/String;Landroidx/compose/runtime/Composer;I)Lcom/mikepenz/markdown/model/ImageData;
489492
}
490493

494+
public final class com/mikepenz/markdown/model/ImageTransformer$Companion {
495+
public static final field DEFAULT_IMAGE_SIDE_LENGTH_DP F
496+
}
497+
491498
public final class com/mikepenz/markdown/model/ImageTransformer$DefaultImpls {
492499
public static fun intrinsicSize-bSu-EZI (Lcom/mikepenz/markdown/model/ImageTransformer;Landroidx/compose/ui/graphics/painter/Painter;Landroidx/compose/runtime/Composer;I)J
493-
public static fun placeholderConfig-cSwnlzA (Lcom/mikepenz/markdown/model/ImageTransformer;Ljava/lang/String;Landroidx/compose/ui/unit/Density;J)Lcom/mikepenz/markdown/model/PlaceholderConfig;
500+
public static fun placeholderConfig-M3cbxlo (Lcom/mikepenz/markdown/model/ImageTransformer;Ljava/lang/String;Landroidx/compose/ui/unit/Density;JJLkotlin/jvm/functions/Function2;)Lcom/mikepenz/markdown/model/PlaceholderConfig;
501+
public static synthetic fun placeholderConfig-M3cbxlo$default (Lcom/mikepenz/markdown/model/ImageTransformer;Ljava/lang/String;Landroidx/compose/ui/unit/Density;JJLkotlin/jvm/functions/Function2;ILjava/lang/Object;)Lcom/mikepenz/markdown/model/PlaceholderConfig;
494502
}
495503

496504
public final class com/mikepenz/markdown/model/Input {
@@ -643,7 +651,7 @@ public final class com/mikepenz/markdown/model/NoOpImageTransformerImpl : com/mi
643651
public static final field $stable I
644652
public fun <init> ()V
645653
public fun intrinsicSize-bSu-EZI (Landroidx/compose/ui/graphics/painter/Painter;Landroidx/compose/runtime/Composer;I)J
646-
public fun placeholderConfig-cSwnlzA (Ljava/lang/String;Landroidx/compose/ui/unit/Density;J)Lcom/mikepenz/markdown/model/PlaceholderConfig;
654+
public fun placeholderConfig-M3cbxlo (Ljava/lang/String;Landroidx/compose/ui/unit/Density;JJLkotlin/jvm/functions/Function2;)Lcom/mikepenz/markdown/model/PlaceholderConfig;
647655
public fun transform (Ljava/lang/String;Landroidx/compose/runtime/Composer;I)Lcom/mikepenz/markdown/model/ImageData;
648656
}
649657

‎multiplatform-markdown-renderer/api/multiplatform-markdown-renderer.klib.api‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,12 @@ abstract interface com.mikepenz.markdown.compose.components/MarkdownComponents {
7777
abstract interface com.mikepenz.markdown.model/ImageTransformer { // com.mikepenz.markdown.model/ImageTransformer|null[0]
7878
abstract fun transform(kotlin/String, androidx.compose.runtime/Composer?, kotlin/Int): com.mikepenz.markdown.model/ImageData? // com.mikepenz.markdown.model/ImageTransformer.transform|transform(kotlin.String;androidx.compose.runtime.Composer?;kotlin.Int){}[0]
7979
open fun intrinsicSize(androidx.compose.ui.graphics.painter/Painter, androidx.compose.runtime/Composer?, kotlin/Int): androidx.compose.ui.geometry/Size // com.mikepenz.markdown.model/ImageTransformer.intrinsicSize|intrinsicSize(androidx.compose.ui.graphics.painter.Painter;androidx.compose.runtime.Composer?;kotlin.Int){}[0]
80-
open fun placeholderConfig(kotlin/String, androidx.compose.ui.unit/Density, androidx.compose.ui.geometry/Size): com.mikepenz.markdown.model/PlaceholderConfig // com.mikepenz.markdown.model/ImageTransformer.placeholderConfig|placeholderConfig(kotlin.String;androidx.compose.ui.unit.Density;androidx.compose.ui.geometry.Size){}[0]
80+
open fun placeholderConfig(kotlin/String, androidx.compose.ui.unit/Density, androidx.compose.ui.geometry/Size, androidx.compose.ui.geometry/Size, kotlin/Function2<kotlin/String, androidx.compose.ui.geometry/Size, kotlin/Unit>? = ...): com.mikepenz.markdown.model/PlaceholderConfig // com.mikepenz.markdown.model/ImageTransformer.placeholderConfig|placeholderConfig(kotlin.String;androidx.compose.ui.unit.Density;androidx.compose.ui.geometry.Size;androidx.compose.ui.geometry.Size;kotlin.Function2<kotlin.String,androidx.compose.ui.geometry.Size,kotlin.Unit>?){}[0]
81+
82+
final object Companion { // com.mikepenz.markdown.model/ImageTransformer.Companion|null[0]
83+
final const val DEFAULT_IMAGE_SIDE_LENGTH_DP // com.mikepenz.markdown.model/ImageTransformer.Companion.DEFAULT_IMAGE_SIDE_LENGTH_DP|{}DEFAULT_IMAGE_SIDE_LENGTH_DP[0]
84+
final fun <get-DEFAULT_IMAGE_SIDE_LENGTH_DP>(): kotlin/Float // com.mikepenz.markdown.model/ImageTransformer.Companion.DEFAULT_IMAGE_SIDE_LENGTH_DP.<get-DEFAULT_IMAGE_SIDE_LENGTH_DP>|<get-DEFAULT_IMAGE_SIDE_LENGTH_DP>(){}[0]
85+
}
8186
}
8287

8388
abstract interface com.mikepenz.markdown.model/MarkdownAnimations { // com.mikepenz.markdown.model/MarkdownAnimations|null[0]

‎multiplatform-markdown-renderer/src/commonMain/kotlin/com/mikepenz/markdown/compose/elements/MarkdownText.kt‎

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import androidx.compose.ui.text.TextLayoutResult
1717
import androidx.compose.ui.text.TextStyle
1818
import androidx.compose.ui.text.buildAnnotatedString
1919
import androidx.compose.ui.unit.Density
20+
import androidx.compose.ui.unit.dp
2021
import androidx.compose.ui.unit.sp
2122
import androidx.compose.ui.unit.toSize
2223
import com.mikepenz.markdown.annotator.AnnotatorSettings
@@ -136,7 +137,7 @@ fun MarkdownText(
136137
},
137138
style = style,
138139
inlineContent = imageSizeByLink.toPersistentMap().let { imageSizeByLinkSnapshot ->
139-
remember(inlineContent.inlineContent, content, containerSize.value, transformer, imageSizeByLink, imageSizeByLinkSnapshot) {
140+
remember(node, inlineContent.inlineContent, content, containerSize.value, transformer, imageSizeByLinkSnapshot) {
140141
inlineContent.inlineContent + buildImageInlineContent(
141142
content,
142143
node,
@@ -170,14 +171,54 @@ private fun buildImageInlineContent(
170171
.distinctBy { it.item }
171172
.associate { annotation ->
172173
val url = annotation.item.removePrefix("${MARKDOWN_TAG_IMAGE_URL}_")
174+
175+
// Try to get stored size, or use default
173176
val imageSize = imageSizeByLink[url] ?: defaultImageSize
177+
174178
val config = transformer.placeholderConfig(url, density, containerSize, imageSize, imageSizeChanged)
179+
// Config size is in DP, convert to SP for Placeholder TextUnit
175180
annotation.item to InlineTextContent(
176-
Placeholder(config.size.width.sp, config.size.height.sp, config.verticalAlign)
181+
Placeholder(
182+
width = with(density) { config.size.width.dp.toSp() },
183+
height = with(density) { config.size.height.dp.toSp() },
184+
placeholderVerticalAlign = config.verticalAlign
185+
)
177186
) { link ->
178-
LocalMarkdownComponents.current.inlineImage(
179-
MarkdownComponentModel(link, node, LocalMarkdownTypography.current)
187+
// Render the image and observe its intrinsic size
188+
MarkdownInlineImageWithSize(
189+
link = link,
190+
node = node,
191+
transformer = transformer,
192+
density = density,
193+
onSizeDetected = { detectedSize ->
194+
// Update size cache when image loads
195+
imageSizeChanged?.invoke(url, detectedSize)
196+
}
180197
)
181198
}
182199
}
183200
}
201+
202+
@Composable
203+
private fun MarkdownInlineImageWithSize(
204+
link: String,
205+
node: ASTNode,
206+
transformer: ImageTransformer,
207+
density: Density,
208+
onSizeDetected: (Size) -> Unit
209+
) {
210+
val imageData = transformer.transform(link)
211+
212+
// Detect intrinsic size when painter becomes available
213+
imageData?.let {
214+
val intrinsicSize = transformer.intrinsicSize(it.painter)
215+
if (intrinsicSize != Size.Unspecified) {
216+
onSizeDetected(intrinsicSize)
217+
}
218+
}
219+
220+
// Delegate to the customizable component
221+
LocalMarkdownComponents.current.inlineImage(
222+
MarkdownComponentModel(link, node, LocalMarkdownTypography.current)
223+
)
224+
}

‎multiplatform-markdown-renderer/src/commonMain/kotlin/com/mikepenz/markdown/model/ImageTransformer.kt‎

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,27 +43,45 @@ interface ImageTransformer {
4343
return Size(size.width * factor, size.height * factor)
4444
}
4545

46-
return PlaceholderConfig(with(density) {
46+
// Work entirely in DP for consistent unit handling
47+
// containerSize and imageSize are in PX, convert to DP
48+
val maxSideDp = DEFAULT_IMAGE_SIDE_LENGTH_DP
49+
50+
val sizeInDp = with(density) {
4751
if (containerSize.isUnspecified) {
48-
Size(DEFAULT_IMAGE_SIDE_LENGTH_SP, DEFAULT_IMAGE_SIDE_LENGTH_SP)
52+
// No container size known, use default
53+
Size(maxSideDp, maxSideDp)
4954
} else if (imageSize.isUnspecified) {
50-
val actualSideLength = minOf(DEFAULT_IMAGE_SIDE_LENGTH_SP, containerSize.height, containerSize.width).toSp().value
55+
// No image size known yet, use container-constrained max side
56+
val containerWidthDp = containerSize.width.toDp().value
57+
val containerHeightDp = containerSize.height.toDp().value
58+
val actualSideLength = minOf(maxSideDp, containerHeightDp, containerWidthDp)
5159
Size(actualSideLength, actualSideLength)
5260
} else {
53-
val actualHeight = minOf(imageSize.height, containerSize.height).toSp().value
54-
val actualWidth = minOf(imageSize.width, containerSize.width).toSp().value
55-
if (actualWidth < imageSize.width || actualHeight < imageSize.height) {
56-
val lowestRation = minOf(actualWidth / imageSize.width, actualHeight / imageSize.height)
57-
scale(imageSize, lowestRation)
61+
// Both sizes known, calculate scaled size to fit container
62+
val imageWidthDp = imageSize.width.toDp().value
63+
val imageHeightDp = imageSize.height.toDp().value
64+
val containerWidthDp = containerSize.width.toDp().value
65+
val containerHeightDp = containerSize.height.toDp().value
66+
67+
val actualHeight = minOf(imageHeightDp, containerHeightDp, maxSideDp)
68+
val actualWidth = minOf(imageWidthDp, containerWidthDp, maxSideDp)
69+
70+
if (actualWidth < imageWidthDp || actualHeight < imageHeightDp) {
71+
// Image needs scaling down to fit
72+
val lowestRatio = minOf(actualWidth / imageWidthDp, actualHeight / imageHeightDp)
73+
scale(Size(imageWidthDp, imageHeightDp), lowestRatio)
5874
} else {
5975
Size(actualWidth, actualHeight)
6076
}
6177
}
62-
})
78+
}
79+
80+
return PlaceholderConfig(sizeInDp)
6381
}
6482

6583
companion object {
66-
const val DEFAULT_IMAGE_SIDE_LENGTH_SP = 180f
84+
const val DEFAULT_IMAGE_SIDE_LENGTH_DP = 100f
6785
}
6886
}
6987

0 commit comments

Comments
 (0)