Skip to content

fix: unfreeze terrain elevation before firing gesture end events - #8102

Draft
xvodddwannaG wants to merge 2 commits into
maplibre:mainfrom
xvodddwannaG:fix/7989-terrain-dragend-moveend-jump
Draft

fix: unfreeze terrain elevation before firing gesture end events#8102
xvodddwannaG wants to merge 2 commits into
maplibre:mainfrom
xvodddwannaG:fix/7989-terrain-dragend-moveend-jump

Conversation

@xvodddwannaG

@xvodddwannaG xvodddwannaG commented Aug 4, 2026

Copy link
Copy Markdown

Launch Checklist

Refs #7989.

_fireEvents dispatched the end events first and lifted the elevation freeze after, so recalculateZoomAndCenter ran between dragend and moveend. Over Chamonix the center moved 257-708 m across the ~2.5 ms gap.

This unfreezes the elevation above the end-event dispatch, which is the order easeTo already uses: _finalizeElevation (same unfreeze, same recalculateZoomAndCenter) runs before _afterEase fires moveend. The inertial gesture already goes down that path (inertialEase.freezeElevation = true); this gives the non-inertial one the same order.

Scope, since #7989 asks for more than this. What changes is the camera reported by the gesture's end events, dragend and zoomend on a terrain zoom, which now agree with moveend. Nothing moves on screen that didn't before.

The visible jump can't be removed while centerClampedToGround is on: if the ground rises under the center mid-gesture and the center has to stay on the ground, the camera has to move. Skipping the correction (@NeluQi's preference in the issue) removes the jump but leaves the center off-ground, and the correction likely reappears on the next gesture. Animating it after moveend removes the discontinuity without giving up the invariant, at the cost of motion nobody asked for. Happy to switch to either.

Rebased onto #8067; its _terrainGestureAnchorElevation reset moves with the rest of the block.

  • Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
  • Briefly describe the changes in this PR.
  • Link to related issues.
  • Write tests for all new functionality.
  • Add an entry to CHANGELOG.md under the ## main section.
  • Confirm you have read our AI policy here.

Assisted-By: Claude Code (Opus 5)

@xvodddwannaG
xvodddwannaG force-pushed the fix/7989-terrain-dragend-moveend-jump branch from 7f2f6fa to db5bbb9 Compare August 4, 2026 11:24
@codecov

codecov Bot commented Aug 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.10%. Comparing base (ab44bff) to head (6fd7b7e).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8102   +/-   ##
=======================================
  Coverage   93.09%   93.10%           
=======================================
  Files         300      300           
  Lines       24899    24899           
  Branches     6540     6540           
=======================================
+ Hits        23179    23181    +2     
+ Misses       1720     1718    -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@HarelM

HarelM commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Also please check if freeze elevation is always "released", I think there might be cases where it not, but I'm not sure...

@xvodddwannaG
xvodddwannaG force-pushed the fix/7989-terrain-dragend-moveend-jump branch from db5bbb9 to 50da5f8 Compare August 9, 2026 16:56
@xvodddwannaG

Copy link
Copy Markdown
Author

Rebased; still relevant, the test fails on main and passes with the change.

On the freeze: yes, outside the gesture path. easeTo and flyTo call _prepareElevation whenever terrain is set, which sets the flag, but only call _finalizeElevation when freezeElevation is passed. I checked all three: a gesture clears it, easeTo({center, duration}) and flyTo({center, duration}) end with it still true, and after that _handleTerrainDataEvent and _render skip the center setElevation.

@HarelM

HarelM commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

I'm not sure I fully follow the last statement, is there a bug that needs to be fixed with freeze elevation besides the bug you solves here, which as far as I understand is mostly about moving the fire a bit down the call logic...?

Also if you want this to be merged it needs to be in "ready to review" state, it's currently in draft.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants