Skip to content
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

Large image causes images to not load #39

Closed
psyanite opened this issue Jan 1, 2018 · 29 comments
Closed

Large image causes images to not load #39

psyanite opened this issue Jan 1, 2018 · 29 comments

Comments

@psyanite
Copy link
Contributor

psyanite commented Jan 1, 2018

Just wanted to say this repo is AWESOME!

I experienced some issues rendering approximately 10 of these 4K images from https://imgur.com/gallery/8Yxub. For example I would swipe back and forth and images would disappear then re-appear, some pages was completely blank, some pages would say image cannot be loaded.

Could be a memory issue rather than a dimension issue?

Currently I don't need to render 4K images so that's good!

Many Thanks

@Exilz
Copy link
Contributor

Exilz commented Jan 2, 2018

Hi, thanks for the kind words.

Are you experiencing this on iOS or Android ? I don't doubt Android will have memory problems with such images, but I don't know how iOS would behave.

@psyanite
Copy link
Contributor Author

psyanite commented Jan 2, 2018

Thanks for the reply!

I just re-tried and ensured that debug JS mode is off.
If the first image fails to load, the first page ends up being black.
Some images do load and display correctly.
Most images do not load and thus the Image cannot be displayed... is displayed.

Is it because it should wait for all the images to be fetched before displaying the Gallery?

Code:

    const images = [
      { source: { uri: 'https://i.imgur.com/LpHPPwv.jpg' } },
      { source: { uri: 'https://i.imgur.com/PkMfiGI.jpg' } },
      { source: { uri: 'https://i.imgur.com/WafiSvc.jpg' } },
      { source: { uri: 'https://i.imgur.com/7CDO3RM.jpg' } },
      { source: { uri: 'https://i.imgur.com/SGCmh12.jpg' } },
      { source: { uri: 'https://i.imgur.com/EdKhHD9.jpg' } },
      { source: { uri: 'https://i.imgur.com/HQ9dCRk.jpg' } },
      { source: { uri: 'https://i.imgur.com/8cMhX6m.jpg' } },
      { source: { uri: 'https://i.imgur.com/mCvboVY.jpg' } },
      { source: { uri: 'https://i.imgur.com/MT1ZXG1.jpg' } },
      { source: { uri: 'https://i.imgur.com/WJiTF5Y.jpg' } },
      { source: { uri: 'https://i.imgur.com/AXAHCh2.jpg' } },
      { source: { uri: 'https://i.imgur.com/kj33W9j.jpg' } },
    ]
     <Gallery
        style={{ flex: 1, backgroundColor: 'black' }}
        images={images}
      />

Screenshots:
26233908_1516632391790441_1373092835_n
26235378_1516632031790477_762719199_n
26241455_1516632335123780_532507276_n

@Exilz
Copy link
Contributor

Exilz commented Jan 2, 2018

This error is displayed when Image.getSize failed. It's done here.

It would be nice to dig more into this to know exactly why this is failing.

Maybe preloading your images before displaying the gallery would fix this, but this isn't a solution. If it works that way, it may explain the error with Image.getSize.

@psyanite
Copy link
Contributor Author

psyanite commented Jan 9, 2018

It could possibly be happening as Imgur doesn't like having so many requests, if problem persists with a different image host, I'll re-open the issue. Thanks!

@psyanite psyanite closed this as completed Jan 9, 2018
@guptayush
Copy link

I am too having this same issue in Android with images captured from my One Plus 5 mobile camera.
Maybe the images captured are of high quality

@msageryd
Copy link
Contributor

Without having looked into this I'll put an idea on the table:

On iPhone, when images takes too much memory they are simply not rendered. I think that Gallery loads three images (previous, current, next). This might deplete the memory if each one of the images is very large.

I don't work with Android (for now), but I've heard that there is even less available memory on Android.

@guptayush Would it be possible for you to verify if Gallery is working if you scale down the images first?

@guptayush
Copy link

@msageryd after scaling down images Gallery is working absolutely fine

@msageryd
Copy link
Contributor

It's quite expensive to have three fullres images loaded. I think that the correct solution would be to use lowres-images when swiping between pages. There would then be two lowres and one fullres image loaded at any same time.

I don't know how much work that would be to fix though..

@guptayush
Copy link

@msageryd if i am setting windowSize to 1 then also the first image doesn't load. after swipping it loads

@psyanite psyanite changed the title 4K Image Issues! Awesome! Large image causes images to not load Jan 12, 2018
@psyanite psyanite reopened this Jan 12, 2018
@msageryd
Copy link
Contributor

I'm guessing this is a Flatlist issue/misuse. windowSize is sent all the way through to the Flatlist, but I can't see how it's used by Flatlist https://facebook.github.io/react-native/docs/flatlist.html. I still haven't migrated my other stuff from ListView, so I don't know much about FlatList.

I still think that the right solution would be to use lower res "swipe-images". Even if we get the "windowSize=1" solution to work you'd not be able to see the next image in a nice transition when swiping. Actually, I think the loading of fullres image should be postponed until the user tries to zoom more than the lowres image allows, hence (#38).

May I ask what resolution your images has? I'm trying to dial in an image size for floor plan blueprints that can be handled reliably one at a time (i.e. no gallery swiping). For now I'm using 3500x2500 on iPhone 6, 7, 8 and X. I want to use as large images as possible without having to write a scrolling-tiles algorithm.

@psyanite
Copy link
Contributor Author

Bugs out when I do this on Android:

 const images = [
      { source: { uri: 'https://i.imgur.com/LpHPPwv.jpg' } },
      { source: { uri: 'https://i.imgur.com/PkMfiGI.jpg' } },
      { source: { uri: 'https://i.imgur.com/WafiSvc.jpg' } },
      { source: { uri: 'https://i.imgur.com/7CDO3RM.jpg' } },
      { source: { uri: 'https://i.imgur.com/SGCmh12.jpg' } },
      { source: { uri: 'https://i.imgur.com/EdKhHD9.jpg' } },
      { source: { uri: 'https://i.imgur.com/HQ9dCRk.jpg' } },
      { source: { uri: 'https://i.imgur.com/8cMhX6m.jpg' } },
      { source: { uri: 'https://i.imgur.com/mCvboVY.jpg' } },
      { source: { uri: 'https://i.imgur.com/MT1ZXG1.jpg' } },
      { source: { uri: 'https://i.imgur.com/WJiTF5Y.jpg' } },
      { source: { uri: 'https://i.imgur.com/AXAHCh2.jpg' } },
      { source: { uri: 'https://i.imgur.com/kj33W9j.jpg' } },
    ]
     <Gallery
        style={{ flex: 1, backgroundColor: 'black' }}
        images={images}
      />

I'd guess that one at a time should have no issues.

@guptayush
Copy link

@msageryd the images resolution I'm trying is 4608*2592

@msageryd
Copy link
Contributor

Those images seems to be about the same size as the ones I'm handling (the first image is 3840x2160). An iPhone can handle a couple of those loaded at the same time, but it's a bit sketchy since you have to be aware of everything else you do at the same time so you don't deplete the memory. I'd definitely say that those images should be handled one by one. I've also heard that Android is even more memory constrained than iPhone (I don't work with Android).

@msageryd
Copy link
Contributor

msageryd commented Jan 12, 2018

4608*2592 is about 12Mpixels. That's about 36MB at 24bit/px. I fall short of understanding why modern multi-GB-memory phones cannot handle this easier. I only have empirical testing to back this up with, but these tests tells me that multiple images at this size is a no-go on any device.

@guptayush
Copy link

@msageryd Can we try something to wait for Image.getSize so that it loads correctly as @Exilz mentioned error is displayed when Image.getSize failed

@msageryd
Copy link
Contributor

This is a long shot again (and I don't have time to test right now).

  • What happens if you input the size manually in the images array?
  • What happens if you scale down the size proportionally (i.e. the numbers you enter) to half the size

ViewTransformer use transform to pan and zoom the image. Transform is said to be more performant than other prop changes. Maybe some optimization is performed at the native level so the memory footprint wont be as large if you say that the resolution is lower than it actually is? I'm totally guessing here..

@msageryd
Copy link
Contributor

msageryd commented Jan 12, 2018

One more shot.. I suppose you can reproduce the error reliably? I would be great to know the actual error message (is it a timeout?). TransformableImage doesn't read the actual error message when Image.getSize fails.

Could you try to get hold of the message by temporary altering the call to Image.getSize in TransformableImage/index.getImageSize() link as per below (i.e. just add the error param and console log it):

Image.getSize(
  source.uri,
  (width, height) => {
    if (width && height) {
      if (
        this.state.imageDimensions &&
        this.state.imageDimensions.width === width &&
        this.state.imageDimensions.height === height
      ) {
        // no need to update state
      } else {
        this._mounted && this.setState({ imageDimensions: { width, height } });
      }
    }
  },
  (error) => {
    console.log(error)
    this._mounted && this.setState({ error: true });
  }
);

@msageryd
Copy link
Contributor

msageryd commented Jan 12, 2018

This thread has become a bit hard to follow (my fault). I have mixed my answers for two separate issues. Maybe I should clear some up:

  • getSize problem -> more info needed
  • problem with large local images -> scale down or load fewer

(p.s. I'm not a maintainer. Just a user who is very happy that someone made this awesome lib. Trying to contribute..)

@guptayush
Copy link

Error: Pool hard cap violation? Hard cap = 201326592 Used size = 163927868 Free size = 0 Request size = 63700992
at createErrorFromErrorData (NativeModules.js:123)
at NativeModules.js:80
at MessageQueue.__invokeCallback (MessageQueue.js:307)
at MessageQueue.js:120
at MessageQueue.__guard (MessageQueue.js:231)
at MessageQueue.invokeCallbackAndReturnFlushedQueue (MessageQueue.js:119)
at debuggerWorker.js:72

This is the error thrown

@msageryd
Copy link
Contributor

msageryd commented Jan 12, 2018

Great info.

Found this: facebook/react-native#10569

Seems to be a problem only on Android.
Possible solutions:

  • android: largeHeap="true"
  • for ScrollView, removeClippedSubviews={true}
  • People say that Flatlist should handle this (i.e. letting images unmount correctly), but there might be something in image-gallery implementation that causes the images to hang around indefinitely (guessing)

I haven't read this, but people refer to it from the above thread: https://zeemee.engineering/react-native-on-android-lessons-learned-99fee8f1d390

@guptayush
Copy link

@msageryd largeHeap="true" worked for me

@msageryd
Copy link
Contributor

msageryd commented Jan 12, 2018

Great!
I suppose that it's still advicable to downsize the images.

@Exilz might want to put this solution/workaround in the readme so other Androiders won't have to come all the way this lengthy thread for the solution.

@Exilz
Copy link
Contributor

Exilz commented Jan 12, 2018

Although largeHeap="true" works for those out of memory issues, I strongly recommend you not to use it as you will make your user experience significantly worse.

There's something going on with the garbage collection of the system that makes it slower, hence an app starting much slowly. When you're resuming your app it might also restart completely and reload your bundle.

It's an easy fix but I'm warning people reading this issue not to use it unless you're pretty much forced to.

We should focus on improving memory usage with large images in the gallery, but it's a pretty huge work. 😄

@msageryd
Copy link
Contributor

@Exilz My fork is somewhat a mess right now (because I'm not a git wiz). I'd like to start over fresh and take a stab on the "load highres image when user starts to zoom" feature. This would help a lot of the memory problems, albeit not the underlaying GC problem, but still..

If you could accept my Rect-fix PR so I can make a new PR out of my dependency bundling fix, I would be on track with your repo and I could make the new zoom-PR from a clean slate.

@Exilz
Copy link
Contributor

Exilz commented Jan 12, 2018

@msageryd I'll test it out this afternoon and merge it if it's okay. Thanks a ton for your help.

@psyanite
Copy link
Contributor Author

psyanite commented Jan 12, 2018

Wouldn't the best solution be that when the gallery pops up, you only fetch the current image, the previous image, and the next image, but onSwipe you would load the next ones, if they're not loaded? Isn't this how most galleries are implemented? I guess for now for my massive galleries I can just make it 1 image galleries.

😍 Oh my, it says I'm a contributor, I only updated the README though, so misleading.

@bahaeddine4
Copy link

I had an issue with images not appearing randomly , and it shows instead
" this image cannot be displayed ", I added dimensions key to the array this way :

const images = [
{
source: { uri: 'https://i.imgur.com/LpHPPwv.jpg' },
dimensions: {width: value, height: value}
},
{
source: { uri: 'https://i.imgur.com/LpHPPwv.jpg' },
dimensions: {width: value, height: value}
},
...
]
<Gallery
style={{ flex: 1, backgroundColor: 'black' }}
images={images}
/>

@vbuch
Copy link
Contributor

vbuch commented Jan 25, 2018

@psyanite that's how it works in 2.1.4 (see #21)
In 2.1.5 getItemLayout is not being used causing FlatList to be practically needles. The main advantage of FlatList is the optimizations it has. You cpould try using 2.1.4 and check if it works there with large images. Also you could use flatListProps and control the windowSize prop

@msageryd @Exilz rendering really high res images really needs some understanding of how they are rendered and when they are store in memory. What gives the best control here would be the resizeMethod prop of the Image component.

@psyanite psyanite closed this as completed Feb 4, 2018
@psyanite
Copy link
Contributor Author

psyanite commented Feb 4, 2018

@vbuch Thanks for the in-depth explanation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants