Unified OutputSurface::SwapBuffers.

cc/output/output_surface.cc

Index: cc/output/output_surface.cc
diff --git a/cc/output/output_surface.cc b/cc/output/output_surface.cc
index 84d94b843edfec34473e9fc95623c51445157280..55923ec2e0fd91b6f9de9cbf6583a4fdfea779c7 100644
--- a/cc/output/output_surface.cc
+++ b/cc/output/output_surface.cc
@@ -11,6 +11,7 @@
 #include "base/logging.h"
 #include "base/string_util.h"
 #include "base/strings/string_split.h"
+#include "cc/output/compositor_frame.h"
 #include "cc/output/output_surface_client.h"
 #include "third_party/WebKit/public/platform/WebGraphicsContext3D.h"
 #include "third_party/khronos/GLES2/gl2.h"
@@ -35,7 +36,7 @@ class OutputSurfaceCallbacks
   }
 
   // WK:WGC3D::WGSwapBuffersCompleteCallbackCHROMIUM implementation.
-  virtual void onSwapBuffersComplete() { client_->OnSwapBuffersComplete(); }
+  virtual void onSwapBuffersComplete() { client_->OnSwapBuffersComplete(NULL); }
 
   // WK:WGC3D::WGContextLostCallback implementation.
   virtual void onContextLost() { client_->DidLoseOutputSurface(); }
@@ -49,6 +50,7 @@ OutputSurface::OutputSurface(
     : client_(NULL),
       context3d_(context3d.Pass()),
       has_gl_discard_backbuffer_(false),
+      has_swap_buffers_complete_callback_(false),
       device_scale_factor_(-1) {
 }
 
@@ -57,6 +59,7 @@ OutputSurface::OutputSurface(
     : client_(NULL),
       software_device_(software_device.Pass()),
       has_gl_discard_backbuffer_(false),
+      has_swap_buffers_complete_callback_(false),
       device_scale_factor_(-1) {
 }
 
@@ -67,6 +70,7 @@ OutputSurface::OutputSurface(
       context3d_(context3d.Pass()),
       software_device_(software_device.Pass()),
       has_gl_discard_backbuffer_(false),
+      has_swap_buffers_complete_callback_(false),
       device_scale_factor_(-1) {
 }
 
@@ -129,6 +133,9 @@ void OutputSurface::SetContext3D(
   set<string> extensions(extensions_list.begin(), extensions_list.end());
   has_gl_discard_backbuffer_ =
       extensions.count("GL_CHROMIUM_discard_backbuffer") > 0;
+  has_swap_buffers_complete_callback_ =
+       extensions.count("GL_CHROMIUM_swapbuffers_complete_callback") > 0;
+
 
   context3d_ = context3d.Pass();
   callbacks_.reset(new OutputSurfaceCallbacks(client_));
@@ -136,10 +143,6 @@ void OutputSurface::SetContext3D(
   context3d_->setContextLostCallback(callbacks_.get());
 }
 
-void OutputSurface::SendFrameToParentCompositor(CompositorFrame* frame) {
-  NOTIMPLEMENTED();
-}
-
 void OutputSurface::EnsureBackbuffer() {
   DCHECK(context3d_);
   if (has_gl_discard_backbuffer_)
@@ -162,6 +165,8 @@ void OutputSurface::Reshape(gfx::Size size, float scale_factor) {
     context3d_->reshapeWithScaleFactor(
         size.width(), size.height(), scale_factor);
   }
+  if (software_device_)
+    software_device_->Resize(size);
 }
 
 gfx::Size OutputSurface::SurfaceSize() const {
@@ -173,18 +178,24 @@ void OutputSurface::BindFramebuffer() {
   context3d_->bindFramebuffer(GL_FRAMEBUFFER, 0);
 }
 
-void OutputSurface::SwapBuffers(const ui::LatencyInfo& latency_info) {
+void OutputSurface::SwapBuffers(cc::CompositorFrame* frame) {
   DCHECK(context3d_);
-  // Note that currently this has the same effect as SwapBuffers; we should
-  // consider exposing a different entry point on WebGraphicsContext3D.
-  context3d_->prepareTexture();
-}
+  DCHECK(frame->gl_frame_data);
+
+  gfx::Rect sub_buffer_rect = frame->gl_frame_data->sub_buffer_rect;
+  if (!sub_buffer_rect.IsEmpty()) {
+    context3d()->postSubBufferCHROMIUM(sub_buffer_rect.x(),
+                                       sub_buffer_rect.y(),
+                                       sub_buffer_rect.width(),
+                                       sub_buffer_rect.height());
+  } else {
+    // Note that currently this has the same effect as SwapBuffers; we should
+    // consider exposing a different entry point on WebGraphicsContext3D.
+    context3d()->prepareTexture();
+  }

[joth, 2013/06/10 18:43:32]

fwiw, I found I had to wrap this lot in "if context3d_" (and remove the DCHECK)
to have the SW renderer LTHI unit tests not timeout, as they will now need the
OnSwapBuffersComplete() below.

The alternative is write a dedicated FakeOutputSurface (just adds test
clutter?), or a deeper refactoring of this class to make context3d  optional by
design which is probably a nice idea, but beyond scope of this patch.

[aelias_OOO_until_Jul13, 2013/06/10 18:57:57]

cc_unittests package is already passing as of Patch Set 2.  It did need some
extra logic in FakeOutputSurface.  I agree the OutputSurface hierarchy needs
more refactoring but it was biting off too much to try to add that to this
patch.

[joth, 2013/06/10 19:04:49]

oh yes! Nice. I totally missed that somehow :)

 
-void OutputSurface::PostSubBuffer(gfx::Rect rect,
-                                  const ui::LatencyInfo& latency_info) {
-  DCHECK(context3d_);
-  context3d_->postSubBufferCHROMIUM(
-      rect.x(), rect.y(), rect.width(), rect.height());
+  if (client_ && !has_swap_buffers_complete_callback_)
+    client_->OnSwapBuffersComplete(NULL);
 }
 
 }  // namespace cc