From ce29d21e9d304c85e6e3b1fb920591e5cf8a8238 Mon Sep 17 00:00:00 2001 From: Alex Hart Date: Tue, 29 Sep 2026 15:38:00 -0300 Subject: [PATCH] Fix sticker keyboard rendering and cache animated stickers. --- .../v2/ChatStickerConfirmation.kt | 2 +- .../ApngInputStreamFactoryResourceDecoder.kt | 2 +- .../SignalStickerKeyboardRepository.kt | 5 +- .../stickers/manage/StickerPackListItems.kt | 2 + .../preview/StickerPackPreviewActivityV2.kt | 2 + .../data/StickerKeyboardRepository.kt | 4 +- .../screens/MediaKeyboardPageViewModel.kt | 35 ++++++ .../screens/emoji/EmojiPageScreen.kt | 1 + .../screens/emoji/EmojiPageViewModel.kt | 16 +-- .../screens/gif/GifPageViewModel.kt | 16 +-- .../screens/sticker/StickerPageScreen.kt | 75 +++++++++---- .../screens/sticker/StickerPageViewModel.kt | 24 ++-- .../screens/emoji/EmojiPageViewModelTest.kt | 11 +- .../screens/gif/GifPageViewModelTest.kt | 11 +- .../sticker/StickerPageViewModelTest.kt | 11 +- .../org/signal/glide/compose/GlideImage.kt | 103 +----------------- 16 files changed, 138 insertions(+), 182 deletions(-) create mode 100644 feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/MediaKeyboardPageViewModel.kt diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ChatStickerConfirmation.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ChatStickerConfirmation.kt index 5458bdc9d7..647ec80188 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ChatStickerConfirmation.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ChatStickerConfirmation.kt @@ -173,7 +173,7 @@ private fun StickerConfirmationPanel( GlideImage( model = confirmation.sticker.image, - enableApngAnimation = confirmation.sticker.isAnimated, + enableApngAnimation = true, modifier = Modifier .size(StickerSize) .align(Alignment.Center) diff --git a/app/src/main/java/org/thoughtcrime/securesms/glide/cache/ApngInputStreamFactoryResourceDecoder.kt b/app/src/main/java/org/thoughtcrime/securesms/glide/cache/ApngInputStreamFactoryResourceDecoder.kt index c1665a9536..901993b6aa 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/glide/cache/ApngInputStreamFactoryResourceDecoder.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/glide/cache/ApngInputStreamFactoryResourceDecoder.kt @@ -17,7 +17,7 @@ class ApngInputStreamFactoryResourceDecoder : ResourceDecoder( + tag: String, + shouldLogEvents: Boolean = false +) : EventDrivenViewModel(tag, shouldLogEvents) { + + private val actionChannel = Channel(Channel.UNLIMITED) + + /** Actions raised for the host. Buffered, so none are lost between hosts. */ + val actions: Flow = actionChannel.receiveAsFlow() + + protected fun emitAction(action: MediaKeyboardAction) { + // Unlimited buffer means this will always succeed + actionChannel.trySend(action) + } +} diff --git a/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/emoji/EmojiPageScreen.kt b/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/emoji/EmojiPageScreen.kt index 22dea94052..ee9ce8a70d 100644 --- a/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/emoji/EmojiPageScreen.kt +++ b/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/emoji/EmojiPageScreen.kt @@ -371,6 +371,7 @@ private fun EmojiImage( Text( text = emoji, fontSize = if (emoji.isAsciiEmoticon()) 13.sp else 22.sp, + color = MaterialTheme.colorScheme.onSurface, maxLines = 1, softWrap = false, modifier = modifier diff --git a/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/emoji/EmojiPageViewModel.kt b/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/emoji/EmojiPageViewModel.kt index f0ce56a77d..8d6ac2c0e2 100644 --- a/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/emoji/EmojiPageViewModel.kt +++ b/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/emoji/EmojiPageViewModel.kt @@ -9,15 +9,11 @@ import androidx.annotation.VisibleForTesting import androidx.lifecycle.ViewModel import androidx.lifecycle.ViewModelProvider import androidx.lifecycle.viewModelScope -import kotlinx.coroutines.channels.Channel -import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach -import kotlinx.coroutines.flow.receiveAsFlow -import org.signal.core.ui.compose.EventDrivenViewModel import org.signal.core.util.logging.Log import org.signal.mediakeyboard.MediaKeyboardAction import org.signal.mediakeyboard.MediaKeyboardState @@ -25,11 +21,12 @@ import org.signal.mediakeyboard.MediaKeyboardTab import org.signal.mediakeyboard.data.EmojiCategoryPage import org.signal.mediakeyboard.data.EmojiKeyboardCategory import org.signal.mediakeyboard.data.EmojiKeyboardRepository +import org.signal.mediakeyboard.screens.MediaKeyboardPageViewModel class EmojiPageViewModel( private val repository: EmojiKeyboardRepository, private val parentState: StateFlow -) : EventDrivenViewModel(TAG, shouldLogEvents = false) { +) : MediaKeyboardPageViewModel(TAG, shouldLogEvents = false) { companion object { private val TAG = Log.tag(EmojiPageViewModel::class) @@ -38,11 +35,6 @@ class EmojiPageViewModel( private val _state = MutableStateFlow(EmojiPageState()) val state: StateFlow = _state.asStateFlow() - private val actionChannel = Channel(Channel.UNLIMITED) - - /** What the user did that only the host can carry out, for whichever host is current. */ - val actions: Flow = actionChannel.receiveAsFlow() - init { onEvent(EmojiPageScreenEvents.Initialize) parentState @@ -93,7 +85,7 @@ class EmojiPageViewModel( is EmojiPageScreenEvents.EmojiClicked -> { val display = state.displayEmoji(event.emoji) repository.onEmojiUsed(display) - actionChannel.trySend(MediaKeyboardAction.EmojiSelected(display)) + emitAction(MediaKeyboardAction.EmojiSelected(display)) } is EmojiPageScreenEvents.EmojiLongPressed -> { @@ -105,7 +97,7 @@ class EmojiPageViewModel( is EmojiPageScreenEvents.VariationSelected -> { repository.setPreferredVariation(event.emoji.canonical, event.variation) repository.onEmojiUsed(event.variation) - actionChannel.trySend(MediaKeyboardAction.EmojiSelected(event.variation)) + emitAction(MediaKeyboardAction.EmojiSelected(event.variation)) stateEmitter( state.copy( preferredVariations = repository.getPreferredVariations(), diff --git a/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/gif/GifPageViewModel.kt b/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/gif/GifPageViewModel.kt index 70da98b302..97c20deb0f 100644 --- a/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/gif/GifPageViewModel.kt +++ b/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/gif/GifPageViewModel.kt @@ -8,21 +8,18 @@ package org.signal.mediakeyboard.screens.gif import androidx.annotation.VisibleForTesting import androidx.lifecycle.ViewModel import androidx.lifecycle.ViewModelProvider -import kotlinx.coroutines.channels.Channel -import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow -import kotlinx.coroutines.flow.receiveAsFlow -import org.signal.core.ui.compose.EventDrivenViewModel import org.signal.core.util.Result import org.signal.core.util.logging.Log import org.signal.mediakeyboard.MediaKeyboardAction import org.signal.mediakeyboard.data.GifKeyboardRepository +import org.signal.mediakeyboard.screens.MediaKeyboardPageViewModel class GifPageViewModel( private val repository: GifKeyboardRepository -) : EventDrivenViewModel(TAG) { +) : MediaKeyboardPageViewModel(TAG) { companion object { private val TAG = Log.tag(GifPageViewModel::class) @@ -32,11 +29,6 @@ class GifPageViewModel( private val _state = MutableStateFlow(GifPageState()) val state: StateFlow = _state.asStateFlow() - private val actionChannel = Channel(Channel.UNLIMITED) - - /** What the user did that only the host can carry out, for whichever host is current. */ - val actions: Flow = actionChannel.receiveAsFlow() - init { onEvent(GifPageScreenEvents.Initialize) } @@ -71,11 +63,11 @@ class GifPageViewModel( } is GifPageScreenEvents.GifClicked -> { - actionChannel.trySend(MediaKeyboardAction.GifSelected(event.gif)) + emitAction(MediaKeyboardAction.GifSelected(event.gif)) } is GifPageScreenEvents.SearchClicked -> { - actionChannel.trySend(MediaKeyboardAction.GifSearchClicked) + emitAction(MediaKeyboardAction.GifSearchClicked) } } } diff --git a/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/sticker/StickerPageScreen.kt b/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/sticker/StickerPageScreen.kt index a0f3266b6b..f4994a3dd1 100644 --- a/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/sticker/StickerPageScreen.kt +++ b/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/sticker/StickerPageScreen.kt @@ -7,10 +7,14 @@ package org.signal.mediakeyboard.screens.sticker import androidx.compose.foundation.background import androidx.compose.foundation.combinedClickable +import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.BoxWithConstraints import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.aspectRatio +import androidx.compose.foundation.layout.calculateEndPadding +import androidx.compose.foundation.layout.calculateStartPadding import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding @@ -38,9 +42,13 @@ 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.layout.ContentScale +import androidx.compose.ui.platform.LocalLayoutDirection import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.tooling.preview.PreviewWrapper +import androidx.compose.ui.unit.Dp +import androidx.compose.ui.unit.DpSize import androidx.compose.ui.unit.dp import org.signal.core.ui.compose.DayNightPreviews import org.signal.core.ui.compose.Dialogs @@ -59,6 +67,9 @@ import org.signal.mediakeyboard.screens.MediaKeyboardSearchField import org.signal.mediakeyboard.screens.PinnedRailLayout import org.signal.mediakeyboard.screens.SearchFieldReveal +private const val STICKER_COLUMN_COUNT = 5 +private val STICKER_CELL_SPACING = 12.dp + /** * @param onSearchFieldRevealedChange Reports whether the grid is scrolled far enough up to show the * search field, so the top bar can drop its now-redundant search icon. @@ -156,6 +167,7 @@ private fun PackButton( } else { GlideImage( model = pack.cover, + imageSize = DpSize(28.dp, 28.dp), modifier = Modifier.size(28.dp) ) } @@ -209,31 +221,44 @@ private fun StickerGrid( visiblePackId?.let { onEvent(StickerPageScreenEvents.VisiblePackChanged(it)) } } - LazyVerticalGrid( - columns = GridCells.Adaptive(minSize = 72.dp), - state = gridState, - contentPadding = GRID_CONTENT_PADDING, - modifier = modifier.fillMaxWidth() - ) { - item(key = "search", span = { GridItemSpan(maxLineSpan) }) { - MediaKeyboardSearchField( - hint = stringResource(R.string.MediaKeyboard__search_stickers), - onClick = { onEvent(StickerPageScreenEvents.SearchClicked) } - ) - } + // Each cell decodes its sticker at the cell's size, which the grid does not report, so work it out the way the grid + // will and hand it down. + BoxWithConstraints(modifier = modifier.fillMaxWidth()) { + val layoutDirection = LocalLayoutDirection.current + val rowWidth = maxWidth - + GRID_CONTENT_PADDING.calculateStartPadding(layoutDirection) - + GRID_CONTENT_PADDING.calculateEndPadding(layoutDirection) + val cellSize = (rowWidth - STICKER_CELL_SPACING * (STICKER_COLUMN_COUNT - 1)) / STICKER_COLUMN_COUNT - state.packs.forEach { pack -> - item(key = "header:${pack.id}", span = { GridItemSpan(maxLineSpan) }) { - StickerPackHeader(pack = pack, onEvent = onEvent) + LazyVerticalGrid( + columns = GridCells.Fixed(STICKER_COLUMN_COUNT), + state = gridState, + contentPadding = GRID_CONTENT_PADDING, + horizontalArrangement = Arrangement.spacedBy(STICKER_CELL_SPACING), + verticalArrangement = Arrangement.spacedBy(STICKER_CELL_SPACING), + modifier = Modifier.fillMaxWidth() + ) { + item(key = "search", span = { GridItemSpan(maxLineSpan) }) { + MediaKeyboardSearchField( + hint = stringResource(R.string.MediaKeyboard__search_stickers), + onClick = { onEvent(StickerPageScreenEvents.SearchClicked) } + ) } - pack.stickers.forEachIndexed { index, sticker -> - item(key = "${pack.id}:${sticker.stickerId}:$index") { - StickerCell( - sticker = sticker, - allowAnimation = state.allowAnimation, - onEvent = onEvent - ) + state.packs.forEach { pack -> + item(key = "header:${pack.id}", span = { GridItemSpan(maxLineSpan) }) { + StickerPackHeader(pack = pack, onEvent = onEvent) + } + + pack.stickers.forEachIndexed { index, sticker -> + item(key = "${pack.id}:${sticker.stickerId}:$index") { + StickerCell( + sticker = sticker, + allowAnimation = state.allowAnimation, + cellSize = cellSize, + onEvent = onEvent + ) + } } } } @@ -375,6 +400,7 @@ private fun StickerPackHeaderPreview() { private fun StickerCell( sticker: KeyboardSticker, allowAnimation: Boolean, + cellSize: Dp, onEvent: (StickerPageScreenEvents) -> Unit ) { val controller = remember { DropdownMenus.MenuController() } @@ -392,12 +418,13 @@ private fun StickerCell( controller.show() } ) - .padding(8.dp) ) { GlideImage( model = sticker.image, - enableApngAnimation = allowAnimation && sticker.isAnimated, + enableApngAnimation = allowAnimation, skipMemoryCache = true, + imageSize = DpSize(cellSize, cellSize), + contentScale = ContentScale.Fit, modifier = Modifier.fillMaxSize() ) diff --git a/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/sticker/StickerPageViewModel.kt b/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/sticker/StickerPageViewModel.kt index bc21878116..b6588b83dd 100644 --- a/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/sticker/StickerPageViewModel.kt +++ b/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/sticker/StickerPageViewModel.kt @@ -9,22 +9,19 @@ import androidx.annotation.VisibleForTesting import androidx.lifecycle.ViewModel import androidx.lifecycle.ViewModelProvider import androidx.lifecycle.viewModelScope -import kotlinx.coroutines.channels.Channel -import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach -import kotlinx.coroutines.flow.receiveAsFlow -import org.signal.core.ui.compose.EventDrivenViewModel import org.signal.core.util.logging.Log import org.signal.mediakeyboard.MediaKeyboardAction import org.signal.mediakeyboard.data.StickerKeyboardRepository +import org.signal.mediakeyboard.screens.MediaKeyboardPageViewModel class StickerPageViewModel( private val repository: StickerKeyboardRepository -) : EventDrivenViewModel(TAG, shouldLogEvents = false) { +) : MediaKeyboardPageViewModel(TAG, shouldLogEvents = false) { companion object { private val TAG = Log.tag(StickerPageViewModel::class) @@ -33,11 +30,6 @@ class StickerPageViewModel( private val _state = MutableStateFlow(StickerPageState(allowAnimation = repository.allowStickerAnimation)) val state: StateFlow = _state.asStateFlow() - private val actionChannel = Channel(Channel.UNLIMITED) - - /** What the user did that only the host can carry out, for whichever host is current. */ - val actions: Flow = actionChannel.receiveAsFlow() - init { onEvent(StickerPageScreenEvents.Initialize) repository.observeStickerPacks() @@ -79,24 +71,24 @@ class StickerPageViewModel( } is StickerPageScreenEvents.StickerClicked -> { - actionChannel.trySend(MediaKeyboardAction.StickerSelected(event.sticker)) + emitAction(MediaKeyboardAction.StickerSelected(event.sticker)) } is StickerPageScreenEvents.StickerSendClicked -> { repository.onStickerUsed(event.sticker) - actionChannel.trySend(MediaKeyboardAction.StickerSendClicked(event.sticker)) + emitAction(MediaKeyboardAction.StickerSendClicked(event.sticker)) } is StickerPageScreenEvents.SearchClicked -> { - actionChannel.trySend(MediaKeyboardAction.StickerSearchClicked) + emitAction(MediaKeyboardAction.StickerSearchClicked) } is StickerPageScreenEvents.ViewStickerPackClicked -> { - actionChannel.trySend(MediaKeyboardAction.ViewStickerPackClicked(event.packId, event.packKey)) + emitAction(MediaKeyboardAction.ViewStickerPackClicked(event.packId, event.packKey)) } is StickerPageScreenEvents.SendStickerPackClicked -> { - actionChannel.trySend(MediaKeyboardAction.SendStickerPackClicked(event.packId, event.packKey)) + emitAction(MediaKeyboardAction.SendStickerPackClicked(event.packId, event.packKey)) } is StickerPageScreenEvents.RemoveStickerPackClicked -> { @@ -108,7 +100,7 @@ class StickerPageViewModel( stateEmitter(state.copy(confirmRemovePack = null)) if (pack != null) { - actionChannel.trySend(MediaKeyboardAction.RemoveStickerPackConfirmed(pack.packId, pack.packKey)) + emitAction(MediaKeyboardAction.RemoveStickerPackConfirmed(pack.packId, pack.packKey)) } } diff --git a/feature/media-keyboard/src/test/java/org/signal/mediakeyboard/screens/emoji/EmojiPageViewModelTest.kt b/feature/media-keyboard/src/test/java/org/signal/mediakeyboard/screens/emoji/EmojiPageViewModelTest.kt index 68febeb7e1..9ec89ec865 100644 --- a/feature/media-keyboard/src/test/java/org/signal/mediakeyboard/screens/emoji/EmojiPageViewModelTest.kt +++ b/feature/media-keyboard/src/test/java/org/signal/mediakeyboard/screens/emoji/EmojiPageViewModelTest.kt @@ -5,7 +5,6 @@ package org.signal.mediakeyboard.screens.emoji -import androidx.lifecycle.viewModelScope import assertk.assertThat import assertk.assertions.isEqualTo import assertk.assertions.isNotNull @@ -14,11 +13,12 @@ import io.mockk.coEvery import io.mockk.coVerify import io.mockk.mockk import io.mockk.verify +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.cancel import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.launchIn -import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.launch import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.resetMain import kotlinx.coroutines.test.setMain @@ -44,6 +44,7 @@ class EmojiPageViewModelTest { private lateinit var repository: EmojiKeyboardRepository private lateinit var parentState: MutableStateFlow private lateinit var actions: MutableList + private lateinit var collectorScope: CoroutineScope @Before fun setup() { @@ -55,16 +56,18 @@ class EmojiPageViewModelTest { parentState = MutableStateFlow(MediaKeyboardState(offeredTabs = MediaKeyboardTab.entries, preferredTab = MediaKeyboardTab.EMOJI, initialized = true)) actions = mutableListOf() + collectorScope = CoroutineScope(testDispatcher) } @After fun tearDown() { + collectorScope.cancel() Dispatchers.resetMain() } private fun createViewModel(): EmojiPageViewModel { val viewModel = EmojiPageViewModel(repository, parentState) - viewModel.actions.onEach(actions::add).launchIn(viewModel.viewModelScope) + collectorScope.launch { viewModel.actions.collect { actions += it } } testDispatcher.scheduler.advanceUntilIdle() return viewModel } diff --git a/feature/media-keyboard/src/test/java/org/signal/mediakeyboard/screens/gif/GifPageViewModelTest.kt b/feature/media-keyboard/src/test/java/org/signal/mediakeyboard/screens/gif/GifPageViewModelTest.kt index 2a224d61d0..123134198e 100644 --- a/feature/media-keyboard/src/test/java/org/signal/mediakeyboard/screens/gif/GifPageViewModelTest.kt +++ b/feature/media-keyboard/src/test/java/org/signal/mediakeyboard/screens/gif/GifPageViewModelTest.kt @@ -5,7 +5,6 @@ package org.signal.mediakeyboard.screens.gif -import androidx.lifecycle.viewModelScope import assertk.assertThat import assertk.assertions.hasSize import assertk.assertions.isEqualTo @@ -13,10 +12,11 @@ import assertk.assertions.isFalse import assertk.assertions.isTrue import io.mockk.coEvery import io.mockk.mockk +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.flow.launchIn -import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.cancel +import kotlinx.coroutines.launch import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.resetMain import kotlinx.coroutines.test.setMain @@ -37,6 +37,7 @@ class GifPageViewModelTest { private lateinit var repository: GifKeyboardRepository private lateinit var actions: MutableList + private lateinit var collectorScope: CoroutineScope @Before fun setup() { @@ -44,16 +45,18 @@ class GifPageViewModelTest { repository = mockk() coEvery { repository.getGifs("", 0, any()) } returns Result.success(GifPage(gifs(20), hasMore = true)) actions = mutableListOf() + collectorScope = CoroutineScope(testDispatcher) } @After fun tearDown() { + collectorScope.cancel() Dispatchers.resetMain() } private fun createViewModel(): GifPageViewModel { val viewModel = GifPageViewModel(repository) - viewModel.actions.onEach(actions::add).launchIn(viewModel.viewModelScope) + collectorScope.launch { viewModel.actions.collect { actions += it } } testDispatcher.scheduler.advanceUntilIdle() return viewModel } diff --git a/feature/media-keyboard/src/test/java/org/signal/mediakeyboard/screens/sticker/StickerPageViewModelTest.kt b/feature/media-keyboard/src/test/java/org/signal/mediakeyboard/screens/sticker/StickerPageViewModelTest.kt index 6fe6daeaab..e47b4cbf03 100644 --- a/feature/media-keyboard/src/test/java/org/signal/mediakeyboard/screens/sticker/StickerPageViewModelTest.kt +++ b/feature/media-keyboard/src/test/java/org/signal/mediakeyboard/screens/sticker/StickerPageViewModelTest.kt @@ -5,7 +5,6 @@ package org.signal.mediakeyboard.screens.sticker -import androidx.lifecycle.viewModelScope import assertk.assertThat import assertk.assertions.isEmpty import assertk.assertions.isEqualTo @@ -13,11 +12,12 @@ import assertk.assertions.isNull import io.mockk.every import io.mockk.mockk import io.mockk.verify +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.cancel import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.launchIn -import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.launch import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.resetMain import kotlinx.coroutines.test.setMain @@ -43,6 +43,7 @@ class StickerPageViewModelTest { private lateinit var repository: StickerKeyboardRepository private lateinit var packsFlow: MutableStateFlow> private lateinit var actions: MutableList + private lateinit var collectorScope: CoroutineScope @Before fun setup() { @@ -53,16 +54,18 @@ class StickerPageViewModelTest { every { repository.observeStickerPacks() } returns packsFlow actions = mutableListOf() + collectorScope = CoroutineScope(testDispatcher) } @After fun tearDown() { + collectorScope.cancel() Dispatchers.resetMain() } private fun createViewModel(): StickerPageViewModel { val viewModel = StickerPageViewModel(repository) - viewModel.actions.onEach(actions::add).launchIn(viewModel.viewModelScope) + collectorScope.launch { viewModel.actions.collect { actions += it } } testDispatcher.scheduler.advanceUntilIdle() return viewModel } diff --git a/lib/glide/src/main/java/org/signal/glide/compose/GlideImage.kt b/lib/glide/src/main/java/org/signal/glide/compose/GlideImage.kt index 7ec577df5b..89ad5a0658 100644 --- a/lib/glide/src/main/java/org/signal/glide/compose/GlideImage.kt +++ b/lib/glide/src/main/java/org/signal/glide/compose/GlideImage.kt @@ -5,11 +5,7 @@ package org.signal.glide.compose -import android.graphics.Outline import android.graphics.drawable.Drawable -import android.view.View -import android.view.ViewOutlineProvider -import android.widget.ImageView import androidx.compose.foundation.Image import androidx.compose.runtime.Composable import androidx.compose.runtime.DisposableEffect @@ -23,7 +19,6 @@ import androidx.compose.ui.layout.ContentScale import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.unit.DpSize -import androidx.compose.ui.viewinterop.AndroidView import com.bumptech.glide.Glide import com.bumptech.glide.TransitionOptions import com.bumptech.glide.load.engine.DiskCacheStrategy @@ -35,8 +30,9 @@ import org.signal.glide.apng.ApngOptions /** * Our very own GlideImage. The GlideImage composable provided by the bumptech library is not suitable because it was is using our encrypted cache decoder/encoder. * - * @param contentScale How the loaded drawable is scaled into the available space. Ignored when [enableApngAnimation] is - * set, as that path hands scaling to the underlying [ImageView] via [scaleType]. + * @param contentScale How the loaded drawable is scaled into the available space. This is applied when drawing, so + * unlike [scaleType], which becomes a Glide request transform, it also reaches an animated APNG. + * @param enableApngAnimation Plays the model as an animated APNG when it is one. * @param skipMemoryCache Set this when the same model is loaded at several sizes, so that a stateful resource such as * an APNG frame decoder is not shared across differently-sized targets. */ @@ -53,70 +49,6 @@ fun GlideImage( contentScale: ContentScale = ContentScale.Crop, enableApngAnimation: Boolean = false, skipMemoryCache: Boolean = false -) { - if (enableApngAnimation) { - val density = LocalDensity.current - - AndroidView( - factory = { context -> ImageView(context) }, - update = { imageView -> - // Request-level transforms don't reach the animated APNG resource, so scaling/clipping is done on - // the view. Set in update (not factory) so a recycled view picks up a changed scaleType. - imageView.scaleType = scaleType.toImageViewScaleType() - imageView.outlineProvider = if (scaleType == GlideImageScaleType.CIRCLE_CROP) CircleOutlineProvider else null - imageView.clipToOutline = scaleType == GlideImageScaleType.CIRCLE_CROP - - Glide.with(imageView.context) - .load(model) - .fallback(fallback) - .error(error) - .diskCacheStrategy(diskCacheStrategy) - .set(ApngOptions.ANIMATE, enableApngAnimation) - .skipMemoryCache(skipMemoryCache) - .apply { - transition?.let(this::transition) - - if (imageSize != null) { - with(density) { - this@apply.override(imageSize.width.toPx().toInt(), imageSize.height.toPx().toInt()) - } - } - } - .into(imageView) - }, - onReset = { - Glide.with(it.context).clear(it) - }, - modifier = modifier - ) - } else { - GlideImage( - model = model, - imageSize = imageSize, - scaleType = scaleType, - fallback = fallback, - error = error, - transition = transition, - diskCacheStrategy = diskCacheStrategy, - contentScale = contentScale, - skipMemoryCache = skipMemoryCache, - modifier = modifier - ) - } -} - -@Composable -private fun GlideImage( - modifier: Modifier = Modifier, - model: T?, - imageSize: DpSize? = null, - scaleType: GlideImageScaleType = GlideImageScaleType.FIT_CENTER, - fallback: Drawable? = null, - error: Drawable? = fallback, - transition: TransitionOptions<*, Drawable>? = null, - diskCacheStrategy: DiskCacheStrategy = DiskCacheStrategy.ALL, - contentScale: ContentScale = ContentScale.Crop, - skipMemoryCache: Boolean = false ) { var drawable by remember { mutableStateOf(null) @@ -136,13 +68,15 @@ private fun GlideImage( val density = LocalDensity.current val context = LocalContext.current - DisposableEffect(model, fallback, error, diskCacheStrategy, density, imageSize, skipMemoryCache) { + DisposableEffect(model, fallback, error, diskCacheStrategy, density, imageSize, enableApngAnimation, skipMemoryCache) { val requestManager = Glide.with(context) val builder = requestManager .load(model) .fallback(fallback) .error(error) .diskCacheStrategy(diskCacheStrategy) + // ApngOptions.ANIMATE defaults to true, so a caller that did not ask for animation has to say so. + .set(ApngOptions.ANIMATE, enableApngAnimation) .skipMemoryCache(skipMemoryCache) .apply { scaleType.applyTo(this) @@ -175,19 +109,6 @@ private fun GlideImage( } } -/** - * Clips a view to the largest circle that fits it, centred, matching what - * [com.bumptech.glide.request.RequestOptions.circleCrop] would have produced. - */ -private object CircleOutlineProvider : ViewOutlineProvider() { - override fun getOutline(view: View, outline: Outline) { - val diameter = minOf(view.width, view.height) - val left = (view.width - diameter) / 2 - val top = (view.height - diameter) / 2 - outline.setOval(left, top, left + diameter, top + diameter) - } -} - enum class GlideImageScaleType { /** @see [com.bumptech.glide.request.RequestOptions.fitCenter] */ FIT_CENTER, @@ -209,16 +130,4 @@ enum class GlideImageScaleType { CIRCLE_CROP -> builder.circleCrop() } } - - /** - * [CIRCLE_CROP] scales like [CENTER_CROP] here; the circle itself is clipped from the view, since - * an [ImageView] scaleType cannot describe one. - */ - fun toImageViewScaleType(): ImageView.ScaleType { - return when (this) { - FIT_CENTER -> ImageView.ScaleType.FIT_CENTER - CENTER_INSIDE -> ImageView.ScaleType.CENTER_INSIDE - CENTER_CROP, CIRCLE_CROP -> ImageView.ScaleType.CENTER_CROP - } - } }