[Chromoting] Fallback to use software renderer if desktop image size is larger than GPU limitaiton

remoting/client/plugin/pepper_video_renderer_3d.cc

 // Copyright 2014 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/plugin/pepper_video_renderer_3d.h"
 
 #include <math.h>
 
 #include <algorithm>
 #include <utility>
@@ skipping 73 matching lines @@
   if (shader_program_)
     gles2_if_->DeleteProgram(graphics_.pp_resource(), shader_program_);
 }
 
 void PepperVideoRenderer3D::SetPepperContext(
     pp::Instance* instance,
     EventHandler* event_handler) {
   DCHECK(event_handler);
   DCHECK(!event_handler_);
 
+  fallback_renderer_.SetPepperContext(instance, event_handler);
+
   event_handler_ = event_handler;
   pp_instance_ = instance;
 }
 
 void PepperVideoRenderer3D::OnViewChanged(const pp::View& view) {
+  fallback_renderer_.OnViewChanged(view);
+
   pp::Size size = view.GetRect().size();
   float scale = view.GetDeviceScale();
+  if (scale == 0) {

[Sergey Ulanov, 2016/10/12 21:53:31]

scale is never expected to be 0. maybe replace it with DCHECK()?

[Hzj_jie, 2016/10/12 23:42:46]

Done.

+    LOG(WARNING) << "Unexpected device scale factor received.";
+    scale = 1;
+  }
+  gl_scaled_limits_.texture_size = gl_limits_.texture_size / scale;

[Sergey Ulanov, 2016/10/12 21:53:31]

I'm not sure this is correct. textures should not be scaled until they are
rendered, so I don't see how the scale factor can affect max texture size. If
you believe this is correct, maybe add a comment to explain it.

[Hzj_jie, 2016/10/12 23:42:46]

Yes, the GetDeviceScale returns a scale factor to represent pixels per logical
pixel. But hardware knows only hardware pixel. So if an hardware supports only
8192 x 8192, and we want to render a 5000 x 5000 in logic pixel, with scale
factor as 2, the hardware will still need to render 10000 x 10000 hardware
pixels, which exceeds the limitation.
I will add comment to explain this behavior.

But these lines have no relationship with the following view_size_.set()
statement. I put them here simply because this is the only place we can get a
pp::View object.

+  gl_scaled_limits_.viewport_size[0] = gl_limits_.viewport_size[0] / scale;
+  gl_scaled_limits_.viewport_size[1] = gl_limits_.viewport_size[1] / scale;
   view_size_.set(ceilf(size.width() * scale), ceilf(size.height() * scale));
   graphics_.ResizeBuffers(view_size_.width(), view_size_.height());
 
   force_repaint_ = true;
   PaintIfNeeded();
 }
 
 void PepperVideoRenderer3D::EnableDebugDirtyRegion(bool enable) {
+  fallback_renderer_.EnableDebugDirtyRegion(enable);
   debug_dirty_region_ = enable;
 }
 
 bool PepperVideoRenderer3D::Initialize(
     const ClientContext& context,
     protocol::FrameStatsConsumer* stats_consumer) {
+  CHECK(fallback_renderer_.Initialize(context, stats_consumer));

[Sergey Ulanov, 2016/10/12 21:53:31]

Please change this to 
  if (!fallback_renderer_.Initialize(context, stats_consumer))
    LOG(FATAL) << "Failed to initialize fallback_renderer_";

CHECKS() and DCHECKS() should not have side-effects. This code will break if one
decides to replace this CHECK with a DCHECK.

[Hzj_jie, 2016/10/12 23:42:46]

Done.

+
   stats_consumer_ = stats_consumer;
 
   const int32_t context_attributes[] = {
       PP_GRAPHICS3DATTRIB_ALPHA_SIZE,     8,
       PP_GRAPHICS3DATTRIB_BLUE_SIZE,      8,
       PP_GRAPHICS3DATTRIB_GREEN_SIZE,     8,
       PP_GRAPHICS3DATTRIB_RED_SIZE,       8,
       PP_GRAPHICS3DATTRIB_DEPTH_SIZE,     0,
       PP_GRAPHICS3DATTRIB_STENCIL_SIZE,   0,
       PP_GRAPHICS3DATTRIB_SAMPLES,        0,
@@ skipping 35 matching lines @@
       -1, -1, -1, 1, 1, -1, 1, 1,  // Position coordinates.
       0,  1,  0,  0, 1, 1,  1, 0,  // Texture coordinates.
   };
 
   GLuint buffer;
   gles2_if_->GenBuffers(graphics_3d, 1, &buffer);
   gles2_if_->BindBuffer(graphics_3d, GL_ARRAY_BUFFER, buffer);
   gles2_if_->BufferData(graphics_3d, GL_ARRAY_BUFFER, sizeof(kVertices),
                         kVertices, GL_STATIC_DRAW);
 
+  gles2_if_->GetIntegerv(
+      graphics_3d, GL_MAX_TEXTURE_SIZE, &gl_limits_.texture_size);
+  gles2_if_->GetIntegerv(
+      graphics_3d, GL_MAX_VIEWPORT_DIMS, gl_limits_.viewport_size);
+
   CheckGLError();
 
   return true;
 }
 
 void PepperVideoRenderer3D::OnSessionConfig(
     const protocol::SessionConfig& config) {
+  fallback_renderer_.OnSessionConfig(config);
+
   PP_VideoProfile video_profile = PP_VIDEOPROFILE_VP8_ANY;
   switch (config.video_config().codec) {
     case protocol::ChannelConfig::CODEC_VP8:
       video_profile = PP_VIDEOPROFILE_VP8_ANY;
       break;
     case protocol::ChannelConfig::CODEC_VP9:
       video_profile = PP_VIDEOPROFILE_VP9_ANY;
       break;
     default:
       NOTREACHED();
@@ skipping 18 matching lines @@
   return nullptr;
 }
 
 protocol::FrameStatsConsumer* PepperVideoRenderer3D::GetFrameStatsConsumer() {
   return stats_consumer_;
 }
 
 void PepperVideoRenderer3D::ProcessVideoPacket(
     std::unique_ptr<VideoPacket> packet,
     const base::Closure& done) {
+  if (!use_fallback_renderer_) {
+    use_fallback_renderer_ =
+        packet->format().has_screen_width() &&
+        packet->format().has_screen_height() &&
+        (packet->format().screen_width() > gl_scaled_limits_.texture_size ||

[Sergey Ulanov, 2016/10/12 21:53:31]

We only want to initialize fallback when resolution changes - that guarantees
that the fallback software decoder will get first key frame after resolution
change.

[Hzj_jie, 2016/10/12 23:42:46]

Sorry, I cannot quite get the point of your comment. If there is not screen
resolution change, the result of this condition won't be impacted.

+         packet->format().screen_height() > gl_scaled_limits_.texture_size ||
+         packet->format().screen_width() > gl_scaled_limits_.viewport_size[0] ||

[Sergey Ulanov, 2016/10/12 21:53:31]

It seems strange to compare host screen size with client viewport size. It looks
to me that the right thing to do is to compare view_size_ and
gl_limits_.viewport_size in OnViewChanged() when the view is resized.
Here when the screen gets resized we only need to make sure that the screen fits
into a texture.

[Hzj_jie, 2016/10/12 23:42:46]

Have I made a misunderstanding here? I believe the hardware limitation impacts
the maximum size of image we are rendering, i.e. packet->format().screen_width()
& packet->format().screen_height(), instead of view_size_.

And viewport size here is just a name in OpenGl, means the maximum image it can
render.

+         packet->format().screen_height() > gl_scaled_limits_.viewport_size[1]);
+    if (use_fallback_renderer_) {

[Sergey Ulanov, 2016/10/12 21:53:31]

It would be easier to understand this code if it was written with a single if
statement:
if (!use_fallback_renderer_ &&
    packet->format().has_screen_width() && 
    .....) {
  use_fallback_renderer_ = true;
  ... state cleanup ...
}

[Hzj_jie, 2016/10/12 23:42:46]

Done.

+      // Clear current instance and use fallback_renderer_.
+      current_picture_frames_.clear();
+      current_picture_.reset();
+      next_picture_frames_.clear();
+      next_picture_.reset();
+      decoded_frames_.clear();
+      pending_frames_.clear();
+      graphics_ = pp::Graphics3D();
+      video_decoder_ = pp::VideoDecoder();
+    }
+  }
+
+  if (use_fallback_renderer_) {
+    fallback_renderer_.GetVideoStub()->ProcessVideoPacket(
+        std::move(packet), done);
+    return;
+  }
+
   VideoPacket* packet_ptr = packet.get();
   std::unique_ptr<FrameTracker> frame_tracker(
       new FrameTracker(std::move(packet), stats_consumer_, done));
 
   // Don't need to do anything if the packet is empty. Host sends empty video
   // packets when the screen is not changing.
   if (packet_ptr->data().empty())
     return;
 
   if (!frame_received_) {
@@ skipping 305 matching lines @@
   gles2_if_->AttachShader(graphics_.pp_resource(), shader_program_, shader);
   gles2_if_->DeleteShader(graphics_.pp_resource(), shader);
 }
 
 void PepperVideoRenderer3D::CheckGLError() {
   GLenum error = gles2_if_->GetError(graphics_.pp_resource());
   CHECK_EQ(error, static_cast<GLenum>(GL_NO_ERROR)) << "GL error: " << error;
 }
 
 }  // namespace remoting