[Remoting Client] DesktopViewport Implementation

remoting/client/desktop_viewport.h

Index: remoting/client/desktop_viewport.h
diff --git a/remoting/client/desktop_viewport.h b/remoting/client/desktop_viewport.h
new file mode 100644
index 0000000000000000000000000000000000000000..dda426fe2b70635ccfc83ae72021bbd44be14fbb
--- /dev/null
+++ b/remoting/client/desktop_viewport.h
@@ -0,0 +1,94 @@
+// Copyright 2017 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.
+
+#ifndef REMOTING_CLIENT_DESKTOP_VIEWPORT_H_
+#define REMOTING_CLIENT_DESKTOP_VIEWPORT_H_
+
+#include <array>
+
+#include "base/callback.h"
+#include "remoting/client/simple_matrix.h"
+

[joedow, 2017/04/28 21:29:53]

You should include 'remoting/client/viewport_geometry.h' as well since you use
Point and Bound and they are defined there.

[Yuwei, 2017/04/28 23:53:40]

Obsolete. I broke down viewport_geometry and moved structs inside
DesktopViewport and SimpleMatrix.

+namespace remoting {
+
+// The viewport is a sliding window on the desktop image. This class defines
+// the viewport's position and size, provides methods to manipulate it, and
+// outputs a desktop space -> surface space transformation matrix for drawing
+// the desktop onto the client's view surface on the screen.

[joedow, 2017/04/28 21:29:53]

This is a good comment, but I think it would be good to add a bit more about
what 'desktop' and 'surface' are as a concept and how they interact.  This is
the main concept for a maintainer to wrap their head around and I think it could
be described a bit more to clarify.

[Yuwei, 2017/04/28 23:53:40]

Done.

+//
+// You may either manipulate the desktop on the surface coordinate or manipulate
+// the viewport on the desktop coordinate, depending on how you choose the
+// reference frame.
+class DesktopViewport {
+ public:
+  using TransformationCallback =
+      base::Callback<void(const std::array<float, 9>&)>;
+
+  DesktopViewport();
+  ~DesktopViewport();
+
+  // Sets the desktop size.

[joedow, 2017/04/28 21:29:53]

nit: You could just refer to the actual member -> 'Sets |desktop_size_|.'  The
same goes for a few of the other comments in the CL.

[Yuwei, 2017/04/28 23:53:40]

Done.

+  void SetDesktopSize(int desktop_width, int desktop_height);
+
+  // Sets the surface size.
+  void SetSurfaceSize(int surface_width, int surface_height);
+
+  // Resizes the desktop such that the image is displayed without borders in
+  // minimum possible zoom-level. This will be called once both the desktop
+  // and the surface size are set.

[joedow, 2017/04/28 21:29:53]

"This will be called once both the desktop and the surface size are set."

Can you clarify this comment?  It is a public method so it could be called
multiple times.  Or is the comment telling the consumer of the class to call it
once both are set?

[Yuwei, 2017/04/28 23:53:40]

I just moved it to the private block. SetDesktopSize() and SetSurfaceSize() will
call it when necessary so for now there is no need to allow user to call this
function.

+  void ResizeToFit();
+
+  // Translates the desktop on the surface's reference frame by <dx, dy>.
+  void MoveDesktop(float dx, float dy);
+
+  // Scales the desktop on the surface's reference frame at pivot point (px, py)
+  // by |scale|.
+  void ScaleDesktop(float px, float py, float scale);
+
+  // Sets the viewport center to (x, y) on the desktop's coordinate.
+  void SetViewportCenter(float x, float y);
+
+  // Registers the callback to be called once the transformation has changed.
+  // run_immediately: If true and the viewport is ready to be used, the callback
+  //     will be called immedately with the transformation matrix.
+  void RegisterOnTransformationChangedCallback(
+      const TransformationCallback& callback,
+      bool run_immediately);
+
+ private:
+  // True if desktop size and surface size are set.
+  bool IsViewportReady() const;
+
+  // Adjusts the size and position of the viewport so that the constrains always
+  // hold, then feed the matrix to |on_transformation_changed_|.
+  void UpdateViewport();
+
+  // Gets a rectangle of all possible positions where the viewport's center can
+  // locate.
+  Bounds GetViewportCenterBounds() const;
+
+  // Returns the current center of the viewport on the desktop's coordinate.
+  Point GetViewportCenter() const;
+
+  // Translates the viewport on the desktop's reference frame by <dx, dy>,
+  // without calling UpdateViewport().
+  void MoveViewportCenterWithoutUpdate(float dx, float dy);
+
+  Vector2D desktop_size_;
+  Vector2D surface_size_;
+
+  bool desktop_size_ready_ = false;
+  bool surface_size_ready_ = false;
+
+  SimpleMatrix desktop_to_surface_;

[joedow, 2017/04/28 21:29:53]

desktop_to_surface_transform_?

[Yuwei, 2017/04/28 23:53:40]

Done.

+
+  TransformationCallback on_transformation_changed_;
+
+  // DesktopViewport is neither copyable nor movable.
+  DesktopViewport(const DesktopViewport&) = delete;
+  DesktopViewport& operator=(const DesktopViewport&) = delete;
+};
+
+}  // namespace remoting
+#endif  // REMOTING_CLIENT_DESKTOP_VIEWPORT_H_