From f16a7bb6d628e32bfe93f76d28c389341ea3d2ea Mon Sep 17 00:00:00 2001 From: Damontecres Date: Sun, 26 Jul 2026 16:36:49 -0400 Subject: [PATCH] Fix trying to fetch images with zero fillWidth (#1753) ## Description The bug report was caused by a race condition where the image URL was built before the grid was fully laid out causing a `fillWidth=0` and the `remember` would not reset after the `fillWidth` was updated to the actual value. So this PR fixes the `remember`. Furthermore, some extra guards are added to only pass the `fillWidth` & `fillHeight` params if greater than 0. Finally, the `EpisodeCard` code is deleted because it's height & width logic was overly complicated and `BannerCardWithTitle` provides better functionality anyway. ### Related issues Should fix #1751 Maybe #1156 is also fixed by this ### Testing Emulator, but I haven't been able to reproduce the race condition, but the fix makes sense given the bug report ## Screenshots N/A ## AI or LLM usage None --- .../wholphin/services/ImageUrlService.kt | 8 +- .../wholphin/ui/cards/EpisodeCard.kt | 154 ------------------ .../wholphin/ui/cards/ItemCardImage.kt | 5 +- .../wholphin/ui/cards/SeasonCard.kt | 9 +- .../wholphin/ui/detail/CardGrid.kt | 2 +- .../wholphin/ui/main/SearchPage.kt | 8 +- 6 files changed, 19 insertions(+), 167 deletions(-) delete mode 100644 app/src/main/java/com/github/damontecres/wholphin/ui/cards/EpisodeCard.kt diff --git a/app/src/main/java/com/github/damontecres/wholphin/services/ImageUrlService.kt b/app/src/main/java/com/github/damontecres/wholphin/services/ImageUrlService.kt index 1dd642b0..6b71d683 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/services/ImageUrlService.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/services/ImageUrlService.kt @@ -223,11 +223,11 @@ class ImageUrlService imageType = imageType, maxWidth = maxWidth, maxHeight = maxHeight, - width = width, - height = height, + width = width?.takeIf { it > 0 }, + height = height?.takeIf { it > 0 }, quality = quality, - fillWidth = fillWidth, - fillHeight = fillHeight, + fillWidth = fillWidth?.takeIf { it > 0 }, + fillHeight = fillHeight?.takeIf { it > 0 }, tag = tag, format = format, percentPlayed = percentPlayed, diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/cards/EpisodeCard.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/cards/EpisodeCard.kt deleted file mode 100644 index 3cad76ab..00000000 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/cards/EpisodeCard.kt +++ /dev/null @@ -1,154 +0,0 @@ -package com.github.damontecres.wholphin.ui.cards - -import androidx.compose.animation.core.animateDpAsState -import androidx.compose.foundation.background -import androidx.compose.foundation.interaction.MutableInteractionSource -import androidx.compose.foundation.interaction.collectIsFocusedAsState -import androidx.compose.foundation.layout.Arrangement -import androidx.compose.foundation.layout.Box -import androidx.compose.foundation.layout.Column -import androidx.compose.foundation.layout.aspectRatio -import androidx.compose.foundation.layout.fillMaxSize -import androidx.compose.foundation.layout.fillMaxWidth -import androidx.compose.foundation.layout.padding -import androidx.compose.foundation.layout.size -import androidx.compose.foundation.shape.RoundedCornerShape -import androidx.compose.runtime.Composable -import androidx.compose.runtime.LaunchedEffect -import androidx.compose.runtime.getValue -import androidx.compose.runtime.mutableStateOf -import androidx.compose.runtime.remember -import androidx.compose.runtime.setValue -import androidx.compose.ui.Alignment -import androidx.compose.ui.Modifier -import androidx.compose.ui.graphics.Color -import androidx.compose.ui.platform.LocalDensity -import androidx.compose.ui.text.font.FontWeight -import androidx.compose.ui.text.style.TextAlign -import androidx.compose.ui.unit.Dp -import androidx.compose.ui.unit.dp -import androidx.tv.material3.Card -import androidx.tv.material3.CardDefaults -import androidx.tv.material3.MaterialTheme -import androidx.tv.material3.Text -import com.github.damontecres.wholphin.data.model.BaseItem -import com.github.damontecres.wholphin.ui.AppColors -import com.github.damontecres.wholphin.ui.AspectRatios -import com.github.damontecres.wholphin.ui.enableMarquee -import com.github.damontecres.wholphin.ui.seasonEpisode -import kotlinx.coroutines.delay - -@Composable -fun EpisodeCard( - item: BaseItem?, - onClick: () -> Unit, - onLongClick: () -> Unit, - modifier: Modifier = Modifier, - imageHeight: Dp = Dp.Unspecified, - imageWidth: Dp = Dp.Unspecified, - showImageOverlay: Boolean = false, - interactionSource: MutableInteractionSource = remember { MutableInteractionSource() }, -) { - val dto = item?.data - val focused by interactionSource.collectIsFocusedAsState() - var focusedAfterDelay by remember { mutableStateOf(false) } - - val hideOverlayDelay = 500L - if (focused) { - LaunchedEffect(Unit) { - delay(hideOverlayDelay) - if (focused) { - focusedAfterDelay = true - } else { - focusedAfterDelay = false - } - } - } else { - focusedAfterDelay = false - } - val aspectRatio = item?.aspectRatio?.coerceAtLeast(AspectRatios.MIN) ?: AspectRatios.MIN - val width = imageHeight * aspectRatio - val height = imageWidth * (1f / aspectRatio) - val density = LocalDensity.current - val imageWidthPx = remember(imageWidth) { with(density) { imageWidth.roundToPx() } } - Column( - verticalArrangement = Arrangement.spacedBy(4.dp), - modifier = modifier.size(width, height), - ) { - Card( - modifier = - Modifier - .size(imageWidth, imageHeight) - .aspectRatio(aspectRatio), - onClick = onClick, - onLongClick = onLongClick, - interactionSource = interactionSource, - colors = - CardDefaults.colors( - containerColor = Color.Transparent, - ), - ) { - Box( - modifier = - Modifier - .fillMaxSize(), - ) { - ItemCardImage( - item = item, - name = item?.name, - showOverlay = showImageOverlay, - favorite = dto?.userData?.isFavorite ?: false, - watched = dto?.userData?.played ?: false, - unwatchedCount = dto?.userData?.unplayedItemCount ?: -1, - watchedPercent = dto?.userData?.playedPercentage, - numberOfVersions = dto?.mediaSourceCount ?: 0, - useFallbackText = false, - fillWidth = imageWidthPx, - modifier = - Modifier - .fillMaxSize(), - ) - Box( - modifier = - Modifier - .align(Alignment.TopEnd) - .background( - AppColors.TransparentBlack50, - shape = RoundedCornerShape(8.dp), - ), - ) { - Text( - text = dto?.seasonEpisode ?: "", - modifier = Modifier.padding(4.dp), - ) - } - } - } - SlidingCardText(focused) { - Text( - text = dto?.seriesName ?: "", - maxLines = 1, - textAlign = TextAlign.Center, - style = MaterialTheme.typography.bodyMedium, - fontWeight = FontWeight.SemiBold, - modifier = - Modifier - .fillMaxWidth() - .padding(horizontal = 4.dp) - .enableMarquee(focusedAfterDelay), - ) - Text( - text = item?.name ?: "", - maxLines = 1, - textAlign = TextAlign.Center, - style = MaterialTheme.typography.bodySmall, - fontWeight = FontWeight.Normal, - modifier = - Modifier - .fillMaxWidth() - .padding(horizontal = 4.dp) - .enableMarquee(focusedAfterDelay), - ) - } - } -} diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/cards/ItemCardImage.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/cards/ItemCardImage.kt index 41a3aca4..3f7a6b1a 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/cards/ItemCardImage.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/cards/ItemCardImage.kt @@ -39,6 +39,7 @@ import com.github.damontecres.wholphin.ui.AppColors import com.github.damontecres.wholphin.ui.Cards import com.github.damontecres.wholphin.ui.FontAwesome import com.github.damontecres.wholphin.ui.LocalImageUrlService +import com.github.damontecres.wholphin.ui.gt import com.github.damontecres.wholphin.ui.isNotNullOrBlank import com.github.damontecres.wholphin.ui.logCoilError import org.jellyfin.sdk.model.api.ImageType @@ -68,13 +69,15 @@ fun ItemCardImage( val imageUrlService = LocalImageUrlService.current val imageUrl = remember(item, imageType, fillWidth, fillHeight) { - item?.let { + if (item != null && (fillWidth.gt(0) || fillHeight.gt(0))) { imageUrlService.getItemImageUrl( item, imageType, fillWidth = fillWidth, fillHeight = fillHeight, ) + } else { + null } } ItemCardImage( diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/cards/SeasonCard.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/cards/SeasonCard.kt index 5caa45bf..a6f69071 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/cards/SeasonCard.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/cards/SeasonCard.kt @@ -24,6 +24,7 @@ import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp +import androidx.compose.ui.unit.isSpecified import androidx.tv.material3.Card import androidx.tv.material3.CardDefaults import androidx.tv.material3.MaterialTheme @@ -36,15 +37,15 @@ import kotlinx.coroutines.delay import org.jellyfin.sdk.model.api.ImageType /** - * A Card for a TV Show Season, but can generically show most items + * A Card for a TV Show Season, but can generally show most items */ @Composable fun SeasonCard( item: BaseItem?, onClick: () -> Unit, onLongClick: () -> Unit, + imageHeight: Dp, modifier: Modifier = Modifier, - imageHeight: Dp = Dp.Unspecified, imageWidth: Dp = Dp.Unspecified, interactionSource: MutableInteractionSource = remember { MutableInteractionSource() }, showImageOverlay: Boolean = false, @@ -56,7 +57,7 @@ fun SeasonCard( remember(item, imageHeight, imageWidth, density) { if (item != null) { val fillHeight = - if (imageHeight != Dp.Unspecified) { + if (imageHeight.isSpecified) { with(density) { imageHeight.roundToPx() } @@ -64,7 +65,7 @@ fun SeasonCard( null } val fillWidth = - if (imageWidth != Dp.Unspecified) { + if (imageWidth.isSpecified) { with(density) { imageWidth.roundToPx() } diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/CardGrid.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/CardGrid.kt index 32c4a64a..7d34d0b1 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/CardGrid.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/CardGrid.kt @@ -345,7 +345,7 @@ fun CardGrid( items(pager.size) { index -> val item = pager[index] val details = - remember(index, item) { + remember(index, item, cardWidthPx) { val mod = if ((index == currentFocusedIndex) or (currentFocusedIndex < 0 && index == 0)) { if (DEBUG) Timber.d("Adding firstFocus to focusedIndex $index") diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/main/SearchPage.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/main/SearchPage.kt index 2b54f693..135c3b74 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/main/SearchPage.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/main/SearchPage.kt @@ -84,8 +84,8 @@ import com.github.damontecres.wholphin.services.deleteItem import com.github.damontecres.wholphin.ui.AspectRatios import com.github.damontecres.wholphin.ui.Cards import com.github.damontecres.wholphin.ui.SlimItemFields +import com.github.damontecres.wholphin.ui.cards.BannerCardWithTitle import com.github.damontecres.wholphin.ui.cards.DiscoverItemCard -import com.github.damontecres.wholphin.ui.cards.EpisodeCard import com.github.damontecres.wholphin.ui.cards.GridCard import com.github.damontecres.wholphin.ui.cards.ItemRow import com.github.damontecres.wholphin.ui.cards.ItemRowTitle @@ -855,15 +855,17 @@ fun SearchPage( onClickPosition = { setPosition(it) }, modifier = Modifier.fillMaxWidth(), cardContent = @Composable { index, item, mod, onClick, onLongClick -> - EpisodeCard( + BannerCardWithTitle( + title = item?.title, + subtitle = item?.subtitle, item = item, onClick = { setPosition(RowColumn(EPISODE_ROW, index)) onClick.invoke() }, onLongClick = onLongClick, - imageHeight = 140.dp, modifier = mod.padding(horizontal = 8.dp), + cardHeight = Cards.heightEpisode, ) }, )