-
-
Notifications
You must be signed in to change notification settings - Fork 790
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
Quick vertical dragging of pitched 3d map with mouse results in zoom action [3.0.0-pre.3] #2094
Comments
When I started playing around with the 3.0.0 pre-release, I noticed that when I drag the mouse, then hold it and look at the map, as soon as I release the mouse the map moves a very tiny amount. As if the last on_mousemove were not the final position, but the on_mouseup would do something else which finally changes this position. For some reason I believe that this could be related to changes introduced to fix the issue of the map not zooming into the point where the cursor position is located. |
Thanks for sharing! I'll check with version 3.0.0-pre.2 to see if this is related to the fix I did. |
I just checked, 3.0.0-pre.2 does not have the issue: |
Related bug: #1024 fixed by PR: #1977. |
I can't notice it when I slowly pan, but that doesn't mean that it's not happening. It seems to be related to mow much is panned in a certain amount of time. The thing I mentioned in my first comment, where releasing the mouse which was dragged and held then moves the map a tiny amount, seems to be unrelated, because 3.0.0-pre.2 does exhibit the same "bug", but is does not have this zooming issue. Since it is possible to drag the mouse around fast without having the zoom effect as long as the mouse is not released, it appears to be something which happens between the last I will look closer at the PR, but I'm already confused when looking at the changes in src/ui/handler/scroll_zoom.ts and src/ui/handler_manager.ts since I haven't worked on it. |
In pre.2 there was this It could also be some if else-if else branch execution which is getting skipped, something like this one which got merged into the opening |
This is the first bounty we are defining. |
Is anyone else able to replicate this issue? I'm willing to try to fix, but I can't recreate the zooming out situation no matter how I try to do the rapid panning. |
I can't reproduce this locally when running |
@HarelM I fixed the original demos, the issue is that MapTiler is no longer serving the I now use the demo tiles from https://demotiles.maplibre.org/ which are limited to the Innsbruck area. Yet it turns out that this bug occurs only where an actual height displacement is under the cursor. I also added a new demo using 3.0.0-pre.5 from last week and the problem still persists: https://output.jsbin.com/tuzorab/2 Hope this helps. |
I can't reproduce, sorry, which OS are you using? |
I created the issue while on Windows 7, the results from yesterday were on Windows 11. I now checked on Kubuntu 22.04 with Firefox and also have that issue there. Keep in mind that you need to make many fast mouse movements in of short durations one direction in order to clearly notice the effect. If you press and drag for large amounts, it will barely be noticeable. It may be related to texture loading? But it's always like this: panning to south zooms out, to north zooms in. |
I can't reproduce on win 11, chrome or fire fox, I tried hard to drag fast... |
Maybe this is related: It happens when releasing the cursor and may be dependent on where the cursor is placed and the zoom level it is at, but it can be very noticeable. No quick movements required. This is the location/zoom on the map: https://output.jsbin.com/tuzorab#8.21/46.832/11.683/0/45 |
This is a different issue (although related) which the elevation is adjusted at the end of the movement. This existed in 2.4 however. |
I think I managed to reproduce this without using the mouse by using small movements (compass). |
Now that you mention it, pressing the down key zooms out of the terrain, while the up-key zooms into the center. This feels wrong. Zoom out for a while, and as soon as the terrain is out of bounds, it begins to pan. Google Map's behavior is to always pan with the arrow keys and it feels like the correct thing to do. That might be related to this issue, but in any case I'm wondering if I should open a new one for this. While the issue in this entire thread is about is some kind of regression between 2.4.0 and 3.0.0, what I just mentioned is also occurring in 2.4.0. Edit: And panning sideways with the arrow keys seems to be really broken above terrain. |
Sounds like a different issue... |
I've added the following code to the jsbin you provided inside the map load event: let a = 5;
setInterval(() => {
a *= -1;
map.easeTo(
{
center,
bearing: 100 + a,
animate: true,
easing: (x) => x,
offset: [0, 100]
})
}, 200) In 2.4 it zooms in and in 3.1 it zooms out... strange... |
The entire panning behavior is confusing. It appears to work per drag for a short amount of time. And sometimes when zooming out (if one manages to do that) it tries to zoom in up to a certain point and then stays there. Also it's nearly impossible to pan sideways so much that the patch with the terrain stays out of view, for some reason it always tries to show it (possibly by zooming out). https://output.jsbin.com/biqoyusaha/1#9.67/47.3258/11.6297/5/45 |
Ha, no, no, this is using |
Ok, I thought that the center would adjust after each pan, but you're right, it's a fixed variable. But then, why is it possible to actually change the center by doing many small pans? It's possible to have the original center to be at the corner of the screen while back-and-forth rotation occurs at the center of the screen. Some internal variable appears to be getting modified slightly during the first step of a pan, some sort of offset. |
Ok, so I just upgraded the demo of this entire issue so that it uses version 3.1.0, and the issue still persists: https://output.jsbin.com/nepitazehe/1#13/47.27574/11.39085/0/45 But disregarding the sideway-panning, maybe the fact that the key-up and key-down events actually perform a zoom in and out respectively, when over terrain, instead of a pan forward / backward could be part of the explanation of what is happening with the mouse. Bear in mind that I don't known the code, but what if during the first animation frame a mouse-move-forward gets interpreted or handled as a key-up event and mouse-move-backwards as a key-down event, that could explain the slight zoom effect. Albeit "slight" is relative, because on fast, long "short-durationed" mouse movements the zoom effect is much stronger. It appears to be time-related. The time between the mouse-down and the mouse-up event will decide how much zooming will occur, also in relation to the amount which the mouse moved. Is there a way to simulate mouse events? |
@Pheonor any chance you can draw it and publish the drawing? If I understand correctly, instead of changing the zoom one can change the transform parameters to achieve the same results? Seems like a better course of action since the zoom is a "publicly" available parameter while the transform is more "internal", I think... |
May i have to clarify the camera-center-distance. I do not mean the variable So to get rid of all the center-elevation stuff, some other parameter (as @Pheonor explained) must represent that logic. And i think this can only be the zoom-value. So here are two drawings which explains switching from 2D to 3D. First the current state with the elevation-value, second to change zoom-value. But this scenario is only one to think about. But the next big issue is (as explained above): In drawing two you are then in zoom 15.4, but the distance from the camera to the peak of the mountain looks like, that we are in zoomlevel 17. And because all LevelOfDetail Logic is based on the current zoom, the peak is rendered with much to less details. So, i think this can all be changed, but is it worth?
@Pheonor: i do not fully understand what you mean. When letting the camera position as it is, the terrain beside the map-center has to offset in z-direction. But isn't that the same, than offset the camera in -z direction (as it is now)? |
To clarify, I propose to store the camera 3D position and to compute the other element as the Zoom Level from this 3D position wheen needed. Do you think the LOD system could use a Zoom Level computed from the sea level without taking acount the terrain elevation ? |
If i understand your question right: i think this is only be relevant when working with offsets, either camera in -z direction or render terrain in center-coordinate to sealevel and all around with a z-offset. So, i would say: this is exactly what currently is going on. On the other hand, in a scenario like drawing 2, i think the distance (in meters) from the camera to the center-elevation is relevant for the zoomlevel of which the LOD stuff is working on. If you mean that in your question: yes, i think this can be done. May it is easy do-able with a second, internal, zoom-level, but i am not sure if the visibility checks of the surrounding tiles is then working correct. I think it is an interesting idea to change the elevation-offset stuff, but i think this change needs some more efficient discussion, for example an "online-meeting". First to identify the problems with the current implementation, and second to build scenarios for a new implementation. Else i think this gets an endless discussion thread :) |
Can we use the next monthly meeting for this? It is in the coming Wednesday (10.1). Will you guys be able to join? |
Yes, i think this is possible for me. |
Yes, I will try to join. |
The notification from the rejection of the PR #3511 (comment) made me look at this issue again. I did just test this with 4.0.0-pre.4 and it appears to be fixed, at least in regards to mouse movement. I believe there are still some minor offsets which get introduced upon mouse release, but they are barely noticeable and don't seem to add up in a bad way. I don't know what changed with 4.0.0-pre.X and what the PR was supposed to do, so I won't comment on the rejection of the PR. A remaining problematic issue is that the navigation with the arrow keys is not behaving as expected. This might justify opening a new issue, but in my opinion it is strongly related to this one and also existed for as long as this one existed. Arrow up key zooms in when over terrain, and arrow down zooms out. When not over terrain, up and down are panning actions. For reference, this was @v3.1.0: https://output.jsbin.com/nepitazehe/1#13/47.27574/11.39085/0/45 This comment was tested on 4.0.0-pre.4: https://output.jsbin.com/rewogeneyi/1#13/47.27574/11.39085/0/45 You can reproduce this by first zooming out a bit in order to pan out of the terrain area more easily and pan north with the mouse until you're well out of the terrain area (a bit over München), then use the downward arrow key to navigate to the south. As soon as the terrain section appears, the southward panning changes to zooming out. As soon as the section is left again, it will switch to panning again. My apologies for not contributing code-wise. |
The "rejection" (I closed my own PR :-)) was a consequence of the discussion here and the discussion in the monthly meeting. My solution was simply too naive, as can be read in this thread. |
Regarding the arrow navigation, I just visited https://maplibre.org/maplibre-gl-js/docs/examples/3d-terrain/ and set the perspective to be perpendicular to the ground and used the arrow keys to navigate. An odd thing is that this zooming is not constantly present, but every couple of key presses it then pans instead of zooms. Regarding to the browser version change or something on my end, I just checked which version fixed the mouse panning issue, and while version 3.3.0 still had it, 3.4.0 no longer had it. I thought it was a fix related to 4.0 because I just tested the last one available, but it was 3.4.0. I can clearly reproduce the problem with 3.3.0. v3.3.0: https://output.jsbin.com/wohipew/1#13/47.27574/11.39085/0/45 v3.4.0: https://output.jsbin.com/qizucan/1#13/47.27574/11.39085/0/45 |
Navigation using the arrows is a different issue, I would recommend moving the discussion there once you open it. |
Thanks. I will open an issue but not right now. While creating the issue I also tested on a Linux bare bones installation as well as Linux and Windows inside VirtualBox and QEMU/KVM and noticed different behaviors so I cancelled the creation of the issue. I will first look into this before opening a new one. |
This issue seems to the same/related to an issue we are experincing at Koordinates with our maplibre map-viewer renderer (it uses openlayers by default but you can opt into maplibre and we are intending to replace openlayers with maplibre). Basically any pan action when you are zoomed out enough to see most of Australia (I assume the same effect happens in other locations but Australia reproduces it easily), as long as the camera centre is over Australia (having Australia in view but centre over the sea has no issues), causes you to zoom further out until you can't zoom any further. See this video for an example. This issue is only present with terrain enabled. Since I don't know where you can find an example to use with the maptiler terrain and up to date maplibre you can test this using our mapviewer at https://koordinates.com/data/ opt into the 3d map viewer from the modal that pops up (or opt into the 3D map viewer trial from the View menu in the mapviewer toolbar) This issue is present in v4.0.1 across all browsers/OS's we have tested (Vivaldi on Ubuntu 22 and Windows 12, Safari/Firefox/Chrome on macOS). I found this issue today while investigating the problem and found the changes in #3511, I took the changes in that PR and applied them as a patch to maplibre in our repo and verified that the problem is resolved. Not that there aren't other issues caused by the patch, noticeably around being closely zoomed into terrain and the map "whiting out", and separately of course you can now move the camera into terrain (I say separately even though you would think those two things are related because you can certainly position the camera inside the terrain without the map "whiting out") We're still doing testing on that change but I think we may accept the trade off of wonky camera terrain interactions for resolving the panning bug. As a company based in New Zealand panning over Australia is not an uncommon thing to do and having it zoom you out when you do that is... not great 😅 |
Hi, thank you for your explanation. I've encountered the same issue in Australia as well, which is beneficial for identifying the bug. As a temporary fix, you can adjust the minzoom setting of your terrain source to, for instance, 7 or 8. At these zoom levels, the visual disparity is minimal, thus mitigating the problem to some extent. Although the issue persists at higher zoom levels, it is considerably less pronounced. Additionally, I am currently swamped with tasks, making it impossible for me to address this issue in the near future. |
Interesting. Setting the minzoom to 7 for the terrain source solves the problem. Even when I can still see the terrain (I am zoomed out though so I can't tell if there's some fakery going with "2d" terrain at low zooms since it would be hard to tell the difference). So we've deployed that change to production and it's good enough for our purposes for now, thanks :) |
Hey, I ran into another glitch when checking out this issue. When I use the arrow keys to move from a spot with no terrain to one that does have terrain, right at the edge, the map starts freaking out and shaking like crazy. Do you think this is happening for the same reason you guys talked about before, something to do with the camera reference plate? Anyway, I'm trying to fix this in my spare time. |
Along the same lines: if you have implemented a Custom Layer Interface and you have terrain enabled and you find that your custom layer appears to be changing its Z-height relative to the terrain at the end of a panning motion - you might instead be experiencing the somewhat related but different issue #3797 |
|
As described in maplibre#2094 panning the map can result in a sudden jump in zoom-level as the drag is released. This jump tends to be worse when more zoomed out, and repeated short pans cause the zoom jumps to happen repeatedly in the same direction, which will grow exponentially if each one is zooming out - each zoom out causes the bug to get worse. The problem lies in recalculateZoom, which tries to calculate a new zoom level such that the previous camera altitude + terrain elevation is the same as the current camera altitude + terrain elevation. Camera altitude here refers specifically to the camera's height above the terrain elevation, and terrain elevation is the terrain height at the centre of the screen. (This doesn't really leave any good nouns for describing the sum of these two variables, but this sum should remain constant and is the camera's total height above sea level). A specific zoom maps - more-or-less - to a specific camera altitude. Since the terrain elevation can be different (eg 100m higher) at the end of a terrain drag, we need the camera altitude to be different too (eg 100m lower to compensate). So, we need to recalculate the zoom to one that maps to the required camera altitude. And so, the function recalculateZoom exists to fulfil this purpose. Unfortunately, the function miscalculates the required zoom, as can be verified by comparing camera altitude + terrain elevation before recalculateZoom is called vs after - very slight changes in terrain elevation (10s of metres) lead to large and unrelated changes in camera altitude (1000s of metres) when the camera is very zoomed out. Currently, the logic in recalculateZoom uses different math to the math used to calculate the camera's altitude from the current zoom. This different math may be close to correct, but, it's impossible for me to debug. In this commit I've simply replaced it with the inverse of the math used to calcualte the camera's altitude from its zoom - ie: if a zoom Z gives a camera altitude f(Z) Then, to get the camera to required altitude A, we need to set the zoom to Z = inverse-of-f(A)
The unwanted-zooming-when-finishing-panning behaviour is fixed by #3836 Any unwanted panning when finishing panning is not fixed, anything to do with keypresses is not fixed - this only affects how the zoom is calculated at the end of a pan. |
Added asserts that camera altitude remains invariant throughout the recalculateZoom test. These new asserts fail before the fix for 2094. Moved the asserts that the zoom is a particular value to after the altitude asserts - a particular zoom is correct if and only if it results in the correct altitude, so whether the altitude assert fails is more important information to have in a failing test. Updated CHANGELOG.md
* Fix for #2094 - unwanted zoom changes As described in #2094 panning the map can result in a sudden jump in zoom-level as the drag is released. This jump tends to be worse when more zoomed out, and repeated short pans cause the zoom jumps to happen repeatedly in the same direction, which will grow exponentially if each one is zooming out - each zoom out causes the bug to get worse. The problem lies in recalculateZoom, which tries to calculate a new zoom level such that the previous camera altitude + terrain elevation is the same as the current camera altitude + terrain elevation. Camera altitude here refers specifically to the camera's height above the terrain elevation, and terrain elevation is the terrain height at the centre of the screen. (This doesn't really leave any good nouns for describing the sum of these two variables, but this sum should remain constant and is the camera's total height above sea level). A specific zoom maps - more-or-less - to a specific camera altitude. Since the terrain elevation can be different (eg 100m higher) at the end of a terrain drag, we need the camera altitude to be different too (eg 100m lower to compensate). So, we need to recalculate the zoom to one that maps to the required camera altitude. And so, the function recalculateZoom exists to fulfil this purpose. Unfortunately, the function miscalculates the required zoom, as can be verified by comparing camera altitude + terrain elevation before recalculateZoom is called vs after - very slight changes in terrain elevation (10s of metres) lead to large and unrelated changes in camera altitude (1000s of metres) when the camera is very zoomed out. Currently, the logic in recalculateZoom uses different math to the math used to calculate the camera's altitude from the current zoom. This different math may be close to correct, but, it's impossible for me to debug. In this commit I've simply replaced it with the inverse of the math used to calcualte the camera's altitude from its zoom - ie: if a zoom Z gives a camera altitude f(Z) Then, to get the camera to required altitude A, we need to set the zoom to Z = inverse-of-f(A) * Tests for fix for #2094 (unwanted zoom changes) Added asserts that camera altitude remains invariant throughout the recalculateZoom test. These new asserts fail before the fix for 2094. Moved the asserts that the zoom is a particular value to after the altitude asserts - a particular zoom is correct if and only if it results in the correct altitude, so whether the altitude assert fails is more important information to have in a failing test. Updated CHANGELOG.md --------- Co-authored-by: Harel M <[email protected]>
@olsen232 if you want the bounty, please follow the relevant procedure. |
I've never collected a bounty before so I'm gonna do it |
While the main issue here is solved, there's still issues that I think are somewhat related I believe @olsen232 if you happen to want to test your fixes with the above issue let me know, I tend to think it's somewhat related to what you are working on. |
Thanks for fixing this issue @olsen232 and for submitting the expense https://opencollective.com/maplibre/expenses/193319 on opencollective. Can you fill out the developer application form on https://maplibre.org/jobs/ ? |
maplibre-gl-js version: 3.0.0-pre.3
browser: Google Chrome Version 109.0.5414.75 (Offizieller Build) (64-Bit)
Steps to Trigger Behavior
Link to Demonstration
https://output.jsbin.com/decicab/1 -- [email protected] -- broken behaviorhttps://output.jsbin.com/netasuw/1 -- [email protected] -- correct behavior(Does no longer work because of new MapTiler restrictions)
https://output.jsbin.com/decicab/11 -- [email protected] -- broken behavior
https://output.jsbin.com/netasuw/11 -- [email protected] -- correct behavior
(Terrain limited to Innsbruck area)
Expected Behavior
The altitude of the camera should remain at the same height, making a mouse up/down drag result only in panning of the map, like in version 2.4.0.
Actual Behavior
Panning has zooming added to it. So multiple quick drags backing off of the visible location result in not only panning backward, but also in zooming out. Multiple quick drags to fly over the visible location not only result in panning forward, but also in zooming in.
The text was updated successfully, but these errors were encountered: