Improve rotate UX (#5074)

* Improve rotate UX * Update changelog * Fix build test
maplibre · Nov 20, 2024 · 18a1ee3 · 18a1ee3
1 parent 1137935
commit 18a1ee3
Show file tree

Hide file tree

Showing 12 changed files with 87 additions and 39 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,7 @@
 ## main
 
 ### ✨ Features and improvements
+- ⚠️ Change drag rotate behavior to be around the center of the screen ([#5074](https://github.com/maplibre/maplibre-gl-js/pull/5074))
 - Export `Event` class ([#5016](https://github.com/maplibre/maplibre-gl-js/pull/5016))
 - Support Vertical Perspective projection ([#5023](https://github.com/maplibre/maplibre-gl-js/pull/5023))
 - _...Add new stuff here..._

diff --git a/src/ui/camera.ts b/src/ui/camera.ts
@@ -1139,7 +1139,7 @@ export abstract class Camera extends Evented {
         this._rotating = this._rotating || (startBearing !== bearing);
         this._pitching = this._pitching || (pitch !== startPitch);
         this._rolling = this._rolling || (roll !== startRoll);
-        this._padding = !tr.isPaddingEqual(padding as PaddingOptions);
+        this._padding = !tr.isPaddingEqual(padding);
         this._zooming = this._zooming || easeHandler.isZooming;
         this._easeId = options.easeId;
         this._prepareEase(eventData, options.noMoveStart, currently);
@@ -1163,7 +1163,8 @@ export abstract class Camera extends Evented {
         return this;
     }
 
-    _prepareEase(eventData: any, noMoveStart: boolean, currently: any = {}) {
+    _prepareEase(eventData: any, noMoveStart: boolean, 
+        currently: { moving?: boolean; zooming?: boolean; rotating?: boolean; pitching?: boolean; rolling?: boolean} = {}) {
         this._moving = true;
         if (!noMoveStart && !currently.moving) {
             this.fire(new Event('movestart', eventData));

diff --git a/src/ui/control/navigation_control.ts b/src/ui/control/navigation_control.ts
@@ -190,7 +190,7 @@ class MouseRotateWrapper {
         const mapRotateTolerance = map.dragRotate._mouseRotate.getClickTolerance();
         const mapPitchTolerance = map.dragRotate._mousePitch.getClickTolerance();
         this.element = element;
-        this.mouseRotate = generateMouseRotationHandler({clickTolerance: mapRotateTolerance, enable: true});
+        this.mouseRotate = generateMouseRotationHandler({clickTolerance: mapRotateTolerance, enable: true, aroundCenter: false});
         this.touchRotate = generateOneFingerTouchRotationHandler({clickTolerance: mapRotateTolerance, enable: true});
         this.map = map;
         if (pitch) {

diff --git a/src/ui/handler/drag_handler.ts b/src/ui/handler/drag_handler.ts
@@ -1,5 +1,5 @@
 import {DOM} from '../../util/dom';
-import type Point from '@mapbox/point-geometry';
+import Point from '@mapbox/point-geometry';
 import {DragMoveStateManager} from './drag_move_state_manager';
 import {Handler} from '../handler_manager';
 
@@ -28,7 +28,7 @@ export interface DragRollResult extends DragMovementResult {
     rollDelta: number;
 }
 
-type DragMoveFunction<T extends DragMovementResult> = (lastPoint: Point, point: Point) => T;
+type DragMoveFunction<T extends DragMovementResult> = (lastPoint: Point, currnetPoint: Point, center: Point) => T;
 
 export interface DragMoveHandler<T extends DragMovementResult, E extends Event> extends Handler {
     dragStart: (e: E, point: Point) => void;
@@ -140,13 +140,13 @@ export class DragHandler<T extends DragMovementResult, E extends Event> implemen
             return;
         }
 
-        const movePoint = point['length'] ? point[0] : point;
+        const movePoint = Array.isArray(point) ? point[0] : point;
 
         if (!this._moved && movePoint.dist(lastPoint) < this._clickTolerance) return;
         this._moved = true;
         this._lastPoint = movePoint;
 
-        return this._move(lastPoint, movePoint);
+        return this._move(lastPoint, movePoint, new Point(window.innerWidth / 2, window.innerHeight / 2));
     }
 
     dragEnd(e: E) {

diff --git a/src/ui/handler/drag_rotate.test.ts b/src/ui/handler/drag_rotate.test.ts
@@ -59,7 +59,7 @@ describe('drag rotate', () => {
         expect(rotate).toHaveBeenCalledTimes(0);
         expect(rotateend).toHaveBeenCalledTimes(0);
 
-        simulate.mousemove(map.getCanvas(), {buttons: 2, clientX: 10, clientY: 10});
+        simulate.mousemove(map.getCanvas(), {buttons: 2, clientX: 200, clientY: 10});
         map._renderTaskQueue.run();
         expect(rotatestart).toHaveBeenCalledTimes(1);
         expect(rotate).toHaveBeenCalledTimes(1);
@@ -121,7 +121,7 @@ describe('drag rotate', () => {
         map.on('rotateend',   spy);
 
         simulate.mousedown(map.getCanvas(), {buttons: 2, button: 2});
-        simulate.mousemove(map.getCanvas(), {buttons: 2, clientX: 10, clientY: 10});
+        simulate.mousemove(map.getCanvas(), {buttons: 2, clientX: 200, clientY: 10});
         map._renderTaskQueue.run();
         simulate.mouseup(map.getCanvas(),   {buttons: 0, button: 2});
         map._renderTaskQueue.run();
@@ -155,7 +155,7 @@ describe('drag rotate', () => {
         expect(rotate).toHaveBeenCalledTimes(0);
         expect(rotateend).toHaveBeenCalledTimes(0);
 
-        simulate.mousemove(map.getCanvas(), {buttons: 1,            ctrlKey: true, clientX: 10, clientY: 10});
+        simulate.mousemove(map.getCanvas(), {buttons: 1,            ctrlKey: true, clientX: 200, clientY: 10});
         map._renderTaskQueue.run();
         expect(rotatestart).toHaveBeenCalledTimes(1);
         expect(rotate).toHaveBeenCalledTimes(1);

diff --git a/src/ui/handler/mouse.ts b/src/ui/handler/mouse.ts
@@ -1,8 +1,9 @@
-import type Point from '@mapbox/point-geometry';
+import Point from '@mapbox/point-geometry';
 
 import {DOM} from '../../util/dom';
 import {DragMoveHandler, DragPanResult, DragRotateResult, DragPitchResult, DragHandler, DragRollResult} from './drag_handler';
 import {MouseMoveStateManager} from './drag_move_state_manager';
+import {getAngleDelta} from '../../util/util';
 
 /**
  * `MousePanHandler` allows the user to pan the map by clicking and dragging
@@ -33,10 +34,10 @@ const assignEvents = (handler: DragHandler<DragPanResult, MouseEvent>) => {
     };
 };
 
-export const generateMousePanHandler = ({enable, clickTolerance,}: {
+export function generateMousePanHandler({enable, clickTolerance}: {
     clickTolerance: number;
     enable?: boolean;
-}): MousePanHandler => {
+}): MousePanHandler {
     const mouseMoveStateManager = new MouseMoveStateManager({
         checkCorrectEvent: (e: MouseEvent) => DOM.mouseButton(e) === LEFT_BUTTON && !e.ctrlKey,
     });
@@ -51,20 +52,25 @@ export const generateMousePanHandler = ({enable, clickTolerance,}: {
     });
 };
 
-export const generateMouseRotationHandler = ({enable, clickTolerance, bearingDegreesPerPixelMoved = 0.8}: {
+export function generateMouseRotationHandler({enable, clickTolerance, aroundCenter = true}: {
     clickTolerance: number;
-    bearingDegreesPerPixelMoved?: number;
     enable?: boolean;
-}): MouseRotateHandler => {
+    aroundCenter?: boolean;
+}): MouseRotateHandler {
     const mouseMoveStateManager = new MouseMoveStateManager({
         checkCorrectEvent: (e: MouseEvent): boolean =>
             (DOM.mouseButton(e) === LEFT_BUTTON && e.ctrlKey) ||
             (DOM.mouseButton(e) === RIGHT_BUTTON && !e.ctrlKey),
     });
     return new DragHandler<DragRotateResult, MouseEvent>({
         clickTolerance,
-        move: (lastPoint: Point, point: Point) =>
-            ({bearingDelta: (point.x - lastPoint.x) * bearingDegreesPerPixelMoved}),
+        move: (lastPoint: Point, currentPoint: Point, center: Point) => {
+            if (aroundCenter) {
+                // Avoid rotation related to y axis since it is "saved" for pitch
+                return {bearingDelta: getAngleDelta(new Point(lastPoint.x, currentPoint.y), currentPoint, center)};
+            }
+            return {bearingDelta: (currentPoint.x - lastPoint.x) * 0.8}
+        },
         // prevent browser context menu when necessary; we don't allow it with rotation
         // because we can't discern rotation gesture start from contextmenu on Mac
         moveStateManager: mouseMoveStateManager,
@@ -73,19 +79,19 @@ export const generateMouseRotationHandler = ({enable, clickTolerance, bearingDeg
     });
 };
 
-export const generateMousePitchHandler = ({enable, clickTolerance, pitchDegreesPerPixelMoved = -0.5}: {
+export function generateMousePitchHandler({enable, clickTolerance, pitchDegreesPerPixelMoved = -0.5}: {
     clickTolerance: number;
     pitchDegreesPerPixelMoved?: number;
     enable?: boolean;
-}): MousePitchHandler => {
+}): MousePitchHandler {
     const mouseMoveStateManager = new MouseMoveStateManager({
         checkCorrectEvent: (e: MouseEvent): boolean =>
             (DOM.mouseButton(e) === LEFT_BUTTON && e.ctrlKey) ||
             (DOM.mouseButton(e) === RIGHT_BUTTON),
     });
     return new DragHandler<DragPitchResult, MouseEvent>({
         clickTolerance,
-        move: (lastPoint: Point, point: Point) =>
+        move: (lastPoint: Point, point: Point) => 
             ({pitchDelta: (point.y - lastPoint.y) * pitchDegreesPerPixelMoved}),
         // prevent browser context menu when necessary; we don't allow it with rotation
         // because we can't discern rotation gesture start from contextmenu on Mac
@@ -95,19 +101,24 @@ export const generateMousePitchHandler = ({enable, clickTolerance, pitchDegreesP
     });
 };
 
-export const generateMouseRollHandler = ({enable, clickTolerance, rollDegreesPerPixelMoved = 0.8}: {
+export function generateMouseRollHandler({enable, clickTolerance, rollDegreesPerPixelMoved = 0.3}: {
     clickTolerance: number;
     rollDegreesPerPixelMoved?: number;
     enable?: boolean;
-}): MouseRollHandler => {
+}): MouseRollHandler {
     const mouseMoveStateManager = new MouseMoveStateManager({
         checkCorrectEvent: (e: MouseEvent): boolean =>
             (DOM.mouseButton(e) === RIGHT_BUTTON && e.ctrlKey),
     });
     return new DragHandler<DragRollResult, MouseEvent>({
         clickTolerance,
-        move: (lastPoint: Point, point: Point) =>
-            ({rollDelta: (point.x - lastPoint.x) * rollDegreesPerPixelMoved}),
+        move: (lastPoint: Point, currentPoint: Point, center: Point) => {
+            let rollDelta = (currentPoint.x - lastPoint.x) * rollDegreesPerPixelMoved;
+            if (currentPoint.y < center.y) {
+                rollDelta = -rollDelta;
+            }
+            return {rollDelta};
+        },
         // prevent browser context menu when necessary; we don't allow it with roll
         // because we can't discern roll gesture start from contextmenu on Mac
         moveStateManager: mouseMoveStateManager,

diff --git a/src/ui/handler/mouse_handler_interface.test.ts b/src/ui/handler/mouse_handler_interface.test.ts
@@ -1,6 +1,7 @@
 import Point from '@mapbox/point-geometry';
 
 import {generateMousePanHandler, generateMousePitchHandler, generateMouseRollHandler, generateMouseRotationHandler} from './mouse';
+import {DragRotateResult} from './drag_handler';
 
 describe('mouse handler tests', () => {
     test('MouseRotateHandler', () => {
@@ -19,7 +20,7 @@ describe('mouse handler tests', () => {
         expect(mouseRotate.isActive()).toBe(false);
 
         const overToleranceMove = new MouseEvent('mousemove', {buttons: 2, clientX: 10, clientY: 10});
-        expect(mouseRotate.dragMove(overToleranceMove, new Point(10, 10))).toEqual({'bearingDelta': 8});
+        expect((mouseRotate.dragMove(overToleranceMove, new Point(10, 10)) as DragRotateResult).bearingDelta).toBeCloseTo(-0.53988378, 7);
         expect(mouseRotate.isActive()).toBe(true);
 
         mouseRotate.dragEnd(new MouseEvent('mouseup', {buttons: 0, button: 2}));
@@ -125,7 +126,7 @@ describe('mouse handler tests', () => {
         expect(mouseRoll.isActive()).toBe(false);
 
         const overToleranceMove = new MouseEvent('mousemove', {buttons: 2, clientX: 10, clientY: 10});
-        expect(mouseRoll.dragMove(overToleranceMove, new Point(10, 10))).toEqual({'rollDelta': 8});
+        expect(mouseRoll.dragMove(overToleranceMove, new Point(10, 10))).toEqual({'rollDelta': -3});
         expect(mouseRoll.isActive()).toBe(true);
 
         mouseRoll.dragEnd(new MouseEvent('mouseup', {buttons: 0, button: 2}));

diff --git a/src/ui/handler/one_finger_touch_drag.ts b/src/ui/handler/one_finger_touch_drag.ts
@@ -1,7 +1,8 @@
-import type Point from '@mapbox/point-geometry';
+import Point from '@mapbox/point-geometry';
 
 import {DragMoveHandler, DragRotateResult, DragPitchResult, DragHandler} from './drag_handler';
 import {OneFingerTouchMoveStateManager} from './drag_move_state_manager';
+import {getAngleDelta} from '../../util/util';
 
 export interface OneFingerTouchRotateHandler extends DragMoveHandler<DragRotateResult, TouchEvent> {}
 export interface OneFingerTouchPitchHandler extends DragMoveHandler<DragPitchResult, TouchEvent> {}
@@ -12,27 +13,32 @@ const assignEvents = (handler: DragHandler<DragRotateResult, TouchEvent>) => {
     handler.touchend = handler.dragEnd;
 };
 
-export const generateOneFingerTouchRotationHandler = ({enable, clickTolerance, bearingDegreesPerPixelMoved = 0.8}: {
+export function generateOneFingerTouchRotationHandler({enable, clickTolerance, aroundCenter = true}: {
     clickTolerance: number;
-    bearingDegreesPerPixelMoved?: number;
     enable?: boolean;
-}): OneFingerTouchRotateHandler => {
+    aroundCenter?: boolean;
+}): OneFingerTouchRotateHandler {
     const touchMoveStateManager = new OneFingerTouchMoveStateManager();
     return new DragHandler<DragRotateResult, TouchEvent>({
         clickTolerance,
-        move: (lastPoint: Point, point: Point) =>
-            ({bearingDelta: (point.x - lastPoint.x) * bearingDegreesPerPixelMoved}),
+        move: (lastPoint: Point, currentPoint: Point, center: Point) => {
+            if (aroundCenter) {
+                // Avoid rotation related to y axis since it is "saved" for pitch
+                return {bearingDelta: getAngleDelta(new Point(lastPoint.x, currentPoint.y), currentPoint, center)};
+            }
+            return {bearingDelta: (currentPoint.x - lastPoint.x) * 0.8}
+        },
         moveStateManager: touchMoveStateManager,
         enable,
         assignEvents,
     });
 };
 
-export const generateOneFingerTouchPitchHandler = ({enable, clickTolerance, pitchDegreesPerPixelMoved = -0.5}: {
+export function generateOneFingerTouchPitchHandler({enable, clickTolerance, pitchDegreesPerPixelMoved = -0.5}: {
     clickTolerance: number;
     pitchDegreesPerPixelMoved?: number;
     enable?: boolean;
-}): OneFingerTouchPitchHandler => {
+}): OneFingerTouchPitchHandler {
     const touchMoveStateManager = new OneFingerTouchMoveStateManager();
     return new DragHandler<DragPitchResult, TouchEvent>({
         clickTolerance,

diff --git a/src/ui/handler/one_finger_touch_drag_handler_interface.test.ts b/src/ui/handler/one_finger_touch_drag_handler_interface.test.ts
@@ -1,6 +1,7 @@
 import Point from '@mapbox/point-geometry';
 
 import {generateOneFingerTouchPitchHandler, generateOneFingerTouchRotationHandler} from './one_finger_touch_drag';
+import {DragRotateResult} from './drag_handler';
 
 const testTouch = {identifier: 0} as Touch;
 
@@ -21,7 +22,7 @@ describe('one touch drag handler tests', () => {
         expect(oneTouchRotate.isActive()).toBe(false);
 
         const overToleranceMove = new TouchEvent('touchmove', {targetTouches: [testTouch]});
-        expect(oneTouchRotate.dragMove(overToleranceMove, new Point(10, 10))).toEqual({'bearingDelta': 8});
+        expect((oneTouchRotate.dragMove(overToleranceMove, new Point(10, 10)) as DragRotateResult).bearingDelta).toBeCloseTo(-0.5398837825, 7);
         expect(oneTouchRotate.isActive()).toBe(true);
 
         oneTouchRotate.dragEnd(new TouchEvent('touchend', {targetTouches: [testTouch]}));

diff --git a/src/util/util.test.ts b/src/util/util.test.ts
@@ -1,5 +1,5 @@
 import Point from '@mapbox/point-geometry';
-import {arraysIntersect, bezier, clamp, clone, deepEqual, easeCubicInOut, extend, filterObject, findLineIntersection, isCounterClockwise, isPowerOfTwo, keysDifference, mapObject, nextPowerOfTwo, parseCacheControl, pick, readImageDataUsingOffscreenCanvas, readImageUsingVideoFrame, uniqueId, wrap, mod, distanceOfAnglesRadians, distanceOfAnglesDegrees, differenceOfAnglesRadians, differenceOfAnglesDegrees, solveQuadratic, remapSaturate, radiansToDegrees, degreesToRadians, rollPitchBearingToQuat, getRollPitchBearing} from './util';
+import {arraysIntersect, bezier, clamp, clone, deepEqual, easeCubicInOut, extend, filterObject, findLineIntersection, isCounterClockwise, isPowerOfTwo, keysDifference, mapObject, nextPowerOfTwo, parseCacheControl, pick, readImageDataUsingOffscreenCanvas, readImageUsingVideoFrame, uniqueId, wrap, mod, distanceOfAnglesRadians, distanceOfAnglesDegrees, differenceOfAnglesRadians, differenceOfAnglesDegrees, solveQuadratic, remapSaturate, radiansToDegrees, degreesToRadians, rollPitchBearingToQuat, getRollPitchBearing, getAngleDelta} from './util';
 import {Canvas} from 'canvas';
 
 describe('util', () => {
@@ -500,3 +500,21 @@ describe('util rotations', () => {
         expect(wrap(angles.bearing + angles.roll, -180, 180)).toBeCloseTo(wrap(bearing + roll, -180, 180), 6);
     });
 });
+
+describe('util getAngleDelta', () => {
+    test('positive direction', () => {
+        const lastPoint = new Point(0, 1);
+        const currentPoint = new Point(1, 0);
+        const center = new Point(0, 0);
+
+        expect(getAngleDelta(lastPoint, currentPoint, center)).toBe(90);
+    });
+
+    test('positive direction', () => {
+        const lastPoint = new Point(1, 0);
+        const currentPoint = new Point(0, 1);
+        const center = new Point(0, 0);
+
+        expect(getAngleDelta(lastPoint, currentPoint, center)).toBe(-90);
+    });
+});
diff --git a/src/util/util.ts b/src/util/util.ts
@@ -3,7 +3,7 @@ import UnitBezier from '@mapbox/unitbezier';
 import {isOffscreenCanvasDistorted} from './offscreen_canvas_distorted';
 import type {Size} from './image';
 import type {WorkerGlobalScopeInterface} from './web_worker';
-import {mat3, mat4, quat, vec3, vec4} from 'gl-matrix';
+import {mat3, mat4, quat, vec2, vec3, vec4} from 'gl-matrix';
 import {pixelsToTileUnits} from '../source/pixels_to_tile_units';
 import {OverscaledTileID} from '../source/tile_id';
 
@@ -921,6 +921,15 @@ export function getRollPitchBearing(rotation: quat): RollPitchBearing {
     return {roll, pitch: xAngle + 90.0, bearing};
 }
 
+export function getAngleDelta(lastPoint: Point, currentPoint: Point, center: Point): number {
+    const pointVect = vec2.fromValues(currentPoint.x - center.x, currentPoint.y - center.y);
+    const lastPointVec = vec2.fromValues(lastPoint.x - center.x, lastPoint.y - center.y);
+
+    const crossProduct = pointVect[0] * lastPointVec[1] - pointVect[1] * lastPointVec[0];
+    const angleRadians = Math.atan2(crossProduct, vec2.dot(pointVect, lastPointVec));
+    return radiansToDegrees(angleRadians);
+}
+
 /**
  * This method converts roll, pitch, and bearing angles in degrees to a rotation quaternion.
  * @param roll - Roll angle in degrees

diff --git a/test/build/min.test.ts b/test/build/min.test.ts
@@ -37,7 +37,7 @@ describe('test min build', () => {
         const decreaseQuota = 4096;
 
         // feel free to update this value after you've checked that it has changed on purpose :-)
-        const expectedBytes = 896666;
+        const expectedBytes = 897777;
 
         expect(actualBytes).toBeLessThan(expectedBytes + increaseQuota);
         expect(actualBytes).toBeGreaterThan(expectedBytes - decreaseQuota);