fix issues: SDWebImageDecodeFirstFrameOnly flag is ignored when image loaded from cache

[cntrump]

… loaded from cache.

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: ...

Pull Request Description

first time, load from internet, it is a thumbnail imageview.

 [imageView sd_setImageWithURL:[NSURL URLWithString:url]
                         placeholderImage:nil
                                  options:SDWebImageDecodeFirstFrameOnly];

second time, load the same image from loader, it is a detail imageview.

 [imageView sd_setImageWithURL:[NSURL URLWithString:url]
                         placeholderImage:nil
                                  options:SDWebImageFromLoaderOnly];

and when thumbnail imageview load the image using SDWebImageDecodeFirstFrameOnly on next time, SDWebImage will give it an animated image, it should be a static image.

[codecov]

Codecov Report

Merging #2725 into master will decrease coverage by 0.18%.
The diff coverage is 25%.

@@            Coverage Diff             @@
##           master    #2725      +/-   ##
==========================================
- Coverage   83.94%   83.75%   -0.19%     
==========================================
  Files          56       56              
  Lines        6221     6225       +4     
==========================================
- Hits         5222     5214       -8     
- Misses        999     1011      +12

Flag	Coverage Δ
#ios	84.72% <50%> (-0.18%)	⬇️
#macos	82.78% <33.33%> (-0.26%)	⬇️

Impacted Files	Coverage Δ
SDWebImage/SDImageCache.m	80.17% <25%> (-0.5%)	⬇️
SDWebImage/SDAnimatedImageView.m	77.39% <0%> (-2.46%)	⬇️
SDWebImage/SDWebImagePrefetcher.m	88.6% <0%> (-0.64%)	⬇️
SDWebImage/SDWebImageManager.m	86.47% <0%> (+0.81%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e9eb60e...bc2b520. Read the comment docs.

[dreampiggy]

The issue I undertood. But the PR seems need some polishing.

For UIImage, you can just grab images property to get the first object.
For NSImage, you can just grab CGImage to get the first object. No need to use the imageWithSize:flipped:drawingHandler:

So, the fix it not hard. And there are no need to create that category method. (I think this will misleading some user).

if ((options & SDImageCacheDecodeFirstFrameOnly) && image.sd_isAnimated) {
#if SD_UIKIT || SD_WATCH
image = image.images.firstObject;
#else
image = [NSImage initWithCGImage:image.CGImage scale:image.scale orientation:kCGImagePropertyOrientationUp];
#endif
}

[dreampiggy]

Actually, this is a interesting problem. Assume the case:

// ListView.m
SDAnimatedImageView *listImageView = [SDAnimatedImageView new];
// List page, load a GIF and play animation
[listImageView sd_setImageWithURL:gifURL];
// Now, the `SDAnimatedImage` for gifURL is cached in memory

// DetailView.m
SDAnimatedImageView *detailImageView = [SDAnimatedImageView new];
[detailImageView sd_setImageWithURL:gifURL options:SDWebImageDecodeFirstFrameOnly]; // Same GIF URL, but this time I specify first frame only
// Should hit the memory cache, so now comes the problem

Should we still populate the original SDAnimatedImage for detailImageView ? Since it use SDWebImageDecodeFirstFrameOnly

Or need we just create a empty first frame image for this ?

[dreampiggy]

Maybe we should also fix the same issue for SDWebImageYYPlugin 😅

[dreampiggy]

@cntrump Hi. Any update ?

This fix should be picked in v5.0.3 release. If you have time, please check the review status for better fix.

[dreampiggy]

LGTM. It's better to provide a test case for this. If you have time, you can add one, or I can do this for you.

[cntrump]

LGTM. It's better to provide a test case for this. If you have time, you can add one, or I can do this for you.

emmm... I don't know how to do this, so would you mind helping me add one ?

[dreampiggy]

@cntrump Done via 8adfe3f. If you find some issue in the future, you can try to write test cases as well.

[cntrump]

@cntrump Done via 8adfe3f. If you find some issue in the future, you can try to write test cases as well.

😃OK, thanks !