From 6af2b2810793b6b74720487aca3d9863d541cf01 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 6 Feb 2026 06:53:33 +0000 Subject: [PATCH] Prevent redundant image request enqueueing in ReaderScreen Co-authored-by: Aatricks <113598245+Aatricks@users.noreply.github.com> --- .../novelscraper/ui/screens/ReaderScreen.kt | 60 +++++++----- .../ui/screens/ReaderScreenPrefetchTest.kt | 92 +++++++++++++++++++ 2 files changed, 131 insertions(+), 21 deletions(-) create mode 100644 app/src/test/java/io/aatricks/novelscraper/ui/screens/ReaderScreenPrefetchTest.kt diff --git a/app/src/main/java/io/aatricks/novelscraper/ui/screens/ReaderScreen.kt b/app/src/main/java/io/aatricks/novelscraper/ui/screens/ReaderScreen.kt index 4e6ab02..e21eafb 100644 --- a/app/src/main/java/io/aatricks/novelscraper/ui/screens/ReaderScreen.kt +++ b/app/src/main/java/io/aatricks/novelscraper/ui/screens/ReaderScreen.kt @@ -469,29 +469,13 @@ private fun ContentArea( } } + val requestedIndices = remember(content.url) { mutableSetOf() } + LaunchedEffect(listState.firstVisibleItemIndex, pagerState.currentPage, content.url) { val currentIndex = if (uiState.isPagedMode) pagerState.currentPage else listState.firstVisibleItemIndex - val prefetchRange = 10 - - val startRange = (currentIndex - 3).coerceAtLeast(0) - val endRange = (currentIndex + prefetchRange).coerceAtMost(content.paragraphs.size - 1) - - for (i in startRange..endRange) { - content.paragraphs.getOrNull(i)?.let { element -> - when (element) { - is ContentElement.Image -> { - val request = ImageRequest.Builder(context).data(element.url).build() - SingletonImageLoader.get(context).enqueue(request) - } - is ContentElement.ImageGroup -> { - element.images.forEach { img -> - val request = ImageRequest.Builder(context).data(img.url).build() - SingletonImageLoader.get(context).enqueue(request) - } - } - else -> {} - } - } + prefetchImages(currentIndex, content, requestedIndices) { url -> + val request = ImageRequest.Builder(context).data(url).build() + SingletonImageLoader.get(context).enqueue(request) } } @@ -1024,3 +1008,37 @@ private fun rememberReaderNestedScrollConnection( } } } + +internal fun prefetchImages( + currentIndex: Int, + content: ChapterContent, + requestedIndices: MutableSet, + onEnqueue: (String) -> Unit +) { + val prefetchRange = 10 + + val startRange = (currentIndex - 3).coerceAtLeast(0) + val endRange = (currentIndex + prefetchRange).coerceAtMost(content.paragraphs.size - 1) + + for (i in startRange..endRange) { + if (i in requestedIndices) continue + + content.paragraphs.getOrNull(i)?.let { element -> + when (element) { + is ContentElement.Image -> { + onEnqueue(element.url) + requestedIndices.add(i) + } + is ContentElement.ImageGroup -> { + element.images.forEach { img -> + onEnqueue(img.url) + } + requestedIndices.add(i) + } + else -> { + requestedIndices.add(i) + } + } + } + } +} diff --git a/app/src/test/java/io/aatricks/novelscraper/ui/screens/ReaderScreenPrefetchTest.kt b/app/src/test/java/io/aatricks/novelscraper/ui/screens/ReaderScreenPrefetchTest.kt new file mode 100644 index 0000000..24ce852 --- /dev/null +++ b/app/src/test/java/io/aatricks/novelscraper/ui/screens/ReaderScreenPrefetchTest.kt @@ -0,0 +1,92 @@ +package io.aatricks.novelscraper.ui.screens + +import io.aatricks.novelscraper.data.model.ChapterContent +import io.aatricks.novelscraper.data.model.ContentElement +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +class ReaderScreenPrefetchTest { + + @Test + fun `prefetchImages enqueues requests only for new indices`() { + // Setup + val paragraphs = listOf( + ContentElement.Text("p0"), + ContentElement.Image("url1"), // index 1 + ContentElement.Text("p2"), + ContentElement.Image("url3"), // index 3 + ContentElement.Image("url4"), // index 4 + ContentElement.Text("p5"), + ContentElement.ImageGroup(listOf(ContentElement.Image("url6a"), ContentElement.Image("url6b"))) // index 6 + ) + val content = ChapterContent( + paragraphs = paragraphs, + url = "http://example.com/chapter1" + ) + val requestedIndices = mutableSetOf() + val enqueuedUrls = mutableListOf() + val onEnqueue: (String) -> Unit = { url -> enqueuedUrls.add(url) } + + // Act 1: Initial prefetch (index 0) + // Window: -3 to 10. (0..10) + // Should verify indices 0..6 + prefetchImages( + currentIndex = 0, + content = content, + requestedIndices = requestedIndices, + onEnqueue = onEnqueue + ) + + // Assert 1 + // Expected requests: url1 (idx 1), url3 (idx 3), url4 (idx 4), url6a (idx 6), url6b (idx 6) + assertEquals(5, enqueuedUrls.size) + assertTrue(enqueuedUrls.containsAll(listOf("url1", "url3", "url4", "url6a", "url6b"))) + assertTrue(requestedIndices.containsAll(listOf(0, 1, 2, 3, 4, 5, 6))) + + // Clear enqueued to verify new ones + enqueuedUrls.clear() + + // Act 2: Scroll slightly (index 1) + // Window: -2 to 11. (0..11) + // Range 0..6 is already requested. + // If prefetchImages is optimized, it should NOT request anything for 0..6. + // It might check 7, 8... but they don't exist. + prefetchImages( + currentIndex = 1, + content = content, + requestedIndices = requestedIndices, + onEnqueue = onEnqueue + ) + + // Assert 2 + assertEquals(0, enqueuedUrls.size) + + // Act 3: Add new content/simulate scrolling to new area if we had more content + // Let's create a larger content list to test valid scrolling + val largeParagraphs = (0..20).map { i -> + if (i % 2 == 0) ContentElement.Text("p$i") else ContentElement.Image("url$i") + } + val largeContent = ChapterContent(paragraphs = largeParagraphs, url = "http://test.com") + val largeRequested = mutableSetOf() + val largeEnqueued = mutableListOf() + + // Scroll to 0. Range: 0..10 + prefetchImages(0, largeContent, largeRequested, { largeEnqueued.add(it) }) + // Images at 1, 3, 5, 7, 9. (5 images) + assertEquals(5, largeEnqueued.size) + assertTrue(largeRequested.contains(1)) + + largeEnqueued.clear() + + // Scroll to 2. Range: 0..12. + // New indices to check: 11, 12. + // 11 is Image("url11"). 12 is Text. + // Should request url11. + prefetchImages(2, largeContent, largeRequested, { largeEnqueued.add(it) }) + + assertEquals(1, largeEnqueued.size) + assertEquals("url11", largeEnqueued[0]) + assertTrue(largeRequested.contains(11)) + } +}