[iOS]:- fixed image resize mode in release mode for iOS

[Biki-das]

Summary:

On iOS, when attempting to use a local image with a different resizeMode than the default one in a release build, it doesn't work as expected if the image is a local asset. The problem lies in how the image is loaded and processed.
In a debug build, the image loads correctly with the desired resizeMode. This is because the method RCTDecodeImageWithData is employed, which handles the resizeMode properly.

debug.mp4

In contrast, in a release build, the image does not load with the desired resizeMode. This is due to the use of RCTImageFromLocalAssetURL, which is designed specifically to load an image by URL and does not handle resizeMode.

To address this issue, a new method called RCTDecodeImageWithLocalAssetURL has been introduced. This method replicates the resizeMode handling strategy of RCTDecodeImageWithData but loads the image by URL instead of by data.
To ensure backwards compatibility and handle edge cases where the image cannot be loaded using the new method, a fallback to RCTImageFromLocalAssetURL is retained.

Following has been done to fix the same:-

1. Created RCTDecodeImageWithLocalAssetURL to handle loading images by URL with proper resizeMode.
2. Retained fallback mechanism to use RCTImageFromLocalAssetURL when necessary.

Changelog:

[IOS] [FIXED] - fixed resize mode in release mode for iOS

Test Plan:

tried to implement the changes in a react native scaffold by changing the node_module files and it worked perfectly

Release.mp4

cc @NickGerleman @cipolleschi @javache @blakef

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	19,741,485	-754,632
android	hermes	armeabi-v7a	n/a	--
android	hermes	x86	n/a	--
android	hermes	x86_64	n/a	--
android	jsc	arm64-v8a	22,946,151	-747,330
android	jsc	armeabi-v7a	n/a	--
android	jsc	x86	n/a	--
android	jsc	x86_64	n/a	--

Base commit: 4a8f0ee
Branch: main

[Biki-das]

@cipolleschi 😅 can you review this?

[cipolleschi]

nit, but it looks good!

[facebook-github-bot]

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[Biki-das]

@cipolleschi 😅 are the fb internal linter fail related to my change since it is only accessible to meta employees!

[Biki-das]

@cipolleschi friendly ping 😀
Is this good to go? Or there are some changes to be made?

[cipolleschi]

Is it good. I had to lint it with the internal linter and there were some unrelated tests failing. I rebased it as well, and I hope we can land it before EOW.
I appreciate your patience, we are a bit busy preparing the next release.

[Biki-das]

thanks @cipolleschi , for helping me out with this PR! hoping to make react native more awesome 😊

[cipolleschi]

Some tests were failing internally. I'm fixing them and I hope to get this approved internally soon, so I can land it.
Sorry for the delay.

[Biki-das]

hey @cipolleschi its definitely okay and i understand appreciate the work you are doing 😊.

[cipolleschi]

Sorry for the late rieview. A concern has been risen internally.
I added the comment in the code.

[cipolleschi]

Sorry for the late reviewev. A concern has been raised internallt that this implementation might bypass [UIImage imageNamed:] which also works as a cache and it is more efficient for small images like icons.

A suggestion is to use decodeImageFromCGImageSourceRef only if the resizeMode is different from the default.

So, the logic would be (pseudocode here):

if (resizeMode is the default) {
  return RCTImageFromLocalAssetURL(url);
}

//rest of the current method

what do you think?

[Biki-das]

Friendly ping @cipolleschi :-)

[facebook-github-bot]

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[cipolleschi]

Hi @Biki-das! Sorry for this to taking too long, but we are not convinced by the fix. We are a bit scared that the there would be perf implications and this difference between Debug and Release is a bit weird.

The right approach here is to understand why image decoding would fail, and avoid going into the CGImageSourceCreateWithURL path in the first place, as it might be more expensive.

It could also be that the issue is not in the image loading code but in the sequence that is used to set the resizeMode.

If it is not too much of an ask, would you prepare a simple repro, like the one you are using in the videos, starting from this template?

[Biki-das]

The reason image decoding fails is because in debug only mode it's using the method RCTDecodeImageWithData which is handling the resize mode properly. But in release build it is using RCTImageFromLocalAssetURL which is only made to load an image by url, so the resize mode is just never used. so the difference that you are implying is bound to occur as this is the way the code was implemented when it was written.

the point that i understand as of now is the CGImageSourceCreateWithURL method which seems to be the issue?
am i right

the image decoding does not fail as far as i understand, the resize mode just gets ignored in case of release build while using the RCTImageFromLocalAssetURL

well further ok let me try to create a reproduce repo

cc @cipolleschi

[Biki-das]

just tinkering and have a added a CFRelease method call to ensure the sourceRef is release one thing i am trying to understand is there a lot of performance diff between calling CGImageSourceCreateWithUrl and CGImageSourceCreateWithData?.

would like to know more as i don't have much idea of the same

[Biki-das]

https://github.com/facebook/react-native/blob/main/packages/react-native/Libraries/Image/RCTBundleAssetImageLoader.mm#L52

this is the method being used currently to handle the image decoding for the release mode. no resize mode aspects are taken here

[cipolleschi]

I think that the suggestion revolves more around: how can we make RCTImageFromLocalAssetURL honor the resizeMode?

The root difference, IIUC, is that:

• in debug mode, the asset is provided by Metro as a URL. So we need to decode the url and use the RCTDecodeImageWithData
• in release mode, there is no Metro. When building, we package the JS and all the assets in a file and we add the file to the bundle. So we can use the RCTImageFromLocalAssetURL to read the assets from the bundle directly. This should save performances as we don't need some of the decoding put in place by RCTDecodeImageWithData and we can leverage the internal cache of UIImage.

The proper solution would be not to decode everything from scratch, but to extend RCTImageFromLocalAssetURL so that it accept and respect the resizeMode. Or, alternatively, apply the resize mode after RCTImageFromLocalAssetURL returns, if possible.

What do you think? Does it make more sense?

[Biki-das]

Thanks @cipolleschi for the explanation yeah now it is clear to me what is expected let me take a look at the same during the weekend to come up with something

[thepdatoi]

Hello,
I am having this exact same issue on version 0.74.3. Is there any solution to this? Or if I downgrade RN will I still get this error?

[cipolleschi]

@thepdatoi sorry to hear that you are having the same error and yes, even if you downgrade, you'll still have the same issue, sadly.