Converted video frame and image callbacks to use new sync tokens.

gpu/command_buffer/service/in_process_command_buffer.cc

 // Copyright 2013 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 "gpu/command_buffer/service/in_process_command_buffer.h"
 
 #include <queue>
 #include <set>
 #include <utility>
 
@@ skipping 47 matching lines @@
 base::StaticAtomicSequenceNumber g_next_command_buffer_id;
 
 template <typename T>
 static void RunTaskWithResult(base::Callback<T(void)> task,
                               T* result,
                               base::WaitableEvent* completion) {
   *result = task.Run();
   completion->Signal();
 }
 
+struct ScopedOrderNumberProcessor {
+  ScopedOrderNumberProcessor(SyncPointOrderData* order_data, uint32_t order_num)
+      : order_data_(order_data), order_num_(order_num) {
+    order_data_->BeginProcessingOrderNumber(order_num_);
+  }
+
+  ~ScopedOrderNumberProcessor() {
+    order_data_->FinishProcessingOrderNumber(order_num_);
+  }
+
+ private:
+  SyncPointOrderData* order_data_;
+  uint32_t order_num_;
+};
+
 struct GpuInProcessThreadHolder {
   GpuInProcessThreadHolder()
       : sync_point_manager(new SyncPointManager(false)),
         gpu_thread(new GpuInProcessThread(sync_point_manager.get())) {}
   scoped_ptr<SyncPointManager> sync_point_manager;
   scoped_refptr<InProcessCommandBuffer::Service> gpu_thread;
 };
 
 base::LazyInstance<GpuInProcessThreadHolder> g_default_service =
     LAZY_INSTANCE_INITIALIZER;
@@ skipping 412 matching lines @@
   GetStateFast();
   return last_state_.token;
 }
 
 void InProcessCommandBuffer::FlushOnGpuThread(int32 put_offset,
                                               uint32_t order_num) {
   CheckSequencedThread();
   ScopedEvent handle_flush(&flush_event_);
   base::AutoLock lock(command_buffer_lock_);
 
-  sync_point_order_data_->BeginProcessingOrderNumber(order_num);
-  command_buffer_->Flush(put_offset);
   {
-    // Update state before signaling the flush event.
-    base::AutoLock lock(state_after_last_flush_lock_);
-    state_after_last_flush_ = command_buffer_->GetLastState();
+    ScopedOrderNumberProcessor scoped_order_num(sync_point_order_data_.get(),
+                                                order_num);
+    command_buffer_->Flush(put_offset);
+    {
+      // Update state before signaling the flush event.
+      base::AutoLock lock(state_after_last_flush_lock_);
+      state_after_last_flush_ = command_buffer_->GetLastState();
+    }
+    DCHECK((!error::IsError(state_after_last_flush_.error) && !context_lost_) ||
+           (error::IsError(state_after_last_flush_.error) && context_lost_));
+
+    // Currently the in process command buffer does not support being
+    // descheduled, if it does we would need to back off on calling the finish
+    // processing number function until the message is rescheduled and finished
+    // processing. This DCHECK is to enforce this.
+    DCHECK(context_lost_ || put_offset == state_after_last_flush_.get_offset);
   }
-  DCHECK((!error::IsError(state_after_last_flush_.error) && !context_lost_) ||
-         (error::IsError(state_after_last_flush_.error) && context_lost_));
-
-  // Currently the in process command buffer does not support being descheduled,
-  // if it does we would need to back off on calling the finish processing
-  // order number function until the message is rescheduled and finished
-  // processing. This DCHECK is to enforce this.
-  DCHECK(context_lost_ || put_offset == state_after_last_flush_.get_offset);
-  sync_point_order_data_->FinishProcessingOrderNumber(order_num);
 
   // If we've processed all pending commands but still have pending queries,
   // pump idle work until the query is passed.
   if (put_offset == state_after_last_flush_.get_offset &&
       (gpu_scheduler_->HasMoreIdleWork() ||
        gpu_scheduler_->HasPendingQueries())) {
     ScheduleDelayedWorkOnGpuThread();
   }
 }
 
@@ skipping 135 matching lines @@
       internalformat, gpu_memory_buffer->GetFormat()));
 
   // This handle is owned by the GPU thread and must be passed to it or it
   // will leak. In otherwords, do not early out on error between here and the
   // queuing of the CreateImage task below.
   bool requires_sync_point = false;
   gfx::GpuMemoryBufferHandle handle =
       ShareGpuMemoryBufferToGpuThread(gpu_memory_buffer->GetHandle(),
                                       &requires_sync_point);
 
+  SyncPointManager* sync_manager = service_->sync_point_manager();
+  const uint32_t order_num =
+      sync_point_order_data_->GenerateUnprocessedOrderNumber(sync_manager);
+
+  uint64_t fence_sync = 0;
+  if (requires_sync_point) {
+    fence_sync = GenerateFenceSyncRelease();
+
+    // Previous fence syncs should be flushed already.
+    DCHECK((fence_sync - 1) == flushed_fence_sync_release_);

[dcheng, 2015/11/04 19:22:43]

Nit: DCHECK_EQ

[David Yen, 2015/11/04 19:47:39]

Done.

+  }
+
   QueueTask(base::Bind(&InProcessCommandBuffer::CreateImageOnGpuThread,

[dcheng, 2015/11/04 19:22:43]

Can you throw in a comment about why base::Unretained() is safe here?

[David Yen, 2015/11/04 19:47:40]

This whole class is structured this way to pass messages to the gpu thread:

"function -> QueueTask(functionOnGpuThread, base::Unratained(this)...)"

Seems out of place to add a comment here when it's the pattern of the whole
file. Perhaps it can be added somewhere else? maybe above QueueTask()?

[dcheng, 2015/11/05 00:59:21]

It doesn't have to be this patch (as this appears to be a pre-existing pattern).
But anytime base::Unretained is used, it's hard for a reviewer like me (who
isn't an expert in GPU stuff) to figure out how/if this sort of thing is safe.

If it's something prevalent in a file, maybe document it in a comment block at
the top that details how the callbacks of this class work and why Unretained is
used? etc.

-                       base::Unretained(this),
-                       new_id,
-                       handle,
-                       gfx::Size(width, height),
-                       gpu_memory_buffer->GetFormat(),
-                       internalformat));
+                       base::Unretained(this), new_id, handle,
+                       gfx::Size(width, height), gpu_memory_buffer->GetFormat(),
+                       internalformat, order_num, fence_sync));
 
-  if (requires_sync_point) {
-    gpu_memory_buffer_manager_->SetDestructionSyncPoint(gpu_memory_buffer,
-                                                        InsertSyncPoint());
+  if (fence_sync) {
+    flushed_fence_sync_release_ = fence_sync;
+    SyncToken sync_token(GetNamespaceID(), GetCommandBufferID(), fence_sync);
+    sync_token.SetVerifyFlush();
+    gpu_memory_buffer_manager_->SetDestructionSyncToken(gpu_memory_buffer,
+                                                        sync_token);
   }
 
   return new_id;
 }
 
 void InProcessCommandBuffer::CreateImageOnGpuThread(
     int32 id,
     const gfx::GpuMemoryBufferHandle& handle,
     const gfx::Size& size,
     gfx::BufferFormat format,
-    uint32 internalformat) {
+    uint32 internalformat,
+    uint32_t order_num,
+    uint64_t fence_sync) {
+  ScopedOrderNumberProcessor scoped_order_num(sync_point_order_data_.get(),
+                                              order_num);
   if (!decoder_)
     return;
 
   gpu::gles2::ImageManager* image_manager = decoder_->GetImageManager();
   DCHECK(image_manager);
   if (image_manager->LookupImage(id)) {
     LOG(ERROR) << "Image already exists with same ID.";
     return;
   }
 
@@ skipping 23 matching lines @@
               handle, size, format, internalformat, kClientId);
       if (!image.get()) {
         LOG(ERROR) << "Failed to create image for buffer.";
         return;
       }
 
       image_manager->AddImage(image.get(), id);
       break;
     }
   }
+
+  if (fence_sync) {
+    sync_point_client_->ReleaseFenceSync(fence_sync);
+  }
 }
 
 void InProcessCommandBuffer::DestroyImage(int32 id) {
   CheckSequencedThread();
 
   QueueTask(base::Bind(&InProcessCommandBuffer::DestroyImageOnGpuThread,
                        base::Unretained(this),
                        id));
 }
 
@@ skipping 337 matching lines @@
     framebuffer_completeness_cache_ =
         new gpu::gles2::FramebufferCompletenessCache;
   return framebuffer_completeness_cache_;
 }
 
 SyncPointManager* GpuInProcessThread::sync_point_manager() {
   return sync_point_manager_;
 }
 
 }  // namespace gpu