From d37e510b9a81279db30aaeb7b970275cb1cecd1c Mon Sep 17 00:00:00 2001 From: Noah Bond Date: Mon, 26 Jun 2023 10:42:15 -0700 Subject: [PATCH] Bug 1815579 - Improve performance of image loading in tab items --- .../org/mozilla/fenix/compose/TabThumbnail.kt | 101 ++++++++++++++ .../mozilla/fenix/compose/ThumbnailCard.kt | 73 +++------- .../mozilla/fenix/compose/ThumbnailImage.kt | 127 ++++++++++++++++++ .../mozilla/fenix/compose/list/ListItem.kt | 33 ++++- .../fenix/compose/tabstray/TabGridItem.kt | 31 +++-- .../fenix/compose/tabstray/TabListItem.kt | 7 +- .../fenix/home/recenttabs/view/RecentTabs.kt | 7 +- .../tabstray/inactivetabs/InactiveTabs.kt | 7 + 8 files changed, 302 insertions(+), 84 deletions(-) create mode 100644 app/src/main/java/org/mozilla/fenix/compose/TabThumbnail.kt create mode 100644 app/src/main/java/org/mozilla/fenix/compose/ThumbnailImage.kt diff --git a/app/src/main/java/org/mozilla/fenix/compose/TabThumbnail.kt b/app/src/main/java/org/mozilla/fenix/compose/TabThumbnail.kt new file mode 100644 index 0000000000..36e58d52d7 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/compose/TabThumbnail.kt @@ -0,0 +1,101 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.compose + +import androidx.compose.foundation.Image +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.size +import androidx.compose.foundation.shape.RoundedCornerShape +import androidx.compose.material.Card +import androidx.compose.runtime.Composable +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.clip +import androidx.compose.ui.graphics.Color +import androidx.compose.ui.graphics.asImageBitmap +import androidx.compose.ui.layout.ContentScale +import androidx.compose.ui.tooling.preview.Preview +import androidx.compose.ui.unit.Dp +import androidx.compose.ui.unit.dp +import mozilla.components.browser.state.state.TabSessionState +import mozilla.components.browser.state.state.createTab +import org.mozilla.fenix.theme.FirefoxTheme + +private const val THUMBNAIL_SIZE = 108 +private const val FALLBACK_ICON_SIZE = 36 + +/** + * Thumbnail belonging to a [tab]. If a thumbnail is not available, the favicon + * will be displayed until the thumbnail is loaded. + * + * @param tab The given [TabSessionState] to render a thumbnail for. + * @param size [Dp] size of the thumbnail. + * @param backgroundColor [Color] used for the background of the favicon. + * @param modifier [Modifier] used to draw the image content. + * @param contentDescription Text used by accessibility services + * to describe what this image represents. + * @param contentScale [ContentScale] used to draw image content. + * @param alignment [Alignment] used to draw the image content. + */ +@Composable +@Suppress("LongParameterList") +fun TabThumbnail( + tab: TabSessionState, + modifier: Modifier = Modifier, + size: Dp = THUMBNAIL_SIZE.dp, + backgroundColor: Color = FirefoxTheme.colors.layer2, + contentDescription: String? = null, + contentScale: ContentScale = ContentScale.FillWidth, + alignment: Alignment = Alignment.TopCenter, +) { + Card( + modifier = modifier, + backgroundColor = backgroundColor, + ) { + ThumbnailImage( + key = tab.id, + size = size, + modifier = modifier, + contentScale = contentScale, + alignment = alignment, + ) { + Box( + modifier = Modifier.size(FALLBACK_ICON_SIZE.dp), + contentAlignment = Alignment.Center, + ) { + val icon = tab.content.icon + if (icon != null) { + icon.prepareToDraw() + Image( + bitmap = icon.asImageBitmap(), + contentDescription = contentDescription, + modifier = Modifier + .size(FALLBACK_ICON_SIZE.dp) + .clip(RoundedCornerShape(8.dp)), + contentScale = contentScale, + ) + } else { + Favicon( + url = tab.content.url, + size = FALLBACK_ICON_SIZE.dp, + ) + } + } + } + } +} + +@Preview +@Composable +private fun ThumbnailCardPreview() { + FirefoxTheme { + TabThumbnail( + tab = createTab(url = "www.mozilla.com", title = "Mozilla"), + modifier = Modifier + .size(THUMBNAIL_SIZE.dp, 80.dp) + .clip(RoundedCornerShape(8.dp)), + ) + } +} diff --git a/app/src/main/java/org/mozilla/fenix/compose/ThumbnailCard.kt b/app/src/main/java/org/mozilla/fenix/compose/ThumbnailCard.kt index 7eb0b91d94..2f6b3cc138 100644 --- a/app/src/main/java/org/mozilla/fenix/compose/ThumbnailCard.kt +++ b/app/src/main/java/org/mozilla/fenix/compose/ThumbnailCard.kt @@ -11,28 +11,23 @@ import androidx.compose.foundation.layout.size import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material.Card import androidx.compose.runtime.Composable -import androidx.compose.runtime.LaunchedEffect -import androidx.compose.runtime.mutableStateOf -import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip import androidx.compose.ui.graphics.Color -import androidx.compose.ui.graphics.ImageBitmap -import androidx.compose.ui.graphics.asImageBitmap -import androidx.compose.ui.graphics.painter.BitmapPainter import androidx.compose.ui.layout.ContentScale -import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp import mozilla.components.browser.icons.compose.Loader import mozilla.components.browser.icons.compose.Placeholder import mozilla.components.browser.icons.compose.WithIcon -import mozilla.components.concept.base.images.ImageLoadRequest import org.mozilla.fenix.components.components import org.mozilla.fenix.theme.FirefoxTheme +private const val THUMBNAIL_SIZE = 108 +private const val FALLBACK_ICON_SIZE = 36 + /** * Card which will display a thumbnail. If a thumbnail is not available for [url], the favicon * will be displayed until the thumbnail is loaded. @@ -51,7 +46,7 @@ import org.mozilla.fenix.theme.FirefoxTheme fun ThumbnailCard( url: String, key: String, - size: Dp = 108.dp, + size: Dp = THUMBNAIL_SIZE.dp, backgroundColor: Color = FirefoxTheme.colors.layer2, modifier: Modifier = Modifier, contentDescription: String? = null, @@ -62,76 +57,38 @@ fun ThumbnailCard( modifier = modifier, backgroundColor = backgroundColor, ) { - if (inComposePreview) { - Box( - modifier = Modifier.background(color = FirefoxTheme.colors.layer3), - ) - } else { + ThumbnailImage( + key = key, + size = size, + modifier = modifier, + contentScale = contentScale, + alignment = alignment, + ) { components.core.icons.Loader(url) { Placeholder { - Box( - modifier = Modifier.background(color = FirefoxTheme.colors.layer3), - ) + Box(modifier = Modifier.background(color = FirefoxTheme.colors.layer3)) } WithIcon { icon -> Box( - modifier = Modifier.size(36.dp), + modifier = Modifier.size(FALLBACK_ICON_SIZE.dp), contentAlignment = Alignment.Center, ) { Image( painter = icon.painter, contentDescription = contentDescription, modifier = Modifier - .size(36.dp) + .size(FALLBACK_ICON_SIZE.dp) .clip(RoundedCornerShape(8.dp)), contentScale = contentScale, ) } } } - - ThumbnailImage( - key = key, - size = size, - modifier = modifier, - contentScale = contentScale, - alignment = alignment, - ) } } } -@Composable -private fun ThumbnailImage( - key: String, - size: Dp, - modifier: Modifier, - contentScale: ContentScale, - alignment: Alignment, -) { - val rememberBitmap = remember(key) { mutableStateOf(null) } - val thumbnailSize = LocalDensity.current.run { size.toPx().toInt() } - val request = ImageLoadRequest(key, thumbnailSize) - val storage = components.core.thumbnailStorage - val bitmap = rememberBitmap.value - - LaunchedEffect(key) { - rememberBitmap.value = storage.loadThumbnail(request).await()?.asImageBitmap() - } - - if (bitmap != null) { - val painter = BitmapPainter(bitmap) - Image( - painter = painter, - contentDescription = null, - modifier = modifier, - contentScale = contentScale, - alignment = alignment, - ) - } -} - @Preview @Composable private fun ThumbnailCardPreview() { @@ -140,7 +97,7 @@ private fun ThumbnailCardPreview() { url = "https://mozilla.com", key = "123", modifier = Modifier - .size(108.dp, 80.dp) + .size(THUMBNAIL_SIZE.dp) .clip(RoundedCornerShape(8.dp)), ) } diff --git a/app/src/main/java/org/mozilla/fenix/compose/ThumbnailImage.kt b/app/src/main/java/org/mozilla/fenix/compose/ThumbnailImage.kt new file mode 100644 index 0000000000..5121f33dbd --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/compose/ThumbnailImage.kt @@ -0,0 +1,127 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.compose + +import android.graphics.Bitmap +import android.os.Parcelable +import androidx.compose.foundation.Image +import androidx.compose.foundation.background +import androidx.compose.foundation.layout.Box +import androidx.compose.runtime.Composable +import androidx.compose.runtime.DisposableEffect +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.runtime.saveable.rememberSaveable +import androidx.compose.runtime.setValue +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.graphics.asImageBitmap +import androidx.compose.ui.graphics.painter.BitmapPainter +import androidx.compose.ui.layout.ContentScale +import androidx.compose.ui.platform.LocalDensity +import androidx.compose.ui.tooling.preview.Preview +import androidx.compose.ui.unit.Dp +import androidx.compose.ui.unit.dp +import kotlinx.coroutines.launch +import kotlinx.parcelize.Parcelize +import mozilla.components.concept.base.images.ImageLoadRequest +import org.mozilla.fenix.components.components +import org.mozilla.fenix.theme.FirefoxTheme + +/** + * Thumbnail belonging to a [key]. Asynchronously fetches the bitmap from storage. + * + * @param key Key used to remember the thumbnail for future compositions. + * @param size [Dp] size of the thumbnail. + * @param modifier [Modifier] used to draw the image content. + * @param contentScale [ContentScale] used to draw image content. + * @param alignment [Alignment] used to draw the image content. + */ +@Composable +@Suppress("LongParameterList") +fun ThumbnailImage( + key: String, + size: Dp, + modifier: Modifier, + contentScale: ContentScale, + alignment: Alignment, + fallbackContent: @Composable () -> Unit, +) { + if (inComposePreview) { + Box(modifier = Modifier.background(color = FirefoxTheme.colors.layer3)) + } else { + val thumbnailSize = LocalDensity.current.run { size.toPx().toInt() } + val request = ImageLoadRequest(key, thumbnailSize) + val storage = components.core.thumbnailStorage + var state by rememberSaveable { mutableStateOf(ThumbnailImageState(null, false)) } + val scope = rememberCoroutineScope() + + DisposableEffect(Unit) { + if (!state.hasLoaded) { + scope.launch { + val thumbnailBitmap = storage.loadThumbnail(request).await() + thumbnailBitmap?.prepareToDraw() + state = ThumbnailImageState( + bitmap = thumbnailBitmap, + hasLoaded = true, + ) + } + } + + onDispose { + // Recycle the bitmap to liberate the RAM. Without this, a list of [ThumbnailImage] + // will bloat the memory. This is a trade-off, however, as the bitmap + // will be re-fetched if this Composable is disposed and re-loaded. + state.bitmap?.recycle() + state = ThumbnailImageState( + bitmap = null, + hasLoaded = false, + ) + } + } + + if (state.bitmap == null && state.hasLoaded) { + fallbackContent() + } else { + state.bitmap?.let { bitmap -> + Image( + painter = BitmapPainter(bitmap.asImageBitmap()), + contentDescription = null, + modifier = modifier, + contentScale = contentScale, + alignment = alignment, + ) + } + } + } +} + +/** + * State wrapper for [ThumbnailImage]. + */ +@Parcelize +private data class ThumbnailImageState( + val bitmap: Bitmap?, + val hasLoaded: Boolean, +) : Parcelable + +/** + * This preview does not demo anything. This is to ensure that [ThumbnailImage] does not break other previews. +*/ +@Preview +@Composable +private fun ThumbnailImagePreview() { + FirefoxTheme { + ThumbnailImage( + key = "", + size = 1.dp, + modifier = Modifier, + contentScale = ContentScale.Crop, + alignment = Alignment.Center, + fallbackContent = {}, + ) + } +} diff --git a/app/src/main/java/org/mozilla/fenix/compose/list/ListItem.kt b/app/src/main/java/org/mozilla/fenix/compose/list/ListItem.kt index af357f7ea4..c48722423b 100644 --- a/app/src/main/java/org/mozilla/fenix/compose/list/ListItem.kt +++ b/app/src/main/java/org/mozilla/fenix/compose/list/ListItem.kt @@ -5,6 +5,7 @@ package org.mozilla.fenix.compose.list import android.content.res.Configuration +import androidx.compose.foundation.Image import androidx.compose.foundation.background import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Box @@ -86,6 +87,7 @@ fun TextListItem( * * @param label The label in the list item. * @param description An optional description text below the label. + * @param faviconPainter Optional painter to use when fetching a new favicon is unnecessary. * @param onClick Called when the user clicks on the item. * @param url Website [url] for which the favicon will be shown. * @param iconPainter [Painter] used to display an [IconButton] after the list item. @@ -96,6 +98,7 @@ fun TextListItem( fun FaviconListItem( label: String, description: String? = null, + faviconPainter: Painter? = null, onClick: (() -> Unit)? = null, url: String, iconPainter: Painter? = null, @@ -107,11 +110,21 @@ fun FaviconListItem( description = description, onClick = onClick, beforeListAction = { - Favicon( - url = url, - size = ICON_SIZE, - modifier = Modifier.padding(horizontal = 16.dp), - ) + if (faviconPainter != null) { + Image( + painter = faviconPainter, + contentDescription = null, + modifier = Modifier + .padding(horizontal = 16.dp) + .size(ICON_SIZE), + ) + } else { + Favicon( + url = url, + size = ICON_SIZE, + modifier = Modifier.padding(horizontal = 16.dp), + ) + } }, afterListAction = { if (iconPainter != null && onIconClick != null) { @@ -325,7 +338,7 @@ private fun IconListItemWithRightIconPreview() { ) private fun FaviconListItemPreview() { FirefoxTheme { - Box(Modifier.background(FirefoxTheme.colors.layer1)) { + Column(Modifier.background(FirefoxTheme.colors.layer1)) { FaviconListItem( label = "Favicon + right icon + clicks", description = "Description text", @@ -334,6 +347,14 @@ private fun FaviconListItemPreview() { iconPainter = painterResource(R.drawable.ic_menu), onIconClick = { println("icon click") }, ) + + FaviconListItem( + label = "Favicon + painter", + description = "Description text", + faviconPainter = painterResource(id = R.drawable.ic_tab_collection), + onClick = { println("list item click") }, + url = "", + ) } } } diff --git a/app/src/main/java/org/mozilla/fenix/compose/tabstray/TabGridItem.kt b/app/src/main/java/org/mozilla/fenix/compose/tabstray/TabGridItem.kt index ba402cac76..8599606f66 100644 --- a/app/src/main/java/org/mozilla/fenix/compose/tabstray/TabGridItem.kt +++ b/app/src/main/java/org/mozilla/fenix/compose/tabstray/TabGridItem.kt @@ -6,6 +6,7 @@ package org.mozilla.fenix.compose.tabstray import androidx.compose.foundation.BorderStroke import androidx.compose.foundation.ExperimentalFoundationApi +import androidx.compose.foundation.Image import androidx.compose.foundation.background import androidx.compose.foundation.border import androidx.compose.foundation.clickable @@ -13,12 +14,14 @@ import androidx.compose.foundation.combinedClickable import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.requiredHeight import androidx.compose.foundation.layout.size +import androidx.compose.foundation.layout.width import androidx.compose.foundation.layout.wrapContentHeight import androidx.compose.foundation.layout.wrapContentSize import androidx.compose.foundation.shape.CircleShape @@ -33,6 +36,7 @@ import androidx.compose.runtime.Composable import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clipToBounds +import androidx.compose.ui.graphics.asImageBitmap import androidx.compose.ui.platform.LocalConfiguration import androidx.compose.ui.platform.testTag import androidx.compose.ui.res.colorResource @@ -52,10 +56,9 @@ import mozilla.components.browser.state.state.createTab import mozilla.components.support.ktx.kotlin.MAX_URI_LENGTH import org.mozilla.fenix.R import org.mozilla.fenix.compose.Divider -import org.mozilla.fenix.compose.Favicon import org.mozilla.fenix.compose.HorizontalFadingEdgeBox import org.mozilla.fenix.compose.SwipeToDismiss -import org.mozilla.fenix.compose.ThumbnailCard +import org.mozilla.fenix.compose.TabThumbnail import org.mozilla.fenix.compose.annotation.LightDarkPreview import org.mozilla.fenix.tabstray.TabsTrayTestTag import org.mozilla.fenix.tabstray.ext.toDisplayTitle @@ -146,13 +149,18 @@ fun TabGridItem( .fillMaxWidth() .wrapContentHeight(), ) { - Favicon( - url = tab.content.url, - size = 16.dp, - modifier = Modifier - .align(Alignment.CenterVertically) - .padding(start = 8.dp), - ) + Spacer(modifier = Modifier.width(8.dp)) + + tab.content.icon?.let { icon -> + icon.prepareToDraw() + Image( + bitmap = icon.asImageBitmap(), + contentDescription = null, + modifier = Modifier + .align(Alignment.CenterVertically) + .size(16.dp), + ) + } HorizontalFadingEdgeBox( modifier = Modifier @@ -230,9 +238,8 @@ private fun Thumbnail( testTag = TabsTrayTestTag.tabItemThumbnail }, ) { - ThumbnailCard( - url = tab.content.url, - key = tab.id, + TabThumbnail( + tab = tab, size = LocalConfiguration.current.screenWidthDp.dp, modifier = Modifier.fillMaxSize(), ) diff --git a/app/src/main/java/org/mozilla/fenix/compose/tabstray/TabListItem.kt b/app/src/main/java/org/mozilla/fenix/compose/tabstray/TabListItem.kt index fed27f47b7..5770f2a551 100644 --- a/app/src/main/java/org/mozilla/fenix/compose/tabstray/TabListItem.kt +++ b/app/src/main/java/org/mozilla/fenix/compose/tabstray/TabListItem.kt @@ -41,7 +41,7 @@ import mozilla.components.browser.state.state.createTab import mozilla.components.support.ktx.kotlin.MAX_URI_LENGTH import org.mozilla.fenix.R import org.mozilla.fenix.compose.SwipeToDismiss -import org.mozilla.fenix.compose.ThumbnailCard +import org.mozilla.fenix.compose.TabThumbnail import org.mozilla.fenix.compose.annotation.LightDarkPreview import org.mozilla.fenix.ext.toShortUrl import org.mozilla.fenix.tabstray.TabsTrayTestTag @@ -174,9 +174,8 @@ private fun Thumbnail( onMediaIconClicked: ((TabSessionState) -> Unit), ) { Box { - ThumbnailCard( - url = tab.content.url, - key = tab.id, + TabThumbnail( + tab = tab, modifier = Modifier .size(width = 92.dp, height = 72.dp) .semantics(mergeDescendants = true) { diff --git a/app/src/main/java/org/mozilla/fenix/home/recenttabs/view/RecentTabs.kt b/app/src/main/java/org/mozilla/fenix/home/recenttabs/view/RecentTabs.kt index 29de1ad323..e26a7ff934 100644 --- a/app/src/main/java/org/mozilla/fenix/home/recenttabs/view/RecentTabs.kt +++ b/app/src/main/java/org/mozilla/fenix/home/recenttabs/view/RecentTabs.kt @@ -56,7 +56,7 @@ import org.mozilla.fenix.components.components import org.mozilla.fenix.compose.ContextualMenu import org.mozilla.fenix.compose.Image import org.mozilla.fenix.compose.MenuItem -import org.mozilla.fenix.compose.ThumbnailCard +import org.mozilla.fenix.compose.TabThumbnail import org.mozilla.fenix.compose.annotation.LightDarkPreview import org.mozilla.fenix.compose.inComposePreview import org.mozilla.fenix.home.recenttabs.RecentTab @@ -228,9 +228,8 @@ fun RecentTabImage( contentScale = ContentScale.Crop, ) } - else -> ThumbnailCard( - url = tab.state.content.url, - key = tab.state.id, + else -> TabThumbnail( + tab = tab.state, modifier = modifier, contentScale = contentScale, ) diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/inactivetabs/InactiveTabs.kt b/app/src/main/java/org/mozilla/fenix/tabstray/inactivetabs/InactiveTabs.kt index 26cdb12f31..8152b9e052 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/inactivetabs/InactiveTabs.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/inactivetabs/InactiveTabs.kt @@ -29,6 +29,8 @@ 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.asImageBitmap +import androidx.compose.ui.graphics.painter.BitmapPainter import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource import androidx.compose.ui.tooling.preview.Preview @@ -101,10 +103,15 @@ fun InactiveTabsList( Column { inactiveTabs.forEach { tab -> val tabUrl = tab.content.url.toShortUrl() + val faviconPainter = tab.content.icon?.run { + prepareToDraw() + BitmapPainter(asImageBitmap()) + } FaviconListItem( label = tab.toDisplayTitle(), description = tabUrl, + faviconPainter = faviconPainter, onClick = { onTabClick(tab) }, url = tabUrl, iconPainter = painterResource(R.drawable.mozac_ic_close),