Rotation gesture threshold and simultaneousRotateAndPinchToZoomEnabled option #1429

evil159 · 2022-07-05T14:03:48Z

This PR adds filtering to rotation gesture handling - the map won't start rotating unless rotation magnitude/velocity exceeds certain level/combination of the two. This is done to prevent the map from being rotated accidentally while being zoomed.

Along this change rotation handling was delegated to a dedicated handler/recogniser which led to GestureOptions.pinchRotateEnabled being deprecated in favour of GestureOptions.rotateEnabled(this also brings Android and iOS closer in terms of gesture options API).

Additionally this PR adds an option to disable simultaneous rotation and pinch zooming GestureOptions.simultaneousRotateAndPinchToZoomEnabled.

Before	After
RPReplay_Final1657028966.MP4	RPReplay_Final1657029054.MP4

Pull request checklist:

Write tests for all new functionality. If tests were not written, please explain why.
Add documentation comments for any added or updated public APIs.
Describe the changes in this PR, especially public API changes.
Add a changelog entry to to bottom of the relevant section (typically the ## main heading near the top).
Review and agree to the Contributor License Agreement (CLA).

Sources/MapboxMaps/Gestures/GestureHandlers/PinchGestureHandler.swift

Sources/MapboxMaps/Gestures/GestureHandlers/RotateGestureHandler.swift

Chaoba · 2022-07-06T11:30:27Z

Sources/MapboxMaps/Gestures/GestureHandlers/RotateGestureHandler.swift

+
+     private func shouldStartRotating(with velocity: CGFloat, deltaSinceStart: CGFloat) -> Bool {
+         let deltaSinceStartInDegrees = deltaSinceStart.toDegrees()
+         let velocityInDegreesPerMillisecond = abs(velocity) * 0.057295779513082


Please make this magic number a const variable.

Converted to formula 👍

Chaoba

LGTM

OdNairy

There is a rotation issue during my tests. The attached video is 1/3 playback speed.

Rotation.junk.-.x1.3.slow.mp4

OdNairy · 2022-07-11T09:39:06Z

Sources/MapboxMaps/Gestures/GestureHandlers/RotateGestureHandler.swift

+             mapboxMap.setCamera(
+                 to: CameraOptions(
+                    anchor: focalPoint ?? midpoint,
+                    bearing: (initialBearing + rotationInDegrees).truncatingRemainder(dividingBy: 360.0))


Looks like we can use wrapped here:

Suggested change

bearing: (initialBearing + rotationInDegrees).truncatingRemainder(dividingBy: 360.0))

bearing: (initialBearing + rotationInDegrees).wrapped(to: 0..<(2 * .pi)))

Changed it to (initialBearing + rotationInDegrees).wrapped(to: 0..<360)

OdNairy · 2022-07-11T09:39:50Z

Sources/MapboxMaps/Gestures/GestureHandlers/RotateGestureHandler.swift

+             isMapRotating = true
+             // pretend to be pinch gesture for backwards compatibility
+             delegate?.gestureBegan(for: .pinch)


Why do we ignore the began state for this event/flag?

Delegate is notified when we actually start/end rotating the map, in order to get raw gesture recogniser events one would need to subscribe to the gesture recogniser directly, e.g.:
mapView.gestures.rotateGestureRecognizer.addTarget(self, action: #selector(handleRotationUpdate(_:)))

Sources/MapboxMaps/Gestures/GestureHandlers/RotateGestureHandler.swift

… correct anchor

…d option (#1429)

* Update changelog (#1418) * Add style toggle to location example (#1413) * [XCParty] Add tag name if HEAD has any (#1424) * [XCParty] Export crash reports for XCResult (#1434) * Expose a way to initialise TilesetDescriptorOptionsForTilesets (#1431) * Add cast shadow option to 3d puck config (#1435) * Mark camera functions as unsupported for globe projection, invocation results in a no-op (#1440) * Rotation gesture threshold and simultaneousRotateAndPinchToZoomEnabled option (#1429) * Add option to support measurements from the only last run (#1445) * Update examples (#1443) * Fix flaky test (#1450) * Fix view annotation losing its feature association after update (#1446) * Enable `modelCastsShadow` option for custom location puck config (#1447) * enabled model casts shadows option for custom location puck configuration * Provide default value for list of last-run metrics (#1453) * Update CoreMaps to `10.7.0-rc.1` (#1456) * Update CoreMaps to 10.7.0-rc.1 * Update changelog * Update changelog header (#1459) * Update 10.7.0 versions (#1460) Co-authored-by: ZiZi <44972592+ZiZasaurus@users.noreply.github.com> Co-authored-by: Roman Laitarenko <roman.laitarenko@mapbox.com> Co-authored-by: Tobrun <tobrun.van.nuland@gmail.com>

* Add style toggle to location example (#1413) * [XCParty] Add tag name if HEAD has any (#1424) * [XCParty] Export crash reports for XCResult (#1434) * Expose a way to initialise TilesetDescriptorOptionsForTilesets (#1431) * Add cast shadow option to 3d puck config (#1435) * Mark camera functions as unsupported for globe projection, invocation results in a no-op (#1440) * Rotation gesture threshold and simultaneousRotateAndPinchToZoomEnabled option (#1429) * Add option to support measurements from the only last run (#1445) * Update examples (#1443) * Fix flaky test (#1450) * Fix view annotation losing its feature association after update (#1446) * Enable `modelCastsShadow` option for custom location puck config (#1447) * enabled model casts shadows option for custom location puck configuration * Provide default value for list of last-run metrics (#1453) * Update CoreMaps to `10.7.0-rc.1` (#1456) * Update CoreMaps to 10.7.0-rc.1 * Update changelog * Update changelog header (#1459) * Update 10.7.0 versions (#1460) Co-authored-by: ZiZi <44972592+ZiZasaurus@users.noreply.github.com> Co-authored-by: Roman Laitarenko <roman.laitarenko@mapbox.com> Co-authored-by: Tobrun <tobrun.van.nuland@gmail.com>

* Update TestLab devices list (#1429) * Use iPhone 8 iOS 16.2 on TestLab (#1432) * Replace iPhone 8 iOS 16.2 with iPhone 11 Pro iOS 16.2 (#1434)

evil159 requested review from Chaoba and kiryldz July 5, 2022 14:03

evil159 requested a review from a team as a code owner July 5, 2022 14:03

evil159 added 20 commits July 5, 2022 17:07

draft

4aac153

sddf

f882e64

sdf

1298605

Add RotateGestureHandler

5e77479

Remove pinch rotation handling

8c9b18b

Fix unit tests

3a94023

Use simultaneousRotateAndPinchZoomEnabled

53d4d33

Focal point support for rotate gesture

8eb01de

Fix rotate gesture not being reset

5009b14

Disable rotation gesture recogniser when rotate is disabled

0d06e84

Notify gesture delegate about rotate gesture events

53b2ae2

Fix unit test build errors

2914ea0

Small tweaks to rotate handler for more consistent behavior

ac7ce02

Add unit test for rotate gesture handling

498da4e

Fix rotate gesture handler notifying about end of non-started gesture

aefcb64

Add more tests

0b8ce05

Fix some tests

d39af51

Fix lint errors

a736064

More lint errors fixed

2d8490c

Add changelog entry

1b3170d

evil159 force-pushed the rl/MAPSIOS-210_rotation_v1.5 branch from 63d90ef to 1b3170d Compare July 5, 2022 14:13

Make the compiler happy

3898903

evil159 mentioned this pull request Jul 6, 2022

Expose and modify the rotation threshold #755

Closed

Chaoba reviewed Jul 6, 2022

View reviewed changes

evil159 added 3 commits July 7, 2022 13:33

Add a comment why rotation gesture output is flipped

66a57c7

Convert angular velocity with a formula instead of constant

0b45d7f

Merge branch 'main' into rl/MAPSIOS-210_rotation_v1.5

3888ce8

evil159 requested a review from Chaoba July 7, 2022 10:48

Chaoba approved these changes Jul 7, 2022

View reviewed changes

OdNairy requested changes Jul 11, 2022

View reviewed changes

evil159 added 3 commits July 12, 2022 11:49

Make sure rotation is updated after pinch

a1533a9

Use wrapped instead of truncatingRemainder

f60de32

Add unit tests for scheduled rotation update

d446453

evil159 requested a review from OdNairy July 12, 2022 09:37

lint

a6558ce

OdNairy approved these changes Jul 12, 2022

View reviewed changes

Sources/MapboxMaps/Gestures/GestureHandlers/RotateGestureHandler.swift Outdated Show resolved Hide resolved

evil159 added 5 commits July 12, 2022 15:09

Remove force casting in RotationGestureHandler

ce51087

Use pinch pinpoint as anchor for zoom behavior

6b14a2c

Reset bearing pin pan-only behavior so rotation is applied around the…

603aa24

… correct anchor

Merge branch 'main' into rl/MAPSIOS-210_rotation_v1.5

1622444

Fix unit tests

2a21ebf

evil159 enabled auto-merge (squash) July 12, 2022 12:59

evil159 merged commit 7364de8 into main Jul 12, 2022

evil159 deleted the rl/MAPSIOS-210_rotation_v1.5 branch July 12, 2022 13:08

OdNairy pushed a commit that referenced this pull request Jul 14, 2022

Rotation gesture threshold and simultaneousRotateAndPinchToZoomEnable…

7a11c4b

…d option (#1429)

OdNairy pushed a commit that referenced this pull request Jul 14, 2022

Rotation gesture threshold and simultaneousRotateAndPinchToZoomEnable…

f1acf65

…d option (#1429)

OdNairy pushed a commit that referenced this pull request Aug 22, 2023

Update TestLab devices list (#1429)

d0f55dd

OdNairy pushed a commit that referenced this pull request Aug 22, 2023

Update TestLab devices list (#1429) (#1430)

8bae380

* Update TestLab devices list (#1429) * Use iPhone 8 iOS 16.2 on TestLab (#1432) * Replace iPhone 8 iOS 16.2 with iPhone 11 Pro iOS 16.2 (#1434)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rotation gesture threshold and simultaneousRotateAndPinchToZoomEnabled option #1429

Rotation gesture threshold and simultaneousRotateAndPinchToZoomEnabled option #1429

evil159 commented Jul 5, 2022 •

edited

Loading

Chaoba Jul 6, 2022

evil159 Jul 7, 2022

Chaoba left a comment

OdNairy left a comment

OdNairy Jul 11, 2022

evil159 Jul 12, 2022

OdNairy Jul 11, 2022

evil159 Jul 12, 2022

	bearing: (initialBearing + rotationInDegrees).truncatingRemainder(dividingBy: 360.0))
	bearing: (initialBearing + rotationInDegrees).wrapped(to: 0..<(2 * .pi)))

Rotation gesture threshold and simultaneousRotateAndPinchToZoomEnabled option #1429

Rotation gesture threshold and simultaneousRotateAndPinchToZoomEnabled option #1429

Conversation

evil159 commented Jul 5, 2022 • edited Loading

Pull request checklist:

Chaoba Jul 6, 2022

Choose a reason for hiding this comment

evil159 Jul 7, 2022

Choose a reason for hiding this comment

Chaoba left a comment

Choose a reason for hiding this comment

OdNairy left a comment

Choose a reason for hiding this comment

OdNairy Jul 11, 2022

Choose a reason for hiding this comment

evil159 Jul 12, 2022

Choose a reason for hiding this comment

OdNairy Jul 11, 2022

Choose a reason for hiding this comment

evil159 Jul 12, 2022

Choose a reason for hiding this comment

evil159 commented Jul 5, 2022 •

edited

Loading