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

Fix bug causing pitched camera to animate on screen coordinates rather than geographic coordinates #3119

Closed
yeldarby opened this issue Aug 31, 2016 · 3 comments
Labels

Comments

@yeldarby
Copy link
Contributor

mapbox-gl-js version: v0.23.0

I've been trying to pin down issue #3112 (I think I found the root of it; going to post a comment over there with my findings in a moment) but encountered another bug in the process.

Steps to Trigger Behavior

  1. Set a map pitch to 60
  2. Use panBy to move the map in the vertical coordinate of the viewport (positive delta y)
  3. Use panBy in the opposite direction (negative delta y)

Expected Behavior

The map should return to where it started like it does when the map is not pitched

Actual Behavior

The map moves much more in one direction than the other and hops along

Demo

Here's a JS Fiddle demonstrating the issue. Click the map to toggle between pitch of 0 (working as expected) and pitch of 60 (hopping):
http://jsfiddle.net/qq3pqub2/3/

Explanation

panBy is defined to move by a certain number of pixels. When the map is pitched the number pixels in the viewport is not the same as the number of pixels in the map. When these get projected back and forth they are producing unexpected behavior.

Proposed solution

Change panBy to pan by an amount of degrees of latitude/longitude or a distance rather than by number of pixels. An alternative would be to pretend the map is not pitched and pan by that amount. Changing the pitch of the map shouldn't affect how far it pans.

@indus
Copy link
Contributor

indus commented Sep 2, 2016

I think panBy using viewport coordinates is preferable over using latitude/longitude and that the behaviour is correct. And there are use-cases to pan by screen pixels, e.g. if you want to move the map by X/2px when a Xpx wide sidebar slides over the map from an edge.

When you use lat/lng you would probably face the trouble of a rotated map pretty soon (and rotation is even more common then tilting). Why not use easeTo with subtracted coordinates? Or maybe this function may help you.

@yeldarby
Copy link
Contributor Author

yeldarby commented Sep 2, 2016

Yeah, I agree. In my PR I ended up going with option 2. It pans by map pixels rather than viewport pixels.

panBy should be a reversible function. eg this operation should leave the map in the same place it started:

panBy([x, y]);
panBy([-x, -y]);

@lucaswoj lucaswoj changed the title panBy does not work as expected when map is pitched Fix bug causing pitched camera to animate on screen coordinates rather than geographic coordinates Sep 2, 2016
@lucaswoj
Copy link
Contributor

lucaswoj commented Sep 6, 2016

I think this is a symptom of the same problem we're tracking in #3112

@lucaswoj lucaswoj closed this as completed Sep 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants