Workaround EXT_texture_norm16 for OpenGL ES drivers

Implement a workaround for widespread bugs calling glReadPixels with
RGBA/UNSIGNED_SHORT against R16/RG16 color attachments. Read back the
data using the GL_IMPLEMENTATION_COLOR_READ_FORMAT, and then rearrange
the read back pixels to fit the RGBA layout.

Also skip RGB16/RGB16_SNORM texture sample test on Nexus 5X/Nexus 6P
due to a another driver bug.

Bug: chromium:1000354, angleproject:4214, angleproject:4215, angleproject:4245
Change-Id: Iedea6f4136878cac5ad0dec3757c77b73502e1cd
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/1952166
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: Shrek Shao <shrekshao@google.com>

include/platform/FeaturesGL.h

@@ -386,6 +386,14 @@ struct FeaturesGL : FeatureSetBase
         "All Mac drivers do not handle struct scopes correctly. This workaround overwrites a struct"
         "name with a unique prefix.",
         &members, "http://crbug.com/403957"};
+
+    // Quite some OpenGL ES drivers don't implement readPixels for RGBA/UNSIGNED_SHORT from
+    // EXT_texture_norm16 correctly
+    Feature readPixelsUsingImplementationColorReadFormatForNorm16 = {
+        "read_pixels_using_implementation_color_read_format", FeatureCategory::OpenGLWorkarounds,
+        "Quite some OpenGL ES drivers don't implement readPixels for RGBA/UNSIGNED_SHORT from "
+        "EXT_texture_norm16 correctly",
+        &members, "http://anglebug.com/4214"};
 };
 
 inline FeaturesGL::FeaturesGL()  = default;

src/libANGLE/formatutils.cpp

@@ -992,8 +992,10 @@ static InternalFormatInfoMap BuildInternalFormatInfoMap()
     //                 | Internal format  |sized | R | G | B | A |S | Format           | Type                          | Component type        | SRGB | Texture supported                               | Filterable     | Texture attachment                            | Renderbuffer |
     AddRGBAFormat(&map, GL_RED,            false,  8,  0,  0,  0, 0, GL_RED,            GL_UNSIGNED_BYTE,               GL_UNSIGNED_NORMALIZED, false, RequireExt<&Extensions::textureRG>,               AlwaysSupported, RequireExt<&Extensions::textureRG>,             NeverSupported);
     AddRGBAFormat(&map, GL_RED,            false,  8,  0,  0,  0, 0, GL_RED,            GL_BYTE,                        GL_SIGNED_NORMALIZED,   false, NeverSupported,                                   NeverSupported,  NeverSupported,                                 NeverSupported);
+    AddRGBAFormat(&map, GL_RED,            false, 16,  0,  0,  0, 0, GL_RED,            GL_UNSIGNED_SHORT,              GL_UNSIGNED_NORMALIZED, false, RequireExt<&Extensions::textureNorm16>,           AlwaysSupported, RequireExt<&Extensions::textureNorm16>,         NeverSupported);
     AddRGBAFormat(&map, GL_RG,             false,  8,  8,  0,  0, 0, GL_RG,             GL_UNSIGNED_BYTE,               GL_UNSIGNED_NORMALIZED, false, RequireExt<&Extensions::textureRG>,               AlwaysSupported, RequireExt<&Extensions::textureRG>,             NeverSupported);
     AddRGBAFormat(&map, GL_RG,             false,  8,  8,  0,  0, 0, GL_RG,             GL_BYTE,                        GL_SIGNED_NORMALIZED,   false, NeverSupported,                                   NeverSupported,  NeverSupported,                                 NeverSupported);
+    AddRGBAFormat(&map, GL_RG,             false, 16, 16,  0,  0, 0, GL_RG,             GL_UNSIGNED_SHORT,              GL_UNSIGNED_NORMALIZED, false, RequireExt<&Extensions::textureNorm16>,           AlwaysSupported, RequireExt<&Extensions::textureNorm16>,         NeverSupported);
     AddRGBAFormat(&map, GL_RGB,            false,  8,  8,  8,  0, 0, GL_RGB,            GL_UNSIGNED_BYTE,               GL_UNSIGNED_NORMALIZED, false, AlwaysSupported,                                  AlwaysSupported, RequireES<2, 0>,                                NeverSupported);
     AddRGBAFormat(&map, GL_RGB,            false,  5,  6,  5,  0, 0, GL_RGB,            GL_UNSIGNED_SHORT_5_6_5,        GL_UNSIGNED_NORMALIZED, false, AlwaysSupported,                                  AlwaysSupported, RequireES<2, 0>,                                NeverSupported);
     AddRGBAFormat(&map, GL_RGB,            false,  8,  8,  8,  0, 0, GL_RGB,            GL_BYTE,                        GL_SIGNED_NORMALIZED,   false, NeverSupported,                                   NeverSupported,  NeverSupported,                                 NeverSupported);

src/libANGLE/renderer/gl/FramebufferGL.cpp

@@ -230,6 +230,150 @@ bool IsEmulatedAlphaChannelTextureAttachment(const FramebufferAttachment *attach
     return textureGL->hasEmulatedAlphaChannel(attachment->getTextureImageIndex());
 }
 
+class ScopedEXTTextureNorm16ReadbackWorkaround
+{
+  public:
+    ScopedEXTTextureNorm16ReadbackWorkaround()
+        : tmpPixels(nullptr), clientPixels(nullptr), enabled(false)
+    {}
+
+    ~ScopedEXTTextureNorm16ReadbackWorkaround()
+    {
+        if (tmpPixels)
+        {
+            delete[] tmpPixels;
+        }
+    }
+
+    angle::Result Initialize(const gl::Context *context,
+                             const gl::Rectangle &area,
+                             GLenum originalReadFormat,
+                             GLenum format,
+                             GLenum type,
+                             GLuint skipBytes,
+                             GLuint rowBytes,
+                             GLuint pixelBytes,
+                             GLubyte *pixels)
+    {
+        // Separate from constructor as there may be checked math result exception that needs to
+        // early return
+        ASSERT(tmpPixels == nullptr);
+        ASSERT(clientPixels == nullptr);
+
+        ContextGL *contextGL              = GetImplAs<ContextGL>(context);
+        const angle::FeaturesGL &features = GetFeaturesGL(context);
+
+        enabled = features.readPixelsUsingImplementationColorReadFormatForNorm16.enabled &&
+                  type == GL_UNSIGNED_SHORT && originalReadFormat == GL_RGBA &&
+                  (format == GL_RED || format == GL_RG);
+
+        clientPixels = pixels;
+
+        if (enabled)
+        {
+            CheckedNumeric<GLuint> checkedRowBytes(rowBytes);
+            CheckedNumeric<GLuint> checkedRows(area.height);
+            CheckedNumeric<GLuint> checkedSkipBytes(skipBytes);
+            auto checkedAllocatedBytes = checkedSkipBytes + checkedRowBytes * checkedRows;
+            if (rowBytes < area.width * pixelBytes)
+            {
+                checkedAllocatedBytes += area.width * pixelBytes - rowBytes;
+            }
+            ANGLE_CHECK_GL_MATH(contextGL, checkedAllocatedBytes.IsValid());
+            tmpPixels = new GLubyte[checkedAllocatedBytes.ValueOrDie()];
+            memset(tmpPixels, 0, checkedAllocatedBytes.ValueOrDie());
+        }
+
+        return angle::Result::Continue;
+    }
+
+    GLubyte *Pixels() const { return tmpPixels ? tmpPixels : clientPixels; }
+
+    bool IsEnabled() const { return enabled; }
+
+  private:
+    // Temporarily allocated pixel readback buffer
+    GLubyte *tmpPixels;
+    // Client pixel array pointer passed to readPixels
+    GLubyte *clientPixels;
+
+    bool enabled;
+};
+
+// Workaround to rearrange pixels read by RED/RG to RGBA for RGBA/UNSIGNED_SHORT pixel type
+// combination
+angle::Result RearrangeEXTTextureNorm16Pixels(const gl::Context *context,
+                                              const gl::Rectangle &area,
+                                              GLenum originalReadFormat,
+                                              GLenum format,
+                                              GLenum type,
+                                              GLuint skipBytes,
+                                              GLuint rowBytes,
+                                              GLuint pixelBytes,
+                                              const gl::PixelPackState &pack,
+                                              GLubyte *clientPixels,
+                                              GLubyte *tmpPixels)
+{
+    ASSERT(tmpPixels != nullptr);
+    ASSERT(originalReadFormat == GL_RGBA);
+    ASSERT(format == GL_RED_EXT || format == GL_RG_EXT);
+    ASSERT(type == GL_UNSIGNED_SHORT);
+
+    ContextGL *contextGL = GetImplAs<ContextGL>(context);
+
+    const gl::InternalFormat &glFormatOriginal =
+        gl::GetInternalFormatInfo(originalReadFormat, type);
+
+    GLuint originalReadFormatRowBytes = 0;
+    ANGLE_CHECK_GL_MATH(
+        contextGL, glFormatOriginal.computeRowPitch(type, area.width, pack.alignment,
+                                                    pack.rowLength, &originalReadFormatRowBytes));
+    GLuint originalReadFormatSkipBytes = 0;
+    ANGLE_CHECK_GL_MATH(contextGL,
+                        glFormatOriginal.computeSkipBytes(type, originalReadFormatRowBytes, 0, pack,
+                                                          false, &originalReadFormatSkipBytes));
+
+    GLuint originalReadFormatPixelBytes = glFormatOriginal.computePixelBytes(type);
+    GLuint alphaChannelBytes            = glFormatOriginal.alphaBits / 8;
+
+    ASSERT(originalReadFormatPixelBytes > pixelBytes);
+    ASSERT(originalReadFormatPixelBytes > alphaChannelBytes);
+    ASSERT(alphaChannelBytes != 0);
+    ASSERT(glFormatOriginal.alphaBits % 8 == 0);
+
+    // Populating rearrangedPixels values from pixels
+    GLubyte *srcRowStart = tmpPixels;
+    GLubyte *dstRowStart = clientPixels;
+
+    srcRowStart += skipBytes;
+    dstRowStart += originalReadFormatSkipBytes;
+
+    for (GLint y = 0; y < area.height; ++y)
+    {
+        GLubyte *src = srcRowStart;
+        GLubyte *dst = dstRowStart;
+        for (GLint x = 0; x < area.width; ++x)
+        {
+            GLushort *srcPixel = reinterpret_cast<GLushort *>(src);
+            GLushort *dstPixel = reinterpret_cast<GLushort *>(dst);
+            dstPixel[0]        = srcPixel[0];
+            dstPixel[1]        = format == GL_RG ? srcPixel[1] : 0;
+            // Set other channel of RGBA to 0 (GB when format == GL_RED, B when format == GL_RG)
+            dstPixel[2] = 0;
+            // Set alpha channel to 1
+            dstPixel[3] = 0xFFFF;
+
+            src += pixelBytes;
+            dst += originalReadFormatPixelBytes;
+        }
+
+        srcRowStart += rowBytes;
+        dstRowStart += originalReadFormatRowBytes;
+    }
+
+    return angle::Result::Continue;
+}
+
 }  // namespace
 
 FramebufferGL::FramebufferGL(const gl::FramebufferState &data,
@@ -483,7 +627,8 @@ angle::Result FramebufferGL::readPixels(const gl::Context *context,
     const angle::FeaturesGL &features = GetFeaturesGL(context);
 
     // Clip read area to framebuffer.
-    const gl::Extents fbSize = getState().getReadAttachment()->getSize();
+    const auto *readAttachment = mState.getReadAttachment();
+    const gl::Extents fbSize   = readAttachment->getSize();
     const gl::Rectangle fbRect(0, 0, fbSize.width, fbSize.height);
     gl::Rectangle clippedArea;
     if (!ClipRectangle(area, fbRect, &clippedArea))
@@ -496,10 +641,20 @@ angle::Result FramebufferGL::readPixels(const gl::Context *context,
     const gl::Buffer *packBuffer =
         context->getState().getTargetBuffer(gl::BufferBinding::PixelPack);
 
+    GLenum attachmentReadFormat =
+        readAttachment->getFormat().info->getReadPixelsFormat(context->getExtensions());
     nativegl::ReadPixelsFormat readPixelsFormat =
-        nativegl::GetReadPixelsFormat(functions, features, format, type);
+        nativegl::GetReadPixelsFormat(functions, features, attachmentReadFormat, format, type);
     GLenum readFormat = readPixelsFormat.format;
     GLenum readType   = readPixelsFormat.type;
+    if (features.readPixelsUsingImplementationColorReadFormatForNorm16.enabled &&
+        readType == GL_UNSIGNED_SHORT)
+    {
+        ANGLE_CHECK(contextGL, readFormat == GL_RED || readFormat == GL_RG || readFormat == GL_RGBA,
+                    "glReadPixels: GL_IMPLEMENTATION_COLOR_READ_FORMAT advertised by the driver is "
+                    "not handled by RGBA16 readPixels workaround.",
+                    GL_INVALID_OPERATION);
+    }
 
     stateManager->bindFramebuffer(GL_READ_FRAMEBUFFER, mFramebufferID);
 
@@ -535,7 +690,8 @@ angle::Result FramebufferGL::readPixels(const gl::Context *context,
 
     if (cannotSetDesiredRowLength || useOverlappingRowsWorkaround)
     {
-        return readPixelsRowByRow(context, clippedArea, readFormat, readType, packState, outPtr);
+        return readPixelsRowByRow(context, clippedArea, format, readFormat, readType, packState,
+                                  outPtr);
     }
 
     bool useLastRowPaddingWorkaround = false;
@@ -546,8 +702,8 @@ angle::Result FramebufferGL::readPixels(const gl::Context *context,
             readFormat, readType, false, outPtr, &useLastRowPaddingWorkaround));
     }
 
-    return readPixelsAllAtOnce(context, clippedArea, readFormat, readType, packState, outPtr,
-                               useLastRowPaddingWorkaround);
+    return readPixelsAllAtOnce(context, clippedArea, format, readFormat, readType, packState,
+                               outPtr, useLastRowPaddingWorkaround);
 }
 
 angle::Result FramebufferGL::blit(const gl::Context *context,
@@ -1275,14 +1431,16 @@ bool FramebufferGL::modifyInvalidateAttachmentsForEmulatedDefaultFBO(
 
 angle::Result FramebufferGL::readPixelsRowByRow(const gl::Context *context,
                                                 const gl::Rectangle &area,
+                                                GLenum originalReadFormat,
                                                 GLenum format,
                                                 GLenum type,
                                                 const gl::PixelPackState &pack,
                                                 GLubyte *pixels) const
 {
-    ContextGL *contextGL         = GetImplAs<ContextGL>(context);
-    const FunctionsGL *functions = GetFunctionsGL(context);
-    StateManagerGL *stateManager = GetStateManagerGL(context);
+    ContextGL *contextGL              = GetImplAs<ContextGL>(context);
+    const FunctionsGL *functions      = GetFunctionsGL(context);
+    StateManagerGL *stateManager      = GetStateManagerGL(context);
+    GLubyte *originalReadFormatPixels = pixels;
 
     const gl::InternalFormat &glFormat = gl::GetInternalFormatInfo(format, type);
 
@@ -1293,15 +1451,32 @@ angle::Result FramebufferGL::readPixelsRowByRow(const gl::Context *context,
     ANGLE_CHECK_GL_MATH(contextGL,
                         glFormat.computeSkipBytes(type, rowBytes, 0, pack, false, &skipBytes));
 
+    ScopedEXTTextureNorm16ReadbackWorkaround workaround;
+    angle::Result result =
+        workaround.Initialize(context, area, originalReadFormat, format, type, skipBytes, rowBytes,
+                              glFormat.computePixelBytes(type), pixels);
+    if (result != angle::Result::Continue)
+    {
+        return result;
+    }
+
     gl::PixelPackState directPack;
     directPack.alignment = 1;
     stateManager->setPixelPackState(directPack);
 
-    pixels += skipBytes;
+    GLubyte *readbackPixels = workaround.Pixels();
+    readbackPixels += skipBytes;
     for (GLint y = area.y; y < area.y + area.height; ++y)
     {
-        functions->readPixels(area.x, y, area.width, 1, format, type, pixels);
-        pixels += rowBytes;
+        functions->readPixels(area.x, y, area.width, 1, format, type, readbackPixels);
+        readbackPixels += rowBytes;
+    }
+
+    if (workaround.IsEnabled())
+    {
+        return RearrangeEXTTextureNorm16Pixels(
+            context, area, originalReadFormat, format, type, skipBytes, rowBytes,
+            glFormat.computePixelBytes(type), pack, originalReadFormatPixels, workaround.Pixels());
     }
 
     return angle::Result::Continue;
@@ -1309,41 +1484,61 @@ angle::Result FramebufferGL::readPixelsRowByRow(const gl::Context *context,
 
 angle::Result FramebufferGL::readPixelsAllAtOnce(const gl::Context *context,
                                                  const gl::Rectangle &area,
+                                                 GLenum originalReadFormat,
                                                  GLenum format,
                                                  GLenum type,
                                                  const gl::PixelPackState &pack,
                                                  GLubyte *pixels,
                                                  bool readLastRowSeparately) const
 {
-    ContextGL *contextGL         = GetImplAs<ContextGL>(context);
-    const FunctionsGL *functions = GetFunctionsGL(context);
-    StateManagerGL *stateManager = GetStateManagerGL(context);
+    ContextGL *contextGL              = GetImplAs<ContextGL>(context);
+    const FunctionsGL *functions      = GetFunctionsGL(context);
+    StateManagerGL *stateManager      = GetStateManagerGL(context);
+    GLubyte *originalReadFormatPixels = pixels;
+
+    const gl::InternalFormat &glFormat = gl::GetInternalFormatInfo(format, type);
+
+    GLuint rowBytes = 0;
+    ANGLE_CHECK_GL_MATH(contextGL, glFormat.computeRowPitch(type, area.width, pack.alignment,
+                                                            pack.rowLength, &rowBytes));
+    GLuint skipBytes = 0;
+    ANGLE_CHECK_GL_MATH(contextGL,
+                        glFormat.computeSkipBytes(type, rowBytes, 0, pack, false, &skipBytes));
+
+    ScopedEXTTextureNorm16ReadbackWorkaround workaround;
+    angle::Result result =
+        workaround.Initialize(context, area, originalReadFormat, format, type, skipBytes, rowBytes,
+                              glFormat.computePixelBytes(type), pixels);
+    if (result != angle::Result::Continue)
+    {
+        return result;
+    }
 
     GLint height = area.height - readLastRowSeparately;
     if (height > 0)
     {
         stateManager->setPixelPackState(pack);
-        functions->readPixels(area.x, area.y, area.width, height, format, type, pixels);
+        functions->readPixels(area.x, area.y, area.width, height, format, type,
+                              workaround.Pixels());
     }
 
     if (readLastRowSeparately)
     {
-        const gl::InternalFormat &glFormat = gl::GetInternalFormatInfo(format, type);
-
-        GLuint rowBytes = 0;
-        ANGLE_CHECK_GL_MATH(contextGL, glFormat.computeRowPitch(type, area.width, pack.alignment,
-                                                                pack.rowLength, &rowBytes));
-        GLuint skipBytes = 0;
-        ANGLE_CHECK_GL_MATH(contextGL,
-                            glFormat.computeSkipBytes(type, rowBytes, 0, pack, false, &skipBytes));
-
         gl::PixelPackState directPack;
         directPack.alignment = 1;
         stateManager->setPixelPackState(directPack);
 
-        pixels += skipBytes + (area.height - 1) * rowBytes;
+        GLubyte *readbackPixels = workaround.Pixels();
+        readbackPixels += skipBytes + (area.height - 1) * rowBytes;
         functions->readPixels(area.x, area.y + area.height - 1, area.width, 1, format, type,
-                              pixels);
+                              readbackPixels);
+    }
+
+    if (workaround.IsEnabled())
+    {
+        return RearrangeEXTTextureNorm16Pixels(
+            context, area, originalReadFormat, format, type, skipBytes, rowBytes,
+            glFormat.computePixelBytes(type), pack, originalReadFormatPixels, workaround.Pixels());
     }
 
     return angle::Result::Continue;

src/libANGLE/renderer/gl/FramebufferGL.h

@@ -101,6 +101,7 @@ class FramebufferGL : public FramebufferImpl
 
     angle::Result readPixelsRowByRow(const gl::Context *context,
                                      const gl::Rectangle &area,
+                                     GLenum originalReadFormat,
                                      GLenum format,
                                      GLenum type,
                                      const gl::PixelPackState &pack,
@@ -108,6 +109,7 @@ class FramebufferGL : public FramebufferImpl
 
     angle::Result readPixelsAllAtOnce(const gl::Context *context,
                                       const gl::Rectangle &area,
+                                      GLenum originalReadFormat,
                                       GLenum format,
                                       GLenum type,
                                       const gl::PixelPackState &pack,

src/libANGLE/renderer/gl/formatutilsgl.cpp

@@ -655,9 +655,17 @@ static GLenum GetNativeReadType(const FunctionsGL *functions,
 
 static GLenum GetNativeReadFormat(const FunctionsGL *functions,
                                   const angle::FeaturesGL &features,
-                                  GLenum format)
+                                  GLenum attachmentReadFormat,
+                                  GLenum format,
+                                  GLenum type)
 {
     GLenum result = format;
+    if (features.readPixelsUsingImplementationColorReadFormatForNorm16.enabled &&
+        type == GL_UNSIGNED_SHORT && format == GL_RGBA &&
+        (attachmentReadFormat == GL_RED || attachmentReadFormat == GL_RG))
+    {
+        result = attachmentReadFormat;
+    }
     return result;
 }
 
@@ -736,11 +744,12 @@ RenderbufferFormat GetRenderbufferFormat(const FunctionsGL *functions,
 }
 ReadPixelsFormat GetReadPixelsFormat(const FunctionsGL *functions,
                                      const angle::FeaturesGL &features,
+                                     GLenum attachmentReadFormat,
                                      GLenum format,
                                      GLenum type)
 {
     ReadPixelsFormat result;
-    result.format = GetNativeReadFormat(functions, features, format);
+    result.format = GetNativeReadFormat(functions, features, attachmentReadFormat, format, type);
     result.type   = GetNativeReadType(functions, features, type);
     return result;
 }

src/libANGLE/renderer/gl/formatutilsgl.h

@@ -131,6 +131,7 @@ struct ReadPixelsFormat
 };
 ReadPixelsFormat GetReadPixelsFormat(const FunctionsGL *functions,
                                      const angle::FeaturesGL &features,
+                                     GLenum readAttachmentFormat,
                                      GLenum format,
                                      GLenum type);
 }  // namespace nativegl

src/libANGLE/renderer/gl/renderergl_utils.cpp

@@ -1623,6 +1623,12 @@ void InitializeFeatures(const FunctionsGL *functions, angle::FeaturesGL *feature
 
     // Ported from gpu_driver_bug_list.json (#184)
     ANGLE_FEATURE_CONDITION(features, preAddTexelFetchOffsets, IsApple() && isIntel);
+
+    // Workaround for the widespread OpenGL ES driver implementaion bug
+    ANGLE_FEATURE_CONDITION(features, readPixelsUsingImplementationColorReadFormatForNorm16,
+                            functions->standard == STANDARD_GL_ES &&
+                                functions->isAtLeastGLES(gl::Version(3, 1)) &&
+                                functions->hasGLESExtension("GL_EXT_texture_norm16"));
 }
 
 void InitializeFrontendFeatures(const FunctionsGL *functions, angle::FrontendFeatures *features)

src/tests/gl_tests/TextureTest.cpp

@@ -4124,6 +4124,10 @@ class Texture2DNorm16TestES3 : public Texture2DTestES3
 
     void testNorm16Texture(GLint internalformat, GLenum format, GLenum type)
     {
+        // TODO(http://anglebug.com/4089) Fails on Win Intel OpenGL driver
+        ANGLE_SKIP_TEST_IF(IsIntel() && IsOpenGL());
+        ANGLE_SKIP_TEST_IF(!IsGLExtensionEnabled("GL_EXT_texture_norm16"));
+
         GLushort pixelValue  = (type == GL_SHORT) ? 0x7FFF : 0x6A35;
         GLushort imageData[] = {pixelValue, pixelValue, pixelValue, pixelValue};
 
@@ -4154,15 +4158,101 @@ class Texture2DNorm16TestES3 : public Texture2DTestES3
         ASSERT_GL_NO_ERROR();
     }
 
-    void testNorm16Render(GLint internalformat, GLenum format, GLenum type)
+    void testReadPixelsRGBAWithRangeAndPixelStoreMode(GLuint x,
+                                                      GLuint y,
+                                                      GLuint width,
+                                                      GLuint height,
+                                                      GLint packRowLength,
+                                                      GLint packAlignment,
+                                                      GLint packSkipPixels,
+                                                      GLint packSkipRows,
+                                                      GLenum type,
+                                                      GLColor16UI color)
     {
+        // PACK modes debugging
+        GLint s = 2;  // single component size in bytes, UNSIGNED_SHORT -> 2 in our case
+        GLint n = 4;  // 4 components per pixel, stands for GL_RGBA
+
+        GLuint l       = packRowLength == 0 ? width : packRowLength;
+        const GLint &a = packAlignment;
+
+        // According to
+        // https://www.khronos.org/registry/OpenGL-Refpages/es3.0/html/glPixelStorei.xhtml
+        GLint k                    = (s >= a) ? n * l : a / s * (1 + (s * n * l - 1) / a);
+        std::size_t componentCount = n * packSkipPixels + k * (packSkipRows + height);
+        if (static_cast<GLuint>(packRowLength) < width)
+        {
+            componentCount += width * n * s - k;
+        }
+
+        // Populate the pixels array with random dirty value
+        constexpr GLushort kDirtyValue = 0x1234;
+        std::vector<GLushort> pixels(componentCount, kDirtyValue);
+        glReadPixels(x, y, width, height, GL_RGBA, type, pixels.data());
+
+        EXPECT_GL_NO_ERROR();
+
+        GLushort *pixelRowStart = pixels.data();
+        pixelRowStart += n * packSkipPixels + k * packSkipRows;
+
+        std::vector<bool> modifiedPixels(componentCount, false);
+
+        char errorInfo[200];
+
+        for (GLuint y = 0; y < height; ++y)
+        {
+            GLushort *curPixel = pixelRowStart;
+            for (GLuint x = 0, len = (y == height - 1) ? width : std::min(l, width); x < len; ++x)
+            {
+                snprintf(errorInfo, sizeof(errorInfo),
+                         "extent: {%u, %u}, coord: (%u, %u), rowLength: %d, alignment: %d, "
+                         "skipPixels: %d, skipRows: %d\n",
+                         width, height, x, y, packRowLength, packAlignment, packSkipPixels,
+                         packSkipRows);
+                EXPECT_EQ(color.R, curPixel[0]) << errorInfo;
+                EXPECT_EQ(color.G, curPixel[1]) << errorInfo;
+                EXPECT_EQ(color.B, curPixel[2]) << errorInfo;
+                EXPECT_EQ(color.A, curPixel[3]) << errorInfo;
+
+                std::ptrdiff_t diff      = curPixel - pixels.data();
+                modifiedPixels[diff + 0] = true;
+                modifiedPixels[diff + 1] = true;
+                modifiedPixels[diff + 2] = true;
+                modifiedPixels[diff + 3] = true;
+
+                curPixel += n;
+            }
+            pixelRowStart += k;
+        }
+
+        for (std::size_t i = 0; i < modifiedPixels.size(); ++i)
+        {
+            if (!modifiedPixels[i])
+            {
+                EXPECT_EQ(pixels[i], kDirtyValue);
+            }
+        }
+    }
+
+    void testNorm16RenderAndReadPixels(GLint internalformat, GLenum format, GLenum type)
+    {
+        // TODO(http://anglebug.com/4089) Fails on Win Intel OpenGL driver
+        ANGLE_SKIP_TEST_IF(IsIntel() && IsOpenGL());
+        // TODO(http://anglebug.com/4245) Fails on Win AMD OpenGL driver
+        ANGLE_SKIP_TEST_IF(IsWindows() && IsAMD() && IsDesktopOpenGL());
+        ANGLE_SKIP_TEST_IF(!IsGLExtensionEnabled("GL_EXT_texture_norm16"));
+
         GLushort pixelValue  = 0x6A35;
         GLushort imageData[] = {pixelValue, pixelValue, pixelValue, pixelValue};
+        GLColor16UI color    = SliceFormatColor16UI(
+            format, GLColor16UI(pixelValue, pixelValue, pixelValue, pixelValue));
+        // Size of drawing viewport
+        constexpr GLint width = 8, height = 8;
 
         setUpProgram();
 
         glBindTexture(GL_TEXTURE_2D, mTextures[1]);
-        glTexImage2D(GL_TEXTURE_2D, 0, internalformat, 1, 1, 0, format, type, nullptr);
+        glTexImage2D(GL_TEXTURE_2D, 0, internalformat, width, height, 0, format, type, nullptr);
 
         glBindFramebuffer(GL_FRAMEBUFFER, mFBO);
         glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, mTextures[1],
@@ -4170,14 +4260,72 @@ class Texture2DNorm16TestES3 : public Texture2DTestES3
 
         glBindTexture(GL_TEXTURE_2D, mTextures[2]);
         glTexImage2D(GL_TEXTURE_2D, 0, internalformat, 1, 1, 0, format, type, imageData);
+        glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+        glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
 
         EXPECT_GL_NO_ERROR();
 
         drawQuad(mProgram, "position", 0.5f);
 
-        EXPECT_PIXEL_16UI_COLOR(0, 0,
-                                SliceFormatColor16UI(format, GLColor16UI(pixelValue, pixelValue,
-                                                                         pixelValue, pixelValue)));
+        // ReadPixels against different width, height, pixel pack mode combinations to test
+        // workaround of pixels rearrangement
+
+        // {x, y, width, height}
+        std::vector<std::array<GLint, 4>> areas = {
+            {0, 0, 1, 1}, {0, 0, 1, 2}, {0, 0, 2, 1}, {0, 0, 2, 2},
+            {0, 0, 3, 2}, {0, 0, 3, 3}, {0, 0, 4, 3}, {0, 0, 4, 4},
+
+            {1, 3, 3, 2}, {1, 3, 3, 3}, {3, 2, 4, 3}, {3, 2, 4, 4},
+
+            {0, 0, 5, 6}, {2, 1, 5, 6}, {0, 0, 6, 1}, {0, 0, 7, 1},
+            {0, 0, 7, 3}, {0, 0, 7, 8}, {1, 0, 7, 8}, {0, 0, 8, 8},
+        };
+
+        // Put default settings at the last
+        std::vector<GLint> paramsPackRowLength                        = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 0};
+        std::vector<GLint> paramsPackAlignment                        = {1, 2, 8, 4};
+        std::vector<std::array<GLint, 2>> paramsPackSkipPixelsAndRows = {{1, 0}, {0, 1},   {1, 1},
+                                                                         {3, 1}, {20, 20}, {0, 0}};
+
+        // Restore pixel pack modes later
+        GLint restorePackAlignment;
+        glGetIntegerv(GL_PACK_ALIGNMENT, &restorePackAlignment);
+        GLint restorePackRowLength;
+        glGetIntegerv(GL_PACK_ROW_LENGTH, &restorePackRowLength);
+        GLint restorePackSkipPixels;
+        glGetIntegerv(GL_PACK_SKIP_PIXELS, &restorePackSkipPixels);
+        GLint restorePackSkipRows;
+        glGetIntegerv(GL_PACK_SKIP_ROWS, &restorePackSkipRows);
+
+        // Variable symbols are based on:
+        // https://www.khronos.org/registry/OpenGL-Refpages/es3.0/html/glPixelStorei.xhtml
+        for (const auto &skipped : paramsPackSkipPixelsAndRows)
+        {
+            glPixelStorei(GL_PACK_SKIP_PIXELS, skipped[0]);
+            glPixelStorei(GL_PACK_SKIP_ROWS, skipped[1]);
+            for (GLint a : paramsPackAlignment)
+            {
+                glPixelStorei(GL_PACK_ALIGNMENT, a);
+                for (GLint l : paramsPackRowLength)
+                {
+                    glPixelStorei(GL_PACK_ROW_LENGTH, l);
+
+                    for (const auto &area : areas)
+                    {
+                        ASSERT(area[0] + area[2] <= width);
+                        ASSERT(area[1] + area[3] <= height);
+                        testReadPixelsRGBAWithRangeAndPixelStoreMode(area[0], area[1], area[2],
+                                                                     area[3], l, a, skipped[0],
+                                                                     skipped[1], type, color);
+                    }
+                }
+            }
+        }
+
+        glPixelStorei(GL_PACK_ALIGNMENT, restorePackAlignment);
+        glPixelStorei(GL_PACK_ROW_LENGTH, restorePackRowLength);
+        glPixelStorei(GL_PACK_SKIP_PIXELS, restorePackSkipPixels);
+        glPixelStorei(GL_PACK_SKIP_ROWS, restorePackSkipRows);
 
         glBindRenderbuffer(GL_RENDERBUFFER, mRenderbuffer);
         glRenderbufferStorage(GL_RENDERBUFFER, internalformat, 1, 1);
@@ -4210,27 +4358,65 @@ class Texture2DNorm16TestES3 : public Texture2DTestES3
     GLuint mRenderbuffer;
 };
 
-// Test texture formats enabled by the GL_EXT_texture_norm16 extension.
-TEST_P(Texture2DNorm16TestES3, TextureNorm16Test)
+TEST_P(Texture2DNorm16TestES3, TextureNorm16R16TextureTest)
 {
-    // TODO(crbug.com/angleproject/4089) Fails on Nexus5X Adreno
-    // TODO(crbug.com/1024387) Fails on Nexus6P
-    ANGLE_SKIP_TEST_IF(IsNexus5X() || IsNexus6P());
-    // TODO(crbug.com/angleproject/4089) Fails on Win Intel OpenGL driver
-    ANGLE_SKIP_TEST_IF(IsIntel() && IsOpenGL());
-
-    ANGLE_SKIP_TEST_IF(!IsGLExtensionEnabled("GL_EXT_texture_norm16"));
-
     testNorm16Texture(GL_R16_EXT, GL_RED, GL_UNSIGNED_SHORT);
-    testNorm16Texture(GL_RG16_EXT, GL_RG, GL_UNSIGNED_SHORT);
-    testNorm16Texture(GL_RGB16_EXT, GL_RGB, GL_UNSIGNED_SHORT);
-    testNorm16Texture(GL_RGBA16_EXT, GL_RGBA, GL_UNSIGNED_SHORT);
-    testNorm16Texture(GL_R16_SNORM_EXT, GL_RED, GL_SHORT);
-    testNorm16Texture(GL_RG16_SNORM_EXT, GL_RG, GL_SHORT);
-    testNorm16Texture(GL_RGB16_SNORM_EXT, GL_RGB, GL_SHORT);
-    testNorm16Texture(GL_RGBA16_SNORM_EXT, GL_RGBA, GL_SHORT);
+}
 
-    testNorm16Render(GL_RGBA16_EXT, GL_RGBA, GL_UNSIGNED_SHORT);
+TEST_P(Texture2DNorm16TestES3, TextureNorm16R16SNORMTextureTest)
+{
+    testNorm16Texture(GL_R16_SNORM_EXT, GL_RED, GL_SHORT);
+}
+
+TEST_P(Texture2DNorm16TestES3, TextureNorm16RG16TextureTest)
+{
+    testNorm16Texture(GL_RG16_EXT, GL_RG, GL_UNSIGNED_SHORT);
+}
+
+TEST_P(Texture2DNorm16TestES3, TextureNorm16RG16SNORMTextureTest)
+{
+    testNorm16Texture(GL_RG16_SNORM_EXT, GL_RG, GL_SHORT);
+}
+
+TEST_P(Texture2DNorm16TestES3, TextureNorm16RGB16TextureTest)
+{
+    // (http://anglebug.com/4215) Driver bug on some Qualcomm Adreno gpu
+    ANGLE_SKIP_TEST_IF((IsNexus5X() || IsNexus6P()) && IsOpenGLES());
+
+    testNorm16Texture(GL_RGB16_EXT, GL_RGB, GL_UNSIGNED_SHORT);
+}
+
+TEST_P(Texture2DNorm16TestES3, TextureNorm16RGB16SNORMTextureTest)
+{
+    // (http://anglebug.com/4215) Driver bug on some Qualcomm Adreno gpu
+    ANGLE_SKIP_TEST_IF((IsNexus5X() || IsNexus6P()) && IsOpenGLES());
+
+    testNorm16Texture(GL_RGB16_SNORM_EXT, GL_RGB, GL_SHORT);
+}
+
+TEST_P(Texture2DNorm16TestES3, TextureNorm16RGBA16TextureTest)
+{
+    testNorm16Texture(GL_RGBA16_EXT, GL_RGBA, GL_UNSIGNED_SHORT);
+}
+
+TEST_P(Texture2DNorm16TestES3, TextureNorm16RGBA16SNORMTextureTest)
+{
+    testNorm16Texture(GL_RGBA16_SNORM_EXT, GL_RGBA, GL_SHORT);
+}
+
+TEST_P(Texture2DNorm16TestES3, TextureNorm16R16RenderTest)
+{
+    testNorm16RenderAndReadPixels(GL_R16_EXT, GL_RED, GL_UNSIGNED_SHORT);
+}
+
+TEST_P(Texture2DNorm16TestES3, TextureNorm16RG16RenderTest)
+{
+    testNorm16RenderAndReadPixels(GL_RG16_EXT, GL_RG, GL_UNSIGNED_SHORT);
+}
+
+TEST_P(Texture2DNorm16TestES3, TextureNorm16RGBA16RenderTest)
+{
+    testNorm16RenderAndReadPixels(GL_RGBA16_EXT, GL_RGBA, GL_UNSIGNED_SHORT);
 }
 
 class Texture2DRGTest : public Texture2DTest