Fix unwanted roll #5083

CHANGELOG.md

@@ -5,6 +5,7 @@
 - _...Add new stuff here..._
 
 ### 🐞 Bug fixes
+- Fix unwanted roll when motion is interrupted ([#5083](https://github.com/maplibre/maplibre-gl-js/pull/5083))
 - _...Add new stuff here..._
 
 ## 5.0.0-pre.7

src/geo/projection/camera_helper.ts

@@ -6,6 +6,7 @@ import {PaddingOptions} from '../edge_insets';
 import {LngLatBounds} from '../lng_lat_bounds';
 import {getRollPitchBearing, RollPitchBearing, warnOnce} from '../../util/util';
 import {quat} from 'gl-matrix';
+import {interpolates} from '@maplibre/maplibre-gl-style-spec';
 
 export type MapControlsDeltas = {
     panDelta: Point;
@@ -93,30 +94,40 @@ export interface ICameraHelper {
     handleEaseTo(tr: ITransform, options: EaseToHandlerOptions): EaseToHandlerResult;
 
     handleFlyTo(tr: ITransform, options: FlyToHandlerOptions): FlyToHandlerResult;
+
+    setRollEnabled(rollEnabled: boolean): void;
 }
 
 /**
  * @internal
  * Set a transform's rotation to a value interpolated between startRotation and endRotation
  * @param startRotation - the starting rotation (rotation when k = 0)
  * @param endRotation - the end rotation (rotation when k = 1)
+ * @param startEulerAngles - the starting Euler angles.
  * @param endEulerAngles - the end Euler angles. This is needed in case `endRotation` has an ambiguous Euler angle representation.
  * @param tr - the transform to be updated
  * @param k - the interpolation fraction, between 0 and 1.
+ * @param useSlerp - if true, use spherical linear interpolation. If false, use linear interpolation of Euler angles.
  */
-export function updateRotation(startRotation: quat, endRotation: quat, endEulerAngles: RollPitchBearing, tr: ITransform, k: number) {
-    // At pitch ==0, the Euler angle representation is ambiguous. In this case, set the Euler angles
-    // to the representation requested by the caller
-    if (k < 1) {
-        const rotation: quat = new Float64Array(4) as any;
-        quat.slerp(rotation, startRotation, endRotation, k);
-        const eulerAngles = getRollPitchBearing(rotation);
-        tr.setRoll(eulerAngles.roll);
-        tr.setPitch(eulerAngles.pitch);
-        tr.setBearing(eulerAngles.bearing);
+export function updateRotation(startRotation: quat, endRotation: quat, startEulerAngles: RollPitchBearing, endEulerAngles: RollPitchBearing, tr: ITransform, k: number, useSlerp: boolean) {
+    if (useSlerp) {
+        // At pitch ==0, the Euler angle representation is ambiguous. In this case, set the Euler angles
+        // to the representation requested by the caller
+        if (k < 1) {
+            const rotation: quat = new Float64Array(4) as any;
+            quat.slerp(rotation, startRotation, endRotation, k);
+            const eulerAngles = getRollPitchBearing(rotation);
+            tr.setRoll(eulerAngles.roll);
+            tr.setPitch(eulerAngles.pitch);
+            tr.setBearing(eulerAngles.bearing);
+        } else {
+            tr.setRoll(endEulerAngles.roll);
+            tr.setPitch(endEulerAngles.pitch);
+            tr.setBearing(endEulerAngles.bearing);
+        }
     } else {
-        tr.setRoll(endEulerAngles.roll);
-        tr.setPitch(endEulerAngles.pitch);
-        tr.setBearing(endEulerAngles.bearing);
+        tr.setRoll(interpolates.number(startEulerAngles.roll, endEulerAngles.roll, k));
+        tr.setPitch(interpolates.number(startEulerAngles.pitch, endEulerAngles.pitch, k));
+        tr.setBearing(interpolates.number(startEulerAngles.bearing, endEulerAngles.bearing, k));
     }
 }

src/geo/projection/globe_camera_helper.ts

@@ -17,12 +17,26 @@ import {interpolates} from '@maplibre/maplibre-gl-style-spec';
  * @internal
  */
 export class GlobeCameraHelper implements ICameraHelper {
+
     private _globe: GlobeProjection;
     private _mercatorCameraHelper: MercatorCameraHelper;
 
     constructor(globe: GlobeProjection) {
         this._globe = globe;
         this._mercatorCameraHelper = new MercatorCameraHelper();
+        this._rollEnabled = false;
+    }
+
+    /**
+     * @internal
+     * If `false`, the map's roll control with "drag to rotate" interaction will be disabled.
+     * @defaultValue false
+     */
+    _rollEnabled: boolean;
+
+    setRollEnabled(rollEnabled: boolean): void {
+        this._rollEnabled = rollEnabled;
+        this._mercatorCameraHelper.setRollEnabled(rollEnabled);
     }
 
     get useGlobeControls(): boolean { return this._globe.useGlobeRendering; }
@@ -263,6 +277,7 @@ export class GlobeCameraHelper implements ICameraHelper {
         const startZoom = tr.zoom;
         const startCenter = tr.center;
         const startRotation = rollPitchBearingToQuat(tr.roll, tr.pitch, tr.bearing);
+        const startEulerAngles = {roll: tr.roll, pitch: tr.pitch, bearing: tr.bearing};
         const endRoll = options.roll === undefined ? tr.roll : options.roll;
         const endPitch = options.pitch === undefined ? tr.pitch : options.pitch;
         const endBearing = options.bearing === undefined ? tr.bearing : options.bearing;
@@ -318,7 +333,7 @@ export class GlobeCameraHelper implements ICameraHelper {
 
         const easeFunc = (k: number) => {
             if (!quat.equals(startRotation, endRotation)) {
-                updateRotation(startRotation, endRotation, {roll: endRoll, pitch: endPitch, bearing: endBearing}, tr, k);
+                updateRotation(startRotation, endRotation, startEulerAngles, {roll: endRoll, pitch: endPitch, bearing: endBearing}, tr, k, this._rollEnabled);
             }
 
             if (options.around) {

src/geo/projection/mercator_camera_helper.ts

@@ -15,6 +15,22 @@ import {quat} from 'gl-matrix';
  * @internal
  */
 export class MercatorCameraHelper implements ICameraHelper {
+
+    /**
+     * @internal
+     * If `false`, the map's roll control with "drag to rotate" interaction will be disabled.
+     * @defaultValue false
+     */
+    _rollEnabled: boolean;
+
+    constructor() {
+        this._rollEnabled = false;
+    }
+
+    setRollEnabled(rollEnabled: boolean): void {
+        this._rollEnabled = rollEnabled;
+    }
+
     get useGlobeControls(): boolean { return false; }
 
     handlePanInertia(pan: Point, transform: IReadonlyTransform): {
@@ -129,6 +145,7 @@ export class MercatorCameraHelper implements ICameraHelper {
         const startZoom = tr.zoom;
         const startPadding = tr.padding;
         const startRotation = rollPitchBearingToQuat(tr.roll, tr.pitch, tr.bearing);
+        const startEulerAngles = {roll: tr.roll, pitch: tr.pitch, bearing: tr.bearing};
         const endRoll = options.roll === undefined ? tr.roll : options.roll;
         const endPitch = options.pitch === undefined ? tr.pitch : options.pitch;
         const endBearing = options.bearing === undefined ? tr.bearing : options.bearing;
@@ -161,7 +178,7 @@ export class MercatorCameraHelper implements ICameraHelper {
                 tr.setZoom(interpolates.number(startZoom, endZoom, k));
             }
             if (!quat.equals(startRotation, endRotation)) {
-                updateRotation(startRotation, endRotation, {roll: endRoll, pitch: endPitch, bearing: endBearing}, tr, k);
+                updateRotation(startRotation, endRotation, startEulerAngles, {roll: endRoll, pitch: endPitch, bearing: endBearing}, tr, k, this._rollEnabled);
             }
             if (doPadding) {
                 tr.interpolatePadding(startPadding, options.padding, k);

src/ui/camera.test.ts

@@ -822,12 +822,14 @@ describe('#easeTo', () => {
 
     test('rolls to specified roll', () => {
         const camera = createCamera();
+        camera.setRollEnabled(true);
         camera.easeTo({pitch: 1, roll: 45, duration: 0});
         expect(camera.getRoll()).toBeCloseTo(45, 6);
     });
 
     test('roll behavior at Euler angle singularity', () => {
         const camera = createCamera();
+        camera.setRollEnabled(true);
         camera.easeTo({bearing: 0, pitch: 0, roll: 45, duration: 0});
         expect(camera.getRoll()).toBeCloseTo(45, 6);
         expect(camera.getPitch()).toBeCloseTo(0, 6);
@@ -836,6 +838,7 @@ describe('#easeTo', () => {
 
     test('bearing behavior at Euler angle singularity', () => {
         const camera = createCamera();
+        camera.setRollEnabled(true);
         camera.easeTo({bearing: 45, pitch: 0, roll: 0, duration: 0});
         expect(camera.getRoll()).toBeCloseTo(0, 6);
         expect(camera.getPitch()).toBeCloseTo(0, 6);
@@ -1330,12 +1333,14 @@ describe('#flyTo', () => {
 
     test('rolls to specified roll', () => {
         const camera = createCamera();
+        camera.setRollEnabled(true);
         camera.flyTo({pitch: 1, roll: 45, animate: false});
         expect(camera.getRoll()).toBeCloseTo(45, 6);
     });
 
     test('roll behavior at Euler angle singularity', () => {
         const camera = createCamera();
+        camera.setRollEnabled(true);
         camera.flyTo({bearing: 0, pitch: 0, roll: 45, animate: false});
         expect(camera.getRoll()).toBeCloseTo(45, 6);
         expect(camera.getPitch()).toBeCloseTo(0, 6);
@@ -1344,6 +1349,7 @@ describe('#flyTo', () => {
 
     test('bearing behavior at Euler angle singularity', () => {
         const camera = createCamera();
+        camera.setRollEnabled(true);
         camera.flyTo({bearing: 45, pitch: 0, roll: 0, animate: false});
         expect(camera.getRoll()).toBeCloseTo(0, 6);
         expect(camera.getPitch()).toBeCloseTo(0, 6);
@@ -1535,6 +1541,62 @@ describe('#flyTo', () => {
         expect(fixedLngLat(camera.getCenter())).toEqual({lng: 100, lat: 0});
     });
 
+    test('no roll when motion is interrupted', () => {
+        const stub = jest.spyOn(browser, 'now');
+
+        const camera = createCamera();
+        camera.setRollEnabled(false);
+        stub.mockImplementation(() => 0);
+        camera.easeTo({pitch: 10, bearing: 100, duration: 1000});
+        stub.mockImplementation(() => 100);
+        camera.simulateFrame();
+        camera.easeTo({elevation: 1, duration: 0});
+        expect(camera.getRoll()).toBe(0);
+    });
+
+    test('no roll when motion is interrupted: globe', () => {
+        const stub = jest.spyOn(browser, 'now');
+
+        const camera = createCameraGlobe();
+        camera.setRollEnabled(false);
+        stub.mockImplementation(() => 0);
+        camera.easeTo({pitch: 10, bearing: 100, duration: 1000});
+        stub.mockImplementation(() => 100);
+        camera.simulateFrame();
+        camera.easeTo({elevation: 1, duration: 0});
+        expect(camera.getRoll()).toBe(0);
+    });
+
+    test('angles when motion is interrupted', () => {
+        const stub = jest.spyOn(browser, 'now');
+
+        const camera = createCamera();
+        camera.setRollEnabled(true);
+        stub.mockImplementation(() => 0);
+        camera.easeTo({pitch: 10, bearing: 20, roll: 30, duration: 1000});
+        stub.mockImplementation(() => 500);
+        camera.simulateFrame();
+        camera.easeTo({elevation: 1, duration: 0});
+        expect(camera.getRoll()).toBeCloseTo(25.041890412598942);
+        expect(camera.getPitch()).toBeCloseTo(8.116189398053095);
+        expect(camera.getBearing()).toBeCloseTo(15.041890412599061);
+    });
+
+    test('angles when motion is interrupted: globe', () => {
+        const stub = jest.spyOn(browser, 'now');
+
+        const camera = createCameraGlobe();
+        camera.setRollEnabled(true);
+        stub.mockImplementation(() => 0);
+        camera.easeTo({pitch: 10, bearing: 20, roll: 30, duration: 1000});
+        stub.mockImplementation(() => 500);
+        camera.simulateFrame();
+        camera.easeTo({elevation: 1, duration: 0});
+        expect(camera.getRoll()).toBeCloseTo(25.041890412598942);
+        expect(camera.getPitch()).toBeCloseTo(8.116189398053095);
+        expect(camera.getBearing()).toBeCloseTo(15.041890412599061);
+    });
+
     test('can be called from within a moveend event handler', async () => {
         const camera = createCamera();
         const stub = jest.spyOn(browser, 'now');
@@ -2713,13 +2775,15 @@ describe('#easeTo globe projection', () => {
 
         test('rolls to specified roll', () => {
             const camera = createCameraGlobe();
+            camera.setRollEnabled(true);
             camera.easeTo({pitch: 1, roll: 45, duration: 0});
             expect(camera.getPitch()).toBeCloseTo(1, 6);
             expect(camera.getRoll()).toBeCloseTo(45, 6);
         });
 
         test('roll behavior at Euler angle singularity', () => {
             const camera = createCameraGlobe();
+            camera.setRollEnabled(true);
             camera.easeTo({bearing: 0, pitch: 0, roll: 45, duration: 0});
             expect(camera.getRoll()).toBeCloseTo(45, 6);
             expect(camera.getPitch()).toBeCloseTo(0, 6);
@@ -2728,6 +2792,7 @@ describe('#easeTo globe projection', () => {
 
         test('bearing behavior at Euler angle singularity', () => {
             const camera = createCameraGlobe();
+            camera.setRollEnabled(true);
             camera.easeTo({bearing: 45, pitch: 0, roll: 0, duration: 0});
             expect(camera.getRoll()).toBeCloseTo(0, 6);
             expect(camera.getPitch()).toBeCloseTo(0, 6);
@@ -3032,13 +3097,15 @@ describe('#flyTo globe projection', () => {
 
         test('rolls to specified roll', () => {
             const camera = createCameraGlobe();
+            camera.setRollEnabled(true);
             camera.flyTo({pitch: 1, roll: 45, animate: false});
             expect(camera.getPitch()).toBeCloseTo(1, 6);
             expect(camera.getRoll()).toBeCloseTo(45, 6);
         });
 
         test('roll behavior at Euler angle singularity', () => {
             const camera = createCameraGlobe();
+            camera.setRollEnabled(true);
             camera.flyTo({bearing: 0, pitch: 0, roll: 45, animate: false});
             expect(camera.getRoll()).toBeCloseTo(45, 6);
             expect(camera.getPitch()).toBeCloseTo(0, 6);
@@ -3047,6 +3114,7 @@ describe('#flyTo globe projection', () => {
 
         test('bearing behavior at Euler angle singularity', () => {
             const camera = createCameraGlobe();
+            camera.setRollEnabled(true);
             camera.flyTo({bearing: 45, pitch: 0, roll: 0, animate: false});
             expect(camera.getRoll()).toBeCloseTo(0, 6);
             expect(camera.getPitch()).toBeCloseTo(0, 6);

src/ui/camera.ts

@@ -746,6 +746,10 @@ export abstract class Camera extends Evented {
         return this;
     }
 
+    setRollEnabled(rollEnabled: boolean) {
+        this.cameraHelper.setRollEnabled(rollEnabled);
+    }
+
     /**
      * @param bounds - Calculate the center for these bounds in the viewport and use
      * the highest zoom level up to and including `Map#getMaxZoom()` that fits

src/ui/map.ts

@@ -643,6 +643,7 @@ export class Map extends Camera {
         this._trackResize = resolvedOptions.trackResize === true;
         this._bearingSnap = resolvedOptions.bearingSnap;
         this._centerClampedToGround = resolvedOptions.centerClampedToGround;
+        this.setRollEnabled(resolvedOptions.rollEnabled === true);
         this._refreshExpiredTiles = resolvedOptions.refreshExpiredTiles === true;
         this._fadeDuration = resolvedOptions.fadeDuration;
         this._crossSourceCollisions = resolvedOptions.crossSourceCollisions === true;