Decoupling backbuffer allocation suggestion from frontbuffer allocation suggestion.

content/common/gpu/image_transport_surface_linux.cc

 // Copyright (c) 2012 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 "content/common/gpu/image_transport_surface.h"
 
 // This conflicts with the defines in Xlib.h and must come first.
 #include "content/common/gpu/gpu_messages.h"
 
 #include <map>
@@ skipping 75 matching lines @@
   // gfx::GLSurface implementation
   virtual bool Initialize() OVERRIDE;
   virtual void Destroy() OVERRIDE;
   virtual bool IsOffscreen() OVERRIDE;
   virtual bool SwapBuffers() OVERRIDE;
   virtual bool PostSubBuffer(int x, int y, int width, int height) OVERRIDE;
   virtual std::string GetExtensions() OVERRIDE;
   virtual gfx::Size GetSize() OVERRIDE;
   virtual bool OnMakeCurrent(gfx::GLContext* context) OVERRIDE;
   virtual unsigned int GetBackingFrameBufferObject() OVERRIDE;
-  virtual void SetBufferAllocation(BufferAllocationState state) OVERRIDE;
+  virtual void SetBackbufferAllocation(bool allocated) OVERRIDE;
+  virtual void SetFrontbufferAllocation(bool allocated) OVERRIDE;
 
  protected:
   // ImageTransportSurface implementation
   virtual void OnNewSurfaceACK(
       uint64 surface_handle, TransportDIB::Handle shm_handle) OVERRIDE;
   virtual void OnBuffersSwappedACK() OVERRIDE;
   virtual void OnPostSubBufferACK() OVERRIDE;
   virtual void OnResizeViewACK() OVERRIDE;
   virtual void OnResize(gfx::Size size) OVERRIDE;
 
  private:
   virtual ~EGLImageTransportSurface() OVERRIDE;
   void ReleaseSurface(scoped_refptr<EGLAcceleratedSurface>* surface);
   void SendBuffersSwapped();
   void SendPostSubBuffer(int x, int y, int width, int height);
+  void AdjustBufferAllocations();
 
   // Tracks the current buffer allocation state.
-  BufferAllocationState buffer_allocation_state_;
+  bool backbuffer_suggested_allocation_;
+  bool frontbuffer_suggested_allocation_;
 
   uint32 fbo_id_;
 
   scoped_refptr<EGLAcceleratedSurface> back_surface_;
   scoped_refptr<EGLAcceleratedSurface> front_surface_;
   gfx::Rect previous_damage_rect_;
 
   // Whether or not we've successfully made the surface current once.
   bool made_current_;
 
@@ skipping 13 matching lines @@
                            GpuCommandBufferStub* stub);
 
   // gfx::GLSurface implementation:
   virtual bool Initialize() OVERRIDE;
   virtual void Destroy() OVERRIDE;
   virtual bool SwapBuffers() OVERRIDE;
   virtual bool PostSubBuffer(int x, int y, int width, int height) OVERRIDE;
   virtual std::string GetExtensions();
   virtual gfx::Size GetSize() OVERRIDE;
   virtual bool OnMakeCurrent(gfx::GLContext* context) OVERRIDE;
-  virtual void SetBufferAllocation(BufferAllocationState state) OVERRIDE;
+  virtual void SetBackbufferAllocation(bool allocated) OVERRIDE;
+  virtual void SetFrontbufferAllocation(bool allocated) OVERRIDE;
 
  protected:
   // ImageTransportSurface implementation:
   virtual void OnNewSurfaceACK(
       uint64 surface_handle, TransportDIB::Handle shm_handle) OVERRIDE;
   virtual void OnBuffersSwappedACK() OVERRIDE;
   virtual void OnPostSubBufferACK() OVERRIDE;
   virtual void OnResizeViewACK() OVERRIDE;
   virtual void OnResize(gfx::Size size) OVERRIDE;
 
  private:
   virtual ~GLXImageTransportSurface();
 
   // Tell the browser to release the surface.
   void ReleaseSurface();
 
   void SendBuffersSwapped();
   void SendPostSubBuffer(int x, int y, int width, int height);
 
   void ResizeSurface(gfx::Size size);
+  void AdjustBufferAllocations();
 
   // Tracks the current buffer allocation state.
-  BufferAllocationState buffer_allocation_state_;
+  bool backbuffer_suggested_allocation_;
+  bool frontbuffer_suggested_allocation_;
 
   XID dummy_parent_;
   gfx::Size size_;
 
   // Whether or not the image has been bound on the browser side.
   bool bound_;
 
   // Whether or not we need to send a resize on the next swap.
   bool needs_resize_;
 
@@ skipping 81 matching lines @@
 EGLAcceleratedSurface::~EGLAcceleratedSurface() {
   glDeleteTextures(1, &texture_);
   eglDestroyImageKHR(gfx::GLSurfaceEGL::GetHardwareDisplay(), image_);
   XFreePixmap(gfx::GLSurfaceEGL::GetNativeDisplay(), pixmap_);
 }
 
 EGLImageTransportSurface::EGLImageTransportSurface(
     GpuChannelManager* manager,
     GpuCommandBufferStub* stub)
     : gfx::PbufferGLSurfaceEGL(false, gfx::Size(16, 16)),
-      buffer_allocation_state_(BUFFER_ALLOCATION_FRONT_AND_BACK),
+      backbuffer_suggested_allocation_(true),
+      frontbuffer_suggested_allocation_(true),
       fbo_id_(0),
       made_current_(false) {
   helper_.reset(new ImageTransportHelper(this,
                                          manager,
                                          stub,
                                          gfx::kNullPluginWindow));
 }
 
 EGLImageTransportSurface::~EGLImageTransportSurface() {
   Destroy();
@@ skipping 45 matching lines @@
   }
 
   made_current_ = true;
   return true;
 }
 
 unsigned int EGLImageTransportSurface::GetBackingFrameBufferObject() {
   return fbo_id_;
 }
 
