Skip to content

refactor(mediaviewer): replace android-gif-drawable with Coil #6785

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rapterjet2004 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

refactor(mediaviewer): replace android-gif-drawable with Coil #6785

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion app/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,6 @@ dependencies {
implementation("androidx.media3:media3-effect:$media3Version")

implementation("com.github.chrisbanes:PhotoView:2.3.0")
implementation("pl.droidsonroids.gif:android-gif-drawable:1.2.32")

implementation("io.noties.markwon:core:$markwonVersion")
implementation("io.noties.markwon:ext-strikethrough:$markwonVersion")
Expand Down
28 changes: 9 additions & 19 deletions app/src/main/java/com/nextcloud/talk/mediaviewer/activities/MediaViewerScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/
package com.nextcloud.talk.mediaviewer.activities

import android.graphics.drawable.Animatable
import android.graphics.drawable.Drawable
import android.util.Log
import android.view.View
Expand Down Expand Up @@ -89,11 +90,8 @@ import com.nextcloud.talk.utils.DateConstants
import com.nextcloud.talk.utils.DateUtils
import com.nextcloud.talk.utils.DrawableUtils
import com.nextcloud.talk.utils.Mimetype
import com.nextcloud.talk.utils.MimetypeUtils
import java.io.File
import kotlinx.coroutines.launch
import pl.droidsonroids.gif.GifDrawable
import pl.droidsonroids.gif.GifImageView

private const val TOOLBAR_ALPHA = 0.6f
private const val MAX_SCALE = 6.0f
Expand Down Expand Up @@ -306,7 +304,6 @@ private fun MediaPage(
controllerExtraBottomInsetPx: Int
) {
val isVideo = item.mimeType.startsWith(Mimetype.VIDEO_PREFIX)
val isGif = MimetypeUtils.isGif(item.mimeType)

Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) {
when {
Expand All @@ -320,7 +317,6 @@ private fun MediaPage(
} else {
PreviewPlaceholder(item, onToggleControls = onToggleControls)
}
isGif -> GifPage(localPath = localPath, onToggleControls = onToggleControls)
else -> ImagePage(localPath = localPath, onToggleControls = onToggleControls)
}
}
Expand All @@ -338,19 +334,6 @@ private fun PreviewPlaceholder(item: MediaViewerItem, onToggleControls: () -> Un
}
}

@Composable
private fun GifPage(localPath: String, onToggleControls: () -> Unit) {
AndroidView(
factory = { ctx ->
GifImageView(ctx).apply {
setImageDrawable(GifDrawable(localPath))
setOnClickListener { onToggleControls() }
}
},
modifier = Modifier.fillMaxSize()
)
}

@Composable
private fun ImagePage(localPath: String, onToggleControls: () -> Unit) {
val context = LocalContext.current
Expand Down Expand Up @@ -379,7 +362,14 @@ private fun ImagePage(localPath: String, onToggleControls: () -> Unit) {
setOnOutsidePhotoTapListener { onToggleControls() }
}
},
update = { photoView -> drawable?.let { photoView.setImageDrawable(it) } },
update = { photoView ->
drawable?.let {
photoView.setImageDrawable(it)
// Animated images (GIF, animated WebP/HEIF) decoded by Coil are not started automatically
(it as? Animatable)?.start()
}
},
onRelease = { photoView -> (photoView.drawable as? Animatable)?.stop() },
Comment on lines +365 to +372

@coderabbitai coderabbitai Bot Sep 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

sed -n '270,390p' app/src/main/java/com/nextcloud/talk/mediaviewer/activities/MediaViewerScreen.kt
rg -n 'HorizontalPager|PagerState|currentPage|beyondViewportPageCount|ImagePage\\(' app/src/main/java/com/nextcloud/talk/mediaviewer

Repository: nextcloud/talk-android

Length of output: 4472

🏁 Script executed:

#!/bin/bash
set -e
sed -n '1,280p' app/src/main/java/com/nextcloud/talk/mediaviewer/activities/MediaViewerScreen.kt
printf '\n--- relevant usages ---\n'
rg -n -e 'HorizontalPager' -e 'PagerState' -e 'currentPage' -e 'beyondViewportPageCount' -e 'ImagePage' app/src/main/java/com/nextcloud/talk/mediaviewer
printf '\n--- PR diff for target file ---\n'
git diff --unified=30 f704b95fba3c9ac6b3136df59863a011a760e7aa 4ee468d7ebe069c815b97c05d8bac24db96ded53 -- app/src/main/java/com/nextcloud/talk/mediaviewer/activities/MediaViewerScreen.kt

