Remove pitching and clipping while zooming or dragging with terrain (#12354)

* remove code that pitches camera above terrain

* update integration tests - since pitch is not being adjusted

* add new render test to show camera stops and doesn't pitch when colliding with terrain

* fix clipping terrain with drag gestures

Co-authored-by: Aidan H <[email protected]>
Co-authored-by: Karim Naaji <[email protected]>

Author: 3 people

src/geo/transform.js

@@ -149,6 +149,7 @@ class Transform {
     _nearZ: number;
     _farZ: number;
     _mercatorScaleRatio: number;
+    _isCameraConstrained: boolean;
 
     constructor(minZoom: ?number, maxZoom: ?number, minPitch: ?number, maxPitch: ?number, renderWorldCopies: boolean | void, projection?: ?ProjectionSpecification, bounds: ?LngLatBounds) {
         this.tileSize = 512; // constant
@@ -225,13 +226,14 @@ class Transform {
         this._updateCameraOnTerrain();
         this._calcMatrices();
     }
-    updateElevation(constrainCameraOverTerrain: boolean) { // On render, no need for higher granularity on update reasons.
+
+    updateElevation(constrainCameraOverTerrain: boolean, adaptCameraAltitude: boolean = false) {
         const centerAltitudeChanged = this._elevation && this._elevation.exaggeration() !== this._centerAltitudeValidForExaggeration;
         if (this._seaLevelZoom == null || centerAltitudeChanged) {
             this._updateCameraOnTerrain();
         }
         if (constrainCameraOverTerrain || centerAltitudeChanged) {
-            this._constrainCameraAltitude();
+            this._constrainCamera(adaptCameraAltitude);
         }
         this._calcMatrices();
     }
@@ -1620,7 +1622,6 @@ class Transform {
     }
 
     recenterOnTerrain() {
-
         if (!this._elevation || this.projection.name === 'globe')
             return;
 
@@ -1659,41 +1660,41 @@ class Transform {
         }
     }
 
-    _constrainCameraAltitude() {
+    _constrainCamera(adaptCameraAltitude: boolean = false) {
         if (!this._elevation)
             return;
 
         const elevation: Elevation = this._elevation;
-        this._updateCameraState();
 
         // Find uncompensated camera position for elevation sampling.
         // The default camera position might have been compensated by the active projection model.
         const mercPixelsPerMeter = mercatorZfromAltitude(1, this._center.lat) * this.worldSize;
         const pos = this._computeCameraPosition(mercPixelsPerMeter);
-
         const elevationAtCamera = elevation.getAtPointOrZero(new MercatorCoordinate(...pos));
-        const minHeight = this._minimumHeightOverTerrain() * Math.cos(degToRad(this._maxPitch));
         const terrainElevation = this.pixelsPerMeter / this.worldSize * elevationAtCamera;
-        const cameraHeight = this._camera.position[2] - terrainElevation;
-
-        if (cameraHeight < minHeight) {
-            const center = this.locationCoordinate(this._center, this._centerAltitude);
-            const cameraToCenter = [center.x - pos[0], center.y - pos[1], center.z - pos[2]];
-            const prevDistToCamera = vec3.length(cameraToCenter);
-
-            // Adjust the camera vector so that the camera is placed above the terrain.
-            // Distance between the camera and the center point is kept constant.
-            cameraToCenter[2] -= (minHeight - cameraHeight) / this._pixelsPerMercatorPixel;
-
-            const newDistToCamera = vec3.length(cameraToCenter);
-            if (newDistToCamera === 0)
-                return;
-
-            vec3.scale(cameraToCenter, cameraToCenter, prevDistToCamera / newDistToCamera * this._pixelsPerMercatorPixel);
-            this._camera.position = [center.x - cameraToCenter[0], center.y - cameraToCenter[1], center.z * this._pixelsPerMercatorPixel - cameraToCenter[2]];
-
-            this._camera.orientation = orientationFromFrame(cameraToCenter, this._camera.up());
-            this._updateStateFromCamera();
+        const minHeight = this._minimumHeightOverTerrain();
+        const cameraHeight = pos[2] - terrainElevation;
+
+        if (cameraHeight <= minHeight) {
+            if (cameraHeight < 0 || adaptCameraAltitude) {
+                const center = this.locationCoordinate(this._center, this._centerAltitude);
+                const cameraToCenter = [pos[0], pos[1], center.z - pos[2]];
+
+                const prevDistToCamera = vec3.length(cameraToCenter);
+                // Adjust the camera vector so that the camera is placed above the terrain.
+                // Distance between the camera and the center point is kept constant.
+                cameraToCenter[2] -= (minHeight - cameraHeight) / this._pixelsPerMercatorPixel;
+                const newDistToCamera = vec3.length(cameraToCenter);
+
+                if (newDistToCamera === 0)
+                    return;
+
+                vec3.scale(cameraToCenter, cameraToCenter, prevDistToCamera / newDistToCamera * this._pixelsPerMercatorPixel);
+                this._camera.position = [pos[0], pos[1], center.z * this._pixelsPerMercatorPixel - cameraToCenter[2]];
+                this._updateStateFromCamera();
+            } else {
+                this._isCameraConstrained = true;
+            }
         }
     }
 
@@ -1754,7 +1755,7 @@ class Transform {
             this.zoom += this.scaleZoom(s);
         }
 
-        this._constrainCameraAltitude();
+        this._constrainCamera();
         this._unmodified = unmodified;
         this._constraining = false;
     }
@@ -2041,10 +2042,10 @@ class Transform {
 
     _minimumHeightOverTerrain(): number {
         // Determine minimum height for the camera over the terrain related to current zoom.
-        // Values above than 2 allow max-pitch camera closer to e.g. top of the hill, exposing
+        // Values above 4 allow camera closer to e.g. top of the hill, exposing
         // drape raster overscale artifacts or cut terrain (see under it) as it gets clipped on
         // near plane. Returned value is in mercator coordinates.
-        const MAX_DRAPE_OVERZOOM = 2;
+        const MAX_DRAPE_OVERZOOM = 4;
         const zoom = Math.min((this._seaLevelZoom != null ? this._seaLevelZoom : this._zoom) + MAX_DRAPE_OVERZOOM, this._maxZoom);
         return this._mercatorZfromZoom(zoom);
     }

src/render/painter.js

@@ -185,15 +185,15 @@ class Painter {
         this._backgroundTiles = {};
     }
 
-    updateTerrain(style: Style, cameraChanging: boolean) {
+    updateTerrain(style: Style, adaptCameraAltitude: boolean) {
         const enabled = !!style && !!style.terrain && this.transform.projection.supportsTerrain;
         if (!enabled && (!this._terrain || !this._terrain.enabled)) return;
         if (!this._terrain) {
             this._terrain = new Terrain(this, style);
         }
         const terrain: Terrain = this._terrain;
         this.transform.elevation = enabled ? terrain : null;
-        terrain.update(style, this.transform, cameraChanging);
+        terrain.update(style, this.transform, adaptCameraAltitude);
     }
 
     _updateFog(style: Style) {

src/terrain/terrain.js

@@ -285,9 +285,8 @@ export class Terrain extends Elevation {
      * Validate terrain and update source cache used for elevation.
      * Explicitly pass transform to update elevation (Transform.updateElevation)
      * before using transform for source cache update.
-     * cameraChanging is true when camera is zooming, panning or orbiting.
      */
-    update(style: Style, transform: Transform, cameraChanging: boolean) {
+    update(style: Style, transform: Transform, adaptCameraAltitude: boolean) {
         if (style && style.terrain) {
             if (this._style !== style) {
                 this.style = style;
@@ -324,9 +323,9 @@ export class Terrain extends Elevation {
             }
 
             updateSourceCache();
-            // Camera, when changing, gets constrained over terrain. Issue constrainCameraOverTerrain = true
-            // here to cover potential under terrain situation on data or style change.
-            transform.updateElevation(!cameraChanging);
+            // Camera gets constrained over terrain. Issue constrainCameraOverTerrain = true
+            // here to cover potential under terrain situation on data, style, or other camera changes.
+            transform.updateElevation(true, adaptCameraAltitude);
 
             // Reset tile lookup cache and update draped tiles coordinates.
             this.resetTileLookupCache(this.proxySourceCache.id);

src/ui/handler/keyboard.js

@@ -125,11 +125,11 @@ class KeyboardHandler {
         return {
             cameraAnimation: (map: Map) => {
                 const zoom = map.getZoom();
+
                 map.easeTo({
                     duration: 300,
                     easeId: 'keyboardHandler',
                     easing: easeOut,
-
                     zoom: zoomDir ? Math.round(zoom) + zoomDir * (e.shiftKey ? 2 : 1) : zoom,
                     bearing: map.getBearing() + bearingDir * this._bearingStep,
                     pitch: map.getPitch() + pitchDir * this._pitchStep,

src/ui/handler_manager.js

@@ -322,6 +322,10 @@ class HandlerManager {
         return !!isMoving(this._eventsInProgress) || this.isZooming();
     }
 
+    _isDragging(): boolean {
+        return !!this._eventsInProgress.drag;
+    }
+
     _blockedByActive(activeHandlers: { [string]: Handler }, allowed: Array<string>, myName: string): boolean {
         for (const name in activeHandlers) {
             if (name === myName) continue;
@@ -491,12 +495,22 @@ class HandlerManager {
             if (preZoom !== tr.zoom) this._map._update(true);
         }
 
+        // Catches double click and double tap zooms when camera is constrained over terrain
+        if (tr._isCameraConstrained) map._stop(true);
+
         if (!hasChange(combinedResult)) {
             this._fireEvents(combinedEventsInProgress, deactivatedHandlers, true);
             return;
         }
+
         let {panDelta, zoomDelta, bearingDelta, pitchDelta, around, aroundCoord, pinchAround} = combinedResult;
 
+        if (tr._isCameraConstrained) {
+            // Catches wheel zoom events when camera is constrained over terrain
+            if (zoomDelta > 0) zoomDelta = 0;
+            tr._isCameraConstrained = false;
+        }
+
         if (pinchAround !== undefined) {
             around = pinchAround;
         }
@@ -582,7 +596,6 @@ class HandlerManager {
         this._map._update();
         if (!combinedResult.noInertia) this._inertia.record(combinedResult);
         this._fireEvents(combinedEventsInProgress, deactivatedHandlers, true);
-
     }
 
     _fireEvents(newEventsInProgress: { [string]: Object }, deactivatedHandlers: Object, allowEndAnimation: boolean) {

src/ui/map.js

@@ -1348,6 +1348,10 @@ class Map extends Camera {
         return this._rotating || (this.handlers && this.handlers.isRotating()) || false;
     }
 
+    _isDragging(): boolean {
+        return (this.handlers && this.handlers._isDragging()) || false;
+    }
+
     _createDelegatedListener(type: MapEvent, layers: Array<any>, listener: any): any {
         if (type === 'mouseenter' || type === 'mouseover') {
             let mousein = false;
@@ -3410,7 +3414,8 @@ class Map extends Camera {
     _updateTerrain() {
         // Recalculate if enabled/disabled and calculate elevation cover. As camera is using elevation tiles before
         // render (and deferred update after zoom recalculation), this needs to be called when removing terrain source.
-        this.painter.updateTerrain(this.style, this.isMoving() || this.isRotating() || this.isZooming());
+        const adaptCameraAltitude = this._isDragging();
+        this.painter.updateTerrain(this.style, adaptCameraAltitude);
     }
 
     _calculateSpeedIndex(): number {

test/integration/render-tests/debug/terrain/camera-collides-with-terrain/expected.png

@@ -0,0 +1,48 @@
+{
+  "version": 8,
+  "metadata": {
+    "test": {
+      "height": 256,
+      "width": 256
+    }
+  },
+  "center": [
+    -104.93611234965806,
+    38.85724324489064
+  ],
+  "zoom": 22,
+  "pitch": 85,
+  "bearing": -79,
+  "terrain": {
+    "source": "rgbterrain",
+    "exaggeration": 1
+  },
+  "sources": {
+    "rgbterrain": {
+      "type": "raster-dem",
+      "tiles": [
+        "local://tiles/12-759-1609.terrain.png"
+      ],
+      "maxzoom": 11,
+      "tileSize": 256
+    }
+  },
+  "glyphs": "local://glyphs/{fontstack}/{range}.pbf",
+  "layers": [
+    {
+      "id": "background",
+      "type": "background",
+      "paint": {
+        "background-color": "gray"
+      }
+    },
+    {
+      "id": "hillshade-translucent",
+      "type": "hillshade",
+      "source": "rgbterrain",
+      "paint": {
+        "hillshade-exaggeration": 1
+      }
+    }
+  ]
+}