rotateCamera does not work properly with terrain exaggeration

wiesehahn commented 1 year ago

If I try to let the camera rotate around a point this works as long as no terrain exaggeration is added, however with terrain the view constantly changes (zoom and center).

E.g.:

<!DOCTYPE html>
<html>
  <head>
    <title>Animate map camera around Point</title>
    <meta property="og:description" content="Animate the map camera around Point." />
    <meta charset='utf-8'>
    <meta name="viewport" content="width=device-width, initial-scale=1">
    <link rel='stylesheet' href='https://unpkg.com/maplibre-gl@3.2.1/dist/maplibre-gl.css' />
    <script src='https://unpkg.com/maplibre-gl@3.2.1/dist/maplibre-gl.js'></script>
    <style>
        body { margin: 0; padding: 0; }
        html, body, #map { height: 100%; }
    </style>
  </head>

  <body>
    <div id="map"></div>

    <script>
      const map = new maplibregl.Map({
        container: "map",
        style: 'https://api.maptiler.com/maps/streets/style.json?key=XXX',
        center: [10.615815, 51.799092],
        pitch: 60,
        maxPitch: 60,
        zoom: 10,
        attributionControl: false,
      });

      function rotateCamera(timestamp) {
        // clamp the rotation between 0 -360 degrees
        // Divide timestamp by 100 to slow rotation to ~10 degrees / sec
        map.rotateTo((timestamp / 100) % 360, {duration: 0});
        // Request the next frame of the animation.
        requestAnimationFrame(rotateCamera);
      }

      map.on('load', () => {

        // Start the animation.
        rotateCamera(0);

        map.addSource("terrain", {
              "type": "raster-dem",
              "url": "https://api.maptiler.com/tiles/terrain-rgb/tiles.json?key=XXX"
            });
           map.setTerrain({
               source: "terrain",
               exaggeration: 1.5 });

        });
    </script>
  </body>
</html>

HarelM commented 1 year ago

Yes, this is a side effect of the zoom adjustment at the end of a movement when the terrain is on. A bug we haven't figured out how to solve yet. Feel free to look into it. If you think a bounty will help let me know and I'll assign one.

prozessor13 commented 1 year ago

I think you have to play with the freezeElevation setting in AnimationOptions.

If this does not help, then there existed in previous (gone in this PR) maplibre versions the transform.freezeElevation logic, which was useful for exact such usecases, ala:

map.transform.freezeElevation = true;
const progress = 0
const play = () => {
   if (progress > 1) {
      map.transform.freezeElevation = false;
      map.transform.recalculateZoom(map.gl.terrain);
   } else {
      map.jumpTo({ heading: progress * 360 });
      progress += 0.01
      requestAnimationFrame(play);
  }
}
play();

So may a good solution would be to create some kind of animate function which takes a callback instead of an AnimationOptions Object. And this function could handle all the freezeElevation and requestAnimationFrame stuff.

HarelM commented 1 year ago

It's not gone, just moved to the camera/map object instead of being part of the transform.

prozessor13 commented 1 year ago

Do you mean map._freezeElevation? If yes, the problem with this could be, that all easeTo (and all children, flyTo, rotateTo, ...) method calls are setting _freezeElevation internally, so to use this from outside is not really possible. But i have not tested it, may the freezeElevation setting in rotoateTo is enough for the above code. If not, and because the old transform.freezeElevation logic was not easy to understand, i propose the following snippet to keep internal stuff internal and offer an interface to the outside:

in camera.ts:

export type Animation = {
   duration?: number;
   setup?: () => void;
   frame: (_: number) => JumpToOptions;
   finish?: () => void;
};

abstract class Camera extends Evented {
   ...

   animate(animation: Animation) {
      const duration = animation.duration || 300; // ms
      const start = Date.now();
      const render = () => {
         const progress = (Date.now() - start) / duration;
         if (progress >= 1) {
            this._freezeElevation = false;
            this.transform.recalculateZoom(this.terrain);
            animation.finish && animation.finish();
         } else {
            this.jumpTo(animation.frame(progress)); // use jumpTo because this not touches this._freezeElevation
            requestAnimationFrame(render);
         }
      };
      this._freezeElevation = true;
      animation.setup && animation.setup();
      render();
      return this;
   }

   ...
}

Usage:

map.animate({
   duration: 1000,
   frame: (progress) => {
      return { bearing: progress * 360 };
   }
});

prozessor13 commented 1 year ago

Ahh. currently realized, the _freezeElevation is the same as transform.freezeElevation. Beacuse _freezeElevation can also used in combination with jumpTo without any interaction. My fault. But anyhow. I think an animation interface would be a good thing. e.g. for route-flights, ...

HarelM commented 1 year ago

I think there's a ease method you can use when calling easeto or moveto, not sure, but this elevation issue needs to be solved out of the box regardless of adding a new API, there's also https://github.com/maplibre/maplibre-gl-js/issues/2094 which is another side effect of this issue.

twittner commented 9 months ago

While adding terrain data to Timatis Maps I ran into the same issue. It seems to be caused by _finalizeElevation https://github.com/maplibre/maplibre-gl-js/blob/71f092a73d87bca8f76231e37b5bba67cc3503e2/src/ui/camera.ts#L1084 which recalculates the zoom level as @HarelM mentioned. Since the distance changes when the camera rotates over terrain the zoom level is changed whereas for rotation around a centre it should stay constant. As a quick fix I added yet another parameter to animation options (lockTerrainZoom) which inhibits the call to recalculateZoom in _finalizeElevation. The option is implicitly set in rotateTo, resetNorth and resetNorthPitch:

diff --git a/src/ui/camera.ts b/src/ui/camera.ts
index 0631b1149..8fc2a256f 100644
--- a/src/ui/camera.ts
+++ b/src/ui/camera.ts
@@ -218,9 +218,14 @@ export type AnimationOptions = {
     /**
      * Default false. Needed in 3D maps to let the camera stay in a constant
      * height based on sea-level. After the animation finished the zoom-level will be recalculated in respect of
-     * the distance from the camera to the center-coordinate-altitude.
+     * the distance from the camera to the center-coordinate-altitude unless `lockTerrainZoom` is `true`.
      */
     freezeElevation?: boolean;
+
+    /**
+     * If `true`, the zoom level will not be recalculated after animation ends.
+     */
+    lockTerrainZoom?: boolean;
 };

 /**
@@ -549,7 +554,8 @@ export abstract class Camera extends Evented {
      */
     rotateTo(bearing: number, options?: AnimationOptions, eventData?: any): this {
         return this.easeTo(extend({
-            bearing
+            bearing,
+            lockTerrainZoom: true
         }, options), eventData);
     }

@@ -580,6 +586,7 @@ export abstract class Camera extends Evented {
         this.easeTo(extend({
             bearing: 0,
             pitch: 0,
+            lockTerrainZoom: true,
             duration: 1000
         }, options), eventData);
         return this;
@@ -1036,7 +1043,7 @@ export abstract class Camera extends Evented {
             this._fireMoveEvents(eventData);

         }, (interruptingEaseId?: string) => {
-            if (this.terrain) this._finalizeElevation();
+            if (this.terrain) this._finalizeElevation(!options.lockTerrainZoom);
             this._afterEase(eventData, interruptingEaseId);
         }, options as any);

@@ -1079,9 +1086,11 @@ export abstract class Camera extends Evented {
         this.transform.elevation = interpolates.number(this._elevationStart, this._elevationTarget, k);
     }

-    _finalizeElevation() {
+    _finalizeElevation(rezoom: boolean) {
         this._elevationFreeze = false;
-        this.transform.recalculateZoom(this.terrain);
+        if (rezoom) {
+            this.transform.recalculateZoom(this.terrain);
+        }
     }

     /**
@@ -1362,7 +1371,7 @@ export abstract class Camera extends Evented {
             this._fireMoveEvents(eventData);

         }, () => {
-            if (this.terrain) this._finalizeElevation();
+            if (this.terrain) this._finalizeElevation(!options.lockTerrainZoom);
             this._afterEase(eventData);
         }, options);

-- 
2.43.0

I have deployed a patched version (based on v3.6.2) which seems to work well. You can try at https://www.timatis.com/map-o-matic if you select animation around. It is probably too hacky to upstream but I wanted to share how I worked around this bug. Maybe it helps.

HarelM commented 9 months ago

Thanks for the info! Yes, seems a bit too hacky, but might help others nonetheless.

maplibre / maplibre-gl-js

rotateCamera does not work properly with terrain exaggeration #2937