Fixes ReadPixel to have a result so the client

side can know if it should or should not copy
the results.

This is the way the spec is. If ReadPixels fails
it's not supposed to modify client side memory.

Unfortunately that makes it slow because it stalls
the GPU pipeline 3 times.

The service side has to  call glGetError to save up 
any current errors. That's one stall. Then it calls
ReadPixels, another stall, finally calls glGetError 
a 3rd time to see if  ReadPixels succeeded so that 
it can tell the client side implementation of
ReadPixels whether or not to copy memory.

We could decide to change the behavior of ReadPixels
so it always copies. That wouldn't be spec complient
but suspect most developers never expect it to fail
and don't count on it not always writing to their
memory.

TEST=none
BUG=none

Review URL: http://codereview.chromium.org/652213

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@39930 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 24 insertions, 8 deletions

diff --git a/gpu/command_buffer/client/gles2_cmd_helper_autogen.h b/gpu/command_buffer/client/gles2_cmd_helper_autogen.h
index 2b6185a..df3ed0e 100644
--- a/gpu/command_buffer/client/gles2_cmd_helper_autogen.h
+++ b/gpu/command_buffer/client/gles2_cmd_helper_autogen.h
@@ -667,10 +667,12 @@
 
   void ReadPixels(
       GLint x, GLint y, GLsizei width, GLsizei height, GLenum format,
-      GLenum type, uint32 pixels_shm_id, uint32 pixels_shm_offset) {
+      GLenum type, uint32 pixels_shm_id, uint32 pixels_shm_offset,
+      uint32 result_shm_id, uint32 result_shm_offset) {
     gles2::ReadPixels& c = GetCmdSpace<gles2::ReadPixels>();
     c.Init(
-        x, y, width, height, format, type, pixels_shm_id, pixels_shm_offset);
+        x, y, width, height, format, type, pixels_shm_id, pixels_shm_offset,
+        result_shm_id, result_shm_offset);
   }
 
   void RenderbufferStorage(
diff --git a/gpu/command_buffer/client/gles2_implementation.cc b/gpu/command_buffer/client/gles2_implementation.cc
index cc895fe..450e48d 100644
--- a/gpu/command_buffer/client/gles2_implementation.cc
+++ b/gpu/command_buffer/client/gles2_implementation.cc
@@ -495,6 +495,8 @@ void GLES2Implementation::ReadPixels(
   if (width == 0 || height == 0) {
     return;
   }
+  typedef gles2::ReadPixels::Result Result;
+  Result* result = static_cast<Result*>(result_buffer_);
   int8* dest = reinterpret_cast<int8*>(pixels);
   GLsizeiptr max_size = transfer_buffer_.GetLargestFreeOrPendingSize();
   GLsizeiptr unpadded_row_size = GLES2Util::ComputeImageDataSize(
@@ -508,12 +510,18 @@ void GLES2Implementation::ReadPixels(
       GLint num_rows = std::min(height, max_rows);
       GLsizeiptr part_size = num_rows * padded_row_size;
       void* buffer = transfer_buffer_.Alloc(part_size);
-      // TODO(gman): handle errors.
+      *result = 0;  // mark as failed.
       helper_->ReadPixels(
           xoffset, yoffset, width, num_rows, format, type,
-          transfer_buffer_id_, transfer_buffer_.GetOffset(buffer));
+          transfer_buffer_id_, transfer_buffer_.GetOffset(buffer),
+          result_shm_id(), result_shm_offset());
+      WaitForCmd();
+      // If it was not marked as successful exit.
+      if (*result == 0) {
+        return;
+      }
       memcpy(dest, buffer, part_size);
-      transfer_buffer_.FreePendingToken(buffer, helper_->InsertToken());
+      transfer_buffer_.Free(buffer);
       yoffset += num_rows;
       dest += part_size;
       height -= num_rows;
@@ -532,12 +540,18 @@ void GLES2Implementation::ReadPixels(
         GLint num_pixels = std::min(width, max_sub_row_pixels);
         GLsizeiptr part_size = num_pixels * element_size;
         void* buffer = transfer_buffer_.Alloc(part_size);
-        // TODO(gman): handle errors.
+        *result = 0;  // mark as failed.
         helper_->ReadPixels(
             temp_xoffset, yoffset, temp_width, 1, format, type,
-            transfer_buffer_id_, transfer_buffer_.GetOffset(buffer));
+            transfer_buffer_id_, transfer_buffer_.GetOffset(buffer),
+            result_shm_id(), result_shm_offset());
+        WaitForCmd();
+        // If it was not marked as successful exit.
+        if (*result == 0) {
+          return;
+        }
         memcpy(row_dest, buffer, part_size);
-        transfer_buffer_.FreePendingToken(buffer, helper_->InsertToken());
+        transfer_buffer_.Free(buffer);
         row_dest += part_size;
         temp_xoffset += num_pixels;
         temp_width -= num_pixels;