Integrating with new rendering for CRD iOS.

remoting/client/ios/display/gl_display_handler.mm

 // 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.
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
 #endif
 
+#include <array>
+
 #import "remoting/client/ios/display/gl_display_handler.h"
 
+#import "base/mac/bind_objc_block.h"
 #import "remoting/client/display/sys_opengl.h"
 #import "remoting/client/ios/display/gl_demo_screen.h"
 
 #include "base/bind.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/memory/weak_ptr.h"
-#include "remoting/client/chromoting_client.h"
 #include "remoting/client/chromoting_client_runtime.h"
 #include "remoting/client/cursor_shape_stub_proxy.h"
 #include "remoting/client/display/gl_canvas.h"
 #include "remoting/client/display/gl_renderer.h"
 #include "remoting/client/display/gl_renderer_delegate.h"
 #include "remoting/client/dual_buffer_frame_consumer.h"
 #include "remoting/client/software_video_renderer.h"
 
 namespace remoting {
 namespace GlDisplayHandler {
 
 // The core that lives on the display thread.
 class Core : public protocol::CursorShapeStub, public GlRendererDelegate {
  public:
   Core();
   ~Core() override;
 
+  void Initialize();
+
   // CursorShapeStub interface.
   void SetCursorShape(const protocol::CursorShapeInfo& cursor_shape) override;
 
   // GlRendererDelegate interface.
   bool CanRenderFrame() override;
   void OnFrameRendered() override;
   void OnSizeChanged(int width, int height) override;
 
-  void Created();
+  void OnFrameReceived(std::unique_ptr<webrtc::DesktopFrame> frame,
+                       const base::Closure& done);
   void Stop();
   void SurfaceChanged(int width, int height);
   std::unique_ptr<protocol::FrameConsumer> GrabFrameConsumer();
+  EAGLContext* GetEAGLContext();
   base::WeakPtr<Core> GetWeakPtr();
 
  private:
+  remoting::ChromotingClientRuntime* runtime_;
+
   // Will be std::move'd when GrabFrameConsumer() is called.
-  remoting::ChromotingClientRuntime* runtime_;
   std::unique_ptr<DualBufferFrameConsumer> owned_frame_consumer_;
+  base::WeakPtr<DualBufferFrameConsumer> frame_consumer_;
 
-  base::WeakPtr<DualBufferFrameConsumer> frame_consumer_;
   EAGLContext* eagl_context_;
-  GlRenderer renderer_;
-  GlDemoScreen demo_screen_;
+  std::unique_ptr<GlRenderer> renderer_;
+  //  GlDemoScreen *demo_screen_;

[Jamie, 2017/04/21 17:24:18]

We don't tend to comment-out unused code. Is there a reason not to simply delete
it?

[nicholss, 2017/04/21 17:35:15]

I was not sure if I was done with it yet was all, I am still integrating with
the rest of the system.

[Jamie, 2017/04/21 17:43:01]

Acknowledged.

 
   // Used on display thread.
   base::WeakPtr<Core> weak_ptr_;
   base::WeakPtrFactory<Core> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(Core);
 };
 
 Core::Core() : weak_factory_(this) {
   runtime_ = ChromotingClientRuntime::GetInstance();
+  DCHECK(!runtime_->display_task_runner()->BelongsToCurrentThread());
+
   weak_ptr_ = weak_factory_.GetWeakPtr();
-  renderer_.SetDelegate(weak_ptr_);
+
+  runtime_->display_task_runner()->PostTask(
+      FROM_HERE, base::Bind(&Core::Initialize, base::Unretained(this)));

[Jamie, 2017/04/21 17:24:18]

Optional: In general, you should do as little work as possible in the ctor
(among other things, it tends to lead to more testable code, although since this
is a private class you probably don't want to test it in isolation anyway).
Consider refactoring this so that Init() can be called on any thread and bounces
itself to the correct thread if necessary, and making it mandatory for the
caller to call Init() after construction.

[nicholss, 2017/04/21 17:35:15]

Acknowledged. For this case I think having the call to init makes sense because
it would be in the ctor of the display handler otherwise. Nothing else will use
this core class, and i would not tend to do something like this in a normal
public class, so I take the point. 

+
+  // Do not bind GlRenderer::OnFrameReceived. |renderer_| is not ready yet.
   owned_frame_consumer_.reset(new remoting::DualBufferFrameConsumer(
-      base::Bind(&remoting::GlRenderer::OnFrameReceived,
-                 renderer_.GetWeakPtr()),
+      base::Bind(&Core::OnFrameReceived, weak_ptr_),
       runtime_->display_task_runner(),
-      remoting::protocol::FrameConsumer::PixelFormat::FORMAT_RGBA));
+      protocol::FrameConsumer::PixelFormat::FORMAT_RGBA));
   frame_consumer_ = owned_frame_consumer_->GetWeakPtr();
-  renderer_.AddDrawable(demo_screen_.GetWeakPtr());
 }
 
 Core::~Core() {}
 
+void Core::Initialize() {
+  DCHECK(runtime_->display_task_runner()->BelongsToCurrentThread());
+
+  eagl_context_ = [EAGLContext currentContext];
+  if (!eagl_context_) {
+    // TODO(nicholss): For prod code, make sure to check for ES3 support and
+    // fall back to ES2 if needed.

[Jamie, 2017/04/21 17:24:18]

Does this need a tracking bug? What devices only support ES2?

[nicholss, 2017/04/21 17:35:15]

iphone < 5 I think and some iPads. I think a tracking bug will be lost, I have a
lot of TODOs in the code at the moment, I plan to sweep them at some point and I
am not sure I need the overhead of creating a bunch of bugs.

[Jamie, 2017/04/21 17:43:01]

SGTM. Please create a bug assigned to yourself to make sure we don't forget to
do that in case you win the lottery :)

+    eagl_context_ =
+        [[EAGLContext alloc] initWithAPI:kEAGLRenderingAPIOpenGLES3];
+    [EAGLContext setCurrentContext:eagl_context_];
+  }
+
+  renderer_ = remoting::GlRenderer::CreateGlRendererWithDesktop();
+
+  //  renderer_.RequestCanvasSize();
+
+  renderer_->OnSurfaceCreated(
+      base::MakeUnique<GlCanvas>(static_cast<int>([eagl_context_ API])));
+
+  SurfaceChanged(1024, 640);  // TODO(nicholss): Where does this data comefrom?

[Jamie, 2017/04/21 17:24:18]

s/comefrom/come from/

+
+  // TODO(nicholss): This are wrong values but it lets us get something on the
+  // screen.
+  std::array<float, 9> matrix;
+  matrix[0] = 1;
+  matrix[4] = 1;
+  matrix[8] = 1;
+
+  renderer_->OnPixelTransformationChanged(matrix);
+
+  // demo_screen_ = new GlDemoScreen();
+  // renderer_->AddDrawable(demo_screen_->GetWeakPtr());
+  renderer_->SetDelegate(weak_ptr_);
+}
+
 void Core::SetCursorShape(const protocol::CursorShapeInfo& cursor_shape) {
   DCHECK(runtime_->display_task_runner()->BelongsToCurrentThread());
-  renderer_.OnCursorShapeChanged(cursor_shape);
+  renderer_->OnCursorShapeChanged(cursor_shape);
 }
 
 bool Core::CanRenderFrame() {
   DCHECK(runtime_->display_task_runner()->BelongsToCurrentThread());
   return eagl_context_ != NULL;
 }
 
 std::unique_ptr<protocol::FrameConsumer> Core::GrabFrameConsumer() {
   DCHECK(owned_frame_consumer_) << "The frame consumer is already grabbed.";
   return std::move(owned_frame_consumer_);
 }
 
