coil-kt / coil

Image loading for Android and Compose Multiplatform.
https://coil-kt.github.io/coil/
Apache License 2.0
10.46k stars 639 forks source link

Load very big image throw OOM #1349

Open XuQK opened 1 year ago

XuQK commented 1 year ago

Describe the bug Load a 107MB png image, sometimes throw OOM.

Logs/Screenshots attached. 2022-07-07 14:36:25.093 4541-4541.txt

Version coil-2.1.0 What library version are you using? Does this occur on a specific API level or Android device?

colinrtwhite commented 1 year ago

There isn’t enough information in this report. What code are you using to load the image? How can I reproduce this?

XuQK commented 1 year ago

I success reproduce it in a empty project just now.

  1. Download images attached, copy them 3 times, now we have 15 images.
  2. Put 15 images to /sdcard/Pictures/bigpic.
  3. Install the demo, give it storage permission manual in Settings.
  4. Then Run the demo, you should see there are image list loaded, keep sliding up and down for a while, then the OOM occured.
  5. If not reproduce, copy the picture into 30 pieces, repeat.

Wait for your news.

This is demo: CoilTest.zip

This is images: https://drive.google.com/file/d/1-CNSN7AxCfsG7TX4PKU2CUvLeDLlu6rF/view?usp=sharing

colinrtwhite commented 1 year ago

Thanks for the repro project. I’ll take a look soon.

spacecowboy commented 1 year ago

Same issue in Coil 2.2.0 when trying to load this image (on an Android Emulator):

http://www.corsix.org/images/495f98eb0abe80037a8bfb0b3dfb943df9e04495.svg

Note that the image is only 14kb but coil is attempting to decode a 142mb file?

