-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Reader: Add saliency detection for smart cropping #25451
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
base: trunk
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,176 @@ | ||
| import Collections | ||
| import UIKit | ||
| import Vision | ||
|
|
||
| /// Detects the most salient (visually interesting) region in images using Vision framework. | ||
| /// Results are cached by image URL. | ||
| public actor ImageSaliencyService { | ||
| public nonisolated static let shared = ImageSaliencyService() | ||
|
|
||
| private nonisolated let cache = SaliencyCache() | ||
| private nonisolated let detector = SaliencyDetector() | ||
| private var inflightTasks: [URL: Task<CGRect?, Never>] = [:] | ||
|
|
||
| init() { | ||
| Task { [cache] in | ||
| cache.loadFromDisk() | ||
| } | ||
| } | ||
|
|
||
| /// Returns a cached rect synchronously without starting a task, or `nil` if not yet cached. | ||
| public nonisolated func cachedSaliencyRect(for url: URL) -> CGRect? { | ||
| cache.cachedRect(for: url) | ||
| } | ||
|
|
||
| /// Returns the bounding rect of the most salient region in UIKit normalized coordinates | ||
| /// (origin top-left, values 0–1), or `nil` if detection fails or no salient objects are found. | ||
| /// | ||
| /// - warning: The underlying `Vision` framework works _only_ on the device. | ||
| public func saliencyRect(for image: UIImage, url: URL) async -> CGRect? { | ||
| if cache.isCached(for: url) { | ||
| return cache.cachedRect(for: url) | ||
| } | ||
| if let existing = inflightTasks[url] { | ||
| return await existing.value | ||
| } | ||
| let task = Task<CGRect?, Never> { [detector] in | ||
| await detector.detect(in: image) | ||
| } | ||
| inflightTasks[url] = task | ||
| let result = await task.value | ||
| inflightTasks[url] = nil | ||
| cache.store(result, for: url) | ||
| return result | ||
| } | ||
|
|
||
| /// Returns the frame for the image view within a container such that `saliencyRect` | ||
| /// appears at `topInset` points from the top. Returns `nil` when no adjustment is needed | ||
| /// (i.e. the image is not portrait relative to the container). | ||
| public nonisolated func adjustedFrame( | ||
| saliencyRect: CGRect, | ||
| imageSize: CGSize, | ||
| in containerSize: CGSize, | ||
| topInset: CGFloat = 16 | ||
| ) -> CGRect? { | ||
| guard imageSize.width > 0, imageSize.height > 0, | ||
| containerSize.width > 0, containerSize.height > 0 else { return nil } | ||
|
|
||
| let imageAspect = imageSize.width / imageSize.height | ||
| let containerAspect = containerSize.width / containerSize.height | ||
|
|
||
| // Only adjust for portrait images shown in a wider container. | ||
| guard imageAspect < containerAspect else { return nil } | ||
|
|
||
| // Scale to fill container width; the scaled height will exceed container height. | ||
| let scale = containerSize.width / imageSize.width | ||
| let scaledHeight = imageSize.height * scale | ||
|
|
||
| let salientTopInScaled = saliencyRect.origin.y * scaledHeight | ||
| let desiredY = topInset - salientTopInScaled | ||
|
|
||
| // Clamp so the image always covers the full container without empty gaps. | ||
| let minY = containerSize.height - scaledHeight // negative | ||
| let clampedY = min(0, max(minY, desiredY)) | ||
|
|
||
| return CGRect(x: 0, y: clampedY, width: containerSize.width, height: scaledHeight) | ||
| } | ||
|
|
||
| } | ||
|
|
||
| /// Runs saliency detection serially — one image at a time. | ||
| private actor SaliencyDetector { | ||
| func detect(in image: UIImage) -> CGRect? { | ||
| guard let cgImage = image.cgImage else { return nil } | ||
| let request = VNGenerateObjectnessBasedSaliencyImageRequest() | ||
| let handler = VNImageRequestHandler(cgImage: cgImage, options: [:]) | ||
| do { | ||
| try handler.perform([request]) | ||
| } catch { | ||
| return nil | ||
| } | ||
| guard let observation = request.results?.first, | ||
| let salientObjects = observation.salientObjects, | ||
| !salientObjects.isEmpty else { | ||
| return nil | ||
| } | ||
| // Union all salient object bounding boxes. | ||
| // Vision coordinates: origin at bottom-left, Y increases upward. | ||
| let union = salientObjects.reduce(CGRect.null) { $0.union($1.boundingBox) } | ||
| // Convert to UIKit coordinates (origin at top-left, Y increases downward). | ||
| return CGRect( | ||
| x: union.origin.x, | ||
| y: 1.0 - union.origin.y - union.height, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is this |
||
| width: union.width, | ||
| height: union.height | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| private final class SaliencyCache: @unchecked Sendable { | ||
| private var store: OrderedDictionary<String, CGRect?> = [:] | ||
| private let lock = NSLock() | ||
| private var isDirty = false | ||
| private var observer: AnyObject? | ||
|
|
||
| private static let maxCount = 1000 | ||
| private static let diskURL: URL = { | ||
| let caches = FileManager.default.urls(for: .cachesDirectory, in: .userDomainMask)[0] | ||
| return caches.appendingPathComponent("saliency_cache.json") | ||
| }() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if it's worth saving the cache to disc. It should not be too bad to recalculate again on the next launch, right? Also, the disk cache will only be usable if the same images appear again in the Reader feed. |
||
|
|
||
| init() { | ||
| observer = NotificationCenter.default.addObserver( | ||
| forName: UIApplication.didEnterBackgroundNotification, | ||
| object: nil, | ||
| queue: .main | ||
| ) { [weak self] _ in | ||
| guard let self else { return } | ||
| Task.detached(priority: .utility) { self.saveToDisk() } | ||
| } | ||
| } | ||
|
|
||
| deinit { | ||
| if let observer { NotificationCenter.default.removeObserver(observer) } | ||
| } | ||
|
|
||
| func isCached(for url: URL) -> Bool { | ||
| lock.withLock { store[url.absoluteString] != nil } | ||
| } | ||
|
|
||
| func cachedRect(for url: URL) -> CGRect? { | ||
| lock.withLock { store[url.absoluteString] ?? nil } | ||
| } | ||
|
|
||
| func store(_ rect: CGRect?, for url: URL) { | ||
| lock.withLock { | ||
| let key = url.absoluteString | ||
| store.updateValue(rect, forKey: key) | ||
| if store.count > Self.maxCount, let oldest = store.keys.first { | ||
| store.removeValue(forKey: oldest) | ||
| } | ||
| isDirty = true | ||
| } | ||
| } | ||
|
|
||
| func loadFromDisk() { | ||
| guard let data = try? Data(contentsOf: Self.diskURL), | ||
| let decoded = try? JSONDecoder().decode([String: CGRect?].self, from: data) else { | ||
| return | ||
| } | ||
| lock.withLock { | ||
| store = OrderedDictionary(uniqueKeysWithValues: decoded.map { ($0.key, $0.value) }) | ||
| } | ||
| } | ||
|
|
||
| func saveToDisk() { | ||
| let snapshot: OrderedDictionary<String, CGRect?>? = lock.withLock { | ||
| guard isDirty else { return nil } | ||
| isDirty = false | ||
| return store | ||
| } | ||
| guard let snapshot else { return } | ||
| let dict = snapshot.reduce(into: [String: CGRect?]()) { $0[$1.key] = $1.value } | ||
| guard let data = try? JSONEncoder().encode(dict) else { return } | ||
| try? data.write(to: Self.diskURL, options: .atomic) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,25 @@ public final class AsyncImageView: UIView { | |
| private var spinner: UIActivityIndicatorView? | ||
| private let controller = ImageLoadingController() | ||
|
|
||
| // MARK: - Saliency | ||
|
|
||
| /// When enabled, detects the most visually interesting region of portrait images | ||
| /// and adjusts the crop so that region appears near the top of the container. | ||
| public var isSaliencyDetectionEnabled = false | ||
|
|
||
| /// When `true`, saliency detection only runs for images whose height exceeds their | ||
| /// width (portrait images). Landscape and square images are displayed immediately | ||
| /// without blocking on detection. Default is `true`. | ||
| public var isSaliencyPortraitOnly = true | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: These two properties can be merged into one property of |
||
|
|
||
| private var currentImageURL: URL? | ||
| private var saliencyTask: Task<Void, Never>? | ||
| private var saliencyRect: CGRect? { | ||
| didSet { setNeedsLayout() } | ||
| } | ||
|
|
||
| // MARK: - Configuration | ||
|
|
||
| public enum LoadingStyle { | ||
| /// Shows a secondary background color during the download. | ||
| case background | ||
|
|
@@ -63,25 +82,31 @@ public final class AsyncImageView: UIView { | |
| controller.onStateChanged = { [weak self] in self?.setState($0) } | ||
|
|
||
| addSubview(imageView) | ||
| imageView.translatesAutoresizingMaskIntoConstraints = false | ||
| NSLayoutConstraint.activate([ | ||
| imageView.topAnchor.constraint(equalTo: topAnchor), | ||
| imageView.trailingAnchor.constraint(equalTo: trailingAnchor), | ||
| imageView.bottomAnchor.constraint(equalTo: bottomAnchor), | ||
| imageView.leadingAnchor.constraint(equalTo: leadingAnchor), | ||
| ]) | ||
| imageView.translatesAutoresizingMaskIntoConstraints = true | ||
| imageView.autoresizingMask = [] | ||
| imageView.frame = bounds | ||
|
|
||
| imageView.clipsToBounds = true | ||
| imageView.contentMode = .scaleAspectFill | ||
| imageView.accessibilityIgnoresInvertColors = true | ||
|
|
||
| clipsToBounds = true | ||
| backgroundColor = .secondarySystemBackground | ||
| } | ||
|
|
||
| public override func layoutSubviews() { | ||
| super.layoutSubviews() | ||
| imageView.frame = saliencyAdjustedFrame() | ||
| } | ||
|
|
||
| /// Removes the current image and stops the outstanding downloads. | ||
| public func prepareForReuse() { | ||
| controller.prepareForReuse() | ||
| image = nil | ||
| saliencyRect = nil | ||
| currentImageURL = nil | ||
| saliencyTask?.cancel() | ||
| saliencyTask = nil | ||
| } | ||
|
|
||
| /// - parameter size: Target image size in pixels. | ||
|
|
@@ -90,11 +115,13 @@ public final class AsyncImageView: UIView { | |
| host: MediaHostProtocol? = nil, | ||
| size: ImageSize? = nil | ||
| ) { | ||
| currentImageURL = imageURL | ||
| let request = ImageRequest(url: imageURL, host: host, options: ImageRequestOptions(size: size)) | ||
| controller.setImage(with: request) | ||
| } | ||
|
|
||
| public func setImage(with request: ImageRequest, completion: (@MainActor (Result<UIImage, Error>) -> Void)? = nil) { | ||
| currentImageURL = request.source.url | ||
| controller.setImage(with: request, completion: completion) | ||
| } | ||
|
|
||
|
|
@@ -113,15 +140,54 @@ public final class AsyncImageView: UIView { | |
| } | ||
| case .success(let image): | ||
| self.image = image | ||
| imageView.isHidden = false | ||
| backgroundColor = .clear | ||
| let needsDetection = isSaliencyDetectionEnabled | ||
| && !(isSaliencyPortraitOnly && image.size.width >= image.size.height) | ||
| if needsDetection, let url = currentImageURL { | ||
| if let cached = ImageSaliencyService.shared.cachedSaliencyRect(for: url) { | ||
| saliencyRect = cached | ||
| imageView.isHidden = false | ||
| backgroundColor = .clear | ||
| } else { | ||
| triggerSaliencyDetection(image: image, url: url) | ||
| } | ||
| } else { | ||
| imageView.isHidden = false | ||
| backgroundColor = .clear | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably should call |
||
| case .failure: | ||
| if configuration.isErrorViewEnabled { | ||
| makeErrorView().isHidden = false | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private func triggerSaliencyDetection(image: UIImage, url: URL) { | ||
| saliencyTask = Task { @MainActor [weak self] in | ||
| guard let self else { return } | ||
| let rect = await ImageSaliencyService.shared.saliencyRect(for: image, url: url) | ||
| guard !Task.isCancelled else { return } | ||
| // Reveal the image only after saliency detection finishes (with or without a result). | ||
| self.saliencyRect = rect | ||
| self.imageView.isHidden = false | ||
| self.backgroundColor = .clear | ||
| } | ||
| } | ||
|
|
||
| // MARK: - Frame Calculation | ||
|
|
||
| private func saliencyAdjustedFrame() -> CGRect { | ||
| guard isSaliencyDetectionEnabled, let image, let saliencyRect else { | ||
| return bounds | ||
| } | ||
| return ImageSaliencyService.shared.adjustedFrame( | ||
| saliencyRect: saliencyRect, | ||
| imageSize: image.size, | ||
| in: bounds.size | ||
| ) ?? bounds | ||
| } | ||
|
|
||
| // MARK: - Helpers | ||
|
|
||
| private func didUpdateConfiguration(_ configuration: Configuration) { | ||
| if let tintColor = configuration.tintColor { | ||
| imageView.tintColor = tintColor | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
SaliencyDetector.detectlooks like a pure function. Does this type need to be an actor?