mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-06 11:48:24 +00:00
Merge pull request #3913 from vitorpamplona/claude/pdf-preview-aspect-ratio-uwtui3
Fix PDF preview layout stability and aspect ratio handling
This commit is contained in:
@@ -56,12 +56,16 @@ object MediaAspectRatioCache : MutableMediaAspectRatioCache {
|
||||
|
||||
override fun get(url: String): Float? = entry(url).value
|
||||
|
||||
// Both sides are checked, not just the divisor: a zero width divides cleanly to 0f, and 0f is
|
||||
// just as unusable downstream as a division by zero — `Modifier.aspectRatio` throws on it. A
|
||||
// reported size that cannot be laid out is stored as no size at all, leaving the entry empty
|
||||
// for a later, better report to fill.
|
||||
override fun add(
|
||||
url: String,
|
||||
width: Int,
|
||||
height: Int,
|
||||
) {
|
||||
if (height > 1) {
|
||||
if (width > 0 && height > 1) {
|
||||
entry(url).value = width.toFloat() / height.toFloat()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -66,11 +66,11 @@ fun GifVideoView(
|
||||
accountViewModel: AccountViewModel,
|
||||
thumbhash: String? = null,
|
||||
) {
|
||||
// Pure read path — DimensionTag.aspectRatio() is a one-line int division and
|
||||
// Pure read path — DimensionTag.aspectRatioOrNull() is a one-line int division and
|
||||
// MediaAspectRatioCache.get() is a synchronized LruCache lookup. Wrapping this in
|
||||
// remember() to avoid the recompute would cost more (slot read + N equality checks)
|
||||
// than the work it saves; that's why this stays as a plain expression.
|
||||
val ratio = dimensions?.aspectRatio() ?: MediaAspectRatioCache.get(videoUri)
|
||||
val ratio = dimensions?.aspectRatioOrNull() ?: MediaAspectRatioCache.get(videoUri)
|
||||
val autoPlay = accountViewModel.settings.autoPlayVideos()
|
||||
val borderModifier = if (roundedCorner) MaterialTheme.colorScheme.imageModifier else Modifier
|
||||
val context = LocalContext.current
|
||||
|
||||
@@ -62,11 +62,7 @@ private data class FirstImageOrientation(
|
||||
val isLandscape: Boolean,
|
||||
)
|
||||
|
||||
private fun MediaUrlImage.resolvedAspectRatio(): Float? =
|
||||
dim
|
||||
?.takeIf { it.hasSize() }
|
||||
?.aspectRatio()
|
||||
?: MediaAspectRatioCache.get(url)
|
||||
private fun MediaUrlImage.resolvedAspectRatio(): Float? = dim?.aspectRatioOrNull() ?: MediaAspectRatioCache.get(url)
|
||||
|
||||
private fun MediaUrlImage?.resolveOrientation(
|
||||
landscapeDefaultAspectRatio: Float,
|
||||
|
||||
+11
-1
@@ -240,13 +240,23 @@ fun BlurhashGridBackdrop(media: List<MediaUrlImage>) {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Sizes a media container from a reported width/height ratio, falling back to width-only when
|
||||
* there is no usable one.
|
||||
*
|
||||
* A ratio that is not finite and positive counts as no ratio: `Modifier.aspectRatio` throws
|
||||
* `IllegalArgumentException` on `0f` and on `NaN`, which would take down the composition around
|
||||
* the media rather than just mis-sizing it. Callers are expected to hand over a clean value
|
||||
* (`DimensionTag.aspectRatioOrNull`, `MediaAspectRatioCache`), so this is the backstop for the one
|
||||
* that forgets, not the primary guard.
|
||||
*/
|
||||
fun mediaSizingModifier(
|
||||
ratio: Float?,
|
||||
contentScale: ContentScale,
|
||||
): Modifier =
|
||||
when {
|
||||
contentScale == ContentScale.Crop -> Modifier.fillMaxSize()
|
||||
ratio != null -> Modifier.fillMaxWidth().aspectRatio(ratio)
|
||||
ratio != null && ratio > 0f && ratio.isFinite() -> Modifier.fillMaxWidth().aspectRatio(ratio)
|
||||
else -> Modifier.fillMaxWidth()
|
||||
}
|
||||
|
||||
|
||||
+2
-2
@@ -628,7 +628,7 @@ private fun RenderImageOrVideo(
|
||||
Modifier.fillMaxWidth()
|
||||
}
|
||||
|
||||
val ratio = content.dim?.aspectRatio() ?: MediaAspectRatioCache.get(content.url)
|
||||
val ratio = content.dim?.aspectRatioOrNull() ?: MediaAspectRatioCache.get(content.url)
|
||||
val useLocalBlossomBridge by accountViewModel.useLocalBlossomBridge.collectAsStateWithLifecycle()
|
||||
val bridgedUrl =
|
||||
remember(content.url, useLocalBlossomBridge) {
|
||||
@@ -694,7 +694,7 @@ private fun RenderImageOrVideo(
|
||||
}
|
||||
|
||||
content.localFile?.let {
|
||||
val ratio = content.dim?.aspectRatio() ?: MediaAspectRatioCache.get(it.toUri().toString())
|
||||
val ratio = content.dim?.aspectRatioOrNull() ?: MediaAspectRatioCache.get(it.toUri().toString())
|
||||
|
||||
val modifier =
|
||||
if (ratio != null) {
|
||||
|
||||
+4
-4
@@ -181,7 +181,7 @@ fun ZoomableContentView(
|
||||
|
||||
when (content) {
|
||||
is MediaUrlImage -> {
|
||||
val ratio = content.dim?.aspectRatio() ?: MediaAspectRatioCache.get(content.url)
|
||||
val ratio = content.dim?.aspectRatioOrNull() ?: MediaAspectRatioCache.get(content.url)
|
||||
val bridgedUrl =
|
||||
remember(content.url, useLocalBlossomBridge) {
|
||||
content.toCoilModel(useLocalBlossomBridge)
|
||||
@@ -228,7 +228,7 @@ fun ZoomableContentView(
|
||||
unknownMediaAspectRatio(content.mimeType, content.url)
|
||||
}
|
||||
val ratio =
|
||||
content.dim?.aspectRatio()
|
||||
content.dim?.aspectRatioOrNull()
|
||||
?: MediaAspectRatioCache.get(content.url)
|
||||
?: fallbackRatio
|
||||
val bridgedUrl =
|
||||
@@ -386,7 +386,7 @@ fun LocalImageView(
|
||||
)
|
||||
}
|
||||
|
||||
val ratio = remember(content) { content.dim?.aspectRatio() ?: MediaAspectRatioCache.get(content.localFile.toString()) }
|
||||
val ratio = remember(content) { content.dim?.aspectRatioOrNull() ?: MediaAspectRatioCache.get(content.localFile.toString()) }
|
||||
val context = LocalContext.current
|
||||
val imageModel =
|
||||
if (fullResolution) {
|
||||
@@ -510,7 +510,7 @@ fun UrlImageView(
|
||||
alwayShowImage: Boolean = false,
|
||||
fullResolution: Boolean = false,
|
||||
) {
|
||||
val ratio = content.dim?.aspectRatio() ?: MediaAspectRatioCache.get(content.url)
|
||||
val ratio = content.dim?.aspectRatioOrNull() ?: MediaAspectRatioCache.get(content.url)
|
||||
|
||||
val showImage =
|
||||
remember {
|
||||
|
||||
@@ -41,11 +41,16 @@ object PdfFetcher {
|
||||
suspend fun fetchSnapshot(
|
||||
url: String,
|
||||
okHttpClient: (String) -> OkHttpClient,
|
||||
): DiskCache.Snapshot {
|
||||
val diskCache = Amethyst.instance.diskCache
|
||||
diskCache.openSnapshot(url)?.let { return it }
|
||||
): DiskCache.Snapshot =
|
||||
withContext(Dispatchers.IO) {
|
||||
val diskCache = Amethyst.instance.diskCache
|
||||
// Covers the cache-hit fast path too, not just the download below it. openSnapshot()
|
||||
// contends on the global DiskLruCache lock, which Coil's cleanup pass holds across a
|
||||
// burst of unlink syscalls (see DeferredDeleteFileSystem) — calling it from a caller
|
||||
// that happens to be on the main thread stalls the frame for that whole burst, and the
|
||||
// hit path is exactly the one a feed takes when a PDF card scrolls back into view.
|
||||
diskCache.openSnapshot(url)?.let { return@withContext it }
|
||||
|
||||
return withContext(Dispatchers.IO) {
|
||||
val editor = diskCache.openEditor(url) ?: throw IOException("Unable to open cache editor for $url")
|
||||
try {
|
||||
val request =
|
||||
@@ -71,5 +76,4 @@ object PdfFetcher {
|
||||
throw t
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+84
-7
@@ -26,6 +26,7 @@ import android.os.ParcelFileDescriptor
|
||||
import androidx.compose.foundation.ExperimentalFoundationApi
|
||||
import androidx.compose.foundation.Image
|
||||
import androidx.compose.foundation.combinedClickable
|
||||
import androidx.compose.foundation.layout.Box
|
||||
import androidx.compose.foundation.layout.Column
|
||||
import androidx.compose.foundation.layout.Spacer
|
||||
import androidx.compose.foundation.layout.aspectRatio
|
||||
@@ -36,6 +37,7 @@ import androidx.compose.runtime.getValue
|
||||
import androidx.compose.runtime.mutableStateOf
|
||||
import androidx.compose.runtime.produceState
|
||||
import androidx.compose.runtime.remember
|
||||
import androidx.compose.ui.Alignment
|
||||
import androidx.compose.ui.Modifier
|
||||
import androidx.compose.ui.graphics.FilterQuality
|
||||
import androidx.compose.ui.graphics.asImageBitmap
|
||||
@@ -44,11 +46,15 @@ import androidx.compose.ui.platform.LocalWindowInfo
|
||||
import androidx.core.graphics.createBitmap
|
||||
import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols
|
||||
import com.vitorpamplona.amethyst.commons.richtext.MediaUrlPdf
|
||||
import com.vitorpamplona.amethyst.commons.ui.components.LoadingAnimation
|
||||
import com.vitorpamplona.amethyst.model.MediaAspectRatioCache
|
||||
import com.vitorpamplona.amethyst.ui.components.ClickableUrl
|
||||
import com.vitorpamplona.amethyst.ui.components.FileAttachmentRow
|
||||
import com.vitorpamplona.amethyst.ui.components.ShareMediaAction
|
||||
import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel
|
||||
import com.vitorpamplona.amethyst.ui.theme.DoubleVertSpacer
|
||||
import com.vitorpamplona.amethyst.ui.theme.Size40dp
|
||||
import com.vitorpamplona.amethyst.ui.theme.Size6dp
|
||||
import com.vitorpamplona.amethyst.ui.theme.innerPostModifier
|
||||
import com.vitorpamplona.quartz.utils.Log
|
||||
import kotlinx.coroutines.CancellationException
|
||||
@@ -58,11 +64,43 @@ import kotlinx.coroutines.withContext
|
||||
// Hard ceiling on the inline thumbnail bitmap, in pixels. Prevents OOM on very tall/large pages.
|
||||
private const val THUMBNAIL_MAX_DIM_PX = 1600
|
||||
|
||||
// Floor for the width/height ratio the card lays out with. A pathologically tall page (a receipt,
|
||||
// a single-column banner) would otherwise reserve a screenful of height for a sliver of content.
|
||||
private const val MIN_PREVIEW_ASPECT_RATIO = 0.2f
|
||||
|
||||
// Shape to assume when the page reports no usable size at all. US Letter portrait — the
|
||||
// overwhelmingly common page shape, and the least surprising box to hold open for an unknown one.
|
||||
private const val DEFAULT_PAGE_ASPECT_RATIO = 612f / 792f
|
||||
|
||||
/**
|
||||
* The ratio the card actually lays out with, given a page's natural width/height. The placeholder
|
||||
* and the loaded thumbnail both go through here so the box reserved while loading is the exact box
|
||||
* the rendered page lands in — the clamp has to be applied on both sides or the reservation is
|
||||
* wrong for the very pages it exists to protect.
|
||||
*
|
||||
* This is also the one place that guarantees `Modifier.aspectRatio` is handed a finite, positive
|
||||
* number. It throws `IllegalArgumentException` on `0f` and on `NaN`, and the clamp below does not
|
||||
* catch either: `NaN.coerceAtLeast(x)` is `NaN`, because every comparison against `NaN` is false.
|
||||
* Both are reachable from real input — a malformed PDF whose first page measures 0x0, or an imeta
|
||||
* `dim` that survives DimensionTag.parse's only-rejects-literal-"0x0" check as 0x0 anyway
|
||||
* (`"0.4x0.4"` truncates to it). Without this guard either one takes down the whole feed's
|
||||
* composition, from a tag any relay can carry.
|
||||
*/
|
||||
internal fun previewAspectRatio(pageAspectRatio: Float): Float =
|
||||
if (!pageAspectRatio.isFinite() || pageAspectRatio <= 0f) {
|
||||
DEFAULT_PAGE_ASPECT_RATIO
|
||||
} else {
|
||||
pageAspectRatio.coerceAtLeast(MIN_PREVIEW_ASPECT_RATIO)
|
||||
}
|
||||
|
||||
data class PdfPreview(
|
||||
val thumbnail: Bitmap,
|
||||
val pageCount: Int,
|
||||
val aspectRatio: Float,
|
||||
)
|
||||
val pageWidth: Int,
|
||||
val pageHeight: Int,
|
||||
) {
|
||||
val aspectRatio: Float = pageWidth.toFloat() / pageHeight.toFloat()
|
||||
}
|
||||
|
||||
private sealed class PdfLoadState {
|
||||
data object Loading : PdfLoadState()
|
||||
@@ -104,6 +142,14 @@ private fun LoadedPdfPreviewCard(
|
||||
containerWidthPx.coerceAtMost(THUMBNAIL_MAX_DIM_PX).coerceAtLeast(1)
|
||||
}
|
||||
|
||||
// Read in composition — never inside a remember — so this recomposes the moment the render
|
||||
// below fills the entry in. On a revisit the entry is already there and the placeholder can
|
||||
// reserve the right box from the first frame, which is the whole point of caching it.
|
||||
// The cache is consulted ahead of the imeta `dim` because it holds the page size this card
|
||||
// measured itself: an author-supplied `dim` that disagrees would guarantee the jump on every
|
||||
// single visit, which is exactly what this is here to stop.
|
||||
val knownAspectRatio = MediaAspectRatioCache.get(content.url) ?: content.dim?.aspectRatioOrNull()
|
||||
|
||||
@Suppress("ProduceStateDoesNotAssignValue")
|
||||
val state by produceState<PdfLoadState>(initialValue = PdfLoadState.Loading, key1 = content.url, key2 = targetWidthPx) {
|
||||
value =
|
||||
@@ -115,6 +161,13 @@ private fun LoadedPdfPreviewCard(
|
||||
withContext(Dispatchers.IO) {
|
||||
renderFirstPage(snapshot.data.toFile(), targetWidthPx)
|
||||
}
|
||||
}.also { result ->
|
||||
// Same cache the image and video paths use, so a PDF that has been rendered
|
||||
// once lays out at its real shape on every later visit instead of growing
|
||||
// from a bare filename row into a full page.
|
||||
if (result is PdfLoadState.Ready) {
|
||||
MediaAspectRatioCache.add(content.url, result.preview.pageWidth, result.preview.pageHeight)
|
||||
}
|
||||
}
|
||||
} catch (e: Exception) {
|
||||
if (e is CancellationException) throw e
|
||||
@@ -134,7 +187,7 @@ private fun LoadedPdfPreviewCard(
|
||||
|
||||
when (val current = state) {
|
||||
is PdfLoadState.Loading -> {
|
||||
PdfSkeletonCard(filename)
|
||||
PdfSkeletonCard(filename, knownAspectRatio)
|
||||
}
|
||||
|
||||
is PdfLoadState.Failed -> {
|
||||
@@ -150,15 +203,20 @@ private fun LoadedPdfPreviewCard(
|
||||
onLongClick = { sharePopupExpanded.value = true },
|
||||
),
|
||||
) {
|
||||
// asImageBitmap() allocates a fresh wrapper on every call and the wrapper compares
|
||||
// by identity, so calling it inline would defeat the remember(bitmap) that Image
|
||||
// uses to hold its BitmapPainter — every recomposition would rebuild the painter.
|
||||
val thumbnail = remember(current.preview.thumbnail) { current.preview.thumbnail.asImageBitmap() }
|
||||
|
||||
Image(
|
||||
bitmap = current.preview.thumbnail.asImageBitmap(),
|
||||
bitmap = thumbnail,
|
||||
contentDescription = content.description ?: filename,
|
||||
contentScale = ContentScale.FillWidth,
|
||||
filterQuality = FilterQuality.High,
|
||||
modifier =
|
||||
Modifier
|
||||
.fillMaxWidth()
|
||||
.aspectRatio(current.preview.aspectRatio.coerceAtLeast(0.2f)),
|
||||
.aspectRatio(previewAspectRatio(current.preview.aspectRatio)),
|
||||
)
|
||||
|
||||
FilenameRow(filename = filename, subtitle = pageCountLabel(current.preview.pageCount))
|
||||
@@ -187,8 +245,26 @@ private fun PlaceholderPdfCard(
|
||||
}
|
||||
|
||||
@Composable
|
||||
private fun PdfSkeletonCard(filename: String) {
|
||||
private fun PdfSkeletonCard(
|
||||
filename: String,
|
||||
aspectRatio: Float?,
|
||||
) {
|
||||
Column(modifier = MaterialTheme.colorScheme.innerPostModifier.fillMaxWidth()) {
|
||||
// Only known on a revisit — the first render is what fills the cache — so the first sight of
|
||||
// a PDF still grows into place. From then on the page's box is held open while it renders
|
||||
// and the feed stays put.
|
||||
if (aspectRatio != null) {
|
||||
Box(
|
||||
modifier =
|
||||
Modifier
|
||||
.fillMaxWidth()
|
||||
.aspectRatio(previewAspectRatio(aspectRatio)),
|
||||
contentAlignment = Alignment.Center,
|
||||
) {
|
||||
LoadingAnimation(Size40dp, Size6dp)
|
||||
}
|
||||
}
|
||||
|
||||
FilenameRow(filename = filename, subtitle = "Loading…")
|
||||
Spacer(modifier = DoubleVertSpacer)
|
||||
}
|
||||
@@ -223,7 +299,8 @@ private fun renderFirstPage(
|
||||
PdfPreview(
|
||||
thumbnail = bitmap,
|
||||
pageCount = pageCount,
|
||||
aspectRatio = page.width.toFloat() / page.height.toFloat(),
|
||||
pageWidth = page.width,
|
||||
pageHeight = page.height,
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
@@ -107,7 +107,7 @@ fun PictureDisplay(
|
||||
}
|
||||
|
||||
if (images.size == 1) {
|
||||
val ratio = first.dim?.aspectRatio() ?: MediaAspectRatioCache.get(first.url)
|
||||
val ratio = first.dim?.aspectRatioOrNull() ?: MediaAspectRatioCache.get(first.url)
|
||||
ContentWarningGate(
|
||||
isSensitive = isSensitive,
|
||||
reasons = reasons,
|
||||
|
||||
@@ -92,7 +92,7 @@ fun JustVideoDisplay(
|
||||
)
|
||||
}
|
||||
|
||||
val ratio = imeta.dimension?.aspectRatio() ?: MediaAspectRatioCache.get(imeta.url)
|
||||
val ratio = imeta.dimension?.aspectRatioOrNull() ?: MediaAspectRatioCache.get(imeta.url)
|
||||
|
||||
ContentWarningGate(
|
||||
isSensitive = isSensitive,
|
||||
|
||||
+1
-1
@@ -125,7 +125,7 @@ private fun PictureCardImage(
|
||||
|
||||
if (images.size == 1) {
|
||||
val single = images.first()
|
||||
val ratio = single.dim?.aspectRatio() ?: MediaAspectRatioCache.get(single.url)
|
||||
val ratio = single.dim?.aspectRatioOrNull() ?: MediaAspectRatioCache.get(single.url)
|
||||
ContentWarningGate(
|
||||
isSensitive = isSensitive,
|
||||
reasons = reasons,
|
||||
|
||||
+1
-1
@@ -141,7 +141,7 @@ private fun VideoCardImage(
|
||||
)
|
||||
}
|
||||
|
||||
val ratio = imeta.dimension?.aspectRatio() ?: MediaAspectRatioCache.get(imeta.url)
|
||||
val ratio = imeta.dimension?.aspectRatioOrNull() ?: MediaAspectRatioCache.get(imeta.url)
|
||||
|
||||
ContentWarningGate(
|
||||
isSensitive = isSensitive,
|
||||
|
||||
+1
-1
@@ -125,7 +125,7 @@ private fun FileHeaderCardImage(
|
||||
}
|
||||
|
||||
val isImage = content is MediaUrlImage
|
||||
val ratio = dimensions?.aspectRatio() ?: MediaAspectRatioCache.get(fullUrl)
|
||||
val ratio = dimensions?.aspectRatioOrNull() ?: MediaAspectRatioCache.get(fullUrl)
|
||||
|
||||
ContentWarningGate(
|
||||
isSensitive = isSensitive,
|
||||
|
||||
+100
@@ -0,0 +1,100 @@
|
||||
/*
|
||||
* Copyright (c) 2025 Vitor Pamplona
|
||||
*
|
||||
* Permission is hereby granted, free of charge, to any person obtaining a copy of
|
||||
* this software and associated documentation files (the "Software"), to deal in
|
||||
* the Software without restriction, including without limitation the rights to use,
|
||||
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
|
||||
* Software, and to permit persons to whom the Software is furnished to do so,
|
||||
* subject to the following conditions:
|
||||
*
|
||||
* The above copyright notice and this permission notice shall be included in all
|
||||
* copies or substantial portions of the Software.
|
||||
*
|
||||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
|
||||
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
|
||||
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
|
||||
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
|
||||
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
|
||||
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
|
||||
*/
|
||||
package com.vitorpamplona.amethyst.ui.components.pdf
|
||||
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Test
|
||||
|
||||
/**
|
||||
* The placeholder shown while a previously-seen PDF re-renders reserves its box from the ratio
|
||||
* cached in `MediaAspectRatioCache`, and the loaded thumbnail sizes itself from the ratio the
|
||||
* renderer reports. Those are the same number, so both sides must clamp it identically — a
|
||||
* placeholder that skipped the clamp would reserve a different box than the page lands in, which is
|
||||
* the jump it exists to remove.
|
||||
*/
|
||||
class PdfPreviewAspectRatioTest {
|
||||
@Test
|
||||
fun ordinaryPortraitPageIsUnchanged() {
|
||||
// US Letter, in PostScript points.
|
||||
assertEquals(612f / 792f, previewAspectRatio(612f / 792f), 0.0001f)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun landscapePageIsUnchanged() {
|
||||
assertEquals(792f / 612f, previewAspectRatio(792f / 612f), 0.0001f)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun pathologicallyTallPageIsClamped() {
|
||||
// A 200x4000 receipt would otherwise reserve 20 screens of height for a sliver of content.
|
||||
assertEquals(0.2f, previewAspectRatio(200f / 4000f), 0.0001f)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun theClampIsTheSameNumberBothSidesUse() {
|
||||
val cachedOnRevisit = 200f / 4000f
|
||||
val reportedByTheRenderer = 200f / 4000f
|
||||
|
||||
assertEquals(previewAspectRatio(reportedByTheRenderer), previewAspectRatio(cachedOnRevisit), 0.0f)
|
||||
}
|
||||
|
||||
/**
|
||||
* `Modifier.aspectRatio` requires a finite ratio greater than zero and throws otherwise, so
|
||||
* every one of these would crash the composition of the whole feed if it reached the modifier.
|
||||
* A page measuring 0x0 and an imeta `dim` of `"0.4x0.4"` (which truncates to 0x0, slipping past
|
||||
* the parser's literal-`"0x0"` rejection) both arrive here as 0/0.
|
||||
*/
|
||||
@Test
|
||||
fun degenerateSizesNeverReachTheModifier() {
|
||||
assertUsable(previewAspectRatio(ratioOf(0, 0)))
|
||||
assertUsable(previewAspectRatio(ratioOf(100, 0)))
|
||||
assertUsable(previewAspectRatio(ratioOf(0, 100)))
|
||||
assertUsable(previewAspectRatio(-1f))
|
||||
}
|
||||
|
||||
/**
|
||||
* Pins the reason the guard above cannot just be the clamp: every comparison against NaN is
|
||||
* false, so `coerceAtLeast` returns NaN unchanged rather than lifting it to the floor.
|
||||
*/
|
||||
@Test
|
||||
fun clampingAloneDoesNotCatchNaN() {
|
||||
assertTrue(ratioOf(0, 0).coerceAtLeast(0.2f).isNaN())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun unknownSizedPagesFallBackToPortrait() {
|
||||
// Taller than wide, so an unknown page reserves a portrait box rather than a squat one.
|
||||
assertTrue(previewAspectRatio(ratioOf(0, 0)) < 1f)
|
||||
}
|
||||
|
||||
// Computed rather than written as a literal so the compiler doesn't fold it into a
|
||||
// division-by-zero warning — these degenerate sizes are the point of the test.
|
||||
private fun ratioOf(
|
||||
width: Int,
|
||||
height: Int,
|
||||
): Float = width.toFloat() / height.toFloat()
|
||||
|
||||
private fun assertUsable(ratio: Float) {
|
||||
assertTrue("$ratio is not finite", ratio.isFinite())
|
||||
assertTrue("$ratio is not > 0", ratio > 0f)
|
||||
}
|
||||
}
|
||||
+45
-3
@@ -24,15 +24,44 @@ import androidx.compose.runtime.Stable
|
||||
import com.vitorpamplona.quartz.nip01Core.core.has
|
||||
import com.vitorpamplona.quartz.utils.ensure
|
||||
import kotlinx.serialization.Serializable
|
||||
import kotlinx.serialization.Transient
|
||||
|
||||
@Serializable
|
||||
@Stable
|
||||
class DimensionTag(
|
||||
val width: Int,
|
||||
val height: Int,
|
||||
/**
|
||||
* The ratio exactly as the author declared it, kept because [width] and [height] are whole
|
||||
* pixels and truncating to them can lose it — entirely, for a dim below `1` on either axis.
|
||||
* Only [parse] supplies it, and it is deliberately [Transient]: it is derived from the tag
|
||||
* text, so a deserialized instance simply falls back to the pixel counts.
|
||||
*/
|
||||
@Transient private val declaredRatio: Float? = null,
|
||||
) {
|
||||
/**
|
||||
* The raw width/height ratio in whole pixels, which is only meaningful when [hasSize] is true.
|
||||
* Anywhere the result reaches a layout, use [aspectRatioOrNull] instead.
|
||||
*/
|
||||
fun aspectRatio() = width.toFloat() / height.toFloat()
|
||||
|
||||
/**
|
||||
* The ratio to lay out with, or null when the tag carries no usable shape.
|
||||
*
|
||||
* Prefers the declared ratio over the truncated pixel counts, so a fractional dim keeps the
|
||||
* shape its author meant: `"0.75x1"` is a legitimate 3:4 even though it rounds down to `0x1`,
|
||||
* and `"317.9x498.4"` is fractionally more accurate than `317x498`. For whole-number dims —
|
||||
* every well-formed tag — the two are the same number.
|
||||
*
|
||||
* Returning null matters as much as the value. [parse] only rejects the literal string
|
||||
* `"0x0"`, so a tag can still arrive with no usable shape at all, and [aspectRatio] then
|
||||
* returns `0/0`, which is `NaN`. Compose's `Modifier.aspectRatio` throws
|
||||
* `IllegalArgumentException` on `NaN` and on `0f`, and clamping does not rescue either: every
|
||||
* comparison against `NaN` is false, so `coerceAtLeast` returns it unchanged. A tag any relay
|
||||
* can carry would otherwise crash the composition around it.
|
||||
*/
|
||||
fun aspectRatioOrNull(): Float? = declaredRatio ?: if (hasSize()) aspectRatio() else null
|
||||
|
||||
fun hasSize() = width > 0 && height > 0
|
||||
|
||||
override fun toString() = "${width}x$height"
|
||||
@@ -59,15 +88,28 @@ class DimensionTag(
|
||||
// Some clients (e.g. Primal) emit floating-point dimensions like "317.0x498.0"
|
||||
// in NIP-92 imeta tags. Parse as Double and truncate to keep those tags usable
|
||||
// for pre-load layout reservation.
|
||||
val width = parts[0].toDouble().toInt()
|
||||
val height = parts[1].toDouble().toInt()
|
||||
val width = parts[0].toDouble()
|
||||
val height = parts[1].toDouble()
|
||||
|
||||
DimensionTag(width, height)
|
||||
DimensionTag(width.toInt(), height.toInt(), declaredRatio(width, height))
|
||||
} catch (e: Exception) {
|
||||
null
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The declared ratio, or null when the declared size cannot describe a shape at all.
|
||||
* Computed before the truncation to whole pixels, which is the whole point: a dim under
|
||||
* `1` on either axis truncates to `0` and takes its shape with it.
|
||||
*/
|
||||
private fun declaredRatio(
|
||||
width: Double,
|
||||
height: Double,
|
||||
): Float? {
|
||||
if (width <= 0.0 || height <= 0.0) return null
|
||||
return (width / height).toFloat().takeIf { it.isFinite() && it > 0f }
|
||||
}
|
||||
|
||||
fun assemble(dim: DimensionTag) = arrayOf(TAG_NAME, dim.toString())
|
||||
}
|
||||
}
|
||||
|
||||
+100
@@ -22,8 +22,10 @@ package com.vitorpamplona.quartz.nip94FileMetadata.tags
|
||||
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertFalse
|
||||
import kotlin.test.assertNotNull
|
||||
import kotlin.test.assertNull
|
||||
import kotlin.test.assertTrue
|
||||
|
||||
class DimensionTagTest {
|
||||
@Test
|
||||
@@ -72,4 +74,102 @@ class DimensionTagTest {
|
||||
assertNotNull(tag)
|
||||
assertEquals(317f / 498f, tag.aspectRatio())
|
||||
}
|
||||
|
||||
/**
|
||||
* [rejectsZeroByZero] passes by comparing the string, so the float path of the Primal
|
||||
* tolerance above reaches the same 0x0 pixel counts by truncation and is kept.
|
||||
*/
|
||||
@Test
|
||||
fun subPixelFloatsTruncateToNoPixels() {
|
||||
val tag = DimensionTag.parse("0.4x0.4")
|
||||
assertNotNull(tag)
|
||||
assertEquals(0, tag.width)
|
||||
assertEquals(0, tag.height)
|
||||
assertFalse(tag.hasSize())
|
||||
}
|
||||
|
||||
/**
|
||||
* ...but the shape the author declared survives that truncation. A square is a square whether
|
||||
* it was declared as "0.4x0.4" or "400x400"; only the pixel counts are unusable.
|
||||
*/
|
||||
@Test
|
||||
fun subPixelFloatsKeepTheDeclaredShape() {
|
||||
assertEquals(1f, DimensionTag.parse("0.4x0.4")?.aspectRatioOrNull())
|
||||
assertEquals(0.75f, DimensionTag.parse("0.75x1")?.aspectRatioOrNull())
|
||||
assertEquals(2f, DimensionTag.parse("0.5x0.25")?.aspectRatioOrNull())
|
||||
}
|
||||
|
||||
/**
|
||||
* The declared ratio wins over the truncated one wherever they differ, being the more faithful
|
||||
* of the two. They differ only for fractional dims — for whole numbers it is the same number.
|
||||
*/
|
||||
@Test
|
||||
fun declaredRatioBeatsTheTruncatedOne() {
|
||||
val tag = DimensionTag.parse("317.9x498.4")
|
||||
assertNotNull(tag)
|
||||
assertEquals((317.9 / 498.4).toFloat(), tag.aspectRatioOrNull())
|
||||
assertEquals(317f / 498f, tag.aspectRatio())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun wholeNumberDimsAreUnaffected() {
|
||||
val tag = DimensionTag.parse("1920x1080")
|
||||
assertNotNull(tag)
|
||||
assertEquals(tag.aspectRatio(), tag.aspectRatioOrNull())
|
||||
}
|
||||
|
||||
/**
|
||||
* The declared ratio is derived from the tag text, so it is not serialized — a tag rebuilt
|
||||
* from its width and height falls back to the pixel counts rather than carrying a stale one.
|
||||
*/
|
||||
@Test
|
||||
fun aDirectlyConstructedTagFallsBackToPixelCounts() {
|
||||
assertEquals(1920f / 1080f, DimensionTag(1920, 1080).aspectRatioOrNull())
|
||||
assertNull(DimensionTag(0, 0).aspectRatioOrNull())
|
||||
}
|
||||
|
||||
/**
|
||||
* Why [DimensionTag.aspectRatioOrNull] exists: `Modifier.aspectRatio` throws on both of these,
|
||||
* so a tag that reaches a layout through the raw accessor crashes the composition around it.
|
||||
*/
|
||||
@Test
|
||||
fun rawAspectRatioOfAZeroSizedTagIsNotLayoutSafe() {
|
||||
assertTrue(DimensionTag(0, 0).aspectRatio().isNaN())
|
||||
assertEquals(0f, DimensionTag(0, 100).aspectRatio())
|
||||
}
|
||||
|
||||
/**
|
||||
* A fractional dim keeps its shape (see [subPixelFloatsKeepTheDeclaredShape]); one that
|
||||
* declares no shape at all still has to come back null, since `Modifier.aspectRatio` throws on
|
||||
* everything this would otherwise produce.
|
||||
*/
|
||||
@Test
|
||||
fun aspectRatioOrNullRefusesEveryShapeALayoutCannotUse() {
|
||||
assertNull(DimensionTag.parse("0x5")?.aspectRatioOrNull())
|
||||
assertNull(DimensionTag.parse("-3x4")?.aspectRatioOrNull())
|
||||
assertNull(DimensionTag(0, 0).aspectRatioOrNull())
|
||||
assertNull(DimensionTag(100, 0).aspectRatioOrNull())
|
||||
assertNull(DimensionTag(0, 100).aspectRatioOrNull())
|
||||
assertNull(DimensionTag(-1, 10).aspectRatioOrNull())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun aspectRatioOrNullKeepsUsableSizes() {
|
||||
assertEquals(317f / 498f, DimensionTag(317, 498).aspectRatioOrNull())
|
||||
}
|
||||
|
||||
/**
|
||||
* `"NaN"` and `"Infinity"` are legal input to Kotlin's `String.toDouble()` and a relay can
|
||||
* carry either, so both reach the ratio maths. The `<= 0.0` rejection cannot stop `NaN` —
|
||||
* every comparison against it is false — which is why the finite check is the one holding the
|
||||
* line. Whatever comes out, it is never a shape a layout would throw on.
|
||||
*/
|
||||
@Test
|
||||
fun nonNumericDoublesNeverProduceAnUnusableShape() {
|
||||
assertNull(DimensionTag.parse("NaNxNaN")?.aspectRatioOrNull())
|
||||
assertNull(DimensionTag.parse("0.4xNaN")?.aspectRatioOrNull())
|
||||
|
||||
// Infinity saturates to Int.MAX_VALUE on both axes: garbage in, square out, never a throw.
|
||||
assertEquals(1f, DimensionTag.parse("InfinityxInfinity")?.aspectRatioOrNull())
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user