+++ This bug was initially created as a clone of Bug #1083530 +++ This patch improves the touch resampling algorithm a bit. First, since we can use bug 1083530 which uses mozilla::TimeStamps, we can delete some touch tracking code to get accurate time differences between touches. We also need two improvements. 1) Vsync events could run ahead of touch events. For example, a touch event might be 20ms behind the last vsync event. In these cases, just dispatch the last touch event and don't resample. 2) Previously, we could resample touch events that were ahead of vsync events. For example, a vsync could occur at time t=16 ms, a touch event at time t=18ms, and due to the main thread latency, the vsync event would process at time t=19ms. We would resample the touch event that occurred AFTER vsync, which is a bug. We fix this to only resample touch events that occur before the vsync event.

Rebased on master and latest patch from bug 1083530.

Quick note - patches for review should show the function in each chunk. See https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F for recommended .hgrc settings. In particular, see the diff section. I don't insist on 8 lines of context, though I appreciate it for more complicated patches.

Proper diff with functions and more context.

Rebased off bug 1094525. Introduces two heuristics: 1) We only resample the two touch events at or before the vsync time. Before, we could get touch events after the vsync event. By the time the main thread processed the vsync event, a touch event AFTER the vsync event would be resampled. 2) Check to see if a vsync event is far ahead of touch events. This can happen if the android touch thread hasn't added a touch in a while. In this case, we just dispatch the last touch event and clear out the touch queue.

Comment on attachment 8518461 [details] [diff] [review] Improve Resampling Heuristics Review of attachment 8518461 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/gonk/GeckoTouchDispatcher.cpp @@ +285,5 @@ > */ > +bool > +GeckoTouchDispatcher::GetTouchEvents(MultiTouchInput& aBaseTouch, > + MultiTouchInput& aCurrentTouch, > + TimeStamp aVsyncTime) I don't understand why this is here. Don't we always want to grab the last two events? If the vsync time runs behind the base touch time, we can choose not to resample. It shouldn't take a function like this to do that. Maybe this made more sense when there was a difference between interpolation and extrapolation?

(In reply to Michael Wu [:mwu] from comment #5) > Comment on attachment 8518461 [details] [diff] [review] > Improve Resampling Heuristics > > Review of attachment 8518461 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: widget/gonk/GeckoTouchDispatcher.cpp > @@ +285,5 @@ > > */ > > +bool > > +GeckoTouchDispatcher::GetTouchEvents(MultiTouchInput& aBaseTouch, > > + MultiTouchInput& aCurrentTouch, > > + TimeStamp aVsyncTime) > > I don't understand why this is here. Don't we always want to grab the last > two events? If the vsync time runs behind the base touch time, we can choose > not to resample. It shouldn't take a function like this to do that. Maybe > this made more sense when there was a difference between interpolation and > extrapolation? Attached is a spreadsheet detailing why we need this. In some cases, the time between a vsync notification and the main thread processing the vsync can be a couple of milliseconds. Between the vsync notification and the main thread processing the touch event, a touch event can occur. Thus, getting the latest touch event will actually be processing a touch event AFTER the vsync event. See row 4 in the spreadsheet. On a flame, touch events come in at 13 ms. Vsyncs every 16.66 ms. The spreadsheet assumes 1 pixel per millisecond movement. Let's say for example at vsync t=49, we have a main thread delay that executes at t=53, a touch event at t=52 can occur. When we resample, if we resample the last two touch events (touch t=39, t=52), we have a layer displacement of 29 pixels, when we really want to move only 16.6 pixels. This is too far. If we resample the last two touch events before the vsync (touch t=26, t=39), we resample to the correct 16.6 px. Does that explain it?

Nevermind, I had a bug in the spreedsheet, I think you're right from comment 5. I'm going to take another look at this.

Improves touch resampling heuristics by adding one more: Don't let the vsync events get too far ahead of the touch events. If the touch input thread is lagging and we get a vsync event, just send the last touch event. Also cleans up the code a bit.

Comment on attachment 8521015 [details] [diff] [review] Improve Resampling Heuristics Review of attachment 8521015 [details] [diff] [review]: ----------------------------------------------------------------- Getting rid of mTouchTimeDiff and mLastTouchTime is nice.