From 1ca561b41b1c5172b081937d95add84155073bcd Mon Sep 17 00:00:00 2001
From: Ken Russell <kbr@chromium.org>
Date: Wed, 08 Jul 2026 16:58:51 -0700
Subject: [PATCH] GL: workaround: pack large row lengths separately.

Apply this workaround to Mali GPUs. Add a stress test verifying the
workaround, which is skipped on Metal, Vulkan, and Intel OpenGL for
the time being.

Co-authored with Gemini.

Fixed: chromium:529867799
Change-Id: I32e0fce82e44335598a4fc5016c9311fa45f4c76
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/8067312
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com>
Commit-Queue: Kenneth Russell <kbr@chromium.org>
---

diff --git a/include/platform/autogen/FeaturesGL_autogen.h b/include/platform/autogen/FeaturesGL_autogen.h
index a0f1e29..6b43b42 100644
--- a/include/platform/autogen/FeaturesGL_autogen.h
+++ b/include/platform/autogen/FeaturesGL_autogen.h
@@ -86,6 +86,12 @@
         &members,
     };
 
+    FeatureInfo packLargeRowLengthSeparatelyPackBuffer = {
+        "packLargeRowLengthSeparatelyPackBuffer",
+        FeatureCategory::OpenGLWorkarounds,
+        &members,
+    };
+
     FeatureInfo initializeCurrentVertexAttributes = {
         "initializeCurrentVertexAttributes",
         FeatureCategory::OpenGLWorkarounds,
diff --git a/include/platform/gl_features.json b/include/platform/gl_features.json
index 73fdf1d..79e0837 100644
--- a/include/platform/gl_features.json
+++ b/include/platform/gl_features.json
@@ -86,6 +86,15 @@
             ]
         },
         {
+            "name": "pack_large_row_length_separately_pack_buffer",
+            "category": "Workarounds",
+            "description": [
+                "When packing to a pixel pack buffer with a row pitch >= 256MiB, pack row by row.",
+                "Mali GLES computes stride in bits as int32; row_length*bpp*8 wraps at 0x10000000."
+            ],
+            "issue": "http://crbug.com/529867799"
+        },
+        {
             "name": "initialize_current_vertex_attributes",
             "category": "Workarounds",
             "description": [
diff --git a/src/libANGLE/renderer/gl/FramebufferGL.cpp b/src/libANGLE/renderer/gl/FramebufferGL.cpp
index 609e86b..abbf39c 100644
--- a/src/libANGLE/renderer/gl/FramebufferGL.cpp
+++ b/src/libANGLE/renderer/gl/FramebufferGL.cpp
@@ -792,22 +792,26 @@
         stateManager->getHasSeparateFramebufferBindings() ? GL_READ_FRAMEBUFFER : GL_FRAMEBUFFER;
     stateManager->bindFramebuffer(framebufferTarget, mFramebufferID);
 
+    const gl::InternalFormat &glFormat = gl::GetInternalFormatInfo(readFormat, readType);
+    GLuint rowBytes                    = 0;
+    ANGLE_CHECK_GL_MATH(contextGL,
+                        glFormat.computeRowPitch(readType, area.width, packState.alignment,
+                                                 packState.rowLength, &rowBytes));
+
     bool useOverlappingRowsWorkaround = features.packOverlappingRowsSeparatelyPackBuffer.enabled &&
                                         packBuffer && packState.rowLength != 0 &&
                                         packState.rowLength < clippedArea.width;
 
+    bool useLargeRowLengthWorkaround =
+        features.packLargeRowLengthSeparatelyPackBuffer.enabled && packBuffer &&
+        rowBytes >= 0x10000000u;  // Mali int32 stride-in-bits wrap threshold
+
     GLubyte *outPtr = static_cast<GLubyte *>(pixels);
     int leftClip    = clippedArea.x - area.x;
     int topClip     = clippedArea.y - area.y;
     if (leftClip || topClip)
     {
         // Adjust destination to match portion clipped off left and/or top.
-        const gl::InternalFormat &glFormat = gl::GetInternalFormatInfo(readFormat, readType);
-
-        GLuint rowBytes = 0;
-        ANGLE_CHECK_GL_MATH(contextGL,
-                            glFormat.computeRowPitch(readType, area.width, packState.alignment,
-                                                     packState.rowLength, &rowBytes));
         ANGLE_UNSAFE_TODO(outPtr += leftClip * glFormat.pixelBytes + topClip * rowBytes);
     }
 
@@ -825,7 +829,8 @@
     bool usePackSkipWorkaround = features.emulatePackSkipRowsAndPackSkipPixels.enabled &&
                                  (packState.skipRows != 0 || packState.skipPixels != 0);
 
-    if (cannotSetDesiredRowLength || useOverlappingRowsWorkaround || usePackSkipWorkaround)
+    if (cannotSetDesiredRowLength || useOverlappingRowsWorkaround || useLargeRowLengthWorkaround ||
+        usePackSkipWorkaround)
     {
         return readPixelsRowByRow(context, clippedArea, format, readFormat, readType, packState,
                                   outPtr);
diff --git a/src/libANGLE/renderer/gl/renderergl_utils.cpp b/src/libANGLE/renderer/gl/renderergl_utils.cpp
index 9d84d2e..92597d3 100644
--- a/src/libANGLE/renderer/gl/renderergl_utils.cpp
+++ b/src/libANGLE/renderer/gl/renderergl_utils.cpp
@@ -2403,6 +2403,13 @@
 
     ANGLE_FEATURE_CONDITION(features, unpackOverlappingRowsSeparatelyUnpackBuffer, isNvidia);
     ANGLE_FEATURE_CONDITION(features, packOverlappingRowsSeparatelyPackBuffer, isNvidia);
+    // Mali GLES computes readPixels row_stride as (int32_t)(row_length*bpp)*8;
+    // wraps negative when the byte pitch >= 0x10000000 -> OOB write into the
+    // PBO's cmem mapping. Route through readPixelsRowByRow so Mali only ever
+    // sees PACK_ROW_LENGTH=0. Also apply to Imagination GPUs which crash on
+    // the new test. crbug.com/529867799
+    ANGLE_FEATURE_CONDITION(features, packLargeRowLengthSeparatelyPackBuffer,
+                            isMali || IsPowerVR(vendor));
 
     std::array<int, 2> powerVRVersion = {0, 0};
     bool isPowerVRDriver =
diff --git a/src/tests/angle_end2end_tests_expectations.txt b/src/tests/angle_end2end_tests_expectations.txt
index 41fc454..9b93783 100644
--- a/src/tests/angle_end2end_tests_expectations.txt
+++ b/src/tests/angle_end2end_tests_expectations.txt
@@ -2783,6 +2783,10 @@
 // Unimplemented parts.
 515493666 D3D11 : Restart/DrawElementsVariantsTest.Draw/* = SKIP
 
+529867799 METAL : ReadPixelsPBOTest.PackLargeRowLength/* = SKIP
+529867799 VULKAN : ReadPixelsPBOTest.PackLargeRowLength/* = SKIP
+529867799 INTEL OPENGL : ReadPixelsPBOTest.PackLargeRowLength/* = SKIP
+
 // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
 // Slow tests, should appear last in this file
 // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
diff --git a/src/tests/gl_tests/ReadPixelsTest.cpp b/src/tests/gl_tests/ReadPixelsTest.cpp
index 217abca..16be449 100644
--- a/src/tests/gl_tests/ReadPixelsTest.cpp
+++ b/src/tests/gl_tests/ReadPixelsTest.cpp
@@ -775,6 +775,52 @@
     ASSERT_GL_NO_ERROR();
 }
 
+// Test that readPixels with a large PACK_ROW_LENGTH into a PBO does not overflow int32 stride
+// calculation. Ported from crbug.com/528175330 / crbug.com/529867799.
+TEST_P(ReadPixelsPBOTest, PackLargeRowLength)
+{
+    reset(16, 8, 8);
+
+    const GLColor kExpectedColor(65, 128, 192, 255);
+    constexpr GLint kLargeRowLength          = 0x7fffffc;
+    constexpr GLsizeiptr kByteOffsetToVerify = 0x1ffffff0;
+    constexpr GLsizeiptr kBufferSize         = kByteOffsetToVerify + 256;
+
+    glBindBuffer(GL_PIXEL_PACK_BUFFER, mPBO);
+    glBufferData(GL_PIXEL_PACK_BUFFER, kBufferSize, nullptr, GL_STREAM_READ);
+    ANGLE_SKIP_TEST_IF(glGetError() == GL_OUT_OF_MEMORY);
+    ASSERT_GL_NO_ERROR();
+
+    glClearColor(kExpectedColor.R / 255.0f, kExpectedColor.G / 255.0f, kExpectedColor.B / 255.0f,
+                 kExpectedColor.A / 255.0f);
+    glClear(GL_COLOR_BUFFER_BIT);
+
+    glPixelStorei(GL_PACK_ROW_LENGTH, kLargeRowLength);
+    glPixelStorei(GL_PACK_ALIGNMENT, 4);
+    glReadPixels(0, 0, 1, 2, GL_RGBA, GL_UNSIGNED_BYTE, nullptr);
+    EXPECT_GL_NO_ERROR();
+
+    void *mappedPtr = glMapBufferRange(GL_PIXEL_PACK_BUFFER, 0, kBufferSize, GL_MAP_READ_BIT);
+    ASSERT_NE(nullptr, mappedPtr);
+
+    const GLColor *colorPtr = static_cast<const GLColor *>(mappedPtr);
+    GLColor actualColorRow0;
+    GLColor actualColorRow1;
+
+    // Check row 0 pixel (at byte offset 0)
+    actualColorRow0 = colorPtr[0];
+
+    // Check row 1 pixel (at byte offset kByteOffsetToVerify)
+    constexpr size_t kRow1OffsetInPixels = kByteOffsetToVerify / sizeof(GLColor);
+    // SAFETY: test-only code.
+    ANGLE_UNSAFE_BUFFERS(actualColorRow1 = colorPtr[kRow1OffsetInPixels]);
+
+    glUnmapBuffer(GL_PIXEL_PACK_BUFFER);
+
+    EXPECT_EQ(kExpectedColor, actualColorRow0);
+    EXPECT_EQ(kExpectedColor, actualColorRow1);
+}
+
 class ReadPixelsPBODrawTest : public ReadPixelsPBOTest
 {
   protected:
diff --git a/util/autogen/angle_features_autogen.cpp b/util/autogen/angle_features_autogen.cpp
index cfcf801..0869671 100644
--- a/util/autogen/angle_features_autogen.cpp
+++ b/util/autogen/angle_features_autogen.cpp
@@ -261,6 +261,7 @@
     {Feature::MutableMipmapTextureUpload, "mutableMipmapTextureUpload"},
     {Feature::NoperspectiveInterpolationBrokenWithPassthroughShaders, "noperspectiveInterpolationBrokenWithPassthroughShaders"},

diff --git a/src/tests/angle_end2end_tests_expectations.txt b/src/tests/angle_end2end_tests_expectations.txt
index 41fc454..9b93783 100644
--- a/src/tests/angle_end2end_tests_expectations.txt
+++ b/src/tests/angle_end2end_tests_expectations.txt
@@ -2783,6 +2783,10 @@
 // Unimplemented parts.
 515493666 D3D11 : Restart/DrawElementsVariantsTest.Draw/* = SKIP
 
+529867799 METAL : ReadPixelsPBOTest.PackLargeRowLength/* = SKIP
+529867799 VULKAN : ReadPixelsPBOTest.PackLargeRowLength/* = SKIP
+529867799 INTEL OPENGL : ReadPixelsPBOTest.PackLargeRowLength/* = SKIP
+
 // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
 // Slow tests, should appear last in this file
 // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
diff --git a/src/tests/gl_tests/ReadPixelsTest.cpp b/src/tests/gl_tests/ReadPixelsTest.cpp
index 217abca..16be449 100644
--- a/src/tests/gl_tests/ReadPixelsTest.cpp
+++ b/src/tests/gl_tests/ReadPixelsTest.cpp
@@ -775,6 +775,52 @@
     ASSERT_GL_NO_ERROR();
 }
 
+// Test that readPixels with a large PACK_ROW_LENGTH into a PBO does not overflow int32 stride
+// calculation. Ported from crbug.com/528175330 / crbug.com/529867799.
+TEST_P(ReadPixelsPBOTest, PackLargeRowLength)
+{
+    reset(16, 8, 8);
+
+    const GLColor kExpectedColor(65, 128, 192, 255);
+    constexpr GLint kLargeRowLength          = 0x7fffffc;
+    constexpr GLsizeiptr kByteOffsetToVerify = 0x1ffffff0;
+    constexpr GLsizeiptr kBufferSize         = kByteOffsetToVerify + 256;
+
+    glBindBuffer(GL_PIXEL_PACK_BUFFER, mPBO);
+    glBufferData(GL_PIXEL_PACK_BUFFER, kBufferSize, nullptr, GL_STREAM_READ);
+    ANGLE_SKIP_TEST_IF(glGetError() == GL_OUT_OF_MEMORY);
+    ASSERT_GL_NO_ERROR();
+
+    glClearColor(kExpectedColor.R / 255.0f, kExpectedColor.G / 255.0f, kExpectedColor.B / 255.0f,
+                 kExpectedColor.A / 255.0f);
+    glClear(GL_COLOR_BUFFER_BIT);
+
+    glPixelStorei(GL_PACK_ROW_LENGTH, kLargeRowLength);
+    glPixelStorei(GL_PACK_ALIGNMENT, 4);
+    glReadPixels(0, 0, 1, 2, GL_RGBA, GL_UNSIGNED_BYTE, nullptr);
+    EXPECT_GL_NO_ERROR();
+
+    void *mappedPtr = glMapBufferRange(GL_PIXEL_PACK_BUFFER, 0, kBufferSize, GL_MAP_READ_BIT);
+    ASSERT_NE(nullptr, mappedPtr);
+
+    const GLColor *colorPtr = static_cast<const GLColor *>(mappedPtr);
+    GLColor actualColorRow0;
+    GLColor actualColorRow1;
+
+    // Check row 0 pixel (at byte offset 0)
+    actualColorRow0 = colorPtr[0];
+
+    // Check row 1 pixel (at byte offset kByteOffsetToVerify)
+    constexpr size_t kRow1OffsetInPixels = kByteOffsetToVerify / sizeof(GLColor);
+    // SAFETY: test-only code.
+    ANGLE_UNSAFE_BUFFERS(actualColorRow1 = colorPtr[kRow1OffsetInPixels]);
+
+    glUnmapBuffer(GL_PIXEL_PACK_BUFFER);
+
+    EXPECT_EQ(kExpectedColor, actualColorRow0);
+    EXPECT_EQ(kExpectedColor, actualColorRow1);
+}
+
 class ReadPixelsPBODrawTest : public ReadPixelsPBOTest
 {
   protected: