Map jumps from one position to another #4214

brunoabinader · 2016-03-07T17:27:03Z

When panning the map jumps from on location to another.
I'm guessing it's something related to the dateline.
You can see the bug in action here

tobrun · 2016-03-07T03:01:37Z

@brunoabinader

I was able to git bisect it to:

abe3f9ce7ab8087b13871472a11e7f5021084642 is the first bad commit
commit abe3f9ce7ab8087b13871472a11e7f5021084642
Author: Bruno de Oliveira Abinader <bruno@mapbox.com>
Date:   Wed Feb 24 00:54:03 2016 +0200

    [core] Moved wrapping to LatLng scope

    Fixes a precision loss when converting unwrapped LatLngs.

:040000 040000 c0390f84f9ead167fafd74abcb6a63fa195d91f1 f4cc90ac8b3b4fa2d9abf8a21012d60b2b017647 M  include
:040000 040000 e58dc7af20109af7a2f0b1cdcfab883f29a98b3d 07852720a309f98be353b22be8aea7cee0034be7 M  src
:040000 040000 1515c004c9d1d50075821a66d15825d548e922b0 aa28e3b0cc331fc40fff6053177183760faa64c6 M  test

brunoabinader · 2016-03-07T11:10:02Z

@tobrun interesting, this should have been fixed in fd123de - on it 👍

brunoabinader · 2016-03-07T17:28:45Z

👀 @kkaefer

1ec5 · 2016-03-07T17:38:43Z

@brunoabinader, can you add a blurb to the Android and iOS sections of the changelog? Thanks.

tobrun · 2016-03-07T23:09:23Z

I retested but still seeing the issue:
https://youtu.be/gC489mSfS44

brunoabinader · 2016-03-09T01:47:23Z

@tobrun I've updated the PR and tested it on GLFW & iOS. Could you please check it on Android? Thanks!

tobrun · 2016-03-09T03:29:05Z

@brunoabinader yes this issue is no longer reproducible 🙇

tobrun · 2016-03-09T03:30:57Z

FWIW I retested #4155 and this is still an issue

- Make returning LatLngs unwrapped by default. - PointAnnotation and ShapeAnnotation are always wrapped so they can be selected via intersection from the visible tile boundaries. - Fixes LatLng::wrap() calculation. - Fixes LatLng::unwrapForShortestPath() calculation. The new unwrapForShortestPath algorithm unwraps the start coordinate either forwards or backwards depending on the end coordinate value, so we can always cross the antimeridian when needed and still obtain a wrapped end coordinate in the end. Fixes #4214.

vec2<T>::operator bool() checks for NaNs already.

brunoabinader · 2016-03-10T11:58:34Z

Fix for #4155 is in 949ed9e (together with unit tests). I have tested it on both Android Nexus 5X and iPad 2 👍 .

If the center and point coordinates are not in the same side of the antimeridian, we need to unwrap the point longitude to make sure it can still be seen from the visible side of the antimeridian that is opposite to the center side. Fixes #4155.

1ec5 · 2016-03-10T15:47:52Z

src/mbgl/map/transform.cpp

