Adding the iOS app and integration example with GlRenderer.

remoting/client/ios/display/gl_demo_screen.mm

Index: remoting/client/ios/display/gl_demo_screen.mm
diff --git a/remoting/client/ios/display/gl_demo_screen.mm b/remoting/client/ios/display/gl_demo_screen.mm
new file mode 100644
index 0000000000000000000000000000000000000000..6f61668005ea63fd30a374c8dda20ec7fbb34359
--- /dev/null
+++ b/remoting/client/ios/display/gl_demo_screen.mm
@@ -0,0 +1,117 @@
+// 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.
+
+#include "remoting/client/ios/display/gl_demo_screen.h"
+
+#import <Foundation/Foundation.h>

[Yuwei, 2017/01/26 20:18:25]

Is core foundation used in this file?

[nicholss, 2017/01/26 23:46:44]

Done.

+
+#include "base/logging.h"
+#include "remoting/client/display/canvas.h"
+#include "remoting/client/display/gl_math.h"
+
+namespace remoting {
+
+namespace {
+
+const GLfloat square[] = {-1.0, -1.0, 1.0, -1.0, -1.0, 1.0, 1.0, 1.0};
+
+const GLchar* fragmentShaderSource =
+    "precision mediump float;"
+    "void main() {"
+    "  gl_FragColor = vec4(1.0, 1.0, 0.0, 1.0);"
+    "} ";
+
+const GLchar* vertexShaderSource =
+    "precision mediump float;"
+    "attribute vec4 a_position;"
+    "void main() {"
+    "  gl_Position = a_position;"
+    "}";
+
+const GLchar* a_position = "a_position";
+
+}  // namespace
+
+// This is a demo screen that can be added to the renderer to test the drawable
+// integration. This will draw an expanding checkerboard pattern to the screen.
+GlDemoScreen::GlDemoScreen() : weak_factory_(this)  {}
+
+GlDemoScreen::~GlDemoScreen() {}
+
+void GlDemoScreen::SetCanvas(base::WeakPtr<Canvas> canvas) {
+  canvas_ = canvas;

[Yuwei, 2017/01/26 20:18:25]

On Android, SetCanvas(null) will be called once the surface is destroyed. Please
clean up in that case.

[nicholss, 2017/01/26 23:46:44]

WeakPtr deals with this case.

[Yuwei, 2017/01/26 23:55:18]

When will the shaders and program below get destroyed?

[nicholss, 2017/01/27 00:51:05]

Not bothering with cleanup on a demo.

[Yuwei, 2017/01/27 01:11:48]

I agree that a demo may not need to cleanup, but I guess it's still better to
say that in a comment?

+
+  // Create and compile vertex shader.
+  GLuint vertexShader = glCreateShader(GL_VERTEX_SHADER);
+  glShaderSource(vertexShader, 1, &vertexShaderSource, NULL);
+  glCompileShader(vertexShader);
+
+  // Create and compile fragment shader.
+  GLuint fragmentShader = glCreateShader(GL_FRAGMENT_SHADER);
+  glShaderSource(fragmentShader, 1, &fragmentShaderSource, NULL);
+  glCompileShader(fragmentShader);
+
+  // Create and link program.
+  program_ = glCreateProgram();
+  glAttachShader(program_, vertexShader);
+  glAttachShader(program_, fragmentShader);
+  glLinkProgram(program_);
+}
+
+int GlDemoScreen::GetZIndex() {
+  return Drawable::DESKTOP + 1;

[joedow, 2017/01/26 21:30:23]

nit: This would be better cleaner if it were a constant defined at the top of
this file than well-known value + magic number inline (i.e. DEMO_DESKTOP =
DESKTOP + 1).  

+}
+
+bool GlDemoScreen::Draw() {
+  if (!canvas_) {
+    return false;
+  }
+
+  // TODO(nicholss): width and height should be dynamic based on the canvas.
+  int width = 640;
+  int height = 1024;
+  square_size_++;
+  if (square_size_ > 300) {
+    square_size_ = 1;
+  }
+
+  // Set the viewport.
+  glViewport(0, 0, width, height);
+
+  // Clear.
+  glClearColor(0, 1, 0, 1);
+  glClear(GL_COLOR_BUFFER_BIT);
+
+  // Use program.
+  glUseProgram(program_);
+
+  int skip = 0;
+  for (int i = 0; i < width; i += square_size_) {
+    if (skip == square_size_) {
+      skip = 0;
+    } else {
+      skip = square_size_;
+    }
+    for (int j = skip; j < height; j += square_size_ * 2) {
+      glViewport(i, j, square_size_, square_size_);
+
+      // Send geometry to vertex shader.
+      GLuint aPosition = glGetAttribLocation(program_, a_position);

[Yuwei, 2017/01/26 20:18:25]

|a_position| just stores the name of the variable and |aPosition| is the actual
position. The name is kind of confusing to me... Maybe changing a_position to
position_var_name and changing aPosition to just position? "a" is just a prefix
for attribute in GLSL.

[nicholss, 2017/01/26 23:46:44]

this is just a demo.

+
+      glVertexAttribPointer(aPosition, 2, GL_FLOAT, GL_FALSE, 0, square);
+      glEnableVertexAttribArray(aPosition);
+
+      // Draw.
+      glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
+    }
+  }
+  return true;
+}
+
+base::WeakPtr<Drawable> GlDemoScreen::GetWeakPtr() {
+  DCHECK(thread_checker_.CalledOnValidThread());

[Yuwei, 2017/01/26 20:18:25]

Maybe rename thread_checker_ to get_weak_ptr_thread_checker_? The class is
mainly used on the display thread but GetWeakPtr() is called on the main
thread...

[joedow, 2017/01/26 21:30:23]

thread_checker_ is pretty standard in our codebase, it implies that the method
which references it is to be called on the same thread the c'tor and d'tor are
called on (or the thread the checker is re-attached to).  I'd prefer that the
name remain the same.

[Yuwei, 2017/01/26 21:33:53]

It implies the thread being checked is the thread that the class is mainly used
on, which is not true in this case and it just made it think that GetWeakPtr()
is called on the display thread...

[joedow, 2017/01/26 21:40:51]

ThreadChecker binds to the thread it is created on unless Detach...() is called.
 In this instance, thread_checker_ is validating that the WeakPtr is created on
the same thread (and weak_ptr_ is bound to the same thread) that the instance it
refers to was created on and will be destroyed on.  This method may not be
called on a display thread in some cases, say for testing. 

+  return weak_factory_.GetWeakPtr();
+}
+
+}  // namespace remoting