Early wait on texture resource sync points in gl_renderer.

Previously texture resource sync points were waited on
a just-in-time basis, just before drawing with the texture.

This change moves all sync point waits ahead of drawing a
frame, to ensure that we don't switch GLContexts in the
middle of drawing.

BUG=394547

Review URL: https://codereview.chromium.org/411643002

Cr-Commit-Position: refs/heads/master@{#290109}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@290109 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 207 insertions, 11 deletions

diff --git a/cc/output/delegating_renderer_unittest.cc b/cc/output/delegating_renderer_unittest.cc
index ac026ca..f8e4b69 100644
--- a/cc/output/delegating_renderer_unittest.cc
+++ b/cc/output/delegating_renderer_unittest.cc
@@ -126,13 +126,12 @@ class DelegatingRendererTestResources : public DelegatingRendererTest {
     ASSERT_TRUE(last_frame.delegated_frame_data);
 
     EXPECT_EQ(2u, last_frame.delegated_frame_data->render_pass_list.size());
-    // Each render pass has 10 resources in it. And the root render pass has a
+    // Each render pass has 11 resources in it. And the root render pass has a
     // mask resource used when drawing the child render pass, as well as its
-    // replica (it's added twice). The number 10 may change if
+    // replica (it's added twice). The number 11 may change if
     // AppendOneOfEveryQuadType() is updated, and the value here should be
     // updated accordingly.
-    EXPECT_EQ(
-        22u, last_frame.delegated_frame_data->resource_list.size());
+    EXPECT_EQ(24u, last_frame.delegated_frame_data->resource_list.size());
 
     EndTest();
   }
diff --git a/cc/output/direct_renderer.cc b/cc/output/direct_renderer.cc
index be5d2bf..8b7277b 100644
--- a/cc/output/direct_renderer.cc
+++ b/cc/output/direct_renderer.cc
@@ -205,6 +205,7 @@ void DirectRenderer::DrawFrame(RenderPassList* render_passes_in_draw_order,
   DCHECK(root_render_pass);
 
   DrawingFrame frame;
+  frame.render_passes_in_draw_order = render_passes_in_draw_order;
   frame.root_render_pass = root_render_pass;
   frame.root_damage_rect = Capabilities().using_partial_swap
                                ? root_render_pass->damage_rect
diff --git a/cc/output/direct_renderer.h b/cc/output/direct_renderer.h
index 1a84a87..442d371 100644
--- a/cc/output/direct_renderer.h
+++ b/cc/output/direct_renderer.h
@@ -40,6 +40,7 @@ class CC_EXPORT DirectRenderer : public Renderer {
     DrawingFrame();
     ~DrawingFrame();
 
+    const RenderPassList* render_passes_in_draw_order;
     const RenderPass* root_render_pass;
     const RenderPass* current_render_pass;
     const ScopedResource* current_texture;
diff --git a/cc/output/gl_renderer.cc b/cc/output/gl_renderer.cc
index 30fde1a..92af7bc 100644
--- a/cc/output/gl_renderer.cc
+++ b/cc/output/gl_renderer.cc
@@ -458,6 +458,13 @@ void GLRenderer::ClearFramebuffer(DrawingFrame* frame,
   }
 }
 
+static ResourceProvider::ResourceId WaitOnResourceSyncPoints(
+    ResourceProvider* resource_provider,
+    ResourceProvider::ResourceId resource_id) {
+  resource_provider->WaitSyncPointIfNeeded(resource_id);
+  return resource_id;
+}
+
 void GLRenderer::BeginDrawingFrame(DrawingFrame* frame) {
   if (frame->device_viewport_rect.IsEmpty())
     return;
@@ -492,6 +499,19 @@ void GLRenderer::BeginDrawingFrame(DrawingFrame* frame) {
   }
   resource_provider_->SetReadLockFence(read_lock_fence.get());
 
+  // Insert WaitSyncPointCHROMIUM on quad resources prior to drawing the frame,
+  // so that drawing can proceed without GL context switching interruptions.
+  DrawQuad::ResourceIteratorCallback wait_on_resource_syncpoints_callback =
+      base::Bind(&WaitOnResourceSyncPoints, resource_provider_);
+
+  for (size_t i = 0; i < frame->render_passes_in_draw_order->size(); ++i) {
+    RenderPass* pass = frame->render_passes_in_draw_order->at(i);
+    for (size_t j = 0; j < pass->quad_list.size(); ++j) {
+      DrawQuad* quad = pass->quad_list[j];
+      quad->IterateResources(wait_on_resource_syncpoints_callback);
+    }
+  }
+
   // TODO(enne): Do we need to reinitialize all of this state per frame?
   ReinitializeGLState();
 }
diff --git a/cc/output/gl_renderer_unittest.cc b/cc/output/gl_renderer_unittest.cc
index 8e33de0..6999c08 100644
--- a/cc/output/gl_renderer_unittest.cc
+++ b/cc/output/gl_renderer_unittest.cc
@@ -747,6 +747,7 @@ class TextureStateTrackingContext : public TestWebGraphicsContext3D {
     test_capabilities_.gpu.egl_image_external = true;
   }
 
+  MOCK_METHOD1(waitSyncPoint, void(unsigned sync_point));
   MOCK_METHOD3(texParameteri, void(GLenum target, GLenum pname, GLint param));
   MOCK_METHOD4(drawElements,
                void(GLenum mode, GLsizei count, GLenum type, GLintptr offset));
@@ -801,6 +802,12 @@ TEST_F(GLRendererTest, ActiveTextureState) {
   {
     InSequence sequence;
 
+    // The sync points for all quads are waited on first. This sync point is
+    // for a texture quad drawn later in the frame.
+    EXPECT_CALL(*context,
+                waitSyncPoint(TestRenderPass::kSyncPointForMailboxTextureQuad))
+        .Times(1);
+
     // yuv_quad is drawn with the default linear filter.
     EXPECT_CALL(*context, drawElements(_, _, _, _));
 
@@ -822,7 +829,7 @@ TEST_F(GLRendererTest, ActiveTextureState) {
 
     // The remaining quads also use GL_LINEAR because nearest neighbor
     // filtering is currently only used with tile quads.
-    EXPECT_CALL(*context, drawElements(_, _, _, _)).Times(6);
+    EXPECT_CALL(*context, drawElements(_, _, _, _)).Times(7);
   }
 
   gfx::Rect viewport_rect(100, 100);
diff --git a/cc/resources/resource_provider.cc b/cc/resources/resource_provider.cc
index 30d5849..fc31e00 100644
--- a/cc/resources/resource_provider.cc
+++ b/cc/resources/resource_provider.cc
@@ -1042,12 +1042,13 @@ const ResourceProvider::Resource* ResourceProvider::LockForRead(ResourceId id) {
   if (resource->type == GLTexture && !resource->gl_id) {
     DCHECK(resource->origin != Resource::Internal);
     DCHECK(resource->mailbox.IsTexture());
+
+    // Mailbox sync_points must be processed by a call to
+    // WaitSyncPointIfNeeded() prior to calling LockForRead().
+    DCHECK(!resource->mailbox.sync_point());
+
     GLES2Interface* gl = ContextGL();
     DCHECK(gl);
-    if (resource->mailbox.sync_point()) {
-      GLC(gl, gl->WaitSyncPointCHROMIUM(resource->mailbox.sync_point()));
-      resource->mailbox.set_sync_point(0);
-    }
     resource->gl_id = texture_id_allocator_->NextId();
     GLC(gl, gl->BindTexture(resource->target, resource->gl_id));
     GLC(gl,
@@ -2245,6 +2246,21 @@ void ResourceProvider::CopyResource(ResourceId source_id, ResourceId dest_id) {
   }
 }
 
+void ResourceProvider::WaitSyncPointIfNeeded(ResourceId id) {
+  Resource* resource = GetResource(id);
+  DCHECK_EQ(resource->exported_count, 0);
+  DCHECK(resource->allocated);
+  if (resource->type != GLTexture || resource->gl_id)
+    return;
+  if (!resource->mailbox.sync_point())
+    return;
+  DCHECK(resource->mailbox.IsValid());
+  GLES2Interface* gl = ContextGL();
+  DCHECK(gl);
+  GLC(gl, gl->WaitSyncPointCHROMIUM(resource->mailbox.sync_point()));
+  resource->mailbox.set_sync_point(0);
+}
+
 GLint ResourceProvider::GetActiveTextureUnit(GLES2Interface* gl) {
   GLint active_unit = 0;
   gl->GetIntegerv(GL_ACTIVE_TEXTURE, &active_unit);
diff --git a/cc/resources/resource_provider.h b/cc/resources/resource_provider.h
index bf1c33b..dfbff55 100644
--- a/cc/resources/resource_provider.h
+++ b/cc/resources/resource_provider.h
@@ -366,6 +366,8 @@ class CC_EXPORT ResourceProvider {
   // Copy pixels from source to destination.
   void CopyResource(ResourceId source_id, ResourceId dest_id);
 
+  void WaitSyncPointIfNeeded(ResourceId id);
+
   static GLint GetActiveTextureUnit(gpu::gles2::GLES2Interface* gl);
 
  private:
diff --git a/cc/resources/resource_provider_unittest.cc b/cc/resources/resource_provider_unittest.cc
index 9c47cb2..0d85648 100644
--- a/cc/resources/resource_provider_unittest.cc
+++ b/cc/resources/resource_provider_unittest.cc
@@ -339,6 +339,7 @@ void GetResourcePixels(ResourceProvider* resource_provider,
                        const gfx::Size& size,
                        ResourceFormat format,
                        uint8_t* pixels) {
+  resource_provider->WaitSyncPointIfNeeded(id);
   switch (resource_provider->default_resource_type()) {
     case ResourceProvider::GLTexture: {
       ResourceProvider::ScopedReadLockGL lock_gl(resource_provider, id);
@@ -664,6 +665,7 @@ TEST_P(ResourceProviderTest, TransferGLResources) {
     EXPECT_NE(list[0].mailbox_holder.sync_point,
               context3d_->last_waited_sync_point());
     {
+      resource_provider_->WaitSyncPointIfNeeded(list[0].id);
       ResourceProvider::ScopedReadLockGL lock(resource_provider_.get(),
                                               list[0].id);
     }
@@ -753,6 +755,7 @@ TEST_P(ResourceProviderTest, TransferGLResources) {
   EXPECT_FALSE(child_resource_provider_->InUseByConsumer(id4));
 
   {
+    child_resource_provider_->WaitSyncPointIfNeeded(id1);
     ResourceProvider::ScopedReadLockGL lock(child_resource_provider_.get(),
                                             id1);
     ASSERT_NE(0U, lock.texture_id());
@@ -761,6 +764,7 @@ TEST_P(ResourceProviderTest, TransferGLResources) {
     EXPECT_EQ(0, memcmp(data1, result, pixel_size));
   }
   {
+    child_resource_provider_->WaitSyncPointIfNeeded(id2);
     ResourceProvider::ScopedReadLockGL lock(child_resource_provider_.get(),
                                             id2);
     ASSERT_NE(0U, lock.texture_id());
@@ -769,6 +773,7 @@ TEST_P(ResourceProviderTest, TransferGLResources) {
     EXPECT_EQ(0, memcmp(data2, result, pixel_size));
   }
   {
+    child_resource_provider_->WaitSyncPointIfNeeded(id3);
     ResourceProvider::ScopedReadLockGL lock(child_resource_provider_.get(),
                                             id3);
     ASSERT_NE(0U, lock.texture_id());
@@ -854,6 +859,7 @@ TEST_P(ResourceProviderTest, ReadLockCountStopsReturnToChildOrDelete) {
 
     resource_provider_->ReceiveFromChild(child_id, list);
 
+    resource_provider_->WaitSyncPointIfNeeded(list[0].id);
     ResourceProvider::ScopedReadLockGL lock(resource_provider_.get(),
                                             list[0].id);
 
@@ -866,6 +872,7 @@ TEST_P(ResourceProviderTest, ReadLockCountStopsReturnToChildOrDelete) {
   child_resource_provider_->ReceiveReturnsFromParent(returned_to_child);
 
   {
+    child_resource_provider_->WaitSyncPointIfNeeded(id1);
     ResourceProvider::ScopedReadLockGL lock(child_resource_provider_.get(),
                                             id1);
     child_resource_provider_->DeleteResource(id1);
@@ -1742,6 +1749,7 @@ class ResourceProviderTestTextureFilters : public ResourceProviderTest {
       EXPECT_CALL(*parent_context, consumeTextureCHROMIUM(GL_TEXTURE_2D, _));
       parent_resource_provider->ReceiveFromChild(child_id, list);
       {
+        parent_resource_provider->WaitSyncPointIfNeeded(list[0].id);
         ResourceProvider::ScopedReadLockGL lock(parent_resource_provider.get(),
                                                 list[0].id);
       }
@@ -2518,9 +2526,13 @@ TEST_P(ResourceProviderTest, TextureMailbox_GLTexture2D) {
   Mock::VerifyAndClearExpectations(context);
 
   {
+    // Mailbox sync point WaitSyncPoint before using the texture.
+    EXPECT_CALL(*context, waitSyncPoint(sync_point));
+    resource_provider->WaitSyncPointIfNeeded(id);
+    Mock::VerifyAndClearExpectations(context);
+
     // Using the texture does a consume of the mailbox.
     EXPECT_CALL(*context, bindTexture(target, texture_id));
-    EXPECT_CALL(*context, waitSyncPoint(sync_point));
     EXPECT_CALL(*context, consumeTextureCHROMIUM(target, _));
 
     EXPECT_CALL(*context, insertSyncPoint()).Times(0);
@@ -2582,9 +2594,13 @@ TEST_P(ResourceProviderTest, TextureMailbox_GLTextureExternalOES) {
   Mock::VerifyAndClearExpectations(context);
 
   {
+    // Mailbox sync point WaitSyncPoint before using the texture.
+    EXPECT_CALL(*context, waitSyncPoint(sync_point));
+    resource_provider->WaitSyncPointIfNeeded(id);
+    Mock::VerifyAndClearExpectations(context);
+
     // Using the texture does a consume of the mailbox.
     EXPECT_CALL(*context, bindTexture(target, texture_id));
-    EXPECT_CALL(*context, waitSyncPoint(sync_point));
     EXPECT_CALL(*context, consumeTextureCHROMIUM(target, _));
 
     EXPECT_CALL(*context, insertSyncPoint()).Times(0);
@@ -2604,6 +2620,108 @@ TEST_P(ResourceProviderTest, TextureMailbox_GLTextureExternalOES) {
   }
 }
 
+TEST_P(ResourceProviderTest,
+       TextureMailbox_WaitSyncPointIfNeeded_WithSyncPoint) {
+  // Mailboxing is only supported for GL textures.
+  if (GetParam() != ResourceProvider::GLTexture)
+    return;
+
+  scoped_ptr<TextureStateTrackingContext> context_owned(
+      new TextureStateTrackingContext);
+  TextureStateTrackingContext* context = context_owned.get();
+
+  FakeOutputSurfaceClient output_surface_client;
+  scoped_ptr<OutputSurface> output_surface(FakeOutputSurface::Create3d(
+      context_owned.PassAs<TestWebGraphicsContext3D>()));
+  CHECK(output_surface->BindToClient(&output_surface_client));
+
+  scoped_ptr<ResourceProvider> resource_provider(ResourceProvider::Create(
+      output_surface.get(), shared_bitmap_manager_.get(), 0, false, 1, false));
+
+  uint32 sync_point = 30;
+  unsigned target = GL_TEXTURE_2D;
+
+  EXPECT_CALL(*context, bindTexture(_, _)).Times(0);
+  EXPECT_CALL(*context, waitSyncPoint(_)).Times(0);
+  EXPECT_CALL(*context, insertSyncPoint()).Times(0);
+  EXPECT_CALL(*context, produceTextureCHROMIUM(_, _)).Times(0);
+  EXPECT_CALL(*context, consumeTextureCHROMIUM(_, _)).Times(0);
+
+  gpu::Mailbox gpu_mailbox;
+  memcpy(gpu_mailbox.name, "Hello world", strlen("Hello world") + 1);
+  scoped_ptr<SingleReleaseCallback> callback =
+      SingleReleaseCallback::Create(base::Bind(&EmptyReleaseCallback));
+
+  TextureMailbox mailbox(gpu_mailbox, target, sync_point);
+
+  ResourceProvider::ResourceId id =
+      resource_provider->CreateResourceFromTextureMailbox(mailbox,
+                                                          callback.Pass());
+  EXPECT_NE(0u, id);
+
+  Mock::VerifyAndClearExpectations(context);
+
+  {
+    // First call to WaitSyncPointIfNeeded should call waitSyncPoint.
+    EXPECT_CALL(*context, waitSyncPoint(sync_point));
+    resource_provider->WaitSyncPointIfNeeded(id);
+    Mock::VerifyAndClearExpectations(context);
+
+    // Subsequent calls to WaitSyncPointIfNeeded shouldn't call waitSyncPoint.
+    EXPECT_CALL(*context, waitSyncPoint(_)).Times(0);
+    resource_provider->WaitSyncPointIfNeeded(id);
+    Mock::VerifyAndClearExpectations(context);
+  }
+}
+
+TEST_P(ResourceProviderTest, TextureMailbox_WaitSyncPointIfNeeded_NoSyncPoint) {
+  // Mailboxing is only supported for GL textures.
+  if (GetParam() != ResourceProvider::GLTexture)
+    return;
+
+  scoped_ptr<TextureStateTrackingContext> context_owned(
+      new TextureStateTrackingContext);
+  TextureStateTrackingContext* context = context_owned.get();
+
+  FakeOutputSurfaceClient output_surface_client;
+  scoped_ptr<OutputSurface> output_surface(FakeOutputSurface::Create3d(
+      context_owned.PassAs<TestWebGraphicsContext3D>()));
+  CHECK(output_surface->BindToClient(&output_surface_client));
+
+  scoped_ptr<ResourceProvider> resource_provider(ResourceProvider::Create(
+      output_surface.get(), shared_bitmap_manager_.get(), 0, false, 1, false));
+
+  uint32 sync_point = 0;
+  unsigned target = GL_TEXTURE_2D;
+
+  EXPECT_CALL(*context, bindTexture(_, _)).Times(0);
+  EXPECT_CALL(*context, waitSyncPoint(_)).Times(0);
+  EXPECT_CALL(*context, insertSyncPoint()).Times(0);
+  EXPECT_CALL(*context, produceTextureCHROMIUM(_, _)).Times(0);
+  EXPECT_CALL(*context, consumeTextureCHROMIUM(_, _)).Times(0);
+
+  gpu::Mailbox gpu_mailbox;
+  memcpy(gpu_mailbox.name, "Hello world", strlen("Hello world") + 1);
+  scoped_ptr<SingleReleaseCallback> callback =
+      SingleReleaseCallback::Create(base::Bind(&EmptyReleaseCallback));
+
+  TextureMailbox mailbox(gpu_mailbox, target, sync_point);
+
+  ResourceProvider::ResourceId id =
+      resource_provider->CreateResourceFromTextureMailbox(mailbox,
+                                                          callback.Pass());
+  EXPECT_NE(0u, id);
+
+  Mock::VerifyAndClearExpectations(context);
+
+  {
+    // WaitSyncPointIfNeeded with sync_point == 0 shouldn't call waitSyncPoint.
+    EXPECT_CALL(*context, waitSyncPoint(_)).Times(0);
+    resource_provider->WaitSyncPointIfNeeded(id);
+    Mock::VerifyAndClearExpectations(context);
+  }
+}
+
 class AllocationTrackingContext3D : public TestWebGraphicsContext3D {
  public:
   MOCK_METHOD0(NextTextureId, GLuint());
diff --git a/cc/test/render_pass_test_common.cc b/cc/test/render_pass_test_common.cc
index 8e81320..9b04370 100644
--- a/cc/test/render_pass_test_common.cc
+++ b/cc/test/render_pass_test_common.cc
@@ -4,6 +4,7 @@
 
 #include "cc/test/render_pass_test_common.h"
 
+#include "base/bind.h"
 #include "cc/quads/checkerboard_draw_quad.h"
 #include "cc/quads/debug_border_draw_quad.h"
 #include "cc/quads/io_surface_draw_quad.h"
@@ -19,6 +20,9 @@
 
 namespace cc {
 
+static void EmptyReleaseCallback(uint32 sync_point, bool lost_resource) {
+}
+
 void TestRenderPass::AppendOneOfEveryQuadType(
     ResourceProvider* resource_provider,
     RenderPass::Id child_pass) {
@@ -26,6 +30,7 @@ void TestRenderPass::AppendOneOfEveryQuadType(
   gfx::Rect opaque_rect(10, 10, 80, 80);
   gfx::Rect visible_rect(0, 0, 100, 100);
   const float vertex_opacity[] = {1.0f, 1.0f, 1.0f, 1.0f};
+
   ResourceProvider::ResourceId resource1 = resource_provider->CreateResource(
       gfx::Size(45, 5),
       GL_CLAMP_TO_EDGE,
@@ -69,6 +74,17 @@ void TestRenderPass::AppendOneOfEveryQuadType(
       resource_provider->best_texture_format());
   resource_provider->AllocateForTesting(resource7);
 
+  unsigned target = GL_TEXTURE_2D;
+  gpu::Mailbox gpu_mailbox;
+  memcpy(gpu_mailbox.name, "Hello world", strlen("Hello world") + 1);
+  scoped_ptr<SingleReleaseCallback> callback =
+      SingleReleaseCallback::Create(base::Bind(&EmptyReleaseCallback));
+  TextureMailbox mailbox(gpu_mailbox, target, kSyncPointForMailboxTextureQuad);
+  ResourceProvider::ResourceId resource8 =
+      resource_provider->CreateResourceFromTextureMailbox(mailbox,
+                                                          callback.Pass());
+  resource_provider->AllocateForTesting(resource8);
+
   SharedQuadState* shared_state = this->CreateAndAppendSharedQuadState();
   shared_state->SetAll(gfx::Transform(),
                        rect.size(),
@@ -151,6 +167,20 @@ void TestRenderPass::AppendOneOfEveryQuadType(
                        vertex_opacity,
                        false);
 
+  TextureDrawQuad* mailbox_texture_quad =
+      this->CreateAndAppendDrawQuad<TextureDrawQuad>();
+  mailbox_texture_quad->SetNew(shared_state,
+                               rect,
+                               opaque_rect,
+                               visible_rect,
+                               resource8,
+                               false,
+                               gfx::PointF(0.f, 0.f),
+                               gfx::PointF(1.f, 1.f),
+                               SK_ColorTRANSPARENT,
+                               vertex_opacity,
+                               false);
+
   TileDrawQuad* scaled_tile_quad =
       this->CreateAndAppendDrawQuad<TileDrawQuad>();
   scaled_tile_quad->SetNew(shared_state,
diff --git a/cc/test/render_pass_test_common.h b/cc/test/render_pass_test_common.h
index a7b65ef..c220dfd 100644
--- a/cc/test/render_pass_test_common.h
+++ b/cc/test/render_pass_test_common.h
@@ -19,6 +19,8 @@ class TestRenderPass : public RenderPass {
     return make_scoped_ptr(new TestRenderPass);
   }
 
+  static const unsigned int kSyncPointForMailboxTextureQuad = 30;
+
   void AppendOneOfEveryQuadType(ResourceProvider* resource_provider,
                                 RenderPass::Id child_pass);