Fix the bug when FLAnimatedImageView firstly show one EXIF rotation JPEG `UIImage`, later animated GIF `FLAnimatedImage` will also be rotated

[dreampiggy]

Related Issue

The related issue in FLAnimatedImage repo, it's https://github.com/Flipboard/FLAnimatedImage/issues/181

The related issue in SDWebImage repo, it's https://github.com/rs/SDWebImage/issues/2402 https://github.com/rs/SDWebImage/issues/1317

Reproduce Demo

https://github.com/ericeddy/FLAnimatedImage-RotationIssue

What cause this problem

FLAnimatedImageView, it's a UIImageView subclass, which manually specify CALayer.contents with the current rendering frame's CGImage. And they use a CADisplayLink as a timer, to refresh current frame, finally produce the visual animation.

However, CALayer itself, contains a private property, called contentsTransform. You can check the private header of CALayer. CALayer will calculate current UIImage.imageOrientation and translate to CGAffineTransform to render the bitmap. Why they to do this it's because it does not actually take rotation on CGImage's bitmap when you create UIImage with -[UIImage imageWithCGImage:scale:orientation:], The both scale and imageOrientation are just hint to the CALayer's property contentsScale and contentsTransform. So that CALayer use the contentsTransform to actually do rotation about the bitmap.

Which means, actually, CALayer draw its contents, based on the both public CALayer.transform and private CALayer.contentsTransform two properties.

See the screenshot during View Debug and print out the CALayer information. The actually CALayer.contents bitmap show the correct orientation, but the rendering result cause a rotation because the CALayer contains a contentsTransform = CGAffineTransform (0 1.06667; -0.9375 0; 375 0), which is a right-90 degress rotation.

After I use KVO to observe this contentsTransform changes, I found that the only write on this property was happend on -[UIImageView setImage:] function call with a non-nil UIImage. The non-nil is really important, when you sepcify nil, the contentsTransform property leave there without reset to identity transform. (Because UIKit assume you don't touch layer.contents and so they don't need to reset the state)

So now, you may know the reason now. Correct, the previous JPEG image which contains non-Up image orienation will change the conrtentsTransform, but the newly set FLAnimatedImage does not clear that transform, cause the final rotation. I check the current FLAnimatedImage code and finally find the bug. The root case is that -[FLAnimatedImage setAnimatedImage:] does not do the correct thing, to reset the super UIImageView state about image.

- (void)setAnimatedImage:(FLAnimatedImage *)animatedImage
{
    if (![_animatedImage isEqual:animatedImage]) {
        if (animatedImage) {
            // Clear out the image.
            super.image = nil;   // -> This will not reset `contentsTransform`, not actually "Clear out the image."
            // Ensure disabled highlighting; it's not supported (see `-setHighlighted:`).
            super.highlighted = NO;
           //...
    }
}

Solution

The naive solution, it's quite simple, firstlly call super.image = non-nil image and then call super.image = nil. Only one line code and the issue disappear.

--- a/FLAnimatedImage/FLAnimatedImageView.m
+++ b/FLAnimatedImage/FLAnimatedImageView.m
@@ -101,6 +101,7 @@
 {
     if (![_animatedImage isEqual:animatedImage]) {
         if (animatedImage) {
+            // Reset `contentsTransform` state
+            super.image = animatedImage.posterImage;
             // Clear out the image.
             super.image = nil;
             // Ensure disabled highlighting; it's not supported (see `-setHighlighted:`).

From my personal opinion, I think they should always call [super setImage:] during [FLAnimatedImageView setAnimatedImage:]. If you don't call super, you may loss internal consistent behavior during subclass. But anyway, this fix is much simpler.

[matrush]

The idea is similar to what I have in https://github.com/Flipboard/FLAnimatedImage/pull/174 :) Nice writeup!

[StephenQin]

great!!!

[matrush]

@dreampiggy with #174 I think we don't need to merge this for now? If so I'm going to close this.

[dreampiggy]

@dreampiggy with #174 I think we don't need to merge this for now? If so I'm going to close this.

This can solve 99% use case, but will fail when some case that UIImage is not CGImage based.

[dreampiggy]

🎉