Merge pull request #2454 from onevcat/fix/2453-defaultcacheserializer-gif

Fix GIF disk cache losing animation when original data is missing

Sources/Cache/CacheSerializer.swift

@@ -76,6 +76,13 @@ public extension CacheSerializer {
 /// When converting an `image` to the data, it will only be converted to the corresponding data type when `original`
 /// contains valid PNG, JPEG, and GIF format data. If the `original` is provided but not valid, or if `original` is
 /// `nil`, the input `image` will be encoded as PNG data.
+///
+/// If `original` is `nil` but the input `image` contains embedded GIF data (for example, a cached animated image
+/// created from GIF data), the serializer will prefer the embedded GIF data and store it as GIF instead of falling
+/// back to PNG.
+///
+/// > Tip: If you create a new image instance from an animated image in a custom processor, use
+/// > ``KingfisherWrapper/copyKingfisherState(to:)`` to propagate the embedded animated data to the new image.
 public struct DefaultCacheSerializer: CacheSerializer {
     
     /// The default general cache serializer utilized throughout Kingfisher's caching mechanism.
@@ -102,18 +109,21 @@ public struct DefaultCacheSerializer: CacheSerializer {
     public init() { }
 
     public func data(with image: KFCrossPlatformImage, original: Data?) -> Data? {
+        let format: ImageFormat = {
+            if let original = original { return original.kf.imageFormat }
+
+            if let animatedData = image.kf.gifRepresentation(), animatedData.kf.imageFormat == .GIF {
+                return .GIF
+            }
+            return .unknown
+        }()
+
         if preferCacheOriginalData {
-            return original ??
-                image.kf.data(
-                    format: original?.kf.imageFormat ?? .unknown,
-                    compressionQuality: compressionQuality
-                )
-        } else {
-            return image.kf.data(
-                format: original?.kf.imageFormat ?? .unknown,
-                compressionQuality: compressionQuality
-            )
+            if let original = original { return original }
+            if format == .GIF { return image.kf.gifRepresentation() }
         }
+
+        return image.kf.data(format: format, compressionQuality: compressionQuality)
     }
     
     public func image(with data: Data, options: KingfisherParsedOptionsInfo) -> KFCrossPlatformImage? {

Sources/Documentation.docc/CommonTasks/CommonTasks_Serializer.md

@@ -15,6 +15,26 @@ imageView.kf.setImage(with: url, options: [.cacheSerializer(DefaultCacheSerializ
 ``DefaultCacheSerializer`` is responsible for converting cached data into a corresponding image object and vice versa. 
 It supports PNG, JPEG, and GIF formats by default.
 
+When storing an image to disk, if the `original` data is available (for example, when the image is just downloaded),
+``DefaultCacheSerializer`` uses it to determine the format.
+If `original` is `nil` (for example, when the image is retrieved from cache), Kingfisher will try to infer the format:
+if the image still carries embedded GIF data, it will be stored as GIF; otherwise it falls back to encoding as PNG.
+
+### Notes for animated images and custom processors
+
+For animated images, Kingfisher keeps the original GIF bytes as an internal associated object on the image instance.
+If a custom ``ImageProcessor`` creates and returns a new `UIImage`/`NSImage` instance from an animated image, this
+internal animated data is not automatically carried over, and the disk cache may fall back to encoding as PNG (first
+frame only).
+
+To avoid this:
+
+- For animated inputs, return the input image directly when possible.
+- If you need to create a new image instance in the `.image` branch, copy Kingfisher internal states to the new image
+  by calling ``KingfisherWrapper/copyKingfisherState(to:)``.
+- If your goal is to avoid re-encoding, consider caching original data by using a serializer with
+  ``CacheSerializer/originalDataUsed`` enabled (e.g. configure ``DefaultCacheSerializer/preferCacheOriginalData``).
+
 ### Enforce a format
 
 To enforce a specific image format, use ``FormatIndicatedCacheSerializer``, which offers serializers for all supported

Sources/Image/Image.swift

@@ -137,6 +137,27 @@ extension KingfisherWrapper where Base: KFCrossPlatformImage {
         set { setRetainedAssociatedObject(base, imageSourceKey, newValue) }
     }
 
+    /// Copies Kingfisher internal image states from `base` to a `target` image.
+    ///
+    /// This includes the embedded animated image data and related metadata that are used by Kingfisher for caching and
+    /// animated image rendering. It is useful when a custom processor creates and returns a new image instance from
+    /// an animated image in `.image` branch.
+    ///
+    /// - Important: This method does not make the `target` image animated by itself. It only propagates Kingfisher's
+    ///   internal metadata so the cache can preserve the original animated bytes when possible.
+    ///
+    /// - Parameter target: The target image to which the internal states will be copied.
+    public func copyKingfisherState(to target: KFCrossPlatformImage) {
+        target.kf.animatedImageData = animatedImageData
+        target.kf.imageFrameCount = imageFrameCount
+        target.kf.frameSource = frameSource
+        target.kf.imageCreatingOptions = imageCreatingOptions
+        #if os(macOS)
+        target.kf.images = images
+        target.kf.duration = duration
+        #endif
+    }
+
     // Bitmap memory cost with bytes.
     var cost: Int {
         let pixel = Int(size.width * size.height * scale * scale)

Tests/KingfisherTests/ImageCacheTests.swift

@@ -201,6 +201,80 @@ class ImageCacheTests: XCTestCase {
         XCTAssertNotNil(result.image)
         XCTAssertEqual(result.cacheType, .memory)
     }
+
+    func testStoreGIFToDiskWithNilOriginalShouldPreserveGIFFormat() {
+        struct TestProcessor: ImageProcessor {
+            let identifier: String = "com.onevcat.KingfisherTests.TestProcessor"
+            func process(item: ImageProcessItem, options: KingfisherParsedOptionsInfo) -> KFCrossPlatformImage? {
+                switch item {
+                case .image(let image): return image
+                case .data(let data): return DefaultImageProcessor.default.process(item: .data(data), options: options)
+                }
+            }
+        }
+
+        let exp = expectation(description: #function)
+        let image = KingfisherWrapper<KFCrossPlatformImage>.animatedImage(data: testImageGIFData, options: .init())!
+        XCTAssertEqual(image.kf.gifRepresentation()?.kf.imageFormat, .GIF)
+
+        let options = KingfisherParsedOptionsInfo([.processor(TestProcessor())])
+        let key = "test-gif"
+        cache.store(image, original: nil, forKey: key, options: options, toDisk: true) { _ in
+            do {
+                let storedKey = key.computedKey(with: TestProcessor().identifier)
+                let storedData = try self.cache.diskStorage.value(forKey: storedKey)
+                XCTAssertEqual(storedData?.kf.imageFormat, .GIF)
+            } catch {
+                XCTFail("Unexpected error: \(error)")
+            }
+            exp.fulfill()
+        }
+
+        waitForExpectations(timeout: 3, handler: nil)
+    }
+
+    func testCopyKingfisherStateShouldKeepEmbeddedGIFDataForDiskCache() {
+        struct TestProcessor: ImageProcessor {
+            let identifier: String = "com.onevcat.KingfisherTests.TestProcessor.CopyState"
+            func process(item: ImageProcessItem, options: KingfisherParsedOptionsInfo) -> KFCrossPlatformImage? {
+                switch item {
+                case .image(let image):
+                    #if os(macOS)
+                    guard let cgImage = image.kf.cgImage else { return image }
+                    let newImage = KFCrossPlatformImage(cgImage: cgImage, size: image.kf.size)
+                    image.kf.copyKingfisherState(to: newImage)
+                    return newImage
+                    #else
+                    guard let cgImage = image.cgImage else { return image }
+                    let newImage = KFCrossPlatformImage(cgImage: cgImage, scale: image.scale, orientation: image.imageOrientation)
+                    image.kf.copyKingfisherState(to: newImage)
+                    return newImage
+                    #endif
+                case .data(let data):
+                    return DefaultImageProcessor.default.process(item: .data(data), options: options)
+                }
+            }
+        }
+
+        let exp = expectation(description: #function)
+        let image = KingfisherWrapper<KFCrossPlatformImage>.animatedImage(data: testImageGIFData, options: .init())!
+        XCTAssertEqual(image.kf.gifRepresentation()?.kf.imageFormat, .GIF)
+
+        let options = KingfisherParsedOptionsInfo([.processor(TestProcessor())])
+        let key = "test-gif-copy-state"
+        cache.store(image, original: nil, forKey: key, options: options, toDisk: true) { _ in
+            do {
+                let storedKey = key.computedKey(with: TestProcessor().identifier)
+                let storedData = try self.cache.diskStorage.value(forKey: storedKey)
+                XCTAssertEqual(storedData?.kf.imageFormat, .GIF)
+            } catch {
+                XCTFail("Unexpected error: \(error)")
+            }
+            exp.fulfill()
+        }
+
+        waitForExpectations(timeout: 3, handler: nil)
+    }
     
     func testStoreMultipleImages() {
         let exp = expectation(description: #function)