Prevent complete animators from causing drawing

.swiftpm/xcode/xcshareddata/xcschemes/MapboxMaps.xcscheme

@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <Scheme
-   LastUpgradeVersion = "1240"
+   LastUpgradeVersion = "1250"
    version = "1.7">
    <BuildAction
       parallelizeBuildables = "YES"

Apps/DebugApp/DebugApp.xcodeproj/project.pbxproj

@@ -227,7 +227,7 @@
 			isa = PBXProject;
 			attributes = {
 				LastSwiftUpdateCheck = 1100;
-				LastUpgradeCheck = 1240;
+				LastUpgradeCheck = 1250;
 				ORGANIZATIONNAME = "Mapbox Inc";
 				TargetAttributes = {
 					35F08DD72347987700768B9F = {

Apps/DebugApp/DebugApp.xcodeproj/xcshareddata/xcschemes/DebugApp.xcscheme

@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <Scheme
-   LastUpgradeVersion = "1240"
+   LastUpgradeVersion = "1250"
    version = "1.3">
    <BuildAction
       parallelizeBuildables = "YES"

Apps/Examples/Examples.xcodeproj/project.pbxproj

@@ -441,7 +441,7 @@
 			isa = PBXProject;
 			attributes = {
 				LastSwiftUpdateCheck = 1200;
-				LastUpgradeCheck = 1240;
+				LastUpgradeCheck = 1250;
 				TargetAttributes = {
 					077C4EE9252F7E88007636F1 = {
 						CreatedOnToolsVersion = 12.0;

Apps/Examples/Examples.xcodeproj/xcshareddata/xcschemes/Examples.xcscheme

@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <Scheme
-   LastUpgradeVersion = "1240"
+   LastUpgradeVersion = "1250"
    version = "1.3">
    <BuildAction
       parallelizeBuildables = "YES"

Apps/StressTest/StressTest.xcodeproj/xcshareddata/xcschemes/StressTest.xcscheme

@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <Scheme
-   LastUpgradeVersion = "1240"
+   LastUpgradeVersion = "1250"
    version = "1.3">
    <BuildAction
       parallelizeBuildables = "YES"

CHANGELOG.md

@@ -7,6 +7,8 @@ Mapbox welcomes participation and contributions from everyone.
 ### Bug fixes 🐞
 
 * Fixed a crash at runtime, when an application is built with the xcframework (direct download) rather than from source (i.e. Swift Package Manager). ([#497](https://github.com/mapbox/mapbox-maps-ios/pull/497))
+* Fixed an issue where animators created by fly to and ease to were not released until the next fly to or ease to began. ([#505](https://github.com/mapbox/mapbox-maps-ios/pull/505))
+* Fixed an issue where a complete animator would trigger redrawing unnecessarily. ([#505](https://github.com/mapbox/mapbox-maps-ios/pull/505))
 
 ## 10.0.0-rc.2 - June 23, 2021
 

Package.swift

@@ -23,15 +23,15 @@ let package = Package(
             name: "MapboxMaps",
             dependencies: ["MapboxCoreMaps", "Turf", "MapboxMobileEvents", "MapboxCommon"],
             exclude: [
-                "MapView/Info.plist"
+                "Info.plist"
             ]
         ),
         .testTarget(
             name: "MapboxMapsTests",
             dependencies: ["MapboxMaps"],
             exclude: [
-                "MapView/Info.plist",
-                "MapView/Integration Tests/HTTP/HTTPIntegrationTests.swift",
+                "Info.plist",
+                "Foundation/Integration Tests/HTTP/HTTPIntegrationTests.swift",
             ],
             resources: [
                 .copy("Foundation/GeoJSON/Fixtures/point.geojson"),

Sources/MapboxMaps/Foundation/Camera/CameraAnimationsManager+CameraAnimatorDelegate.swift

@@ -103,8 +103,10 @@ extension CameraAnimationsManager: CameraAnimatorDelegate {
 
     // MARK: CameraAnimatorDelegate functions
     func schedulePendingCompletion(forAnimator animator: CameraAnimator, completion: @escaping AnimationCompletion, animatingPosition: UIViewAnimatingPosition) {
-        guard let mapView = mapView else { return }
-        mapView.pendingAnimatorCompletionBlocks.append((completion, animatingPosition))
+        mapView?.pendingAnimatorCompletionBlocks.append(
+            PendingAnimationCompletion(
+                completion: completion,
+                animatingPosition: animatingPosition))
     }
 
     var camera: CameraState {

Sources/MapboxMaps/Foundation/Camera/CameraAnimationsManager.swift

@@ -94,7 +94,12 @@ public class CameraAnimationsManager {
 
         mapView.addCameraAnimator(flyToAnimator)
 
-        flyToAnimator.addCompletion { (position) in
+        flyToAnimator.addCompletion { [weak self, weak flyToAnimator] (position) in
+            if let internalAnimator = self?.internalAnimator,
+               let animator = flyToAnimator,
+               internalAnimator === animator {
+                self?.internalAnimator = nil
+            }
             // Call the developer-provided completion (if present)
             completion?(position)
         }
@@ -128,7 +133,12 @@ public class CameraAnimationsManager {
         }
 
         // Nil out the `internalAnimator` once the "ease to" finishes
-        animator.addCompletion { (position) in
+        animator.addCompletion { [weak self, weak animator] (position) in
+            if let internalAnimator = self?.internalAnimator,
+               let animator = animator,
+               internalAnimator === animator {
+                self?.internalAnimator = nil
+            }
             completion?(position)
         }
 

Sources/MapboxMaps/Foundation/DisplayLinkProtocol.swift

@@ -0,0 +1,9 @@
+import QuartzCore
+
+internal protocol DisplayLinkProtocol: AnyObject {
+    var preferredFramesPerSecond: Int { get set }
+    func add(to runloop: RunLoop, forMode mode: RunLoop.Mode)
+    func invalidate()
+}
+
+extension CADisplayLink: DisplayLinkProtocol {}

Sources/MapboxMaps/Foundation/ForwardingDispalyLinkTarget.swift

@@ -0,0 +1,14 @@
+import QuartzCore
+
+internal class ForwardingDisplayLinkTarget: NSObject {
+    private let handler: (CADisplayLink) -> Void
+
+    internal init(handler: @escaping (CADisplayLink) -> Void) {
+        self.handler = handler
+        super.init()
+    }
+
+    @objc internal func update(with displayLink: CADisplayLink) {
+        handler(displayLink)
+    }
+}

Sources/MapboxMaps/Foundation/MapView.swift

@@ -6,8 +6,6 @@
 import UIKit
 import Turf
 
-internal typealias PendingAnimationCompletion = (completion: AnimationCompletion, animatingPosition: UIViewAnimatingPosition)
-
 open class MapView: UIView {
 
     // mapbox map depends on MapInitOptions, which is not available until
@@ -90,7 +88,7 @@ open class MapView: UIView {
     private var needsDisplayRefresh: Bool = false
     private var dormant: Bool = false
     private var displayCallback: (() -> Void)?
-    @objc dynamic internal var displayLink: CADisplayLink?
+    private var displayLink: DisplayLinkProtocol?
 
     /// Holding onto this value that comes from `MapOptions` since there is a race condition between
     /// getting a `MetalView`, and intializing a `MapView`
@@ -102,6 +100,8 @@ open class MapView: UIView {
     /// a nib.
     @IBOutlet internal private(set) weak var mapInitOptionsProvider: MapInitOptionsProvider?
 
+    private let dependencyProvider: MapViewDependencyProviderProtocol
+
     /// The preferred frames per second used for map rendering
     public var preferredFramesPerSecond: PreferredFPS = .maximum {
         didSet {
@@ -132,6 +132,20 @@ open class MapView: UIView {
     ///   - mapInitOptions: `MapInitOptions`; default uses
     ///    `ResourceOptionsManager.default` to retrieve a shared default resource option, including the access token.
     public init(frame: CGRect, mapInitOptions: MapInitOptions = MapInitOptions()) {
+        dependencyProvider = MapViewDependencyProvider()
+        super.init(frame: frame)
+        commonInit(mapInitOptions: mapInitOptions, overridingStyleURI: nil)
+    }
+
+    required public init?(coder: NSCoder) {
+        dependencyProvider = MapViewDependencyProvider()
+        super.init(coder: coder)
+    }
+
+    internal init(frame: CGRect,
+                  mapInitOptions: MapInitOptions,
+                  dependencyProvider: MapViewDependencyProviderProtocol) {
+        self.dependencyProvider = dependencyProvider
         super.init(frame: frame)
         commonInit(mapInitOptions: mapInitOptions, overridingStyleURI: nil)
     }
@@ -265,44 +279,41 @@ open class MapView: UIView {
         mapboxMap.size = bounds.size
     }
 
-    func validateDisplayLink() {
-        if superview != nil
-            && window != nil
-            && displayLink == nil {
-            let target = BaseMapViewProxy(mapView: self)
-            displayLink = window?.screen.displayLink(withTarget: target, selector: #selector(target.updateFromDisplayLink))
-
+    private func validateDisplayLink() {
+        if let window = window, displayLink == nil {
+            displayLink = dependencyProvider.makeDisplayLink(
+                window: window,
+                target: ForwardingDisplayLinkTarget { [weak self] in
+                    self?.updateFromDisplayLink(displayLink: $0)
+                },
+                selector: #selector(ForwardingDisplayLinkTarget.update(with:)))
             updateDisplayLinkPreferredFramesPerSecond()
             displayLink?.add(to: .current, forMode: .common)
-
         }
     }
 
-    @objc func updateFromDisplayLink(displayLink: CADisplayLink) {
+    private func updateFromDisplayLink(displayLink: CADisplayLink) {
         if window == nil {
             return
         }
 
-        if needsDisplayRefresh
-            || cameraAnimatorsSet.allObjects.count > 0
-            || !pendingAnimatorCompletionBlocks.isEmpty {
-            needsDisplayRefresh = false
-
-            for animator in cameraAnimatorsSet.allObjects {
-                if let cameraOptions = animator.currentCameraOptions {
-                    mapboxMap.setCamera(to: cameraOptions)
-                }
+        for animator in cameraAnimatorsSet.allObjects {
+            if let cameraOptions = animator.currentCameraOptions {
+                mapboxMap.setCamera(to: cameraOptions)
             }
+        }
 
-            /// This executes the series of scheduled animation completion blocks and also removes them from the list
-            while !pendingAnimatorCompletionBlocks.isEmpty {
-                let pendingCompletion = pendingAnimatorCompletionBlocks.removeFirst()
-                let completion = pendingCompletion.completion
-                let animatingPosition = pendingCompletion.animatingPosition
-                completion(animatingPosition)
-            }
+        /// This executes the series of scheduled animation completion blocks and also removes them from the list
+        while !pendingAnimatorCompletionBlocks.isEmpty {
+            let pendingCompletion = pendingAnimatorCompletionBlocks.removeFirst()
+            let completion = pendingCompletion.completion
+            let animatingPosition = pendingCompletion.animatingPosition
+            completion(animatingPosition)
+        }
 
-            self.displayCallback?()
+        if needsDisplayRefresh {
+            needsDisplayRefresh = false
+            displayCallback?()
         }
     }
 
@@ -353,8 +364,17 @@ open class MapView: UIView {
         eventsListener.push(event: .memoryWarning)
     }
 
-    required public init?(coder: NSCoder) {
-        super.init(coder: coder)
+    // MARK: Telemetry
+
+    private func setUpTelemetryLogging() {
+        guard let validResourceOptions = resourceOptions else { return }
+        let eventsListener = EventsManager(accessToken: validResourceOptions.accessToken)
+
+        DispatchQueue.main.async {
+            eventsListener.push(event: .map(event: .loaded))
+        }
+
+        self.eventsListener = eventsListener
     }
 }
 
@@ -368,7 +388,7 @@ extension MapView: DelegatingMapClientDelegate {
     }
 
     internal func getMetalView(for metalDevice: MTLDevice?) -> MTKView? {
-        let metalView = MTKView(frame: frame, device: metalDevice)
+        let metalView = dependencyProvider.makeMetalView(frame: frame, device: metalDevice)
         displayCallback = {
             metalView.setNeedsDisplay()
         }
@@ -389,30 +409,3 @@ extension MapView: DelegatingMapClientDelegate {
         return metalView
     }
 }
-
-private class BaseMapViewProxy: NSObject {
-    weak var mapView: MapView?
-
-    init(mapView: MapView) {
-        self.mapView = mapView
-        super.init()
-    }
-
-    @objc func updateFromDisplayLink(displayLink: CADisplayLink) {
-        mapView?.updateFromDisplayLink(displayLink: displayLink)
-    }
-}
-
-// MARK: Telemetry
-extension MapView {
-    internal func setUpTelemetryLogging() {
-        guard let validResourceOptions = resourceOptions else { return }
-        let eventsListener = EventsManager(accessToken: validResourceOptions.accessToken)
-
-        DispatchQueue.main.async {
-            eventsListener.push(event: .map(event: .loaded))
-        }
-
-        self.eventsListener = eventsListener
-    }
-}

Sources/MapboxMaps/Foundation/MapViewDependencyProvider.swift

@@ -0,0 +1,16 @@
+import MetalKit
+
+internal protocol MapViewDependencyProviderProtocol {
+    func makeMetalView(frame: CGRect, device: MTLDevice?) -> MTKView
+    func makeDisplayLink(window: UIWindow, target: Any, selector: Selector) -> DisplayLinkProtocol?
+}
+
+internal final class MapViewDependencyProvider: MapViewDependencyProviderProtocol {
+    func makeMetalView(frame: CGRect, device: MTLDevice?) -> MTKView {
+        MTKView(frame: frame, device: device)
+    }
+
+    func makeDisplayLink(window: UIWindow, target: Any, selector: Selector) -> DisplayLinkProtocol? {
+        window.screen.displayLink(withTarget: target, selector: selector)
+    }
+}

Sources/MapboxMaps/Foundation/PendingAnimationCompletion.swift

@@ -0,0 +1,6 @@
+import UIKit
+
+internal struct PendingAnimationCompletion {
+    var completion: AnimationCompletion
+    var animatingPosition: UIViewAnimatingPosition
+}

Tests/MapboxMapsTests/MapView/Integration Tests/Map/MapViewIntegrationTests.swift → Tests/MapboxMapsTests/Foundation/Integration Tests/Map/MapViewIntegrationTests.swift

@@ -2,7 +2,7 @@ import XCTest
 @testable import MapboxMaps
 import MapboxCoreMaps
 
-class MapViewIntegrationTests: IntegrationTestCase {
+final class MapViewIntegrationTests: IntegrationTestCase {
     var rootView: UIView!
     var mapView: MapView!
     var dataPathURL: URL!
@@ -77,46 +77,6 @@ class MapViewIntegrationTests: IntegrationTestCase {
         XCTAssertNil(weakMapView)
     }
 
-    func testUpdatePreferredFPS() {
-        let originalFPS = mapView.preferredFramesPerSecond
-        XCTAssertNotNil(originalFPS)
-        XCTAssertEqual(originalFPS.rawValue, 0)
-
-        let newFPS = 12
-        mapView.preferredFramesPerSecond = PreferredFPS(rawValue: newFPS)
-        XCTAssertNotEqual(originalFPS, mapView.preferredFramesPerSecond)
-        XCTAssertEqual(mapView.preferredFramesPerSecond.rawValue, newFPS)
-    }
-
-    func testUpdateFromDisplayLink() {
-        let originalFPS = mapView.preferredFramesPerSecond
-        XCTAssertNotNil(mapView.displayLink)
-        mapView.preferredFramesPerSecond = .lowPower
-        XCTAssertNotEqual(originalFPS, mapView.preferredFramesPerSecond)
-        XCTAssertEqual(mapView.preferredFramesPerSecond.rawValue, mapView.displayLink?.preferredFramesPerSecond)
-    }
-
-    func testAnimatorCompletionBlocksAreRemoved() {
-        let firstCompletion = PendingAnimationCompletion(completion: {_ in}, animatingPosition: .end)
-        let secondCompletion = PendingAnimationCompletion(completion: {_ in}, animatingPosition: .current)
-
-        mapView.pendingAnimatorCompletionBlocks.append(firstCompletion)
-        mapView.pendingAnimatorCompletionBlocks.append(secondCompletion)
-        mapView.scheduleRepaint()
-        XCTAssertEqual(mapView.pendingAnimatorCompletionBlocks.count, 2)
-
-        mapView.updateFromDisplayLink(displayLink: CADisplayLink())
-        XCTAssertEqual(mapView.pendingAnimatorCompletionBlocks.count, 0)
-    }
-
-    func testUpdateFromDisplayLinkWhenNil() {
-        mapView.displayLink = nil
-        mapView.preferredFramesPerSecond = .maximum
-
-        XCTAssertNil(mapView.displayLink?.preferredFramesPerSecond)
-        XCTAssertNotEqual(mapView.preferredFramesPerSecond.rawValue, mapView.displayLink?.preferredFramesPerSecond)
-    }
-
     func testDataClearing() {
         defer {
             mapView?.removeFromSuperview()