703311 - Use Android Java API for panning and flinging

Status: RESOLVED WONTFIX

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Chris Lord [:cwiiis]]

Some devices have various quirks in their raw events that trip up our hand-written code for panning and flinging. For example, my HTC Flyer always has two identical motion events before delivering the touch-up event, which breaks the velocity calculation.

Android has events for panning/flinging that have more context than would be available to us, and will guarantee that we behave similarly to other Android applications - We should use them.

Comment 1

[Chris Lord [:cwiiis]]

Attachment: Use GestureDetector

This patch replaces our hand-written code with usage of GestureDetector.

Comment 2

[Chris Lord [:cwiiis]]

There's an error with this patch that causes the panning to happen at twice the speed if it was initiated during kinetic panning. I'll get this fixed.

From bug #698154, there was some conversation on not using the Android API for gesture detection, as it can introduce some latency.

My comment on this;

"I'm still in favour of using the Android API. Devices are likely to have many different quirks and we'd be better off relying on the device manufacturer fixing these in the Android gesture detector than to write a general-purpose piece of code that we expect to work on every device.

If we aren't going to go with the Android API, which would take some effort, it'd be good to get some metrics that supports our decision."

Comment 3

[Wesley Johnston (:wesj)]

I'd like to see some builds with both options up and let people play?

I have a feeling that the native detector is a bit laggier than just using straight up events because it is trying harder to to determine what you're doing than we do? (i.e. I know we had to introduce some "lag" in XUL Fennec while we waited to determine if a touch was a click/pan/doubletap/etc.) Just guessing...?

Comment 4

[Chris Lord [:cwiiis]]

Attachment: Use GestureDetector

This fixes the problem that pcwalton mentioned on IRC - the problem was I forgot to cancel the fling timer, so position was being offset twice after flings. This also rebases it on latest birch (including the axis-locking change).

I'm still of the mind that we ought to use the Android GestureDetector - there is a slight increased latency, but we'll almost certainly incur the same or similar penalty when we need to distinguish between panning and touching, or when we have to smooth motion events due to inconsistent data.

wesj's idea is also a good one, though I think a test at this point will come out against GestureDetector (except on devices like mine where the raw events are semi-broken, of course :)) as we don't do much with the events yet.

Comment 5

[Patrick Walton (:pcwalton)]

Technically I have no issues with the patch. I'm just a little nervous about relying on Android for touch responsiveness. Touch responsiveness is really important and the built-in gesture detector does all sorts of rounding that makes me nervous.

I've never really been happy with Android's responsiveness. Others have noticed it too, for example [1]. (Note that the video in that link is about the importance of asynchronous panning and zooming, but the article was more general than that.)

I don't really have any specific concerns I can point to, so I won't stand in the way of this patch going in. But I'd just like to reiterate a general worry about using the stock libraries for critical stuff like this just because they're there. We will never have better platform integration than the stock browser. The way we beat it is to have a better UX, and that necessarily involves doing things our own way from time to time.

[1]: http://flyosity.com/iphone/androids-touch-responsiveness-is-terrible.php

Comment 6

[Patrick Walton (:pcwalton)]

Also, are you sure that different devices have different gesture detectors? I would think that the manufacturers wouldn't be touching the gesture detector APIs.

Comment 7

[Chris Lord [:cwiiis]]

hmm, I've changed my mind on this now, I think you're right - if we can do a better job, let's do it. We should experiment with using onFling, as it does all the motion smoothing that we want, but if it doesn't match up we've got code in browser.js that does the same thing reasonably well.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

So then do we want to get rid of using the ScaleGestureDetector as well? I haven't really noticed much lag with that though.

Comment 9

[Chris Lord [:cwiiis]]

I'd say that the lag is minor enough and that pinch-zooming is rare enough that there should be no problem sticking with ScaleGestureDetector, but just my opinion.

Comment 10

[Chris Lord [:cwiiis]]

An additional note, I think it would be nice to implement things in a similar way to the Android API to 1- reduce the complexity of PanZoomController, and 2- make it easy to switch if we ever change our mind.

Comment 11

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 575684 [details] [diff] [review]
Use GestureDetector

r-'ing this one since we decided not to go this route (for now, anyway) and I want to get it out of my review queue.

Comment 12

[Chris Lord [:cwiiis]]

We've decided we don't want to use the Android gesture detector for now, we can re-open/re-file this in the future.

Comment 13

[Patrick Walton (:pcwalton)]

Comment on attachment 575684 [details] [diff] [review]
Use GestureDetector

Review of attachment 575684 [details] [diff] [review]:
-----------------------------------------------------------------

f- to get this out of my queue.