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

remoting/host/chromeos/display_rotation_filter_ozone.cc

Index: remoting/host/chromeos/display_rotation_filter_ozone.cc
diff --git a/remoting/host/chromeos/display_rotation_filter_ozone.cc b/remoting/host/chromeos/display_rotation_filter_ozone.cc
new file mode 100644
index 0000000000000000000000000000000000000000..45ab5551396c6dd1cfeb0c5c0f49b3fcfccd4a14
--- /dev/null
+++ b/remoting/host/chromeos/display_rotation_filter_ozone.cc
@@ -0,0 +1,80 @@
+// 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/display_rotation_filter.h"

[Wez, 2014/11/26 02:28:34]

nit: We typically leave the platform-specific implementations of cross-platform
interfaces alongside the cross-platform parts; I'm fine with us moving away from
that model, but we should make a deliberate decision to do so, or not.

[kelvinp, 2014/12/02 00:12:26]

Done.

+
+#include "ash/shell.h"
+#include "base/memory/scoped_ptr.h"
+#include "remoting/proto/internal.pb.h"
+#include "ui/aura/window_tree_host.h"
+#include "ui/compositor/dip_util.h"
+
+namespace {
+
+using remoting::protocol::MouseEvent;
+using remoting::protocol::InputFilter;
+using remoting::protocol::InputStub;
+
+// When the host display is rotated (e.g from 1280 x 850 -> 850 x 1280),
+// the client sends mouse locations in the rotated co-ordinates system
+// (850 x 1280).  However, Ozone still expects all ui::Event locations to be
+// encoded in the original co-ordinates system (1280 x 850) prior to the
+// rotation.  This class serves to map the rotated mouse co-ordinates back to
+// the original ones in Ozone.

[Wez, 2014/11/26 02:28:34]

This is confusingly worded; IIUC what you're saying is that if the physical
display is 1280x850, but its rotated 90deg then it is effectively an 850x1200
display and that's the coordinate system that Chromoting will send events for,
while Ozone expects raw events to match the native resolution?

+class DisplayRotationFilterOzone : public InputFilter {
+ public:
+  explicit DisplayRotationFilterOzone(InputStub* input_stub);
+  void InjectMouseEvent(const MouseEvent& event) override;
+
+ private:
+  aura::Window* root_window_;
+
+  DISALLOW_COPY_AND_ASSIGN(DisplayRotationFilterOzone);
+};
+
+DisplayRotationFilterOzone::DisplayRotationFilterOzone(InputStub* input_stub)
+    : InputFilter(input_stub) {
+  if (ash::Shell::HasInstance()) {
+    root_window_ = ash::Shell::GetPrimaryRootWindow();
+    DCHECK(root_window_);

[Wez, 2014/11/26 02:28:33]

Why DCHECK here, if the rest of the code always has to cope w/ there being no
|root_window_|?

What is it that ash::Shell::HasInstance() does that is different from just
checking for a non-null result from GetPrimaryRootWindow?

[kelvinp, 2014/12/02 00:12:26]

This is mainly to allow the test not to crash in Unit tests.

+  }
+}
+
+void DisplayRotationFilterOzone::InjectMouseEvent(const MouseEvent& event) {
+  if (!event.has_x() || !event.has_y() || !root_window_) {
+    InputFilter::InjectMouseEvent(event);
+    return;
+  }
+
+  float scale = ui::GetDeviceScaleFactor(root_window_->layer());
+
+  gfx::Transform transform;
+  // event.x() and event.y() are expressed in device pixels whereas
+  // root_window_->layer()->transform() assumes device independence pixels.
+  // Since display rotation involves a translation of the origin, it must be
+  // performed in the correct units.
+  transform.Scale(scale, scale);
+  transform *= root_window_->layer()->transform();
+  transform.Scale(1 / scale, 1 / scale);

[Wez, 2014/11/26 02:28:33]

You're scaling by the device scale factor, applying a transform, then scaling
back - the comment doesn't clearly explain why this is required (i.e. that
rotation moves the origin off of (0,0)?)

[kelvinp, 2014/12/02 00:12:26]

Done.

+
+  gfx::Point3F rotated_location(event.x(), event.y(), 0);
+  transform.TransformPoint(&rotated_location);
+
+  MouseEvent transformed_event(event);
+  transformed_event.set_x(rotated_location.x());
+  transformed_event.set_y(rotated_location.y());
+
+  InputFilter::InjectMouseEvent(transformed_event);
+}
+
+}  // namespace
+
+namespace remoting {
+
+// static
+scoped_ptr<InputFilter> CreateDisplayRotationFilter(InputStub* input_stub) {
+  return make_scoped_ptr(new DisplayRotationFilterOzone(input_stub));
+}
+
+}  // namespace remoting