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 map.unproject regression in 2D mode #10222

Closed
wants to merge 3 commits into from
Closed

Conversation

mourner
Copy link
Member

@mourner mourner commented Dec 17, 2020

An attempt at fixing #10215, using @karimnaaji's suggestion. I'm not sure why the sourceCache.tilesIn tests behave differently and if it's an issue in practice though. Also @arindam1993 you had an alternative suggestion for a fix in mind — would do a PR for it?

Launch Checklist

  • briefly describe the changes in this PR
  • write tests for all new functionality
  • manually test the debug page
  • tagged @astojilj if this PR includes shader changes or needs a native port
  • apply changelog label ('bug', 'feature', 'docs', etc) or use the label 'skip changelog'
  • add an entry inside this element for inclusion in the mapbox-gl-js changelog: <changelog>Fix a regression with map.unproject and map.panBy acting unpredictably in certain cases.</changelog>

Copy link
Contributor

@karimnaaji karimnaaji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, can we get unit tests covering the initial examples you hit with this issue? I'm thinking of this specific case:

map.unproject([0, -1000]);
map.unproject([0, 0]);

I'll also give some time for @arindam1993 to add more info concerning some potential failure case he was suggesting on chat prior to merging.

@arindam1993
Copy link
Contributor

Patch for alternative approach is in
#10224

It addresses this case PR wherenin if the horizon is just above 0 and a panBy reverses the direction of motion:

ezgif com-gif-maker (2)

@karimnaaji
Copy link
Contributor

Closing in favor of #10224.

@karimnaaji karimnaaji closed this Dec 19, 2020
@mourner mourner deleted the fix-unproject-2d branch February 5, 2021 09:09
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.

3 participants