Skip to content

Commit

Permalink
Improve rotate experience when close to center (#5104)
Browse files Browse the repository at this point in the history
* Improve rotate experience

* Update changelog

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

* Solve spellcheck issue.

* Update CHANGELOG.md
  • Loading branch information
HarelM authored Nov 27, 2024
1 parent 0ac30b1 commit 985b657
Show file tree
Hide file tree
Showing 11 changed files with 128 additions and 202 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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..._

Expand Down
4 changes: 2 additions & 2 deletions src/ui/control/navigation_control.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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});
Expand Down Expand Up @@ -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}]});
Expand Down
84 changes: 32 additions & 52 deletions src/ui/control/navigation_control.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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() {
Expand All @@ -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();
};

Expand All @@ -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);
}
Expand All @@ -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);
}
};

Expand All @@ -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();
Expand Down
9 changes: 4 additions & 5 deletions src/ui/handler/drag_handler.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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> = {
Expand Down Expand Up @@ -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;
}
Expand All @@ -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) {
Expand Down
28 changes: 28 additions & 0 deletions src/ui/handler/drag_move_state_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
22 changes: 15 additions & 7 deletions src/ui/handler/mouse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions src/ui/handler/mouse_handler_interface.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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}));
Expand Down Expand Up @@ -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);
Expand Down
39 changes: 39 additions & 0 deletions src/ui/handler/mouse_rotate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
Loading

0 comments on commit 985b657

Please sign in to comment.