Last CL to make ReadPixels ES3 conformant.

1) Make command buffer handle pack_skip_* params.
   Do not send them down to driver.
2) Add a workaround to handle ReadPixels into PBO and pack params do not
   work correctly.

BUG=563714
TEST=gpu_unittests,webgl2 conformance
R=piman@chromium.org

Review URL: https://codereview.chromium.org/1575013004

Cr-Commit-Position: refs/heads/master@{#369088}

11 files changed, 279 insertions, 62 deletions

diff --git a/gpu/command_buffer/client/gles2_implementation.cc b/gpu/command_buffer/client/gles2_implementation.cc
index 1ae946d..0f23569 100644
--- a/gpu/command_buffer/client/gles2_implementation.cc
+++ b/gpu/command_buffer/client/gles2_implementation.cc
@@ -3564,7 +3564,8 @@ void GLES2Implementation::ReadPixels(
                                            &size,
                                            &unpadded_row_size,
                                            &padded_row_size,
-                                           &skip_size)) {
+                                           &skip_size,
+                                           nullptr)) {
     SetGLError(GL_INVALID_VALUE, "glReadPixels", "size too large.");
     return;
   }
diff --git a/gpu/command_buffer/common/gles2_cmd_utils.cc b/gpu/command_buffer/common/gles2_cmd_utils.cc
index 86c0a7f..4c7fcf7 100644
--- a/gpu/command_buffer/common/gles2_cmd_utils.cc
+++ b/gpu/command_buffer/common/gles2_cmd_utils.cc
@@ -511,22 +511,30 @@ bool GLES2Util::ComputeImageRowSizeHelper(int width,
                                           uint32_t bytes_per_group,
                                           int alignment,
                                           uint32_t* rt_unpadded_row_size,
-                                          uint32_t* rt_padded_row_size) {
+                                          uint32_t* rt_padded_row_size,
+                                          uint32_t* rt_padding) {
   DCHECK(alignment == 1 || alignment == 2 ||
          alignment == 4 || alignment == 8);
   uint32_t unpadded_row_size;
   if (!SafeMultiplyUint32(width, bytes_per_group, &unpadded_row_size)) {
     return false;
   }
-  uint32_t temp;
-  if (!SafeAddUint32(unpadded_row_size, alignment - 1, &temp)) {
-    return false;
+  uint32_t residual = unpadded_row_size % alignment;
+  uint32_t padding = 0;
+  uint32_t padded_row_size = unpadded_row_size;
+  if (residual > 0) {
+    padding = alignment - residual;
+    if (!SafeAddUint32(unpadded_row_size, padding, &padded_row_size)) {
+      return false;
+    }
   }
-  uint32_t padded_row_size = (temp / alignment) * alignment;
+
   if (rt_unpadded_row_size)
     *rt_unpadded_row_size = unpadded_row_size;
   if (rt_padded_row_size)
     *rt_padded_row_size = padded_row_size;
+  if (rt_padding)
+    *rt_padding = padding;
   return true;
 }
 
@@ -537,7 +545,7 @@ bool GLES2Util::ComputeImagePaddedRowSize(int width,
                                           uint32_t* padded_row_size) {
   uint32_t bytes_per_group = ComputeImageGroupSize(format, type);
   return ComputeImageRowSizeHelper(
-      width, bytes_per_group, alignment, nullptr, padded_row_size);
+      width, bytes_per_group, alignment, nullptr, padded_row_size, nullptr);
 }
 
 // Returns the amount of data glTexImage*D or glTexSubImage*D will access.
@@ -554,14 +562,15 @@ bool GLES2Util::ComputeImageDataSizes(int width,
   params.alignment = alignment;
   return ComputeImageDataSizesES3(
       width, height, depth, format, type, params,
-      size, opt_unpadded_row_size, opt_padded_row_size, nullptr);
+      size, opt_unpadded_row_size, opt_padded_row_size, nullptr, nullptr);
 }
 
 bool GLES2Util::ComputeImageDataSizesES3(
     int width, int height, int depth, int format, int type,
     const PixelStoreParams& params,
     uint32_t* size, uint32_t* opt_unpadded_row_size,
-    uint32_t* opt_padded_row_size, uint32_t* opt_skip_size) {
+    uint32_t* opt_padded_row_size, uint32_t* opt_skip_size,
+    uint32_t* opt_padding) {
   DCHECK(width >= 0 && height >= 0 && depth >= 0);
 
   uint32_t bytes_per_group = ComputeImageGroupSize(format, type);
@@ -569,12 +578,14 @@ bool GLES2Util::ComputeImageDataSizesES3(
   uint32_t unpadded_row_size;
   uint32_t padded_row_size;
   if (!ComputeImageRowSizeHelper(width, bytes_per_group, params.alignment,
-                                 &unpadded_row_size, &padded_row_size)) {
+                                 &unpadded_row_size, &padded_row_size,
+                                 opt_padding)) {
     return false;
   }
   if (params.row_length > 0 &&
       !ComputeImageRowSizeHelper(params.row_length, bytes_per_group,
-                                 params.alignment, nullptr, &padded_row_size)) {
+                                 params.alignment, nullptr, &padded_row_size,
+                                 opt_padding)) {
     // Here we re-compute the padded_row_size, but the unpadded_row_size
     // isn't affected. That is, the last row isn't affected by ROW_LENGTH.
     return false;
diff --git a/gpu/command_buffer/common/gles2_cmd_utils.h b/gpu/command_buffer/common/gles2_cmd_utils.h
index ea79b2b..848abca 100644
--- a/gpu/command_buffer/common/gles2_cmd_utils.h
+++ b/gpu/command_buffer/common/gles2_cmd_utils.h
@@ -143,7 +143,8 @@ class GLES2_UTILS_EXPORT GLES2Util {
       int width, int height, int depth, int format, int type,
       const PixelStoreParams& params,
       uint32_t* size, uint32_t* opt_unpadded_row_size,
-      uint32_t* opt_padded_row_size, uint32_t* opt_skip_size);
+      uint32_t* opt_padded_row_size, uint32_t* opt_skip_size,
+      uint32_t* opt_padding);
 
   static size_t RenderbufferBytesPerPixel(int format);
 
@@ -224,7 +225,8 @@ class GLES2_UTILS_EXPORT GLES2Util {
                                         uint32_t bytes_per_group,
                                         int alignment,
                                         uint32_t* rt_unpadded_row_size,
-                                        uint32_t* rt_padded_row_size);
+                                        uint32_t* rt_padded_row_size,
+                                        uint32_t* rt_padding);
 
   static const EnumToString* const enum_to_string_table_;
   static const size_t enum_to_string_table_len_;
diff --git a/gpu/command_buffer/common/gles2_cmd_utils_unittest.cc b/gpu/command_buffer/common/gles2_cmd_utils_unittest.cc
index e2b0950..d1f731f 100644
--- a/gpu/command_buffer/common/gles2_cmd_utils_unittest.cc
+++ b/gpu/command_buffer/common/gles2_cmd_utils_unittest.cc
@@ -329,17 +329,19 @@ TEST_F(GLES2UtilTest, ComputeImageDataSizePixelStoreParams) {
   uint32_t unpadded_row_size;
   uint32_t padded_row_size;
   uint32_t skip_size;
+  uint32_t padding;
 
   {  // Default
     PixelStoreParams params;
     EXPECT_TRUE(GLES2Util::ComputeImageDataSizesES3(
         kWidth, kHeight, kDepth, GL_RGB, GL_UNSIGNED_BYTE, params,
-        &size, &unpadded_row_size, &padded_row_size, &skip_size));
+        &size, &unpadded_row_size, &padded_row_size, &skip_size, &padding));
     EXPECT_EQ(kWidth * 3, unpadded_row_size);
     EXPECT_EQ(kWidth * 3 + 3, padded_row_size);
     EXPECT_EQ(padded_row_size * (kHeight * kDepth - 1) + unpadded_row_size,
               size);
     EXPECT_EQ(0u, skip_size);
+    EXPECT_EQ(3u, padding);
   }
 
   {  // row_length > width
@@ -348,13 +350,14 @@ TEST_F(GLES2UtilTest, ComputeImageDataSizePixelStoreParams) {
     uint32_t kPadding = 1;  // 5 * 3 = 15 -> 16
     EXPECT_TRUE(GLES2Util::ComputeImageDataSizesES3(
         kWidth, kHeight, kDepth, GL_RGB, GL_UNSIGNED_BYTE, params,
-        &size, &unpadded_row_size, &padded_row_size, &skip_size));
+        &size, &unpadded_row_size, &padded_row_size, &skip_size, &padding));
     EXPECT_EQ(static_cast<uint32_t>(kWidth * 3), unpadded_row_size);
     EXPECT_EQ(static_cast<uint32_t>(params.row_length * 3 + kPadding),
               padded_row_size);
     EXPECT_EQ(padded_row_size * (kHeight * kDepth - 1) + unpadded_row_size,
               size);
     EXPECT_EQ(0u, skip_size);
+    EXPECT_EQ(kPadding, padding);
   }
 
   {  // row_length < width
@@ -363,13 +366,14 @@ TEST_F(GLES2UtilTest, ComputeImageDataSizePixelStoreParams) {
     uint32_t kPadding = 2;  // 2 * 3 = 6 -> 8
     EXPECT_TRUE(GLES2Util::ComputeImageDataSizesES3(
         kWidth, kHeight, kDepth, GL_RGB, GL_UNSIGNED_BYTE, params,
-        &size, &unpadded_row_size, &padded_row_size, &skip_size));
+        &size, &unpadded_row_size, &padded_row_size, &skip_size, &padding));
     EXPECT_EQ(static_cast<uint32_t>(kWidth * 3), unpadded_row_size);
     EXPECT_EQ(static_cast<uint32_t>(params.row_length * 3 + kPadding),
               padded_row_size);
     EXPECT_EQ(padded_row_size * (kHeight * kDepth - 1) + unpadded_row_size,
               size);
     EXPECT_EQ(0u, skip_size);
+    EXPECT_EQ(kPadding, padding);
   }
 
   {  // image_height > height
@@ -378,12 +382,13 @@ TEST_F(GLES2UtilTest, ComputeImageDataSizePixelStoreParams) {
     uint32_t kPadding = 3; // 3 * 3 = 9 -> 21
     EXPECT_TRUE(GLES2Util::ComputeImageDataSizesES3(
         kWidth, kHeight, kDepth, GL_RGB, GL_UNSIGNED_BYTE, params,
-        &size, &unpadded_row_size, &padded_row_size, &skip_size));
+        &size, &unpadded_row_size, &padded_row_size, &skip_size, &padding));
     EXPECT_EQ(kWidth * 3, unpadded_row_size);
     EXPECT_EQ(kWidth * 3 + kPadding, padded_row_size);
     EXPECT_EQ((params.image_height * (kDepth - 1) + kHeight - 1) *
               padded_row_size + unpadded_row_size, size);
     EXPECT_EQ(0u, skip_size);
+    EXPECT_EQ(kPadding, padding);
   }
 
   {  // image_height < height
@@ -392,12 +397,13 @@ TEST_F(GLES2UtilTest, ComputeImageDataSizePixelStoreParams) {
     uint32_t kPadding = 3; // 3 * 3 = 9 -> 12
     EXPECT_TRUE(GLES2Util::ComputeImageDataSizesES3(
         kWidth, kHeight, kDepth, GL_RGB, GL_UNSIGNED_BYTE, params,
-        &size, &unpadded_row_size, &padded_row_size, &skip_size));
+        &size, &unpadded_row_size, &padded_row_size, &skip_size, &padding));
     EXPECT_EQ(kWidth * 3, unpadded_row_size);
     EXPECT_EQ(kWidth * 3 + kPadding, padded_row_size);
     EXPECT_EQ((params.image_height * (kDepth - 1) + kHeight - 1) *
               padded_row_size + unpadded_row_size, size);
     EXPECT_EQ(0u, skip_size);
+    EXPECT_EQ(kPadding, padding);
   }
 
   {  // skip_pixels, skip_rows, skip_images, alignment = 4, RGB
@@ -408,7 +414,7 @@ TEST_F(GLES2UtilTest, ComputeImageDataSizePixelStoreParams) {
     uint32_t kPadding = 3; // 3 * 3 = 9 -> 12
     EXPECT_TRUE(GLES2Util::ComputeImageDataSizesES3(
         kWidth, kHeight, kDepth, GL_RGB, GL_UNSIGNED_BYTE, params,
-        &size, &unpadded_row_size, &padded_row_size, &skip_size));
+        &size, &unpadded_row_size, &padded_row_size, &skip_size, &padding));
     EXPECT_EQ(kWidth * 3, unpadded_row_size);
     EXPECT_EQ(kWidth * 3 + kPadding, padded_row_size);
     EXPECT_EQ(padded_row_size * kHeight * params.skip_images +
@@ -416,6 +422,7 @@ TEST_F(GLES2UtilTest, ComputeImageDataSizePixelStoreParams) {
               skip_size);
     EXPECT_EQ(padded_row_size * (kWidth * kDepth - 1) + unpadded_row_size,
               size);
+    EXPECT_EQ(kPadding, padding);
   }
 
   {  // skip_pixels, skip_rows, skip_images, alignment = 8, RGBA
@@ -427,7 +434,7 @@ TEST_F(GLES2UtilTest, ComputeImageDataSizePixelStoreParams) {
     uint32_t kPadding = 4; // 3 * 4 = 12 -> 16
     EXPECT_TRUE(GLES2Util::ComputeImageDataSizesES3(
         kWidth, kHeight, kDepth, GL_RGBA, GL_UNSIGNED_BYTE, params,
-        &size, &unpadded_row_size, &padded_row_size, &skip_size));
+        &size, &unpadded_row_size, &padded_row_size, &skip_size, &padding));
     EXPECT_EQ(kWidth * 4, unpadded_row_size);
     EXPECT_EQ(kWidth * 4 + kPadding, padded_row_size);
     EXPECT_EQ(padded_row_size * kHeight * params.skip_images +
@@ -435,6 +442,7 @@ TEST_F(GLES2UtilTest, ComputeImageDataSizePixelStoreParams) {
               skip_size);
     EXPECT_EQ(padded_row_size * (kWidth * kDepth - 1) + unpadded_row_size,
               size);
+    EXPECT_EQ(kPadding, padding);
   }
 }
 
diff --git a/gpu/command_buffer/service/context_state.cc b/gpu/command_buffer/service/context_state.cc
index 1a3a8c0..8b299dc 100644
--- a/gpu/command_buffer/service/context_state.cc
+++ b/gpu/command_buffer/service/context_state.cc
@@ -451,12 +451,8 @@ void ContextState::UpdatePackParameters() const {
     return;
   if (bound_pixel_pack_buffer.get()) {
     glPixelStorei(GL_PACK_ROW_LENGTH, pack_row_length);
-    glPixelStorei(GL_PACK_SKIP_PIXELS, pack_skip_pixels);
-    glPixelStorei(GL_PACK_SKIP_ROWS, pack_skip_rows);
   } else {
     glPixelStorei(GL_PACK_ROW_LENGTH, 0);
-    glPixelStorei(GL_PACK_SKIP_PIXELS, 0);
-    glPixelStorei(GL_PACK_SKIP_ROWS, 0);
   }
 }
 
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index 026e2a9..515a825 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -9130,17 +9130,19 @@ error::Error GLES2DecoderImpl::HandleReadPixels(uint32_t immediate_data_size,
   uint32_t unpadded_row_size = 0;
   uint32_t padded_row_size = 0;
   uint32_t skip_size = 0;
+  uint32_t padding = 0;
   if (!GLES2Util::ComputeImageDataSizesES3(width, height, 1,
                                            format, type,
                                            params,
                                            &pixels_size,
                                            &unpadded_row_size,
                                            &padded_row_size,
-                                           &skip_size)) {
+                                           &skip_size,
+                                           &padding)) {
     return error::kOutOfBounds;
   }
 
