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

Shared Element Transition crashes if image is not yet loaded #7078

Closed
mrousavy opened this issue Apr 9, 2021 · 1 comment · Fixed by #7079
Closed

Shared Element Transition crashes if image is not yet loaded #7078

mrousavy opened this issue Apr 9, 2021 · 1 comment · Fixed by #7079

Comments

@mrousavy
Copy link
Collaborator

mrousavy commented Apr 9, 2021

🐛 Bug Report

When running a SET from an image (<FastImage>) to another where the other one has not yet been loaded, the app crashes because the SET code cannot handle NaN as a layer Rect.

Screenshot 2021-04-09 at 12 27 58

Full Exception message:

Thread 1: "CALayer bounds contains NaN: [0 0; nan nan]. Layer: <CALayer:0x2822a2560; position = CGPoint (64.25 80.25); bounds = CGRect (0 0; 128.5 160.5); delegate = <FFFastImageView: 0x138c9da90; baseClass = UIImageView; frame = (0 0; 128.5 160.5); clipsToBounds = YES; opaque = NO; autoresize = W+H; layer = <CALayer: 0x2822a2560>>; contents = <CGImage 0x150b36420> (DP)\n\t<<CGColorSpace 0x2803bcf00> (kCGColorSpaceICCBased; kCGColorSpaceModelRGB; sRGB IEC61966-2.1)>\n\t\twidth = 536, height = 1000, bpc = 8, bpp = 32, row bytes = 2144 \n\t\tkCGImageAlphaNoneSkipFirst | kCGImageByteOrder32Little  | kCGImagePixelFormatPacked \n\t\tis mask? No, has masking color? No, has soft mask? No, has matte? No, should interpolate? Yes; masksToBounds = YES; allowsGroupOpacity = YES; backgroundColor = <CGColor 0x280d874e0> [<CGColorSpace 0x2803b1bc0> (kCGColorSpaceICCBased; kCGColorSpaceModelRGB; sRGB IEC61966-2.1; extended range)] ( 0 0 0 0 ); contentsMultiplyColor = (null)>"

This is a regression of e184102 since it worked before.

Have you read the Contributing Guidelines on issues?

yea

To Reproduce

  1. Create 2 screens with <FastImage> components which have a remote URI source
  2. Animate transition (showModal) using SET
  3. Open a few times. It doesn't happen always as it depends on the image being loaded.

Expected behavior

It shouldn't crash. As for the animation itself, we should fall back to transitioning to the FFFastImageView's layer bounds since we don't have the underlying UIImage's size to calculate resize modes. This technically breaks "different resizeMode transitions", but at least it doesn't crash.

Additionally, we could also add a listener to the FastImage to get the actual .image's dimensions, and update the currently ongoing transition's to value accordingly. This adds a lot of complexity, but then the image would always animate correctly.

Actual Behavior

Crash

Your Environment

  • React Native Navigation version: 7.13.0-snapshot.1512
  • React Native version: 0.64
  • Platform(s) (iOS, Android, or both?): iOS
  • Device info (Simulator/Device? OS version? Debug/Release?): everywhere

Reproducible Demo

I think you can reproduce this in the playground if you use remote URLs instead of the local images - I'll try

@mrousavy
Copy link
Collaborator Author

mrousavy commented Apr 9, 2021

Oh I have another idea - why don't we just use the 'from' Image's .image (UIImage) to calculate resizeModes/aspect ratios? This makes a lot more sense and doesn't introduce race conditions between the image loading and the view starting to animate. wdyt @yogevbd ?

yogevbd pushed a commit that referenced this issue Apr 11, 2021
Fixes #7078.

@yogevbd was there a specific reason to only account for the Image Scale in `UIViewContentModeScaleAspectFit`? All other contentModes just used the image width/height without dividing by it's scale. I changed it so that now every contentMode respects the scale.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants