Move touch CTM from X into Chrome

ui/aura/window_tree_host_x11.cc

 // Copyright (c) 2012 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/aura/window_tree_host_x11.h"
 
 #include <strings.h>
 #include <X11/cursorfont.h>
 #include <X11/extensions/Xfixes.h>
 #include <X11/extensions/XInput2.h>
@@ skipping 131 matching lines @@
   virtual ~TouchEventCalibrate() {
     base::MessageLoopForUI::current()->RemoveObserver(this);
   }
 
   // Modify the location of the |event|,
   // expanding it from |bounds| to (|bounds| + bezels).
   // Required when touchscreen is bigger than screen (i.e. has bezels),
   // because we receive events in touchscreen coordinates,
   // which need to be expanded when converting to screen coordinates,
   // so that location on bezels will be outside of screen area.
   void Calibrate(ui::TouchEvent* event, const gfx::Rect& bounds) {

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

Why do we still need this? Shouldn't this be part of the CTM? We shouldn't need
to calibrate twice.

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

I agree. Originally the second calibration was needed, since setting X's CTM to
account for bezels would have resulted in negative coordinates, which would have
been clipped.

Something that we need to think about, though, is how to determine whether the
touch is in bezels or not. The problem is that bezel size in pixels is different
in different resolutions. And when we set up mirror mode with pillarboxing, we
want to ignore the pillar area but respond to touches in bezel area (and both
will have negative coordinates).
One solution is to apply CTM to "native resolution - bezels" rectangle, i.e.,
assuming bezel sizes 20 on all sides, the (20,20)-(2539,1679) rectangle.
If we are in 1024x768 resolution, the picture would be in (164,20)-(2395,1679)
touchscreen coordinates, so the transformation would be
x = (x - 164) / (2395 - 164) * 1023
y = (y - 20) / (1679 - 20) * 767
So, "native resolution - bezels" would be translated to (-66,0)-(1089,767) and
everything outside that rectangle can be treated as bezels, for example top left
corner, which would be (-75,-9).

Note: In case we are mirroring to a touchscreen, we should use bezels for
internal touchscreen CTM, but not use them for external.

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

Lets do this in a separate CL if necessary.

 #if defined(USE_XI2_MT)
     int x = event->x();
     int y = event->y();
 
     if (!left_ && !right_ && !top_ && !bottom_)
       return;
 
     const int resolution_x = bounds.width();
     const int resolution_y = bounds.height();
     // The "grace area" (10% in this case) is to make it easier for the user to
@@ skipping 79 matching lines @@
 ////////////////////////////////////////////////////////////////////////////////
 // WindowTreeHostX11
 
 WindowTreeHostX11::WindowTreeHostX11(const gfx::Rect& bounds)
     : xdisplay_(gfx::GetXDisplay()),
       xwindow_(0),
       x_root_window_(DefaultRootWindow(xdisplay_)),
       current_cursor_(ui::kCursorNull),
       window_mapped_(false),
       bounds_(bounds),
-      is_internal_display_(false),
       touch_calibrate_(new internal::TouchEventCalibrate),
       atom_cache_(xdisplay_, kAtomsToCache) {
   XSetWindowAttributes swa;
   memset(&swa, 0, sizeof(swa));
   swa.background_pixmap = None;
   swa.override_redirect = default_override_redirect;
   xwindow_ = XCreateWindow(
       xdisplay_, x_root_window_,
       bounds.x(), bounds.y(), bounds.width(), bounds.height(),
       0,               // border width
@@ skipping 139 matching lines @@
       DCHECK_EQ(xwindow_, xev->xconfigure.event);
       DCHECK_EQ(xwindow_, xev->xconfigure.window);
       // It's possible that the X window may be resized by some other means
       // than from within aura (e.g. the X window manager can change the
       // size). Make sure the root window size is maintained properly.
       gfx::Rect bounds(xev->xconfigure.x, xev->xconfigure.y,
           xev->xconfigure.width, xev->xconfigure.height);
       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_) {
         UnConfineCursor();
         ConfineCursorToRootWindow();
       }
       if (size_changed)
         OnHostResized(bounds.size());
       if (origin_changed)
         OnHostMoved(bounds_.origin());
@@ skipping 126 matching lines @@
   }
   if (value_mask)
     XConfigureWindow(xdisplay_, xwindow_, value_mask, &changes);
 
   // Assume that the resize will go through as requested, which should be the
   // case if we're running without a window manager.  If there's a window
   // manager, it can modify or ignore the request, but (per ICCCM) we'll get a
   // (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) {
     OnHostResized(bounds.size());
   } else {
     window()->SchedulePaintInRect(window()->bounds());
   }
 }
 
 gfx::Insets WindowTreeHostX11::GetInsets() const {
@@ skipping 147 matching lines @@
 }
 
 void WindowTreeHostX11::OnCursorVisibilityChangedNative(bool show) {
   SetCrOSTapPaused(!show);
 }
 
 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.
   SetCrOSTapPaused(false);
 }
 
 ui::EventProcessor* WindowTreeHostX11::GetEventProcessor() {
   return dispatcher();
 }
 
@@ skipping 10 matching lines @@
 
   ui::EventType type = ui::EventTypeFromNative(xev);
   XEvent last_event;
   int num_coalesced = 0;
 
   switch (type) {
     case ui::ET_TOUCH_MOVED:
     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;
     }
     case ui::ET_MOUSE_MOVED:
     case ui::ET_MOUSE_DRAGGED:
     case ui::ET_MOUSE_PRESSED:
     case ui::ET_MOUSE_RELEASED:
     case ui::ET_MOUSE_ENTERED:
     case ui::ET_MOUSE_EXITED: {
       if (type == ui::ET_MOUSE_MOVED || type == ui::ET_MOUSE_DRAGGED) {
@@ skipping 58 matching lines @@
     // host window, then convert it back to this host window's coordinate.
     screen_position_client->ConvertHostPointToScreen(root_window, &location);
     screen_position_client->ConvertPointFromScreen(root_window, &location);
     ConvertPointToHost(&location);
     event->set_location(location);
     event->set_root_location(location);
   }
   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())
     return;
   // Temporarily pause tap-to-click when the cursor is hidden.
   Atom prop = atom_cache_.GetAtom("Tap Paused");
   unsigned char value = state;
   XIDeviceList dev_list =
       ui::DeviceListCacheX::GetInstance()->GetXI2DeviceList(xdisplay_);
 
@@ skipping 10 matching lines @@
       if (result != Success)
         continue;
       XFree(data);
       XIChangeProperty(xdisplay_, dev_list[i].deviceid, prop, XA_INTEGER, 8,
                        PropModeReplace, &value, 1);
     }
   }
 #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);
 }
 
 // static
 gfx::Size WindowTreeHost::GetNativeScreenSize() {
   ::XDisplay* xdisplay = gfx::GetXDisplay();
   return gfx::Size(DisplayWidth(xdisplay, 0), DisplayHeight(xdisplay, 0));
 }
 
 namespace test {
 
 void SetUseOverrideRedirectWindowByDefault(bool override_redirect) {
   default_override_redirect = override_redirect;
 }
 
 }  // namespace test
 }  // namespace aura