Perform glReadPixels with PBOs in the gpu, if PBOs are available.

gpu/command_buffer/service/query_manager.cc

Index: gpu/command_buffer/service/query_manager.cc
diff --git a/gpu/command_buffer/service/query_manager.cc b/gpu/command_buffer/service/query_manager.cc
index 3f81f64dccff8e1bc07fa28b571a30406a4c0f5e..69692fe69c5d2a64e417b690423b8843a963d203 100644
--- a/gpu/command_buffer/service/query_manager.cc
+++ b/gpu/command_buffer/service/query_manager.cc
@@ -195,6 +195,8 @@ class AsyncPixelTransfersCompletedQuery
     base::subtle::MemoryBarrier();
     sync->process_count = submit_count;
   }
+  void MarkAsCompletedStage1(
+      uint32 submit_count, const AsyncMemoryParams& mem_params);
 };
 
 AsyncPixelTransfersCompletedQuery::AsyncPixelTransfersCompletedQuery(
@@ -218,6 +220,17 @@ bool AsyncPixelTransfersCompletedQuery::End(uint32 submit_count) {
   mem_params.shm_data_offset = shm_offset();
   mem_params.shm_data_size = sizeof(QuerySync);
 
+  manager()->decoder()->WaitForReadPixels(
+      base::Bind(&AsyncPixelTransfersCompletedQuery::MarkAsCompletedStage1,
+                 this, submit_count, mem_params));

[piman, 2013/06/17 19:55:06]

The problem here is that for async texture upload, you will delay completion
significantly. For correctness on those, we only need to guarantee that the
command has been submitted to GL, not that it has completed on the GPU. It'll
add possibly a frame of latency to uploads, which means we may have more
checkerboarding or more jank when we wait on the completion to produce a frame.

You may want to separate the completion query for readpixels, or keep track here
if you're waiting on any readpixels or not.

[hubbe, 2013/06/25 20:02:51]

I changed this so that WaitForReadPixels doesn't automatically create a new
fence and callbacks are called immediately if no readpixels is actually pending
right now.

[piman, 2013/06/26 22:17:08]

So as I read it, and please explain to me if I missed something,
WaitForReadPixels will attach to the last fence inserted, whether it's within
the query or not. So in the case where you do an async readpixels followed by an
async upload, you'll still be waiting on the readpixels to be done and the fence
to be passed before returning the query for the async upload.

[hubbe, 2013/06/28 22:17:49]

Fixed by implementing a new query type.

+
+  return AddToPendingTransferQueue(submit_count);
+}
+
+void AsyncPixelTransfersCompletedQuery::MarkAsCompletedStage1(
+    uint32 submit_count,
+    const AsyncMemoryParams& mem_params) {
+
   // Ask AsyncPixelTransferDelegate to run completion callback after all
   // previous async transfers are done. No guarantee that callback is run
   // on the current thread.
@@ -226,7 +239,7 @@ bool AsyncPixelTransfersCompletedQuery::End(uint32 submit_count) {
       base::Bind(AsyncPixelTransfersCompletedQuery::MarkAsCompletedThreadSafe,
                  submit_count));
 
-  return AddToPendingTransferQueue(submit_count);
+
 }
 
 bool AsyncPixelTransfersCompletedQuery::Process() {
@@ -432,7 +445,21 @@ QueryManager::Query::Query(
   manager_->StartTracking(this);
 }
 
+void QueryManager::Query::RunCallbacks() {
+  for (size_t i = 0; i < callbacks_.size(); i++) {
+    callbacks_[i].Run();
+  }
+  callbacks_.clear();
+}
+
+void QueryManager::Query::AddCallback(base::Closure callback) {
+  callbacks_.push_back(callback);

[piman, 2013/06/17 19:55:06]

This should call the callback if the query is not pending any more.

[hubbe, 2013/06/25 20:02:51]

Done. (In other CL)

+}
+
 QueryManager::Query::~Query() {
+  // Seems like this channel is shutting down with some callbacks

[piman, 2013/06/17 19:55:06]

note: this could also happen if the query is destroyed explicitly.
I think it's still good to call the callback either way. It could be useful to
send a bool with the callback on whether the query has completed, or whether it
was destroyed - though arguably the client-side can know.

[hubbe, 2013/06/25 20:02:51]

Handled in other CL.

+  // still active. Call the queries now to avoid leaks.
+  RunCallbacks();
   if (manager_) {
     manager_->StopTracking(this);
     manager_ = NULL;
@@ -466,6 +493,7 @@ bool QueryManager::ProcessPendingQueries() {
     if (query->pending()) {
       break;
     }
+    query->RunCallbacks();
     pending_queries_.pop_front();
   }
 
@@ -485,6 +513,7 @@ bool QueryManager::ProcessPendingTransferQueries() {
     if (query->pending()) {
       break;
     }
+    query->RunCallbacks();
     pending_transfer_queries_.pop_front();
   }
 
@@ -562,5 +591,3 @@ bool QueryManager::EndQuery(Query* query, uint32 submit_count) {
 
 }  // namespace gles2
 }  // namespace gpu
-
-