2022-08-31 00:39:32.474 6398-6398/com.nononsenseapps.feeder.debug E/AndroidRuntime: FATAL EXCEPTION: main
    Process: com.nononsenseapps.feeder.debug, PID: 6398
    java.lang.RuntimeException: Canvas: trying to draw too large(142657200bytes) bitmap.
        at android.graphics.RecordingCanvas.throwIfCannotDraw(RecordingCanvas.java:266)
        at android.graphics.BaseRecordingCanvas.drawBitmap(BaseRecordingCanvas.java:94)
        at androidx.compose.ui.graphics.AndroidCanvas.drawImageRect-HPBpro0(AndroidCanvas.android.kt:271)
        at androidx.compose.ui.graphics.drawscope.CanvasDrawScope.drawImage-AZ2fEMs(CanvasDrawScope.kt:263)
        at androidx.compose.ui.node.LayoutNodeDrawScope.drawImage-AZ2fEMs(Unknown Source:40)
        at androidx.compose.ui.graphics.drawscope.DrawScope.drawImage-AZ2fEMs$default(DrawScope.kt:510)
        at androidx.compose.ui.graphics.painter.BitmapPainter.onDraw(BitmapPainter.kt:93)
        at androidx.compose.ui.graphics.painter.Painter.draw-x_KDEd0(Painter.kt:212)
        at coil.compose.AsyncImagePainter.onDraw(AsyncImagePainter.kt:210)
        at androidx.compose.ui.graphics.painter.Painter.draw-x_KDEd0(Painter.kt:212)
        at androidx.compose.ui.draw.PainterModifier.draw(PainterModifier.kt:281)
        at androidx.compose.ui.node.DrawEntity.draw(DrawEntity.kt:98)
        at androidx.compose.ui.node.LayoutNodeWrapper.drawContainedDrawModifiers(LayoutNodeWrapper.kt:336)
        at androidx.compose.ui.node.LayoutNodeWrapper.draw(LayoutNodeWrapper.kt:326)
        at androidx.compose.ui.node.ModifiedLayoutNode.performDraw(ModifiedLayoutNode.kt:242)
        at androidx.compose.ui.node.LayoutNodeWrapper.drawContainedDrawModifiers(LayoutNodeWrapper.kt:334)
        at androidx.compose.ui.node.LayoutNodeWrapper.access$drawContainedDrawModifiers(LayoutNodeWrapper.kt:64)
        at androidx.compose.ui.node.LayoutNodeWrapper$invoke$1.invoke(LayoutNodeWrapper.kt:358)
        at androidx.compose.ui.node.LayoutNodeWrapper$invoke$1.invoke(LayoutNodeWrapper.kt:357)
        at androidx.compose.runtime.snapshots.Snapshot$Companion.observe(Snapshot.kt:2118)
        at androidx.compose.runtime.snapshots.SnapshotStateObserver$observeReads$1$1.invoke(SnapshotStateObserver.kt:129)
        at androidx.compose.runtime.snapshots.SnapshotStateObserver$observeReads$1$1.invoke(SnapshotStateObserver.kt:125)
        at androidx.compose.runtime.SnapshotStateKt__DerivedStateKt.observeDerivedStateRecalculations(DerivedState.kt:336)
        at androidx.compose.runtime.SnapshotStateKt.observeDerivedStateRecalculations(Unknown Source:1)
        at androidx.compose.runtime.snapshots.SnapshotStateObserver.observeReads(SnapshotStateObserver.kt:125)
        at androidx.compose.ui.node.OwnerSnapshotObserver.observeReads$ui_release(OwnerSnapshotObserver.kt:120)
        at androidx.compose.ui.node.LayoutNodeWrapper.invoke(LayoutNodeWrapper.kt:357)
        at androidx.compose.ui.node.LayoutNodeWrapper.invoke(LayoutNodeWrapper.kt:64)
        at androidx.compose.ui.platform.RenderNodeApi29.record(RenderNodeApi29.android.kt:180)
        at androidx.compose.ui.platform.RenderNodeLayer.updateDisplayList(RenderNodeLayer.android.kt:298)
        at androidx.compose.ui.platform.AndroidComposeView.dispatchDraw(AndroidComposeView.android.kt:1005)
        at android.view.View.draw(View.java:23198)
        at android.view.View.updateDisplayListIfDirty(View.java:22062)
        at android.view.ViewGroup.recreateChildDisplayList(ViewGroup.java:4513)
        at android.view.ViewGroup.dispatchGetDisplayList(ViewGroup.java:4486)
        at android.view.View.updateDisplayListIfDirty(View.java:22018)
        at android.view.ViewGroup.recreateChildDisplayList(ViewGroup.java:4513)
        at android.view.ViewGroup.dispatchGetDisplayList(ViewGroup.java:4486)
        at android.view.View.updateDisplayListIfDirty(View.java:22018)
        at android.view.ViewGroup.recreateChildDisplayList(ViewGroup.java:4513)
        at android.view.ViewGroup.dispatchGetDisplayList(ViewGroup.java:4486)
        at android.view.View.updateDisplayListIfDirty(View.java:22018)
        at android.view.ViewGroup.recreateChildDisplayList(ViewGroup.java:4513)
        at android.view.ViewGroup.dispatchGetDisplayList(ViewGroup.java:4486)
        at android.view.View.updateDisplayListIfDirty(View.java:22018)
        at android.view.ThreadedRenderer.updateViewTreeDisplayList(ThreadedRenderer.java:682)
2022-08-31 00:39:32.474 6398-6398/com.nononsenseapps.feeder.debug E/AndroidRuntime:     at android.view.ThreadedRenderer.updateRootDisplayList(ThreadedRenderer.java:688)
        at android.view.ThreadedRenderer.draw(ThreadedRenderer.java:786)
        at android.view.ViewRootImpl.draw(ViewRootImpl.java:4579)
        at android.view.ViewRootImpl.performDraw(ViewRootImpl.java:4290)
        at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:3517)
        at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:2286)
        at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:8948)
        at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1231)
        at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1239)
        at android.view.Choreographer.doCallbacks(Choreographer.java:899)
        at android.view.Choreographer.doFrame(Choreographer.java:832)
        at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:1214)
        at android.os.Handler.handleCallback(Handler.java:942)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loopOnce(Looper.java:201)
        at android.os.Looper.loop(Looper.java:288)
        at android.app.ActivityThread.main(ActivityThread.java:7898)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:936)

Using the following code:

Image(
                                    painter = rememberAsyncImagePainter(
                                        model = ImageRequest.Builder(LocalContext.current)
                                            .data(imageUrl)
                                            .scale(Scale.FILL)
                                            .placeholder(placeHolder)
                                            .error(placeHolder)
                                            .precision(Precision.INEXACT)
                                            .size(1000)
                                            .build(),
                                        contentScale = ContentScale.Crop,
                                    ),
                                    contentScale = ContentScale.Crop,
                                    contentDescription = null,
                                    modifier = modifier
                                        .fillMaxWidth()
                                        .aspectRatio(16.0f / 9.0f)
                                )
spacecowboy commented 1 year ago

Adding a simple try-catch ensures the app doesn't crash

diff --git i/coil-compose-base/src/main/java/coil/compose/AsyncImagePainter.kt w/coil-compose-base/src/main/java/coil/compose/AsyncImagePainter.kt
index 237b82da..67674e5d 100644
--- i/coil-compose-base/src/main/java/coil/compose/AsyncImagePainter.kt
+++ w/coil-compose-base/src/main/java/coil/compose/AsyncImagePainter.kt
@@ -3,6 +3,7 @@ package coil.compose
 import android.graphics.drawable.BitmapDrawable
 import android.graphics.drawable.ColorDrawable
 import android.graphics.drawable.Drawable
+import android.util.Log
 import androidx.compose.foundation.Image
 import androidx.compose.foundation.lazy.LazyColumn
 import androidx.compose.foundation.lazy.LazyRow