-void EGLImageTransportSurface::SetBufferAllocation(
-    BufferAllocationState state) {
-  if (buffer_allocation_state_ == state)
+void EGLImageTransportSurface::SetBackbufferAllocation(bool allocated) {
+  if (backbuffer_suggested_allocation_ == allocated)
     return;
-  buffer_allocation_state_ = state;
+  backbuffer_suggested_allocation_ = allocated;
+  AdjustBufferAllocations();
+}
 
-  switch (state) {
-    case BUFFER_ALLOCATION_FRONT_AND_BACK:
-      if (!back_surface_.get() && front_surface_.get())
-        OnResize(front_surface_->size());
-      break;
+void EGLImageTransportSurface::SetFrontbufferAllocation(bool allocated) {
+  if (frontbuffer_suggested_allocation_ == allocated)
+    return;
+  frontbuffer_suggested_allocation_ = allocated;
+  AdjustBufferAllocations();
+}
 
-    case BUFFER_ALLOCATION_FRONT_ONLY:
-      if (back_surface_.get() && front_surface_.get())
-        ReleaseSurface(&back_surface_);
-      break;
-
-    case BUFFER_ALLOCATION_NONE:
-      if (back_surface_.get() && front_surface_.get())
-        ReleaseSurface(&back_surface_);
-      break;
-
-    default:
-      NOTREACHED();
+void EGLImageTransportSurface::AdjustBufferAllocations() {
+  if (backbuffer_suggested_allocation_) {
+    if (back_surface_.get())
+      return;
+    if (!front_surface_.get())
+      return;
+    OnResize(front_surface_->size());

[jonathan.backer, 2012/05/09 18:13:01]

AFAICT, we're only managing the backbuffer here. So why are we calling it when
the frontbuffer allocation changes?

The only reason we need a front_surface_ to create a back_surface_ is to get a
size. Maybe we should keep this state separately to simplify the code?

If not, maybe some of these early exits should become DCHECKS so that it's
easier to reason about.

[mmocny, 2012/05/09 18:59:15]

Yes, we need front only to read size, and we should just be caching it.  I was
attempting to limit drive by fixme's and just replicate existing functionality,
but this is probably worth the fix, especially given the next comment..

As for why this gets called on frontbuffer allocation change:  Just for
consistency with the other paths.  It is a noop for now, but that may change.

On 2012/05/09 18:13:01, jonathan.backer wrote:

+  } else {
+    if (!back_surface_.get())
+      return;
+    if (!front_surface_.get())
+      return;

[jonathan.backer, 2012/05/09 18:13:01]

Why do we exit early here? Perhaps we should never be in this state (in which
case a DCHECK is appropriate). But I don't understand early exit.

[mmocny, 2012/05/09 18:59:15]

Its possible that this is testing for a case that isn't expected and a DCHECK
would suffice.  This is only needed because we expect a front when we recreate,
and if we just cache the size that wont be needed.

On 2012/05/09 18:13:01, jonathan.backer wrote:

+    ReleaseSurface(&back_surface_);
   }
 }
 
+
 void EGLImageTransportSurface::ReleaseSurface(
     scoped_refptr<EGLAcceleratedSurface>* surface) {
   if (surface->get()) {
     GpuHostMsg_AcceleratedSurfaceRelease_Params params;
     params.identifier = (*surface)->pixmap();
     helper_->SendAcceleratedSurfaceRelease(params);
     *surface = NULL;
   }
 }
 
@@ skipping 16 matching lines @@
   GpuHostMsg_AcceleratedSurfaceNew_Params params;
   params.width = size.width();
   params.height = size.height();
   params.surface_handle = back_surface_->pixmap();
   helper_->SendAcceleratedSurfaceNew(params);
 
   helper_->SetScheduled(false);
 }
 
 bool EGLImageTransportSurface::SwapBuffers() {
+  DCHECK(backbuffer_suggested_allocation_);
+  DCHECK(frontbuffer_suggested_allocation_);
   front_surface_.swap(back_surface_);
   DCHECK_NE(front_surface_.get(), static_cast<EGLAcceleratedSurface*>(NULL));
   helper_->DeferToFence(base::Bind(
       &EGLImageTransportSurface::SendBuffersSwapped,
       AsWeakPtr()));
 
   gfx::Size expected_size = front_surface_->size();
   if (!back_surface_.get() || back_surface_->size() != expected_size) {
     OnResize(expected_size);
   } else {
     glFramebufferTexture2DEXT(GL_FRAMEBUFFER,
                               GL_COLOR_ATTACHMENT0,
                               GL_TEXTURE_2D,
                               back_surface_->texture(),
                               0);
   }
   previous_damage_rect_ = gfx::Rect(front_surface_->size());
   return true;
 }
 
 void EGLImageTransportSurface::SendBuffersSwapped() {
   GpuHostMsg_AcceleratedSurfaceBuffersSwapped_Params params;
   params.surface_handle = front_surface_->pixmap();
   helper_->SendAcceleratedSurfaceBuffersSwapped(params);
   helper_->SetScheduled(false);
 }
 
 bool EGLImageTransportSurface::PostSubBuffer(
     int x, int y, int width, int height) {
-
+  DCHECK(backbuffer_suggested_allocation_);
+  DCHECK(frontbuffer_suggested_allocation_);
   DCHECK_NE(back_surface_.get(), static_cast<EGLAcceleratedSurface*>(NULL));
   gfx::Size expected_size = back_surface_->size();
   bool surfaces_same_size = front_surface_.get() &&
       front_surface_->size() == expected_size;
 
   const gfx::Rect new_damage_rect(x, y, width, height);
 
   // An empty damage rect is a successful no-op.
   if (new_damage_rect.IsEmpty())
     return true;
@@ skipping 78 matching lines @@
 }
 
 void EGLImageTransportSurface::OnResizeViewACK() {
   NOTREACHED();
 }
 
 GLXImageTransportSurface::GLXImageTransportSurface(
     GpuChannelManager* manager,
     GpuCommandBufferStub* stub)
     : gfx::NativeViewGLSurfaceGLX(),
-      buffer_allocation_state_(BUFFER_ALLOCATION_FRONT_AND_BACK),
+      backbuffer_suggested_allocation_(true),
+      frontbuffer_suggested_allocation_(true),
       dummy_parent_(0),
       size_(1, 1),
       bound_(false),
       needs_resize_(false),
       made_current_(false) {
   helper_.reset(new ImageTransportHelper(this,
                                          manager,
                                          stub,
                                          gfx::kNullPluginWindow));
 }
@@ skipping 63 matching lines @@
 }
 
 void GLXImageTransportSurface::ReleaseSurface() {
   DCHECK(bound_);
   GpuHostMsg_AcceleratedSurfaceRelease_Params params;
   params.identifier = window_;
   helper_->SendAcceleratedSurfaceRelease(params);
   bound_ = false;
 }
 
