[Bugfix] Fix crash on previewing image to upload on Android P #7184
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Using hardware bitmap allocation on Android framework versions prior to Android Q causes a crash when decoding a bitmap if GL context wasn't initialised. The issue is not documented in ImageDecoder reference but it is mentioned in the comments of glide[1] with a link to internal google discussion. [1] https://github.com/bumptech/glide/blob/f83cc274b42cf02af6611d2a8c21e4a9b1f5d592/library/src/main/java/com/bumptech/glide/load/resource/bitmap/HardwareConfigState.java#L22 Signed-off-by: Paweł Matuszewski <[email protected]>
atpamat
force-pushed
the
camera-crash-pie-fix
branch
from
be52209 to
e5e6444
Compare
bmarty
approved these changes
Sep 21, 2022
Member
bmarty
left a comment
bmarty
left a comment
There was a problem hiding this comment.
Thanks for the fix and the Pull Request!
LGTM, I have triggered the CI to check the code.
| val listener = ImageDecoder.OnHeaderDecodedListener { decoder, _, _ -> | ||
| if (Build.VERSION.SDK_INT < Build.VERSION_CODES.Q) { | ||
| // Allocating hardware bitmap may cause a crash on framework versions prior to Android Q | ||
| decoder.setAllocator(ImageDecoder.ALLOCATOR_SOFTWARE) |
Member
There was a problem hiding this comment.
There is a compilation warning, it could be replaced by
Suggested change
| decoder.setAllocator(ImageDecoder.ALLOCATOR_SOFTWARE) | |
| decoder.allocator = ImageDecoder.ALLOCATOR_SOFTWARE |
| ImageDecoder.decodeBitmap(ImageDecoder.createSource(context.contentResolver, uri)) | ||
| val source = ImageDecoder.createSource(context.contentResolver, uri) | ||
| val listener = ImageDecoder.OnHeaderDecodedListener { decoder, _, _ -> | ||
| if (Build.VERSION.SDK_INT < Build.VERSION_CODES.Q) { |
Member
There was a problem hiding this comment.
I think in this case, it could be replaced by
Suggested change
| if (Build.VERSION.SDK_INT < Build.VERSION_CODES.Q) { | |
| if (Build.VERSION.SDK_INT == Build.VERSION_CODES.P) { |
But I understand that it's more logic to keep the code like that.
Contributor
Author
There was a problem hiding this comment.
done, I assumed there might be something in between, like there is with O_MR1 but there indeed isn't.
Contributor
Author
|
I can rebase and squash the commits it it's ready to be merged |
bmarty
approved these changes
Sep 22, 2022
Member
bmarty
left a comment
bmarty
left a comment
There was a problem hiding this comment.
Thanks for the update. I will squash the commits when merging the PR.
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Type of change
Closes #1404, closes #1851, closes #4545
Content
Use software bitmap allocation when previewing images to upload on devices with Android Pie.
Motivation and context
Using hardware bitmap allocation on Android framework versions prior to Android Q causes a crash when decoding a bitmap if GL context wasn't initialised. The issue is not documented in ImageDecoder reference but it is mentioned in the comments of glide[1] with a link to internal google discussion.
[1] https://github.com/bumptech/glide/blob/f83cc274b42cf02af6611d2a8c21e4a9b1f5d592/library/src/main/java/com/bumptech/glide/load/resource/bitmap/HardwareConfigState.java#L22
Screenshots / GIFs
Tests
Sending images from within Element
Sharing images to Element from external app
Tested devices
Checklist