[IOS][FIXED] Fix RCTImageBlurUtils.m Crash
Summary:
RCTIMageBlurUtils.m is causing a crash for some specific images. More context in this PR: https://github.com/facebook/react-native/pull/37508#issuecomment-1558964327
I was under the impression that the change in the previous PR actually resolved the issue, however some images were still crashing. It turns out that the issue actually lies within the new UIGraphicsImageRenderer not properly aligning the bitmap data.
For simplicity I opted to use the older UIGraphicsBeginImageContextWithOptions which now works reliably, however might be a bit slower as the API is older.
There could be a path to use the newer UIGraphicsImageRenderer, however as the new API does not provide a lot of customizability it would involve writing more code and dealing with additional complexity, particularly when dealing with memory management in order to manually ensure that the buffer is properly formatted and aligned.
Changelog:
[IOS] [FIXED] - Fix RCTImageBlurUtils.m Image Blurring Crash
Test Plan:
<ImageBackground
blurRadius={18}
source={{uri: 'https://i.scdn.co/image/ab67616d0000b2737663b2f75fe4d8fb2cac8c27'}}
/>
<ImageBackground
blurRadius={5}
source={{uri: 'https://i.scdn.co/image/ab67616d0000b273d5a219b270d74a266131df18'}}
/>
| Platform | Engine | Arch | Size (bytes) | Diff |
|---|---|---|---|---|
| android | hermes | arm64-v8a | 8,733,277 | -6,102 |
| android | hermes | armeabi-v7a | 8,044,464 | -5,809 |
| android | hermes | x86 | 9,223,388 | -6,180 |
| android | hermes | x86_64 | 9,075,373 | -6,023 |
| android | jsc | arm64-v8a | 9,298,078 | -4,645 |
| android | jsc | armeabi-v7a | 8,486,816 | -4,382 |
| android | jsc | x86 | 9,359,243 | -4,734 |
| android | jsc | x86_64 | 9,615,338 | -4,579 |
Base commit: 670fa8832751c9c1445fc8f46063f7898c90e273 Branch: main
I'm not sure this is the right direction. This code has not been changed in a while, so what's causing it to now cause crashes? iOS has introduced many other API's like UIVisualEffect or CIGaussianBlur which we should consider adopting instead.
however might be a bit slower as the API is older
Can we quantify this somehow? Could you get some timing measurements for a large image?
this fix works for me, the previous one still crashes with this image and blurRadius: 50
This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.
This issue is still present and we manually reverted to the old way using the old API to prevent the crashes...
This patch unfortunately does not solve the problem for us. We have a greyscale b/w image crashing on a device only. The only thing that worked was adding a red pixel to the image.
This patch unfortunately does not solve the problem for us. We have a greyscale b/w image crashing on a device only. The only thing that worked was adding a red pixel to the image.
I was mistaken. This DOES fix it for us :) thanks @OskarEichler
Problem is still present on RN 0.72.10 and this PR still fixes it, we should merge it :)
E.g. with this image and blurRadius={20}
The PR does work and we've been using it in production for the past year, however I think the RN team is concerned that it is using an older API. Ideally someone should fix it using the new API.
this fix works for me 👍🏻
This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.
This PR was closed because it has been stalled for 7 days with no activity.