-  void* pixels = nullptr;
+  uint8_t* pixels = nullptr;
   Buffer* buffer = state_.bound_pixel_pack_buffer.get();
   if (pixels_shm_id == 0) {
     if (buffer) {
@@ -9150,7 +9152,7 @@ error::Error GLES2DecoderImpl::HandleReadPixels(uint32_t immediate_data_size,
         return error::kNoError;
       }
       uint32_t size = 0;
-      if (!SafeAddUint32(pixels_size, pixels_shm_offset, &size)) {
+      if (!SafeAddUint32(pixels_size + skip_size, pixels_shm_offset, &size)) {
         LOCAL_SET_GL_ERROR(
             GL_INVALID_VALUE, "glReadPixels", "size + offset overflow");
         return error::kNoError;
@@ -9160,7 +9162,8 @@ error::Error GLES2DecoderImpl::HandleReadPixels(uint32_t immediate_data_size,
             "pixel pack buffer is not large enough");
         return error::kNoError;
       }
-      pixels = reinterpret_cast<void *>(pixels_shm_offset);
+      pixels = reinterpret_cast<uint8_t *>(pixels_shm_offset);
+      pixels += skip_size;
     } else {
       return error::kInvalidArguments;
     }
@@ -9168,7 +9171,8 @@ error::Error GLES2DecoderImpl::HandleReadPixels(uint32_t immediate_data_size,
     if (buffer) {
       return error::kInvalidArguments;
     } else {
-      pixels = GetSharedMemoryAs<void*>(
+      DCHECK_EQ(0u, skip_size);
+      pixels = GetSharedMemoryAs<uint8_t*>(
           pixels_shm_id, pixels_shm_offset, pixels_size);
       if (!pixels) {
         return error::kOutOfBounds;
@@ -9318,27 +9322,17 @@ error::Error GLES2DecoderImpl::HandleReadPixels(uint32_t immediate_data_size,
   if (!max_rect.Contains(rect)) {
     rect.Intersect(max_rect);
     if (!rect.IsEmpty()) {
-      // TODO(yunchao): need to handle the out-of-bounds case for reading pixels
-      // into PIXEL_PACK buffer.
-      if (pixels_shm_id == 0) {
-        LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, "glReadPixels",
-            "read pixels out of bounds into PIXEL_PACK buffer");
-        return error::kNoError;
-      }
-
-      int8_t* dst = static_cast<int8_t*>(pixels);
       if (y < 0) {
-        uint32_t skip_rows_bytes = static_cast<uint32_t>(-y) * padded_row_size;
-        dst += skip_rows_bytes;
+        pixels += static_cast<uint32_t>(-y) * padded_row_size;;
       }
       if (x < 0) {
         uint32_t group_size = GLES2Util::ComputeImageGroupSize(format, type);
         uint32_t leading_bytes = static_cast<uint32_t>(-x) * group_size;
-        dst += leading_bytes;
+        pixels += leading_bytes;
       }
       for (GLint iy = rect.y(); iy < rect.bottom(); ++iy) {
-        glReadPixels(rect.x(), iy, rect.width(), 1, format, type, dst);
-        dst += padded_row_size;
+        glReadPixels(rect.x(), iy, rect.width(), 1, format, type, pixels);
+        pixels += padded_row_size;
       }
     }
   } else {
@@ -9373,7 +9367,34 @@ error::Error GLES2DecoderImpl::HandleReadPixels(uint32_t immediate_data_size,
         glDeleteBuffersARB(1, &buffer);
       }
     }
-    glReadPixels(x, y, width, height, format, type, pixels);
+    if (pixels_shm_id == 0 &&
+        workarounds().pack_parameters_workaround_with_pack_buffer) {
+      if (state_.pack_row_length > 0 && state_.pack_row_length < width) {
+        // Some drivers (for example, NVidia Linux) reset in this case.
+        for (GLint iy = y; iy < y + height; ++iy) {
+          // Need to set PACK_ALIGNMENT for last row. See comment below.
+          if (iy + 1 == y + height && padding > 0)
+            glPixelStorei(GL_PACK_ALIGNMENT, 1);
+          glReadPixels(x, iy, width, 1, format, type, pixels);
+          if (iy + 1 == y + height && padding > 0)
+            glPixelStorei(GL_PACK_ALIGNMENT, state_.pack_alignment);
+          pixels += padded_row_size;
+        }
+      } else if (padding > 0) {
+        // Some drivers (for example, NVidia Linux) incorrectly require the
+        // pack buffer to have padding for the last row.
+        if (height > 1)
+          glReadPixels(x, y, width, height - 1, format, type, pixels);
+        glPixelStorei(GL_PACK_ALIGNMENT, 1);
+        pixels += padded_row_size * (height - 1);
+        glReadPixels(x, y + height - 1, width, 1, format, type, pixels);
+        glPixelStorei(GL_PACK_ALIGNMENT, state_.pack_alignment);
+      } else {
+        glReadPixels(x, y, width, height, format, type, pixels);
+      }
+    } else {
+      glReadPixels(x, y, width, height, format, type, pixels);
+    }
   }
   if (pixels_shm_id != 0) {
     GLenum error = LOCAL_PEEK_GL_ERROR("glReadPixels");
@@ -9425,16 +9446,20 @@ error::Error GLES2DecoderImpl::HandlePixelStorei(uint32_t immediate_data_size,
     default:
       break;
   }
-  // For pack and unpack parameters (except for alignment), we don't apply them
-  // if no buffer is bound at PIXEL_PACK or PIXEL_UNPACK. We will handle pack
-  // and unpack according to the user specified parameters on the client side.
+  // For pack skip parameters, we don't apply them and handle them in command
+  // buffer.
+  // For alignment parameters, we always apply them.
+  // For other parameters, we don't apply them if no buffer is bound at
+  // PIXEL_PACK or PIXEL_UNPACK. We will handle pack and unpack according to
+  // the user specified parameters on the client side.
   switch (pname) {
     case GL_PACK_ROW_LENGTH:
-    case GL_PACK_SKIP_PIXELS:
-    case GL_PACK_SKIP_ROWS:
       if (state_.bound_pixel_pack_buffer.get())
         glPixelStorei(pname, param);
       break;
+    case GL_PACK_SKIP_PIXELS:
+    case GL_PACK_SKIP_ROWS:
+      break;
     case GL_UNPACK_ROW_LENGTH:
     case GL_UNPACK_IMAGE_HEIGHT:
     case GL_UNPACK_SKIP_PIXELS:
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.cc b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.cc
index 4c4e9f0..88a46bd 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.cc
@@ -841,12 +841,6 @@ void GLES2DecoderTestBase::DoBindBuffer(
     EXPECT_CALL(*gl_, PixelStorei(GL_PACK_ROW_LENGTH, _))
         .Times(1)
         .RetiresOnSaturation();
-    EXPECT_CALL(*gl_, PixelStorei(GL_PACK_SKIP_PIXELS, _))
-        .Times(1)
-        .RetiresOnSaturation();
-    EXPECT_CALL(*gl_, PixelStorei(GL_PACK_SKIP_ROWS, _))
-        .Times(1)
-        .RetiresOnSaturation();
   }
   cmds::BindBuffer cmd;
   cmd.Init(target, client_id);
@@ -1825,6 +1819,15 @@ void GLES2DecoderTestBase::DoScissor(GLint x,
   EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
 }
 
+void GLES2DecoderTestBase::DoPixelStorei(GLenum pname, GLint param) {
+  EXPECT_CALL(*gl_, PixelStorei(pname, param))
+      .Times(1)
+      .RetiresOnSaturation();
+  cmds::PixelStorei cmd;
+  cmd.Init(pname, param);
+  EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
+}
+
 void GLES2DecoderTestBase::SetupVertexBuffer() {
   DoEnableVertexAttribArray(1);
   DoBindBuffer(GL_ARRAY_BUFFER, client_buffer_id_, kServiceBufferId);
@@ -1929,12 +1932,6 @@ void GLES2DecoderTestBase::SetupInitStateManualExpectations(bool es3_capable) {
     EXPECT_CALL(*gl_, PixelStorei(GL_PACK_ROW_LENGTH, 0))
         .Times(1)
         .RetiresOnSaturation();
-    EXPECT_CALL(*gl_, PixelStorei(GL_PACK_SKIP_PIXELS, 0))
-        .Times(1)
-        .RetiresOnSaturation();
-    EXPECT_CALL(*gl_, PixelStorei(GL_PACK_SKIP_ROWS, 0))
-        .Times(1)
-        .RetiresOnSaturation();
     EXPECT_CALL(*gl_, PixelStorei(GL_UNPACK_ROW_LENGTH, 0))
         .Times(1)
         .RetiresOnSaturation();
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.h b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.h
index ff8c0ab..1e08254 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.h
+++ b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.h
@@ -356,6 +356,8 @@ class GLES2DecoderTestBase : public ::testing::TestWithParam<bool> {
 
   void DoScissor(GLint x, GLint y, GLsizei width, GLsizei height);
 
+  void DoPixelStorei(GLenum pname, GLint param);
+
   void SetupVertexBuffer();
   void SetupAllNeededVertexBuffers();
 
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_framebuffers.cc b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_framebuffers.cc
index 3383e26..ec0234d 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_framebuffers.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_framebuffers.cc
@@ -818,6 +818,166 @@ TEST_P(GLES3DecoderTest, ReadPixelsPixelPackBufferIsNotLargeEnough) {
   EXPECT_EQ(GL_INVALID_OPERATION, GetGLError());
 }
 
+TEST_P(GLES2DecoderManualInitTest, ReadPixels2RowLengthWorkaround) {
+  base::CommandLine command_line(0, NULL);
+  command_line.AppendSwitchASCII(
+      switches::kGpuDriverBugWorkarounds,
+      base::IntToString(gpu::PACK_PARAMETERS_WORKAROUND_WITH_PACK_BUFFER));
+  command_line.AppendSwitch(switches::kEnableUnsafeES3APIs);
+  InitState init;
+  init.gl_version = "OpenGL ES 3.0";
+  init.bind_generates_resource = true;
+  init.context_type = CONTEXT_TYPE_OPENGLES3;
+  InitDecoderWithCommandLine(init, &command_line);
+
+  const GLsizei kWidth = 5;
+  const GLsizei kHeight = 3;
+  const GLint kBytesPerPixel = 4;
+  const GLenum kFormat = GL_RGBA;
+  const GLenum kType = GL_UNSIGNED_BYTE;
+  const GLint kRowLength = 4;
+  GLint size = (kRowLength * (kHeight - 1) + kWidth) * kBytesPerPixel;
+
+  DoBindBuffer(GL_PIXEL_PACK_BUFFER, client_buffer_id_, kServiceBufferId);
+  DoBufferData(GL_PIXEL_PACK_BUFFER, size);
+
+  DoPixelStorei(GL_PACK_ROW_LENGTH, kRowLength);
+
+  EXPECT_CALL(*gl_, GetError())
+      .WillOnce(Return(GL_NO_ERROR))
+      .RetiresOnSaturation();
+  for (GLint ii = 0; ii < kHeight; ++ii) {
+    void* offset = reinterpret_cast<void*>(ii * kRowLength * kBytesPerPixel);
+    EXPECT_CALL(*gl_, ReadPixels(0, ii, kWidth, 1, kFormat, kType, offset))
+        .Times(1)
+        .RetiresOnSaturation();
+  }
+
+  ReadPixels cmd;
+  cmd.Init(0, 0, kWidth, kHeight,
+           kFormat, kType,
+           0, 0, 0, 0,
+           false);
+  EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
+  EXPECT_EQ(GL_NO_ERROR, GetGLError());
+}
+
+TEST_P(GLES2DecoderManualInitTest, ReadPixels2AlignmentWorkaround) {
+  base::CommandLine command_line(0, NULL);
+  command_line.AppendSwitchASCII(
+      switches::kGpuDriverBugWorkarounds,
+      base::IntToString(gpu::PACK_PARAMETERS_WORKAROUND_WITH_PACK_BUFFER));
+  command_line.AppendSwitch(switches::kEnableUnsafeES3APIs);
+  InitState init;
+  init.gl_version = "OpenGL ES 3.0";
+  init.bind_generates_resource = true;
+  init.context_type = CONTEXT_TYPE_OPENGLES3;
+  InitDecoderWithCommandLine(init, &command_line);
+
+  const GLsizei kWidth = 5;
+  const GLsizei kHeight = 3;
+  const GLint kBytesPerPixel = 4;
+  const GLenum kFormat = GL_RGBA;
+  const GLenum kType = GL_UNSIGNED_BYTE;
+  const GLint kAlignment = 8;
+  const GLint kPadding = 4;
+  GLint size = kWidth * kBytesPerPixel * kHeight + kPadding * (kHeight - 1);
+
+  DoBindBuffer(GL_PIXEL_PACK_BUFFER, client_buffer_id_, kServiceBufferId);
+  DoBufferData(GL_PIXEL_PACK_BUFFER, size);
+
+  DoPixelStorei(GL_PACK_ALIGNMENT, kAlignment);
+
+  EXPECT_CALL(*gl_, GetError())
+      .WillOnce(Return(GL_NO_ERROR))
+      .RetiresOnSaturation();
+  uint8_t* offset = reinterpret_cast<uint8_t*>(0);
+  EXPECT_CALL(*gl_,
+              ReadPixels(0, 0, kWidth, kHeight - 1, kFormat, kType, offset))
+      .Times(1)
+      .RetiresOnSaturation();
+  EXPECT_CALL(*gl_, PixelStorei(GL_PACK_ALIGNMENT, 1))
+      .Times(1)
+      .RetiresOnSaturation();
+  offset += (kWidth * kBytesPerPixel + kPadding) * (kHeight - 1);
+  EXPECT_CALL(*gl_,
+              ReadPixels(0, kHeight - 1, kWidth, 1, kFormat, kType, offset))
+      .Times(1)
+      .RetiresOnSaturation();
+  EXPECT_CALL(*gl_, PixelStorei(GL_PACK_ALIGNMENT, kAlignment))
+      .Times(1)
+      .RetiresOnSaturation();
+
+  ReadPixels cmd;
+  cmd.Init(0, 0, kWidth, kHeight,
+           kFormat, kType,
+           0, 0, 0, 0,
+           false);
+  EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
+  EXPECT_EQ(GL_NO_ERROR, GetGLError());
+}
+
+TEST_P(GLES2DecoderManualInitTest,
+       ReadPixels2RowLengthAndAlignmentWorkarounds) {
+  base::CommandLine command_line(0, NULL);
+  command_line.AppendSwitchASCII(
+      switches::kGpuDriverBugWorkarounds,
+      base::IntToString(gpu::PACK_PARAMETERS_WORKAROUND_WITH_PACK_BUFFER));
+  command_line.AppendSwitch(switches::kEnableUnsafeES3APIs);
+  InitState init;
+  init.gl_version = "OpenGL ES 3.0";
+  init.bind_generates_resource = true;
+  init.context_type = CONTEXT_TYPE_OPENGLES3;
+  InitDecoderWithCommandLine(init, &command_line);
+
+  const GLsizei kWidth = 5;
+  const GLsizei kHeight = 3;
+  const GLint kBytesPerPixel = 4;
+  const GLenum kFormat = GL_RGBA;
+  const GLenum kType = GL_UNSIGNED_BYTE;
+  const GLint kAlignment = 8;
+  const GLint kRowLength = 3;
+  const GLint kPadding = 4;
+  GLint padded_row_size = kRowLength * kBytesPerPixel + kPadding;
+  GLint unpadded_row_size = kWidth * kBytesPerPixel;
+  GLint size = padded_row_size * (kHeight - 1) + unpadded_row_size;
+
+  DoBindBuffer(GL_PIXEL_PACK_BUFFER, client_buffer_id_, kServiceBufferId);
+  DoBufferData(GL_PIXEL_PACK_BUFFER, size);
+
+  DoPixelStorei(GL_PACK_ALIGNMENT, kAlignment);
+  DoPixelStorei(GL_PACK_ROW_LENGTH, kRowLength);
+
+  EXPECT_CALL(*gl_, GetError())
+      .WillOnce(Return(GL_NO_ERROR))
+      .RetiresOnSaturation();
+  for (GLint ii = 0; ii < kHeight - 1; ++ii) {
+    void* offset = reinterpret_cast<void*>(ii * padded_row_size);
+    EXPECT_CALL(*gl_, ReadPixels(0, ii, kWidth, 1, kFormat, kType, offset))
+        .Times(1)
+        .RetiresOnSaturation();
+  }
+  EXPECT_CALL(*gl_, PixelStorei(GL_PACK_ALIGNMENT, 1))
+      .Times(1)
+      .RetiresOnSaturation();
+  void* offset = reinterpret_cast<void*>((kHeight - 1) * padded_row_size);
+  EXPECT_CALL(*gl_,
+              ReadPixels(0, kHeight - 1, kWidth, 1, kFormat, kType, offset))
+      .Times(1)
+      .RetiresOnSaturation();
+  EXPECT_CALL(*gl_, PixelStorei(GL_PACK_ALIGNMENT, kAlignment))
+      .Times(1)
+      .RetiresOnSaturation();
+
+  ReadPixels cmd;
+  cmd.Init(0, 0, kWidth, kHeight,
+           kFormat, kType,
+           0, 0, 0, 0,
+           false);
+  EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
+  EXPECT_EQ(GL_NO_ERROR, GetGLError());
+}
+
 TEST_P(GLES2DecoderRGBBackbufferTest, ReadPixelsNoAlphaBackbuffer) {
   const GLsizei kWidth = 3;
   const GLsizei kHeight = 3;
diff --git a/gpu/config/gpu_driver_bug_list_json.cc b/gpu/config/gpu_driver_bug_list_json.cc
index cff0b94..1f88d35 100644
--- a/gpu/config/gpu_driver_bug_list_json.cc
+++ b/gpu/config/gpu_driver_bug_list_json.cc
@@ -19,7 +19,7 @@ const char kGpuDriverBugListJson[] = LONG_STRING_CONST(
 {
   "name": "gpu driver bug list",
   // Please update the version number whenever you change this file.
-  "version": "8.38",
+  "version": "8.39",
   "entries": [
     {
       "id": 1,
@@ -1684,6 +1684,19 @@ LONG_STRING_CONST(
       "features": [
         "disable_discard_framebuffer"
       ]
+    },
+    {
+      "id": 142,
+      "cr_bugs": [563714],
+      "description": "Pack parameters work incorrectly with pack buffer bound",
+      "os": {
+        "type": "linux"
+      },
+      "vendor_id": "0x10de",
+      "gl_vendor": "NVIDIA.*",
+      "features": [
+        "pack_parameters_workaround_with_pack_buffer"
+      ]
     }
   ]
 }
diff --git a/gpu/config/gpu_driver_bug_workaround_type.h b/gpu/config/gpu_driver_bug_workaround_type.h
index 864627c..46cbb03 100644
--- a/gpu/config/gpu_driver_bug_workaround_type.h
+++ b/gpu/config/gpu_driver_bug_workaround_type.h
@@ -106,6 +106,8 @@
          needs_glsl_built_in_function_emulation)             \
   GPU_OP(NEEDS_OFFSCREEN_BUFFER_WORKAROUND,                  \
          needs_offscreen_buffer_workaround)                  \
+  GPU_OP(PACK_PARAMETERS_WORKAROUND_WITH_PACK_BUFFER,        \
+         pack_parameters_workaround_with_pack_buffer)        \
   GPU_OP(REGENERATE_STRUCT_NAMES,                            \
          regenerate_struct_names)                            \
   GPU_OP(REMOVE_POW_WITH_CONSTANT_EXPONENT,                  \