[Remoting] OpenGL Rendering Layer

remoting/client/opengl/gl_canvas.h

+// Copyright 2016 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_OPENGL_GL_CANVAS_H_
+#define REMOTING_CLIENT_OPENGL_GL_CANVAS_H_
+
+#include "base/macros.h"
+#include "base/threading/thread_checker.h"
+#include "remoting/client/opengl_wrapper.h"
+
+namespace remoting {
+
+// This class holds zoom and pan configurations of the canvas and is used to
+// draw textures on the canvas.
+// Must be constructed after the OpenGL surface is created and destructed before
+// the surface is destroyed.
+// For more details, see the design doc: https://goo.gl/v549OW

[Sergey Ulanov, 2016/06/27 23:15:29]

Please don't refer to design docs from the code. Design docs are rarely up to
date, so such links will never be useful.

[Yuwei, 2016/06/28 22:51:11]

Done. Removed.

+class GlCanvas {
+ public:
+  GlCanvas();
+  ~GlCanvas();
+
+  // Sets the normalized transformation matrix.
+  // 3 by 3 transformation matrix, [ m0, m1, m2, m3, m4, m5, m6, m7, m8 ].
+  // The matrix will be multiplied with the positions (with projective space,
+  // (x, y, 1)) to draw the textures with the right zoom and pan configuration.
+  //
+  // | m0, m1, m2, |   | x |
+  // | m3, m4, m5, | * | y |
+  // | m6, m7, m8  |   | 1 |
+  //
+  // By default the transformation is a zero matrix.

[Sergey Ulanov, 2016/06/27 23:15:29]

Why is it 0-matrix? wouldn't it make more sense to set it to identity-matrix
instead?

[Yuwei, 2016/06/28 01:31:20]

I don't have strong opinion... I made it 0-matrix so that nothing will be drawn
if the cursor shape comes earlier than the desktop frame (which determines the
dimension of the canvas), otherwise a huge cursor will be shown and stretched to
the whole screen.

[Sergey Ulanov, 2016/06/30 00:34:52]

I guess even if it's 0-matrix it doesn't mean nothing is drawn on the screen.
All coordinates will collapse to (0,0), so you still get one pixel drawn there,
no?

Maybe it's better to require that this function is always called before
DrawTexture() and add a DCHECK() for that.

[Yuwei, 2016/07/07 22:07:45]

Just made it draw nothing if the transformation matrix is not set.

DCHECK may be too strict for the use case. There is no guarantee that a
transformation will be set before the desktop frame is received.

+  void SetNormalizedTransformation(const float* matrix);
+
+  // Draws the texture on the canvas.
+  // vertex_buffer: reference to the 2x4x2 float vertex buffer.
+  //                [ four (x, y) normalized vertex positions on the canvas,
+  //                  four (x, y) vertex positions to define the visible area ]
+  void DrawTexture(int texture_id, GLuint texture_handle, GLuint vertex_buffer);
+
+ private:
+  // Handles.
+  GLuint vertex_shader_;
+  GLuint fragment_shader_;
+  GLuint program_;
+
+  // Indices.
+  GLuint transform_loc_;

[Sergey Ulanov, 2016/06/27 23:15:29]

it's not clear from the comment or name what this is. Call them *_location_ and
explain in the comments that they are locations of the corresponding shader
attributes.

[Yuwei, 2016/06/28 22:51:11]

Done.

+  GLuint texture_loc_;
+  GLuint position_loc_;
+  GLuint tex_cord_loc_;
+
+  base::ThreadChecker thread_checker_;
+
+  DISALLOW_COPY_AND_ASSIGN(GlCanvas);
+};
+
+}  // namespace remoting
+
+#endif  // REMOTING_CLIENT_OPENGL_GL_CANVAS_H_