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

Constrain panning on single-world map #3738

Merged
merged 9 commits into from
Mar 1, 2024

Conversation

sbachinin
Copy link
Collaborator

@sbachinin sbachinin commented Feb 25, 2024

This PR fixes an ugly panning behaviour on a single-world map (when {renderWorldCopies: false}).
AFAIK there was no relevant issue.

  • 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.
  • Include before/after visuals or gifs if this PR includes visual changes.
  • Write tests for all new functionality.
  • Document any changes to public APIs.
  • Post benchmark scores.
  • Add an entry to CHANGELOG.md under the ## main section.

Wrong behaviour: when panning the map to the far left/right, map appears on the opposite side of the viewport with a splash of special effects.

How I suggest to improve it: constrain the panning on single-world map so that the map always fills the whole viewport width.

Solution is to set transform.lngRange (that normally comes from maxBounds) to the full globe width.
Unfortunately when I tried to set lngRange to [-180, 180], it didn't work (it's treated as if there are no bounds at all). For this reason I had to use "almost 180".

single.world.panning.mp4

@codecov-commenter
Copy link

codecov-commenter commented Feb 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.86%. Comparing base (a632997) to head (e6b9f9c).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3738      +/-   ##
==========================================
+ Coverage   86.36%   86.86%   +0.49%     
==========================================
  Files         240      240              
  Lines       32262    32273      +11     
  Branches     1973     1963      -10     
==========================================
+ Hits        27864    28033     +169     
+ Misses       3458     3308     -150     
+ Partials      940      932       -8     

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

@HarelM
Copy link
Member

HarelM commented Feb 26, 2024

Thanks for taking the time to open this PR!
I do remember an issue that sounds similar to the problem described here, but I can't find it.
In any case, the solution looks good, my only concern is how would it affect other behaviors.
Setting the bounds of a map, what about inertia? touch behavior? Can you zoom out enough so that the map doesn't cover the view port?

@HarelM
Copy link
Member

HarelM commented Feb 26, 2024

Bottom line, code-wise this looks, but I think a bit more tests are needed here.
Also, we need to think if this is a breaking change in terms of behavior that requires a major version bump or not.

@sbachinin
Copy link
Collaborator Author

Can you zoom out enough so that the map doesn't cover the view port?

In this solution map always fills the whole viewport.
It also means that you will not see the whole map unless your screen proportions are exactly the same as the globe's.

This is perhaps a disatvantage of this approach. Can it be critical for a user to see the whole globe at once? IDK.

There are 2 more possible solutions where you can view the whole map:

  1. horizontal panning is not constrained at all. There can be white empty space on the left and/or right. You just pan the map freely and you can outpan it from the viewport completely.
    This non-constrained panning is easy to implement. But surely there will be a lot of side problems. I suspect it won't be easy to place markers & other supplementary stuff in the right place.
  2. Same free panning but you can't outpan the map from the viewport. It will look slightly cooler but side problems will be roughly the same. + I'm not yet sure how to add such constraint. transform._constrain is quite unreadable and there are 15 magic variables so it's difficult to make changes there.

is it a breaking change

For the above reason (user can't view the whole globe) it can be regarded as a breaking change.
But I think it's rather not.
Old behaviour on low zoom levels was quite bad so I don't think anyone depends on it.

@sbachinin
Copy link
Collaborator Author

sbachinin commented Feb 28, 2024

what about intertia?

There was a problem in my initial solution. When the zoomed map was "thrown" hard against the horizontal edge, the center lng soon reached some extreme values, they were wrapped and therefore map switched to the opposite side of the globe.
I disabled wrapping in the next commit, now it works ok

How would it affect other behaviours

Right, there are some interesting details...

  1. flyTo isn't always animated well.

When it tries to zoom in/out near the edges of the globe, the horizontal constraint leads to a slightly weird trajectory. Here in the end of animation it switches from smooth curve to a rather linear movement:

flyTo.single-globe.mp4

Actually this is not exactly a new problem: vertical constraints also lead to bad trajectory on flyTo.
It can be seen in live-geojson example. Currently this page isn't ok. And if you set {renderWorldCopies: false} on my branch, it gets really awful.

Perhaps it would be a good (and feasible) idea to restrict the "zooming curve" of flyTo in certain situations (By looking at zoom level and coordinate we can deduce that we are close to some constraint and therefore flyTo shouldn't zoom out too much).

Didn't fix this yet.

Same zooming problem applies to fitBounds.

  1. easeTo with padding

If you call something like map.easeTo({padding: {right: 300}}) on my single-globe map, a white 300px will appear on the right and 300px of the map will be lost on the left.
Didn't fix this yet.

easeTo.mp4
  • With multi-globe map there will be no white padding, just the map gets basically "panned" 300px to the right.
  1. wrap marker and popup?

What should happen when marker/popup is animated horizontally by incrementing its lng?
Should it disappear behind the edge or appear on the opposite side?
I think the second option is better. It's already done in a commit above

@HarelM
Copy link
Member

HarelM commented Feb 29, 2024

Thanks for looking into all aspects of this change!
Generally speaking, it's better than the current behavior, so I think we should merge it as is, feel free to open issues for the remaining problems.
Please add a changelog item and I'll merge this.

@HarelM HarelM merged commit 7a7cc77 into maplibre:main Mar 1, 2024
15 checks passed
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.

None yet

3 participants