+void Core::OnFrameReceived(std::unique_ptr<webrtc::DesktopFrame> frame,
+                           const base::Closure& done) {
+  DCHECK(runtime_->display_task_runner()->BelongsToCurrentThread());
+  renderer_->OnFrameReceived(std::move(frame), done);
+}
+
 void Core::OnFrameRendered() {
   // Nothing to do.
 }
 
 void Core::OnSizeChanged(int width, int height) {
   // Nothing to do.
 }
 
-void Core::Created() {
-  DCHECK(runtime_->display_task_runner()->BelongsToCurrentThread());
-  DCHECK(!eagl_context_);
-
-  eagl_context_ = [EAGLContext currentContext];
-  if (!eagl_context_) {
-    eagl_context_ =
-        [[EAGLContext alloc] initWithAPI:kEAGLRenderingAPIOpenGLES3];
-    [EAGLContext setCurrentContext:eagl_context_];
-  }
-  renderer_.RequestCanvasSize();
-
-  renderer_.OnSurfaceCreated(base::MakeUnique<GlCanvas>(
-      static_cast<int>([eagl_context_ API])));
-}
-
 void Core::Stop() {
   DCHECK(runtime_->display_task_runner()->BelongsToCurrentThread());
 
   eagl_context_ = nil;
-  // demo_screen_
-  // renderer_ = nil;
+  // demo_screen_ = nil;
 }
 
 void Core::SurfaceChanged(int width, int height) {
   DCHECK(runtime_->display_task_runner()->BelongsToCurrentThread());
-  renderer_.OnSurfaceChanged(width, height);
+  renderer_->OnSurfaceChanged(width, height);
+}
+
+EAGLContext* Core::GetEAGLContext() {
+  return eagl_context_;
 }
 
 base::WeakPtr<remoting::GlDisplayHandler::Core> Core::GetWeakPtr() {
   return weak_ptr_;
 }
 
 }  // namespace GlDisplayHandler
 }  // namespace remoting
 
