Skip to content

Try to fix the recursive updateView when using AnimatedImage inside ScrollView/LazyVStack. Which cause App freeze #162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 61 additions & 34 deletions SDWebImageSwiftUI/Classes/AnimatedImage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand All @@ -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
Expand All @@ -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)
Expand Down
25 changes: 0 additions & 25 deletions SDWebImageSwiftUI/Classes/ImageViewWrapper.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 0 additions & 2 deletions Tests/AnimatedImageTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
Expand Down