From d7750aec90053ee54d193579666f99a01fd3dd19 Mon Sep 17 00:00:00 2001 From: Allen Humphreys Date: Sat, 11 Mar 2023 11:57:29 -0500 Subject: [PATCH 1/3] Share knowledge on maintaining SwiftUI frames --- Sources/Annotations/MKMapAnnotationView.swift | 40 ++++++++++++++++++- Sources/Annotations/MapAnnotation.swift | 1 + Sources/Annotations/MapMarker.swift | 3 ++ Sources/Annotations/MapPin.swift | 3 ++ Sources/Annotations/ViewMapAnnotation.swift | 21 +++++++++- Sources/Map/Map+Coordinator.swift | 11 +++++ 6 files changed, 75 insertions(+), 4 deletions(-) diff --git a/Sources/Annotations/MKMapAnnotationView.swift b/Sources/Annotations/MKMapAnnotationView.swift index f65d9ab..865988a 100644 --- a/Sources/Annotations/MKMapAnnotationView.swift +++ b/Sources/Annotations/MKMapAnnotationView.swift @@ -15,17 +15,40 @@ class MKMapAnnotationView: MKAnnotationView { // MARK: Stored Properties private var controller: NativeHostingController? + private var mapAnnotation: ViewMapAnnotation? // MARK: Methods + func update(for mapAnnotation: ViewMapAnnotation) { + controller?.rootView = mapAnnotation.content + if #available(iOS 16.0, *) { + anchorPoint = mapAnnotation.anchorPoint + } else { + // Fallback on earlier versions + } + setNeedsLayout() + } + func setup(for mapAnnotation: ViewMapAnnotation) { annotation = mapAnnotation.annotation clusteringIdentifier = mapAnnotation.clusteringIdentifier + self.mapAnnotation = mapAnnotation + } + + private func addContentIfNeeded() { + guard let mapAnnotation else { return } + guard controller == nil else { return } let controller = NativeHostingController(rootView: mapAnnotation.content) + controller.view.backgroundColor = .clear addSubview(controller.view) - bounds.size = controller.preferredContentSize + bounds.size = controller.sizeThatFits(in: .init(width: CGFloat.greatestFiniteMagnitude, height: CGFloat.greatestFiniteMagnitude)) self.controller = controller + if #available(iOS 16.0, *) { + anchorPoint = mapAnnotation.anchorPoint + } else { + // Fallback on earlier versions + } } // MARK: Overrides @@ -35,8 +58,19 @@ class MKMapAnnotationView: MKAnnotationView { override func layoutSubviews() { super.layoutSubviews() + // Deferring the addition of the SwiftUI hosting view until the layout pass + // Seems to guarantee more accurate sizes on the first view update + addContentIfNeeded() + if let controller = controller { - bounds.size = controller.preferredContentSize + bounds.size = controller.sizeThatFits(in: .init(width: CGFloat.greatestFiniteMagnitude, height: CGFloat.greatestFiniteMagnitude)) + // Setting the frame to zero than immediately back triggers + // The SwiftUI frame to correctly follow the hosting view's frame in + // The map's coordinate space. I think SwiftUI cannot correctly hook into the parent + // view's coordinate updates because MKMapView moves the MKAnnotationView's around in + // non-standard ways. + controller.view.frame = .zero + controller.view.frame = bounds } } @@ -51,6 +85,8 @@ class MKMapAnnotationView: MKAnnotationView { controller?.view.removeFromSuperview() controller?.removeFromParent() controller = nil + mapAnnotation = nil + annotation = nil } } diff --git a/Sources/Annotations/MapAnnotation.swift b/Sources/Annotations/MapAnnotation.swift index d49e2f7..80e377f 100644 --- a/Sources/Annotations/MapAnnotation.swift +++ b/Sources/Annotations/MapAnnotation.swift @@ -33,6 +33,7 @@ public protocol MapAnnotation { func view(for mapView: MKMapView) -> MKAnnotationView? + func updateView(with associatedAnnotation: Any) } extension MapAnnotation { diff --git a/Sources/Annotations/MapMarker.swift b/Sources/Annotations/MapMarker.swift index df612bc..318abbb 100644 --- a/Sources/Annotations/MapMarker.swift +++ b/Sources/Annotations/MapMarker.swift @@ -79,6 +79,9 @@ extension MapMarker: MapAnnotation { return view } + public func updateView(with associatedAnnotation: Any) { + // Only necessary for hosting SwiftUI views + } } #endif diff --git a/Sources/Annotations/MapPin.swift b/Sources/Annotations/MapPin.swift index a67670e..4c7bb3d 100644 --- a/Sources/Annotations/MapPin.swift +++ b/Sources/Annotations/MapPin.swift @@ -100,6 +100,9 @@ extension MapPin: MapAnnotation { return view } + public func updateView(with associatedAnnotation: Any) { + // Only necessary for hosting SwiftUI views + } } #endif diff --git a/Sources/Annotations/ViewMapAnnotation.swift b/Sources/Annotations/ViewMapAnnotation.swift index 38fc01b..c382b4c 100644 --- a/Sources/Annotations/ViewMapAnnotation.swift +++ b/Sources/Annotations/ViewMapAnnotation.swift @@ -10,7 +10,7 @@ import MapKit import SwiftUI -public struct ViewMapAnnotation: MapAnnotation { +public final class ViewMapAnnotation: MapAnnotation { // MARK: Nested Types @@ -42,28 +42,37 @@ public struct ViewMapAnnotation: MapAnnotation { public let annotation: MKAnnotation let clusteringIdentifier: String? - let content: Content + private (set) var content: Content + // The associated view last returned when requested via `view(for:)` + // Must be a weak reference because the view will also have a reference to + // this instance + private weak var associatedView: MKMapAnnotationView? + let anchorPoint: CGPoint // MARK: Initialization public init( coordinate: CLLocationCoordinate2D, + anchorPoint: CGPoint = CGPoint(x: 0.5, y: 0.5), title: String? = nil, subtitle: String? = nil, clusteringIdentifier: String? = nil, @ViewBuilder content: () -> Content ) { self.annotation = Annotation(coordinate: coordinate, title: title, subtitle: subtitle) + self.anchorPoint = anchorPoint self.clusteringIdentifier = clusteringIdentifier self.content = content() } public init( annotation: MKAnnotation, + anchorPoint: CGPoint = CGPoint(x: 0.5, y: 0.5), clusteringIdentifier: String? = nil, @ViewBuilder content: () -> Content ) { self.annotation = annotation + self.anchorPoint = anchorPoint self.clusteringIdentifier = clusteringIdentifier self.content = content() } @@ -77,9 +86,17 @@ public struct ViewMapAnnotation: MapAnnotation { ) as? MKMapAnnotationView view?.setup(for: self) + associatedView = view return view } + public func updateView(with associatedAnnotation: Any) { + guard let updatedAnnotation = associatedAnnotation as? ViewMapAnnotation else { return } + guard let associatedView else { return } + + content = updatedAnnotation.content + associatedView.update(for: self) + } } #endif diff --git a/Sources/Map/Map+Coordinator.swift b/Sources/Map/Map+Coordinator.swift index ec2c6c0..a5c2399 100644 --- a/Sources/Map/Map+Coordinator.swift +++ b/Sources/Map/Map+Coordinator.swift @@ -75,8 +75,10 @@ extension Map { private func updateAnnotations(on mapView: MKMapView, from previousView: Map?, to newView: Map) { let changes: CollectionDifference + var updates: [AnnotationItems.Element] = [] if let previousView = previousView { changes = newView.annotationItems.difference(from: previousView.annotationItems) { $0.id == $1.id } + updates = newView.annotationItems.filter { newItem in previousView.annotationItems.contains { newItem.id == $0.id } } } else { changes = newView.annotationItems.difference(from: []) { $0.id == $1.id } } @@ -111,6 +113,15 @@ extension Map { annotationContentByID.removeValue(forKey: item.id) } } + + // For SwiftUI, we need to forward the updated content to annotations so that values captured + // in the view builder can be updated. This is necessary for views that take inputs to update + for updatedItem in updates { + guard let oldAnnotationContent = annotationContentByID[updatedItem.id] else { continue } + + // This is necessarily the best architecture long term, but it gets the job done + oldAnnotationContent.updateView(with: newView.annotationContent(updatedItem)) + } } private func updateCamera(on mapView: MKMapView, context: Context, animated: Bool) { From 0cbd7942aee2a83d25e145b6181c0b5aeaea734b Mon Sep 17 00:00:00 2001 From: Allen Humphreys Date: Sun, 12 Mar 2023 17:54:59 -0400 Subject: [PATCH 2/3] Remove the view reference cycle for updating SwiftUI content --- Sources/Annotations/MKMapAnnotationView.swift | 10 ++++++++-- Sources/Annotations/MapAnnotation.swift | 5 ++++- Sources/Annotations/MapMarker.swift | 3 --- Sources/Annotations/MapPin.swift | 3 --- Sources/Annotations/ViewMapAnnotation.swift | 16 ++-------------- Sources/Map/Map+Coordinator.swift | 7 +++++-- 6 files changed, 19 insertions(+), 25 deletions(-) diff --git a/Sources/Annotations/MKMapAnnotationView.swift b/Sources/Annotations/MKMapAnnotationView.swift index 865988a..8a4196c 100644 --- a/Sources/Annotations/MKMapAnnotationView.swift +++ b/Sources/Annotations/MKMapAnnotationView.swift @@ -10,7 +10,7 @@ import MapKit import SwiftUI -class MKMapAnnotationView: MKAnnotationView { +class MKMapAnnotationView: MKAnnotationView, UpdatableAnnotationView { // MARK: Stored Properties @@ -19,7 +19,13 @@ class MKMapAnnotationView: MKAnnotationView { // MARK: Methods - func update(for mapAnnotation: ViewMapAnnotation) { + func update(with mapAnnotation: MapAnnotation) { + guard let mapAnnotation = mapAnnotation as? ViewMapAnnotation else { + assertionFailure("Attempting to update an MKMapAnnotationView with an incompatible type") + return + } + self.mapAnnotation = mapAnnotation + controller?.rootView = mapAnnotation.content if #available(iOS 16.0, *) { anchorPoint = mapAnnotation.anchorPoint diff --git a/Sources/Annotations/MapAnnotation.swift b/Sources/Annotations/MapAnnotation.swift index 80e377f..7937603 100644 --- a/Sources/Annotations/MapAnnotation.swift +++ b/Sources/Annotations/MapAnnotation.swift @@ -33,7 +33,10 @@ public protocol MapAnnotation { func view(for mapView: MKMapView) -> MKAnnotationView? - func updateView(with associatedAnnotation: Any) +} + +protocol UpdatableAnnotationView { + func update(with associatedAnnotation: MapAnnotation) } extension MapAnnotation { diff --git a/Sources/Annotations/MapMarker.swift b/Sources/Annotations/MapMarker.swift index 318abbb..df612bc 100644 --- a/Sources/Annotations/MapMarker.swift +++ b/Sources/Annotations/MapMarker.swift @@ -79,9 +79,6 @@ extension MapMarker: MapAnnotation { return view } - public func updateView(with associatedAnnotation: Any) { - // Only necessary for hosting SwiftUI views - } } #endif diff --git a/Sources/Annotations/MapPin.swift b/Sources/Annotations/MapPin.swift index 4c7bb3d..a67670e 100644 --- a/Sources/Annotations/MapPin.swift +++ b/Sources/Annotations/MapPin.swift @@ -100,9 +100,6 @@ extension MapPin: MapAnnotation { return view } - public func updateView(with associatedAnnotation: Any) { - // Only necessary for hosting SwiftUI views - } } #endif diff --git a/Sources/Annotations/ViewMapAnnotation.swift b/Sources/Annotations/ViewMapAnnotation.swift index c382b4c..9f62cff 100644 --- a/Sources/Annotations/ViewMapAnnotation.swift +++ b/Sources/Annotations/ViewMapAnnotation.swift @@ -10,7 +10,7 @@ import MapKit import SwiftUI -public final class ViewMapAnnotation: MapAnnotation { +public struct ViewMapAnnotation: MapAnnotation { // MARK: Nested Types @@ -42,11 +42,7 @@ public final class ViewMapAnnotation: MapAnnotation { public let annotation: MKAnnotation let clusteringIdentifier: String? - private (set) var content: Content - // The associated view last returned when requested via `view(for:)` - // Must be a weak reference because the view will also have a reference to - // this instance - private weak var associatedView: MKMapAnnotationView? + let content: Content let anchorPoint: CGPoint // MARK: Initialization @@ -86,17 +82,9 @@ public final class ViewMapAnnotation: MapAnnotation { ) as? MKMapAnnotationView view?.setup(for: self) - associatedView = view return view } - public func updateView(with associatedAnnotation: Any) { - guard let updatedAnnotation = associatedAnnotation as? ViewMapAnnotation else { return } - guard let associatedView else { return } - - content = updatedAnnotation.content - associatedView.update(for: self) - } } #endif diff --git a/Sources/Map/Map+Coordinator.swift b/Sources/Map/Map+Coordinator.swift index a5c2399..267b72f 100644 --- a/Sources/Map/Map+Coordinator.swift +++ b/Sources/Map/Map+Coordinator.swift @@ -119,8 +119,11 @@ extension Map { for updatedItem in updates { guard let oldAnnotationContent = annotationContentByID[updatedItem.id] else { continue } - // This is necessarily the best architecture long term, but it gets the job done - oldAnnotationContent.updateView(with: newView.annotationContent(updatedItem)) + // This isn't necessarily the best architecture long term, but it gets the job done + guard let currentView = mapView.view(for: oldAnnotationContent.annotation) as? UpdatableAnnotationView else { + continue + } + currentView.update(with: newView.annotationContent(updatedItem)) } } From 366382e898a87aea6d6ecf8906b6a3a8897b15b4 Mon Sep 17 00:00:00 2001 From: Allen Humphreys Date: Mon, 13 Mar 2023 11:51:52 -0400 Subject: [PATCH 3/3] Support anchorPoint pre-iOS16 --- Sources/Annotations/MKMapAnnotationView.swift | 62 +++++++++++++------ 1 file changed, 44 insertions(+), 18 deletions(-) diff --git a/Sources/Annotations/MKMapAnnotationView.swift b/Sources/Annotations/MKMapAnnotationView.swift index 8a4196c..95a213b 100644 --- a/Sources/Annotations/MKMapAnnotationView.swift +++ b/Sources/Annotations/MKMapAnnotationView.swift @@ -16,6 +16,8 @@ class MKMapAnnotationView: MKAnnotationView, UpdatableAnnotationV private var controller: NativeHostingController? private var mapAnnotation: ViewMapAnnotation? + private let deferAddingContentForPreviews: Bool = false + private let colorizeFramesForDebugging: Bool = false // MARK: Methods @@ -30,7 +32,7 @@ class MKMapAnnotationView: MKAnnotationView, UpdatableAnnotationV if #available(iOS 16.0, *) { anchorPoint = mapAnnotation.anchorPoint } else { - // Fallback on earlier versions + centerOffset = convertToCenterOffset(mapAnnotation.anchorPoint, in: bounds) } setNeedsLayout() } @@ -39,6 +41,10 @@ class MKMapAnnotationView: MKAnnotationView, UpdatableAnnotationV annotation = mapAnnotation.annotation clusteringIdentifier = mapAnnotation.clusteringIdentifier self.mapAnnotation = mapAnnotation + + if !deferAddingContentForPreviews { + addContentIfNeeded() + } } private func addContentIfNeeded() { @@ -46,17 +52,35 @@ class MKMapAnnotationView: MKAnnotationView, UpdatableAnnotationV guard controller == nil else { return } let controller = NativeHostingController(rootView: mapAnnotation.content) - controller.view.backgroundColor = .clear - addSubview(controller.view) - bounds.size = controller.sizeThatFits(in: .init(width: CGFloat.greatestFiniteMagnitude, height: CGFloat.greatestFiniteMagnitude)) self.controller = controller + + if colorizeFramesForDebugging { + backgroundColor = .red + controller.view.backgroundColor = .green + } else { + controller.view.backgroundColor = .clear + } + frame.size = controller.view.intrinsicContentSize + addSubview(controller.view) + controller.view.frame = bounds + if #available(iOS 16.0, *) { anchorPoint = mapAnnotation.anchorPoint } else { - // Fallback on earlier versions + centerOffset = convertToCenterOffset(mapAnnotation.anchorPoint, in: bounds) } } + func convertToCenterOffset(_ anchorPoint: CGPoint, in rect: CGRect) -> CGPoint { + assert((0.0...1.0).contains(anchorPoint.x), "Valid anchor point range is 0.0 to 1.0, received x value: \(anchorPoint.x)") + assert((0.0...1.0).contains(anchorPoint.y), "Valid anchor point range is 0.0 to 1.0, received y value: \(anchorPoint.y)") + + return .init( + x: (0.5 - anchorPoint.x) * rect.width, + y: (0.5 - anchorPoint.y) * rect.height + ) + } + // MARK: Overrides #if canImport(UIKit) @@ -64,20 +88,22 @@ class MKMapAnnotationView: MKAnnotationView, UpdatableAnnotationV override func layoutSubviews() { super.layoutSubviews() - // Deferring the addition of the SwiftUI hosting view until the layout pass - // Seems to guarantee more accurate sizes on the first view update - addContentIfNeeded() - - if let controller = controller { - bounds.size = controller.sizeThatFits(in: .init(width: CGFloat.greatestFiniteMagnitude, height: CGFloat.greatestFiniteMagnitude)) - // Setting the frame to zero than immediately back triggers - // The SwiftUI frame to correctly follow the hosting view's frame in - // The map's coordinate space. I think SwiftUI cannot correctly hook into the parent - // view's coordinate updates because MKMapView moves the MKAnnotationView's around in - // non-standard ways. - controller.view.frame = .zero - controller.view.frame = bounds + // In previews, the height of the annotation view ends up being too big, I have no idea why there's a difference, but + // when run on device it's not noticeable + if deferAddingContentForPreviews && frame.origin != .zero { + addContentIfNeeded() } + + guard let controller = controller else { return } + + bounds.size = controller.view.intrinsicContentSize + // Setting the frame to zero than immediately back triggers + // The SwiftUI frame to correctly follow the hosting view's frame in + // The map's coordinate space. I think SwiftUI cannot correctly hook into the parent + // view's coordinate updates because MKMapView moves the MKAnnotationView's around in + // non-standard ways. + controller.view.frame = .zero + controller.view.frame = bounds } #endif