Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Zoom issues with Apple trackpad (Magic Mouse) #2154

Closed
ekelleyv opened this issue Nov 5, 2013 · 29 comments
Closed

Zoom issues with Apple trackpad (Magic Mouse) #2154

ekelleyv opened this issue Nov 5, 2013 · 29 comments
Assignees
Labels
Milestone

Comments

@ekelleyv
Copy link

ekelleyv commented Nov 5, 2013

Zooming with the scroll gesture causes very jumpy behavior between zoom levels. The calls to _performZoom seem to add up, meaning the map keeps zooming after the user has stopped scrolling.

Basically the Apple trackpad creates A LOT of calls to _onWheelScroll and because of inertial scrolling, continues to call it after the user has actually stopped scrolling. For my uses, changing the value

var left = Math.max(40 - (+new Date() - this._startTime), 0);

to something more like:

var left = Math.max(200 - (+new Date() - this._startTime), 0);

works decently. It makes the apple mouse a little too sensitive (harder to move between small numbers of zoom levels), but seems to get rid of the jumping effect. Any ideas of a better solution?

@ekelleyv
Copy link
Author

ekelleyv commented Nov 5, 2013

Probably the best thing would be to throttle the rate at which _onWheelScroll can be called.

@mourner
Copy link
Member

mourner commented Nov 7, 2013

Yeah, I'm still looking into it — see #1436. It's a complicated issue, with different browsers / input devices working differently with mousewheel (some make lots of events and small delta values, others less events and larger values), so it's hard to make it balanced across devices. But I agree it should be fixed somehow.

Feel free to experiment with the scroll-wheel zoom code more and let me know if you get to a point where it works pretty well. Closing in favor of an earlier issue.

@mourner mourner closed this as completed Nov 7, 2013
@mourner
Copy link
Member

mourner commented Nov 7, 2013

In fact I'll better reopen this one in favor of the old one, since it has more info.

@mourner mourner reopened this Nov 7, 2013
@ghost ghost assigned mourner Nov 7, 2013
@mourner
Copy link
Member

mourner commented Nov 7, 2013

@danzel @jfirebaugh thoughts?

@jfirebaugh
Copy link
Member

Study what D3 does? :)

@mourner
Copy link
Member

mourner commented Nov 7, 2013

@jfirebaugh I knew this was coming. :) It's easier with continuous zoom but gets tricky with Leaflet-style one.

@ekelleyv
Copy link
Author

ekelleyv commented Nov 7, 2013

Ya. I think the main issue is that, no matter what, we have to map a smooth scrolling action to a handful of discrete zoom levels. The fix i tried earlier, changing 40 to 200 just really is not a good solution. It makes all the zoom actions feel more delayed and still does not really address the issue.

Maybe we could look into using the velocity of the scroll action to predict what zoom level is desired? Its just hard to come up with a solution that fixes the apple trackpads without breaking other devices.

@danzel
Copy link
Member

danzel commented Nov 7, 2013

Maybe we look at tracking the maximum change we receive and ignore events lower than 1/10th or if or something (Haven't tested, just a random idea)

@ekelleyv
Copy link
Author

ekelleyv commented Nov 7, 2013

@danzel I think the problem with that is the apple trackpad has a very large range in values. Ignoring them by size would ignore the initial part of the scroll or not let you zoom slowly.

My current solution is basically the following in _onWheelScroll:

        if (!this._startTime) {
            this._startTime = +new Date();
            if (!this._windowSize) {
                //If the first delta is really small, it is a trackpad
                this._windowSize = (Math.abs(delta) < 0.1) ? 250 : 40;
            }
        }

        var left = Math.max(this._windowSize - (+new Date() - this._startTime), 0);

I have found that a larger window is much better for the trackpad. However the larger window makes the mouse seem less responsive (because it is waiting 200ms to trigger the event instead of 40). Therefore I check to see the size of the first delta, if it is small then it is a trackpad and should have a larger window.

This is probably not the right solution, but it definitely feels better.

@mourner
Copy link
Member

mourner commented Nov 7, 2013

@ekelleyv that's really interesting! And if it gets us closer to a smooth zoom interaction on all devices, we should go for it. Can you test on different browsers (FF, Chrome, Safari, IE) on both mouse and trackpad where possible, and submit a pull request if everything's OK?

@ekelleyv
Copy link
Author

ekelleyv commented Nov 7, 2013

@mourner Sure, I will take a look at it. Now this is what was working better in a very specific use case (only a couple zoom levels and a custom marker layer), so I would have to try it out on a more general map and see if it is actually decent.

@mourner
Copy link
Member

mourner commented Nov 7, 2013

@ekelleyv use the debug/map/map.html for example — the pages in the debug folder reflect changes immediately without the need to rebuild, and there's enough of "general" maps.

@ekelleyv
Copy link
Author

ekelleyv commented Nov 7, 2013

@mourner Will do. Thanks

@mourner
Copy link
Member

mourner commented Nov 10, 2013

@ekelleyv so, any updates? I'd love to get this improved for the 0.7 release which I'm going to do next week.

@ekelleyv
Copy link
Author

@mourner Sorry, I was away from my computer all weekend. I will give it a try tonight and see how it works.

@mourner
Copy link
Member

mourner commented Nov 12, 2013

Thanks! Fiddled with this a bit, and I don't quite like how it works with the pull merged on my Macbook Pro — there's definitely still a problem (zooming several times in a row most of the time), but much less responsive. The change only helps with short scrolling movements.

So postponing the work on this to 0.8, perhaps we'll be able to come up with some other trick to make it better.

@ekelleyv
Copy link
Author

@mourner Ya, I agree that it is not a production-ready solution. Basically what I see happening (with the current implementation on a trackpad) is the following:

What I was trying to do was capture the entirety of that curve before calling zoom, but that starts to effect the responsiveness very quickly. Basically, in the first 40ms (or whatever window we choose) we must be able to predict the shape of the rest of the curve and then somehow gracefully ignore the rest of the curve after the zoom has occurred.

That itself would not be too difficult, but I have not figured out how to handle if the user changes/adjusts their scroll rate after the zoom has started. Any thoughts?

@mourner
Copy link
Member

mourner commented Dec 4, 2013

I have a new idea: detect if we deal with a high-res trackpad (by tracking wheel delta), and if we do, perform scroll wheel with the same logic we do with TouchZoom while the user pinches the map (no CSS transitions, the map scales until we stop scrolling and only then animates to final integer zoom).

That could work pretty well, but I'll have to make some refactoring to TouchZoom code to be able to reuse the logic.

@danzel
Copy link
Member

danzel commented Dec 4, 2013

sounds good!

@robrecord
Copy link

Great work guys! So when is it going to be included? This has been a problem for years and seems to affect (i.e. frustrates) every mac owner with a magic mouse. Might seem a minor bug to some, but Google maps is almost unusable for us in this state. PLEASE prioritise! :)

http://www.leancrew.com/all-this/2011/05/the-google-maps-scrollzoom-confusion/
http://www.cnet.com/uk/news/google-maps-apple-magic-mouse-fail/

If you came here in search of help with Safari: https://sites.google.com/site/scrollmaps/home
or Chrome: https://gist.github.com/henrik/1062679 / https://chrome.google.com/webstore/detail/scrollmaps/jifommjndpnefcfplgnbhabocomgdjjg

@rossmeissl
Copy link

@mourner do you have thoughts on timing for this? Thanks for your work on Leaflet.

@ghost
Copy link

ghost commented Aug 15, 2014

This works:

var lastScroll = new Date().getTime();
L.Map.ScrollWheelZoom.prototype._onWheelScroll = function (e) {
    if (new Date().getTime() - lastScroll < 400) { return; }
    var delta = L.DomEvent.getWheelDelta(e);
    var debounce = this._map.options.wheelDebounceTime;

    this._delta += delta;
    this._lastMousePos = this._map.mouseEventToContainerPoint(e);

    if (!this._startTime) {
        this._startTime = +new Date();
    }

    var left = Math.max(debounce - (+new Date() - this._startTime), 0);

    clearTimeout(this._timer);
    lastScroll = new Date().getTime();
    this._timer = setTimeout(L.bind(this._performZoom, this), left);

    L.DomEvent.stop(e);
}

@schovi
Copy link

schovi commented Aug 29, 2014

@Mearsheimer Thanks for quick solution. Works perfectly!

@DennisSchiefer
Copy link

Any news on this front? schovi mentioned that this solution works perfectly, but even with the current master, I still see issues when using a Magic Mouse. They can be classified roughly into two categories:

  • Sometimes when zooming, there is a zoom by one level in the wrong direction at the end or at the beginning. I assume this could be when not swiping completely vertical on the mouse, as swiping horizontally also initiates zooming
  • Zooming is not as intuitive as with a scroll wheel. One is likely to overshoot the intended zoom level (sometimes a small touch on the mouse zooms by 2-3 levels - this is also when just trying to perform a click with the mouse). Maybe adding a threshold for _delta could help, i.e. only trigger a zoom after enough delta value has accumulated?

@ccolgrove
Copy link

I'm also interested in the status of this issue. We recently upgraded from 0.7.2 to 0.7.3 and started seeing bizarre scrolling behavior when using an Apple trackpad. It made scrolling so unusable we had to downgrade.

@mourner
Copy link
Member

mourner commented Oct 7, 2015

I had some work in progress on this that should in theory improve things in #3653. Can you try out that branch? I would appreciate feedback.

@ccolgrove
Copy link

Thanks for the quick response! I played around with that branch and it does look like the over-zooming problem is fixed. However, I noticed when I zoom in, the image does not stay centered on the mouse, but instead zooms off to one side and then snaps back into place. Happy to provide more details if you need them.

@DennisSchiefer
Copy link

Thank you for still working on the problem. I know that this is a tough one to solve - I also gave it a quick try but quickly realized that finding a satisfying solution won't be easy.
That said, your new branch unfortunately does not noticably change the behaviour I see in Safari with the MagicMouse as described above.

@yohanboniface
Copy link
Member

Should be fixed with #3653 please open a new issue if needed.

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

Successfully merging a pull request may close this issue.

10 participants