Draw the offscreen texture to reflector's surface without extra copy

content/browser/compositor/offscreen_browser_compositor_output_surface.cc

 // Copyright 2015 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/browser/compositor/offscreen_browser_compositor_output_surface.h"
 
 #include "base/logging.h"
 #include "cc/output/compositor_frame.h"
 #include "cc/output/compositor_frame_ack.h"
 #include "cc/output/gl_frame_data.h"
 #include "cc/output/output_surface_client.h"
 #include "cc/resources/resource_provider.h"
 #include "content/browser/compositor/browser_compositor_overlay_candidate_validator.h"
+#include "content/browser/compositor/owned_mailbox.h"
 #include "content/browser/compositor/reflector_impl.h"
 #include "content/common/gpu/client/context_provider_command_buffer.h"
+#include "content/common/gpu/client/gl_helper.h"
 #include "content/public/browser/browser_thread.h"
 #include "gpu/command_buffer/client/context_support.h"
 #include "gpu/command_buffer/client/gles2_interface.h"
 #include "third_party/khronos/GLES2/gl2.h"
 #include "third_party/khronos/GLES2/gl2ext.h"
 
 using cc::CompositorFrame;
 using cc::GLFrameData;
 using cc::ResourceProvider;
 using gpu::gles2::GLES2Interface;
 
 namespace content {
 
 OffscreenBrowserCompositorOutputSurface::
     OffscreenBrowserCompositorOutputSurface(
         const scoped_refptr<ContextProviderCommandBuffer>& context,
         const scoped_refptr<ui::CompositorVSyncManager>& vsync_manager,
         scoped_ptr<BrowserCompositorOverlayCandidateValidator>
             overlay_candidate_validator)
     : BrowserCompositorOutputSurface(context,
                                      vsync_manager,
                                      overlay_candidate_validator.Pass()),
       fbo_(0),
       is_backbuffer_discarded_(false),
       backing_texture_id_(0),
+      needs_update_reflector_(false),
       weak_ptr_factory_(this) {
   capabilities_.max_frames_pending = 1;
   capabilities_.uses_default_gl_framebuffer = false;
 }
 
 OffscreenBrowserCompositorOutputSurface::
     ~OffscreenBrowserCompositorOutputSurface() {
   DiscardBackbuffer();
 }
 
 void OffscreenBrowserCompositorOutputSurface::EnsureBackbuffer() {
   is_backbuffer_discarded_ = false;
 
   if (!backing_texture_id_) {
+    GLHelper* shared_helper =
+        ImageTransportFactory::GetInstance()->GetGLHelper();
+    mailbox_ = new OwnedMailbox(shared_helper);

[piman, 2015/05/07 01:56:45]

Why not creating mailbox_ with the offline_compositor_gl_helper_? That way you
don't need to ConsumeMailboxToTexture it - you can simply use
mailbox_->texture_id(). (but then you want to release mailbox_ in
DiscardBackbuffer)


I wonder if it might be possible to converge both paths. Especially now that the
thread is gone, it may be easier to always have the
BrowserCompositorOutputSurface produce the mailbox (i.e. in the old path, do the
readback), and then the Reflector just manages the interaction with the
mirroring layers, forwarding the mailbox and damage. What do you think?

[oshima, 2015/05/07 16:43:24]

ok, let me rework on it.

[oshima, 2015/05/13 07:47:03]

Looks like I still need to use shared helper because mailbox needs to be alive
until
the release callback is called, which is set in Layer::SetTextureMailbox.

I confirmed this with danakj@. 

     GLES2Interface* gl = context_provider_->ContextGL();
+
+    offline_compositor_gl_helper_.reset(
+        new GLHelper(gl, context_provider()->ContextSupport()));
+
+    backing_texture_id_ =
+        offline_compositor_gl_helper_->ConsumeMailboxToTexture(
+            mailbox_->mailbox(), mailbox_->sync_point());
+
     cc::ResourceFormat format = cc::RGBA_8888;
-
-    gl->GenTextures(1, &backing_texture_id_);
     gl->BindTexture(GL_TEXTURE_2D, backing_texture_id_);
     gl->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
     gl->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
     gl->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
     gl->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
     gl->TexImage2D(GL_TEXTURE_2D, 0, GLInternalFormat(format),
                    surface_size_.width(), surface_size_.height(), 0,
                    GLDataFormat(format), GLDataType(format), nullptr);
+    needs_update_reflector_ = true;
   }
 }
 
 void OffscreenBrowserCompositorOutputSurface::DiscardBackbuffer() {
   is_backbuffer_discarded_ = true;
 
   GLES2Interface* gl = context_provider_->ContextGL();
 
   if (backing_texture_id_) {
     gl->DeleteTextures(1, &backing_texture_id_);
@@ skipping 21 matching lines @@
 void OffscreenBrowserCompositorOutputSurface::BindFramebuffer() {
   EnsureBackbuffer();
   DCHECK(backing_texture_id_);
 
   GLES2Interface* gl = context_provider_->ContextGL();
 
   if (!fbo_)
     gl->GenFramebuffers(1, &fbo_);
   gl->BindFramebuffer(GL_FRAMEBUFFER, fbo_);
   gl->FramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D,
                            backing_texture_id_, 0);

[piman, 2015/05/07 01:56:45]

I just realized this, you end up calling this every frame, and it's somewhat
expensive. In truth, you can just create the fbo_ together with the texture, it
would simplify.

[oshima, 2015/05/07 16:43:24]

thanks, will do.

 }
 
 void OffscreenBrowserCompositorOutputSurface::SwapBuffers(
     cc::CompositorFrame* frame) {
-  // TODO(oshima): Pass the texture to the reflector so that the
-  // reflector can simply use and draw it on their surface instead of
-  // copying the texture.
   if (reflector_) {
+    if (needs_update_reflector_) {
+      needs_update_reflector_ = false;
+      reflector_->OnSourceTextureMailboxUpdated();
+    }
+    offline_compositor_gl_helper_->InsertOrderingBarrier();

[piman, 2015/05/07 01:56:45]

The InsertOrderingBarrier is needed to order between 2 contexts. Here we just
used the output surface's context. In OnSourceSwapBuffers, it's also the same
context that we use, so I don't think the InsertOrderingBarrier is needed here.
It is needed in OnSourceSwapBuffers to order wrt the mirroring compositor's
context.

[oshima, 2015/05/13 07:47:03]

Removed

+
     if (frame->gl_frame_data->sub_buffer_rect ==
         gfx::Rect(frame->gl_frame_data->size))
       reflector_->OnSourceSwapBuffers();
     else
       reflector_->OnSourcePostSubBuffer(frame->gl_frame_data->sub_buffer_rect);
   }
 
   client_->DidSwapBuffers();
+}
 
-  // TODO(oshima): sync with the reflector's SwapBuffersComplete.
-  uint32_t sync_point =
-      context_provider_->ContextGL()->InsertSyncPointCHROMIUM();
-  context_provider_->ContextSupport()->SignalSyncPoint(
-      sync_point, base::Bind(&OutputSurface::OnSwapBuffersComplete,
-                             weak_ptr_factory_.GetWeakPtr()));
+scoped_refptr<OwnedMailbox>
+OffscreenBrowserCompositorOutputSurface::GetTextureMailbox() {
+  EnsureBackbuffer();

[piman, 2015/05/07 01:56:45]

Is this needed? The compositor is not allowed to draw / call SwapBuffers if it
hasn't called EnsureBackbuffer first. Then maybe you can just call
OnSourceTextureMailboxUpdated directly from EnsureBackbuffer, and not need
needs_update_reflector_.

[oshima, 2015/05/07 16:43:24]

I need GetTextureMailbox call so that I can get the mailbox in
OnSourceOutputSurfaceReady. I'll rework to consolidate two path and hopefully we
can clean this up.

+  // This is called from the reflector, so no need to update
+  // the refletor.
+  needs_update_reflector_ = false;
+  return mailbox_;
+}
+
+base::Closure
+OffscreenBrowserCompositorOutputSurface::CreateCompositionStartedCallback() {
+  return base::Bind(&OutputSurface::OnSwapBuffersComplete,
+                    weak_ptr_factory_.GetWeakPtr());
 }
 
 #if defined(OS_MACOSX)
 
 bool OffscreenBrowserCompositorOutputSurface::
 SurfaceShouldNotShowFramesAfterSuspendForRecycle() const {
   return true;
 }
 
 #endif
 
 }  // namespace content