facebook / fresco

An Android library for managing images and the memory they use.
https://frescolib.org/
MIT License
17.07k stars 3.75k forks source link

Low image quality using <Image/> component on RN > = 0.57 (Fresco >= 1.10.0) #2397

Closed clytras closed 6 months ago

clytras commented 5 years ago

Description

RN issue: RN 0.57.x Bundled large images have low quality when viewing using \<Image/> component with 1:1 AR on Android [CLOSED]

There is low quality when loading large bundled (PNG, GIF and maybe more formats, NOT JPEG) images only on Android:

At the left screenshot we see the exact same code running with RN 0.56.0 and at the right screenshot we see RN 0.57.1. The code is just a simple image <Image source={require('./assets/ELHall1.png')} resizeMethod="resize" /> and the image size is 2111 x 4645 pixels. Both projects are fresh installed using react-native init RN057ImageTest and react-native init --version="0.56.0" RN056ImageTest. This continues to happen from 0.56 and all versions after and latest RN 0.60.x.

This is confirmed to be caused by RN Fresco lib change between 0.56 and 0.57 from 1.9.0 to 1.10.0 https://github.com/facebook/react-native/commit/b6f2aad9c0119d11e52978ff3fa9c6f269f04a14. Check comment https://github.com/facebook/react-native/issues/21301#issuecomment-520155609.

After some search at Fresco issues, I can see some related issues that it's suggested that large images should be divided and recomposed piece by piece, which it resolves many cases (most map related large images) but it can be very inconvenient especially for dynamic loaded/created images. This was working until RN 0.56 and from 0.57 and after it does not.

Reproduction

RN: It's the initial App.js with an <Image/> component added.

...
type Props = {};
export default class App extends Component<Props> {
  render() {
    return (
      <View style={styles.container}>
        <Image source={require('./assets/ELHall1.png')} resizeMethod="resize" />
      </View>
    );
  }
}
...

Additional Information

At this comment https://github.com/facebook/react-native/issues/21301#issuecomment-520418832, lambdapioneer writes that this maybe is related to scale down (sub-sampling) large images:

I assume it's related to how Fresco scales downs (sub-sampling) large images (which is an important feature for memory and performance concerns). There has been some changes in these area during that time mainly for removing native code dependencies to help reducing the also pressing .so unsatisfied link errors. So to say: it might be a side-effect of another large improvement.

FRizzonelli commented 4 years ago

@clytras I understand :( Problem is right now I'm trying to setup a small POC with react-native-web on Expo, which has this issue for Android. And for a small demo, I'd like not to eject. But I think I need to :(

JeffGuKang commented 4 years ago

@clytras I understand :( Problem is right now I'm trying to setup a small POC with react-native-web on Expo, which has this issue for Android. And for a small demo, I'd like not to eject. But I think I need to :(

How about to use jpg format instead of png? I am not sure it can be solved the issue.

FRizzonelli commented 4 years ago

@clytras I understand :( Problem is right now I'm trying to setup a small POC with react-native-web on Expo, which has this issue for Android. And for a small demo, I'd like not to eject. But I think I need to :(

How about to use jpg format instead of png? I am not sure it can be solved the issue.

Unfortunately doesn't work either, and I need png for alpha layer :(

JJMoon commented 3 years ago

I have same problem with RN 0.63.4. The image size of 960 x 13983 is down sampled much, the texts in the image can hardly read. The image size of 680 X 2538 is also down sampled but quite decent. I used Image.getSize to find out the size of the image, and calculate height from it (width is 100%) Images are all jpg. I did @clytras 's method with gradle 3.5.4, NDK 21.4.7075529. But, same issue.

        <Image
          style={{ width: SCREEN_WIDTH, height }}
          resizeMode="contain"
          source={{ uri }}
          resizeMethod="scale"
        />

We solved the problem with FastImage. I hope it would help.

 <FastImage
          style={{ width: SCREEN_WIDTH, height: height }}
          source={{
            uri: uri,
            priority: FastImage.priority.normal,
          }}
          resizeMode={FastImage.resizeMode.center}
 />
trumdongnat commented 3 years ago

@JJMoon Do you know how to force FastImage re-render the image when I change image size in the style. The Image always re-render but FastImage doesn't.

rafaelmaeuer commented 3 years ago

Hello together,

I tried out all possible solutions from this thread, but with RN 0.64.2 and Fresco 2.5.0. I am using NDK v22.1.7171670 and gradle 4.2.1.

I am currently stuck at the following error (is the problem related to android-emulator?):

E/AndroidRuntime: FATAL EXCEPTION: mqt_native_modules
    Process: com.veerle.wiener.debug, PID: 10813
    java.lang.NoSuchMethodError: No static method initialize(Lcom/facebook/imagepipeline/core/ImagePipelineConfig;)V in class Lcom/facebook/imagepipeline/core/ImagePipelineFactory; or its super classes (declaration of 'com.facebook.imagepipeline.core.ImagePipelineFactory' appears in /data/app/<project>/base.apk!classes16.dex)
        at com.facebook.drawee.backends.pipeline.Fresco.initialize(Fresco.java:83)
        at com.facebook.drawee.backends.pipeline.Fresco.initialize(Fresco.java:45)
        at com.facebook.react.modules.fresco.FrescoModule.initialize(FrescoModule.java:114)
        at com.facebook.react.bridge.ModuleHolder.doInitialize(ModuleHolder.java:236)
        at com.facebook.react.bridge.ModuleHolder.markInitializable(ModuleHolder.java:100)
        at com.facebook.react.bridge.NativeModuleRegistry.notifyJSInstanceInitialized(NativeModuleRegistry.java:103)
        at com.facebook.react.bridge.CatalystInstanceImpl$2.run(CatalystInstanceImpl.java:438)
        at android.os.Handler.handleCallback(Handler.java:938)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(MessageQueueThreadHandler.java:27)
        at android.os.Looper.loop(Looper.java:223)
        at com.facebook.react.bridge.queue.MessageQueueThreadImpl$4.run(MessageQueueThreadImpl.java:226)
        at java.lang.Thread.run(Thread.java:923)
I/Process: Sending signal. PID: 10813 SIG: 9

@JJMoon using FastImage with local images (require) gave me a bad flickering experience.

trumdongnat commented 3 years ago

While waiting the team fix this issue, I am using both Image and FastImage like that if(height < blurSize){ return <Image/> }else{ return <FastImage/> }

rafaelmaeuer commented 3 years ago

I finally discarded the fresco-patching approach and fixed the problem using the FastImage lib too.

pateljoel commented 2 years ago

Unfortunately FastImage doesn't work and the issue still persists for me on the Image component, please take a look my Expo snack for reproducing this issue.

https://snack.expo.dev/@joelpateljp/low-quality-image-android-example

pateljoel commented 2 years ago

Also is there a timeline for when this patch will be merged?

ramkumar2098 commented 2 years ago

I ended up using webview as it displays images as is.

<WebView source={{ html:`<img src=${image_url} style="width: 100%;" />`}} />

It works for my usecase.

Mfweb commented 1 year ago

any updates on this?

anandpandey2414 commented 1 year ago

Any update on this guys. or any simple solution Fastimage is not working for me. Don't wanna use patch is there anything else we can do? Please suggest for the same

timoisalive commented 1 year ago

Hey, is the allowDownscaling prop of the expo-image supposed to be an answer to this issue? From the description, that's what it seems, but I wasn't able to notice any difference in the image quality setting it true or false. 🤔

https://docs.expo.dev/versions/unversioned/sdk/image/#allowdownscaling

pateljoel commented 1 year ago

Is there any update on this? And also has anyone managed to solve this issue or have a solution? I'm not sure if the latest fresco 3.0.0 solves this issue either.

sunnylqm commented 1 year ago

need someone from fresco team to review this pr https://github.com/facebook/fresco/pull/2500 but unfortunately for some reason they rarely review prs from the community

pateljoel commented 1 year ago

@sunnylqm Any luck on contacting other maintainers to review this? It looks like it would / should only take a few hours to review and merge but instead we're 'waiting for godot' here.

sunnylqm commented 1 year ago

maybe they just muted this repo. maybe you should try to find their personal sns accounts to dm.

clytras commented 1 year ago

The suprising thing to me is that the issue is still opened. I mean, if they didn't consider this to be a bug, they would have closed the issue stating it's expected behavior, if they would recognize that this is a bug, they would have merged the fix PR. It's kind of strange.

pateljoel commented 11 months ago

@clytras I mean @oprisnik knows about this issue, he has commented on this thread and his last activity on this was on Oct 28, 2019, I don't know about the other maintainers (I believe there are about 3 or 10 of them) but surely they should have looked into even merging the PR if they have access.

@sunnylqm I'm still pinging the PR but I don't think there is any email to contact any of them.

sunnylqm commented 6 months ago

we need to wait for a new fresco release and i'll do the necessary pr on RN side.

clytras commented 6 months ago

A big thank you to @sunnylqm for making downsample configurable. I'm looking forward for a new version and of course the RN implementation.

Thank you @sunnylqm ❤️

onetrace-abdullah commented 3 months ago

@sunnylqm Looks like Fresco released v3.2.0 recently, and the version bump has been merged into React Native https://github.com/facebook/react-native/commit/744024be7f95970bdd786931bdf10dd50c515124

sunnylqm commented 3 months ago

updated the rn config, but do not have time to build and test.

https://github.com/facebook/react-native/pull/45078