Skip to content
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

Large screen: unwanted pan-jump as pan action finishes (3d terrain) #3935

Closed
olsen232 opened this issue Apr 3, 2024 · 4 comments · Fixed by #3940
Closed

Large screen: unwanted pan-jump as pan action finishes (3d terrain) #3935

olsen232 opened this issue Apr 3, 2024 · 4 comments · Fixed by #3940
Assignees
Labels
💰 bounty M Medium Bounty, USD 250 bug Something isn't working PR is more than welcomed Extra attention is needed terrain

Comments

@olsen232
Copy link
Contributor

olsen232 commented Apr 3, 2024

(This is a separate but somewhat related issue to #3872
That issue was that the map would often snap back to a previous position.
The distance snapped was relatively small in terms of metres, and it didn't depend on screen size.)

For certain screen sizes > 2048px wide (and probably similar issue for height, but will focus on width for the purposes of demonstrating this bug) - when the user has finished panning and lets go the mouse, it will suddenly jump to a new location. This jump can be quite large and obvious, and, is always in the same direction, regardless of which way the user panned.

maplibre-gl-js version: 4.1.2

browser: Brave

devicePixelRatio: 2 (not sure if it's relevant)

Steps to Trigger Behavior

Option A - with a large screen.

  1. Open test/examples/3d-terrain.html
  2. Resize map window to about 3000px wide and about 1000px high.
    You can check the map's size with map.transform.width and map.transform.height
  3. Pan by any amount (without leaving the terrain-filled area).
  4. Observe that the camera jumps to the south-east as you let go of the pan.

Option B - no large screen available.

  1. Open test/examples/3d-terrain.html
  2. Open developer tools and find the option to simulate a different screen size.
  3. Set your screen size to 3000px wide by 1024px high.
  4. Pan by any amount (without leaving the terrain-filled area).
  5. Observe that the camera jumps to the south-east as you let go of the pan.

Expected Behavior

No pan-jump after panning motion finishes

Actual Behavior

Large pan jump to south-east.

Further debugging

Open a developer console and input the following function

function test(x, y) {
    x *= map.transform.width;
    y *= map.transform.height;
    return [
        map.transform.pointCoordinate({x: x, y: y}).toLngLat(),
        map.terrain.pointCoordinate({x: x, y: y}).toLngLat(),
    ];
}

This lets you see what ML believes the latitude and longitude of some point on the screen, using two different methods - first one just uses math (but ignores terrain height) and the second uses the coords buffer of the terrain.
So running test(0.5, 0.5) should give you the LngLat for the centre of the screen, calculated in two different ways.
They are allowed to differ slightly since they are answering slightly different questions, but that difference shouldn't depend on the screen size.

Observe that these values are very similar if you set the screen-width to be 2048px or less, but that start to get quite different if you set the screen width to be (for example) 3000px.

@olsen232
Copy link
Contributor Author

olsen232 commented Apr 3, 2024

Okay I've found a simple culprit - once this expression is not returning a consistent result, the math ends up wrong:
map.painter.width / devicePixelRatio
Observe that for small screen sizes <= 2048px, that expression gives the same result as map.transform.width.
For large screen sizes > 2048px, that expression always returns 2048, even though that is less than map.transform.width.
This discrepancy will be confusing the pointCoordinate code in terrain.ts, which uses that expression and a similar one for height to create and query the coords buffer.

See
https://github.com/search?q=repo%3Amaplibre%2Fmaplibre-gl-js+path%3Asrc%2Frender%2Fterrain.ts+devicePixelRatio&type=code

@HarelM
Copy link
Collaborator

HarelM commented Apr 3, 2024

Interesting, this sounds like there might be other issues related to assuming these are equal.
This might be related to max canvas size maybe? 2048 sounds like the default and might be limiting due to how big the web gl buffer is, maybe?
Just a wild guess...

@HarelM
Copy link
Collaborator

HarelM commented Apr 3, 2024

BTW, did you have a chance to look at #3878? I've tested it with latest version and the bug still exists...

@HarelM HarelM added bug Something isn't working terrain 💰 bounty M Medium Bounty, USD 250 PR is more than welcomed Extra attention is needed labels Apr 3, 2024
olsen232 added a commit to olsen232/maplibre-gl-js that referenced this issue Apr 3, 2024
When the map canvas is large - >2048px, probably - the coords canvas
stops growing with the map due to web GL limitations.
This means that a screen-pixel is no longer 1:1 with a coords-canvas
pixel. It needs to be scaled using the ratio of devicePixelRatio and
painter.pixelRatio, which are no longer equal when the map is large.
olsen232 added a commit to olsen232/maplibre-gl-js that referenced this issue Apr 4, 2024
When the map canvas is large - >2048px, probably - the coords canvas
stops growing with the map due to web GL limitations.
This means that a screen-pixel is no longer 1:1 with a coords-canvas
pixel. It needs to be scaled using the ratio of devicePixelRatio and
painter.pixelRatio, which are no longer equal when the map is large.
@HarelM
Copy link
Collaborator

HarelM commented Apr 4, 2024

Bounty direction: maplibre/maplibre#189

olsen232 added a commit to olsen232/maplibre-gl-js that referenced this issue Apr 4, 2024
olsen232 added a commit to olsen232/maplibre-gl-js that referenced this issue Apr 4, 2024
HarelM pushed a commit that referenced this issue Apr 5, 2024
* Fix for #3935 - pan-jump when map is large

When the map canvas is large - >2048px, probably - the coords canvas
stops growing with the map due to web GL limitations.
This means that a screen-pixel is no longer 1:1 with a coords-canvas
pixel. It needs to be scaled using the ratio of devicePixelRatio and
painter.pixelRatio, which are no longer equal when the map is large.

* Fix for #3935 - add test, update CHANGELOG
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💰 bounty M Medium Bounty, USD 250 bug Something isn't working PR is more than welcomed Extra attention is needed terrain
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants