amiqat / osmdroid

Automatically exported from code.google.com/p/osmdroid
0 stars 0 forks source link

Double-tap zooming in rapid succession can relocate maps to wildly incorrect location #351

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
Occasionally double-tapping in rapid succession will scroll the maps to a 
wildly incorrect location. It does not happen all the time, but is pretty easy 
to recreate after a few attempts. Reproducibility may depend on your hardware 
since this appears to be a race condition. This has been a long-standing (but 
rare) bug.

By checking the stack trace at scrollTo() it appears that the scroll position 
goes haywire after a call to computeScroll(). The scrollTo() call preceding 
that is from setZoomLevel() and this appears to scroll to the correct position. 
In computeScroll() the scroll position is set by the mScroller. The Scroller 
keeps track of animated scrolls and it feeds the tweened scroll position to 
scrollTo() in computeScroll(). The Scroller is triggered from onFling() and 
setMapCenter().

So the key to this bug is this - it appears in onFling() and setMapCenter() it 
uses mZoomLevel to determine the world size and this is used to determine how 
far the Scroller will scroll. If the map is zooming when this happens, the 
Scroller will be fed a world size that is incorrect. We should be using 
getZoomLevel(true) which will return the zoom level we are zooming to (the 
target zoom level).

The attached patch fixes this. Please review and comment. If all looks good, I 
will commit.

Original issue reported on code.google.com by kurtzm...@gmail.com on 20 Jun 2012 at 8:58

GoogleCodeExporter commented 8 years ago

Original comment by kurtzm...@gmail.com on 20 Jun 2012 at 8:58

Attachments:

GoogleCodeExporter commented 8 years ago
This issue was closed by revision r1107.

Original comment by kurtzm...@gmail.com on 19 Jul 2012 at 8:15

GoogleCodeExporter commented 8 years ago
Reopening ticket. Problem still not solved.

Original comment by kurtzm...@gmail.com on 14 Aug 2012 at 8:13

GoogleCodeExporter commented 8 years ago
So after using and reviewing the patch I'm not convinced it was the right way 
to go. After looking deeper into it I realized there is still an issue if the 
scrolling started before the zoom. So we start by reverting the changes in the 
revision and let those functions (setMapCenter, scrollTo, onFling) use the 
non-target zoom level again. Bringing us back to where we started, we still 
have the issue of the scroller tweening values that are in the wrong zoom 
level. The solution to that is to force scrolling to finish early before 
changing zoom levels in setZoomLevel. So the Scroller will always scroll values 
in the current (non-target) zoom level, but if we change zoom levels then we 
force it to finish scrolling early since the map coordinate system is about to 
change. So in setZoomLevel:

        if (newZoomLevel != curZoomLevel) {
            // We could use abortAnimation() to jump to the final scroll location but that may cause perceived "jumpiness".
            // Seems best to just stop the Scroller dead in its tracks.
            mScroller.forceFinished(true);
        }

Will continue to test this internally and apply a patch after a review period. 
Please leave comments.

Original comment by kurtzm...@gmail.com on 14 Aug 2012 at 8:32

GoogleCodeExporter commented 8 years ago

Original comment by kurtzm...@gmail.com on 14 Aug 2012 at 8:35

Attachments:

GoogleCodeExporter commented 8 years ago
This issue was closed by revision r1128.

Original comment by kurtzm...@gmail.com on 19 Nov 2012 at 6:53

GoogleCodeExporter commented 8 years ago
I am experiencing this or a related issue in 3.0.10 very frequently. While a 
zoom is in transition and animating, a tap/scroll will wildly mess up the 
location. Is this patch in 3.0.10?

Original comment by vit.hrad...@gmail.com on 26 Jul 2013 at 2:41

GoogleCodeExporter commented 8 years ago
On a related note, is the a way to disable the zoom animation and just jump on 
the new zoom level? 

Original comment by vit.hrad...@gmail.com on 26 Jul 2013 at 2:54

GoogleCodeExporter commented 8 years ago
See issue 448.

Original comment by kurtzm...@gmail.com on 26 Jul 2013 at 1:45

GoogleCodeExporter commented 8 years ago
To disable zoom-animation you can add your own zoom buttons and call setZoom() 
directly.

Original comment by kurtzm...@gmail.com on 26 Jul 2013 at 1:46