From 1edee7f01997a8131889b1d127678007d2763ccd Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Thu, 27 Jun 2024 16:37:09 +0800 Subject: [PATCH 1/3] Re-implements the aspectRatio support on AnimatedImage, fix issue like cornerRadius Use the correct way to override invalidateIntrinsicContentSize to keep aspect ratio to UIKit/SwiftUI engine --- .../SDWebImageSwiftUIDemo/ContentView.swift | 11 +++ SDWebImageSwiftUI/Classes/AnimatedImage.swift | 99 ++++--------------- .../Classes/ImageViewWrapper.swift | 18 +++- 3 files changed, 45 insertions(+), 83 deletions(-) diff --git a/Example/SDWebImageSwiftUIDemo/ContentView.swift b/Example/SDWebImageSwiftUIDemo/ContentView.swift index 6e7e74cf..e1fc7a57 100644 --- a/Example/SDWebImageSwiftUIDemo/ContentView.swift +++ b/Example/SDWebImageSwiftUIDemo/ContentView.swift @@ -17,6 +17,17 @@ class UserSettings: ObservableObject { #endif } +struct ContentView4: View { + var url = URL(string: "https://github.com/SDWebImage/SDWebImageSwiftUI/assets/97430818/72d27f90-e9d8-48d7-b144-82ada828a027")! + var body: some View { + AnimatedImage(url: url) + .resizable() + .scaledToFit() +// .aspectRatio(nil, contentMode: .fit) + .clipShape(RoundedRectangle(cornerRadius: 50, style: .continuous)) + } +} + // Test Switching nil url struct ContentView3: View { @State var isOn = false diff --git a/SDWebImageSwiftUI/Classes/AnimatedImage.swift b/SDWebImageSwiftUI/Classes/AnimatedImage.swift index 3070b0ab..83d0b2c9 100644 --- a/SDWebImageSwiftUI/Classes/AnimatedImage.swift +++ b/SDWebImageSwiftUI/Classes/AnimatedImage.swift @@ -275,6 +275,9 @@ public struct AnimatedImage : PlatformViewRepresentable { self.imageModel.placeholderView?.isHidden = false self.imageHandler.failureBlock?(error ?? NSError()) } + // Finished loading + configureView(view, context: context) + layoutView(view, context: context) } } @@ -361,20 +364,7 @@ public struct AnimatedImage : PlatformViewRepresentable { break // impossible } - #if os(macOS) - if self.isAnimating != view.wrapped.animates { - view.wrapped.animates = self.isAnimating - } - #else - if self.isAnimating != view.wrapped.isAnimating { - if self.isAnimating { - view.wrapped.startAnimating() - } else { - view.wrapped.stopAnimating() - } - } - #endif - + // Finished loading configureView(view, context: context) layoutView(view, context: context) if let viewUpdateBlock = imageHandler.viewUpdateBlock { @@ -442,9 +432,7 @@ public struct AnimatedImage : PlatformViewRepresentable { #endif // Resizable - if let _ = imageLayout.resizingMode { - view.resizable = true - } + view.resizingMode = imageLayout.resizingMode // Animated Image does not support resizing mode and rendering mode if let image = view.wrapped.image { @@ -587,6 +575,21 @@ public struct AnimatedImage : PlatformViewRepresentable { } else { view.wrapped.playbackMode = .normal } + + // Animation + #if os(macOS) + if self.isAnimating != view.wrapped.animates { + view.wrapped.animates = self.isAnimating + } + #else + if self.isAnimating != view.wrapped.isAnimating { + if self.isAnimating { + view.wrapped.startAnimating() + } else { + view.wrapped.stopAnimating() + } + } + #endif } } @@ -630,68 +633,6 @@ extension AnimatedImage { } } -// Aspect Ratio -@available(iOS 14.0, macOS 11.0, tvOS 14.0, watchOS 7.0, *) -extension AnimatedImage { - func setImageLayoutAspectRatio(_ aspectRatio: CGFloat?, contentMode: ContentMode) { - self.imageLayout.aspectRatio = aspectRatio - self.imageLayout.contentMode = contentMode - } - - /// Constrains this view's dimensions to the specified aspect ratio. - /// - Parameters: - /// - aspectRatio: The ratio of width to height to use for the resulting - /// view. If `aspectRatio` is `nil`, the resulting view maintains this - /// view's aspect ratio. - /// - contentMode: A flag indicating whether this view should fit or - /// fill the parent context. - /// - Returns: A view that constrains this view's dimensions to - /// `aspectRatio`, using `contentMode` as its scaling algorithm. - @ViewBuilder - public func aspectRatio(_ aspectRatio: CGFloat? = nil, contentMode: ContentMode) -> some View { - // The `SwifUI.View.aspectRatio(_:contentMode:)` says: - // If `aspectRatio` is `nil`, the resulting view maintains this view's aspect ratio - // But 1: there are no public API to declare what `this view's aspect ratio` is - // So, if we don't override this method, SwiftUI ignore the content mode on actual ImageView - // To workaround, we want to call the default `SwifUI.View.aspectRatio(_:contentMode:)` method - // But 2: there are no way to call a Protocol Extention default implementation in Swift 5.1 - // So, we directly call the implementation detail modifier instead - // Fired Radar: FB7413534 - let _ = self.setImageLayoutAspectRatio(aspectRatio, contentMode: contentMode) - if let aspectRatio { - self.modifier(_AspectRatioLayout(aspectRatio: aspectRatio, contentMode: contentMode)) - } else { - self - } - } - - /// Constrains this view's dimensions to the aspect ratio of the given size. - /// - Parameters: - /// - aspectRatio: A size specifying the ratio of width to height to use - /// for the resulting view. - /// - contentMode: A flag indicating whether this view should fit or - /// fill the parent context. - /// - Returns: A view that constrains this view's dimensions to - /// `aspectRatio`, using `contentMode` as its scaling algorithm. - public func aspectRatio(_ aspectRatio: CGSize, contentMode: ContentMode) -> some View { - return self.aspectRatio(aspectRatio.width / aspectRatio.height, contentMode: contentMode) - } - - /// Scales this view to fit its parent. - /// - Returns: A view that scales this view to fit its parent, - /// maintaining this view's aspect ratio. - public func scaledToFit() -> some View { - return self.aspectRatio(nil, contentMode: .fit) - } - - /// Scales this view to fill its parent. - /// - Returns: A view that scales this view to fit its parent, - /// maintaining this view's aspect ratio. - public func scaledToFill() -> some View { - return self.aspectRatio(nil, contentMode: .fill) - } -} - // AnimatedImage Modifier @available(iOS 14.0, macOS 11.0, tvOS 14.0, watchOS 7.0, *) extension AnimatedImage { diff --git a/SDWebImageSwiftUI/Classes/ImageViewWrapper.swift b/SDWebImageSwiftUI/Classes/ImageViewWrapper.swift index 17c5a383..cfe81baf 100644 --- a/SDWebImageSwiftUI/Classes/ImageViewWrapper.swift +++ b/SDWebImageSwiftUI/Classes/ImageViewWrapper.swift @@ -8,6 +8,7 @@ import Foundation import SDWebImage +import SwiftUI #if !os(watchOS) @@ -18,7 +19,7 @@ public class AnimatedImageViewWrapper : PlatformView { public var wrapped = SDAnimatedImageView() var interpolationQuality = CGInterpolationQuality.default var shouldAntialias = false - var resizable = false + var resizingMode: Image.ResizingMode? public override func draw(_ rect: CGRect) { #if os(macOS) @@ -48,11 +49,20 @@ public class AnimatedImageViewWrapper : PlatformView { public override var intrinsicContentSize: CGSize { /// Match the behavior of SwiftUI.Image, only when image is resizable, use the super implementation to calculate size - if resizable { - return super.intrinsicContentSize + let imageSize = wrapped.intrinsicContentSize + if let _ = resizingMode { + /// Keep aspect ratio + let noIntrinsicMetric = AnimatedImageViewWrapper.noIntrinsicMetric + if (imageSize.width > 0 && imageSize.height > 0) { + let ratio = imageSize.width / imageSize.height + let size = CGSize(width: ratio, height: 1) + return size + } else { + return CGSize(width: noIntrinsicMetric, height: noIntrinsicMetric) + } } else { /// Not resizable, always use image size, like SwiftUI.Image - return wrapped.intrinsicContentSize + return imageSize } } From 3340ea4ed46b10e3a15b95ea10532e4ad2ec93b0 Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Thu, 27 Jun 2024 17:53:07 +0800 Subject: [PATCH 2/3] Fix the compatibility with UIView transition Actually this is not the good design, but at least a workaround --- SDWebImageSwiftUI/Classes/AnimatedImage.swift | 25 ++++++++++++++----- .../Classes/ImageViewWrapper.swift | 20 ++++++++++----- 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/SDWebImageSwiftUI/Classes/AnimatedImage.swift b/SDWebImageSwiftUI/Classes/AnimatedImage.swift index 83d0b2c9..193c9eb2 100644 --- a/SDWebImageSwiftUI/Classes/AnimatedImage.swift +++ b/SDWebImageSwiftUI/Classes/AnimatedImage.swift @@ -275,9 +275,8 @@ public struct AnimatedImage : PlatformViewRepresentable { self.imageModel.placeholderView?.isHidden = false self.imageHandler.failureBlock?(error ?? NSError()) } - // Finished loading - configureView(view, context: context) - layoutView(view, context: context) + // Finished loading, async + finishUpdateView(view, context: context, image: image) } } @@ -310,6 +309,8 @@ public struct AnimatedImage : PlatformViewRepresentable { #endif context.coordinator.imageLoading.imageName = name view.wrapped.image = image + // Finished loading, sync + finishUpdateView(view, context: context, image: image) } private func updateViewForData(_ data: Data?, view: AnimatedImageViewWrapper, context: Context) { @@ -323,6 +324,8 @@ public struct AnimatedImage : PlatformViewRepresentable { } context.coordinator.imageLoading.imageData = data view.wrapped.image = image + // Finished loading, sync + finishUpdateView(view, context: context, image: image) } private func updateViewForURL(_ url: URL?, view: AnimatedImageViewWrapper, context: Context) { @@ -347,6 +350,8 @@ public struct AnimatedImage : PlatformViewRepresentable { setupIndicator(view, context: context) loadImage(view, context: context) } + // Finished loading, sync + finishUpdateView(view, context: context, image: view.wrapped.image) } func updateView(_ view: AnimatedImageViewWrapper, context: Context) { @@ -364,9 +369,6 @@ public struct AnimatedImage : PlatformViewRepresentable { break // impossible } - // Finished loading - configureView(view, context: context) - layoutView(view, context: context) if let viewUpdateBlock = imageHandler.viewUpdateBlock { viewUpdateBlock(view.wrapped, context) } @@ -384,6 +386,17 @@ public struct AnimatedImage : PlatformViewRepresentable { } } + func finishUpdateView(_ view: AnimatedImageViewWrapper, context: Context, image: PlatformImage?) { + // Finished loading + if let imageSize = image?.size { + view.imageSize = imageSize + } else { + view.imageSize = nil + } + configureView(view, context: context) + layoutView(view, context: context) + } + func layoutView(_ view: AnimatedImageViewWrapper, context: Context) { // AspectRatio && ContentMode #if os(macOS) diff --git a/SDWebImageSwiftUI/Classes/ImageViewWrapper.swift b/SDWebImageSwiftUI/Classes/ImageViewWrapper.swift index cfe81baf..e019881f 100644 --- a/SDWebImageSwiftUI/Classes/ImageViewWrapper.swift +++ b/SDWebImageSwiftUI/Classes/ImageViewWrapper.swift @@ -20,6 +20,7 @@ public class AnimatedImageViewWrapper : PlatformView { var interpolationQuality = CGInterpolationQuality.default var shouldAntialias = false var resizingMode: Image.ResizingMode? + var imageSize: CGSize? public override func draw(_ rect: CGRect) { #if os(macOS) @@ -49,20 +50,27 @@ public class AnimatedImageViewWrapper : PlatformView { public override var intrinsicContentSize: CGSize { /// Match the behavior of SwiftUI.Image, only when image is resizable, use the super implementation to calculate size - let imageSize = wrapped.intrinsicContentSize + var contentSize = wrapped.intrinsicContentSize + /// Sometimes, like during the transaction, the wrapped.image == nil, which cause contentSize invalid + /// Use image size as backup + /// TODO: This mixed use of UIKit/SwiftUI animation will cause visial issue because the intrinsicContentSize during animation may be changed + if let imageSize = imageSize { + if contentSize != imageSize { + contentSize = imageSize + } + } if let _ = resizingMode { /// Keep aspect ratio - let noIntrinsicMetric = AnimatedImageViewWrapper.noIntrinsicMetric - if (imageSize.width > 0 && imageSize.height > 0) { - let ratio = imageSize.width / imageSize.height + if contentSize.width > 0 && contentSize.height > 0 { + let ratio = contentSize.width / contentSize.height let size = CGSize(width: ratio, height: 1) return size } else { - return CGSize(width: noIntrinsicMetric, height: noIntrinsicMetric) + return contentSize } } else { /// Not resizable, always use image size, like SwiftUI.Image - return imageSize + return contentSize } } From c8320d4e20d00455cb6401331cf54616bceb4e4c Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Thu, 27 Jun 2024 23:08:56 +0800 Subject: [PATCH 3/3] Revert the wrong changes to fix the unit test --- Example/SDWebImageSwiftUIDemo/ContentView.swift | 2 ++ SDWebImageSwiftUI/Classes/AnimatedImage.swift | 9 +++------ 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/Example/SDWebImageSwiftUIDemo/ContentView.swift b/Example/SDWebImageSwiftUIDemo/ContentView.swift index e1fc7a57..4cb0fa54 100644 --- a/Example/SDWebImageSwiftUIDemo/ContentView.swift +++ b/Example/SDWebImageSwiftUIDemo/ContentView.swift @@ -17,6 +17,7 @@ class UserSettings: ObservableObject { #endif } +#if !os(watchOS) struct ContentView4: View { var url = URL(string: "https://github.com/SDWebImage/SDWebImageSwiftUI/assets/97430818/72d27f90-e9d8-48d7-b144-82ada828a027")! var body: some View { @@ -27,6 +28,7 @@ struct ContentView4: View { .clipShape(RoundedRectangle(cornerRadius: 50, style: .continuous)) } } +#endif // Test Switching nil url struct ContentView3: View { diff --git a/SDWebImageSwiftUI/Classes/AnimatedImage.swift b/SDWebImageSwiftUI/Classes/AnimatedImage.swift index 193c9eb2..66543720 100644 --- a/SDWebImageSwiftUI/Classes/AnimatedImage.swift +++ b/SDWebImageSwiftUI/Classes/AnimatedImage.swift @@ -309,8 +309,6 @@ public struct AnimatedImage : PlatformViewRepresentable { #endif context.coordinator.imageLoading.imageName = name view.wrapped.image = image - // Finished loading, sync - finishUpdateView(view, context: context, image: image) } private func updateViewForData(_ data: Data?, view: AnimatedImageViewWrapper, context: Context) { @@ -324,8 +322,6 @@ public struct AnimatedImage : PlatformViewRepresentable { } context.coordinator.imageLoading.imageData = data view.wrapped.image = image - // Finished loading, sync - finishUpdateView(view, context: context, image: image) } private func updateViewForURL(_ url: URL?, view: AnimatedImageViewWrapper, context: Context) { @@ -350,8 +346,6 @@ public struct AnimatedImage : PlatformViewRepresentable { setupIndicator(view, context: context) loadImage(view, context: context) } - // Finished loading, sync - finishUpdateView(view, context: context, image: view.wrapped.image) } func updateView(_ view: AnimatedImageViewWrapper, context: Context) { @@ -369,6 +363,9 @@ public struct AnimatedImage : PlatformViewRepresentable { break // impossible } + // Finished loading, sync + finishUpdateView(view, context: context, image: view.wrapped.image) + if let viewUpdateBlock = imageHandler.viewUpdateBlock { viewUpdateBlock(view.wrapped, context) }