search

jieter / Leaflet.Sync

Synchronized view of two maps.
http://jieter.github.io/Leaflet.Sync/examples/dual.html
BSD 3-Clause "New" or "Revised" License
235 stars 53 forks source link

Maps out of sync #43

Closed mhm-amaya closed 7 years ago

mhm-amaya commented 7 years ago

Hi, When using inertia on two-way synced maps they often get out of sync if the user pans quickly repeatedly. After the next zoom/pan everything is OK again. As a workaround, disabling inertia fixes the problem. Thanks. Great work!

jjimenezshaw commented 7 years ago

@mhm-amaya : Could you give more details on that issue? Device (pc, phone, tablet... touch devices use different functions internally), more exactly how it happens (for instance, if the mouse/finger gets out of the map area, or only doing some specific movement), check if it happens in different devices (maybe it is a race condition, and CPU performance could affect), etc. If I can easily reproduce, I can have a look at it, and try a fix.

jieter commented 7 years ago

@jjimenezshaw would be really nice!

mhm-amaya commented 7 years ago

I have been able to see the issue in both PC (a Core i5 6500 / 8GB box, not top notch but pretty much capable) (Chrome, Firefox and IE on Win 7) and touch devices (Samsung Galaxy Express2 phone) (Chrome on Android). It is more noticeable on PC, but it might be only because of the screen size :-) The offset happens when panning quickly and repeatedly (in any direction, but only vertically or horizontally will be easier to spot, I guess). Can't be more specific about that, sorry. I attach a screenshot to make it easier to understand.

error_screenshot

jjimenezshaw commented 7 years ago

@mhm-amaya Thanks. Unfortunately I cannot reproduce it. Do you have any environment where I can test it? Is the page where you got the screenshot public?

mhm-amaya commented 7 years ago

Well, in that case it might be not related to Leaflet.Sync, perhaps there is something wrong in my setup. Anyway, you can try it here: http://rediam-test1.cica.es/testSync/index.html Thank you very much.

mhm-amaya commented 7 years ago

I tried to disable others plugins (L.Control.MapCenterCoord.js) and also anything else that I could imagine might be involved, like interact.js and hammer.js, but the only thing that I could find that worked was disabling inertia.

jieter commented 7 years ago

It might be due to the fact that you have quite some layers on your map. I imagine having those could change some timings making the bug visible in more cases...

mhm-amaya commented 7 years ago

Unless there is an error in the code there should be only 2 layers in each map (background and foreground). What you see is only a list of names. The layers are created and added to the map (and the old one removed from it) when the user selects a new layer.

jieter commented 7 years ago

Ah, I assumed the red/grey lines were actual vectors, but now I see they are not...

jjimenezshaw commented 7 years ago

Thanks @mhm-amaya . I found one source of misalignment related to inertia: If you have two synced maps (A and B), with inertia enabled, and you drag the map in A, both have inertia and keep moving when you release the mouse button. However, if you click on map A (or B) before it stops moving, map A (or B) will stop, but B (or A) still keep moving because of inertia. This produces a lack of sync. Obviously without inertia, it doesn't happen. It would be great if you confirm that this is what you see. Moving the mouse fast makes easy to click the map later "accidentally". I am already working in a fix for that case.

mhm-amaya commented 7 years ago

Yes, @jjimenezshaw, that seems to be the case! Looks like whether the click just stops the map or starts a new dragging (most likely what I was doing so far) is irrelevant, the effect is the same.

jjimenezshaw commented 7 years ago

@jieter New leaflet 1.1.0 is released, and there are changes that affects my fix. Due to this change https://github.com/Leaflet/Leaflet/pull/5378 they do not stop when just clicking the map, but wait for an actual drag (I subscribed to events click and dragstart, and now only dragstart would be needed).

The question is: Should I do a pull request for 1.0.3 (already done in my branch named fix_inertia) and later another for 1.1.0 (changing this, and also examples and other stuff), or do I integrate all changes in one pull request for 1.1.0?

jieter commented 7 years ago

Hmm, good question. It's kind of nice to fix for both versions, but I'm not going to maintain two branches.

The best option to support both would be:

jjimenezshaw commented 7 years ago

Ok. Then I will send you soon the first pull request.

jieter commented 7 years ago

Fixed by #44 for leaflet@1.0.3.

mhm-amaya commented 7 years ago

In my case it is working perfectly! It is great to be able to have inertia enabled, as it makes interaction smoother and much more pleasant. Many thanks, @jjimenezshaw and @jieter !

jieter commented 7 years ago

@mhm-amaya is this fixed with 0.2.0?

mhm-amaya commented 7 years ago

I was using the fix submited a month ago with Leaflet 1.0.3+ed36a04 without any problem at all, now I have tried switching to 0.2.0 and it seems to work as well. So, yes, it woks fine. Thanks!

jieter commented 7 years ago

Thanks for checking. Feel free to close any resolved issues next time.