Leaflet / Leaflet

πŸƒ JavaScript library for mobile-friendly interactive maps πŸ‡ΊπŸ‡¦
https://leafletjs.com
BSD 2-Clause "Simplified" License
40.16k stars 5.75k forks source link

Pinch zoom hangs/freezes map on Android with L_DISABLE_3D = true #1759

Open Mathbl opened 10 years ago

Mathbl commented 10 years ago

Hey there,

Am I the only one who have problem with master version?

Whenever I try to pinch zoom, the markers disappear and the map is frozen.

It happens to me on a Nexus S running 4.1.1 in webview, in stock browser and chrome browser.

Setting fadeAnimation and zoomAnimation to false doesn't change anything.

mourner commented 10 years ago

Is it working for you on stable version (0.5.1)?

Mathbl commented 10 years ago

Yes my Android app is up and running using 0.5.1 with the following fixes :

window.L_DISABLE_3D = true;

if (navigator.userAgent.toLowerCase().indexOf('android 4.1') !== -1) { L.DomUtil.TRANSITION_END = 'webkitTransitionEnd'; }

I was willing to try to update to master to benefit of new fixes, but like I said, it hangs/freezes+all markers dissapear on pinch zoom.

mourner commented 10 years ago

Does the same happen if you remove the hacks?

Mathbl commented 10 years ago

On 0.5.1, if I remove disable_3D, it's unpredictable and unstable depending on version and device. I only have 3 different Android models to test on, but I had people telling me there map freezes if I do not use disable_3D.

For the other hack (webkitTransitionEnd), I put it there after noticing the map on galaxy tab 2 10.1 were freezing. It's the only hack that was able to fix it...

Just tested with the master, removing the two hacks above. It works well on samsung nexus S...but it's only one device...I cannot predict if it will make more problems than anything. I guess i'll have to stay with 0.5.1...only problem is the click firing twice when hitting the zoom buttons.

Damn...

mourner commented 10 years ago

@HooliganQC so that's probably the transitionEnd hack that's causing the problem for the master version, so it seems that it's safe to close the issue.

hanicker commented 10 years ago

Having same problem with master and 0.6.3 with disable_3D. Map freezes and marker disappears. If i comment L.Map.addInitHook('addHandler', 'touchZoom', L.Map.TouchZoom); everything works (except pinch). Testing on Android 4.1.2 Everything fine with 0.5.1

Thanks in advance

leCradle commented 10 years ago

I can approve this. @mourner this issue should be reopened again. Device: Nexus 4 - Android 4.2.2 pinch2zoom works as it should on my device, but with window.L_DISABLE_3D = true; markers disappear and map freezes. Just using leaflet 0.6.3 with no additional hacks. Is it true, that 3D is disabled on some devices by default? I'm asking because of this issue https://github.com/Leaflet/Leaflet/issues/1182. Furthermore, my app has 3D enabled but some users reported the pinch2zoom issue described here and nearly all of them are using Galaxy Notes...

kaansoral commented 10 years ago

I was using leaflet 0.5.1 wrapped in phonegap, at ancient times when I stabilized my setup, I had window.L_DISABLE_3D = true for all platforms

After upgrading to v0.7 I experienced freezing on Android 4.2.2, after testing, turned out to be a pinch-zoom issue, I've disabled L_DISABLE_3D - it seems to work without any issues now, my .02

Mathbl commented 10 years ago

@kaansoral did you have the chance to test on different devices? Android 2.3.x, Android 4.x or Samsung devices (I had specific problem with a Galaxy Tab 10.1 2 :/ ).

That's why i'm undecided to try 0.7, and get rid of L_DISABLE_3D = true. I don't own those previously problematic devices. Only have nexus 5 and 2.3.x HTC device. I fear the switch to 0.7 without disabling 3D cause new troubles.

kaansoral commented 10 years ago

I have an iPad and a Galaxy W/2.3 device for testing however I didn't test on them yet, I will update here If I experience any issues

Android is always problematic so I don't think you should worry :) There are also many vendor specific bugs, wait till you test your stuff on Android 4 samsung devices/S4 etc :) - in my case leaflet works on Galaxy S4, however everything else is glitchy UI-wise, including the map markers

