mapbox-gl-js
mapbox-gl-js copied to clipboard
Alternate zoom with terrain
Fixes #10197
Previously, constraining camera altitude avoided going under terrain by changing the pitch along with the camera orientation (see line #1685). However, this has resulted in a not ideal user experience when zooming into terrain and the camera is pitched when that is not the desired interaction.
This change stops zooming when the camera is at an unsafe distance from terrain (e.g. when seeing clipped terrain may be possible). It also pushes the camera above terrain when it is under or when dragging (which is helpful for when a user is trying to traverse across the terrain but zooms too closely).
Launch Checklist
- [x] briefly describe the changes in this PR
- [x] include before/after visuals or gifs if this PR includes visual changes
- [x] write tests for all new functionality
- [x] manually test the debug page
- [x] tagged
@mapbox/gl-native
if this PR includes shader changes or needs a native port - [x] apply changelog label ('bug', 'feature', 'docs', etc) or use the label 'skip changelog'
- [x] add an entry inside this element for inclusion in the
mapbox-gl-js
changelog:<changelog>Allow zooming towards terrain at a safe distance without pitching the camera</changelog>
@SnailBones I updated the keyboard movement (i.e. panning forward and zooming) to be consistent with scroll zooming. The check for cameraHeight less than zero is for when camera is actually under the terrain (which would happen in the case of switching on and off terrain) since the logic there forcefully pushes the camera above it... in cases of zooming though we just want to stop the camera from getting too close (and not forcefully adjust the camera position elsewhere). Thanks for bringing keyboard movements up!
Thanks for the thorough review @SnailBones - Yes I agree it feels a bit risky to place code directly in keyboard. In order to stop zooming though I do need to intercept the gestures to stop the action. I might look into establishing an internal method that can be used to stop gestures in place on the map?
- I'm still noticing that keyboard panning (arrow keys) is stopped by hills instead of being zoomed out like mouse panning. I don't think it's necessarily a problem but it's worth considering the alternative.
Do you mean panning by drag panning using the mouse? This triggers isDragging to be true, unfortunately testing this logic out for keyboard movements doesn't look natural like it it does for drag pan.
- Also noticing a strange behavior with keyboard rotation (shift + arrow keys) into a mountain, where the adjusted position seems quite far away and behind the starting position. I presume this is happening because it's triggering a terrain clip (though I'm not sure why the distance is so large, it might be worth walking through the logic in the case of non-drag clips)
Yes it is from the calculation, so there's a jump when rotating (if you are getting close to being under terrain) - would it be better to stop rotation like zooming? Otherwise we do have to adjust the camera position to allow rotating otherwise we risk terrain clip.
Thanks for the review @SnailBones
I'm also noticing that in some cases, keyboard panning is halted even when it seems like it should allow movement. Again, still an improvement over the changing pitch behavior in
main
.
I can remove the constraint on keyboard forward panning. It doesn't risk clipping. Here is is without constraining for forward panning
https://user-images.githubusercontent.com/42715836/207391843-d28948e6-d184-4a67-a381-6213b01fb81d.mov
I'm also noticing that in some cases, keyboard panning is halted even when it seems like it should allow movement. Again, still an improvement over the changing pitch behavior in
main
.I can remove the constraint on keyboard forward panning. It doesn't risk clipping. Here is is without constraining for forward panning
Do you know if that makes a difference in situations where the camera is grazing a peak, and is blocked by terrain that's not visible onscreen (like in the video I share above)?
This issue doesn't occur on mouse/drag panning due to the behavior you've implemented with the camera zooming out and rising above the obstacle terrain. My intuition is that extending this behavior to keyboard panning would solve this issue (and perhaps ease map navigation by keyboard) But given you tested this and found it looked less natural, I'm happy to merge this as is, and the possibility of getting stuck above a peak is a suitably small edge case that we can address it in the future.
@SnailBones It looks like grazing peak does not fall into the camera jump code path with dragging for keyboarding panning. I have tested changing the camera position (as in dragging) in keyboard movements and it does not feel intuitive. However, I don't believe keyboard panning needs to be accounted for here. Maybe the behavior for keyboard panning has changed in main
since I first opened this PR, as keyboard movements (aside from rotating - which does use the camera change code path to avoid clipping) seem acceptable without being constrained.
https://user-images.githubusercontent.com/42715836/207428264-e9929e1d-358e-4cd7-81dd-d5432d555a8d.mov