[Remoting iOS] Fix screen tearing and screen dimensions

This CL:
1. Fixes the screen tearing issue by getting rid of the GLKViewController,
   which runs its rendering loops and swaps buffers in unexpected moment. The
   GL view will now do on-demand buffer swapping, similar to how the Android
   client works.
2. Passes GL view dimensions to the renderer to fix the screen aspect issues.

BUG=716699

Review-Url: https://codereview.chromium.org/2848143002
Cr-Commit-Position: refs/heads/master@{#468418}

remoting/client/ios/app/host_view_controller.h

@@ -5,11 +5,14 @@
 #ifndef REMOTING_CLIENT_IOS_APP_HOST_VIEW_CONTROLLER_H_
 #define REMOTING_CLIENT_IOS_APP_HOST_VIEW_CONTROLLER_H_
 
-#import <GLKit/GLKit.h>
+#import <UIKit/UIKit.h>
 
 @class RemotingClient;
 
-@interface HostViewController : GLKViewController
+// We don't inherit it from GLKViewController since it uses its rendering loop,
+// which will swap buffers when the GLRenderer is writing and causes screen
+// tearing issues. Instead we use GlDisplayHandler to handle the rendering loop.
+@interface HostViewController : UIViewController
 
 - (id)initWithClient:(RemotingClient*)client;
 

remoting/client/ios/app/host_view_controller.mm

@@ -8,6 +8,8 @@
 
 #import "remoting/client/ios/app/host_view_controller.h"
 
+#import <GLKit/GLKit.h>
+
 #import "ios/third_party/material_components_ios/src/components/Buttons/src/MaterialButtons.h"
 #import "remoting/client/ios/session/remoting_client.h"
 
@@ -15,6 +17,7 @@ static const CGFloat kFabInset = 15.f;
 
 @interface HostViewController () {
   RemotingClient* _client;
+  MDCFloatingButton* _floatingButton;
 }
 @end
 
@@ -30,25 +33,23 @@ static const CGFloat kFabInset = 15.f;
 
 #pragma mark - UIViewController
 
+- (void)loadView {
+  self.view = [[GLKView alloc] initWithFrame:CGRectZero];
+}
+
 - (void)viewDidLoad {
   [super viewDidLoad];
-  MDCFloatingButton* floatingButton =
+  _floatingButton =
       [MDCFloatingButton floatingButtonWithShape:MDCFloatingButtonShapeMini];
-  [floatingButton setTitle:@"+" forState:UIControlStateNormal];
-  [floatingButton addTarget:self
+  [_floatingButton setTitle:@"+" forState:UIControlStateNormal];
+  [_floatingButton addTarget:self
                       action:@selector(didTap:)
             forControlEvents:UIControlEventTouchUpInside];
 
   UIImage* settingsImage = [UIImage imageNamed:@"Settings"];
-  [floatingButton setImage:settingsImage forState:UIControlStateNormal];
-  [floatingButton sizeToFit];
-  CGSize btnSize = floatingButton.frame.size;
-  floatingButton.frame =
-      CGRectMake(self.view.frame.size.width - btnSize.width - kFabInset,
-                 self.view.frame.size.height - btnSize.height - kFabInset,
-                 btnSize.width, btnSize.height);
-
-  [self.view addSubview:floatingButton];
+  [_floatingButton setImage:settingsImage forState:UIControlStateNormal];
+  [_floatingButton sizeToFit];
+  [self.view addSubview:_floatingButton];
 }
 
 - (void)viewDidUnload {
@@ -62,18 +63,35 @@ static const CGFloat kFabInset = 15.f;
 
 - (void)viewDidAppear:(BOOL)animated {
   [super viewDidAppear:animated];
-  GLKView* view = (GLKView*)self.view;
-  view.context = [_client.displayHandler GetEAGLContext];
+  GLKView* glView = (GLKView*)self.view;
+  glView.context = [_client.displayHandler GetEAGLContext];
+  [_client.displayHandler onSurfaceCreated:glView];
+
+  // viewDidLayoutSubviews may be called before viewDidAppear, in which case
+  // the surface is not ready and onSurfaceChanged will be no-op.
+  // Call onSurfaceChanged here to cover that case.
+  [_client.displayHandler onSurfaceChanged:glView.frame];
+}
+
+- (void)viewWillAppear:(BOOL)animated {
+  [super viewWillAppear:animated];
+  ((GLKView*)self.view).enableSetNeedsDisplay = true;
 }
 
 - (void)viewWillDisappear:(BOOL)animated {
   [super viewWillDisappear:animated];
 }
 
-#pragma mark - GLKViewDelegate
+- (void)viewDidLayoutSubviews {
+  [super viewDidLayoutSubviews];
+
+  [_client.displayHandler onSurfaceChanged:((GLKView*)self.view).frame];
 
-- (void)glkView:(GLKView*)view drawInRect:(CGRect)rect {
-  // Nothing to do that is synchronous yet.
+  const CGSize& btnSize = _floatingButton.frame.size;
+  _floatingButton.frame =
+      CGRectMake(self.view.frame.size.width - btnSize.width - kFabInset,
+                 self.view.frame.size.height - btnSize.height - kFabInset,
+                 btnSize.width, btnSize.height);
 }
 
 #pragma mark - Private

remoting/client/ios/display/gl_display_handler.h

@@ -28,7 +28,13 @@ class CursorShapeStub;
 }
 
 - (void)stop;
-- (void)glkView:(GLKView*)view drawInRect:(CGRect)rect;
+
+// Called once the GLKView created.
+- (void)onSurfaceCreated:(GLKView*)view;
+
+// Called every time the GLKView dimension is initialized or changed.
+- (void)onSurfaceChanged:(const CGRect&)frame;
+
 - (std::unique_ptr<remoting::protocol::VideoRenderer>)CreateVideoRenderer;
 - (std::unique_ptr<remoting::protocol::CursorShapeStub>)CreateCursorShapeStub;
 

remoting/client/ios/display/gl_display_handler.mm

@@ -48,6 +48,7 @@ class Core : public protocol::CursorShapeStub, public GlRendererDelegate {
   void OnFrameReceived(std::unique_ptr<webrtc::DesktopFrame> frame,
                        const base::Closure& done);
   void Stop();
+  void SurfaceCreated(GLKView* view);
   void SurfaceChanged(int width, int height);
   std::unique_ptr<protocol::FrameConsumer> GrabFrameConsumer();
   EAGLContext* GetEAGLContext();
@@ -60,6 +61,7 @@ class Core : public protocol::CursorShapeStub, public GlRendererDelegate {
   std::unique_ptr<DualBufferFrameConsumer> owned_frame_consumer_;
   base::WeakPtr<DualBufferFrameConsumer> frame_consumer_;
 
+  GLKView* gl_view_;
   EAGLContext* eagl_context_;
   std::unique_ptr<GlRenderer> renderer_;
   //  GlDemoScreen *demo_screen_;
@@ -108,19 +110,6 @@ void Core::Initialize() {
 
   //  renderer_.RequestCanvasSize();
 
-  renderer_->OnSurfaceCreated(
-      base::MakeUnique<GlCanvas>(static_cast<int>([eagl_context_ API])));
-
-  SurfaceChanged(1024, 640);  // TODO(nicholss): Where does this data comefrom?
-
-  // TODO(nicholss): This are wrong values but it lets us get something on the
-  // screen.
-  std::array<float, 9> matrix = {{1, 0, 0,
-                                  0, 1, 0,
-                                  0, 0, 1}};
-
-  renderer_->OnPixelTransformationChanged(matrix);
-
   // demo_screen_ = new GlDemoScreen();
   // renderer_->AddDrawable(demo_screen_->GetWeakPtr());
   renderer_->SetDelegate(weak_ptr_);
@@ -133,7 +122,7 @@ void Core::SetCursorShape(const protocol::CursorShapeInfo& cursor_shape) {
 
 bool Core::CanRenderFrame() {
   DCHECK(runtime_->display_task_runner()->BelongsToCurrentThread());
-  return eagl_context_ != NULL;
+  return gl_view_ != NULL && eagl_context_ != NULL;
 }
 
 std::unique_ptr<protocol::FrameConsumer> Core::GrabFrameConsumer() {
@@ -148,7 +137,7 @@ void Core::OnFrameReceived(std::unique_ptr<webrtc::DesktopFrame> frame,
 }
 
 void Core::OnFrameRendered() {
-  // Nothing to do.
+  [gl_view_ setNeedsDisplay];
 }
 
 void Core::OnSizeChanged(int width, int height) {
@@ -162,9 +151,29 @@ void Core::Stop() {
   // demo_screen_ = nil;
 }
 
+void Core::SurfaceCreated(GLKView* view) {
+  DCHECK(runtime_->display_task_runner()->BelongsToCurrentThread());
+  gl_view_ = view;
+
+  renderer_->OnSurfaceCreated(
+      base::MakeUnique<GlCanvas>(static_cast<int>([eagl_context_ API])));
+
+  runtime_->network_task_runner()->PostTask(
+      FROM_HERE, base::Bind(&DualBufferFrameConsumer::RequestFullDesktopFrame,
+                            frame_consumer_));
+}
+
 void Core::SurfaceChanged(int width, int height) {
   DCHECK(runtime_->display_task_runner()->BelongsToCurrentThread());
   renderer_->OnSurfaceChanged(width, height);
+
+  // TODO(nicholss): This are wrong values but it lets us get something on the
+  // screen.
+  std::array<float, 9> matrix = {{1, 0, 0,  // Row 1
+                                  0, 1, 0,  // Row 2
+                                  0, 0, 1}};
+
+  renderer_->OnPixelTransformationChanged(matrix);
 }
 
 EAGLContext* Core::GetEAGLContext() {
@@ -217,6 +226,19 @@ base::WeakPtr<remoting::GlDisplayHandler::Core> Core::GetWeakPtr() {
   return _core->GetEAGLContext();
 }
 
+- (void)onSurfaceCreated:(GLKView*)view {
+  _runtime->display_task_runner()->PostTask(
+      FROM_HERE, base::Bind(&remoting::GlDisplayHandler::Core::SurfaceCreated,
+                            _core->GetWeakPtr(), view));
+}
+
+- (void)onSurfaceChanged:(const CGRect&)frame {
+  _runtime->display_task_runner()->PostTask(
+      FROM_HERE,
+      base::Bind(&remoting::GlDisplayHandler::Core::SurfaceChanged,
+                 _core->GetWeakPtr(), frame.size.width, frame.size.height));
+}
+
 // TODO(nicholss): Remove this function, it is not used in the final impl,
 // or it should call RequestRender.
 - (void)glkView:(GLKView*)view drawInRect:(CGRect)rect {