-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Comments
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 |
The bug only appears when you use MapLibre, adding the property 'mapLib' to the Map component and import MapLibre
|
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 ( |
Any update on this? |
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 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}`
); 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
}); |
Any update on this? |
Any progress or likely hood of a fix? |
I just started using this library an hour ago and already ran into this problem. What is the status? |
@Pessimistress whats the status on this? This is a major flaw in the that library. 🙈 |
@lymperis-e did you did find any workaround ? Anyone has a status on this bug ? @Pessimistress how to help ? |
Still encounter this bug with maplibre. Exact same setting using clean maplibre work fine though |
My solution was to write a lighter wrapper around |
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
Environment
Logs
No response
The text was updated successfully, but these errors were encountered: