Separate out resource mailbox creation and fix synchronization issue.

cc/resources/resource_provider.cc

 // Copyright 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 "cc/resources/resource_provider.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
@@ skipping 1132 matching lines @@
   ChildMap::const_iterator it = children_.find(child);
   DCHECK(it != children_.end());
   DCHECK(!it->second.marked_for_deletion);
   return it->second.child_to_parent_map;
 }
 
 void ResourceProvider::PrepareSendToParent(const ResourceIdArray& resources,
                                            TransferableResourceArray* list) {
   DCHECK(thread_checker_.CalledOnValidThread());
   GLES2Interface* gl = ContextGL();
-  bool need_sync_token = false;
 
-  gpu::SyncToken new_sync_token;
-  std::vector<size_t> unverified_token_indexes;
+  // Lazily create any mailboxes and verify all unverified sync tokens.
+  std::vector<GLbyte*> unverified_sync_tokens;
+  std::vector<ResourceIdArray::const_iterator> need_synchronization_indexes;
   for (ResourceIdArray::const_iterator it = resources.begin();
        it != resources.end();
        ++it) {
-    TransferableResource resource;
-    TransferResource(gl, *it, &resource);
-    need_sync_token |= (!resource.mailbox_holder.sync_token.HasData() &&
-                        !resource.is_software);
+    Resource* resource = GetResource(*it);
+    bool need_synchronization = CreateMailboxAndBindResource(gl, resource);
 
-    if (resource.mailbox_holder.sync_token.HasData() &&
-        !resource.mailbox_holder.sync_token.verified_flush()) {
-      unverified_token_indexes.push_back(list->size());
+    // TODO(dyen): Temporarily add missing sync tokens, eventually this should
+    // be removed as we guarantee all resources have associated sync tokens.
+    need_synchronization |= (resource->type != RESOURCE_TYPE_BITMAP &&
+                             !resource->mailbox.HasSyncToken());
+
+    if (output_surface_->capabilities().delegated_sync_points_required &&
+        need_synchronization) {
+      need_synchronization_indexes.push_back(it);

[David Yen, 2016/02/04 19:09:27]

From piman@:
We don't need to create sync tokens if delegated_sync_points_required is not
set. We could either skip the 2 code paths below, or I think simply not add it
to created_mailbox_indexes here.

The other option, I think, is to call CreateResourceMailbox from LazyCreate. I
don't think there's any point in delaying it any more - it used to require a
sync round-trip to the GPU process, so we needed to batch, but now it's all
client-side and a lot cheaper compared to everything else. Also with surfaces
used for the UI, there's very few cases left, if any, where we ever create a
resource that will not be sent to a parent, so I think delaying the mailbox
creation is a false optimization at this point.

[David Yen, 2016/02/04 19:13:29]

Done. Added delegated_sync_points_required check.

I'm fine with leaving the lazy mailbox creation here, because we still need to
synchronize the lazy texture binding code which was not previously synchronized.

+    } else if (resource->mailbox.HasSyncToken() &&
+               !resource->mailbox.sync_token().verified_flush()) {
+      unverified_sync_tokens.push_back(resource->mailbox.GetSyncTokenData());
     }
-
-    ++resources_.find(*it)->second.exported_count;
-    list->push_back(resource);
   }
 
-  // Fill out unverified sync tokens array.
-  std::vector<GLbyte*> unverified_sync_tokens;
-  unverified_sync_tokens.reserve(unverified_token_indexes.size() + 1);
-  for (auto it = unverified_token_indexes.begin();
-       it != unverified_token_indexes.end(); ++it) {
-    unverified_sync_tokens.push_back(
-        list->at(*it).mailbox_holder.sync_token.GetData());
-  }
-
-  if (need_sync_token &&
-      output_surface_->capabilities().delegated_sync_points_required) {
+  // Insert sync point to synchronize the mailbox creation or bound textures.
+  gpu::SyncToken new_sync_token;
+  if (!need_synchronization_indexes.empty()) {
     const uint64_t fence_sync = gl->InsertFenceSyncCHROMIUM();
     gl->OrderingBarrierCHROMIUM();
     gl->GenUnverifiedSyncTokenCHROMIUM(fence_sync, new_sync_token.GetData());
     unverified_sync_tokens.push_back(new_sync_token.GetData());
   }
 
   if (!unverified_sync_tokens.empty()) {
     gl->VerifySyncTokensCHROMIUM(unverified_sync_tokens.data(),
                                  unverified_sync_tokens.size());
   }
 
-  if (new_sync_token.HasData()) {
-    for (TransferableResourceArray::iterator it = list->begin();
-         it != list->end();
-         ++it) {
-      if (!it->mailbox_holder.sync_token.HasData())
-        it->mailbox_holder.sync_token = new_sync_token;
-    }
+  for (ResourceIdArray::const_iterator it : need_synchronization_indexes) {
+    GetResource(*it)->mailbox.set_sync_token(new_sync_token);
+  }
+
+  // Transfer Resources
+  for (ResourceIdArray::const_iterator it = resources.begin();
+       it != resources.end(); ++it) {
+    TransferableResource resource;
+    TransferResource(gl, *it, &resource);
+
+    DCHECK(!output_surface_->capabilities().delegated_sync_points_required ||
+           resource.mailbox_holder.sync_token.HasData() ||
+           resource.is_software);
+
+    ++resources_.find(*it)->second.exported_count;
+    list->push_back(resource);
   }
 }
 
 void ResourceProvider::ReceiveFromChild(
     int child, const TransferableResourceArray& resources) {
   DCHECK(thread_checker_.CalledOnValidThread());
   GLES2Interface* gl = ContextGL();
   Child& child_info = children_.find(child)->second;
   DCHECK(!child_info.marked_for_deletion);
   for (TransferableResourceArray::const_iterator it = resources.begin();
@@ skipping 114 matching lines @@
     resources_for_child[resource->child_id].push_back(local_id);
   }
 
   for (const auto& children : resources_for_child) {
     ChildMap::iterator child_it = children_.find(children.first);
     DCHECK(child_it != children_.end());
     DeleteAndReturnUnusedResourcesToChild(child_it, NORMAL, children.second);
   }
 }
 
+bool ResourceProvider::CreateMailboxAndBindResource(
+    gpu::gles2::GLES2Interface* gl, Resource* resource) {
+  if (resource->type != RESOURCE_TYPE_BITMAP) {
+    bool did_create_or_bind = false;
+    if (!resource->mailbox.IsValid()) {
+      LazyCreate(resource);
+
+      gpu::MailboxHolder mailbox_holder;
+      mailbox_holder.texture_target = resource->target;
+      gl->GenMailboxCHROMIUM(mailbox_holder.mailbox.name);
+      gl->ProduceTextureDirectCHROMIUM(resource->gl_id,
+                                       mailbox_holder.texture_target,
+                                       mailbox_holder.mailbox.name);
+      resource->mailbox = TextureMailbox(mailbox_holder);
+      did_create_or_bind = true;
+    }
+
+    if (resource->image_id && resource->dirty_image) {
+      DCHECK(resource->gl_id);
+      DCHECK(resource->origin == Resource::INTERNAL);
+      BindImageForSampling(resource);
+      did_create_or_bind = true;
+    }
+
+    return did_create_or_bind;
+  }
+  return false;
+}
+
 void ResourceProvider::TransferResource(GLES2Interface* gl,
                                         ResourceId id,
                                         TransferableResource* resource) {
   Resource* source = GetResource(id);
   DCHECK(!source->locked_for_write);
   DCHECK(!source->lock_for_read_count);
   DCHECK(source->origin != Resource::EXTERNAL || source->mailbox.IsValid());
   DCHECK(source->allocated);
   resource->id = id;
   resource->format = source->format;
   resource->mailbox_holder.texture_target = source->target;
   resource->filter = source->filter;
   resource->size = source->size;
   resource->read_lock_fences_enabled = source->read_lock_fences_enabled;
   resource->is_overlay_candidate = source->is_overlay_candidate;
 
   if (source->type == RESOURCE_TYPE_BITMAP) {
     resource->mailbox_holder.mailbox = source->shared_bitmap_id;
     resource->is_software = true;
-  } else if (!source->mailbox.IsValid()) {
-    LazyCreate(source);
-    DCHECK(source->gl_id);
-    DCHECK(source->origin == Resource::INTERNAL);
-    if (source->image_id && source->dirty_image)
-      BindImageForSampling(source);
-    // This is a resource allocated by the compositor, we need to produce it.
-    // Don't set a sync point, the caller will do it.
-    gl->GenMailboxCHROMIUM(resource->mailbox_holder.mailbox.name);
-    gl->ProduceTextureDirectCHROMIUM(source->gl_id,
-                                     resource->mailbox_holder.texture_target,
-                                     resource->mailbox_holder.mailbox.name);
-
-    source->mailbox = TextureMailbox(resource->mailbox_holder);
   } else {
+    DCHECK(source->mailbox.IsValid());
     DCHECK(source->mailbox.IsTexture());
-    if (source->image_id && source->dirty_image) {
-      DCHECK(source->gl_id);
-      DCHECK(source->origin == Resource::INTERNAL);
-      BindImageForSampling(source);
-    }
     // This is either an external resource, or a compositor resource that we
     // already exported. Make sure to forward the sync point that we were given.
     resource->mailbox_holder.mailbox = source->mailbox.mailbox();
     resource->mailbox_holder.texture_target = source->mailbox.target();
     resource->mailbox_holder.sync_token = source->mailbox.sync_token();
-    source->mailbox.set_sync_token(gpu::SyncToken());
   }
 }
 
 void ResourceProvider::DeleteAndReturnUnusedResourcesToChild(
     ChildMap::iterator child_it,
     DeleteStyle style,
     const ResourceIdArray& unused) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(child_it != children_.end());
   Child* child_info = &child_it->second;
@@ skipping 344 matching lines @@
 
     const int kImportance = 2;
     pmd->CreateSharedGlobalAllocatorDump(guid);
     pmd->AddOwnershipEdge(dump->guid(), guid, kImportance);
   }
 
   return true;
 }
 
 }  // namespace cc