-void GLXImageTransportSurface::SetBufferAllocation(
-    BufferAllocationState state) {
-  if (buffer_allocation_state_ == state)
+void GLXImageTransportSurface::SetBackbufferAllocation(bool allocated) {
+  if (backbuffer_suggested_allocation_ == allocated)
     return;
-  buffer_allocation_state_ = state;
+  backbuffer_suggested_allocation_ = allocated;
+  AdjustBufferAllocations();
+}
 
-  switch (state) {
-    case BUFFER_ALLOCATION_FRONT_AND_BACK: {
-      ResizeSurface(size_);
-      break;
-    }
-    case BUFFER_ALLOCATION_FRONT_ONLY: {
-      ResizeSurface(gfx::Size(1,1));
-      break;
-    }
-    case BUFFER_ALLOCATION_NONE: {
-      ResizeSurface(gfx::Size(1,1));
-      if (bound_)
-        ReleaseSurface();
-      break;
-    }
-    default:
-      NOTREACHED();
+void GLXImageTransportSurface::SetFrontbufferAllocation(bool allocated) {
+  if (frontbuffer_suggested_allocation_ == allocated)
+    return;
+  frontbuffer_suggested_allocation_ = allocated;
+  AdjustBufferAllocations();
+}
+
+void GLXImageTransportSurface::AdjustBufferAllocations() {
+  if (backbuffer_suggested_allocation_) {
+    ResizeSurface(size_);
+  } else {
+    ResizeSurface(gfx::Size(1,1));
+    if (!frontbuffer_suggested_allocation_ && bound_)
+      ReleaseSurface();
   }
 }
 
 void GLXImageTransportSurface::ResizeSurface(gfx::Size size) {
   Display* dpy = static_cast<Display*>(GetDisplay());
   XResizeWindow(dpy, window_, size.width(), size.height());
   glXWaitX();
   // Seems necessary to perform a swap after a resize
   // in order to resize the front and back buffers (Intel driver bug).
   // This doesn't always happen with scissoring enabled, so do it now.
   if (gfx::g_GLX_MESA_copy_sub_buffer && gfx::GLSurface::GetCurrent() == this)
     gfx::NativeViewGLSurfaceGLX::SwapBuffers();
   needs_resize_ = true;
 }
 
 void GLXImageTransportSurface::OnResize(gfx::Size size) {
   TRACE_EVENT0("gpu", "GLXImageTransportSurface::OnResize");
   size_ = size;
   ResizeSurface(size_);
 }
 
 bool GLXImageTransportSurface::SwapBuffers() {
+  DCHECK(backbuffer_suggested_allocation_);
+  DCHECK(frontbuffer_suggested_allocation_);
   gfx::NativeViewGLSurfaceGLX::SwapBuffers();
   helper_->DeferToFence(base::Bind(
       &GLXImageTransportSurface::SendBuffersSwapped,
       AsWeakPtr()));
 
   if (needs_resize_) {
     GpuHostMsg_AcceleratedSurfaceNew_Params params;
     params.width = size_.width();
     params.height = size_.height();
     params.surface_handle = window_;
     helper_->SendAcceleratedSurfaceNew(params);
     bound_ = true;
     needs_resize_ = false;
   }
   return true;
 }
 
 void GLXImageTransportSurface::SendBuffersSwapped() {
   GpuHostMsg_AcceleratedSurfaceBuffersSwapped_Params params;
   params.surface_handle = window_;
   helper_->SendAcceleratedSurfaceBuffersSwapped(params);
   helper_->SetScheduled(false);
 }
 
 bool GLXImageTransportSurface::PostSubBuffer(
     int x, int y, int width, int height) {
+  DCHECK(backbuffer_suggested_allocation_);
+  DCHECK(frontbuffer_suggested_allocation_);
   gfx::NativeViewGLSurfaceGLX::PostSubBuffer(x, y, width, height);
   helper_->DeferToFence(base::Bind(
       &GLXImageTransportSurface::SendPostSubBuffer,
       AsWeakPtr(), x, y, width, height));
 
   if (needs_resize_) {
     GpuHostMsg_AcceleratedSurfaceNew_Params params;
     params.width = size_.width();
     params.height = size_.height();
     params.surface_handle = window_;
@@ skipping 244 matching lines @@
                                                    stub,
                                                    surface.get(),
                                                    handle.transport);
   }
 
   if (surface->Initialize())
     return surface;
   else
     return NULL;
 }