Merge pull request #2953 from dreampiggy/behavior_handle_image_foramt_when_no_image_data

Add a better check to handle the cases when call `storeImage` without  imageData

SDWebImage/Core/SDImageCache.h

@@ -162,6 +162,7 @@ typedef NS_OPTIONS(NSUInteger, SDImageCacheOptions) {
  * @param key             The unique image cache key, usually it's image absolute URL
  * @param toDisk          Store the image to disk cache if YES. If NO, the completion block is called synchronously
  * @param completionBlock A block executed after the operation is finished
+ * @note If no image data is provided and encode to disk, we will try to detect the image format (using either `sd_imageFormat` or `SDAnimatedImage` protocol method) and animation status, to choose the best matched format, including GIF, JPEG or PNG.
  */
 - (void)storeImage:(nullable UIImage *)image
             forKey:(nullable NSString *)key
@@ -178,6 +179,7 @@ typedef NS_OPTIONS(NSUInteger, SDImageCacheOptions) {
  * @param key             The unique image cache key, usually it's image absolute URL
  * @param toDisk          Store the image to disk cache if YES. If NO, the completion block is called synchronously
  * @param completionBlock A block executed after the operation is finished
+ * @note If no image data is provided and encode to disk, we will try to detect the image format (using either `sd_imageFormat` or `SDAnimatedImage` protocol method) and animation status, to choose the best matched format, including GIF, JPEG or PNG.
  */
 - (void)storeImage:(nullable UIImage *)image
          imageData:(nullable NSData *)imageData

SDWebImage/Core/SDImageCache.m

@@ -187,14 +187,26 @@
         dispatch_async(self.ioQueue, ^{
             @autoreleasepool {
                 NSData *data = imageData;
+                if (!data && [image conformsToProtocol:@protocol(SDAnimatedImage)]) {
+                    // If image is custom animated image class, prefer its original animated data
+                    data = [((id<SDAnimatedImage>)image) animatedImageData];
+                }
                 if (!data && image) {
+                    // Check image's associated image format, may return .undefined
+                    SDImageFormat format = image.sd_imageFormat;
+                    if (format == SDImageFormatUndefined) {
+                        // If image is animated, use GIF (APNG may be better, but has bugs before macOS 10.14)
+                        if (image.sd_isAnimated) {
+                            format = SDImageFormatGIF;
+                        } else {
                             // If we do not have any data to detect image format, check whether it contains alpha channel to use PNG or JPEG format
-                    SDImageFormat format;
                             if ([SDImageCoderHelper CGImageContainsAlpha:image.CGImage]) {
                                 format = SDImageFormatPNG;
                             } else {
                                 format = SDImageFormatJPEG;
                             }
+                        }
+                    }
                     data = [[SDImageCodersManager sharedManager] encodedDataWithImage:image format:format options:nil];
                 }
                 [self _storeImageDataToDisk:data forKey:key];

Tests/Tests/SDImageCacheTests.m

@@ -391,6 +391,99 @@ static NSString *kTestImageKeyPNG = @"TestImageKey.png";
     [self waitForExpectationsWithCommonTimeout];
 }
 
+- (void)test42StoreCacheWithImageAndFormatWithoutImageData {
+    XCTestExpectation *expectation1 = [self expectationWithDescription:@"StoreImage UIImage without sd_imageFormat should use PNG for alpha channel"];
+    XCTestExpectation *expectation2 = [self expectationWithDescription:@"StoreImage UIImage without sd_imageFormat should use JPEG for non-alpha channel"];
+    XCTestExpectation *expectation3 = [self expectationWithDescription:@"StoreImage UIImage/UIAnimatedImage with sd_imageFormat should use that format"];
+    XCTestExpectation *expectation4 = [self expectationWithDescription:@"StoreImage SDAnimatedImage should use animatedImageData"];
+    XCTestExpectation *expectation5 = [self expectationWithDescription:@"StoreImage UIAnimatedImage without sd_imageFormat should use GIF"];
+    
+    NSString *kAnimatedImageKey1 = @"kAnimatedImageKey1";
+    NSString *kAnimatedImageKey2 = @"kAnimatedImageKey2";
+    NSString *kAnimatedImageKey3 = @"kAnimatedImageKey3";
+    NSString *kAnimatedImageKey4 = @"kAnimatedImageKey4";
+    NSString *kAnimatedImageKey5 = @"kAnimatedImageKey5";
+    
+    // Case 1: UIImage without `sd_imageFormat` should use PNG for alpha channel
+    NSData *pngData = [NSData dataWithContentsOfFile:[self testPNGPath]];
+    UIImage *pngImage = [UIImage sd_imageWithData:pngData];
+    expect(pngImage.sd_isAnimated).beFalsy();
+    expect(pngImage.sd_imageFormat).equal(SDImageFormatPNG);
+    // Remove sd_imageFormat
+    pngImage.sd_imageFormat = SDImageFormatUndefined;
+    // Check alpha channel
+    expect([SDImageCoderHelper CGImageContainsAlpha:pngImage.CGImage]).beTruthy();
+    
+    [SDImageCache.sharedImageCache storeImage:pngImage forKey:kAnimatedImageKey1 toDisk:YES completion:^{
+        UIImage *diskImage = [SDImageCache.sharedImageCache imageFromDiskCacheForKey:kAnimatedImageKey1];
+        // Should save to PNG
+        expect(diskImage.sd_isAnimated).beFalsy();
+        expect(diskImage.sd_imageFormat).equal(SDImageFormatPNG);
+        [expectation1 fulfill];
+    }];
+    
+    // Case 2: UIImage without `sd_imageFormat` should use JPEG for non-alpha channel
+    SDGraphicsImageRendererFormat *format = [SDGraphicsImageRendererFormat preferredFormat];
+    format.opaque = YES;
+    SDGraphicsImageRenderer *renderer = [[SDGraphicsImageRenderer alloc] initWithSize:pngImage.size format:format];
+    // Non-alpha image, also test `SDGraphicsImageRenderer` behavior here :)
+    UIImage *nonAlphaImage = [renderer imageWithActions:^(CGContextRef  _Nonnull context) {
+        [pngImage drawInRect:CGRectMake(0, 0, pngImage.size.width, pngImage.size.height)];
+    }];
+    expect(nonAlphaImage).notTo.beNil();
+    expect([SDImageCoderHelper CGImageContainsAlpha:nonAlphaImage.CGImage]).beFalsy();
+    
+    [SDImageCache.sharedImageCache storeImage:nonAlphaImage forKey:kAnimatedImageKey2 toDisk:YES completion:^{
+        UIImage *diskImage = [SDImageCache.sharedImageCache imageFromDiskCacheForKey:kAnimatedImageKey2];
+        // Should save to JPEG
+        expect(diskImage.sd_isAnimated).beFalsy();
+        expect(diskImage.sd_imageFormat).equal(SDImageFormatJPEG);
+        [expectation2 fulfill];
+    }];
+    
+    NSData *gifData = [NSData dataWithContentsOfFile:[self testGIFPath]];
+    UIImage *gifImage = [UIImage sd_imageWithData:gifData]; // UIAnimatedImage
+    expect(gifImage.sd_isAnimated).beTruthy();
+    expect(gifImage.sd_imageFormat).equal(SDImageFormatGIF);
+    
+    // Case 3: UIImage with `sd_imageFormat` should use that format
+    [SDImageCache.sharedImageCache storeImage:gifImage forKey:kAnimatedImageKey3 toDisk:YES completion:^{
+        UIImage *diskImage = [SDImageCache.sharedImageCache imageFromDiskCacheForKey:kAnimatedImageKey3];
+        // Should save to GIF
+        expect(diskImage.sd_isAnimated).beTruthy();
+        expect(diskImage.sd_imageFormat).equal(SDImageFormatGIF);
+        [expectation3 fulfill];
+    }];
+    
+    // Case 4: SDAnimatedImage should use `animatedImageData`
+    SDAnimatedImage *animatedImage = [SDAnimatedImage imageWithData:gifData];
+    expect(animatedImage.animatedImageData).notTo.beNil();
+    [SDImageCache.sharedImageCache storeImage:animatedImage forKey:kAnimatedImageKey4 toDisk:YES completion:^{
+        NSData *data = [SDImageCache.sharedImageCache diskImageDataForKey:kAnimatedImageKey4];
+        // Should save with animatedImageData
+        expect(data).equal(animatedImage.animatedImageData);
+        [expectation4 fulfill];
+    }];
+    
+    // Case 5: UIAnimatedImage without sd_imageFormat should use GIF not APNG
+    NSData *apngData = [NSData dataWithContentsOfFile:[self testAPNGPath]];
+    UIImage *apngImage = [UIImage sd_imageWithData:apngData];
+    expect(apngImage.sd_isAnimated).beTruthy();
+    expect(apngImage.sd_imageFormat).equal(SDImageFormatPNG);
+    // Remove sd_imageFormat
+    apngImage.sd_imageFormat = SDImageFormatUndefined;
+    
+    [SDImageCache.sharedImageCache storeImage:apngImage forKey:kAnimatedImageKey5 toDisk:YES completion:^{
+        UIImage *diskImage = [SDImageCache.sharedImageCache imageFromDiskCacheForKey:kAnimatedImageKey5];
+        // Should save to GIF
+        expect(diskImage.sd_isAnimated).beTruthy();
+        expect(diskImage.sd_imageFormat).equal(SDImageFormatGIF);
+        [expectation5 fulfill];
+    }];
+    
+    [self waitForExpectationsWithCommonTimeout];
+}
+
 #pragma mark - SDMemoryCache & SDDiskCache
 - (void)test42CustomMemoryCache {
     SDImageCacheConfig *config = [[SDImageCacheConfig alloc] init];
@@ -727,6 +820,15 @@ static NSString *kTestImageKeyPNG = @"TestImageKey.png";
     return reusableImage;
 }
 
+- (UIImage *)testAPNGImage {
+    static UIImage *reusableImage = nil;
+    if (!reusableImage) {
+        NSData *data = [NSData dataWithContentsOfFile:[self testAPNGPath]];
+        reusableImage = [UIImage sd_imageWithData:data];
+    }
+    return reusableImage;
+}
+
 - (NSString *)testJPEGPath {
     NSBundle *testBundle = [NSBundle bundleForClass:[self class]];
     return [testBundle pathForResource:@"TestImage" ofType:@"jpg"];
@@ -743,6 +845,12 @@ static NSString *kTestImageKeyPNG = @"TestImageKey.png";
     return testPath;
 }
 
+- (NSString *)testAPNGPath {
+    NSBundle *testBundle = [NSBundle bundleForClass:[self class]];
+    NSString *testPath = [testBundle pathForResource:@"TestImageAnimated" ofType:@"apng"];
+    return testPath;
+}
+
 - (nullable NSString *)userCacheDirectory {
     NSArray<NSString *> *paths = NSSearchPathForDirectoriesInDomains(NSCachesDirectory, NSUserDomainMask, YES);
     return paths.firstObject;

Tests/Tests/SDWebImageDownloaderTests.m

@@ -187,10 +187,13 @@
 
 - (void)test11ThatCancelAllDownloadWorks {
     XCTestExpectation *expectation = [self expectationWithDescription:@"CancelAllDownloads"];
+    // Previous test case download may not finished, so we just check the download count should + 1 after new request
+    NSUInteger currentDownloadCount = [SDWebImageDownloader sharedDownloader].currentDownloadCount;
     
-    NSURL *imageURL = [NSURL URLWithString:@"http://via.placeholder.com/1100x1100.png"];
+    // Choose a large image to avoid download too fast
+    NSURL *imageURL = [NSURL URLWithString:@"https://www.sample-videos.com/img/Sample-png-image-1mb.png"];
     [[SDWebImageDownloader sharedDownloader] downloadImageWithURL:imageURL completed:nil];
-    expect([SDWebImageDownloader sharedDownloader].currentDownloadCount).to.equal(1);
+    expect([SDWebImageDownloader sharedDownloader].currentDownloadCount).to.equal(currentDownloadCount + 1);
     
     [[SDWebImageDownloader sharedDownloader] cancelAllDownloads];