@@ -517,7 +527,7 @@ double Transform::getAngle() const {
 #pragma mark - Pitch

 void Transform::setPitch(double pitch, const Duration& duration) {
- setPitch(pitch, {NAN, NAN}, duration);
+ setPitch(pitch, ScreenCoordinate {}, duration);


So if you don’t specify an anchor for a setPitch() call, we assume the gesture is anchored in the top-left corner of the view?

If we don't specify a valid anchor (by valid I mean an anchor that does not contain any NaN and contains at least one non-zero value on its axis), these are simply not taken in consideration in startTransition().

I originally made the change to rely on NaN for anchors because anchoring at the origin (or somewhere on the top or left side) may be a valid use case. Though it may be difficult to trigger such a transition via a gesture, it may still be possible to do so in the iOS SDK with some combination of edge insets, user location or course tracking, and zooming or rotation gestures. Moreover, it’s only a matter of time before we need to allow arbitrary anchor points for programmatic animations in the mobile SDKs.

You are right @1ec5 - I am reverting this particular change in #4277.

tobrun added the bug label Mar 7, 2016

tobrun added this to the android-v4.0.0 milestone Mar 7, 2016

brunoabinader self-assigned this Mar 7, 2016

brunoabinader added the in progress label Mar 7, 2016

brunoabinader added ✓ ready for review and removed in progress labels Mar 7, 2016

1ec5 added iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS Android Mapbox Maps SDK for Android labels Mar 7, 2016

brunoabinader force-pushed the 4214-unwrapped-latlng-by-default branch from af7ad29 to ecf8a9f Compare March 7, 2016 20:08

brunoabinader force-pushed the 4214-unwrapped-latlng-by-default branch from ecf8a9f to 68a262d Compare March 7, 2016 20:52

brunoabinader force-pushed the 4214-unwrapped-latlng-by-default branch from 68a262d to bbf1366 Compare March 7, 2016 22:26

brunoabinader mentioned this pull request Mar 7, 2016

PrecisionPoint TransformState::latLngToPoint dateline issue #4155

Closed

brunoabinader added in progress ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold and removed ✓ ready for review labels Mar 8, 2016

brunoabinader force-pushed the 4214-unwrapped-latlng-by-default branch from bbf1366 to d5ece03 Compare March 8, 2016 16:52

brunoabinader force-pushed the 4214-unwrapped-latlng-by-default branch 2 times, most recently from b13b810 to 5aca28c Compare March 8, 2016 21:49

brunoabinader force-pushed the 4214-unwrapped-latlng-by-default branch from 5aca28c to bb8b1d4 Compare March 9, 2016 01:32

brunoabinader force-pushed the 4214-unwrapped-latlng-by-default branch from bb8b1d4 to 12e9309 Compare March 9, 2016 01:41

tobrun removed the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Mar 9, 2016

brunoabinader added 6 commits March 10, 2016 03:36

[core] Fixes point annotations misplaced on maximum scale

dc28535

[core] Remove _validPoint from Transform code

d300f34

vec2<T>::operator bool() checks for NaNs already.

[core] Added Transform::getScreenCoordinate

e8d1276

[core] Check for NaNs in EdgeInsets

b33b2f1

[core] Extra checks in Transform

7dde622

brunoabinader force-pushed the 4214-unwrapped-latlng-by-default branch from 12e9309 to 949ed9e Compare March 10, 2016 01:41

brunoabinader added ✓ ready for review and removed in progress labels Mar 10, 2016

brunoabinader force-pushed the 4214-unwrapped-latlng-by-default branch from 949ed9e to 1a4b8f3 Compare March 10, 2016 12:17

brunoabinader merged commit 1a4b8f3 into master Mar 10, 2016

brunoabinader deleted the 4214-unwrapped-latlng-by-default branch March 10, 2016 12:41

1ec5 reviewed Mar 10, 2016
View reviewed changes

friedbunny removed the ✓ ready for review label Mar 10, 2016

1ec5 mentioned this pull request Mar 12, 2016

Too much drift pan resistance at z0–z1 #3655

Closed

1ec5 mentioned this pull request Aug 29, 2016

Swiping downward in perspective makes the map move in the opposite directions (upwards) #5281

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Map jumps from one position to another #4214

Map jumps from one position to another #4214

brunoabinader commented Mar 7, 2016

tobrun commented Mar 7, 2016

brunoabinader commented Mar 7, 2016

brunoabinader commented Mar 7, 2016

1ec5 commented Mar 7, 2016

tobrun commented Mar 7, 2016

brunoabinader commented Mar 9, 2016

tobrun commented Mar 9, 2016

tobrun commented Mar 9, 2016

brunoabinader commented Mar 10, 2016

1ec5 Mar 10, 2016

brunoabinader Mar 10, 2016

1ec5 Mar 10, 2016

brunoabinader Mar 11, 2016

Map jumps from one position to another #4214

Map jumps from one position to another #4214

Conversation

brunoabinader commented Mar 7, 2016

tobrun commented Mar 7, 2016

brunoabinader commented Mar 7, 2016

brunoabinader commented Mar 7, 2016

1ec5 commented Mar 7, 2016

tobrun commented Mar 7, 2016

brunoabinader commented Mar 9, 2016

tobrun commented Mar 9, 2016

tobrun commented Mar 9, 2016

brunoabinader commented Mar 10, 2016

1ec5 Mar 10, 2016

Choose a reason for hiding this comment

brunoabinader Mar 10, 2016

Choose a reason for hiding this comment

1ec5 Mar 10, 2016

Choose a reason for hiding this comment

brunoabinader Mar 11, 2016

Choose a reason for hiding this comment