Command Buffer: read pixels into pixel pack buffer

gpu/command_buffer/service/gles2_cmd_decoder.cc

Index: gpu/command_buffer/service/gles2_cmd_decoder.cc
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index cc7dc5444b1b1693843552bee260bdec9c4362d5..1f973f2d0fb7ad4a3e88276cd1e819f79f4b11c9 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -8861,11 +8861,32 @@ error::Error GLES2DecoderImpl::HandleReadPixels(uint32 immediate_data_size,
       NULL, NULL)) {
     return error::kOutOfBounds;
   }
-  void* pixels = GetSharedMemoryAs<void*>(
-      c.pixels_shm_id, c.pixels_shm_offset, pixels_size);
-  if (!pixels) {
-    return error::kOutOfBounds;
+
+  void* pixels = NULL;
+  bool pixel_pack_buffer_binding =
+      GetClientId(buffer_manager(), state_.bound_pixel_pack_buffer.get());
+  if (c.pixels_shm_id == 0) {
+    if (pixel_pack_buffer_binding) {
+      pixels = reinterpret_cast<void *>(c.pixels_shm_offset);

[Zhenyao Mo, 2015/12/02 00:25:52]

We probably want to perform a bunch of extra validations here.

1) the bound buffer data store is not currently mapped
2) the bound buffer data store is large enough to write to

Although the underlying driver also does the validation, but this is potentially
out-of-bounds access, so probably we want to be on the safe side and perform
this validation ourselves.

piman: do you have an opinion on this?

[yunchao, 2015/12/02 09:36:55]

Done.

+    } else {
+      LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, "glReadPixels",
+          "no pixel pack buffer bound");
+      return error::kNoError;
+    }
+  } else {
+    if (pixel_pack_buffer_binding) {
+      LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, "glReadPixels",
+          "pixel pack buffer bound");
+      return error::kNoError;
+    } else {
+      pixels = GetSharedMemoryAs<void*>(
+          c.pixels_shm_id, c.pixels_shm_offset, pixels_size);
+      if (!pixels) {
+        return error::kOutOfBounds;
+      }
+    }
   }
+
   Result* result = NULL;
   if (c.result_shm_id != 0) {
     result = GetSharedMemoryAs<Result*>(
@@ -9000,7 +9021,8 @@ error::Error GLES2DecoderImpl::HandleReadPixels(uint32 immediate_data_size,
 
   ScopedResolvedFrameBufferBinder binder(this, false, true);
 
-  if (x < 0 || y < 0 || max_x > max_size.width() || max_y > max_size.height()) {
+  if ((x < 0 || y < 0 || max_x > max_size.width() || max_y > max_size.height())
+      && c.pixels_shm_id != 0) {
     // The user requested an out of range area. Get the results 1 line
     // at a time.
     uint32 temp_size;
@@ -9073,7 +9095,9 @@ error::Error GLES2DecoderImpl::HandleReadPixels(uint32 immediate_data_size,
     if (result != NULL) {
       *result = true;
     }
-    FinishReadPixels(c, 0);
+    if (c.pixels_shm_id != 0) {
+      FinishReadPixels(c, 0);
+    }
   }
 
   return error::kNoError;