From 62183b90eb487ecd612ff4ee1296434630a88d10 Mon Sep 17 00:00:00 2001 From: Nishant Karajgikar Date: Tue, 27 Apr 2021 11:51:18 -0400 Subject: [PATCH] Cleanup --- .../MapboxMaps/Foundation/BaseMapView.swift | 1 + .../Camera/BasicCameraAnimator.swift | 11 +- .../Foundation/Camera/CameraManager.swift | 139 +++++++++--------- .../Foundation/Camera/CameraTransition.swift | 16 +- .../Camera/CameraTransitionTests.swift | 7 +- .../Camera/FlyToAnimatorTests.swift | 2 +- 6 files changed, 90 insertions(+), 86 deletions(-) diff --git a/Sources/MapboxMaps/Foundation/BaseMapView.swift b/Sources/MapboxMaps/Foundation/BaseMapView.swift index e13910f7941..b0a70290e3d 100644 --- a/Sources/MapboxMaps/Foundation/BaseMapView.swift +++ b/Sources/MapboxMaps/Foundation/BaseMapView.swift @@ -6,6 +6,7 @@ import Turf // swiftlint:disable file_length internal typealias PendingAnimationCompletion = (completion: AnimationCompletion, animatingPosition: UIViewAnimatingPosition) +// swiftlint:disable force_cast internal class WeakCameraAnimatorSet { private let hashTable = NSHashTable.weakObjects() diff --git a/Sources/MapboxMaps/Foundation/Camera/BasicCameraAnimator.swift b/Sources/MapboxMaps/Foundation/Camera/BasicCameraAnimator.swift index b25208859a3..a404cdceebf 100644 --- a/Sources/MapboxMaps/Foundation/Camera/BasicCameraAnimator.swift +++ b/Sources/MapboxMaps/Foundation/Camera/BasicCameraAnimator.swift @@ -80,7 +80,7 @@ public class BasicCameraAnimator: NSObject, CameraAnimator, CameraAnimatorInterf // Set up the short lived camera view delegate.addViewToViewHeirarchy(cameraView) - var cameraTransition = CameraTransition(with: delegate.camera, initialAnchor: delegate.anchorAfterPadding()) + var cameraTransition = CameraTransition(cameraOptions: delegate.camera, initialAnchor: delegate.anchorAfterPadding()) animation(&cameraTransition) propertyAnimator.addAnimations { [weak cameraView] in @@ -98,7 +98,7 @@ public class BasicCameraAnimator: NSObject, CameraAnimator, CameraAnimatorInterf /// Starts the animation after a delay /// - Parameter delay: Delay (in seconds) after which the animation should start public func startAnimation(afterDelay delay: TimeInterval) { - delayedAnimationTimer = Timer.scheduledTimer(withTimeInterval: delay, repeats: false, block: { [weak self] (timer) in + delayedAnimationTimer = Timer.scheduledTimer(withTimeInterval: delay, repeats: false, block: { [weak self] (_) in self?.startAnimation() }) } @@ -127,10 +127,9 @@ public class BasicCameraAnimator: NSObject, CameraAnimator, CameraAnimatorInterf internal func wrapCompletion(_ completion: @escaping AnimationCompletion) -> (UIViewAnimatingPosition) -> Void { return { [weak self] animationPosition in - guard let self = self, let delegate = self.delegate else { return } - self.transition = nil // Clear out the transition being animated by this animator, - // since the animation is complete if we are here. - delegate.schedulePendingCompletion(forAnimator: self, completion: completion, animatingPosition: animationPosition) + guard let self = self else { return } + self.transition = nil // Clear out the transition being animated by this animator since the animation is complete if we are here. + self.delegate?.schedulePendingCompletion(forAnimator: self, completion: completion, animatingPosition: animationPosition) // Invalidate the delayed animation timer if it exists self.delayedAnimationTimer?.invalidate() diff --git a/Sources/MapboxMaps/Foundation/Camera/CameraManager.swift b/Sources/MapboxMaps/Foundation/Camera/CameraManager.swift index d8d53ee6375..1dc15d609c5 100644 --- a/Sources/MapboxMaps/Foundation/Camera/CameraManager.swift +++ b/Sources/MapboxMaps/Foundation/Camera/CameraManager.swift @@ -2,10 +2,10 @@ import UIKit import Turf public protocol CameraAnimator { - + /// Stops the animation in its tracks and calls any provided completion func stopAnimation() - + /// The current state of the animation var state: UIViewAnimatingState { get } } @@ -15,13 +15,12 @@ internal protocol CameraAnimatorInterface: CameraAnimator { var currentCameraOptions: CameraOptions? { get } } - /// An object that manages a camera's view lifecycle. public class CameraManager { - + /// Used to set up camera specific configuration public internal(set) var mapCameraOptions: MapCameraOptions - + /// Used to update the map's camera options and pass them to the core Map. internal func updateMapCameraOptions(newOptions: MapCameraOptions) { let boundOptions = BoundOptions(__bounds: newOptions.restrictedCoordinateBounds ?? nil, @@ -32,22 +31,22 @@ public class CameraManager { mapView?.mapboxMap.__map.setBoundsFor(boundOptions) mapCameraOptions = newOptions } - + /// Internal camera animator used for animated transition internal var internalAnimator: CameraAnimator? - + /// May want to convert to an enum. fileprivate let northBearing: CGFloat = 0 - + internal weak var mapView: BaseMapView? - + public init(for mapView: BaseMapView, with mapCameraOptions: MapCameraOptions) { self.mapView = mapView self.mapCameraOptions = mapCameraOptions } - + // MARK: Camera creation - + /// Creates a new `Camera` object to fit a given array of coordinates. /// /// Note: This method does not set the map's camera to the new values. You must call @@ -58,22 +57,22 @@ public class CameraManager { assertionFailure("MapView is nil.") return CameraOptions() } - + let coordinateLocations = coordinates.map { CLLocation(latitude: $0.latitude, longitude: $0.longitude) } - + // Construct new camera options with current values let cameraOptions = MapboxCoreMaps.CameraOptions(mapView.cameraOptions) - + let defaultEdgeInsets = EdgeInsets(top: 0, left: 0, bottom: 0, right: 0) - + // Create a new camera options with adjusted values return CameraOptions(mapView.mapboxMap.__map.cameraForCoordinates( - forCoordinates: coordinateLocations, - padding: cameraOptions.__padding ?? defaultEdgeInsets, - bearing: cameraOptions.__bearing, - pitch: cameraOptions.__pitch)) + forCoordinates: coordinateLocations, + padding: cameraOptions.__padding ?? defaultEdgeInsets, + bearing: cameraOptions.__bearing, + pitch: cameraOptions.__pitch)) } - + /// Returns the camera that best fits the given coordinate bounds, with optional edge padding, bearing, and pitch values. /// - Parameters: /// - coordinateBounds: The coordinate bounds that will be displayed within the viewport. @@ -88,14 +87,14 @@ public class CameraManager { guard let mapView = mapView else { return CameraOptions() } - + return CameraOptions(mapView.mapboxMap.__map.cameraForCoordinateBounds( - for: coordinateBounds, - padding: edgePadding.toMBXEdgeInsetsValue(), - bearing: NSNumber(value: Float(bearing)), - pitch: NSNumber(value: Float(pitch)))) + for: coordinateBounds, + padding: edgePadding.toMBXEdgeInsetsValue(), + bearing: NSNumber(value: Float(bearing)), + pitch: NSNumber(value: Float(pitch)))) } - + /// Returns the camera that best fits the given geometry, with optional edge padding, bearing, and pitch values. /// - Parameters: /// - geometry: The geoemtry that will be displayed within the viewport. @@ -111,14 +110,14 @@ public class CameraManager { assertionFailure("MapView is nil.") return CameraOptions() } - + return CameraOptions(mapView.mapboxMap.__map.cameraForGeometry( - for: MBXGeometry(geometry: geometry), - padding: edgePadding.toMBXEdgeInsetsValue(), - bearing: NSNumber(value: Float(bearing)), - pitch: NSNumber(value: Float(pitch)))) + for: MBXGeometry(geometry: geometry), + padding: edgePadding.toMBXEdgeInsetsValue(), + bearing: NSNumber(value: Float(bearing)), + pitch: NSNumber(value: Float(pitch)))) } - + /// Returns the coordinate bounds for a given `Camera` object's viewport. /// - Parameter camera: The camera that the coordinate bounds will be returned for. /// - Returns: `CoordinateBounds` for the given `Camera` @@ -129,9 +128,9 @@ public class CameraManager { } return mapView.mapboxMap.__map.coordinateBoundsForCamera(forCamera: MapboxCoreMaps.CameraOptions(camera)) } - + // MARK: Setting a new camera - + /// Transition the map's viewport to a new camera. /// - Parameters: /// - targetCamera: The target camera to transition to. @@ -146,21 +145,21 @@ public class CameraManager { assertionFailure("MapView is nil.") return } - + internalAnimator?.stopAnimation() - + let clampedCamera = CameraOptions(center: targetCamera.center, padding: targetCamera.padding, anchor: targetCamera.anchor, zoom: targetCamera.zoom?.clamped(to: mapCameraOptions.minimumZoomLevel...mapCameraOptions.maximumZoomLevel), bearing: targetCamera.bearing, pitch: targetCamera.pitch?.clamped(to: mapCameraOptions.minimumPitch...mapCameraOptions.maximumPitch)) - + // Return early if the cameraView's camera is already at `clampedCamera` guard mapView.cameraOptions != clampedCamera else { return } - + if animated && duration > 0 { let animation = { (transition: inout CameraTransition) in transition.center.toValue = clampedCamera.center @@ -175,7 +174,7 @@ public class CameraManager { mapView.mapboxMap.updateCamera(with: clampedCamera) } } - + /// Interrupts all `active` animation. /// The camera remains at the last point before the cancel request was invoked, i.e., /// the camera is not reset or fast-forwarded to the end of the transition. @@ -186,36 +185,38 @@ public class CameraManager { animator.stopAnimation() } } - + /// Private function to perform camera animation /// - Parameters: /// - duration: If animated, how long the animation takes /// - animation: closure to perform /// - completion: animation block called on completion - fileprivate func performCameraAnimation(duration: TimeInterval, animation: @escaping (inout CameraTransition) -> Void, completion: ((UIViewAnimatingPosition) -> Void)? = nil) { - + fileprivate func performCameraAnimation(duration: TimeInterval, + animation: @escaping (inout CameraTransition) -> Void, + completion: ((UIViewAnimatingPosition) -> Void)? = nil) { + // Stop previously running animations internalAnimator?.stopAnimation() - + // Make a new camera animator for the new properties - + let cameraAnimator = makeAnimator(duration: duration, - curve: .easeOut, - animationOwner: .custom(id: "com.mapbox.maps.cameraManager"), - animations: animation) - + curve: .easeOut, + animationOwner: .custom(id: "com.mapbox.maps.cameraManager"), + animations: animation) + // Add completion - cameraAnimator.addCompletion({ [weak self] (position) in + cameraAnimator.addCompletion({ (position) in completion?(position) }) - + // Start animation cameraAnimator.startAnimation() - + // Store the animator in order to keep it alive internalAnimator = cameraAnimator } - + /// Moves the viewpoint to a different location using a transition animation that /// evokes powered flight and an optional transition duration and timing function /// It seamlessly incorporates zooming and panning to help @@ -230,7 +231,7 @@ public class CameraManager { public func fly(to camera: CameraOptions, duration: TimeInterval? = nil, completion: AnimationCompletion? = nil) -> CameraAnimator? { - + guard let mapView = mapView, let flyToAnimator = FlyToCameraAnimator( inital: mapView.cameraOptions, @@ -242,23 +243,23 @@ public class CameraManager { Log.warning(forMessage: "Unable to start fly-to animation", category: "CameraManager") return nil } - + // Stop the `internalAnimator` before beginning a `flyTo` internalAnimator?.stopAnimation() - + mapView.addCameraAnimator(flyToAnimator) - + // Nil out the internalAnimator after `flyTo` finishes - flyToAnimator.addCompletion { [weak self] (position) in + flyToAnimator.addCompletion { (position) in // Call the developer-provided completion (if present) completion?(position) } - + flyToAnimator.startAnimation() internalAnimator = flyToAnimator return internalAnimator } - + /// Ease the camera to a destination /// - Parameters: /// - camera: the target camera after animation @@ -266,10 +267,12 @@ public class CameraManager { /// - completion: completion to be called after animation /// - Returns: An instance of `CameraAnimatorProtocol` which can be interrupted if necessary @discardableResult - public func ease(to camera: CameraOptions, duration: TimeInterval, completion: AnimationCompletion? = nil) -> CameraAnimator? { - + public func ease(to camera: CameraOptions, + duration: TimeInterval, + completion: AnimationCompletion? = nil) -> CameraAnimator? { + internalAnimator?.stopAnimation() - + let animator = makeAnimator(duration: duration, curve: .easeInOut) { (transition) in transition.center.toValue = camera.center transition.padding.toValue = camera.padding @@ -278,25 +281,25 @@ public class CameraManager { transition.bearing.toValue = camera.bearing transition.pitch.toValue = camera.pitch } - + // Nil out the `internalAnimator` once the "ease to" finishes - animator.addCompletion { [weak self] (position) in + animator.addCompletion { (position) in completion?(position) } - + animator.startAnimation() internalAnimator = animator - + return internalAnimator } - + } fileprivate extension CoordinateBounds { func contains(_ coordinates: [CLLocationCoordinate2D]) -> Bool { let latitudeRange = southwest.latitude...northeast.latitude let longitudeRange = southwest.longitude...northeast.longitude - + for coordinate in coordinates { if latitudeRange.contains(coordinate.latitude) || longitudeRange.contains(coordinate.longitude) { return true diff --git a/Sources/MapboxMaps/Foundation/Camera/CameraTransition.swift b/Sources/MapboxMaps/Foundation/Camera/CameraTransition.swift index 6ba156ffbb3..bbf8cb52553 100644 --- a/Sources/MapboxMaps/Foundation/Camera/CameraTransition.swift +++ b/Sources/MapboxMaps/Foundation/Camera/CameraTransition.swift @@ -38,14 +38,14 @@ public struct CameraTransition { } } - internal init(with renderedCameraOptions: CameraOptions, initialAnchor: CGPoint) { - - guard let renderedCenter = renderedCameraOptions.center, - let renderedZoom = renderedCameraOptions.zoom, - let renderedPadding = renderedCameraOptions.padding, - let renderedPitch = renderedCameraOptions.pitch, - let renderedBearing = renderedCameraOptions.bearing else { - fatalError("Values in rendered CameraOptions cannot be nil") + internal init(cameraOptions: CameraOptions, initialAnchor: CGPoint) { + + guard let renderedCenter = cameraOptions.center, + let renderedZoom = cameraOptions.zoom, + let renderedPadding = cameraOptions.padding, + let renderedPitch = cameraOptions.pitch, + let renderedBearing = cameraOptions.bearing else { + fatalError("Values in CameraOptions cannot be nil") } center = Change(fromValue: renderedCenter) diff --git a/Tests/MapboxMapsTests/Foundation/Camera/CameraTransitionTests.swift b/Tests/MapboxMapsTests/Foundation/Camera/CameraTransitionTests.swift index 571eb592cef..2a86a87364d 100644 --- a/Tests/MapboxMapsTests/Foundation/Camera/CameraTransitionTests.swift +++ b/Tests/MapboxMapsTests/Foundation/Camera/CameraTransitionTests.swift @@ -3,8 +3,9 @@ import XCTest class CameraTransitionTests: XCTestCase { - var cameraTransition = CameraTransition(with: cameraOptionsTestValue, - initialAnchor: .zero) + var cameraTransition = CameraTransition( + cameraOptions: cameraOptionsTestValue, + initialAnchor: .zero) func testOptimizeBearingClockwise() { let startBearing = 0.0 @@ -55,7 +56,7 @@ class CameraTransitionTests: XCTestCase { optimizedBearing = cameraTransition.optimizedBearingToValue XCTAssertEqual(optimizedBearing, 90) } - + func testOptimizeBearingWhenStartBearingIsNegativeAndIsLesserThanMinus360() { var optimizedBearing: CLLocationDirection? diff --git a/Tests/MapboxMapsTests/Foundation/Camera/FlyToAnimatorTests.swift b/Tests/MapboxMapsTests/Foundation/Camera/FlyToAnimatorTests.swift index b2c41db7492..b43fc5e393c 100644 --- a/Tests/MapboxMapsTests/Foundation/Camera/FlyToAnimatorTests.swift +++ b/Tests/MapboxMapsTests/Foundation/Camera/FlyToAnimatorTests.swift @@ -25,7 +25,7 @@ final class FlyToAnimatorTests: XCTestCase { let duration: TimeInterval = 10 var flyToAnimator: FlyToCameraAnimator! - var cameraAnimatorDelegate: CameraAnimatorDelegateMock! + weak var cameraAnimatorDelegate: CameraAnimatorDelegateMock! override func setUp() { super.setUp()