-@interface GlDisplayHandler ()
-@property(nonatomic) remoting::GlDisplayHandler::Core* core_;
-@property(nonatomic) remoting::ChromotingClientRuntime* runtime_;
+@interface GlDisplayHandler () {
+  remoting::GlDisplayHandler::Core* _core;
+  remoting::ChromotingClientRuntime* _runtime;
+}
 @end
 
 @implementation GlDisplayHandler
 
-@synthesize core_ = _core_;
-@synthesize runtime_ = _runtime_;
-
-- (id)initWithRuntime:(remoting::ChromotingClientRuntime*)runtime {
-  self.runtime_ = runtime;
+- (id)init {
+  self = [super init];
+  if (self) {
+    _runtime = remoting::ChromotingClientRuntime::GetInstance();
+    _core = new remoting::GlDisplayHandler::Core();
+  }
   return self;
 }
 
-- (void)created {
-  _core_ = new remoting::GlDisplayHandler::Core();
-
-  self.runtime_->display_task_runner()->PostTask(
-      FROM_HERE, base::Bind(&remoting::GlDisplayHandler::Core::Created,
-                            self.core_->GetWeakPtr()));
-}
+#pragma mark - Public
 
 - (void)stop {
-  self.runtime_->display_task_runner()->PostTask(
-      FROM_HERE, base::Bind(&remoting::GlDisplayHandler::Core::Stop,
-                            self.core_->GetWeakPtr()));
+  _runtime->display_task_runner()->PostTask(
+      FROM_HERE,
+      base::Bind(&remoting::GlDisplayHandler::Core::Stop, _core->GetWeakPtr()));
 }
 
 - (std::unique_ptr<remoting::protocol::VideoRenderer>)CreateVideoRenderer {
   return base::MakeUnique<remoting::SoftwareVideoRenderer>(
-      _core_->GrabFrameConsumer());
+      _core->GrabFrameConsumer());
 }
 
 - (std::unique_ptr<remoting::protocol::CursorShapeStub>)CreateCursorShapeStub {
   return base::MakeUnique<remoting::CursorShapeStubProxy>(
-      _core_->GetWeakPtr(), self.runtime_->display_task_runner());
+      _core->GetWeakPtr(), _runtime->display_task_runner());
+}
+
+- (EAGLContext*)GetEAGLContext {
+  return _core->GetEAGLContext();
 }
 
 // In general, avoid expensive work in this function to maximize frame rate.
 - (void)glkView:(GLKView*)view drawInRect:(CGRect)rect {
-  if (_core_) {
-    _core_->SurfaceChanged(rect.size.width, rect.size.height);
+  if (_core) {
+    _runtime->display_task_runner()->PostTask(
+        FROM_HERE,
+        base::Bind(&remoting::GlDisplayHandler::Core::SurfaceChanged,
+                   _core->GetWeakPtr(), rect.size.width, rect.size.height));
+    // should be RequestRender

[Jamie, 2017/04/21 17:24:18]

Should this be a TODO?

   }
 }
 
 @end