Skip to content

Commit

Permalink
Bugfix/mapsios 538 bearing is incorrect when rotate (#1396)
Browse files Browse the repository at this point in the history
* Correct PanZoomPinchBehavior drag camera options.

* Correct PanPinchBehavior drag camera options
  • Loading branch information
maios committed Mar 15, 2023
1 parent 48b6be7 commit 7c52ddc
Show file tree
Hide file tree
Showing 10 changed files with 21 additions and 83 deletions.
@@ -1,28 +1,20 @@
internal final class PanPinchBehavior: PinchBehavior {
private let initialCameraState: CameraState
private let initialPinchMidpoint: CGPoint
private var currentPinchMidpoint: CGPoint
private let mapboxMap: MapboxMapProtocol

internal init(initialCameraState: CameraState,
initialPinchMidpoint: CGPoint,
mapboxMap: MapboxMapProtocol) {
self.initialCameraState = initialCameraState
self.initialPinchMidpoint = initialPinchMidpoint
internal init(initialPinchMidpoint: CGPoint, mapboxMap: MapboxMapProtocol) {
self.currentPinchMidpoint = initialPinchMidpoint
self.mapboxMap = mapboxMap
}

internal func update(pinchMidpoint: CGPoint, pinchScale: CGFloat) {
mapboxMap.performWithoutNotifying {
mapboxMap.setCamera(to: CameraOptions(
center: initialCameraState.center,
bearing: initialCameraState.bearing))
}

mapboxMap.dragStart(for: initialPinchMidpoint)
mapboxMap.dragStart(for: currentPinchMidpoint)
let dragOptions = mapboxMap.dragCameraOptions(
from: initialPinchMidpoint,
from: currentPinchMidpoint,
to: pinchMidpoint)
mapboxMap.setCamera(to: dragOptions)
mapboxMap.dragEnd()

currentPinchMidpoint = pinchMidpoint
}
}
Expand Up @@ -2,28 +2,22 @@ import CoreGraphics

internal final class PanZoomPinchBehavior: PinchBehavior {
private let initialCameraState: CameraState
private let initialPinchMidpoint: CGPoint
private var currentPinchMidpoint: CGPoint
private let mapboxMap: MapboxMapProtocol

internal init(initialCameraState: CameraState,
initialPinchMidpoint: CGPoint,
mapboxMap: MapboxMapProtocol) {
self.initialCameraState = initialCameraState
self.initialPinchMidpoint = initialPinchMidpoint
self.currentPinchMidpoint = initialPinchMidpoint
self.mapboxMap = mapboxMap
}

internal func update(pinchMidpoint: CGPoint, pinchScale: CGFloat) {
mapboxMap.performWithoutNotifying {
mapboxMap.setCamera(
to: CameraOptions(
center: initialCameraState.center,
zoom: initialCameraState.zoom,
bearing: initialCameraState.bearing))

mapboxMap.dragStart(for: initialPinchMidpoint)
mapboxMap.dragStart(for: currentPinchMidpoint)
let dragOptions = mapboxMap.dragCameraOptions(
from: initialPinchMidpoint,
from: currentPinchMidpoint,
to: pinchMidpoint)
mapboxMap.setCamera(to: dragOptions)
mapboxMap.dragEnd()
Expand All @@ -33,5 +27,6 @@ internal final class PanZoomPinchBehavior: PinchBehavior {
mapboxMap.setCamera(to: CameraOptions(
anchor: pinchMidpoint,
zoom: initialCameraState.zoom + zoomIncrement))
currentPinchMidpoint = pinchMidpoint
}
}
Expand Up @@ -30,7 +30,6 @@ internal final class PinchBehaviorProvider: PinchBehaviorProviderProtocol {
mapboxMap: mapboxMap)
case (true, false):
return PanPinchBehavior(
initialCameraState: initialCameraState,
initialPinchMidpoint: initialPinchMidpoint,
mapboxMap: mapboxMap)
case (false, true):
Expand Down
@@ -1,10 +1,6 @@
import UIKit
@_implementationOnly import MapboxCommon_Private

internal protocol PinchGestureHandlerDelegate: GestureHandlerDelegate {
func pinchGestureHandlerDidUpdateGesture(_ handler: PinchGestureHandlerProtocol)
}

internal protocol PinchGestureHandlerProtocol: FocusableGestureHandlerProtocol {
var zoomEnabled: Bool { get set }
var panEnabled: Bool { get set }
Expand Down Expand Up @@ -76,10 +72,6 @@ internal final class PinchGestureHandler: GestureHandler, PinchGestureHandlerPro
pinchBehavior.update(
pinchMidpoint: gestureRecognizer.location(in: view),
pinchScale: gestureRecognizer.scale)

if let pinchDelegate = (delegate as? PinchGestureHandlerDelegate) {
pinchDelegate.pinchGestureHandlerDidUpdateGesture(self)
}
} else {
start(with: gestureRecognizer)
}
Expand Down
8 changes: 0 additions & 8 deletions Sources/MapboxMaps/Gestures/GestureManager.swift
Expand Up @@ -198,11 +198,3 @@ public final class GestureManager: GestureHandlerDelegate {
delegate?.gestureManager(self, didEndAnimatingFor: gestureType)
}
}

extension GestureManager: PinchGestureHandlerDelegate {
func pinchGestureHandlerDidUpdateGesture(_ handler: PinchGestureHandlerProtocol) {
// Because of a bug in core maps camera state has to be reset before updating pinch drag.
// This call will make sure that bearing is set to correct value after pinch dragging.
rotateGestureHandler.scheduleRotationUpdateIfNeeded()
}
}
@@ -1,6 +1,6 @@
@testable import MapboxMaps

final class MockGestureHandlerDelegate: GestureHandlerDelegate, PinchGestureHandlerDelegate {
final class MockGestureHandlerDelegate: GestureHandlerDelegate {
let gestureBeganStub = Stub<GestureType, Void>()
func gestureBegan(for gestureType: GestureType) {
gestureBeganStub.call(with: gestureType)
Expand All @@ -19,9 +19,4 @@ final class MockGestureHandlerDelegate: GestureHandlerDelegate, PinchGestureHand
func animationEnded(for gestureType: GestureType) {
animationEndedStub.call(with: gestureType)
}

let pinchGestureHandlerDidUpdateGestureStub = Stub<PinchGestureHandlerProtocol, Void>()
func pinchGestureHandlerDidUpdateGesture(_ handler: PinchGestureHandlerProtocol) {
pinchGestureHandlerDidUpdateGestureStub.call(with: handler)
}
}
Expand Up @@ -5,7 +5,6 @@ final class PanPinchBehaviorTests: BasePinchBehaviorTests {
override func setUp() {
super.setUp()
behavior = PanPinchBehavior(
initialCameraState: initialCameraState,
initialPinchMidpoint: initialPinchMidpoint,
mapboxMap: mapboxMap)
}
Expand All @@ -17,17 +16,12 @@ final class PanPinchBehaviorTests: BasePinchBehaviorTests {

behavior.update(pinchMidpoint: pinchMidpoint, pinchScale: .random(in: 0..<2))

// verify camera gets set twice
guard mapboxMap.setCameraStub.invocations.count == 2 else {
// verify camera gets set once
guard mapboxMap.setCameraStub.invocations.count == 1 else {
XCTFail("Did not receive the expected number of setCamera invocations.")
return
}

// verify that the first set camera invocation resets the center
XCTAssertEqual(
mapboxMap.setCameraStub.invocations[0].parameters,
CameraOptions(center: initialCameraState.center, bearing: initialCameraState.bearing))

// verify that dragStart is called once with initial midpoint
XCTAssertEqual(
mapboxMap.dragStartStub.invocations.map(\.parameters),
Expand All @@ -41,7 +35,7 @@ final class PanPinchBehaviorTests: BasePinchBehaviorTests {
// verify that setCamera is invoked a second time with the return value
// from dragCameraOptions
XCTAssertEqual(
mapboxMap.setCameraStub.invocations[1].parameters,
mapboxMap.setCameraStub.invocations[0].parameters,
dragCameraOptions)

// verify drag end is invoked once
Expand Down
Expand Up @@ -18,20 +18,12 @@ final class PanZoomPinchBehaviorTests: BasePinchBehaviorTests {

behavior.update(pinchMidpoint: pinchMidpoint, pinchScale: pinchScale)

// verify that setCamera is invoked 3 times
guard mapboxMap.setCameraStub.invocations.count == 3 else {
// verify that setCamera is invoked 2 times
guard mapboxMap.setCameraStub.invocations.count == 2 else {
XCTFail("Did not receive the expected number of setCamera invocations.")
return
}

// verify that the first setCamera invocation resets center and zoom
XCTAssertEqual(
mapboxMap.setCameraStub.invocations[0].parameters,
CameraOptions(
center: initialCameraState.center,
zoom: initialCameraState.zoom,
bearing: initialCameraState.bearing))

// verify that dragStart is invoked once with the initial pinch midpoint
XCTAssertEqual(
mapboxMap.dragStartStub.invocations.map(\.parameters),
Expand All @@ -46,7 +38,7 @@ final class PanZoomPinchBehaviorTests: BasePinchBehaviorTests {
// verify that setCamera is invoked a second time with the return value
// from dragCameraOptions
XCTAssertEqual(
mapboxMap.setCameraStub.invocations[1].parameters,
mapboxMap.setCameraStub.invocations[0].parameters,
dragCameraOptions)

// verify that dragEnd is invoked once
Expand All @@ -55,9 +47,9 @@ final class PanZoomPinchBehaviorTests: BasePinchBehaviorTests {
// verify that setCamera is invoked a third time with the expected
// anchor and zoom
XCTAssertEqual(
mapboxMap.setCameraStub.invocations[2].parameters.anchor,
mapboxMap.setCameraStub.invocations[1].parameters.anchor,
pinchMidpoint)
let zoom = try XCTUnwrap(mapboxMap.setCameraStub.invocations[2].parameters.zoom)
let zoom = try XCTUnwrap(mapboxMap.setCameraStub.invocations[1].parameters.zoom)
XCTAssertEqual(zoom, initialCameraState.zoom + log2(pinchScale))

// verify that only one camera changed notification was emitted
Expand Down
Expand Up @@ -237,11 +237,4 @@ final class PinchGestureHandlerTests: XCTestCase {

XCTAssertFalse(shouldRecognizeSimultaneously)
}

func testPinchUpdateShouldNotifyDelegate() {
sendActions(with: .began, numberOfTouches: 2)
sendActions(with: .changed, numberOfTouches: 2)

XCTAssertEqual(delegate.pinchGestureHandlerDidUpdateGestureStub.invocations.count, 1)
}
}
6 changes: 0 additions & 6 deletions Tests/MapboxMapsTests/Gestures/GestureManagerTests.swift
Expand Up @@ -544,10 +544,4 @@ final class GestureManagerTests: XCTestCase {
gestureManager.gestureEnded(for: .pinch, willAnimate: false)
XCTAssertEqual(delegate.gestureDidEndStub.invocations.count, 1)
}

func testRotationUpdateScheduledAfterPinchUpdate() {
gestureManager.pinchGestureHandlerDidUpdateGesture(pinchGestureHandler)

XCTAssertEqual(rotateGestureHandler.scheduleRotationUpdateIfNeededStub.invocations.count, 1)
}
}

0 comments on commit 7c52ddc

Please sign in to comment.