I was using 0.5.1, 0.6.4 was in my opinion buggy on transitions on web, so I didn't update the mobile, however v0.7 seems solid, I don't have any feature-related reasons for upgrading, I also see many people still using 0.3 with their phonegap apps on production, dunno why

As far as I've seen the leaflet team consider many devices and in the code there are many compatibility quirks, so if I were in you I would switch to 0.7 and trust the team, my .02

mourner commented 10 years ago

There's one 2.3 regression we're investigating right now #2198, but with a workaround β€” should be fixed soon

leCradle commented 10 years ago

Just tested it on Android 4.4 and the latest 0.7-dev. Everything works fine, but with L_DISABLE_3D = true; it freezes as I described above.

mourner commented 10 years ago

I love how unpredictable Android is. What a lovely platform. Reopening the issue and updating the title about L_DISABLE_3D.

danzel commented 10 years ago

pinch zoom requires 3d transforms. Maybe disable pinch zoom if disable3d is set? What is the issue that disable3d is trying to work around?

Maybe I was wrong and android4 should be added to: https://github.com/Leaflet/Leaflet/blob/master/src/layer/tile/TileLayer.js#L432

mourner commented 10 years ago

@danzel in theory, pinch-zoom only requires transform transitions, but transforms can be 2d.

danzel commented 10 years ago

L_DISABLE_3D in leaflet essentially means disable all transforms and transitions and just use top/left.

According to our definition: http://leafletjs.com/reference.html#global-l_disable_3d We should really be disabling pinch-zoom in this case, as it uses transform(3d) which is hardware accelerated.

mourner commented 10 years ago

@danzel makes sense. Considering Android 2.* should not require L_DISABLE_3D for its problems now, we can disable pinch-zoom if it is specified.

kaansoral commented 10 years ago

This might not be too related to the issue but the initial reason I disabled L_DISABLE_3D was occasional tile bugs, very rarely, probably when webkit is out of memory and 3d acceleration fails, if 3d is enabled with leaflet, the tiles move to higher z-indexes / render the app unusable - I've also experienced it only once with 0.7 (without L_DISABLE_3D=true) - so it very seldomly happens

With L_DISABLE_3D=true - this issue never happened with v0.51, I also can't differentiate between 3d enabled or not

However if pinch-zoom is disabled when L_DISABLE_3D=true, it would force enabling 3d since pinch-zoom is a must-have - and if android issues persist, it would be logical to keep using v0.51

It might be a good idea to at least be able to enable pinch-zoom at initialization when L_DISABLE_3D is true

danzel commented 10 years ago

I have a feeling the Android 2 browser doesn't support pinch zoom anyway as it doesn't support multitouch. It didn't during my testing yesterday at least.

Not sure what the memory issue in android we've introduced is... I can't think of anything we've changed since 0.5 that may have caused it.

kaansoral commented 10 years ago

It's not related to Leaflet, and the memory thing is just a speculation, it happens specifically on Galaxy S4 a lot, however happens on other devices too, suddenly 3d acceleration stops working, 3d accelerated elements lose their styles etc.

It's always ugly but usually harmless, however when tiles are 3d accelerated, if the map is fullpage and there are items at higher z-indexes, the map tiles can overlay other items etc.

I'm using Leaflet with Phonegap, it uses the webkit component, on android devices it's extremely buggy, so my feedback might not be useful for the default android browser compatibility, however as far as I hear that are not so different

danzel commented 10 years ago

Yeah, android browser sucks. Problem will go away in the future by the looks https://developers.google.com/chrome/mobile/docs/webview/overview Or if you really want it now https://github.com/pwnall/chromeview

stevendlander commented 10 years ago

After extensive testing, I can confirm a working system using Android 4.4.2 Webview with the following notes:

bambax commented 10 years ago

Googling leaflet android pinch zoom freezes brought me here; I'm using 7.2 and 8-dev and have this issue on Android 4.4 on a Nexus 7.

Indeed, setting zoomAnimation=truesolves the problem (thank you previous commentator!!).

But zoomAnimation results in weird blending of tiles (maybe because they are served from a slow server); is there a way to have pinch zoom working without setting zoomAnimation to true?

davelosert commented 10 years ago

I can confirm this issue - setting zoomAnimation to false causes a freeze on pinchzoom also on IPad with iOS7.

jbduzan commented 10 years ago

I confirm it too.

ZoomAnimation false freeze my map on android devices.

Setting it to true and enabling 3D work fine

fab1an commented 9 years ago

Check this out pls: https://github.com/Leaflet/Leaflet/issues/2693#issuecomment-46053713

VaroLop commented 9 years ago

fab1an, your workaround doesn't work in my case. ZoomAnimation: false still freezes my map

I am using Leaflet 0.7.3 on Android devices.

Thanks in advance

fab1an commented 9 years ago

Can you output what getScaleString on https://github.com/Leaflet/Leaflet/blob/v0.7.3/dist/leaflet-src.js#L1053 returns?

VaroLop commented 9 years ago

getScaleString isn't executed if I set zoomAnimation to false.

fab1an commented 9 years ago

Did you check that? IIRC, it's not only used for the animation.

VaroLop commented 9 years ago

I have checked it. In my case it isn't executed.

Setting zoomAnimation to false: My map works well until I try to zoom by touch-dragging with two fingers.

If I set zoomAnimation to true, getScaleString is executed.

fab1an commented 9 years ago

Which android-tablet are you using?

PLS visit this site: http://www.whatsmybrowser.org/ and post results.

Can you install Android/Chrome and try it there, thank you.

VaroLop commented 9 years ago

I am using Leaflet in an Android App. I use it in a webview

Phone: Star V1277 (It's a chinese phone) Android version: 4.0.4 Browser: Android browser 4 Full user agent string: Mozilla/5.0 (Linux; U; Android 4.0.4; es-es; V1277 Build/IMM76D) AppleWebKit/534.30 (KHTML, like Gecko) Version/4.0 Mobile Safari/534.30

fab1an commented 9 years ago

Have you checked this as well: https://github.com/Leaflet/Leaflet/issues/2693#issuecomment-45100103 ?

VaroLop commented 9 years ago

In Leaflet 0.7.3 I have inserted these lines in _tryAnimatedZoom and setZoomAround (scale = this.getZoomScale(zoom) is not in _animateZoom) but it continues freezing my map in the moment I try to zoom by touch-dragging with two fingers

VaroLop commented 9 years ago

Besides, my circleMarkers disappear in the moment the map freezes. Thank you for your time

fab1an commented 9 years ago

There are multiple freeze-bugs. Touch-dragging also calls the method _animateZoom, not only the animation.

Pls try to add these lines in _animateZoom: https://github.com/Leaflet/Leaflet/issues/2693#issuecomment-45100103 and tell me if it works.

VaroLop commented 9 years ago

I tried it yesterday, I inserted it after scale = map.getZoomScale(e.zoom).

It doesn't work

Thanks

VaroLop commented 9 years ago

It looks like the transitionend event is not fired.

fnicollet's workaround works in my case: #2693 (comment)

In _onTouchStart:

var map = this._map;
map.on('zoomanim', debounce(map._onZoomTransitionEnd, 250)); //this is the new line

But the touchZoom I get isn't as good as the control zoom.

fab1an commented 9 years ago

Ok, to recapitulate: For my installation to stop crashing I had to apply BOTH of the fixes I mentioned to you. The one preventing the division/0 and the other one preventing duplicate values of css-transform being set. I'm still not sure if you applied both. Some log-output which transform values get set would be interesting, otherwise it's just really hard to debug.

VaroLop commented 9 years ago

I applied both of the fixes at the same time.

my map call this function:

_animateZoom: function (center, zoom, origin, scale, delta, backwards, forTouchZoom) {
...
}

This is what _animateZoom receives (without your fixes):

center: LatLng(37.17498, -3.60759), zoom: 16.99078531442567, origin: Point(191.5, 303), scale: 0.9936332210520357, delta: Point(0, -1), backwards: false, forTouchZoom: true
center: LatLng(37.17496, -3.60757), zoom: 16.9617596479658, origin: Point(191.5, 303), scale: 0.97384201359612, delta: Point(-2.5, -4), backwards: false, forTouchZoom: true
center: LatLng(37.17494, -3.60753), zoom: 16.927320969370797, origin: Point(191.5, 303), scale: 0.9508706264587935, delta: Point(-6, -6.5), backwards: false, forTouchZoom: true
center: LatLng(37.17492, -3.60752), zoom: 16.87214053308592, origin: Point(191.5, 303), scale: 0.9151883123823491, delta: Point(-7.5, -9), backwards: false, forTouchZoom: true
center: LatLng(37.17493, -3.60749), zoom: 16.797883675651022, origin: Point(191.5, 303), scale: 0.869274467755642, delta: Point(-9.5, -9), backwards: false, forTouchZoom: true
center: LatLng(37.17493, -3.60748), zoom: 16.720947193653963, origin: Point(191.5, 303), scale: 0.8241319190550178, delta: Point(-11, -9), backwards: false, forTouchZoom: true
center: LatLng(37.17494, -3.60745), zoom: 16.65291307454225, origin: Point(191.5, 303), scale: 0.7861699222962353, delta: Point(-13, -9), backwards: false, forTouchZoom: true
center: LatLng(37.17496, -3.60744), zoom: 16.59225362454453, origin: Point(191.5, 303), scale: 0.7537999605149059, delta: Point(-13.5, -8), backwards: false, forTouchZoom: true
center: LatLng(37.17498, -3.60744), zoom: 16.53695512135472, origin: Point(191.5, 303), scale: 0.7254535365155208, delta: Point(-13.5, -6.5), backwards: false, forTouchZoom: true
center: LatLng(37.17498, -3.60744), zoom: 16, origin: Point(194.2569, 285.08018), scale: 0.6892240161948714, delta: undefined, backwards: undefined, forTouchZoom: undefined
VaroLop commented 9 years ago

fnicollet's workaround definitely works for me.

I force calling the _onZoomTransitionEnd 250ms after the last zoomanim event.

@fab1an, thank you very much for your help and your time.

This is my code:

_onTouchEnd: function () {
    [...]
    map.on('zoomanim', debounce(map._onZoomTransitionEnd, 250));
    map._animateZoom(center, zoom, origin, scale);
}
fab1an commented 9 years ago

I know it works, it works for me as well, but it doesn't fix the problem and makes the code-flow more complicated and harder to reason about.

fab1an commented 9 years ago

Thanks for your logs. Hm, I guess delta, backwards etc. shouldn't be undefined.

A lot of bugs in leaflet could be caught by checking parameters properly :/

VaroLop commented 9 years ago

But the problem isn't because of these undefined parameters. If you set zoomAnimation to true, it works with the same values.

Thanks.

VaroLop commented 9 years ago

If anyone needs to use fnicollet's workaround while another solution is proposed, It's better to use addOneTimeEventListener in order to avoid memory leaks:

I force calling the _onZoomTransitionEnd 250ms after the last zoomanim event. I call it once and then it is removed.

This is my code:

_onTouchEnd: function () {
    [...]
    map.addOneTimeEventListener('zoomanim', debounce(map._onZoomTransitionEnd, 250));
    map._animateZoom(center, zoom, origin, scale);
}
johnd0e commented 4 years ago

In modern leaflet map don't freeze. But pinch zoom still does not work, instead map offsets related to overlays.

See https://jsbin.com/fifunag/

googol7 commented 3 years ago

L_DISABLE_3D in leaflet essentially means disable all transforms and transitions and just use top/left.

According to our definition: http://leafletjs.com/reference.html#global-l_disable_3d We should really be disabling pinch-zoom in this case, as it uses transform(3d) which is hardware accelerated.

Please at least warn users in the documentation here that pinch zoom wont work anymore. Just tested it with version 1.7.1. Pinch zoom is broken with L_DISABLE_3D and looks like this on iOS 14.1:

L_DISABLE_3D 300 B