Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve rotate experience when close to center #5104

Merged
merged 10 commits into from
Nov 27, 2024
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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this -1 doing here?
Can we add a comment so it's clear what it's doing?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of dragging from the exact center of the button where it won't change the rotation I moved the drag to be one pixel lower.


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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here


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 @@
return this._isOneFingerTouch(e) && this._isSameTouchEvent(e);
}
}

export class MouseOrTouchMoveStateManager implements DragMoveStateManager<MouseEvent | TouchEvent> {
HarelM marked this conversation as resolved.
Show resolved Hide resolved
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);

Check warning on line 141 in src/ui/handler/drag_move_state_manager.ts

View check run for this annotation

Codecov / codecov/patch

src/ui/handler/drag_move_state_manager.ts#L141

Added line #L141 was not covered by tests
}
}
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
Loading