From 48c9197e1df8220aeed6ce4d9ccaa6d601d2857c Mon Sep 17 00:00:00 2001 From: kean Date: Tue, 24 Dec 2024 12:13:30 -0500 Subject: [PATCH 1/6] Update MediaItemHeaderView to use AsyncImageView instead of CachedAnimatedImageView --- .../ViewRelated/Cells/MediaItemHeaderView.swift | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/WordPress/Classes/ViewRelated/Cells/MediaItemHeaderView.swift b/WordPress/Classes/ViewRelated/Cells/MediaItemHeaderView.swift index 6c5b67854f6a..4d19260cbacd 100644 --- a/WordPress/Classes/ViewRelated/Cells/MediaItemHeaderView.swift +++ b/WordPress/Classes/ViewRelated/Cells/MediaItemHeaderView.swift @@ -4,7 +4,7 @@ import WordPressShared import WordPressMedia final class MediaItemHeaderView: UIView { - let imageView = CachedAnimatedImageView() + let imageView = AsyncImageView() private let errorView = UIImageView() private let videoIconView = PlayIconView() private let loadingIndicator = UIActivityIndicatorView(style: .large) @@ -103,13 +103,7 @@ final class MediaItemHeaderView: UIView { Task { let image = try? await MediaImageService.shared.image(for: media, size: .large) loadingIndicator.stopAnimating() - - if let gif = image as? AnimatedImage, let data = gif.gifData { - imageView.animate(withGIFData: data) - } else { - imageView.image = image - } - + imageView.image = image errorView.isHidden = image != nil } From 6dbcf3c2217d9341e88ad3d0cbfbff9f3b44b1cd Mon Sep 17 00:00:00 2001 From: kean Date: Tue, 24 Dec 2024 12:15:31 -0500 Subject: [PATCH 2/6] Fix code formatting in RichTextView --- .../Views/RichTextView/RichTextView.swift | 46 ++++++++++--------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/WordPress/Classes/ViewRelated/Views/RichTextView/RichTextView.swift b/WordPress/Classes/ViewRelated/Views/RichTextView/RichTextView.swift index badf919f0774..637cebc2ddd4 100644 --- a/WordPress/Classes/ViewRelated/Views/RichTextView/RichTextView.swift +++ b/WordPress/Classes/ViewRelated/Views/RichTextView/RichTextView.swift @@ -137,20 +137,20 @@ import UniformTypeIdentifiers // MARK: - Private Methods fileprivate func setupSubviews() { - gesturesRecognizer = UITapGestureRecognizer() + gesturesRecognizer = UITapGestureRecognizer() gesturesRecognizer.addTarget(self, action: #selector(RichTextView.handleTextViewTap(_:))) - textView = UITextView(frame: bounds) - textView.backgroundColor = backgroundColor - textView.contentInset = UIEdgeInsets.zero - textView.textContainerInset = UIEdgeInsets.zero - textView.textContainer.lineFragmentPadding = 0 - textView.layoutManager.allowsNonContiguousLayout = false - textView.isEditable = editable - textView.isScrollEnabled = false - textView.dataDetectorTypes = dataDetectorTypes - textView.delegate = self - textView.gestureRecognizers = [gesturesRecognizer] + textView = UITextView(frame: bounds) + textView.backgroundColor = backgroundColor + textView.contentInset = UIEdgeInsets.zero + textView.textContainerInset = UIEdgeInsets.zero + textView.textContainer.lineFragmentPadding = 0 + textView.layoutManager.allowsNonContiguousLayout = false + textView.isEditable = editable + textView.isScrollEnabled = false + textView.dataDetectorTypes = dataDetectorTypes + textView.delegate = self + textView.gestureRecognizers = [gesturesRecognizer] addSubview(textView) // Setup Layout @@ -183,8 +183,8 @@ import UniformTypeIdentifiers return } - let unwrappedView = attachmentView! - unwrappedView.frame.origin = self.textView.frameForTextInRange(range).integral.origin + let unwrappedView = attachmentView! + unwrappedView.frame.origin = self.textView.frameForTextInRange(range).integral.origin self.textView.addSubview(unwrappedView) self.attachmentViews.append(unwrappedView) } @@ -208,14 +208,16 @@ import UniformTypeIdentifiers // NOTE: Why do we need this? // Because this mechanism allows us to disable DataDetectors, and yet, detect taps on links. // - let textStorage = textView.textStorage - let layoutManager = textView.layoutManager - let textContainer = textView.textContainer - - let locationInTextView = recognizer.location(in: textView) - let characterIndex = layoutManager.characterIndex(for: locationInTextView, - in: textContainer, - fractionOfDistanceBetweenInsertionPoints: nil) + let textStorage = textView.textStorage + let layoutManager = textView.layoutManager + let textContainer = textView.textContainer + + let locationInTextView = recognizer.location(in: textView) + let characterIndex = layoutManager.characterIndex( + for: locationInTextView, + in: textContainer, + fractionOfDistanceBetweenInsertionPoints: nil + ) if characterIndex >= textStorage.length { return From aebedb6448fc9cba77c8321f422f2d4f744c25ed Mon Sep 17 00:00:00 2001 From: kean Date: Tue, 24 Dec 2024 12:17:37 -0500 Subject: [PATCH 3/6] Update AnimatedGifAttachmentViewProvider to use GIFImageView directly --- .../RichTextView/AnimatedGifAttachmentViewProvider.swift | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/WordPress/Classes/ViewRelated/Views/RichTextView/AnimatedGifAttachmentViewProvider.swift b/WordPress/Classes/ViewRelated/Views/RichTextView/AnimatedGifAttachmentViewProvider.swift index 65b20773431c..c9a36d9885c2 100644 --- a/WordPress/Classes/ViewRelated/Views/RichTextView/AnimatedGifAttachmentViewProvider.swift +++ b/WordPress/Classes/ViewRelated/Views/RichTextView/AnimatedGifAttachmentViewProvider.swift @@ -1,4 +1,5 @@ import UIKit +import Gifu /** * This adds custom view rendering for animated Gif images in a UITextView @@ -7,11 +8,11 @@ import UIKit */ class AnimatedGifAttachmentViewProvider: NSTextAttachmentViewProvider { deinit { - guard let animatedImageView = view as? CachedAnimatedImageView else { + guard let animatedImageView = view as? GIFImageView else { return } - animatedImageView.stopAnimating() + animatedImageView.prepareForReuse() } override init(textAttachment: NSTextAttachment, parentView: UIView?, textLayoutManager: NSTextLayoutManager?, location: NSTextLocation) { @@ -20,8 +21,8 @@ class AnimatedGifAttachmentViewProvider: NSTextAttachmentViewProvider { return } - let imageView = CachedAnimatedImageView(frame: parentView?.bounds ?? .zero) - imageView.setAnimatedImage(contents) + let imageView = GIFImageView(frame: parentView?.bounds ?? .zero) + imageView.animate(withGIFData: contents) view = imageView } From c2e261c4fab7df8c1f9ca835b135cc081fb2edfa Mon Sep 17 00:00:00 2001 From: kean Date: Tue, 24 Dec 2024 12:18:11 -0500 Subject: [PATCH 4/6] Remove SolidColorActivityIndicator --- .../Media/SolidColorActivityIndicator.swift | 20 ------------------- 1 file changed, 20 deletions(-) delete mode 100644 WordPress/Classes/ViewRelated/Media/SolidColorActivityIndicator.swift diff --git a/WordPress/Classes/ViewRelated/Media/SolidColorActivityIndicator.swift b/WordPress/Classes/ViewRelated/Media/SolidColorActivityIndicator.swift deleted file mode 100644 index 0a8a87876954..000000000000 --- a/WordPress/Classes/ViewRelated/Media/SolidColorActivityIndicator.swift +++ /dev/null @@ -1,20 +0,0 @@ -import Foundation - -final class SolidColorActivityIndicator: UIView, ActivityIndicatorType { - init(color: UIColor = .secondarySystemBackground) { - super.init(frame: .zero) - backgroundColor = color - } - - required init?(coder: NSCoder) { - fatalError("init(coder:) has not been implemented") - } - - func startAnimating() { - isHidden = false - } - - func stopAnimating() { - isHidden = true - } -} From 36c716162b896faa52680c2eefe6df69045989e5 Mon Sep 17 00:00:00 2001 From: kean Date: Tue, 24 Dec 2024 12:19:16 -0500 Subject: [PATCH 5/6] Remove CachedAnimatedImageView --- ...arProgressView+ActivityIndicatorType.swift | 2 +- .../Media/CachedAnimatedImageView.swift | 285 ------------------ 2 files changed, 1 insertion(+), 286 deletions(-) delete mode 100644 WordPress/Classes/ViewRelated/Media/CachedAnimatedImageView.swift diff --git a/WordPress/Classes/Extensions/Media/CircularProgressView+ActivityIndicatorType.swift b/WordPress/Classes/Extensions/Media/CircularProgressView+ActivityIndicatorType.swift index b05174a15acc..e0cff74a813d 100644 --- a/WordPress/Classes/Extensions/Media/CircularProgressView+ActivityIndicatorType.swift +++ b/WordPress/Classes/Extensions/Media/CircularProgressView+ActivityIndicatorType.swift @@ -1,6 +1,6 @@ import UIKit -extension CircularProgressView: ActivityIndicatorType { +extension CircularProgressView { func startAnimating() { isHidden = false state = .indeterminate diff --git a/WordPress/Classes/ViewRelated/Media/CachedAnimatedImageView.swift b/WordPress/Classes/ViewRelated/Media/CachedAnimatedImageView.swift deleted file mode 100644 index 95e52342a294..000000000000 --- a/WordPress/Classes/ViewRelated/Media/CachedAnimatedImageView.swift +++ /dev/null @@ -1,285 +0,0 @@ -// -// Previously, we were using FLAnimatedImage to show gifs. (https://github.com/Flipboard/FLAnimatedImage) -// It's a good, battle-tested component written in Obj-c with a good solution for memory usage on big files. -// We decided to look for other alternatives and we got to Gifu. (https://github.com/kaishin/Gifu) -// - It has a similar approach to be memory efficient. Tests showed that is more memory efficient than FLAnimatedImage. -// - It's written in Swift, in a protocol oriented approach. That make it easier to implement it in a Swift code base. -// - It has extra features, like stopping and plying gifs, and a special `prepareForReuse` for table/collection views. -// - It seems to be more active, being updated few months ago, in contrast to a couple of years ago of FLAnimatedImage - -import Foundation -import Gifu - -@objc public protocol ActivityIndicatorType where Self: UIView { - func startAnimating() - func stopAnimating() -} - -extension UIActivityIndicatorView: ActivityIndicatorType { -} - -public class CachedAnimatedImageView: UIImageView, GIFAnimatable { - - public enum LoadingIndicatorStyle { - case centered(withSize: CGSize?) - case fullView - } - - // MARK: Public fields - - @objc public var gifStrategy: GIFStrategy { - get { - return gifPlaybackStrategy.gifStrategy - } - set(newGifStrategy) { - gifPlaybackStrategy = newGifStrategy.playbackStrategy - } - } - - @objc public private(set) var animatedGifData: Data? - - public lazy var animator: Gifu.Animator? = { - return Gifu.Animator(withDelegate: self) - }() - - @objc public var shouldShowLoadingIndicator: Bool = true - - // MARK: Private fields - - private var gifPlaybackStrategy: GIFPlaybackStrategy = MediumGIFPlaybackStrategy() - - @objc private var currentTask: URLSessionTask? - - private var customLoadingIndicator: ActivityIndicatorType? - - private var isImageAnimated: Bool { - animatedGifData != nil - } - - private lazy var defaultLoadingIndicator: UIActivityIndicatorView = { - let loadingIndicator = UIActivityIndicatorView(style: .medium) - layoutViewCentered(loadingIndicator, size: nil) - return loadingIndicator - }() - - private var loadingIndicator: ActivityIndicatorType { - guard let custom = customLoadingIndicator else { - return defaultLoadingIndicator - } - return custom - } - - // MARK: Initializers - - public override init(image: UIImage?, highlightedImage: UIImage?) { - super.init(image: image, highlightedImage: highlightedImage) - commonInit() - } - - public override init(frame: CGRect) { - super.init(frame: frame) - commonInit() - } - - public override init(image: UIImage?) { - super.init(image: image) - commonInit() - } - - public required init?(coder aDecoder: NSCoder) { - super.init(coder: aDecoder) - commonInit() - } - - private func commonInit() { - NotificationCenter.default.addObserver(self, - selector: #selector(handleLowMemoryWarningNotification), - name: UIApplication.didReceiveMemoryWarningNotification, - object: nil) - } - - // MARK: - Public methods - - override open func display(_ layer: CALayer) { - // Fixes an unrecognized selector crash on iOS 13 and below when calling super.display(_:) directly - // This was first reported here: p5T066-1xs-p2#comment-5908 - // Investigating the issue I came across this discussion with a workaround in the Gifu repo: https://git.io/JUPxC - if UIImageView.instancesRespond(to: #selector(display(_:))) { - super.display(layer) - } - - updateImageIfNeeded() - } - - @objc public func setAnimatedImage(_ urlRequest: URLRequest, - placeholderImage: UIImage?, - success: (() -> Void)?, - failure: ((NSError?) -> Void)?) { - - currentTask?.cancel() - image = placeholderImage - - if checkCache(urlRequest, success) { - return - } - - let successBlock: (Data, UIImage?) -> Void = { [weak self] animatedImageData, staticImage in - self?.validateAndSetGifData(animatedImageData, alternateStaticImage: staticImage, success: success) - } - - currentTask = AnimatedImageCache.shared.animatedImage(urlRequest, - placeholderImage: placeholderImage, - success: successBlock, - failure: failure) - } - - @objc public func setAnimatedImage(_ animatedImageData: Data, success: (() -> Void)? = nil) { - currentTask?.cancel() - validateAndSetGifData(animatedImageData, alternateStaticImage: nil, success: success) - } - - /// Clean the image view from previous images and ongoing data tasks. - /// - @objc public func clean() { - currentTask?.cancel() - image = nil - animatedGifData = nil - } - - @objc public func prepForReuse() { - if isImageAnimated { - self.prepareForReuse() - } - } - - @objc public func startLoadingAnimation() { - guard shouldShowLoadingIndicator else { - return - } - DispatchQueue.main.async() { - self.loadingIndicator.startAnimating() - } - } - - @objc public func stopLoadingAnimation() { - DispatchQueue.main.async() { - self.loadingIndicator.stopAnimating() - } - } - - public func addLoadingIndicator(_ loadingIndicator: ActivityIndicatorType, style: LoadingIndicatorStyle) { - removeCustomLoadingIndicator() - customLoadingIndicator = loadingIndicator - addCustomLoadingIndicator(loadingIndicator, style: style) - } - - // MARK: - Private methods - - @objc private func handleLowMemoryWarningNotification(_ notification: NSNotification) { - stopAnimatingGIF() - } - - private func validateAndSetGifData(_ animatedImageData: Data, alternateStaticImage: UIImage? = nil, success: (() -> Void)? = nil) { - let didVerifyDataSize = gifPlaybackStrategy.verifyDataSize(animatedImageData) - DispatchQueue.main.async() { - if let staticImage = alternateStaticImage { - self.image = staticImage - } else { - self.image = UIImage(data: animatedImageData) - } - - DispatchQueue.global().async { - if didVerifyDataSize { - self.animate(data: animatedImageData, success: success) - } else { - self.animatedGifData = nil - success?() - } - } - } - } - - private func checkCache(_ urlRequest: URLRequest, _ success: (() -> Void)?) -> Bool { - if let cachedData = AnimatedImageCache.shared.cachedData(url: urlRequest.url) { - // Always attempt to load momentary image to show while gif is loading to avoid flashing. - if let cachedStaticImage = AnimatedImageCache.shared.cachedStaticImage(url: urlRequest.url) { - image = cachedStaticImage - } else { - animatedGifData = nil - let staticImage = UIImage(data: cachedData) - image = staticImage - AnimatedImageCache.shared.cacheStaticImage(url: urlRequest.url, image: staticImage) - } - - if gifPlaybackStrategy.verifyDataSize(cachedData) { - animate(data: cachedData, success: success) - } else { - success?() - } - - return true - } - - return false - } - - private func animate(data: Data, success: (() -> Void)?) { - animatedGifData = data - DispatchQueue.main.async() { - self.setFrameBufferCount(self.gifPlaybackStrategy.frameBufferCount) - self.animate(withGIFData: data, preparationBlock: { - success?() - }) - } - } - - // MARK: Loading indicator - - private func removeCustomLoadingIndicator() { - if let oldLoadingIndicator = customLoadingIndicator { - oldLoadingIndicator.removeFromSuperview() - } - } - - private func addCustomLoadingIndicator(_ loadingView: UIView, style: LoadingIndicatorStyle) { - switch style { - case .centered(let size): - layoutViewCentered(loadingView, size: size) - default: - layoutViewFullView(loadingView) - } - } - - // MARK: Layout - - private func prepareViewForLayout(_ view: UIView) { - if view.superview == nil { - addSubview(view) - } - view.translatesAutoresizingMaskIntoConstraints = false - } - - private func layoutViewCentered(_ view: UIView, size: CGSize?) { - prepareViewForLayout(view) - var constraints: [NSLayoutConstraint] = [ - view.centerXAnchor.constraint(equalTo: centerXAnchor), - view.centerYAnchor.constraint(equalTo: centerYAnchor) - ] - if let size { - constraints.append(view.heightAnchor.constraint(equalToConstant: size.height)) - constraints.append(view.widthAnchor.constraint(equalToConstant: size.width)) - } - NSLayoutConstraint.activate(constraints) - } - - private func layoutViewFullView(_ view: UIView) { - prepareViewForLayout(view) - NSLayoutConstraint.activate([ - view.leadingAnchor.constraint(equalTo: leadingAnchor), - view.trailingAnchor.constraint(equalTo: trailingAnchor), - view.topAnchor.constraint(equalTo: topAnchor), - view.bottomAnchor.constraint(equalTo: bottomAnchor), - ]) - } - -} From fbbfcb93edd2f7cec955f85b8696deccf6e22148 Mon Sep 17 00:00:00 2001 From: kean Date: Tue, 24 Dec 2024 12:19:35 -0500 Subject: [PATCH 6/6] Remove GIFPlaybackStrategy --- .../Utility/Media/GIFPlaybackStrategy.swift | 80 ------------------- 1 file changed, 80 deletions(-) delete mode 100644 WordPress/Classes/Utility/Media/GIFPlaybackStrategy.swift diff --git a/WordPress/Classes/Utility/Media/GIFPlaybackStrategy.swift b/WordPress/Classes/Utility/Media/GIFPlaybackStrategy.swift deleted file mode 100644 index 4a4f80a16a50..000000000000 --- a/WordPress/Classes/Utility/Media/GIFPlaybackStrategy.swift +++ /dev/null @@ -1,80 +0,0 @@ -import Foundation - -@objc -public enum GIFStrategy: Int { - case tinyGIFs - case smallGIFs - case mediumGIFs - case largeGIFs - - /// Returns the corresponding playback strategy instance - /// - var playbackStrategy: GIFPlaybackStrategy { - switch self { - case .tinyGIFs: - return TinyGIFPlaybackStrategy() - case .smallGIFs: - return SmallGIFPlaybackStrategy() - case .mediumGIFs: - return MediumGIFPlaybackStrategy() - case .largeGIFs: - return LargeGIFPlaybackStrategy() - } - } -} - -public protocol GIFPlaybackStrategy { - /// Maximum size GIF data can be in order to be animated. - /// - var maxSize: Int { get } - - /// The number of frames that should be buffered. A high number will result in more - /// memory usage and less CPU load, and vice versa. Default is 50. - /// - var frameBufferCount: Int { get } - - /// Returns the coresponding GIFStrategy enum value. - /// - var gifStrategy: GIFStrategy { get } - - /// Verifies the GIF data against the `maxSize` var. - /// - /// - Parameter data: object containg the GIF - /// - Returns: **true** if data is under the maximum size limit (inclusive) and **false** if over the limit - /// - func verifyDataSize(_ data: Data) -> Bool -} - -extension GIFPlaybackStrategy { - func verifyDataSize(_ data: Data) -> Bool { - guard data.count <= maxSize else { - DDLogDebug("⚠️ Maximum GIF data size exceeded \(maxSize) with \(data.count)") - return false - } - return true - } -} -// This is good for thumbnail GIFs used in a collection view -class TinyGIFPlaybackStrategy: GIFPlaybackStrategy { - var maxSize = 2_000_000 // in MB - var frameBufferCount = 5 - var gifStrategy: GIFStrategy = .tinyGIFs -} - -class SmallGIFPlaybackStrategy: GIFPlaybackStrategy { - var maxSize = 8_000_000 // in MB - var frameBufferCount = 50 - var gifStrategy: GIFStrategy = .smallGIFs -} - -class MediumGIFPlaybackStrategy: GIFPlaybackStrategy { - var maxSize = 20_000_000 // in MB - var frameBufferCount = 50 - var gifStrategy: GIFStrategy = .mediumGIFs -} - -class LargeGIFPlaybackStrategy: GIFPlaybackStrategy { - var maxSize = 50_000_000 // in MB - var frameBufferCount = 50 - var gifStrategy: GIFStrategy = .largeGIFs -}