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

Images Cropped/Sized by Content Mode Animate Incorrectly #150

Open
bcapps opened this issue Jan 27, 2016 · 9 comments
Open

Images Cropped/Sized by Content Mode Animate Incorrectly #150

bcapps opened this issue Jan 27, 2016 · 9 comments
Labels

Comments

@bcapps
Copy link
Contributor

bcapps commented Jan 27, 2016

Images used in the transition animation do not reflect the cropped/resized state of the image within their image views. Instead, they show the image resized to the size of the containing image view, ignoring the aspect ratio and providing a very strange transition.

To test, switch out the image in the sample project with one that is much taller than it is wide. Set a content mode of aspect fill on the image view. Slow down the transition and see how the original image in the transition is now the full image, resized down to the containing image view size, with incorrect aspect ratios. It looks pretty funky.

@bcapps bcapps added the bug label Jan 27, 2016
@bcapps
Copy link
Contributor Author

bcapps commented Feb 1, 2016

Here's an action shot of the transition in 1.0.1 as an example :

simulator screen shot jan 26 2016 7 03 02 pm

@bcapps bcapps changed the title Images Cropped/Sized by Content Mode Now Animate Incorrectly Images Cropped/Sized by Content Mode Animate Incorrectly Feb 2, 2016
@cdzombak
Copy link
Contributor

cdzombak commented Feb 3, 2016

This isn't caused by #116; I just followed @bcapps' instructions (image taller than it is wide, and image view with content mode = aspect-fill) and reproduced consistent results on current develop (6cf8991), v1.0.0, and v0.1.2.

develop

transition-develop

1.0.0

transition-v1 0 0

0.1.2

transition-0 1 2

@bcapps, this is what you're seeing, right? (A gif is worth a thousand pictures.)

@bcapps
Copy link
Contributor Author

bcapps commented Feb 3, 2016

Yup!

@cdzombak
Copy link
Contributor

cdzombak commented Feb 3, 2016

I wonder what a "correct" animation looks like here.

@bcapps
Copy link
Contributor Author

bcapps commented Feb 3, 2016

I also narrowed it down to this line in the transition animator. It assumes that the heights of the images should be used to scale the initial transform, but for content modes that end in "Fill," it should be using the width. You'll notice that changing the height retrieval to CGRectGetWidth with the new image and fill mode provides a more desirable animation.

@bcapps
Copy link
Contributor Author

bcapps commented Feb 3, 2016

One potential solution that I haven't fully tested yet, then, would be to check the contentMode for both the ending and starting views for animation and if it's AspectFill or ScaleToFill, use the width instead.

@djbe
Copy link

djbe commented Apr 20, 2016

@bcapps I just tried your solution and it works for me. Example code:

    CGFloat endingViewInitialTransform = 1;
    switch (self.startingView.contentMode) {
        case UIViewContentModeScaleAspectFill:
        case UIViewContentModeScaleToFill:
            endingViewInitialTransform = CGRectGetWidth(startingViewForAnimation.frame) / CGRectGetWidth(endingViewForAnimation.frame);
            break;
        default:
            endingViewInitialTransform = CGRectGetHeight(startingViewForAnimation.frame) / CGRectGetHeight(endingViewForAnimation.frame);
    }

The thing is, it's still not perfect. Beyond the zoom and fade animations, there should also be a frame animation. It's hard to explain, but this is a good example:
https://github.com/recruit-mp/RMPZoomTransitionAnimator

As a suggestion, maybe switch to that library for the animation, or at least use it's animation code for inspiration?

@shinoys222
Copy link

@djbe This commit doesn't seem to fix the issue.

nytbugvideo 2

shinoys222 pushed a commit to shinoys222/NYTPhotoViewer that referenced this issue May 22, 2016
close image animation glitch fixed
@chlebta
Copy link

chlebta commented May 4, 2017

Having same issue, image are cropped!

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

No branches or pull requests

5 participants