Merge pull request #162 from SDWebImage/bugfix_lazyvstack_recursive_update_freeze

Try to fix the recusive updateView when using AnimatedImage inside `ScrollView/LazyVStack`.

Author: dreampiggy

SDWebImageSwiftUI/Classes/AnimatedImage.swift

@@ -43,6 +43,11 @@ final class AnimatedLoadingModel : ObservableObject, IndicatorReportable {
     @Published var image: PlatformImage? // loaded image, note when progressive loading, this will published multiple times with different partial image
     @Published var isLoading: Bool = false // whether network is loading or cache is querying, should only be used for indicator binding
     @Published var progress: Double = 0 // network progress, should only be used for indicator binding
+    
+    /// Used for loading status recording to avoid recursive `updateView`. There are 3 types of loading (Name/Data/URL)
+    @Published var imageName: String?
+    @Published var imageData: Data?
+    @Published var imageURL: URL?
 }
 
 /// Completion Handler Binding Object, supports dynamic @State changes
@@ -201,12 +206,27 @@ public struct AnimatedImage : PlatformViewRepresentable {
     }
     #endif
 
-    func loadImage(_ view: AnimatedImageViewWrapper, context: Context) {
-        let operationKey = NSStringFromClass(type(of: view.wrapped))
-        let currentOperation = view.wrapped.sd_imageLoadOperation(forKey: operationKey)
-        if currentOperation != nil {
-            return
+    func setupIndicator(_ view: AnimatedImageViewWrapper, context: Context) {
+        view.wrapped.sd_imageIndicator = imageConfiguration.indicator
+        view.wrapped.sd_imageTransition = imageConfiguration.transition
+        if let placeholderView = imageConfiguration.placeholderView {
+            placeholderView.removeFromSuperview()
+            placeholderView.isHidden = true
+            // Placeholder View should below the Indicator View
+            if let indicatorView = imageConfiguration.indicator?.indicatorView {
+                #if os(macOS)
+                view.wrapped.addSubview(placeholderView, positioned: .below, relativeTo: indicatorView)
+                #else
+                view.wrapped.insertSubview(placeholderView, belowSubview: indicatorView)
+                #endif
+            } else {
+                view.wrapped.addSubview(placeholderView)
+            }
+            placeholderView.bindFrameToSuperviewBounds()
         }
+    }
+    
+    func loadImage(_ view: AnimatedImageViewWrapper, context: Context) {
         self.imageLoading.isLoading = true
         let options = imageModel.webOptions
         if options.contains(.delayPlaceholder) {
@@ -228,15 +248,19 @@ public struct AnimatedImage : PlatformViewRepresentable {
             }
             self.imageHandler.progressBlock?(receivedSize, expectedSize)
         }) { (image, data, error, cacheType, finished, _) in
-            // This is a hack because of Xcode 11.3 bug, the @Published does not trigger another `updateUIView` call
-            // Here I have to use UIKit/AppKit API to triger the same effect (the window change implicitly cause re-render)
-            if let hostingView = AnimatedImage.findHostingView(from: view) {
-                if let _ = hostingView.window {
-                    #if os(macOS)
-                    hostingView.viewDidMoveToWindow()
-                    #else
-                    hostingView.didMoveToWindow()
-                    #endif
+            if #available(iOS 14.0, macOS 11.0, watchOS 7.0, tvOS 14.0, *) {
+                // Do nothing. on iOS 14's SwiftUI, the @Published will always trigger another `updateUIView` call with new UIView instance.
+            } else {
+                // This is a hack because of iOS 13's SwiftUI bug, the @Published does not trigger another `updateUIView` call
+                // Here I have to use UIKit/AppKit API to triger the same effect (the window change implicitly cause re-render)
+                if let hostingView = AnimatedImage.findHostingView(from: view) {
+                    if let _ = hostingView.window {
+                        #if os(macOS)
+                        hostingView.viewDidMoveToWindow()
+                        #else
+                        hostingView.didMoveToWindow()
+                        #endif
+                    }
                 }
             }
             self.imageLoading.image = image
@@ -263,37 +287,40 @@ public struct AnimatedImage : PlatformViewRepresentable {
     func updateView(_ view: AnimatedImageViewWrapper, context: Context) {
         // Refresh image, imageModel is the Source of Truth, switch the type
         // Although we have Source of Truth, we can check the previous value, to avoid re-generate SDAnimatedImage, which is performance-cost.
-        if let name = imageModel.name, name != view.wrapped.sd_imageName {
+        if let name = imageModel.name, name != imageLoading.imageName {
             #if os(macOS)
             let image = SDAnimatedImage(named: name, in: imageModel.bundle)
             #else
             let image = SDAnimatedImage(named: name, in: imageModel.bundle, compatibleWith: nil)
             #endif
-            view.wrapped.sd_imageName = name
+            imageLoading.imageName = name
             view.wrapped.image = image
-        } else if let data = imageModel.data, data != view.wrapped.sd_imageData {
+        } else if let data = imageModel.data, data != imageLoading.imageData {
             let image = SDAnimatedImage(data: data, scale: imageModel.scale)
-            view.wrapped.sd_imageData = data
+            imageLoading.imageData = data
             view.wrapped.image = image
-        } else if let url = imageModel.url, url != view.wrapped.sd_imageURL {
-            view.wrapped.sd_imageIndicator = imageConfiguration.indicator
-            view.wrapped.sd_imageTransition = imageConfiguration.transition
-            if let placeholderView = imageConfiguration.placeholderView {
-                placeholderView.removeFromSuperview()
-                placeholderView.isHidden = true
-                // Placeholder View should below the Indicator View
-                if let indicatorView = imageConfiguration.indicator?.indicatorView {
-                    #if os(macOS)
-                    view.wrapped.addSubview(placeholderView, positioned: .below, relativeTo: indicatorView)
-                    #else
-                    view.wrapped.insertSubview(placeholderView, belowSubview: indicatorView)
-                    #endif
+        } else if let url = imageModel.url {
+            // Determine if image already been loaded and URL is match
+            var shouldLoad: Bool
+            if url != imageLoading.imageURL {
+                // Change the URL, need new loading
+                shouldLoad = true
+                imageLoading.imageURL = url
+            } else {
+                // Same URL, check if already loaded
+                if imageLoading.isLoading {
+                    shouldLoad = false
+                } else if let image = imageLoading.image {
+                    shouldLoad = false
+                    view.wrapped.image = image
                 } else {
-                    view.wrapped.addSubview(placeholderView)
+                    shouldLoad = true
                 }
-                placeholderView.bindFrameToSuperviewBounds()
             }
-            loadImage(view, context: context)
+            if shouldLoad {
+                setupIndicator(view, context: context)
+                loadImage(view, context: context)
+            }
         }
 
         #if os(macOS)

SDWebImageSwiftUI/Classes/ImageViewWrapper.swift

@@ -66,31 +66,6 @@ public class AnimatedImageViewWrapper : PlatformView {
     }
 }
 
-
-/// Store the Animated Image loading state, to avoid re-query duinrg `updateView(_:)` until Source of Truth changes
-@available(iOS 13.0, OSX 10.15, tvOS 13.0, watchOS 6.0, *)
-extension PlatformView {
-    static private var sd_imageNameKey: Void?
-    static private var sd_imageDataKey: Void?
-    
-    var sd_imageName: String? {
-        get {
-            objc_getAssociatedObject(self, &PlatformView.sd_imageNameKey) as? String
-        }
-        set {
-            objc_setAssociatedObject(self, &PlatformView.sd_imageNameKey, newValue, .OBJC_ASSOCIATION_RETAIN_NONATOMIC)
-        }
-    }
-    var sd_imageData: Data? {
-        get {
-            objc_getAssociatedObject(self, &PlatformView.sd_imageDataKey) as? Data
-        }
-        set {
-            objc_setAssociatedObject(self, &PlatformView.sd_imageDataKey, newValue, .OBJC_ASSOCIATION_RETAIN_NONATOMIC)
-        }
-    }
-}
-
 /// Use wrapper to solve the `UIProgressView`/`NSProgressIndicator` frame origin NaN crash (SwiftUI's bug)
 @available(iOS 13.0, OSX 10.15, tvOS 13.0, watchOS 6.0, *)
 public class ProgressIndicatorWrapper : PlatformView {

Tests/AnimatedImageTests.swift

@@ -46,7 +46,6 @@ class AnimatedImageTests: XCTestCase {
         } else {
             XCTFail("SDAnimatedImageView.image invalid")
         }
-        XCTAssertEqual(animatedImageView.sd_imageName, imageName)
         expectation.fulfill()
         self.waitForExpectations(timeout: 5, handler: nil)
         ViewHosting.expel()
@@ -64,7 +63,6 @@ class AnimatedImageTests: XCTestCase {
         } else {
             XCTFail("SDAnimatedImageView.image invalid")
         }
-        XCTAssertEqual(animatedImageView.sd_imageData, imageData)
         expectation.fulfill()
         self.waitForExpectations(timeout: 5, handler: nil)
         ViewHosting.expel()