-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[MEDIUM] [Android] Large images crashing the App - 'Canvas trying to draw too large' #34372
Comments
No C+ required yet |
I reached out 1:1 to @WojtekBoman who implemented Expo Image, as I'm not 100% sure whether the exception is fixable within Expo Image or not. I can see some people suggesting that we disable native Android hardware acceleration -- but this worries me as this will likely reduce performance elsewhere... I'm thinking that we might need to enable it just for the React Views that render potentially lage images |
I'm going to generate a couple of test builds to verify the fix and compare performance. |
The crash is resolved... but at great cost. As expected, disabling hardware acceleration leads to horrendous performance. We can't make this an app-wide change. My plan is to disable hardware acceleration just for the specific View/Window that displays image attachments -- this should resolve the crash without harming app performance. |
Before: android-perf-before.mp4After: android-perf-after.mp4 |
To achieve this we could build our own module, or use this existing (but unknown) I'd much prefer to resolve this upstream within the Expo Image library though, so I'm going to ask SWM for help. |
This is being passed onto the Expo team, we'll hopefully hear back soon. |
Can't we just add a similar
(i just looked up this spot, might be a different line file/line) |
Either upstream or in a patch, since i'm not sure if this would be the ideal fix for all users. But it most likely should fix the problem for E/App |
That sounds good, but let's first see what Expo team says 👍 |
Waiting on response. |
Expo have released expo-image version This PR bumps the |
The fix seemed to have not resolved our issue. We're in contact with the Expo team again |
No update yet |
Hey @chrispader, just wondering if you've heard anything else from Expo, or whether this conversation is taking place on a public GH repo? |
Awaiting response from Expo and @chrispader |
Asked for an update here |
Fixed by #38671 |
Nice stuff! Sorry for not replying fast enough 🥲 I think this solution is valid, as long as |
Problem
The Android app crashes due to a Canvas OOM exception when the image dimensions are extremely large. This has been fixed in the past, but we recently switched to Expo Image so I believe we'll need a new solution.
We have spent time on different image libraries and many related issues, and I am now trying to combine our current issues and resolve the remaining canvas OOM exception.
Solution
Collect all past attempts, recent merged PRs, and determine the best path forward for resolving the crash:
react-native-fast-image
ImageZoom
Next question: Do we need a fix in Expo Image?
The text was updated successfully, but these errors were encountered: