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

[Bug] fork MapLibre terrain 3D jitter up & down as we pan #2211

Open
jawensi opened this issue Jun 21, 2023 · 9 comments
Open

[Bug] fork MapLibre terrain 3D jitter up & down as we pan #2211

jawensi opened this issue Jun 21, 2023 · 9 comments
Labels

Comments

@jawensi
Copy link

jawensi commented Jun 21, 2023

Description

When I install MapLibre fork and load a 3D terrain, there is an effect in the navigation. When you make pan with the mouse there is a change in the zoom level. This doesn't happen when you rotate the view.

React.App.-.Google.Chrome.2023-06-21.13-15-34.mp4

Expected Behavior

The expected behaviour is to keep the zoom level constant, avoiding this effect. When is used a MapBox fork, the jitter effect doesn't happen.

Steps to Reproduce

import maplibregl from "maplibre-gl";
import "maplibre-gl/dist/maplibre-gl.css";
import Map, { Source } from "react-map-gl";

export const MyMap = () => {

  return (
    <div style={{ height: "100%", width: "100%" }}>
      <Map
        id="map"
        initialViewState={{
          longitude: 11,
          latitude: 49,
          zoom: 4,
        }}
        mapLib={maplibregl}
        minZoom={0}
        maxZoom={22}
        mapStyle={
          "https://api.maptiler.com/maps/streets/style.json?key=get_your_own_OpIi9ZULNHzrESv6T2vL"
        }
        terrain={{ exaggeration: 1.3, source: "terrainLayer" }}
      >
        <Source
          id="terrainLayer"
          type="raster-dem"
          tiles={[
            "https://wms.wheregroup.com/dem_tileserver/raster_dem/{z}/{x}/{y}.webp",
          ]}
          tileSize={256}
        />
      </Map>
    </div>
  );
};

Environment

  • Framework version: react-map-gl@7.0.23
  • Map library: maplibre-gl@2.4.0,
  • Browser: Chrome 113.0
  • OS: windows 11

Logs

No response

@jawensi jawensi added the bug label Jun 21, 2023
@jawensi jawensi changed the title fork MapLibre terrain 3D jitter up & down as we pan [Bug] [Bug] fork MapLibre terrain 3D jitter up & down as we pan Jun 21, 2023
@Pessimistress
Copy link
Collaborator

The same behaviour when is used a MapBox fork

Not sure what "fork" you are talking about. If you mean Mapbox, this example is built with the latest mapbox-gl and works fine: https://visgl.github.io/react-map-gl/examples/terrain

@jawensi
Copy link
Author

jawensi commented Jun 22, 2023

The bug only appears when you use MapLibre, adding the property 'mapLib' to the Map component and import MapLibre

mapLib={maplibregl}

@mszekiel
Copy link

mszekiel commented Sep 6, 2023

It updates the camera elevation based on the terrain mesh. For version 3.1.0 I can see it's following the terrain during panning, but for latest 3.3.1 it updates the camera position only after panning is finished (idle event I guess). Problem doesn't exists when using mapbox, only when using maptiler service. I guess it is maplibre bug? But problem doesn't exist in maplibre JS examples with terrain.

@pviejo
Copy link

pviejo commented Nov 10, 2023

Any update on this?

@gauthierelcapitan
Copy link

Same behavior here, you guys have any workaround ? The bug is on MapLibre side any issue already exists ?

@lymperis-e
Copy link

lymperis-e commented Apr 2, 2024

Same behavior here, you guys have any workaround ? The bug is on MapLibre side any issue already exists ?

I believe the issue is with react-map-gl rather than maplibre. I 've been looking into it (I admit that I am not very familiar with the internals of either project, although I use maplibre a lot), and I believe the issue has to do with the way react-map-gl applies the ViewState changes.

The implementation is based on mapbox's Transform class, which is significantly different from maplibre's . My understanding is that react-map-gl watches for both user and map changes to the view state, and tries to reconcile the two and apply them smoothly, similar to map.jumpTo, as per the author. This is method that implements that:

src/mapbox/mapbox.ts

  // Adapted from map.jumpTo
  /* Update camera to match props
     @param {object} nextProps
     @param {bool} triggerEvents - should fire camera events
     @returns {bool} true if anything is changed
   */
  _updateViewState(nextProps: MapboxProps<StyleT>, triggerEvents: boolean): boolean {
    if (this._internalUpdate) {
      return false;
    }
    const map = this._map;

    const tr = this._renderTransform;
    // Take a snapshot of the transform before mutation
    const {zoom, pitch, bearing} = tr;
    const isMoving = map.isMoving();

    if (isMoving) {
      // All movement of the camera is done relative to the sea level
      tr.cameraElevationReference = 'sea';
    }
    const changed = applyViewStateToTransform(tr, {
      ...transformToViewState(map.transform),
      ...nextProps
    });
    if (isMoving) {
      // Reset camera reference
      tr.cameraElevationReference = 'ground';
    }

    if (changed && triggerEvents) {
      const deferredEvents = this._deferredEvents;
      // Delay DOM control updates to the next render cycle
      deferredEvents.move = true;
      deferredEvents.zoom ||= zoom !== tr.zoom;
      deferredEvents.rotate ||= bearing !== tr.bearing;
      deferredEvents.pitch ||= pitch !== tr.pitch;
    }

    // Avoid manipulating the real transform when interaction/animation is ongoing
    // as it would interfere with Mapbox's handlers
    if (!isMoving) {
      applyViewStateToTransform(map.transform, nextProps);
    }

    return changed;
  }

So when panning, the wrapper effectively instructs the underlying library to set a new view state (including a new lon/lat). Mapbox's Transform has the concept of transform.cameraElevationReference = 'sea' | 'ground . Maplibre on the other hand does not.
It has the attribute transform.minElevationForCurrentTile, which queries the underlying terrain tile for its elevation. This attribute is updated with the _updateElevation method of maplibre's Camera class, as seen below:

maplibre-gl-js/src/ui/camera.ts

    _updateElevation(k: number) {
        this.transform.minElevationForCurrentTile = this.terrain.getMinTileElevationForLngLatZoom(this._elevationCenter, this.transform.tileZoom);
        const elevation = this.terrain.getElevationForLngLatZoom(this._elevationCenter, this.transform.tileZoom);
        // target terrain updated during flight, slowly move camera to new height
        if (k < 1 && elevation !== this._elevationTarget) {
            const pitch1 = this._elevationTarget - this._elevationStart;
            const pitch2 = (elevation - (pitch1 * k + this._elevationStart)) / (1 - k);
            this._elevationStart += k * (pitch1 - pitch2);
            this._elevationTarget = elevation;
        }
        this.transform.elevation = interpolates.number(this._elevationStart, this._elevationTarget, k);
    }

Now, what I assumed based on the above, is that when maplibre is imperatively instructed to update the ViewState, it recalculates the camera height, based on the new central lat/lng tile's elevation (this._elevationCenter)


Adding a console log, and destructuring elevation from tr, I observed that indeed when panning, the transform returns different elevations, whereas when rotating it returns the same elevation. The map.transform.elevation property, stays the same.

src/mapbox/mapbox.ts

    // Take a snapshot of the transform before mutation
    const {zoom, pitch, bearing, elevation} = tr;
    const isMoving = map.isMoving();

    console.log(
      `map camera elevation: ${map.transform.elevation}, elevation: ${elevation}`
    );

image

Workaround (possibly)

A possible workaround would thus be to capture the map.transform.elevation, and enforce it on the updated view state, instead of using the tr.elevation value. I have not quite figured out how the transform is applied to produce the next view state, although I tried several things, like the example below. Any maintainer help would be more than welcome!

const changed = applyViewStateToTransform(tr, {
      ...transformToViewState(map.transform),
      ...nextProps,
      elevation: map.transform.elevation
    });

@lymperis-e
Copy link

Any update on this?

@ggaans
Copy link

ggaans commented May 10, 2024

Any progress or likely hood of a fix?

@Fabioni
Copy link

Fabioni commented May 23, 2024

I just started using this library an hour ago and already ran into this problem. What is the status?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants