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, 25 insertions, 6 deletions

diff --git a/gpu/command_buffer/common/gles2_cmd_format_autogen.h b/gpu/command_buffer/common/gles2_cmd_format_autogen.h
index 57a8a4e..19824a4 100644
--- a/gpu/command_buffer/common/gles2_cmd_format_autogen.h
+++ b/gpu/command_buffer/common/gles2_cmd_format_autogen.h
@@ -4705,11 +4705,16 @@ COMPILE_ASSERT(offsetof(PolygonOffset, factor) == 4,
 COMPILE_ASSERT(offsetof(PolygonOffset, units) == 8,
                OffsetOf_PolygonOffset_units_not_8);
 
+// ReadPixels has the result separated from the pixel buffer so that
+// it is easier to specify the result going to some specific place
+// that exactly fits the rectangle of pixels.
 struct ReadPixels {
   typedef ReadPixels ValueType;
   static const CommandId kCmdId = kReadPixels;
   static const cmd::ArgFlags kArgFlags = cmd::kFixed;
 
+  typedef uint32 Result;
+
   static uint32 ComputeSize() {
     return static_cast<uint32>(sizeof(ValueType));  // NOLINT
   }
@@ -4720,7 +4725,8 @@ struct ReadPixels {
 
   void Init(
       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) {
     SetHeader();
     x = _x;
     y = _y;
@@ -4730,16 +4736,19 @@ struct ReadPixels {
     type = _type;
     pixels_shm_id = _pixels_shm_id;
     pixels_shm_offset = _pixels_shm_offset;
+    result_shm_id = _result_shm_id;
+    result_shm_offset = _result_shm_offset;
   }
 
   void* Set(
       void* cmd, GLint _x, GLint _y, GLsizei _width, GLsizei _height,
       GLenum _format, GLenum _type, uint32 _pixels_shm_id,
-      uint32 _pixels_shm_offset) {
+      uint32 _pixels_shm_offset, uint32 _result_shm_id,
+      uint32 _result_shm_offset) {
     static_cast<ValueType*>(
         cmd)->Init(
             _x, _y, _width, _height, _format, _type, _pixels_shm_id,
-            _pixels_shm_offset);
+            _pixels_shm_offset, _result_shm_id, _result_shm_offset);
     return NextCmdAddress<ValueType>(cmd);
   }
 
@@ -4752,10 +4761,12 @@ struct ReadPixels {
   uint32 type;
   uint32 pixels_shm_id;
   uint32 pixels_shm_offset;
+  uint32 result_shm_id;
+  uint32 result_shm_offset;
 };
 
-COMPILE_ASSERT(sizeof(ReadPixels) == 36,
-               Sizeof_ReadPixels_is_not_36);
+COMPILE_ASSERT(sizeof(ReadPixels) == 44,
+               Sizeof_ReadPixels_is_not_44);
 COMPILE_ASSERT(offsetof(ReadPixels, header) == 0,
                OffsetOf_ReadPixels_header_not_0);
 COMPILE_ASSERT(offsetof(ReadPixels, x) == 4,
@@ -4774,6 +4785,10 @@ COMPILE_ASSERT(offsetof(ReadPixels, pixels_shm_id) == 28,
                OffsetOf_ReadPixels_pixels_shm_id_not_28);
 COMPILE_ASSERT(offsetof(ReadPixels, pixels_shm_offset) == 32,
                OffsetOf_ReadPixels_pixels_shm_offset_not_32);
+COMPILE_ASSERT(offsetof(ReadPixels, result_shm_id) == 36,
+               OffsetOf_ReadPixels_result_shm_id_not_36);
+COMPILE_ASSERT(offsetof(ReadPixels, result_shm_offset) == 40,
+               OffsetOf_ReadPixels_result_shm_offset_not_40);
 
 struct RenderbufferStorage {
   typedef RenderbufferStorage ValueType;
diff --git a/gpu/command_buffer/common/gles2_cmd_format_test_autogen.h b/gpu/command_buffer/common/gles2_cmd_format_test_autogen.h
index 949cb77..381fea1 100644
--- a/gpu/command_buffer/common/gles2_cmd_format_test_autogen.h
+++ b/gpu/command_buffer/common/gles2_cmd_format_test_autogen.h
@@ -1754,7 +1754,9 @@ TEST(GLES2FormatTest, ReadPixels) {
       static_cast<GLenum>(15),
       static_cast<GLenum>(16),
       static_cast<uint32>(17),
-      static_cast<uint32>(18));
+      static_cast<uint32>(18),
+      static_cast<uint32>(19),
+      static_cast<uint32>(20));
   EXPECT_EQ(static_cast<uint32>(ReadPixels::kCmdId),
             cmd.header.command);
   EXPECT_EQ(sizeof(cmd), cmd.header.size * 4u);
@@ -1768,6 +1770,8 @@ TEST(GLES2FormatTest, ReadPixels) {
   EXPECT_EQ(static_cast<GLenum>(16), cmd.type);
   EXPECT_EQ(static_cast<uint32>(17), cmd.pixels_shm_id);
   EXPECT_EQ(static_cast<uint32>(18), cmd.pixels_shm_offset);
+  EXPECT_EQ(static_cast<uint32>(19), cmd.result_shm_id);
+  EXPECT_EQ(static_cast<uint32>(20), cmd.result_shm_offset);
 }
 
 TEST(GLES2FormatTest, RenderbufferStorage) {