d3 / d3-zoom

Pan and zoom SVG, HTML or Canvas using mouse or touch input.
https://d3js.org/d3-zoom
ISC License
502 stars 143 forks source link

Use touches rather than changedTouches in touchstarted #176

Closed robinhouston closed 4 years ago

robinhouston commented 5 years ago

It seems that the behaviour of Chrome on Android differs from the behaviour of Safari on iOS in the following respect: when a multitouch event is initiated by touching two or more fingers to the screen near-simultaneously, iOS Safari will fire a single touchstart event that includes all the touches (as both event.touches and event.changedTouches) whereas Android Chrome will fire a sequence of touchstart events:

In other words, the behaviour on Android Chrome when the screen is touched with two fingers simultaneously is the same as the behaviour on iOS Safari when you touch it first with one finger and then a second.

The change in PR #169 makes it possible to ignore single-touch gestures on iOS Safari, by returning a falsy value from the filter function when there is only a single touch, but this fails on Android Chrome because the first touch of a multitouch gesture is never recorded by d3-zoom: when the second touch is processed, the changedTouches array contains only the second touch.

This PR changes the behaviour of the touchstarted handler to consider all the touches in the event, rather than just the new touches, which appears to resolve the problem.

mbostock commented 5 years ago

So if I understand correctly: you use zoom.filter to ignore the first touch, but when the second touch comes down, we need to consider not just event.changedTouches (which only includes the second touch) but event.touches (which includes both). That makes sense.

mbostock commented 4 years ago

Merged as c4734c2.

mbostock commented 4 years ago

And reverted in 176ad733ca76be6b7e58ed3ca438ad5e088f8486 1.8.2.

This change broke the gesture it described on iOS Safari in the common case: when you touch it first with one finger and then a second. The problem is that when iterating over the event.touches in the second touchstart, it sets g.touch1 to be the same touch as the g.touch0 set on the first touchstart.

If you want to resubmit the PR, I think you’ll need something like this:

for (i = 0; i < n; ++i) {
  t = touches[i], p = touch(this, touches, t.identifier);
  p = [p, this.__zoom.invert(p), t.identifier];
  if (!g.touch0) g.touch0 = p, started = true, g.taps = 1 + !!touchstarting;
  else if (!g.touch1 && g.touch0[2] !== p[2]) g.touch1 = p, g.taps = 0;
}

This way, if the first touchstart is filtered, the second will get both touches, whereas if the first touchstart is not filtered, the second will only get one additional touch.

mbostock commented 4 years ago

Okay, well, that works so it’ll be in 1.8.3…

robinhouston commented 4 years ago

Ah! Sorry not to have caught this problem. Thanks for fixing it.