@@ -207,7 +208,11 @@ class AsyncImagePainter internal constructor(
         drawSize.value = size

         // Draw the current painter.
-        painter?.apply { draw(size, alpha, colorFilter) }
+        try {
+            painter?.apply { draw(size, alpha, colorFilter) }
+        } catch (e: Exception) {
+            Log.e("JCOIL", "Damn", e)
+        }
     }

     override fun applyAlpha(alpha: Float): Boolean {

but likely some kind of deeper redesign would be needed to return a proper error code since this happens AFTER coil already has returned the success status

colinrtwhite commented 1 year ago

@spacecowboy Your image is being decoded at a very large size, which is why it's throwing when it's drawn. We do want to throw the exception in this case instead of ignoring it as it could be the result of a misconfiguration.

Your SVG has the intrinsic dimensions width="101.484ex" height="2.843ex", which uses the ex unit (an uncommon type) so it's possible they're being interpreted incorrectly by the SVG rendering library. This would be a different issue - could you open a new bug for that?

spacecowboy commented 1 year ago

@spacecowboy Your image is being decoded at a very large size, which is why it's throwing when it's drawn. We do want to throw the exception in this case instead of ignoring it as it could be the result of a misconfiguration.

Your SVG has the intrinsic dimensions width="101.484ex" height="2.843ex", which uses the ex unit (an uncommon type) so it's possible they're being interpreted incorrectly by the SVG rendering library. This would be a different issue - could you open a new bug for that?

Yes I can do that but to reply to your "we want to throw" statement: that's very bad.

There is no possibility for my app to catch this error. Note In the stack trace that there is no mention of my app (com.nononsenseapps.feeder).

I use coil in an RSS reader app. all images come from the RSS feeds added by users so I have no control what images are going to be decoded. It can be an SVG like in this case but I've also seen the error with plain old bitmaps.

if this happens at the wrong place in the app, it is very possible to crash lock it and no way to stop the app from crashing on every startup except clearing the cache memory and opening it whilst in airplane mode.

so please do throw an exception but do so at a call site from which I can catch it please

colinrtwhite commented 1 year ago

@spacecowboy It is possible to catch the error. If you want different behaviour you can use a custom ImageView that wraps super.onDraw(canvas) to catch + ignore this exception. For context, Glide and Picasso also will also throw this exception in the same way.

spacecowboy commented 1 year ago

@spacecowboy It is possible to catch the error. If you want different behaviour you can use a custom ImageView that wraps super.onDraw(canvas) to catch + ignore this exception. For context, Glide and Picasso also will also throw this exception in the same way.

thanks! horrible to have to do that but it'll work

spacecowboy commented 1 year ago

@spacecowboy It is possible to catch the error. If you want different behaviour you can use a custom ImageView that wraps super.onDraw(canvas) to catch + ignore this exception. For context, Glide and Picasso also will also throw this exception in the same way.

I spoke too soon. Custom ImageView? My app is built with Jetpack Compose. Do you have a suggestion for how I can catch the error in compose?

colinrtwhite commented 1 year ago

@spacecowboy I'd create a custom Coil interceptor that throws if the drawable is too large. That has the benefit of triggering onError + the associated behaviour as well. Anything below 2500x2500 is pretty much always small enough to draw.

Also calling a solution "horrible" generally doesn't entice me to offer more help with your problem. If you have a plan for how to improve this that's consistent for both views and Compose I'm open.

spacecowboy commented 1 year ago

@spacecowboy I'd create a custom Coil interceptor that throws if the drawable is too large. That has the benefit of triggering onError + the associated behaviour as well. Anything below 2500x2500 is pretty much always small enough to draw.

Also calling a solution "horrible" generally doesn't entice me to offer more help with your problem. If you have a plan for how to improve this that's consistent for both views and Compose I'm open.

I apologize for my tone @colinrtwhite . I should have phrased it differently.

I'll give the interceptor a try.

brianguertin commented 1 year ago

This is still an issue. Feels like it should be build into Coil to not crash the app...

Anyway, I think this interceptor will fix it:

add { chain ->
    val request = chain.request
    val result = chain.proceed(request)
    val bitmap = (result.drawable as? BitmapDrawable)?.bitmap
    if (bitmap != null && bitmap.byteCount >= MAX_BITMAP_SIZE) {
        ErrorResult(
            request.error,
            request,
            RuntimeException("Bitmap is too large (${bitmap.byteCount} bytes)")
        )
    } else {
        result
    }
}
        /**
         * Copied from RecordingCanvas.MAX_BITMAP_SIZE
         */
        private const val MAX_BITMAP_SIZE = 100 * 1024 * 1024 // 100 MB
spacecowboy commented 1 year ago

This the interceptor I ended up using

/**
 * Ensures an error is returned instead of rendering images that are likely to trigger memory errors
 * onDraw - but are not SO large as too cause a OOM exception during decode.
 */
class TooLargeImageInterceptor : Interceptor {
    override suspend fun intercept(chain: Interceptor.Chain): ImageResult {
        return when (val result = chain.proceed(chain.request)) {
            is ErrorResult -> result
            is SuccessResult -> {
                val sumPixels = result.drawable.intrinsicWidth * result.drawable.intrinsicHeight

                if (sumPixels > MAX_PIXELS) {
                    return ErrorResult(
                        chain.request.error,
                        chain.request,
                        RuntimeException("Image was (probably) too large to render within memory constraints: ${result.drawable.intrinsicWidth} x ${result.drawable.intrinsicHeight} > 2500 x 2500"),
                    )
                } else {
                    result
                }
            }
        }
    }

    companion object {
        const val MAX_PIXELS = 2500 * 2500
    }
}
XuQK commented 1 year ago

I found that if it is just to prevent crashes, it is only necessary to capture the OOM exception in the interceptor。

class OOMInterceptor : Interceptor {
    override suspend fun intercept(chain: Interceptor.Chain): ImageResult {
        return try {
            chain.proceed(chain.request)
        } catch (e: OutOfMemoryError) {
            ErrorResult(null, chain.request, e)
        }
    }
}
spacecowboy commented 1 year ago

I found that if it is just to prevent crashes, it is only necessary to capture the OOM exception in the interceptor。

You are missing an edge case there @XuQK . It is possible to get images with just the right size that won't cause an OOM exception during the decode, but will crash the app when you try to display it

vitorpamplona commented 7 months ago

It would be great to have a Size.MAX_ALLOWED that would check the device's limits from RecordingCanvas and use the maximum allowable size if the image exceeds it, avoiding exceptions. Otherwise, it would behave as Size.ORIGINAL.

nateridderman commented 3 weeks ago

Anything below 2500x2500 is pretty much always small enough to draw.

For my app, running on a Zebra TC57 on Android 10, the magic number was 2350x2350. And for images that were more rectangular than square would work if the total # of pixels was less than 2350x2350.

I should mention my app applies a transformImage modifier in order to pan/zoom the image. When it fails, it does not crash, but instead shows a blank image. This is using the compose version of Coil.