Port Chromium OS touch noise filtering to Chromium

ui/events/ozone/evdev/touch_event_converter_evdev.cc

Index: ui/events/ozone/evdev/touch_event_converter_evdev.cc
diff --git a/ui/events/ozone/evdev/touch_event_converter_evdev.cc b/ui/events/ozone/evdev/touch_event_converter_evdev.cc
index 3aaaa315ba28df765aca924afa6efc9d139e583e..06d0d173728618e7423d92e287fa388dfc477654 100644
--- a/ui/events/ozone/evdev/touch_event_converter_evdev.cc
+++ b/ui/events/ozone/evdev/touch_event_converter_evdev.cc
@@ -29,6 +29,8 @@
 #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_evdev_types.h"
+#include "ui/events/ozone/evdev/touch_noise/touch_noise_finder.h"
 
 namespace {
 
@@ -59,18 +61,6 @@ void GetTouchCalibration(TouchCalibration* cal) {
 
 namespace ui {
 
-TouchEventConverterEvdev::InProgressEvents::InProgressEvents()
-    : altered_(false),
-      x_(0),
-      y_(0),
-      id_(-1),
-      finger_(-1),
-      type_(ET_UNKNOWN),
-      radius_x_(0),
-      radius_y_(0),
-      pressure_(0) {
-}
-
 TouchEventConverterEvdev::TouchEventConverterEvdev(
     int fd,
     base::FilePath path,
@@ -83,6 +73,10 @@ TouchEventConverterEvdev::TouchEventConverterEvdev(
       is_type_a_(false),
       touch_points_(0),
       current_slot_(0) {
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kExtraTouchNoiseFiltering)) {
+    touch_noise_finder_.reset(new TouchNoiseFinder);
+  }
 }
 
 TouchEventConverterEvdev::~TouchEventConverterEvdev() {
@@ -98,7 +92,7 @@ void TouchEventConverterEvdev::Initialize(const EventDeviceInfo& info) {
   y_min_tuxels_ = info.GetAbsMinimum(ABS_MT_POSITION_Y);
   y_num_tuxels_ = info.GetAbsMaximum(ABS_MT_POSITION_Y) - y_min_tuxels_ + 1;
   touch_points_ =
-      std::min<int>(info.GetAbsMaximum(ABS_MT_SLOT) + 1, MAX_FINGERS);
+      std::min<int>(info.GetAbsMaximum(ABS_MT_SLOT) + 1, kNumTouchEvdevSlots);
 
   // Apply --touch-calibration.
   if (type() == INPUT_DEVICE_INTERNAL) {
@@ -119,14 +113,15 @@ void TouchEventConverterEvdev::Initialize(const EventDeviceInfo& info) {
 
   events_.resize(touch_points_);
   for (size_t i = 0; i < events_.size(); ++i) {
-    events_[i].finger_ = info.GetSlotValue(ABS_MT_TRACKING_ID, i);
-    events_[i].type_ =
-        events_[i].finger_ < 0 ? ET_TOUCH_RELEASED : ET_TOUCH_PRESSED;
-    events_[i].x_ = info.GetSlotValue(ABS_MT_POSITION_X, i);
-    events_[i].y_ = info.GetSlotValue(ABS_MT_POSITION_Y, i);
-    events_[i].radius_x_ = info.GetSlotValue(ABS_MT_TOUCH_MAJOR, i);
-    events_[i].radius_y_ = info.GetSlotValue(ABS_MT_TOUCH_MINOR, i);
-    events_[i].pressure_ = info.GetSlotValue(ABS_MT_PRESSURE, i);
+    events_[i].x = info.GetSlotValue(ABS_MT_POSITION_X, i);
+    events_[i].y = info.GetSlotValue(ABS_MT_POSITION_Y, i);
+    events_[i].tracking_id = info.GetSlotValue(ABS_MT_TRACKING_ID, i);
+    events_[i].slot = i;
+    events_[i].type =
+        events_[i].tracking_id < 0 ? ET_TOUCH_RELEASED : ET_TOUCH_PRESSED;
+    events_[i].radius_x = info.GetSlotValue(ABS_MT_TOUCH_MAJOR, i);
+    events_[i].radius_y = info.GetSlotValue(ABS_MT_TOUCH_MINOR, i);
+    events_[i].pressure = info.GetSlotValue(ABS_MT_PRESSURE, i);
   }
 }
 
@@ -152,7 +147,7 @@ int TouchEventConverterEvdev::GetTouchPoints() const {
 }
 
 void TouchEventConverterEvdev::OnFileCanReadWithoutBlocking(int fd) {
-  input_event inputs[MAX_FINGERS * 6 + 1];
+  input_event inputs[kNumTouchEvdevSlots * 6 + 1];
   ssize_t read_size = read(fd, inputs, sizeof(inputs));
   if (read_size < 0) {
     if (errno == EINTR || errno == EAGAIN)
@@ -174,7 +169,7 @@ void TouchEventConverterEvdev::OnFileCanReadWithoutBlocking(int fd) {
 void TouchEventConverterEvdev::ProcessInputEvent(const input_event& input) {
   if (input.type == EV_SYN) {
     ProcessSyn(input);
-  } else if(syn_dropped_) {
+  } else if (syn_dropped_) {
     // Do nothing. This branch indicates we have lost sync with the driver.
   } else if (input.type == EV_ABS) {
     if (events_.size() <= current_slot_) {
@@ -201,28 +196,32 @@ void TouchEventConverterEvdev::ProcessAbs(const input_event& input) {
       // TODO(spang): If we have all of major, minor, and orientation,
       // we can scale the ellipse correctly. However on the Pixel we get
       // neither minor nor orientation, so this is all we can do.
-      events_[current_slot_].radius_x_ = input.value / 2.0f;
+      events_[current_slot_].radius_x = input.value / 2.0f;
       break;
     case ABS_MT_TOUCH_MINOR:
-      events_[current_slot_].radius_y_ = input.value / 2.0f;
+      events_[current_slot_].radius_y = input.value / 2.0f;
       break;
     case ABS_MT_POSITION_X:
-      events_[current_slot_].x_ = input.value;
+      events_[current_slot_].x = input.value;
       break;
     case ABS_MT_POSITION_Y:
-      events_[current_slot_].y_ = input.value;
+      events_[current_slot_].y = input.value;
       break;
     case ABS_MT_TRACKING_ID:
       if (input.value < 0) {
-        events_[current_slot_].type_ = ET_TOUCH_RELEASED;
+        events_[current_slot_].type = ET_TOUCH_RELEASED;
       } else {
-        events_[current_slot_].finger_ = input.value;
-        events_[current_slot_].type_ = ET_TOUCH_PRESSED;
+        events_[current_slot_].tracking_id = input.value;
+        events_[current_slot_].type = ET_TOUCH_PRESSED;
+
+        // Clear the 'cancelled' bit because ET_TOUCH_PRESSED starts a new
+        // touch.
+        events_[current_slot_].cancelled = false;

[spang, 2015/03/18 19:20:12]

I think this block should be cut down to just:

events_[current_slot_].tracking_id = input.value;
events_[current_slot_].cancelled = false;

       }
       break;
     case ABS_MT_PRESSURE:
-      events_[current_slot_].pressure_ = input.value - pressure_min_;
-      events_[current_slot_].pressure_ /= pressure_max_ - pressure_min_;
+      events_[current_slot_].pressure = input.value - pressure_min_;
+      events_[current_slot_].pressure /= pressure_max_ - pressure_min_;
       break;
     case ABS_MT_SLOT:
       if (input.value >= 0 &&
@@ -237,7 +236,7 @@ void TouchEventConverterEvdev::ProcessAbs(const input_event& input) {
       DVLOG(5) << "unhandled code for EV_ABS: " << input.code;
       return;
   }
-  events_[current_slot_].altered_ = true;
+  events_[current_slot_].altered = true;
 }
 
 void TouchEventConverterEvdev::ProcessSyn(const input_event& input) {
@@ -247,8 +246,8 @@ void TouchEventConverterEvdev::ProcessSyn(const input_event& input) {
         // Have to re-initialize.
         if (Reinitialize()) {
           syn_dropped_ = false;
-          for(InProgressEvents& event: events_)
-            event.altered_ = false;
+          for(InProgressTouchEvdev& event : events_)
+            event.altered = false;
         } else {
           LOG(ERROR) << "failed to re-initialize device info";
         }
@@ -261,10 +260,13 @@ void TouchEventConverterEvdev::ProcessSyn(const input_event& input) {
     case SYN_MT_REPORT:
       // For type A devices, we just get a stream of all current contacts,
       // in some arbitrary order.
-      events_[current_slot_].type_ = ET_TOUCH_PRESSED;
+      events_[current_slot_].type = ET_TOUCH_PRESSED;
       if (events_.size() - 1 > current_slot_)
         current_slot_++;
       is_type_a_ = true;
+
+      // TouchNoiseFinder does not support type A devices.
+      touch_noise_finder_.reset();
       break;
     case SYN_DROPPED:
       // Some buffer has overrun. We ignore all events up to and
@@ -276,25 +278,57 @@ void TouchEventConverterEvdev::ProcessSyn(const input_event& input) {
   }
 }
 
-void TouchEventConverterEvdev::ReportEvent(int touch_id,
-                                           const InProgressEvents& event,
-                                           const base::TimeDelta& timestamp) {
+void TouchEventConverterEvdev::MaybeReportEvent(
+    const InProgressTouchEvdev& event,
+    const base::TimeDelta& timestamp,
+    bool* cancelled) {
+  *cancelled = event.cancelled;
+  if (event.cancelled)
+    return;
+
+  EventType event_type = event.type;
+  if (touch_noise_finder_ && touch_noise_finder_->SlotHasNoise(event.slot)) {

[spang, 2015/03/18 19:20:12]

Could we centralize determining the event type here? Something like:

bool contact = (event->tracking_id != -1);
EventType event_type = ET_UNKNOWN;

if (event->cancelled)
  return;

if (touch_noise_finder_ && touch_noise_finder_->SlotHasNoise(event->slot)) {
  event->cancelled = true;
  event->client_contact = false;
  event_type = ET_TOUCH_CANCELLED;
} else if (contact && event->client_contact) {
  event_type = ET_TOUCH_MOVED;
} else if (contact && !event->client_contact) {
  event_type = ET_TOUCH_PRESSED;
  event->client_contact = true;
} else if (!contact && event->client_contact) {
  event_type = ET_TOUCH_RELEASED;
  event->client_contact = false;
}

dispatcher_->DispatchTouchEvent(...);

+    *cancelled = true;
+    if (event_type == ET_TOUCH_PRESSED)
+      return;
+    // Cancel this event and drop subsequent events.
+    event_type = ET_TOUCH_CANCELLED;
+  }
+
   dispatcher_->DispatchTouchEvent(TouchEventParams(
-      id_, touch_id, event.type_, gfx::PointF(event.x_, event.y_),
-      gfx::Vector2dF(event.radius_x_, event.radius_y_), event.pressure_,
+      id_, event.slot, 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) {
+  if (touch_noise_finder_)
+    touch_noise_finder_->HandleTouches(events_, delta);
+
   for (size_t i = 0; i < events_.size(); i++) {
-    if (events_[i].altered_) {
-      ReportEvent(i, events_[i], delta);
+    InProgressTouchEvdev* event = &events_[i];
+    if (!event->altered)
+      continue;
 
-      // Subsequent events for this finger will be touch-move until it
-      // is released.
-      events_[i].type_ = ET_TOUCH_MOVED;
-      events_[i].altered_ = false;
+    bool cancelled = false;
+    MaybeReportEvent(*event, delta, &cancelled);
+    event->cancelled = cancelled;
+
+    // Update |event->type| regardless of whether an event was dispatched
+    // because TouchNoiseFinder::HandleTouches() needs the touch's uncancelled
+    // state to determine whether future touches should be cancelled.

[flackr, 2015/03/17 03:54:15]

I think this would still work with the cancelled type as long as the touch noise
finder continues to get events for cancelled events.

[pkotwicz, 2015/03/17 16:51:23]

SinglePositionTouchNoiseFilter needs to know when a touch was actually released
(If we send ET_TOUCH_CANCELLED instead of ET_TOUCH_RELEASED to
SinglePositionTouchNoiseFilter, we change the behavior)

[flackr, 2015/03/17 16:59:04]

Right, I was thinking that we might be able to recognize the difference from a
still active (but cancelled) touch and no touch from the tracking id in the
events structure. That being said it's not a big deal either way, getting the
original event types is a nice thing to have.

+    switch (event->type) {
+      case ET_TOUCH_PRESSED:
+        event->type = ET_TOUCH_MOVED;
+        break;
+      case ET_TOUCH_RELEASED:
+        event->type = ET_UNKNOWN;
+        break;
+      default:
+        break;
     }
+
+    event->altered = false;
   }
 }