-
Notifications
You must be signed in to change notification settings - Fork 659
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.
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
Load very big image throw OOM #1349
Comments
There isn’t enough information in this report. What code are you using to load the image? How can I reproduce this? |
I success reproduce it in a empty project just now.
Wait for your news. This is demo: This is images: |
Thanks for the repro project. I’ll take a look soon. |
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?
Using the following code:
|
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 |
@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 |
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 |
@spacecowboy It is possible to catch the error. If you want different behaviour you can use a custom |
thanks! horrible to have to do that but it'll work |
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? |
@spacecowboy I'd create a custom Coil interceptor that throws if the drawable is too large. That has the benefit of triggering 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. |
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 |
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
}
} |
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)
}
}
} |
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 |
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. |
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 |
Coil 3.0 has a new |
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?
The text was updated successfully, but these errors were encountered: