Move touch CTM from X into Chrome

ash/touch/touch_ctm_controller.cc

+// Copyright (c) 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 "ash/touch/touch_ctm_controller.h"
+
+#include "ash/display/display_controller.h"
+#include "ash/display/display_manager.h"
+#include "ash/shell.h"
+#include "ui/aura/window_tree_host.h"
+#include "ui/events/touch_ctm.h"
+
+#if defined(OS_CHROMEOS) && defined(USE_X11)

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

Since this make sense only on chromeos + x11, it's better to exclude these files
on other platforms in gyp and ifdef in Shell.

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

Done.

+#include "ui/events/x/device_data_manager.h"
+#endif  // defined(OS_CHROMEOS) && defined(USE_X11)
+
+namespace ash {
+
+namespace {
+
+internal::DisplayManager* GetDisplayManager() {
+  return Shell::GetInstance()->display_manager();
+}
+
+#if defined(OS_CHROMEOS) && defined(USE_X11)
+// This function computes the extended mode TouchCTM for |touch_display|.
+// An example of how to calculate the extended CTM.
+// Suppose we have 2 monitors, the first one has size 1366 x 768.
+// The second one has size 2560 x 1600
+// The total size of framebuffer is 2560 x 2428
+// where 2428 = 768 + 60 (hidden gap) + 1600
+// and the sceond monitor is translated to Point (0, 828) in the

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

second

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

comments removed.

+// framebuffer.
+// X will first map input event location to [0, 2560) x [0, 2428),

[spang, 2014/03/24 18:52:31]

Is this really a half-open interval?

AFAIK aura events can be anywhere within the bounding rectangle of the window,
including on the border. So a closed interval.

Rob was telling me X is similar, and (0,0) is actually on the bounding
rectangle.

I think all of this math may need to be updated.

[ynovikov, 2014/03/24 19:02:41]

No, but I think the numbers make more sense this way.
Would you prefer the comment to say [0, 2559] x [0, 2427]?

[spang, 2014/03/24 19:05:31]

No it's [0, 2560] x [0, 2428]. That's the point.

[spang, 2014/03/26 17:30:34]

sadrul has fixed my understanding of the coordinate system. It is indeed
half-open [0, width) x [0, height).

The points on the left & top border are inside the screen, while the points on
the bottom & right borders are outside the screen.

+// Then the TouchCTM should map touch events from framebuffer space
+// to root window space.
+// For monitor1, we have
+// x_scale = (1366 - 1) / (2560 - 1)
+// x_offset = 0
+// y_scale = (768 - 1) / (2428 - 1)
+// y_offset = 0
+// For Monitor 2, we have
+// x_scale = (2560 - 1) / (2560 - 1)
+// x_offset = 0
+// y_scale = (1600 - 1) / (2428 - 1)
+// y_offset = 0

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

This used to be:
// y_offset = 828 / (2428 -1)

The original code mapped touches for the second monitor from (0,0)-(2559,2427)
rectangle into (0,828)-(2559,2427) rectangle.
Now, combined with a change in WindowTreeHostX11, you actually map the second
monitor into (0,0)-(2560,1599) rectangle instead.
So, the second monitor computation is not different from the first one.

You should simplify the comment, saying that it scales the events from X display
size to the root window / monitor size, and you don't need to distinguish
between monitor 1 and 2.
The original code also used to offset the event to the position of the root
window inside X's display, thus the handling was different, but since you match
the touch to window based on touch device id and not coordinate, we can simplify
this now.

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

Done.

+// See DisplayControllerTest.TouchCTMExtendedMode for the test.
+ui::TouchCTM GetExtendedModeTouchCTM(
+    const internal::DisplayInfo& touch_display,
+    const internal::DisplayInfo& second_display) {
+  ui::TouchCTM ctm;
+  float width = touch_display.bounds_in_native().width();
+  float height = touch_display.bounds_in_native().height();
+  //  float origin_x = touch_display.bounds_in_native().origin().x();
+  //  float origin_y = touch_display.bounds_in_native().origin().y();

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

This comment is misleading, please remove.

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

Done.

+  float framebuffer_width = std::max<int>(
+      width, second_display.bounds_in_native().width());
+  float framebuffer_height =
+      height + chromeos::OutputConfigurator::kVerticalGap +
+      second_display.bounds_in_native().height();

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

I don't think this computation is appropriate here.
It made sense when this code was in OutputConfigurator, since it was the one
that configured X's display in this fashion.
It would be better to pass X's display size to this function instead of
second_display, otherwise changes in OutputConfigurator might have unexpected
effect here.

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

Done.

+  ctm.x_scale = (width - 1) / (framebuffer_width - 1);

[spang, 2014/03/26 17:30:34]

I think this should be:

width / framebuffer_width

Right? We scale from the old width to new; subtracting 1 introduces a small
amount of error here.

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

No, as the comment states for Monitor 2, in case where width==framebuffer_width
x_scale should be 1.
Think of the transformation matrix like this: We map from
[0,framebuffer_width-1] range into [0,1] range by dividing by
(framebuffer_width-1) factor, and then we map from [0,1] range into [0,width-1]
range by multiplying by (width-1) factor.

[spang, 2014/03/28 16:49:31]

Agree completely. However note x/x == (x-1)/(x-1) == 1
It is wrong to try to do geometry with these integer bounds because it doesn't
make sense. Let's work in floats. We're trying to map from the [0,
framebuffer_width) range to the [0, width) range.

Consider a very small example. Say we're scaling a 4x4 rectangle onto a 2x2
rectangle. Then

framebuffer_width = 4
width = 2

Then the current code scale factor is (2 - 1) / (4 - 1) = 1/3. Should we really
scale by 1/3 onto a rectangle that is only 1/2 the size? I don't think so; this
is flawed geometry.

Don't worry about the integer upper bound. The integer upper bound is the left
side of the rightmost pixel, and does not correspond to the border of the
screen. For scaling, the width must be used.

In particular, we absolutely do not need to maintain the invariant

   0 <= x <= width -1

The invariant is actually:

   0 <= x < width

If we truncate the float coordinate into the integers, we will certainly get a
number in the range [0, width-1], but that's only because we've lost a lot of
precision.

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

OK, changed to use width / framebuffer_width.

The concern I had was I wanted to maintain the invariant
0 <= integer_x <= width -1

I mean, it is definitely right we want to maintain
0 <= float_x < width,

but eventually when we convert the xevent's position from float
into ui::Event's location which is in integer, we don't want the
integer_x gets accidentally rounded to |width|.

We are doing truncating in EventLocationFromNative() in events_x.cc using
static_cast<int>(xievent->event_x), so we are ok there.

+  ctm.x_offset = 0;
+  ctm.y_scale = (height -1) / (framebuffer_height - 1);

[spang, 2014/03/26 17:30:34]

Same here.

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

Nit, add space in (height -1)

+  ctm.y_offset = 0;
+  return ctm;
+}
+
+// This function computes the mirror mode TouchCTM for |touch_display|.
+// When internal monitor is applied a resolution that does not have
+// the same aspect ratio as its native resolution, there would be
+// blank regions in the letterboxing/pillarboxing mode.
+// The TouchCTM will make sure the touch events on the blank region
+// have negative coordinates and touch events within the chrome region
+// have the correct positive coordinates.
+ui::TouchCTM GetMirrorModeTouchCTM(
+    const internal::DisplayInfo& touch_display) {
+  ui::TouchCTM ctm;
+  // For external monitor, assuming no letterboxing or pillarboxing
+  // and just use the default TouchCTM.

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

Why? Double checking that aspect ratio hasn't changed is trivial.
In fact, the code already does that.

But, something that you should check is whether the monitor
"is_aspect_preserving_scaling" - if it stretches to full screen, no need for
CTM.

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

Done.

+  if (touch_display.id() != gfx::Display::InternalDisplayId())
+    return ctm;
+
+  float mirror_width = touch_display.bounds_in_native().width();
+  float mirror_height = touch_display.bounds_in_native().height();
+  float native_width = 0;
+  float native_height = 0;
+
+  internal::DisplayMode native_mode;
+  std::vector<internal::DisplayMode> modes = touch_display.display_modes();
+  for (size_t i = 0; i < modes.size(); i++) {
+    if (modes[i].native) {
+      native_width = modes[i].size.width();
+      native_height = modes[i].size.height();
+      break;
+    }
+  }
+
+  if (native_height == 0.0 || mirror_height == 0.0 ||
+      native_width == 0.0 || mirror_width == 0.0)
+    return ctm;
+
+  float native_ar = static_cast<float>(native_width) /
+      static_cast<float>(native_height);
+  float mirror_ar = static_cast<float>(mirror_width) /
+      static_cast<float>(mirror_height);
+
+  if (mirror_ar > native_ar) {  // Letterboxing
+    ctm.x_scale = 1.0;
+    ctm.x_offset = 0.0;
+    ctm.y_scale = mirror_ar / native_ar;
+    ctm.y_offset = (1.0 / mirror_ar - 1.0 / native_ar) * 0.5 * mirror_width;
+    return ctm;
+  }
+
+  if (native_ar > mirror_ar) {  // Pillarboxing
+    ctm.y_scale = 1.0;
+    ctm.y_offset = 0.0;
+    ctm.x_scale = native_ar / mirror_ar;
+    ctm.x_offset = (mirror_ar - native_ar) * 0.5 * mirror_height;
+    return ctm;
+  }
+
+  return ctm;  // Same aspect ratio - return identity
+}
+#endif  // defined(OS_CHROMEOS) && defined(USE_X11)
+
+}  // namespace
+
+TouchCTMController::TouchCTMController() {
+  Shell::GetInstance()->display_controller()->AddObserver(this);
+}
+
+TouchCTMController::~TouchCTMController() {
+  Shell::GetInstance()->display_controller()->RemoveObserver(this);
+}
+
+void TouchCTMController::UpdateTouchCTM() {
+#if defined(OS_CHROMEOS) && defined(USE_X11)
+  ui::DeviceDataManager* device_manager = ui::DeviceDataManager::GetInstance();
+  device_manager->ClearTouchCTM();
+  device_manager->ClearTouchDeviceToDisplayMap();
+
+  // Display IDs and DisplayInfo for mirror or extended mode.
+  int64 display1_id = gfx::Display::kInvalidDisplayID;
+  int64 display2_id = gfx::Display::kInvalidDisplayID;
+  internal::DisplayInfo display1;
+  internal::DisplayInfo display2;
+  // Display ID and DisplayInfo for single display mode.
+  int64 single_display_id = gfx::Display::kInvalidDisplayID;
+  internal::DisplayInfo single_display;
+
+  const std::map<int64, aura::Window*>& root_windows =
+      Shell::GetInstance()->display_controller()->root_windows();
+
+  ui::OutputState output_state =
+      Shell::GetInstance()->output_configurator()->output_state();
+  if (output_state == ui::OUTPUT_STATE_DUAL_MIRROR ||
+      output_state == ui::OUTPUT_STATE_DUAL_EXTENDED) {
+    DisplayIdPair id_pair = GetDisplayManager()->GetCurrentDisplayIdPair();
+    display1_id = id_pair.first;
+    display2_id = id_pair.second;
+    if (display1_id == gfx::Display::kInvalidDisplayID ||
+        display2_id == gfx::Display::kInvalidDisplayID)
+      return;
+    display1 = GetDisplayManager()->GetDisplayInfo(display1_id);
+    display2 = GetDisplayManager()->GetDisplayInfo(display2_id);
+  } else {
+    single_display_id = GetDisplayManager()->first_display_id();
+    if (single_display_id == gfx::Display::kInvalidDisplayID)
+      return;
+    single_display = GetDisplayManager()->GetDisplayInfo(single_display_id);
+  }
+
+  if (output_state == ui::OUTPUT_STATE_DUAL_MIRROR) {
+    std::map<int64, aura::Window*>::const_iterator it = root_windows.begin();

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

In this case, is root_windows.size() == 1?

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

reworked, ptal.

+    // In mirror mode, both displays share the same root window so
+    // both display ids are associated with the root window.
+    for (; it != root_windows.end(); it++)
+      it->second->GetHost()->UpdateDisplayID(display1_id, display2_id);
+    // Update the associated display id and TouchCTM in device manager
+    // if the display is a touch device.
+    if (display1.touch_device_id() != 0) {
+      device_manager->MapTouchDeviceToDisplay(display1.touch_device_id(),
+                                              display1.id());
+      device_manager->SetTouchCTM(display1.touch_device_id(),
+                                  GetMirrorModeTouchCTM(display1));
+    }

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

This block of code should be in DeviceDataManager, e.g.

  DeviceDataManager::UpdateTouchInfoForDisplay(const gfx::Display& display,
                                               const gfx::Transform& x) {
    if (display.touch_device_id() == 0)
      return;
    ....
  }

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

done.

+    if (display2.touch_device_id() != 0) {
+      device_manager->MapTouchDeviceToDisplay(display2.touch_device_id(),
+                                              display2.id());
+      device_manager->SetTouchCTM(display2.touch_device_id(),
+                                  GetMirrorModeTouchCTM(display2));
+    }
+    return;
+  }
+
+  if (output_state == ui::OUTPUT_STATE_DUAL_EXTENDED) {
+    std::map<int64, aura::Window*>::const_iterator it = root_windows.begin();
+    // In extended mode, each display has one associated root window.
+    for (; it != root_windows.end(); it++) {
+      if (it->first == display1_id) {
+        it->second->GetHost()->UpdateDisplayID(display1_id,
+                                               gfx::Display::kInvalidDisplayID);
+      } else if (it->first == display2_id) {
+        it->second->GetHost()->UpdateDisplayID(display2_id,
+                                               gfx::Display::kInvalidDisplayID);
+      }
+    }
+    // Update the associated display id and TouchCTM in device manager
+    // if the display is a touch device.
+    if (display1.touch_device_id() != 0) {
+      device_manager->MapTouchDeviceToDisplay(display1.touch_device_id(),
+                                              display1.id());
+      device_manager->SetTouchCTM(display1.touch_device_id(),
+                                  GetExtendedModeTouchCTM(display1, display2));
+    }
+    if (display2.touch_device_id() != 0) {
+      device_manager->MapTouchDeviceToDisplay(display2.touch_device_id(),
+                                              display2.id());
+      device_manager->SetTouchCTM(display2.touch_device_id(),
+                                  GetExtendedModeTouchCTM(display2, display1));
+    }
+    return;
+  }
+
+  // Single display mode. The root window has one associated display id.
+  std::map<int64, aura::Window*>::const_iterator it = root_windows.begin();
+  for (; it != root_windows.end(); it++) {
+    if (it->first == single_display.id()) {
+      it->second->GetHost()->UpdateDisplayID(single_display.id(),
+                                             gfx::Display::kInvalidDisplayID);
+    }
+  }
+  // If the display is a touch display, we add a deafult TouchCTM to the

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

default

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

Done.

+  // according root window.
+  if (single_display.touch_device_id() != 0) {
+    device_manager->MapTouchDeviceToDisplay(single_display.touch_device_id(),
+                                            single_display.id());
+    device_manager->SetTouchCTM(single_display.touch_device_id(),
+                                ui::TouchCTM());
+  }
+#endif
+}
+
+void TouchCTMController::OnDisplayConfigurationChanged() {
+  UpdateTouchCTM();
+}
+
+}  // namespace ash