Command Buffer: read pixels into pixel pack buffer

gpu/command_buffer/service/gles2_cmd_decoder.cc

 // Copyright (c) 2012 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/gles2_cmd_decoder.h"
 
 #include <stdio.h>
 
 #include <algorithm>
 #include <cmath>
@@ skipping 8843 matching lines @@
     LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, "glReadPixels", "dimensions < 0");
     return error::kNoError;
   }
   typedef cmds::ReadPixels::Result Result;
   uint32 pixels_size;
   if (!GLES2Util::ComputeImageDataSizes(
       width, height, 1, format, type, state_.pack_alignment, &pixels_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*>(
         c.result_shm_id, c.result_shm_offset, sizeof(*result));
     if (!result) {
       return error::kOutOfBounds;
     }
   }
 
   if (!validators_->read_pixel_format.IsValid(format)) {
@@ skipping 114 matching lines @@
   }
 
   if (!CheckBoundFramebuffersValid("glReadPixels")) {
     return error::kNoError;
   }
 
   LOCAL_COPY_REAL_GL_ERRORS_TO_WRAPPER("glReadPixels");
 
   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;
     uint32 unpadded_row_size;
     uint32 padded_row_size;
     if (!GLES2Util::ComputeImageDataSizes(
         width, 2, 1, format, type, state_.pack_alignment, &temp_size,
         &unpadded_row_size, &padded_row_size)) {
       LOCAL_SET_GL_ERROR(
           GL_INVALID_VALUE, "glReadPixels", "dimensions out of range");
@@ skipping 52 matching lines @@
         glDeleteBuffersARB(1, &buffer);
       }
     }
     glReadPixels(x, y, width, height, format, type, pixels);
   }
   GLenum error = LOCAL_PEEK_GL_ERROR("glReadPixels");
   if (error == GL_NO_ERROR) {
     if (result != NULL) {
       *result = true;
     }
-    FinishReadPixels(c, 0);
+    if (c.pixels_shm_id != 0) {
+      FinishReadPixels(c, 0);
+    }
   }
 
   return error::kNoError;
 }
 
 error::Error GLES2DecoderImpl::HandlePixelStorei(uint32 immediate_data_size,
                                                  const void* cmd_data) {
   const gles2::cmds::PixelStorei& c =
       *static_cast<const gles2::cmds::PixelStorei*>(cmd_data);
   GLenum pname = c.pname;
@@ skipping 6207 matching lines @@
   return error::kNoError;
 }
 
 // Include the auto-generated part of this file. We split this because it means
 // we can easily edit the non-auto generated parts right here in this file
 // instead of having to edit some template or the code generator.
 #include "gpu/command_buffer/service/gles2_cmd_decoder_autogen.h"
 
 }  // namespace gles2
 }  // namespace gpu