Port Chromium OS touch noise filtering to Chromium

ui/events/ozone/evdev/touch_event_converter_evdev.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/events/ozone/evdev/touch_event_converter_evdev.h"
 
 #include <errno.h>
 #include <fcntl.h>
 #include <linux/input.h>
 #include <poll.h>
@@ skipping 11 matching lines @@
 #include "base/message_loop/message_loop.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "ui/events/devices/device_data_manager.h"
 #include "ui/events/devices/device_util_linux.h"
 #include "ui/events/event.h"
 #include "ui/events/event_constants.h"
 #include "ui/events/event_switches.h"
 #include "ui/events/ozone/evdev/device_event_dispatcher_evdev.h"
+#include "ui/events/ozone/evdev/touch_noise/touch_noise_remover.h"
 
 namespace {
 
 struct TouchCalibration {
   int bezel_left;
   int bezel_right;
   int bezel_top;
   int bezel_bottom;
 };
 
@@ skipping 34 matching lines @@
     base::FilePath path,
     int id,
     InputDeviceType type,
     DeviceEventDispatcherEvdev* dispatcher)
     : EventConverterEvdev(fd, path, id, type),
       dispatcher_(dispatcher),
       syn_dropped_(false),
       is_type_a_(false),
       touch_points_(0),
       current_slot_(0) {
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kExtraTouchNoiseFiltering)) {
+    touch_noise_remover_.reset(new TouchNoiseRemover);
+  }
 }
 
 TouchEventConverterEvdev::~TouchEventConverterEvdev() {
   Stop();
   close(fd_);
 }
 
 void TouchEventConverterEvdev::Initialize(const EventDeviceInfo& info) {
   pressure_min_ = info.GetAbsMinimum(ABS_MT_PRESSURE);
   pressure_max_ = info.GetAbsMaximum(ABS_MT_PRESSURE);
@@ skipping 173 matching lines @@
     case SYN_DROPPED:
       // Some buffer has overrun. We ignore all events up to and
       // including the next SYN_REPORT.
       syn_dropped_ = true;
       break;
     default:
       NOTIMPLEMENTED() << "invalid code for EV_SYN: " << input.code;
   }
 }
 
-void TouchEventConverterEvdev::ReportEvent(int touch_id,
-                                           const InProgressEvents& event,
-                                           const base::TimeDelta& timestamp) {
-  dispatcher_->DispatchTouchEvent(TouchEventParams(
-      id_, touch_id, event.type_, gfx::PointF(event.x_, event.y_),
-      gfx::Vector2dF(event.radius_x_, event.radius_y_), event.pressure_,
-      timestamp));
-}
-
 void TouchEventConverterEvdev::ReportEvents(base::TimeDelta delta) {
+  std::vector<TouchEventParams> to_dispatch;

[flackr, 2015/03/10 05:23:40]

If I understand correctly, we construct and copy changed events into this temp
vector, and then effectively reconstruct the events_ array in
touch_noise_remover from the changed events vector, and then search for canceled
touches in that copy of the touches array to remove from the to_dispatch vector,
so that we can iterate over it and dispatch.

This seems fairly inefficient. Couldn't we have touch_noise_remover look
directly at the events_ array and maintain a canceled touches bool array or even
update the event type to ET_TOUCH_CANCELLED as necessary? I recognize that this
will require a bit of refactoring of the filters to use the fields from events_
but this is called at 60Hz or more so it's probably worth making sure it's fast.

[pkotwicz, 2015/03/13 03:53:16]

I have refactored this as you requested

[flackr, 2015/03/13 15:08:35]

Thanks!

   for (size_t i = 0; i < events_.size(); i++) {
     if (events_[i].altered_) {
-      ReportEvent(i, events_[i], delta);
+      const InProgressEvents& event = events_[i];
+      to_dispatch.push_back(
+          TouchEventParams(id_, i, event.finger_, event.type_,
+                           gfx::PointF(event.x_, event.y_),
+                           gfx::Vector2dF(event.radius_x_, event.radius_y_),
+                           event.pressure_, delta));
 
       // Subsequent events for this finger will be touch-move until it
       // is released.
       events_[i].type_ = ET_TOUCH_MOVED;

[flackr, 2015/03/10 05:23:39]

Since we change the event type to MOVED regardless of whether the touch was
canceled, it seems very important that once a touch is cancelled it remain
cancelled by the touch_noise_remover_. It's probably worth enforcing this or
verifying this with a test to prevent suddenly getting moved events for a touch
for which you never got a start.

       events_[i].altered_ = false;
     }
   }
+
+  if (touch_noise_remover_)
+    touch_noise_remover_->RemoveNoise(&to_dispatch, delta);

[flackr, 2015/03/10 05:23:39]

Can't just remove updates for noisy touches and not dispatch events for them. In
some cases, we don't identify that a touch is noise on the very first frame in
which that finger shows up. If this happens we'll deliver a touch start, perhaps
some touch moves, and then never send a touch end. 

Instead we need to dispatch an ET_TOUCH_CANCELLED event if an existing touch is
cancelled (doesn't hurt to always send a touch cancelled which is I believe the
current behavior in X). This will cancel any touch start effects (like :active
css pseudoclass) and prevent generation of timer based events (i.e. long press).

[pkotwicz, 2015/03/13 03:53:16]

I have changed the code to dispatch ET_TOUCH_CANCELLED when a touch is
cancelled. This is slightly different from what events_x.cc does.
GetTouchEventType() drops cancelled ET_TOUCH_PRESSes and ET_TOUCH_MOVEs. It
converts cancelled ET_TOUCH_RELEASEs to ET_TOUCH_CANCELLEDs

I have added a test as you requested:
TouchEventConverterEvdevTouchNoiseTest.TouchNoiseFiltering

[flackr, 2015/03/13 15:08:35]

Thanks. That's very strange that we used to keep the "noisy" touch as valid
until the release, and not at all the intention.

+
+  for (const TouchEventParams& params : to_dispatch)
+    dispatcher_->DispatchTouchEvent(params);
 }
 
 }  // namespace ui