Add a workaround for Mac ReadPixels.

BUG=563714 TEST=webgl2_conformance_test on mac R=bajones@chromium.org,piman@chromium.org NOTRY=true Review URL: https://codereview.chromium.org/1590373002 Cr-Commit-Position: refs/heads/master@{#369868}
marcosholgado · Jan 15, 2016 · b7997d0 · b7997d0
1 parent e44b02e
commit b7997d0
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 7 deletions.
diff --git a/content/test/gpu/gpu_tests/webgl2_conformance_expectations.py b/content/test/gpu/gpu_tests/webgl2_conformance_expectations.py
@@ -81,8 +81,6 @@ def SetExpectations(self):
     self.Fail('conformance2/glsl3/forbidden-operators.html', bug=483282)
     self.Fail('conformance2/extensions/promoted-extensions-in-shaders.html',
         bug=295792)
-    self.Skip('conformance2/reading/read-pixels-pack-parameters.html',
-        bug=483282)
     self.Fail('conformance2/samplers/sampler-drawing-test.html', bug=483282)
     self.Fail('conformance2/state/gl-object-get-calls.html', bug=483282)
     # Note that this test fails on ['win', 'intel'] with bug=483282
@@ -207,6 +205,8 @@ def SetExpectations(self):
         ['win'], bug=483282)
     self.Fail('conformance2/reading/read-pixels-from-fbo-test.html',
         ['win'], bug=483282)
+    self.Fail('conformance2/reading/read-pixels-pack-parameters.html',
+        ['win'], bug=483282)
     self.Fail('conformance2/textures/misc/gl-get-tex-parameter.html',
         ['win'], bug=483282)
     self.Fail('conformance2/textures/misc/tex-input-validation.html',

diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -9344,7 +9344,19 @@ error::Error GLES2DecoderImpl::HandleReadPixels(uint32_t immediate_data_size,
         pixels += leading_bytes;
       }
       for (GLint iy = rect.y(); iy < rect.bottom(); ++iy) {
+        bool reset_row_length = false;
+        if (iy + 1 == max_y && pixels_shm_id == 0 &&
+            workarounds().pack_parameters_workaround_with_pack_buffer &&
+            state_.pack_row_length > 0 && state_.pack_row_length < width) {
+          // Some drivers (for example, Mac AMD) incorrecly limit the last
+          // row to ROW_LENGTH in this case.
+          glPixelStorei(GL_PACK_ROW_LENGTH, 0);
+          reset_row_length = true;
+        }
         glReadPixels(rect.x(), iy, rect.width(), 1, format, type, pixels);
+        if (reset_row_length) {
+          glPixelStorei(GL_PACK_ROW_LENGTH, state_.pack_row_length);
+        }
         pixels += padded_row_size;
       }
     }
@@ -9384,23 +9396,27 @@ error::Error GLES2DecoderImpl::HandleReadPixels(uint32_t immediate_data_size,
         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) {
+        // Some drivers (for example, Mac AMD) incorrecly limit the last
+        // row to ROW_LENGTH in this case.
+        glPixelStorei(GL_PACK_ROW_LENGTH, 0);
+        for (GLint iy = y; iy < max_y; ++iy) {
           // Need to set PACK_ALIGNMENT for last row. See comment below.
-          if (iy + 1 == y + height && padding > 0)
+          if (iy + 1 == max_y && padding > 0)
             glPixelStorei(GL_PACK_ALIGNMENT, 1);
           glReadPixels(x, iy, width, 1, format, type, pixels);
-          if (iy + 1 == y + height && padding > 0)
+          if (iy + 1 == max_y && padding > 0)
             glPixelStorei(GL_PACK_ALIGNMENT, state_.pack_alignment);
           pixels += padded_row_size;
         }
+        glPixelStorei(GL_PACK_ROW_LENGTH, state_.pack_row_length);
       } 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);
+        glReadPixels(x, max_y - 1, width, 1, format, type, pixels);
         glPixelStorei(GL_PACK_ALIGNMENT, state_.pack_alignment);
       } else {
         glReadPixels(x, y, width, height, format, type, pixels);

diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_framebuffers.cc b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_framebuffers.cc
@@ -847,10 +847,20 @@ TEST_P(GLES2DecoderManualInitTest, ReadPixels2RowLengthWorkaround) {
       .WillOnce(Return(GL_NO_ERROR))
       .RetiresOnSaturation();
   for (GLint ii = 0; ii < kHeight; ++ii) {
+    if (ii + 1 == kHeight) {
+      EXPECT_CALL(*gl_, PixelStorei(GL_PACK_ROW_LENGTH, 0))
+          .Times(1)
+          .RetiresOnSaturation();
+    }
     void* offset = reinterpret_cast<void*>(ii * kRowLength * kBytesPerPixel);
     EXPECT_CALL(*gl_, ReadPixels(0, ii, kWidth, 1, kFormat, kType, offset))
         .Times(1)
         .RetiresOnSaturation();
+    if (ii + 1 == kHeight) {
+      EXPECT_CALL(*gl_, PixelStorei(GL_PACK_ROW_LENGTH, kRowLength))
+          .Times(1)
+          .RetiresOnSaturation();
+    }
   }
 
   ReadPixels cmd;
@@ -960,6 +970,9 @@ TEST_P(GLES2DecoderManualInitTest,
   EXPECT_CALL(*gl_, PixelStorei(GL_PACK_ALIGNMENT, 1))
       .Times(1)
       .RetiresOnSaturation();
+  EXPECT_CALL(*gl_, PixelStorei(GL_PACK_ROW_LENGTH, 0))
+      .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))
@@ -968,6 +981,9 @@ TEST_P(GLES2DecoderManualInitTest,
   EXPECT_CALL(*gl_, PixelStorei(GL_PACK_ALIGNMENT, kAlignment))
       .Times(1)
       .RetiresOnSaturation();
+  EXPECT_CALL(*gl_, PixelStorei(GL_PACK_ROW_LENGTH, kRowLength))
+      .Times(1)
+      .RetiresOnSaturation();
 
   ReadPixels cmd;
   cmd.Init(0, 0, kWidth, kHeight,

diff --git 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.40",
+  "version": "8.41",
   "entries": [
     {
       "id": 1,
@@ -1711,6 +1711,17 @@ LONG_STRING_CONST(
         "GL_ARB_timer_query",
         "GL_EXT_timer_query"
       ]
+    },
+    {
+      "id": 144,
+      "cr_bugs": [563714],
+      "description": "Pack parameters work incorrectly with pack buffer bound",
+      "os": {
+        "type": "macosx"
+      },
+      "features": [
+        "pack_parameters_workaround_with_pack_buffer"
+      ]
     }
   ]
 }