Improve rotate experience when close to center (#5104)

* Improve rotate experience

* Update changelog

* Reduce complexity for navigation control, use only get center to calculate

* Solve spellcheck issue.

* Update CHANGELOG.md

CHANGELOG.md

@@ -1,10 +1,12 @@
 ## main
 
 ### ✨ Features and improvements
+
 - _...Add new stuff here..._
 
 ### 🐞 Bug fixes
 
+- ⚠️ Change drag rotate behavior to be less abrupt around the center ([#5104](https://github.com/maplibre/maplibre-gl-js/pull/5104))
 - Fix regression in render world copies ([#5101](https://github.com/maplibre/maplibre-gl-js/pull/5101))
 - _...Add new stuff here..._
 

src/ui/control/navigation_control.test.ts

@@ -84,7 +84,7 @@ describe('NavigationControl', () => {
         const navRect = navButton.getClientRects();
 
         const buttonX = (navRect.x ?? 0) + (navRect.width ?? 0) / 2;
-        const buttonY = (navRect.y ?? 0) + (navRect.height ?? 0) / 2;
+        const buttonY = (navRect.y ?? 0) + (navRect.height ?? 0) / 2 - 1;
 
         simulate.mousedown(navButton, {buttons: 1, button: 0, clientX: buttonX, clientY: buttonY});
         simulate.mousemove(window, {buttons: 1, button: 0, clientX: buttonX - 50, clientY: buttonY});
@@ -168,7 +168,7 @@ describe('NavigationControl', () => {
         const navRect = navButton.getClientRects();
 
         const buttonX = (navRect.x ?? 0) + (navRect.width ?? 0) / 2;
-        const buttonY = (navRect.y ?? 0) + (navRect.height ?? 0) / 2;
+        const buttonY = (navRect.y ?? 0) + (navRect.height ?? 0) / 2 - 1;
 
         simulate.touchstart(navButton, {touches: [{clientX: buttonX, clientY: buttonY}], targetTouches: [{clientX: buttonX, clientY: buttonY}]});
         simulate.touchmove(window, {touches: [{clientX: buttonX - 50, clientY: buttonY}], targetTouches: [{clientX: buttonX - 50, clientY: buttonY}]});

src/ui/control/navigation_control.ts

@@ -1,9 +1,9 @@
-import type Point from '@mapbox/point-geometry';
+import Point from '@mapbox/point-geometry';
 
 import {DOM} from '../../util/dom';
-import {extend} from '../../util/util';
-import {generateMousePitchHandler, generateMouseRotationHandler, type MousePitchHandler, type MouseRotateHandler} from '../handler/mouse';
-import {generateOneFingerTouchPitchHandler, generateOneFingerTouchRotationHandler, type OneFingerTouchPitchHandler, type OneFingerTouchRotateHandler} from '../handler/one_finger_touch_drag';
+import {extend, getAngleDelta} from '../../util/util';
+import {DragHandler, type DragMoveHandler, type DragRotateResult} from '../handler/drag_handler';
+import {MouseOrTouchMoveStateManager} from '../handler/drag_move_state_manager';
 
 import type {Map} from '../map';
 import type {IControl} from './control';
@@ -177,62 +177,46 @@ class MouseRotateWrapper {
     map: Map;
     _clickTolerance: number;
     element: HTMLElement;
-    // Rotation and pitch handlers are separated due to different _clickTolerance values
-    mouseRotate: MouseRotateHandler;
-    touchRotate: OneFingerTouchRotateHandler;
-    mousePitch: MousePitchHandler;
-    touchPitch: OneFingerTouchPitchHandler;
+    _rotatePitchHanlder: DragMoveHandler<DragRotateResult, MouseEvent | TouchEvent>;
     _startPos: Point;
     _lastPos: Point;
 
     constructor(map: Map, element: HTMLElement, pitch: boolean = false) {
         this._clickTolerance = 10;
-        const mapRotateTolerance = map.dragRotate._mouseRotate.getClickTolerance();
-        const mapPitchTolerance = map.dragRotate._mousePitch.getClickTolerance();
         this.element = element;
-        this.mouseRotate = generateMouseRotationHandler({clickTolerance: mapRotateTolerance, enable: true, aroundCenter: false});
-        this.touchRotate = generateOneFingerTouchRotationHandler({clickTolerance: mapRotateTolerance, enable: true});
+
+        const moveStateManager = new MouseOrTouchMoveStateManager();
+        this._rotatePitchHanlder = new DragHandler<DragRotateResult, MouseEvent | TouchEvent>({
+            clickTolerance: 3,
+            move: (lastPoint: Point, currentPoint: Point) => {
+                const rect = element.getBoundingClientRect();
+                const center = new Point((rect.bottom - rect.top) / 2, (rect.right - rect.left) / 2);
+                const bearingDelta = getAngleDelta(new Point(lastPoint.x, currentPoint.y), currentPoint, center);
+                const pitchDelta = pitch ? (currentPoint.y - lastPoint.y) * -0.5 : undefined;
+                return {bearingDelta, pitchDelta};
+            },
+            moveStateManager,
+            enable: true,
+            assignEvents: () => {},
+        });
         this.map = map;
-        if (pitch) {
-            this.mousePitch = generateMousePitchHandler({clickTolerance: mapPitchTolerance, enable: true});
-            this.touchPitch = generateOneFingerTouchPitchHandler({clickTolerance: mapPitchTolerance, enable: true});
-        }
 
         DOM.addEventListener(element, 'mousedown', this.mousedown);
         DOM.addEventListener(element, 'touchstart', this.touchstart, {passive: false});
         DOM.addEventListener(element, 'touchcancel', this.reset);
     }
 
-    startMouse(e: MouseEvent, point: Point) {
-        this.mouseRotate.dragStart(e, point);
-        if (this.mousePitch) this.mousePitch.dragStart(e, point);
+    startMove(e: MouseEvent | TouchEvent, point: Point) {
+        this._rotatePitchHanlder.dragStart(e, point);
         DOM.disableDrag();
     }
 
-    startTouch(e: TouchEvent, point: Point) {
-        this.touchRotate.dragStart(e, point);
-        if (this.touchPitch) this.touchPitch.dragStart(e, point);
-        DOM.disableDrag();
-    }
-
-    moveMouse(e: MouseEvent, point: Point) {
-        const map = this.map;
-        const {bearingDelta} = this.mouseRotate.dragMove(e, point) || {};
-        if (bearingDelta) map.setBearing(map.getBearing() + bearingDelta);
-        if (this.mousePitch) {
-            const {pitchDelta} = this.mousePitch.dragMove(e, point) || {};
-            if (pitchDelta) map.setPitch(map.getPitch() + pitchDelta);
-        }
-    }
-
-    moveTouch(e: TouchEvent, point: Point) {
+    move(e: MouseEvent | TouchEvent, point: Point) {
         const map = this.map;
-        const {bearingDelta} = this.touchRotate.dragMove(e, point) || {};
+        const {bearingDelta, pitchDelta} = this._rotatePitchHanlder.dragMove(e, point) || {};
+        console.log(bearingDelta, pitchDelta, point);
         if (bearingDelta) map.setBearing(map.getBearing() + bearingDelta);
-        if (this.touchPitch) {
-            const {pitchDelta} = this.touchPitch.dragMove(e, point) || {};
-            if (pitchDelta) map.setPitch(map.getPitch() + pitchDelta);
-        }
+        if (pitchDelta) map.setPitch(map.getPitch() + pitchDelta);
     }
 
     off() {
@@ -254,18 +238,17 @@ class MouseRotateWrapper {
     }
 
     mousedown = (e: MouseEvent) => {
-        this.startMouse(extend({}, e, {ctrlKey: true, preventDefault: () => e.preventDefault()}), DOM.mousePos(this.element, e));
+        this.startMove(e, DOM.mousePos(this.element, e));
         DOM.addEventListener(window, 'mousemove', this.mousemove);
         DOM.addEventListener(window, 'mouseup', this.mouseup);
     };
 
     mousemove = (e: MouseEvent) => {
-        this.moveMouse(e, DOM.mousePos(this.element, e));
+        this.move(e, DOM.mousePos(this.element, e));
     };
 
     mouseup = (e: MouseEvent) => {
-        this.mouseRotate.dragEnd(e);
-        if (this.mousePitch) this.mousePitch.dragEnd(e);
+        this._rotatePitchHanlder.dragEnd(e);
         this.offTemp();
     };
 
@@ -274,7 +257,7 @@ class MouseRotateWrapper {
             this.reset();
         } else {
             this._startPos = this._lastPos = DOM.touchPos(this.element, e.targetTouches)[0];
-            this.startTouch(e, this._startPos);
+            this.startMove(e, this._startPos);
             DOM.addEventListener(window, 'touchmove', this.touchmove, {passive: false});
             DOM.addEventListener(window, 'touchend', this.touchend);
         }
@@ -285,7 +268,7 @@ class MouseRotateWrapper {
             this.reset();
         } else {
             this._lastPos = DOM.touchPos(this.element, e.targetTouches)[0];
-            this.moveTouch(e, this._lastPos);
+            this.move(e, this._lastPos);
         }
     };
 
@@ -302,10 +285,7 @@ class MouseRotateWrapper {
     };
 
     reset = () => {
-        this.mouseRotate.reset();
-        if (this.mousePitch) this.mousePitch.reset();
-        this.touchRotate.reset();
-        if (this.touchPitch) this.touchPitch.reset();
+        this._rotatePitchHanlder.reset();
         delete this._startPos;
         delete this._lastPos;
         this.offTemp();

src/ui/handler/drag_handler.ts

@@ -1,5 +1,5 @@
 import {DOM} from '../../util/dom';
-import Point from '@mapbox/point-geometry';
+import type Point from '@mapbox/point-geometry';
 import {type DragMoveStateManager} from './drag_move_state_manager';
 import {type Handler} from '../handler_manager';
 
@@ -28,13 +28,12 @@ export interface DragRollResult extends DragMovementResult {
     rollDelta: number;
 }
 
-type DragMoveFunction<T extends DragMovementResult> = (lastPoint: Point, currnetPoint: Point, center: Point) => T;
+type DragMoveFunction<T extends DragMovementResult> = (lastPoint: Point, currnetPoint: Point) => T;
 
 export interface DragMoveHandler<T extends DragMovementResult, E extends Event> extends Handler {
     dragStart: (e: E, point: Point) => void;
     dragMove: (e: E, point: Point) => T | void;
     dragEnd: (e: E) => void;
-    getClickTolerance: () => number;
 }
 
 export type DragMoveHandlerOptions<T, E extends Event> = {
@@ -122,7 +121,7 @@ export class DragHandler<T extends DragMovementResult, E extends Event> implemen
         if (!this._moveStateManager.isValidStartEvent(e)) return;
         this._moveStateManager.startMove(e);
 
-        this._lastPoint = point['length'] ? point[0] : point;
+        this._lastPoint = Array.isArray(point) ? point[0] : point;
 
         if (this._activateOnStart && this._lastPoint) this._active = true;
     }
@@ -146,7 +145,7 @@ export class DragHandler<T extends DragMovementResult, E extends Event> implemen
         this._moved = true;
         this._lastPoint = movePoint;
 
-        return this._move(lastPoint, movePoint, new Point(window.innerWidth / 2, window.innerHeight / 2));
+        return this._move(lastPoint, movePoint);
     }
 
     dragEnd(e: E) {

src/ui/handler/drag_move_state_manager.ts

@@ -113,3 +113,31 @@ export class OneFingerTouchMoveStateManager implements DragMoveStateManager<Touc
         return this._isOneFingerTouch(e) && this._isSameTouchEvent(e);
     }
 }
+
+export class MouseOrTouchMoveStateManager implements DragMoveStateManager<MouseEvent | TouchEvent> {
+    constructor(
+        private mouseMoveStateManager = new MouseMoveStateManager({checkCorrectEvent: () => true}), 
+        private oneFingerTouchMoveStateManager = new OneFingerTouchMoveStateManager()
+    ) {}
+
+    startMove(e: MouseEvent | TouchEvent) {
+        if (e instanceof MouseEvent) this.mouseMoveStateManager.startMove(e);
+        if (e instanceof TouchEvent) this.oneFingerTouchMoveStateManager.startMove(e);
+    }
+    endMove(e?: MouseEvent | TouchEvent) {
+        if (e instanceof MouseEvent) this.mouseMoveStateManager.endMove(e);
+        if (e instanceof TouchEvent) this.oneFingerTouchMoveStateManager.endMove(e);
+    }
+    isValidStartEvent(e: MouseEvent | TouchEvent) {
+        if (e instanceof MouseEvent) return this.mouseMoveStateManager.isValidStartEvent(e);
+        if (e instanceof TouchEvent) return this.oneFingerTouchMoveStateManager.isValidStartEvent(e);
+    }
+    isValidMoveEvent(e: MouseEvent | TouchEvent) {
+        if (e instanceof MouseEvent) return this.mouseMoveStateManager.isValidMoveEvent(e);
+        if (e instanceof TouchEvent) return this.oneFingerTouchMoveStateManager.isValidMoveEvent(e);
+    }
+    isValidEndEvent(e?: MouseEvent | TouchEvent) {
+        if (e instanceof MouseEvent) return this.mouseMoveStateManager.isValidEndEvent(e);
+        if (e instanceof TouchEvent) return this.oneFingerTouchMoveStateManager.isValidEndEvent(e);
+    }
+}

src/ui/handler/mouse.ts

@@ -52,24 +52,31 @@ export function generateMousePanHandler({enable, clickTolerance}: {
     });
 };
 
-export function generateMouseRotationHandler({enable, clickTolerance, aroundCenter = true}: {
+export function generateMouseRotationHandler({enable, clickTolerance, aroundCenter = true, minPixelCenterThreshold = 100, rotateDegreesPerPixelMoved = 0.8}: {
     clickTolerance: number;
     enable?: boolean;
     aroundCenter?: boolean;
-}): MouseRotateHandler {
+    minPixelCenterThreshold?: number;
+    rotateDegreesPerPixelMoved?: number;
+}, getCenter: () => Point): 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, currentPoint: Point, center: Point) => {
-            if (aroundCenter) {
+        move: (lastPoint: Point, currentPoint: Point) => {
+            const center = getCenter();
+            if (aroundCenter && Math.abs(center.y - lastPoint.y) > minPixelCenterThreshold) {
                 // 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}
+            let bearingDelta = (currentPoint.x - lastPoint.x) * rotateDegreesPerPixelMoved;
+            if (aroundCenter && currentPoint.y < center.y) {
+                bearingDelta = -bearingDelta;
+            }
+            return {bearingDelta};
         },
         // 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
@@ -105,14 +112,15 @@ export function generateMouseRollHandler({enable, clickTolerance, rollDegreesPer
     clickTolerance: number;
     rollDegreesPerPixelMoved?: number;
     enable?: boolean;
-}): MouseRollHandler {
+}, getCenter: () => Point): 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, currentPoint: Point, center: Point) => {
+        move: (lastPoint: Point, currentPoint: Point) => {
+            const center = getCenter();
             let rollDelta = (currentPoint.x - lastPoint.x) * rollDegreesPerPixelMoved;
             if (currentPoint.y < center.y) {
                 rollDelta = -rollDelta;

src/ui/handler/mouse_handler_interface.test.ts

@@ -6,7 +6,7 @@ import {type DragRotateResult} from './drag_handler';
 
 describe('mouse handler tests', () => {
     test('MouseRotateHandler', () => {
-        const mouseRotate = generateMouseRotationHandler({clickTolerance: 2});
+        const mouseRotate = generateMouseRotationHandler({clickTolerance: 2}, () => new Point(10, 10));
 
         expect(mouseRotate.isActive()).toBe(false);
         expect(mouseRotate.isEnabled()).toBe(false);
@@ -21,7 +21,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)) as DragRotateResult).bearingDelta).toBeCloseTo(-0.53988378, 7);
+        expect((mouseRotate.dragMove(overToleranceMove, new Point(10, 10)) as DragRotateResult).bearingDelta).toBeCloseTo(8);
         expect(mouseRotate.isActive()).toBe(true);
 
         mouseRotate.dragEnd(new MouseEvent('mouseup', {buttons: 0, button: 2}));
@@ -112,7 +112,7 @@ describe('mouse handler tests', () => {
     });
 
     test('MouseRollHandler', () => {
-        const mouseRoll = generateMouseRollHandler({clickTolerance: 2});
+        const mouseRoll = generateMouseRollHandler({clickTolerance: 2}, () => new Point(11, 11));
 
         expect(mouseRoll.isActive()).toBe(false);
         expect(mouseRoll.isEnabled()).toBe(false);

src/ui/handler/mouse_rotate.test.ts

@@ -60,4 +60,43 @@ describe('mouse rotate', () => {
 
         map.remove();
     });
+
+    test('MouseRotateHandler rotate around center', () => {
+        const map = createMap({interactive: true});
+
+        expect(map.getBearing()).toBe(0);
+
+        // Prevent inertial rotation.
+        simulate.mousedown(map.getCanvas(), {buttons: 2, button: 2});
+        map._renderTaskQueue.run();
+
+        simulate.mousemove(map.getCanvas(), {buttons: 2, clientX: 10, clientY: 10});
+        map._renderTaskQueue.run();
+
+        simulate.mousemove(map.getCanvas(), {buttons: 0, clientX: 10, clientY: 10});
+        map._renderTaskQueue.run();
+
+        expect(map.getBearing()).toBeCloseTo(-1.39233, 4);
+
+        map.remove();
+    });
+
+    test('MouseRotateHandler rotate around center but not too much', () => {
+        const map = createMap({interactive: true});
+
+        expect(map.getBearing()).toBe(0);
+
+        simulate.mousedown(map.getCanvas(), {buttons: 2, button: 2, clientX: map.getCanvas().width / 2 + 10, clientY: map.getCanvas().height / 2 + 10});
+        map._renderTaskQueue.run();
+
+        simulate.mousemove(map.getCanvas(), {buttons: 2, clientX: map.getCanvas().width / 2 + 10, clientY: map.getCanvas().height / 2 - 10});
+        map._renderTaskQueue.run();
+
+        simulate.mousemove(map.getCanvas(), {buttons: 2, clientX: map.getCanvas().width / 2 + 20, clientY: map.getCanvas().height / 2 - 10});
+        map._renderTaskQueue.run();
+
+        expect(map.getBearing()).toBe(-8);
+
+        map.remove();
+    });
 });