Remote assistance on Chrome OS Part IX - Input injection on Ozone

remoting/host/chromeos/display_transform_util.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 "remoting/host/chromeos/display_transform_util.h"
+
+#include "ash/shell.h"
+#include "ui/aura/window_tree_host.h"
+#include "ui/compositor/dip_util.h"
+
+namespace remoting {
+
+gfx::Transform GetRootTransform(bool inverse) {

[Wez, 2014/12/04 01:28:25]

This needs to be static, or in an anonymous namespace.

[kelvinp, 2014/12/05 03:17:28]

No longer applied.

+  ui::Layer* layer = ash::Shell::GetPrimaryRootWindow()->layer();
+  float scale = ui::GetDeviceScaleFactor(layer);

[Wez, 2014/12/04 01:28:25]

nit: Blank line after this, before the comment, to break things up for the
reader

[kelvinp, 2014/12/05 03:17:27]

Done.

+  // When the display is rotated by 90 deg, the display rotation transform
+  // involves a rotation around the origin and then a translation of |h| pixels
+  // along the y-axis, where |h| is the height of the display in device
+  // independent pixels.  Since event.x() and event.y() are expressed in device
+  // pixels, we need to divide it by the scale factor before applying the
+  // rotation transform.

[Wez, 2014/12/04 01:28:25]

This paragraph isn't very helpful; it's not clear how the second sentence
relates to the first?

[kelvinp, 2014/12/05 03:17:27]

Done.

+  gfx::Transform rotation = layer->transform();
+
+  if (inverse) {
+    gfx::Transform inverse;
+    ignore_result(rotation.GetInverse(&inverse));
+    rotation = inverse;
+  }
+
+  gfx::Transform transform;
+  transform.Scale(scale, scale); // Converts back to device pixels.
+  transform *= rotation; // Display rotation.
+  transform.Scale(1/scale, 1/scale); // Converts to device independent pixels.

[Wez, 2014/12/04 01:28:25]

As I commented previously, you need to explain *why* you are doing this
scale/unscale hack here.

e.g. Is the issue that |layer->transform()| returns a transform consisting of a
rotation and translation, but in device pixels, so we need to switch
DIPS->device pixels, apply it, then switch device->DIPS?

If so, why can't we just do:

gfx::Transform rotation = layer->transform();
... inverse logic ...

// Convert the rotation from device pixels to DIPs.
rotation.Scale(1/scale, 1/scale);
return rotation;

i.e. Do we need the separate |transform| variable?

Note that DIPs are Density Independent Pixels, not Device Independent. :)

[kelvinp, 2014/12/05 03:17:27]

Done.

+  return transform;
+}
+
+gfx::PointF ConvertPointToNativeScreen(const gfx::PointF& location) {
+  gfx::Point3F screen_location(location);
+  gfx::Transform transform = GetRootTransform(false);
+  transform.TransformPoint(&screen_location);
+  return screen_location.AsPointF();

[Wez, 2014/12/04 01:28:25]

It seems expensive to keep fetching the root layer transform and scaling it on
each and every point - is there a way we can cache the DIPs-scaled rotation
transform to re-use?

[kelvinp, 2014/12/05 03:17:28]

Done

+}
+
+gfx::PointF ConvertPointFromNativeScreen(const gfx::PointF& location) {
+  gfx::Point3F host_location(location);
+  gfx::Transform transform = GetRootTransform(true);
+  transform.TransformPoint(&host_location);
+  return host_location.AsPointF();
+}
+
+}  // namespace remoting