From 1c1b8f267d5c7e713c7486c3cf32a90b535c7f11 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Sat, 10 Oct 2026 22:55:53 -0400 Subject: [PATCH] fix(chat): load a bubble-sized rendition for chat photos A photo bubble downloaded the full-size original, 3.5-5.5 MB for the recent photos in one group, though the server also derives 800 and 1600px renditions. On a slow link the BlurHash stayed up for the whole download, and leaving the chat cancelled it. The bubble now loads the smallest derived rendition that fills its width and falls back to the original when there is none (sealed photos, or ones still processing). The viewer and share still use the original, with the bubble's cached image as the placeholder. Also logs when a photo download starts, finishes or is cancelled, and when a chat switches to previewing, which blurs every photo in it. --- .../app/messenger/internal/ChatViewModel.kt | 3 ++ .../shared/chat/ui/media/ChatPhotoOverlay.kt | 4 +-- .../shared/chat/ui/media/ChatPhotoSources.kt | 10 ++++-- .../chat/ui/media/MediaMessageBubble.kt | 22 +++++++++++-- .../chat/ui/media/MediaMessageBubbleTest.kt | 31 +++++++++++++++++++ .../shared/chat/media/ChatPhotoFetcher.kt | 18 +++++++++++ 6 files changed, 81 insertions(+), 7 deletions(-) diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt index 8513ef3a9..0b667255b 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt @@ -159,6 +159,7 @@ import com.getcode.ui.utils.generateComplementaryColorPalette import com.getcode.util.resources.ResourceHelper import com.getcode.utils.hexEncodedString import com.getcode.utils.trace +import com.getcode.utils.TraceType import com.getcode.view.BaseViewModel import com.getcode.view.LoadingSuccessState import dagger.assisted.Assisted @@ -982,6 +983,8 @@ internal class ChatViewModel @AssistedInject constructor( /** Whether the viewer is reading a group they have not joined, which shows its photos as BlurHash only. */ private val viewerPreviewing = stateFlow.map { it.isOutsideGroup }.distinctUntilChanged() + // Logged because it blurs every photo in the chat, and nothing else on screen says why. + .onEach { trace(tag = "ChatViewModel", message = "Viewer previewing group: $it", type = TraceType.Log) } private data class ViewerGates( val canPost: Boolean, diff --git a/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/media/ChatPhotoOverlay.kt b/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/media/ChatPhotoOverlay.kt index 3493abc83..c1a4fdd10 100644 --- a/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/media/ChatPhotoOverlay.kt +++ b/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/media/ChatPhotoOverlay.kt @@ -209,12 +209,12 @@ fun ChatPhotoOverlay( // The bubble already has the photo cached at its own size: draw that until the full one lands. val request = remember(source) { ImageRequest.Builder(context) - .data(source.model) + .data(source.fullModel) .placeholderMemoryCacheKey(source.model.cacheKey) .build() } var fullImageLoaded by remember(messageId) { mutableStateOf(false) } - val share = rememberSharePhoto(source.model) + val share = rememberSharePhoto(source.fullModel) Box( modifier = modifier diff --git a/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/media/ChatPhotoSources.kt b/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/media/ChatPhotoSources.kt index 92d708d20..d6ef58644 100644 --- a/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/media/ChatPhotoSources.kt +++ b/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/media/ChatPhotoSources.kt @@ -37,7 +37,10 @@ import com.flipcash.shared.chat.ui.bubbleCorners /** What the overlay needs of one on-screen photo bubble: where it is, and what it draws. */ @Stable class ChatPhotoSource internal constructor( + /** What the bubble draws, a rendition sized to it; its cached image is the viewer's placeholder. */ internal val model: ChatPhoto, + /** The full-size photo the viewer opens and shares. */ + internal val fullModel: ChatPhoto, /** The photo's own size, for the shape it opens to. Null when the message did not say. */ internal val imageSize: Size?, internal val corners: ChatPhotoCorners, @@ -100,19 +103,20 @@ val LocalChatPhotoSources = staticCompositionLocalOf { null } internal fun Modifier.chatPhotoSource( messageId: Long?, model: ChatPhoto?, + fullModel: ChatPhoto?, imageWidth: Int?, imageHeight: Int?, corners: ChatPhotoCorners, ): Modifier { val sources = LocalChatPhotoSources.current - if (sources == null || messageId == null || model == null) return this - val source = remember(model, imageWidth, imageHeight, corners) { + if (sources == null || messageId == null || model == null || fullModel == null) return this + val source = remember(model, fullModel, imageWidth, imageHeight, corners) { val size = if ((imageWidth ?: 0) > 0 && (imageHeight ?: 0) > 0) { Size(imageWidth!!.toFloat(), imageHeight!!.toFloat()) } else { null } - ChatPhotoSource(model, size, corners) + ChatPhotoSource(model, fullModel, size, corners) } DisposableEffect(sources, messageId, source) { sources.register(messageId, source) diff --git a/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/media/MediaMessageBubble.kt b/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/media/MediaMessageBubble.kt index c9584a94b..d0f55a7c0 100644 --- a/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/media/MediaMessageBubble.kt +++ b/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/media/MediaMessageBubble.kt @@ -1,5 +1,6 @@ package com.flipcash.shared.chat.ui.media +import androidx.compose.ui.platform.LocalDensity import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Column import androidx.compose.runtime.Composable @@ -24,6 +25,7 @@ import androidx.compose.ui.unit.Dp import com.getcode.theme.CodeTheme import com.flipcash.shared.chat.ui.BubbleCorners import com.flipcash.services.chat.BlobOpenFailure +import com.flipcash.services.models.chat.MediaItem import com.flipcash.services.models.chat.MediaItemRendition import com.flipcash.services.models.chat.MessageContent import com.flipcash.shared.chat.media.ChatPhoto @@ -88,6 +90,13 @@ internal fun isPhotoUnavailable(error: Throwable): Boolean { } } +/** + * The rendition a photo bubble [bubblePx] wide loads: the smallest derived size that fills it, or + * the original when the server derived none (a sealed photo, or one still being processed). + */ +internal fun bubbleRendition(item: MediaItem, bubblePx: Int): MediaItemRendition? = + item.renditionForSize(bubblePx) ?: item.rendition(MediaItemRendition.Role.ORIGINAL) + internal fun MediaSendProgress.toPhase(): ChatPhotoPhase = when (this) { MediaSendProgress.Preparing -> ChatPhotoPhase.Preparing is MediaSendProgress.Uploading -> ChatPhotoPhase.Uploading(fraction) @@ -115,7 +124,12 @@ internal fun MediaMessageBubble( onDoubleClick: (() -> Unit)? = null, ) { val actionHandler = LocalChatActionHandler.current - val rendition = body.items.firstOrNull()?.let { it.rendition(MediaItemRendition.Role.ORIGINAL) ?: it.renditions.firstOrNull() } + val mediaItem = body.items.firstOrNull() + val rendition = mediaItem?.let { it.rendition(MediaItemRendition.Role.ORIGINAL) ?: it.renditions.firstOrNull() } + // The bubble draws the smallest server rendition that fills it, not the original: originals run + // to several megabytes, and on a slow link the BlurHash stays up for the whole download. + val bubblePx = with(LocalDensity.current) { maxWidth.roundToPx() } + val bubbleRendition = mediaItem?.let { bubbleRendition(it, bubblePx) } ?: rendition val image = rendition?.blob?.image val info = item.photo // The optimistic row has no blob id yet and the staged file as its URL. @@ -125,10 +139,13 @@ internal fun MediaMessageBubble( val sentPreview = remember(item.pendingClientIdHex) { item.pendingClientIdHex?.let(SentPhotoPreviews::forMessage)?.asImageBitmap() } - val model = remember(rendition, info) { + val fullModel = remember(rendition, info) { if (rendition == null || info == null || localUri != null) null else ChatPhoto(info.chatId, rendition, info.senderId, info.sealed, info.redacted) } + val model = remember(bubbleRendition, fullModel) { + if (fullModel == null || bubbleRendition == null) null else fullModel.copy(rendition = bubbleRendition) + } val progressOf = LocalChatMediaProgress.current val progress by remember(item.pendingClientIdHex) { @@ -174,6 +191,7 @@ internal fun MediaMessageBubble( // A photo this device is still sending has no model and cannot open. messageId = item.messageId.takeIf { onClick != null && !failed }, model = model, + fullModel = fullModel, imageWidth = image?.width, imageHeight = image?.height, corners = bubblePhotoCorners( diff --git a/apps/flipcash/shared/chat-ui/src/test/kotlin/com/flipcash/shared/chat/ui/media/MediaMessageBubbleTest.kt b/apps/flipcash/shared/chat-ui/src/test/kotlin/com/flipcash/shared/chat/ui/media/MediaMessageBubbleTest.kt index 2821cb3ed..b50310462 100644 --- a/apps/flipcash/shared/chat-ui/src/test/kotlin/com/flipcash/shared/chat/ui/media/MediaMessageBubbleTest.kt +++ b/apps/flipcash/shared/chat-ui/src/test/kotlin/com/flipcash/shared/chat/ui/media/MediaMessageBubbleTest.kt @@ -193,4 +193,35 @@ class MediaMessageBubbleTest { assertTrue(chip.top >= photo.top && chip.bottom <= photo.bottom, "chip $chip sits outside photo $photo vertically") assertTrue(chip.right - chip.left < photo.right - photo.left, "chip $chip fills the whole photo $photo") } + + private fun sized(role: MediaItemRendition.Role, longestSide: Int, id: Byte) = MediaItemRendition( + role = role, + blobId = BlobId(ByteArray(32) { id }), + blob = BlobMetadata( + mimeType = "image/jpeg", + sizeBytes = longestSide.toLong(), + downloadUrl = "https://cdn/$longestSide", + image = ImageMetadata(width = longestSide, height = longestSide * 3 / 4, blurhash = "abc"), + ), + ) + + @Test + fun `a bubble loads the smallest derived rendition that fills it, not the original`() { + val item = MediaItem( + listOf( + sized(MediaItemRendition.Role.ORIGINAL, 4000, 1), + sized(MediaItemRendition.Role.DISPLAY, 1600, 2), + sized(MediaItemRendition.Role.DISPLAY, 800, 3), + sized(MediaItemRendition.Role.THUMBNAIL, 320, 4), + ), + ) + assertEquals(800, bubbleRendition(item, bubblePx = 700)?.blob?.image?.width) + assertEquals(1600, bubbleRendition(item, bubblePx = 900)?.blob?.image?.width) + } + + @Test + fun `a bubble falls back to the original when the server derived no renditions`() { + val item = MediaItem(listOf(sized(MediaItemRendition.Role.ORIGINAL, 4000, 1))) + assertEquals(MediaItemRendition.Role.ORIGINAL, bubbleRendition(item, bubblePx = 700)?.role) + } } diff --git a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/media/ChatPhotoFetcher.kt b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/media/ChatPhotoFetcher.kt index 3ba43a7da..f802246fa 100644 --- a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/media/ChatPhotoFetcher.kt +++ b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/media/ChatPhotoFetcher.kt @@ -12,6 +12,7 @@ import coil3.request.Options import com.flipcash.services.chat.BlobOpenFailure import com.getcode.utils.TraceType import com.getcode.utils.trace +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext import okhttp3.OkHttpClient @@ -20,6 +21,8 @@ import okio.Buffer import okio.buffer import javax.inject.Inject import javax.inject.Singleton +import kotlin.time.TimeMark +import kotlin.time.TimeSource /** * Keys a [ChatPhoto] by its blob id, so the memory cache never keys on a signed URL that rotates @@ -45,11 +48,19 @@ class ChatPhotoFetcher( private val imageLoader: ImageLoader, ) : Fetcher { + /** When this fetch went to the network, so a slow or cancelled download leaves a trace. */ + private var downloadStarted: TimeMark? = null + override suspend fun fetch(): FetchResult = try { load() } catch (e: ChatPhotoUnavailable) { trace(tag = TAG, message = "Chat photo ${photo.cacheKey} unavailable: ${e.message}", type = TraceType.Log) throw e + } catch (e: CancellationException) { + downloadStarted?.let { + trace(tag = TAG, message = "Chat photo ${photo.cacheKey} cancelled after ${it.elapsedNow()}", type = TraceType.Log) + } + throw e } private suspend fun load(): FetchResult { @@ -58,6 +69,12 @@ class ChatPhotoFetcher( val key = photo.cacheKey cache?.openSnapshot(key)?.let { return it.asResult() } + downloadStarted = TimeSource.Monotonic.markNow() + trace( + tag = TAG, + message = "Chat photo $key downloading ${photo.rendition.role} (${photo.rendition.blob?.sizeBytes} bytes)", + type = TraceType.Log, + ) var url = loader.resolveUrl(photo) ?: throw ChatPhotoUnavailable.NotFound() var bytes = download(url) if (bytes == null) { @@ -66,6 +83,7 @@ class ChatPhotoFetcher( bytes = download(url) ?: throw ChatPhotoUnavailable.Download(null) } val opened = loader.open(photo, bytes) + trace(tag = TAG, message = "Chat photo $key loaded in ${downloadStarted?.elapsedNow()}", type = TraceType.Log) if (cache != null) { cache.write(key, opened)?.let { return it.asResult() }