Only do the Android U gainmap workaround on bitmaps that have a gainmap #5475
Conversation
| if (bitmapRegionDecoder == null) { | ||
| return null; | ||
| } | ||
| Rect onePercentRect = |
There was a problem hiding this comment.
Why one percent instead of a fixed 1x1 size?
There was a problem hiding this comment.
Because a fixed 1x1 size seems to drop the gainmap. I'm not sure if the issue is in Android framework or in the SKIA codec that it uses to decode the image.
There was a problem hiding this comment.
I added a comment clarifying the need to do this. I would have really preferred the 1x1 pixel solution. If we find a way to make 1x1 work, I can send a separate PR to go back to 1x1 along with the workaround (how many layers of workarounds will we have?).
There was a problem hiding this comment.
Hmm if the image is very low res (<200 px dimensions), the check fallbacks to a 1x1 check, which fails.
I can set the bar to something else, like having min values of 4 pixels (or 8) in each direction...maybe that's enough. This feels so hacky.
There was a problem hiding this comment.
Is there a minimum pixel size that is guaranteed to work regardless of image size? Any guidance from the framework team?
| @@ -72,7 +76,10 @@ public static Bitmap decodeFileDescriptor( | |||
|
|
|||
| /** | |||
| * Returns a decoded bitmap for the input stream, ensuring that any associated gainmap is decoded | |||
| * without errors on Android U if it is a valid gainamp. | |||
| * without errors on Android U if it is a valid gainmap. | |||
There was a problem hiding this comment.
Could you document what happens if there are errors with the gainmap - what does this method return then?
There was a problem hiding this comment.
Done. Android silently fails to decode the gainmap.
library/src/main/java/com/bumptech/glide/load/resource/bitmap/GlideBitmapFactory.java
Outdated
Show resolved
Hide resolved
library/src/main/java/com/bumptech/glide/load/resource/bitmap/GlideBitmapFactory.java
Outdated
Show resolved
Hide resolved
This behavior is already being tested, with the test added. This change simply is more memory efficient. There should be no API changes. I could add more test files that I know fail the 1x1 pixel test but pass the 1% test, but unfortunately this bug only manifests in hardware and we would need Glide instrumentation tests actually running on real Samsung S24+ devices that have this bug, which is beyond the scope of what I can do (instrumentation tests are not running this Glide repo, I don't know if S24 devices are available anyway). |
Description
Before attempting to software decode bitmaps on Android U devices that are susceptible to the Android framework gainmap bug, we decode a 1% sized region of the input using a BitmapRegionDecoder with a hardware configuration. If we detect a gainmap has been decoded, then we proceed with the software bitmap workaround.
Motivation and Context
We should only decode software bitmaps as a last resort. See #5474 . I thought about just using BitmapRegionDecoder, but that would break the Glide API where callers select which decoder to use.