Browse Source

Merge pull request #1604 from onevcat/fix/kfimage-state-management

Use `@State` for KFImage state management
Wei Wang 5 years ago
parent
commit
9dfc06dac5

+ 1 - 2
Demo/Demo/Kingfisher-Demo/SwiftUIViews/SwiftUIList.swift

@@ -75,12 +75,11 @@ struct SwiftUIList : View {
                         }
                         .foregroundColor(.gray)
                     }
+                    .fade(duration: 1)
                     .cancelOnDisappear(true)
                     .aspectRatio(contentMode: .fit)
                     .cornerRadius(20)
                     .frame(width: 300, height: 300)
-                    .opacity(done || alreadyCached ? 1.0 : 0.3)
-                    .animation(.linear(duration: 0.4))
 
                 Spacer()
             }.padding(.vertical, 12)

+ 9 - 0
Demo/Demo/Kingfisher-Demo/SwiftUIViews/SwiftUIView.swift

@@ -32,6 +32,8 @@ struct SwiftUIView : View {
 
     @State private var index = 1
 
+    @State private var blackWhite = false
+
     var url: URL {
         URL(string: "https://raw.githubusercontent.com/onevcat/Kingfisher-TestImages/master/DemoAppImage/Loading/kingfisher-\(self.index).jpg")!
     }
@@ -39,6 +41,8 @@ struct SwiftUIView : View {
     var body: some View {
         VStack {
             KFImage(url)
+                .cacheOriginalImage()
+                .setProcessor(blackWhite ? BlackWhiteProcessor() : DefaultImageProcessor())
                 .onSuccess { r in
                     print("suc: \(r)")
                 }
@@ -49,6 +53,8 @@ struct SwiftUIView : View {
                     Image(systemName: "arrow.2.circlepath.circle")
                         .font(.largeTitle)
                 }
+                .fade(duration: 1)
+                .forceTransition()
                 .resizable()
                 .frame(width: 300, height: 300)
                 .cornerRadius(20)
@@ -57,6 +63,9 @@ struct SwiftUIView : View {
             Button(action: {
                 self.index = (self.index % 10) + 1
             }) { Text("Next Image") }
+            Button(action: {
+                self.blackWhite.toggle()
+            }) { Text("Black & White") }
 
         }.navigationBarTitle(Text("Basic Image"), displayMode: .inline)
     }

+ 4 - 1
Sources/Extensions/ImageView+Kingfisher.swift

@@ -383,7 +383,10 @@ extension KingfisherWrapper where Base: KFCrossPlatformImageView {
         switch options.transition {
         case .none:
             return false
-        #if !os(macOS)
+        #if os(macOS)
+        case .fade: // Fade is only a placeholder for SwiftUI on macOS.
+            return false
+        #else
         default:
             if options.forceTransition { return true }
             if cacheType == .none { return true }

+ 26 - 0
Sources/General/ImageSource/Source.swift

@@ -80,6 +80,32 @@ public enum Source {
     }
 }
 
+extension Source: Hashable {
+    public static func == (lhs: Source, rhs: Source) -> Bool {
+        switch (lhs, rhs) {
+        case (.network(let r1), .network(let r2)):
+            return r1.cacheKey == r2.cacheKey && r1.downloadURL == r2.downloadURL
+        case (.provider(let p1), .provider(let p2)):
+            return p1.cacheKey == p2.cacheKey && p1.contentURL == p2.contentURL
+        case (.provider(_), .network(_)):
+            return false
+        case (.network(_), .provider(_)):
+            return false
+        }
+    }
+
+    public func hash(into hasher: inout Hasher) {
+        switch self {
+        case .network(let r):
+            hasher.combine(r.cacheKey)
+            hasher.combine(r.downloadURL)
+        case .provider(let p):
+            hasher.combine(p.cacheKey)
+            hasher.combine(p.contentURL)
+        }
+    }
+}
+
 extension Source {
     var asResource: Resource? {
         guard case .network(let resource) = self else {

+ 0 - 8
Sources/General/KF.swift

@@ -333,14 +333,6 @@ extension KF.Builder {
     }
     #endif
 
-    /// Sets whether the image setting for an image view should happen with transition even when retrieved from cache.
-    /// - Parameter enabled: Enable the force transition or not.
-    /// - Returns: A `KF.Builder` with changes applied.
-    public func forceTransition(_ enabled: Bool = true) -> Self {
-        options.forceTransition = enabled
-        return self
-    }
-
     /// Sets whether keeping the existing image of image view while setting another image to it.
     /// - Parameter enabled: Whether the existing image should be kept.
     /// - Returns: A `KF.Builder` with changes applied.

+ 15 - 6
Sources/General/KFOptionsSetter.swift

@@ -45,15 +45,15 @@ extension KF.Builder: KFOptionSetter {
 @available(iOS 13.0, OSX 10.15, tvOS 13.0, watchOS 6.0, *)
 extension KFImage: KFOptionSetter {
     public var options: KingfisherParsedOptionsInfo {
-        get { binder.options }
-        nonmutating set { binder.options = newValue }
+        get { context.binder.options }
+        nonmutating set { context.binder.options = newValue }
     }
 
-    public var onFailureDelegate: Delegate<KingfisherError, Void> { binder.onFailureDelegate }
-    public var onSuccessDelegate: Delegate<RetrieveImageResult, Void> { binder.onSuccessDelegate }
-    public var onProgressDelegate: Delegate<(Int64, Int64), Void> { binder.onProgressDelegate }
+    public var onFailureDelegate: Delegate<KingfisherError, Void> { context.binder.onFailureDelegate }
+    public var onSuccessDelegate: Delegate<RetrieveImageResult, Void> { context.binder.onSuccessDelegate }
+    public var onProgressDelegate: Delegate<(Int64, Int64), Void> { context.binder.onProgressDelegate }
 
-    public var delegateObserver: AnyObject { binder }
+    public var delegateObserver: AnyObject { context.binder }
 }
 #endif
 
@@ -339,6 +339,15 @@ extension KFOptionSetter {
         options.lowDataModeSource = source
         return self
     }
+
+    /// Sets whether the image setting for an image view should happen with transition even when retrieved from cache.
+    /// - Parameter enabled: Enable the force transition or not.
+    /// - Returns: A `KF.Builder` with changes applied.
+    public func forceTransition(_ enabled: Bool = true) -> Self {
+        options.forceTransition = enabled
+        return self
+    }
+
 }
 
 // MARK: - Request Modifier

+ 3 - 0
Sources/Image/ImageTransition.swift

@@ -24,6 +24,7 @@
 //  OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 //  THE SOFTWARE.
 
+import Foundation
 #if os(iOS) || os(tvOS)
 import UIKit
 
@@ -111,5 +112,7 @@ public enum ImageTransition {
 // Just a placeholder for compiling on macOS.
 public enum ImageTransition {
     case none
+    /// This is a placeholder on macOS now. It is for SwiftUI (KFImage) to identify the fade option only.
+    case fade(TimeInterval)
 }
 #endif

+ 23 - 7
Sources/SwiftUI/ImageBinder.swift

@@ -33,7 +33,7 @@ extension KFImage {
 
     /// Represents a binder for `KFImage`. It takes responsibility as an `ObjectBinding` and performs
     /// image downloading and progress reporting based on `KingfisherManager`.
-    class ImageBinder: ObservableObject {
+    class ImageBinder {
 
         let source: Source?
         var options = KingfisherParsedOptionsInfo(KingfisherManager.shared.defaultOptions)
@@ -48,8 +48,6 @@ extension KFImage {
 
         var isLoaded: Binding<Bool>
 
-        @Published var image: KFCrossPlatformImage?
-
         @available(*, deprecated, message: "The `options` version is deprecated And will be removed soon.")
         init(source: Source?, options: KingfisherOptionsInfo? = nil, isLoaded: Binding<Bool>) {
             self.source = source
@@ -61,7 +59,6 @@ extension KFImage {
                 [.loadDiskFileSynchronously]
             )
             self.isLoaded = isLoaded
-            self.image = nil
         }
 
         init(source: Source?, isLoaded: Binding<Bool>) {
@@ -73,10 +70,9 @@ extension KFImage {
                 [.loadDiskFileSynchronously]
             )
             self.isLoaded = isLoaded
-            self.image = nil
         }
 
-        func start() {
+        func start(_ done: @escaping (Result<RetrieveImageResult, KingfisherError>) -> Void) {
 
             guard !loadingOrSucceeded else { return }
 
@@ -103,21 +99,29 @@ extension KFImage {
                         self.downloadTask = nil
                         switch result {
                         case .success(let value):
+
                             // The normalized version of image is used to solve #1395
                             // It should be not necessary if SwiftUI.Image can handle resizing correctly when created
                             // by `Image.init(uiImage:)`. (The orientation information should be already contained in
                             // a `UIImage`)
                             // https://github.com/onevcat/Kingfisher/issues/1395
                             let image = value.image.kf.normalized
+                            let r = RetrieveImageResult(
+                                image: image, cacheType: value.cacheType, source: value.source, originalSource: value.originalSource
+                            )
                             CallbackQueue.mainCurrentOrAsync.execute {
-                                self.image = image
+                                done(.success(r))
                             }
+
                             CallbackQueue.mainAsync.execute {
                                 self.isLoaded.wrappedValue = true
                                 self.onSuccessDelegate.call(value)
                             }
                         case .failure(let error):
                             self.loadingOrSucceeded = false
+                            CallbackQueue.mainCurrentOrAsync.execute {
+                                done(.failure(error))
+                            }
                             CallbackQueue.mainAsync.execute {
                                 self.onFailureDelegate.call(error)
                             }
@@ -131,4 +135,16 @@ extension KFImage {
         }
     }
 }
+
+@available(iOS 13.0, OSX 10.15, tvOS 13.0, watchOS 6.0, *)
+extension KFImage.ImageBinder: Hashable {
+    static func == (lhs: KFImage.ImageBinder, rhs: KFImage.ImageBinder) -> Bool {
+        return lhs === rhs
+    }
+
+    func hash(into hasher: inout Hasher) {
+        hasher.combine(source)
+        hasher.combine(options.processor.identifier)
+    }
+}
 #endif

+ 118 - 53
Sources/SwiftUI/KFImage.swift

@@ -40,23 +40,10 @@ extension Image {
     }
 }
 
-/// A Kingfisher compatible SwiftUI `View` to load an image from a `Source`.
-/// Declaring a `KFImage` in a `View`'s body to trigger loading from the given `Source`.
 @available(iOS 13.0, OSX 10.15, tvOS 13.0, watchOS 6.0, *)
 public struct KFImage: View {
 
-    // TODO: Replace `@ObservedObject` with `@StateObject` once we do not need to support iOS 13.
-    /// An image binder that manages loading and cancelling image related task.
-    @ObservedObject private(set) var binder: ImageBinder
-
-    // Acts as a placeholder when loading an image.
-    var placeholder: AnyView?
-
-    // Whether the download task should be cancelled when the view disappears.
-    var cancelOnDisappear: Bool = false
-
-    // Configurations should be performed on the image.
-    var configurations: [(Image) -> Image]
+    var context: Context
 
     /// Creates a Kingfisher compatible image view to load image from the given `Source`.
     /// - Parameter source: The image `Source` defining where to load the target image.
@@ -70,10 +57,8 @@ public struct KFImage: View {
     ///               for more.
     @available(*, deprecated, message: "Some options are not available in SwiftUI yet. Use `KFImage(source:isLoaded:)` to create a `KFImage` and configure the options through modifier instead.")
     public init(source: Source?, options: KingfisherOptionsInfo? = nil, isLoaded: Binding<Bool> = .constant(false)) {
-        let binder = ImageBinder(source: source, options: options, isLoaded: isLoaded)
-        self.binder = binder
-        configurations = []
-        binder.start()
+        let binder = KFImage.ImageBinder(source: source, options: options, isLoaded: isLoaded)
+        self.init(binder: binder)
     }
 
     /// Creates a Kingfisher compatible image view to load image from the given `URL`.
@@ -87,7 +72,7 @@ public struct KFImage: View {
     ///               `KFImage` and configure the options through modifier instead. See methods of `KFOptionSetter`
     ///               for more.
     @available(*, deprecated, message: "Some options are not available in SwiftUI yet. Use `KFImage(_:isLoaded:)` to create a `KFImage` and configure the options through modifier instead.")
-    public init(_ url: URL?, options: KingfisherOptionsInfo? = nil, isLoaded: Binding<Bool> = .constant(false)) {
+    init(_ url: URL?, options: KingfisherOptionsInfo? = nil, isLoaded: Binding<Bool> = .constant(false)) {
         self.init(source: url?.convertToSource(), options: options, isLoaded: isLoaded)
     }
 
@@ -99,10 +84,7 @@ public struct KFImage: View {
     ///               wrapped value from outside.
     public init(source: Source?, isLoaded: Binding<Bool> = .constant(false)) {
         let binder = ImageBinder(source: source, isLoaded: isLoaded)
-        self.binder = binder
-        configurations = []
-        // Give the `binder` a chance to accept other options.
-        DispatchQueue.main.async { binder.start() }
+        self.init(binder: binder)
     }
 
     /// Creates a Kingfisher compatible image view to load image from the given `URL`.
@@ -115,40 +97,123 @@ public struct KFImage: View {
         self.init(source: url?.convertToSource(), isLoaded: isLoaded)
     }
 
-    /// Declares the content and behavior of this view.
+    init(binder: ImageBinder) {
+        self.context = Context(binder: binder)
+    }
+
     public var body: some View {
-        Group {
-            if binder.image != nil {
-                configurations
-                    .reduce(Image(crossPlatformImage: binder.image!)) {
-                        current, config in config(current)
-                    }
-            } else {
-                Group {
-                    if placeholder != nil {
-                        placeholder
-                    } else {
-                        Image(crossPlatformImage: .init())
-                    }
+        KFImageRenderer(context)
+            .id(context.binder)
+    }
+}
+
+@available(iOS 13.0, OSX 10.15, tvOS 13.0, watchOS 6.0, *)
+extension KFImage {
+    struct Context {
+        var binder: ImageBinder
+        var configurations: [(Image) -> Image] = []
+        var cancelOnDisappear: Bool = false
+        var placeholder: AnyView? = nil
+
+        init(binder: ImageBinder) {
+            self.binder = binder
+        }
+    }
+}
+
+/// A Kingfisher compatible SwiftUI `View` to load an image from a `Source`.
+/// Declaring a `KFImage` in a `View`'s body to trigger loading from the given `Source`.
+@available(iOS 13.0, OSX 10.15, tvOS 13.0, watchOS 6.0, *)
+struct KFImageRenderer: View {
+
+    /// An image binder that manages loading and cancelling image related task.
+    private let binder: KFImage.ImageBinder
+
+    @State private var loadingResult: Result<RetrieveImageResult, KingfisherError>?
+    @State private var isLoaded = false
+
+    // Acts as a placeholder when loading an image.
+    var placeholder: AnyView?
+
+    // Whether the download task should be cancelled when the view disappears.
+    let cancelOnDisappear: Bool
+
+    // Configurations should be performed on the image.
+    let configurations: [(Image) -> Image]
+
+    init(_ context: KFImage.Context) {
+        self.binder = context.binder
+        self.configurations = context.configurations
+        self.placeholder = context.placeholder
+        self.cancelOnDisappear = context.cancelOnDisappear
+    }
+
+    /// Declares the content and behavior of this view.
+    var body: some View {
+        if case .success(let r) = loadingResult {
+            configurations
+                .reduce(Image(crossPlatformImage: r.image)) {
+                    current, config in config(current)
                 }
-                .frame(minWidth: 0, maxWidth: .infinity, minHeight: 0, maxHeight: .infinity)
-                .onAppear { [weak binder = self.binder] in
-                    guard let binder = binder else {
-                        return
-                    }
-                    if !binder.loadingOrSucceeded {
-                        binder.start()
-                    }
+                .opacity(isLoaded ? 1.0 : 0.0)
+        } else {
+            Group {
+                if placeholder != nil {
+                    placeholder
+                } else {
+                    Color.clear
                 }
-                .onDisappear { [weak binder = self.binder] in
-                    guard let binder = binder else {
-                        return
-                    }
-                    if self.cancelOnDisappear {
-                        binder.cancel()
+            }
+            .onAppear { [weak binder = self.binder] in
+                guard let binder = binder else {
+                    return
+                }
+                if !binder.loadingOrSucceeded {
+                    binder.start { result in
+                        self.loadingResult = result
+                        switch result {
+                        case .success(let value):
+                            CallbackQueue.mainAsync.execute {
+                                let animation = fadeTransitionDuration(cacheType: value.cacheType)
+                                    .map { duration in Animation.linear(duration: duration) }
+                                withAnimation(animation) { isLoaded = true }
+                            }
+                        case .failure(_):
+                            break
+                        }
                     }
                 }
             }
+            .onDisappear { [weak binder = self.binder] in
+                guard let binder = binder else {
+                    return
+                }
+                if self.cancelOnDisappear {
+                    binder.cancel()
+                }
+            }
+        }
+    }
+
+    private func shouldApplyFade(cacheType: CacheType) -> Bool {
+        binder.options.forceTransition || cacheType == .none
+    }
+
+    private func fadeTransitionDuration(cacheType: CacheType) -> TimeInterval? {
+        shouldApplyFade(cacheType: cacheType)
+            ? binder.options.transition.fadeDuration
+            : nil
+    }
+}
+
+extension ImageTransition {
+    // Only for fade effect in SwiftUI.
+    fileprivate var fadeDuration: TimeInterval? {
+        switch self {
+        case .fade(let duration):
+            return duration
+        default:
+            return nil
         }
     }
 }
@@ -162,7 +227,7 @@ extension KFImage {
     /// - Returns: A `KFImage` view that configures internal `Image` with `block`.
     public func configure(_ block: @escaping (Image) -> Image) -> KFImage {
         var result = self
-        result.configurations.append(block)
+        result.context.configurations.append(block)
         return result
     }
 
@@ -191,7 +256,7 @@ extension KFImage {
 struct KFImage_Previews : PreviewProvider {
     static var previews: some View {
         Group {
-            KFImage(URL(string: "https://raw.githubusercontent.com/onevcat/Kingfisher/master/images/logo.png")!)
+            KFImage(source: .network(URL(string: "https://raw.githubusercontent.com/onevcat/Kingfisher/master/images/logo.png")!))
                 .onSuccess { r in
                     print(r)
                 }

+ 15 - 2
Sources/SwiftUI/KFImageOptions.swift

@@ -113,7 +113,7 @@ extension KFImage {
     public func placeholder<Content: View>(@ViewBuilder _ content: () -> Content) -> KFImage {
         let v = content()
         var result = self
-        result.placeholder = AnyView(v)
+        result.context.placeholder = AnyView(v)
         return result
     }
 
@@ -122,8 +122,21 @@ extension KFImage {
     /// - Returns: A `KFImage` view that cancels downloading task when disappears.
     public func cancelOnDisappear(_ flag: Bool) -> KFImage {
         var result = self
-        result.cancelOnDisappear = flag
+        result.context.cancelOnDisappear = flag
         return result
     }
+
+    /// Sets a fade transition for the image task.
+    /// - Parameter duration: The duration of the fade transition.
+    /// - Returns: A `KFImage` with changes applied.
+    ///
+    /// Kingfisher will use the fade transition to animate the image in if it is downloaded from web.
+    /// The transition will not happen when the
+    /// image is retrieved from either memory or disk cache by default. If you need to do the transition even when
+    /// the image being retrieved from cache, also call `forceRefresh()` on the returned `KFImage`.
+    public func fade(duration: TimeInterval) -> KFImage {
+        context.binder.options.transition = .fade(duration)
+        return self
+    }
 }
 #endif