Repository: nextcloud/talk-android

Length of output: 25442

🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '--- Compose dependency declarations ---'
rg -n -i -e 'compose' -e 'foundation' -e 'androidx.compose' --glob '*.gradle' --glob '*.gradle.kts' --glob 'libs.versions.toml' --glob 'gradle.properties' .
printf '%s\n' '--- local AndroidX or pager source candidates ---'
find . -path '*/build' -prune -o -type f \( -iname '*Pager*.kt' -o -iname '*Pager*.java' \) -print | head -80
printf '%s\n' '--- comparable pager lifecycle usage ---'
rg -n -C 3 -e 'onRelease\s*=' -e 'beyondViewportPageCount' -e 'HorizontalPager' --glob '*.kt' .

Repository: nextcloud/talk-android

Length of output: 9187

🌐 Web query:

official AndroidX Compose Foundation HorizontalPager default beyondViewportPageCount composition offscreen pages onRelease

💡 Result:

For AndroidX Compose Foundation `HorizontalPager`, **the default `beyondViewportPageCount` is `0`** (`PagerDefaults.BeyondViewportPageCount`). That means it doesn’t deliberately compose and lay out extra pages before or after the visible pages. ([developer.android.com](https://developer.android.com/reference/kotlin/androidx/compose/foundation/pager/PagerDefaults?utm_source=openai))

It may still compose pages ahead during scrolling: the pager’s prefetcher can compose and lay out pages in the scroll direction independently of that setting. ([developer.android.com](https://developer.android.com/reference/kotlin/androidx/compose/foundation/pager/PagerDefaults?utm_source=openai))

If by “onRelease” you mean what happens to offscreen pages after a gesture ends: `beyondViewportPageCount` isn’t an offscreen-retention limit. Pager is lazy, so pages outside the visible/beyond-viewport range may leave composition; prefetching during scrolling is a separate behavior. ([developer.android.com](https://developer.android.com/reference/kotlin/androidx/compose/foundation/pager/HorizontalPager.composable?utm_source=openai))

Citations:

- 1: https://developer.android.com/reference/kotlin/androidx/compose/foundation/pager/PagerDefaults?utm_source=openai
- 2: https://developer.android.com/reference/kotlin/androidx/compose/foundation/pager/PagerDefaults?utm_source=openai
- 3: https://developer.android.com/reference/kotlin/androidx/compose/foundation/pager/HorizontalPager.composable?utm_source=openai

Stop animated drawables when the page is not current.

HorizontalPager can compose a page during prefetch before that page is visible. ImagePage starts every decoded Animatable regardless of isCurrentPage, so a prefetched animated image can consume animation resources until AndroidView.onRelease removes the page.

Pass the existing activity flag to ImagePage and stop the drawable when the page is not current.

Suggested fix 
-            else -> ImagePage(localPath = localPath, onToggleControls = onToggleControls)
+            else -> ImagePage(
+                localPath = localPath,
+                isCurrentPage = isCurrentPage,
+                onToggleControls = onToggleControls
+            )
...
-private fun ImagePage(localPath: String, onToggleControls: () -> Unit) {
+private fun ImagePage(
+    localPath: String,
+    isCurrentPage: Boolean,
+    onToggleControls: () -> Unit
+) {
...
-                (it as? Animatable)?.start()
+                (it as? Animatable)?.let { animatable ->
+                    if (isCurrentPage) animatable.start() else animatable.stop()
+                }

modifier = Modifier.fillMaxSize()
)
}
Expand Down
39 changes: 0 additions & 39 deletions gradle/verification-metadata.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,6 @@
<trusting group="org.osmdroid" name="osmdroid-android" version="6.1.18"/>
<trusting group="org.osmdroid" name="osmdroid-android" version="6.1.20"/>
</trusted-key>
<trusted-key id="3D9CDB50E2EAB3AA068D74A188518C11ADAEFC68" group="pl.droidsonroids.gif" name="android-gif-drawable" version="1.2.28"/>
<trusted-key id="3F05DDA9F317301E927136D417A27CE7A60FF5F0" group="io.opentelemetry"/>
<trusted-key id="4021EEEAFF5DE8404DCD0A270AA3E5C3D232E79B" group="jakarta.inject" name="jakarta.inject-api" version="2.0.1"/>
<trusted-key id="4161A24D2E7F660FCFDB6AAFAE094C852B4EE5DC">
Expand Down Expand Up @@ -402,12 +401,6 @@
<trusted-key id="AFCC4C7594D09E2182C60E0F7A01B0F236E5430F" group="com.google.code.gson"/>
<trusted-key id="B02335AA54CCF21E52BBF9ABD9C565AA72BA2FDD" group="io.grpc"/>
<trusted-key id="B087A0EB8416563AFE64CEBA4604091C01C3086A" group="com.mebigfatguy.fb-contrib" name="fb-contrib" version="7.6.4"/>
<trusted-key id="B17136D06153558101DB7DF723778689FBFBE047">
<trusting group="pl.droidsonroids.gif" name="android-gif-drawable" version="1.2.29"/>
<trusting group="pl.droidsonroids.gif" name="android-gif-drawable" version="1.2.30"/>
<trusting group="pl.droidsonroids.gif" name="android-gif-drawable" version="1.2.31"/>
<trusting group="pl.droidsonroids.gif" name="android-gif-drawable" version="1.2.32"/>
</trusted-key>
<trusted-key id="B2F967B67DADC1F07172DBDADE453E55DC86FC9B" group="co.touchlab"/>
<trusted-key id="B41089A2DA79B0FA5810252872385FF0AF338D52">
<trusting group="joda-time" name="joda-time" version="2.12.6"/>
Expand Down Expand Up @@ -33979,38 +33972,6 @@
<sha256 value="e154a3231cd6cde68aabf274fd18964d49a9c1a5f5e9ed3bf471e9ae7bdf08e6" origin="Generated by Gradle"/>
</artifact>
</component>
<component group="pl.droidsonroids.gif" name="android-gif-drawable" version="1.2.29">
<artifact name="android-gif-drawable-1.2.29.aar">
<sha256 value="611e2699782ee0d56168b6546962f75a54bdff03136d9db94019a65c0924eddd" origin="Generated by Gradle" reason="A key couldn't be downloaded"/>
</artifact>
<artifact name="android-gif-drawable-1.2.29.module">
<sha256 value="525eb44b891d2977f7e536484d658d837d555e95ff6515ad118749534f915a06" origin="Generated by Gradle" reason="A key couldn't be downloaded"/>
</artifact>
</component>
<component group="pl.droidsonroids.gif" name="android-gif-drawable" version="1.2.30">
<artifact name="android-gif-drawable-1.2.30.aar">
<sha256 value="01abc20e976992eb18490e7884a80f48548583a22a9a8f97ec25a60913b17740" origin="Generated by Gradle"/>
</artifact>
<artifact name="android-gif-drawable-1.2.30.module">
<sha256 value="c3aa27753f013b76922569e554d4c1f2927bb8484121abff861dd2cea7c9c893" origin="Generated by Gradle"/>
</artifact>
</component>
<component group="pl.droidsonroids.gif" name="android-gif-drawable" version="1.2.31">
<artifact name="android-gif-drawable-1.2.31.aar">
<sha256 value="01abc20e976992eb18490e7884a80f48548583a22a9a8f97ec25a60913b17740" origin="Generated by Gradle"/>
</artifact>
<artifact name="android-gif-drawable-1.2.31.module">
<sha256 value="9080415ed02012187b240ad4cb1e4bc8c3dfc7e56dc4c1326d4752e593d82f23" origin="Generated by Gradle"/>
</artifact>
</component>
<component group="pl.droidsonroids.gif" name="android-gif-drawable" version="1.2.32">
<artifact name="android-gif-drawable-1.2.32.aar">
<sha256 value="369f7bed0e364ed2cc1bf6be8e2000c15b1e7a4fd2344a2812e8afbd47602dd6" origin="Generated by Gradle"/>
</artifact>
<artifact name="android-gif-drawable-1.2.32.module">
<sha256 value="7c5ae78d2ee681483867cdda8a2938817cc0934545f03bf8c4b4620751602339" origin="Generated by Gradle"/>
</artifact>
</component>
<component group="xerces" name="xercesImpl" version="2.12.0">
<artifact name="xercesImpl-2.12.0.pom">
<sha256 value="138ebb33cce7080fd5cdb4fb90a0ee550173d7e8d41024da8c0d788b1460a1f3" origin="Generated by Gradle"/>
Expand Down
Loading