From 92df96d81c76b7038b7dd4f0e8d902506cfd98e7 Mon Sep 17 00:00:00 2001 From: Saket Narayan Date: Wed, 20 Sep 2023 16:06:21 -0400 Subject: [PATCH] Workaround https://github.com/Kotlin/kotlinx.coroutines/issues/3895 --- .../subsamplingimage/internal/BitmapCache.kt | 8 +-- .../internal/BitmapCacheTest.kt | 60 +++++++------------ 2 files changed, 24 insertions(+), 44 deletions(-) diff --git a/zoomable-image/sub-sampling-image/src/main/kotlin/me/saket/telephoto/subsamplingimage/internal/BitmapCache.kt b/zoomable-image/sub-sampling-image/src/main/kotlin/me/saket/telephoto/subsamplingimage/internal/BitmapCache.kt index 1e575ee7..947e3c13 100644 --- a/zoomable-image/sub-sampling-image/src/main/kotlin/me/saket/telephoto/subsamplingimage/internal/BitmapCache.kt +++ b/zoomable-image/sub-sampling-image/src/main/kotlin/me/saket/telephoto/subsamplingimage/internal/BitmapCache.kt @@ -15,11 +15,8 @@ import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.transform import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch -import kotlinx.coroutines.withContext import me.saket.telephoto.subsamplingimage.internal.BitmapCache.LoadingState.InFlight import me.saket.telephoto.subsamplingimage.internal.BitmapCache.LoadingState.Loaded -import kotlin.coroutines.CoroutineContext -import kotlin.coroutines.EmptyCoroutineContext import kotlin.time.Duration import kotlin.time.Duration.Companion.milliseconds @@ -27,7 +24,6 @@ internal class BitmapCache( scope: CoroutineScope, private val decoder: ImageRegionDecoder, private val throttleEvery: Duration = 100.milliseconds, - private val throttleDispatcher: CoroutineContext = EmptyCoroutineContext, ) { private val activeTiles = Channel>(capacity = 10) private val cachedBitmaps = MutableStateFlow(emptyMap()) @@ -82,9 +78,7 @@ internal class BitmapCache( private fun Flow.throttleLatest(delay: Duration): Flow { return conflate().transform { emit(it) - withContext(throttleDispatcher) { - delay(delay) - } + delay(delay) } } } diff --git a/zoomable-image/sub-sampling-image/src/test/kotlin/me/saket/telephoto/subsamplingimage/internal/BitmapCacheTest.kt b/zoomable-image/sub-sampling-image/src/test/kotlin/me/saket/telephoto/subsamplingimage/internal/BitmapCacheTest.kt index 3f823b65..8ba3815e 100644 --- a/zoomable-image/sub-sampling-image/src/test/kotlin/me/saket/telephoto/subsamplingimage/internal/BitmapCacheTest.kt +++ b/zoomable-image/sub-sampling-image/src/test/kotlin/me/saket/telephoto/subsamplingimage/internal/BitmapCacheTest.kt @@ -9,19 +9,19 @@ import app.cash.turbine.ReceiveTurbine import app.cash.turbine.test import app.cash.turbine.turbineScope import com.google.common.truth.Truth.assertThat -import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.cancel import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.drop import kotlinx.coroutines.job -import kotlinx.coroutines.launch +import kotlinx.coroutines.plus +import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.runTest -import kotlinx.coroutines.withContext import org.junit.Test -import kotlin.coroutines.CoroutineContext -import kotlin.coroutines.EmptyCoroutineContext import kotlin.random.Random import kotlin.time.Duration import kotlin.time.Duration.Companion.milliseconds @@ -31,13 +31,11 @@ class BitmapCacheTest { private val decoder = FakeImageRegionDecoder() private fun TestScope.bitmapCache( - throttleEvery: Duration = 100.milliseconds, - throttleDispatcher: CoroutineContext = EmptyCoroutineContext, + throttleEvery: Duration = 100.milliseconds ) = BitmapCache( scope = backgroundScope, decoder = decoder, throttleEvery = throttleEvery, - throttleDispatcher = throttleDispatcher, ) @Test fun `when tiles are received, load bitmaps only for new tiles`() = runTest(timeout = 1.seconds) { @@ -118,11 +116,16 @@ class BitmapCacheTest { } } - @Test fun `latest tiles should not be discarded by throttling`() = runTest { - val cache = bitmapCache( + // Note to self: I'm using runBlocking() instead of runTest() here so that I can test delays. + // and also to work around https://github.com/cashapp/paparazzi/issues/1101. + @Test fun `throttle load requests`() = runBlocking { + val scope: CoroutineScope = this.plus(Job()) + val cache = BitmapCache( + scope = scope, + decoder = decoder, throttleEvery = 2.seconds, - throttleDispatcher = Dispatchers.NonDelaySkipping, ) + decoder.requestedRegions.test { val baseTile = fakeBitmapRegionTile(sampleSize = 4) val tile2 = fakeBitmapRegionTile(sampleSize = 1) @@ -130,31 +133,23 @@ class BitmapCacheTest { val tileToSkip = fakeBitmapRegionTile(sampleSize = 8) cache.loadOrUnloadForTiles(listOf(baseTile)) - backgroundScope.launch { - nonSkippedDelay(500.milliseconds) - decoder.decodedBitmaps.send(FakeImageBitmap()) - } - nonSkippedDelay(500.milliseconds) + // This tile should get overridden by the next set of + // tiles because the throttle window hasn't passed yet. + delay(500.milliseconds) cache.loadOrUnloadForTiles(listOf(tileToSkip)) - // These tiles should override the previous one because the throttle window hasn't passed yet. - nonSkippedDelay(500.milliseconds) + delay(500.milliseconds) cache.loadOrUnloadForTiles(listOf(baseTile, tile2, tile3)) - backgroundScope.launch { - nonSkippedDelay(500.milliseconds) - decoder.decodedBitmaps.send(FakeImageBitmap()) - decoder.decodedBitmaps.send(FakeImageBitmap()) - } - - // If the same tiles are requested again within the throttle - // window, they shouldn't get ignored for some reason. + // If the same tiles are requested again within the throttle window, + // neither the old one nor the new one should get ignored for some reason. cache.loadOrUnloadForTiles(listOf(baseTile, tile2, tile3)) assertThat(awaitItem()).isEqualTo(baseTile) - assertThat(awaitItem()).isEqualTo(tile2) - assertThat(awaitItem()).isEqualTo(tile3) + assertThat(listOf(awaitItem(), awaitItem())).containsExactly(tile2, tile3) + + scope.cancel() } } @@ -167,17 +162,8 @@ class BitmapCacheTest { bounds = IntRect(random.nextInt(), random.nextInt(), random.nextInt(), random.nextInt()) ) } - - private suspend fun nonSkippedDelay(duration: Duration) { - withContext(Dispatchers.NonDelaySkipping) { - delay(duration) - } - } } -// Used to prevent runTest() from skipping delays. -private val Dispatchers.NonDelaySkipping get() = Default - private class FakeImageRegionDecoder : ImageRegionDecoder { override val imageSize: IntSize get() = error("unused") override val imageOrientation: ExifMetadata.ImageOrientation get() = error("unused")