When simulating a GPS bearing the map zooms in/out with terrain

[HarelM]

maplibre-gl-js version: 4.1.1
browser: chrome

Steps to Trigger Behavior

See below.

1. Set terrain
2. Move the bearing slowly back and forth

Link to Demonstration

https://jsbin.com/citaminibe/2/edit?html,js,output

Expected Behavior

Like in 2D - no movement

Actual Behavior

Depend on the terrain, this will cause a slow zoom in or zoom out.

[HarelM]

This is the first bounty we are defining.
Link to parent Bounty: maplibre/maplibre#189

[olsen232]

You could argue that this is "working as intended" but certainly it is a bit indicative on the code here having gotten away from us over time, probably due to 3d terrain being added onto a system that wasn't originally designed for it (nobody's fault).

Simplest fix:
When requesting the easeTo, be sure to set freezeElevation to true.
See https://github.com/maplibre/maplibre-gl-js/blob/main/src/ui/camera.ts#L222
From the documentation:

Default false. Needed in 3D maps to let the camera stay in a constant height based on sea-level.

Simplest fix is applied here:
https://jsbin.com/wanucazeve/edit?html,js,output

So, that's why you could argue that this is okay - the documentation says this boolean should be set in a 3D map, and this is a 3D map, and if you set the boolean, things work okay. The documentation doesn't say what will happen if you forget to set the boolean, but it turns out, that this bug is what happens.

However, things could certainly be improved for the case where you forget to set freezeElevation.
Here is one option: #3944
Description from that PR:

This means that elevation logic has two distinct operating modes.
Mode one - freezeElevation is false - default:

• updateElevation is called continuously during the movement
• finalizeElevation is not called at the end.

Mode two - freezeElevation is true:

• updateElevation is not called continuously during the movement
• finalizeElevation is called at the end.

Previously, both methods were called in mode one, which lead to (this) issue.

However, note that this is not fully tested - you would have to try out a variety of different easeTo movements, both with freezeElevation set and unset, to make sure there were no regressions.

Another potential fix would be not to allow the client to supply freezeElevation, but have easeTo decide whether or not it is required in this case.

I don't really understand this code enough to fully understand why calling both updateElevation and finalizeElevation leads to a drift in the zoom over time. But, here's my high level understanding:

• Ordinarily when moving the map (or easing to a new bearing), the zoom should remain constant.
• However, when 3D terrain is active, the apparent altitude of the camera should remain constant. Since the scale that the terrain is drawn at and the offset it is drawn at are both functions of f(zoom, terrain-elevation), this means that zoom needs to be varied to compensate for a potentially varying terrain-elevation.
• However however, this compensation step only happens once at the end of the movement. During the movement, there are other invariants, but I don't yet understand what they are.
• Forgetting to set freezeElevation causes a mix of logic from different cases to happen, which caused some of the supposed invariants to drift.
• Continously updating the bearing shouldn't really cause these kind of issues since the terrain elevation should remain constant throughout. However, finding the terrain elevation depends on code that finds the screen's current centerpoint, which uses the coords buffer - this is surely subject to rounding errors.
  (In practise if you output the elevation of the camera's centrepoint each time it changes bearing, you'll see that it's fluctuating continuously around 1160 +/- 10 metres. If this could be improved, this would also mitigate this kind of issue.)

[olsen232]

PS If anyone wants to rewrite the transform code to make it more like a first-person-shooter - the world has some fixed co-ordinate system, its size never changes, and the camera's position is described (at the lowest level) in terms of its position within this fixed world - I'm all for it, and it would get rid of this class of bugs where the different types of invariants get mixed up. However, I understand this would be a huge breaking change and I am not volunteering.

[HarelM]

There's also the maps' level of details which is zoom defined by the spec, which also complicated everything further.
I tend to say that freezeElevation should be "on" when using terrain much like the freeze elevation that happens when panning, but allow the user to change this of needed.
But I think it's a breaking change, so changing that behavior might need to wait for version 5, maybe, IDK.
@prozessor13 do you remember why you added this flag?

[olsen232]

FYI I roughly diagnosed this issue but I don't have any strong feelings about what happens next - I could merge the linked PR #3944 if useful

[HarelM]

Let's push the PR forward, thanks!