Improve WebGL 2.0 readPixels support.

include/platform/FeaturesGL.h

@@ -464,6 +464,11 @@ struct FeaturesGL : FeatureSetBase
         "disable_native_parallel_compile", FeatureCategory::OpenGLWorkarounds,
         "Do not use native KHR_parallel_shader_compile even when available.", &members,
         "http://crbug.com/1094869"};
+
+    Feature emulatePackSkipRowsAndPackSkipPixels = {
+        "emulate_pack_skip_rows_and_pack_skip_pixels", FeatureCategory::OpenGLWorkarounds,
+        "GL_PACK_SKIP_ROWS and GL_PACK_SKIP_PIXELS are ignored in Apple's OpenGL driver.", &members,
+        "https://anglebug.com/4849"};
 };
 
 inline FeaturesGL::FeaturesGL()  = default;

src/libANGLE/ErrorStrings.h

@@ -252,6 +252,7 @@ MSG kInvalidMultisampledFramebufferOperation = "Invalid operation on multisample
 MSG kInvalidMultitextureUnit = "Specified unit must be in [GL_TEXTURE0, GL_TEXTURE0 + GL_MAX_TEXTURE_UNITS)";
 MSG kInvalidName = "Invalid name.";
 MSG kInvalidNameCharacters = "Name contains invalid characters.";
+MSG kInvalidPackParametersForWebGL = "Invalid combination of pack parameters for WebGL.";
 MSG kInvalidPname = "Invalid pname.";
 MSG kInvalidPointerQuery = "Invalid pointer query.";
 MSG kInvalidPointParameter = "Invalid point parameter.";
@@ -319,6 +320,7 @@ MSG kInvalidType = "Invalid type.";
 MSG kInvalidUniformCount = "Only array uniforms may have count > 1.";
 MSG kInvalidUniformLocation = "Invalid uniform location";
 MSG kInvalidUnpackAlignment = "Unpack alignment must be 1, 2, 4 or 8.";
+MSG kInvalidUnpackParametersForWebGL = "Invalid combination of unpack parameters for WebGL.";
 MSG kInvalidVaryingLocation = "Location exceeds max varying.";
 MSG kInvalidVertexArray = "Vertex array does not exist.";
 MSG kInvalidVertexArrayName = "name is not a valid vertex array.";

src/libANGLE/renderer/gl/FramebufferGL.cpp

@@ -681,7 +681,10 @@ angle::Result FramebufferGL::readPixels(const gl::Context *context,
     bool cannotSetDesiredRowLength =
         packState.rowLength && !GetImplAs<ContextGL>(context)->getNativeExtensions().packSubimage;
 
-    if (cannotSetDesiredRowLength || useOverlappingRowsWorkaround)
+    bool usePackSkipWorkaround = features.emulatePackSkipRowsAndPackSkipPixels.enabled &&
+                                 (packState.skipRows != 0 || packState.skipPixels != 0);
+
+    if (cannotSetDesiredRowLength || useOverlappingRowsWorkaround || usePackSkipWorkaround)
     {
         return readPixelsRowByRow(context, clippedArea, format, readFormat, readType, packState,
                                   outPtr);

src/libANGLE/renderer/gl/renderergl_utils.cpp

@@ -1784,6 +1784,13 @@ void InitializeFeatures(const FunctionsGL *functions, angle::FeaturesGL *feature
 #endif
     ANGLE_FEATURE_CONDITION(features, disableNativeParallelCompile,
                             isTSANBuild && IsLinux() && isNvidia);
+
+    // anglebug.com/4849
+    // This workaround is definitely needed on Intel and AMD GPUs. To
+    // determine whether it's needed on iOS and Apple Silicon, the
+    // workaround's being restricted to existing desktop GPUs.
+    ANGLE_FEATURE_CONDITION(features, emulatePackSkipRowsAndPackSkipPixels,
+                            IsApple() && (isAMD || isIntel || isNvidia));
 }
 
 void InitializeFrontendFeatures(const FunctionsGL *functions, angle::FrontendFeatures *features)

src/libANGLE/validationES.cpp

@@ -5737,6 +5737,22 @@ bool ValidatePixelPack(const Context *context,
         *length = static_cast<GLsizei>(endByte);
     }
 
+    if (context->getExtensions().webglCompatibility)
+    {
+        // WebGL 2.0 disallows the scenario:
+        //   GL_PACK_SKIP_PIXELS + width > DataStoreWidth
+        // where:
+        //   DataStoreWidth = (GL_PACK_ROW_LENGTH ? GL_PACK_ROW_LENGTH : width)
+        // Since these two pack parameters can only be set to non-zero values
+        // on WebGL 2.0 contexts, verify them for all WebGL contexts.
+        GLint dataStoreWidth = pack.rowLength ? pack.rowLength : width;
+        if (pack.skipPixels + width > dataStoreWidth)
+        {
+            context->validationError(GL_INVALID_OPERATION, kInvalidPackParametersForWebGL);
+            return false;
+        }
+    }
+
     return true;
 }
 

src/libANGLE/validationES3.cpp

@@ -698,6 +698,50 @@ bool ValidateES3TexImageParametersBase(const Context *context,
         }
     }
 
+    if (context->getExtensions().webglCompatibility)
+    {
+        // Define:
+        //   DataStoreWidth  = (GL_UNPACK_ROW_LENGTH ? GL_UNPACK_ROW_LENGTH : width)
+        //   DataStoreHeight = (GL_UNPACK_IMAGE_HEIGHT ? GL_UNPACK_IMAGE_HEIGHT : height)
+        //
+        // WebGL 2.0 imposes the following additional constraints:
+        //
+        // 1) texImage2D and texSubImage2D generate INVALID_OPERATION if:
+        //      GL_UNPACK_SKIP_PIXELS + width > DataStoreWidth
+        //    except for texImage2D if no GL_PIXEL_UNPACK_BUFFER is
+        //    bound and _pixels_ is null.
+        //
+        // 2) texImage3D and texSubImage3D generate INVALID_OPERATION if:
+        //      GL_UNPACK_SKIP_PIXELS + width > DataStoreWidth
+        //      GL_UNPACK_SKIP_ROWS + height > DataStoreHeight
+        //    except for texImage3D if no GL_PIXEL_UNPACK_BUFFER is
+        //    bound and _pixels_ is null.
+        if (!pixelUnpackBuffer && !pixels && !isSubImage)
+        {
+            // Exception case for texImage2D or texImage3D, above.
+        }
+        else
+        {
+            const auto &unpack   = context->getState().getUnpackState();
+            GLint dataStoreWidth = unpack.rowLength ? unpack.rowLength : width;
+            if (unpack.skipPixels + width > dataStoreWidth)
+            {
+                context->validationError(GL_INVALID_OPERATION, kInvalidUnpackParametersForWebGL);
+                return false;
+            }
+            if (target == TextureTarget::_3D || target == TextureTarget::_2DArray)
+            {
+                GLint dataStoreHeight = unpack.imageHeight ? unpack.imageHeight : height;
+                if (unpack.skipRows + height > dataStoreHeight)
+                {
+                    context->validationError(GL_INVALID_OPERATION,
+                                             kInvalidUnpackParametersForWebGL);
+                    return false;
+                }
+            }
+        }
+    }
+
     return true;
 }