Fix some regression when SDAnimatedImage created with static format like JPEG #3708
Conversation
Now `SDAnimatedImage` can be static image, so some check is not correct
This should still be compatible for custom image class like YYPlugin
dreampiggy
force-pushed
the
bugfix/sdanimatedimage_check
branch
from
fdf5610
to
019c541
Compare
| @@ -335,7 +335,7 @@ - (NSUInteger)sd_memoryCost { | |||
| @implementation SDAnimatedImage (Metadata) | |||
|
|
|||
| - (BOOL)sd_isAnimated { | |||
| return YES; | |||
| return self.animatedImageFrameCount > 1; | |||
There was a problem hiding this comment.
That https://github.com/SDWebImage/SDWebImageYYPlugin need to update with this behavior as well..
There was a problem hiding this comment.
Currently we have two kind of checking:
animatedImageFrameCount > 1sd_isAnimated == true
Actually, now these two kind of checking share the same implementation. So the force decode logic can still enabled for the JPEG SDAnimatedImage, so it does not blocking the main queue during rendering
| @@ -129,12 +129,12 @@ - (void)setImage:(UIImage *)image | |||
|
|
|||
| // We need call super method to keep function. This will impliedly call `setNeedsDisplay`. But we have no way to avoid this when using animated image. So we call `setNeedsDisplay` again at the end. | |||
| super.image = image; | |||
| if ([image.class conformsToProtocol:@protocol(SDAnimatedImage)]) { | |||
| if ([image.class conformsToProtocol:@protocol(SDAnimatedImage)] && [(id<SDAnimatedImage>)image animatedImageFrameCount] > 1) { | |||
There was a problem hiding this comment.
For some case, like a single frame GIF in SDAnimatedImage, this will cause behavior changes. But I think still worth.
Single frame animated image format (1 frame GIF) actually can always consider as Static image, right ?
| } | ||
| } | ||
| if (scaledImage) { | ||
| SDImageCopyAssociatedObject(image, scaledImage); |
There was a problem hiding this comment.
A bugfix during personal review of this API
| @@ -347,11 +347,21 @@ - (void)setSd_imageLoopCount:(NSUInteger)sd_imageLoopCount { | |||
| } | |||
|
|
|||
| - (NSUInteger)sd_imageFrameCount { | |||
| return self.animatedImageFrameCount; | |||
| NSUInteger frameCount = self.animatedImageFrameCount; | |||
There was a problem hiding this comment.
For non animated SDAnimatedImage, self.animatedImageFrameCount is nil, so the extra check is needed.
dreampiggy
force-pushed
the
bugfix/sdanimatedimage_check
branch
from
019c541
to
7536492
Compare
|
This class will rename into |
|
Verified that the scroll performance issue has been resolved. Looks good! |
New Pull Request Checklist
I have read and understood the CONTRIBUTING guide
I have read the Documentation
I have searched for a similar pull request in the project and found none
I have updated this branch with the latest master to avoid conflicts (via merge from master or rebase)
I have added the required tests to prove the fix/feature I am adding
I have updated the documentation (if necessary)
I have run the tests and they pass
I have run the lint and it passes (
pod lib lint)This merge request fixes / refers to the following issues: close #3706
Pull Request Description
For now, I still prefer to keep the idea that
SDAnimatedImagecan be created with non-static format like JPEG.Why ? Because even without the changes in #3626, you can still do hack using
NSCoderto create a JPEGSDAnimatedImageobject, before 5.19.0 version...馃槀 (See the line of code)And this, match the behavior like
subclassing(For the same input param, the parent classUIImagecan init successfully, but why a subclass return nil ?)And this matches the YYImage's behavior, it's more easy to keep sync of that https://github.com/SDWebImage/SDWebImageYYPlugin
CC @LeoTheCatMeow