Move touch CTM from X into Chrome

ui/aura/window_tree_host_x11.cc

Index: ui/aura/window_tree_host_x11.cc
diff --git a/ui/aura/window_tree_host_x11.cc b/ui/aura/window_tree_host_x11.cc
index efb0a6b5fbc3a705ae562e351ce560cd964e1b1a..ccef1c19cd44b4c082e19889c5cd6af36cdebd75 100644
--- a/ui/aura/window_tree_host_x11.cc
+++ b/ui/aura/window_tree_host_x11.cc
@@ -249,7 +249,6 @@ WindowTreeHostX11::WindowTreeHostX11(const gfx::Rect& bounds)
       current_cursor_(ui::kCursorNull),
       window_mapped_(false),
       bounds_(bounds),
-      is_internal_display_(false),
       touch_calibrate_(new internal::TouchEventCalibrate),
       atom_cache_(xdisplay_, kAtomsToCache) {
   XSetWindowAttributes swa;
@@ -409,7 +408,6 @@ uint32_t WindowTreeHostX11::Dispatch(const base::NativeEvent& event) {
       bool size_changed = bounds_.size() != bounds.size();
       bool origin_changed = bounds_.origin() != bounds.origin();
       bounds_ = bounds;
-      UpdateIsInternalDisplay();
       // Always update barrier and mouse location because |bounds_| might
       // have already been updated in |SetBounds|.
       if (pointer_barriers_) {
@@ -556,7 +554,6 @@ void WindowTreeHostX11::SetBounds(const gfx::Rect& bounds) {
   // (possibly synthetic) ConfigureNotify about the actual size and correct
   // |bounds_| later.
   bounds_ = bounds;
-  UpdateIsInternalDisplay();
   if (origin_changed)
     OnHostMoved(bounds.origin());
   if (size_changed || current_scale != new_scale) {
@@ -724,14 +721,8 @@ void WindowTreeHostX11::OnWindowInitialized(Window* window) {
 }
 
 void WindowTreeHostX11::OnHostInitialized(WindowTreeHost* host) {
-  // TODO(beng): I'm not sure that this comment makes much sense anymore??
-  // UpdateIsInternalDisplay relies on WED's kDisplayIdKey property being set
-  // available by the time WED::Init is called. (set in
-  // DisplayManager::CreateRootWindowForDisplay)
-  // Ready when NotifyHostInitialized is called from WED::Init.
   if (host != this)
     return;
-  UpdateIsInternalDisplay();
 
   // We have to enable Tap-to-click by default because the cursor is set to
   // visible in Shell::InitRootWindowController.
@@ -762,34 +753,18 @@ void WindowTreeHostX11::DispatchXI2Event(const base::NativeEvent& event) {
     case ui::ET_TOUCH_PRESSED:
     case ui::ET_TOUCH_CANCELLED:
     case ui::ET_TOUCH_RELEASED: {
-#if defined(OS_CHROMEOS)
       // Bail out early before generating a ui::TouchEvent if this event
-      // is not within the range of this RootWindow. Converting an xevent
-      // to ui::TouchEvent might change the state of the global touch tracking
-      // state, e.g. touch release event can remove the touch id from the
-      // record, and doing this multiple time when there are multiple
-      // RootWindow will cause problem. So only generate the ui::TouchEvent
-      // when we are sure it belongs to this RootWindow.
-      if (base::SysInfo::IsRunningOnChromeOS() &&
-          !bounds_.Contains(ui::EventLocationFromNative(xev)))
+      // is targeting this RootWindow.

[oshima, 2014/03/14 21:53:53]

then you can do something like

if (GetTouchCalibrateClient(window())->CalibrateTouchEvent(xev, bounds_,
&touchev))
  SendEventToProcessor(&touchev);

?

[Yufeng Shen (Slow to review), 2014/04/29 20:34:18]

I moved TouchCalibrate into DeviceDataManager.

+      // Converting an xevent to ui::TouchEvent might change the state of
+      // the global touch tracking state, e.g. touch release event can
+      // remove the touch id from the record, and doing this multiple time
+      // when there are multiple RootWindow will cause problem. So only
+      // generate the ui::TouchEvent when we are sure it is targeting this
+      // RootWindow.
+      if (!IsTouchEventTargetingThisRootWindow(xev))
         break;
-#endif  // defined(OS_CHROMEOS)
       ui::TouchEvent touchev(xev);
-#if defined(OS_CHROMEOS)
-      if (base::SysInfo::IsRunningOnChromeOS()) {
-        // X maps the touch-surface to the size of the X root-window.
-        // In multi-monitor setup, Coordinate Transformation Matrix
-        // repositions the touch-surface onto part of X root-window
-        // containing aura root-window corresponding to the touchscreen.
-        // However, if aura root-window has non-zero origin,
-        // we need to relocate the event into aura root-window coordinates.
-        touchev.Relocate(bounds_.origin());

[ynovikov, 2014/03/27 23:35:53]

I think this is the only use of Relocate(), could you remove the function?

[Yufeng Shen (Slow to review), 2014/04/29 20:34:18]

Done.

-#if defined(USE_XI2_MT)
-        if (is_internal_display_)
-          touch_calibrate_->Calibrate(&touchev, bounds_);
-#endif  // defined(USE_XI2_MT)
-      }
-#endif  // defined(OS_CHROMEOS)
+      CalibrateTouchEvent(xev, &touchev);
       SendEventToProcessor(&touchev);
       break;
     }
@@ -868,13 +843,6 @@ void WindowTreeHostX11::TranslateAndDispatchMouseEvent(
   SendEventToProcessor(event);
 }
 
-void WindowTreeHostX11::UpdateIsInternalDisplay() {
-  Window* root_window = window();
-  gfx::Screen* screen = gfx::Screen::GetScreenFor(root_window);
-  gfx::Display display = screen->GetDisplayNearestWindow(root_window);
-  is_internal_display_ = display.IsInternal();
-}
-
 void WindowTreeHostX11::SetCrOSTapPaused(bool state) {
 #if defined(OS_CHROMEOS)
   if (!ui::IsXInput2Available())
@@ -905,6 +873,49 @@ void WindowTreeHostX11::SetCrOSTapPaused(bool state) {
 #endif
 }
 
+bool WindowTreeHostX11::IsTouchEventTargetingThisRootWindow(
+    const base::NativeEvent& event) {

[sadrul, 2014/03/15 19:32:51]

Just use XEvent* here.

[Yufeng Shen (Slow to review), 2014/04/29 20:34:18]

Done.

+#if defined(OS_CHROMEOS)
+  XEvent* xev = event;
+  XIDeviceEvent* xiev = static_cast<XIDeviceEvent*>(xev->xcookie.data);
+  int64 touch_display_id =
+      ui::DeviceDataManager::GetInstance()->GetDisplayForTouchDevice(
+          xiev->deviceid);
+  // If we don't have record of display id for this touch device, then
+  // fall backt to check if this touch event is within the bounds of

[sadrul, 2014/03/15 19:32:51]

*back

[Yufeng Shen (Slow to review), 2014/04/29 20:34:18]

Done.

+  // this root window.
+  if (touch_display_id == gfx::Display::kInvalidDisplayID) {
+    if (base::SysInfo::IsRunningOnChromeOS() &&
+        !bounds_.Contains(ui::EventLocationFromNative(xev)))

[ynovikov, 2014/03/27 23:35:53]

As I understand it, this check will be wrong most of the time.
If we don't have display <-> touchscreen mapping, we also don't have  CTM.
So, EventLocationFromNative will return touches in X's display range.
Meaning, if we have 2 monitors and one touchscreen, touches at the top will go
to first monitor, and touches at the bottom will go to the second monitor.

The only case this would work properly, is if we have 1 monitor which is also 1
touchscreen.
Perhaps add a check here whether we are in this situation? And discard the event
if we have more than 1 monitor?

Also, please document this behavior.

It would probably also be useful to add UMA stat or something to track how often
do we end up in a situation where we don't know where touchscreen event should
go to, with perhaps some information (keeping privacy in mind) which will help
us fix it.

[Yufeng Shen (Slow to review), 2014/04/29 20:34:18]

I am more interested in not regress the 1 monitor case so that's why
the behavior of if no record of display <-> touchscreen map, I want
to keep the old logic.

As for the 2 monitor case, if the mapping is wrong then we will get
complaints and we go fix it.

+      return false;
+    // If we do have record of the display id for this touch device,

[sadrul, 2014/03/15 19:32:51]

I don't think this comment is necessary here.

[Yufeng Shen (Slow to review), 2014/04/29 20:34:18]

Done.

+    // then check if this touch display id is associated with this
+    // root window.
+  } else if (touch_display_id != display_ids().first &&
+             touch_display_id != display_ids().second) {
+    return false;
+  }
+#endif  // defined(OS_CHROMEOS)
+  return true;

[sadrul, 2014/03/15 19:32:51]

non-CHROMEOS case should do the bounds check.

This function could look like:

#if chromeos
  touch_display_id = ...
  if (touch_display_id != invalid) {
    return  (touch_display_id == display_ids().first ||
             touch_display_id == display_ids().second));
  }
#endif
  // check bounds

[Yufeng Shen (Slow to review), 2014/04/29 20:34:18]

The original code does not do the bounds check in non-chromeos case.
I would like to keep the original logic the same in this CL. And add the bounds
check for non-chromeos case in a separate CL if we have a concrete bug. 

+}
+
+void WindowTreeHostX11::CalibrateTouchEvent(const base::NativeEvent& event,
+                                            ui::TouchEvent* touchev) {
+#if defined(OS_CHROMEOS) && defined(USE_XI2_MT)
+  XEvent* xev = event;
+  XIDeviceEvent* xiev = static_cast<XIDeviceEvent*>(xev->xcookie.data);
+  int64 touch_display_id =
+      ui::DeviceDataManager::GetInstance()->GetDisplayForTouchDevice(
+          xiev->deviceid);
+  // On ChromeOS, if touch event is from internal display, we need to
+  // calibrate its location for bezel region.
+  if (base::SysInfo::IsRunningOnChromeOS() &&
+      touch_display_id == gfx::Display::InternalDisplayId()) {
+    touch_calibrate_->Calibrate(touchev, bounds_);
+  }
+#endif  // defined(OS_CHROMEOS) && defined(USE_XI2_MT)
+}
+
 // static
 WindowTreeHost* WindowTreeHost::Create(const gfx::Rect& bounds) {
   return new WindowTreeHostX11(bounds);