Keep camera outside terrain

CHANGELOG.md

@@ -4,6 +4,7 @@
 - _...Add new stuff here..._
 
 ### 🐞 Bug fixes
+- Fix camera being able to move into 3D terrain ([#1542](https://github.com/maplibre/maplibre-gl-js/issues/1542))
 - _...Add new stuff here..._
 
 ## 4.5.1

src/ui/camera.test.ts

@@ -44,6 +44,7 @@ function createCamera(options?) {
         .jumpTo(options);
 
     camera._update = () => {};
+    camera._elevateCameraIfInsideTerrain = (_tr : any) => ({});
 
     return camera;
 }

src/ui/camera.ts

@@ -958,10 +958,10 @@ export abstract class Camera extends Evented {
         if (options.animate === false || (!options.essential && browser.prefersReducedMotion)) options.duration = 0;
 
         const tr = this._getTransformForUpdate(),
-            startZoom = this.getZoom(),
-            startBearing = this.getBearing(),
-            startPitch = this.getPitch(),
-            startPadding = this.getPadding(),
+            startZoom = tr.zoom,
+            startBearing = tr.bearing,
+            startPitch = tr.pitch,
+            startPadding = tr.padding,
 
             bearing = 'bearing' in options ? this._normalizeBearing(options.bearing, startBearing) : startBearing,
             pitch = 'pitch' in options ? +options.pitch : startPitch,
@@ -975,7 +975,7 @@ export abstract class Camera extends Evented {
             LngLat.convert(options.center || locationAtOffset),
             options.zoom ?? startZoom
         );
-        this._normalizeCenter(center);
+        this._normalizeCenter(center, tr);
 
         const from = tr.project(locationAtOffset);
         const delta = tr.project(center).sub(from);
@@ -1091,43 +1091,82 @@ export abstract class Camera extends Evented {
     /**
      * @internal
      * Called when the camera is about to be manipulated.
-     * If `transformCameraUpdate` is specified, a copy of the current transform is created to track the accumulated changes.
+     * If `transformCameraUpdate` is specified or terrain is enabled, a copy of
+     * the current transform is created to track the accumulated changes.
      * This underlying transform represents the "desired state" proposed by input handlers / animations / UI controls.
      * It may differ from the state used for rendering (`this.transform`).
      * @returns Transform to apply changes to
      */
     _getTransformForUpdate(): Transform {
-        if (!this.transformCameraUpdate) return this.transform;
+        if (!this.transformCameraUpdate && !this.terrain) return this.transform;
 
         if (!this._requestedCameraState) {
             this._requestedCameraState = this.transform.clone();
         }
         return this._requestedCameraState;
     }
 
+    /**
+     * @internal
+     * Checks the given transform for the camera being below terrain surface and
+     * returns new pitch and zoom to fix that.
+     *
+     * With the new pitch and zoom, the camera will be at the same ground
+     * position but at higher altitude. It will still point to the same spot on
+     * the map.
+     *
+     * @param tr - The transform to check.
+     */
+    _elevateCameraIfInsideTerrain(tr: Transform) : { pitch?: number; zoom?: number } {
+        const camera = tr.getCameraPosition();
+        const minAltitude = this.terrain.getElevationForLngLatZoom(camera.lngLat, tr.zoom);
+        if (camera.altitude < minAltitude) {
+            const newCamera = this.calculateCameraOptionsFromTo(
+                camera.lngLat, minAltitude, tr.center, tr.elevation);
+            return {
+                pitch: newCamera.pitch,
+                zoom: newCamera.zoom,
+            };
+        }
+        return {};
+    }
+
     /**
      * @internal
      * Called after the camera is done being manipulated.
      * @param tr - the requested camera end state
+     * If the camera is inside terrain, it gets elevated.
      * Call `transformCameraUpdate` if present, and then apply the "approved" changes.
      */
     _applyUpdatedTransform(tr: Transform) {
-        if (!this.transformCameraUpdate) return;
-
-        const nextTransform = tr.clone();
-        const {
-            center,
-            zoom,
-            pitch,
-            bearing,
-            elevation
-        } = this.transformCameraUpdate(nextTransform);
-        if (center) nextTransform.center = center;
-        if (zoom !== undefined) nextTransform.zoom = zoom;
-        if (pitch !== undefined) nextTransform.pitch = pitch;
-        if (bearing !== undefined) nextTransform.bearing = bearing;
-        if (elevation !== undefined) nextTransform.elevation = elevation;
-        this.transform.apply(nextTransform);
+        const modifiers : ((tr: Transform) => ReturnType<CameraUpdateTransformFunction>)[] = [];
+        if (this.terrain) {
+            modifiers.push(tr => this._elevateCameraIfInsideTerrain(tr));
+        }
+        if (this.transformCameraUpdate) {
+            modifiers.push(tr => this.transformCameraUpdate(tr));
+        }
+        if (!modifiers.length) {
+            return;
+        }
+        const finalTransform = tr.clone();
+        for (const modifier of modifiers) {
+            const nextTransform = finalTransform.clone();
+            const {
+                center,
+                zoom,
+                pitch,
+                bearing,
+                elevation
+            } = modifier(nextTransform);
+            if (center) nextTransform.center = center;
+            if (zoom !== undefined) nextTransform.zoom = zoom;
+            if (pitch !== undefined) nextTransform.pitch = pitch;
+            if (bearing !== undefined) nextTransform.bearing = bearing;
+            if (elevation !== undefined) nextTransform.elevation = elevation;
+            finalTransform.apply(nextTransform);
+        }
+        this.transform.apply(finalTransform);
     }
 
     _fireMoveEvents(eventData?: any) {
@@ -1232,10 +1271,10 @@ export abstract class Camera extends Evented {
         }, options);
 
         const tr = this._getTransformForUpdate(),
-            startZoom = this.getZoom(),
-            startBearing = this.getBearing(),
-            startPitch = this.getPitch(),
-            startPadding = this.getPadding();
+            startZoom = tr.zoom,
+            startBearing = tr.bearing,
+            startPitch = tr.pitch,
+            startPadding = tr.padding;
 
         const bearing = 'bearing' in options ? this._normalizeBearing(options.bearing, startBearing) : startBearing;
         const pitch = 'pitch' in options ? +options.pitch : startPitch;
@@ -1249,7 +1288,7 @@ export abstract class Camera extends Evented {
             LngLat.convert(options.center || locationAtOffset),
             options.zoom ?? startZoom
         );
-        this._normalizeCenter(center);
+        this._normalizeCenter(center, tr);
         const scale = tr.zoomScale(zoom - startZoom);
 
         const from = tr.project(locationAtOffset);
@@ -1450,8 +1489,7 @@ export abstract class Camera extends Evented {
 
     // If a path crossing the antimeridian would be shorter, extend the final coordinate so that
     // interpolating between the two endpoints will cross it.
-    _normalizeCenter(center: LngLat) {
-        const tr = this.transform;
+    _normalizeCenter(center: LngLat, tr: Transform) {
         if (!tr.renderWorldCopies || tr.lngRange) return;
 
         const delta = center.lng - tr.center.lng;

src/ui/handler/scroll_zoom.test.ts

@@ -303,6 +303,7 @@ describe('ScrollZoomHandler', () => {
         browserNow.mockReturnValue(now);
 
         const map = createMap();
+        map._elevateCameraIfInsideTerrain = (_tr : any) => ({});
         map._renderTaskQueue.run();
         map.terrain = {
             pointCoordinate: () => null
@@ -329,6 +330,7 @@ describe('ScrollZoomHandler', () => {
         browserNow.mockReturnValue(now);
 
         let map = createMap();
+        map._elevateCameraIfInsideTerrain = (_tr : any) => ({});
         map._renderTaskQueue.run();
         map.terrain = {
             pointCoordinate: () => null
@@ -353,6 +355,7 @@ describe('ScrollZoomHandler', () => {
 
         // do the same test on the bottom
         map = createMap();
+        map._elevateCameraIfInsideTerrain = (_tr : any) => ({});
         map._renderTaskQueue.run();
         map.terrain = {
             pointCoordinate: () => null

src/ui/handler/transform-provider.ts

@@ -7,8 +7,9 @@ import {LngLat} from '../../geo/lng_lat';
 /**
  * @internal
  * Shared utilities for the Handler classes to access the correct camera state.
- * If Camera.transformCameraUpdate is specified, the "desired state" of camera may differ from the state used for rendering.
- * The handlers need the "desired state" to track accumulated changes.
+ * If Camera.transformCameraUpdate is specified or terrain is enabled, the
+ * "desired state" of camera may differ from the state used for rendering. The
+ * handlers need the "desired state" to track accumulated changes.
  */
 export class TransformProvider {
     _map: Map;

src/ui/map_tests/map_terrian.test.ts

@@ -69,3 +69,36 @@ describe('getCameraTargetElevation', () => {
         expect(map.getCameraTargetElevation()).toBe(2000);
     });
 });
+
+describe('Keep camera outside terrain', () => {
+    test('Try to move camera into terrain', () => {
+        const map = createMap();
+
+        let terrainElevation = 10;
+        const terrainStub = {} as Terrain;
+        terrainStub.getElevationForLngLatZoom = jest.fn(
+            (_lngLat: LngLat, _zoom: number) => terrainElevation
+        );
+        map.terrain = terrainStub;
+
+        // Terrain elevation is 10 everywhere, we are above it at zoom level 15
+        // with pitch 45 deg.
+        map.jumpTo({center: [0.0, 0.0], bearing: 0, pitch: 45, zoom: 15});
+        const initialCamPosition = map.transform.getCameraPosition();
+        expect(initialCamPosition.altitude).toBeCloseTo(506, 0);
+
+        // Now we set the elevation to 5000 everywhere and try to jump to the
+        // same position. This would lead to a jump into the terrain, which
+        // must not be possible.
+        // Camera should be above the terrain, but at the same location as
+        // before and with decreased pitch.
+        terrainElevation = 5000;
+        map.jumpTo({center: [0.0, 0.0], pitch: 45, zoom: 15});
+
+        expect(map.transform.getCameraPosition().lngLat.lng).toBeCloseTo(initialCamPosition.lngLat.lng);
+        expect(map.transform.getCameraPosition().lngLat.lat).toBeCloseTo(initialCamPosition.lngLat.lat);
+        expect(map.transform.pitch).toBeLessThan(45);
+        expect(map.transform.getCameraPosition().altitude).toBeGreaterThan(initialCamPosition.altitude);
+        expect(map.transform.getCameraPosition().altitude).toBeGreaterThan(terrainElevation);
+    });
+});

test/build/min.test.ts

@@ -36,7 +36,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 = 800300;
+        const expectedBytes = 801147;
 
         expect(actualBytes - expectedBytes).toBeLessThan(increaseQuota);
         expect(expectedBytes - actualBytes).toBeLessThan(decreaseQuota);