Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

LatLngBounds: Fix constrain when crossing antimeridian #13958

Merged
merged 4 commits into from
Feb 26, 2019

Conversation

brunoabinader
Copy link
Member

@brunoabinader brunoabinader commented Feb 20, 2019

This PR fixes the constraint behavior when crossing the antimeridian, but also:

  • Fixes an issue that caused panning to be set to false when the longitude value that was equal to its wrapped one e.g. when doing a full round over the globe;
  • Constrain axes separately in LatLngBounds: this fixes an issue when the bounds crosses the antimeridian and only latitude needed to be constrained;

This PR is an alternative to #13906.

Closes #13672, follow up from #13871

/cc @tobrun

@LukasPaczos
Copy link
Member

I've quickly tested this one and it works well for #13672, however, in contrast to #13906 it doesn't fix #13770 anymore. This is because when the gesture is in progress we are allowing to wrap around the world if the offset exceeds the antimeridian in here.

I think the possible solution can be ignoring, or changing the behavior of the isGestureInProgess flag if the camera constraint is set? Do you think this could be a part of this PR as well @brunoabinader?

@brunoabinader
Copy link
Member Author

I think the possible solution can be ignoring, or changing the behavior of the isGestureInProgess flag if the camera constraint is set? Do you think this could be a part of this PR as well @brunoabinader?

Good point @LukasPaczos - I think we can special case for whether camera bounds is present or not in that line 👍

@brunoabinader brunoabinader force-pushed the latlngbounds-antimeridian-fix branch 2 times, most recently from 28e51d7 to 6e56eee Compare February 21, 2019 14:46
@LukasPaczos LukasPaczos added this to the release-l milestone Feb 25, 2019
@brunoabinader brunoabinader force-pushed the latlngbounds-antimeridian-fix branch 4 times, most recently from 78e4400 to a360f2b Compare February 26, 2019 09:33
@brunoabinader
Copy link
Member Author

This PR also changes the behavior of the internal coordinates stored in Transform. When a camera bound is set, we store unwrapped longitudes and use them to properly calculate bounds check in edge cases such as e.g. when the world horizontal span is visually repeated on the map view. Unwrapped values are also used when transforming screen coordinates into geographical ones (i.e. moveBy uses it).

Important to notice the unwrapped longitude value does not leak outside Transform, unless explicitedly requested when passing LatLng::Unwrapped to Transform.getLatLng(). Realizing this actually helped finding out a "bug" in flyTo behavior, which was throwing unwrapped values when the fly camera movement crosses the antimeridian.

@LukasPaczos
Copy link
Member

I can confirm that this resolves both #13672 and #13770 🎉

Copy link
Contributor

@alexshalamov alexshalamov left a comment

Choose a reason for hiding this comment

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

👍 and thanks for for the explanation.

Copy link
Contributor

@pozdnyakov pozdnyakov left a comment

Choose a reason for hiding this comment

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

LGTM with minor nits

platform/glfw/glfw_view.cpp Outdated Show resolved Hide resolved
src/mbgl/map/transform.cpp Show resolved Hide resolved
src/mbgl/map/transform.cpp Outdated Show resolved Hide resolved
src/mbgl/map/transform.cpp Outdated Show resolved Hide resolved
src/mbgl/map/transform.cpp Outdated Show resolved Hide resolved
src/mbgl/map/transform.cpp Outdated Show resolved Hide resolved
src/mbgl/util/geo.cpp Outdated Show resolved Hide resolved
src/mbgl/util/geo.cpp Outdated Show resolved Hide resolved
@tmpsantos
Copy link
Contributor

Great stuff! kudos

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Map::setLatLngBounds crossing the IDL
5 participants