From 143fa68f50b72153f65bcaa4fb590f697356a3dc Mon Sep 17 00:00:00 2001
From: Alexey Knyazev <lexa.knyazev@gmail.com>
Date: Thu, 27 Jul 2023 00:00:00 +0000
Subject: [PATCH] Disallow read type conversions for signed 16-bit color
 buffers

Signed 16-bit color buffers should not be converted
to unsigned or 8-bit pixel types during readPixels
operations.

Bug: angleproject:8048
Change-Id: I27eaeb3d543732b5079bd53ef4fad1711ce3c3ef
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4727392
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
---
 src/libANGLE/validationES.cpp         |  7 ++--
 src/tests/gl_tests/ReadPixelsTest.cpp | 55 +++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/src/libANGLE/validationES.cpp b/src/libANGLE/validationES.cpp
index 4fe802302..4ab8f3bd4 100644
--- a/src/libANGLE/validationES.cpp
+++ b/src/libANGLE/validationES.cpp
@@ -213,9 +213,10 @@ bool ValidReadPixelsFormatType(const Context *context,
             }
         case GL_SIGNED_NORMALIZED:
             ASSERT(context->getExtensions().renderSnormEXT);
-            return format == GL_RGBA &&
-                   (type == GL_BYTE || (context->getExtensions().textureNorm16EXT &&
-                                        type == GL_SHORT && info->type == GL_SHORT));
+            ASSERT(info->type == GL_BYTE ||
+                   (context->getExtensions().textureNorm16EXT && info->type == GL_SHORT));
+            // Type conversions are not allowed for signed normalized color buffers
+            return format == GL_RGBA && type == info->type;
 
         case GL_INT:
             return (format == GL_RGBA_INTEGER && type == GL_INT);
diff --git a/src/tests/gl_tests/ReadPixelsTest.cpp b/src/tests/gl_tests/ReadPixelsTest.cpp
index 61a4ff427..de414ccd7 100644
--- a/src/tests/gl_tests/ReadPixelsTest.cpp
+++ b/src/tests/gl_tests/ReadPixelsTest.cpp
@@ -1188,6 +1188,40 @@ class ReadPixelsErrorTest : public ReadPixelsTest
         glDeleteFramebuffers(1, &mFBO);
     }
 
+    void testUnsupportedTypeConversions(std::vector<GLenum> internalFormats,
+                                        std::vector<GLenum> unsupportedTypes)
+    {
+        glBindFramebuffer(GL_FRAMEBUFFER, mFBO);
+        for (GLenum internalFormat : internalFormats)
+        {
+            GLRenderbuffer rbo;
+            glBindRenderbuffer(GL_RENDERBUFFER, rbo);
+            glRenderbufferStorage(GL_RENDERBUFFER, internalFormat, 1, 1);
+            ASSERT_GL_NO_ERROR();
+
+            glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_RENDERBUFFER, rbo);
+            ASSERT_GL_FRAMEBUFFER_COMPLETE(GL_FRAMEBUFFER);
+
+            GLenum implementationFormat, implementationType;
+            glGetIntegerv(GL_IMPLEMENTATION_COLOR_READ_FORMAT,
+                          reinterpret_cast<GLint *>(&implementationFormat));
+            ASSERT_GL_NO_ERROR();
+            glGetIntegerv(GL_IMPLEMENTATION_COLOR_READ_TYPE,
+                          reinterpret_cast<GLint *>(&implementationType));
+            ASSERT_GL_NO_ERROR();
+
+            for (GLenum type : unsupportedTypes)
+            {
+                uint8_t pixel[8] = {};
+                if (implementationFormat != GL_RGBA || implementationType != type)
+                {
+                    glReadPixels(0, 0, 1, 1, GL_RGBA, type, pixel);
+                    EXPECT_GL_ERROR(GL_INVALID_OPERATION);
+                }
+            }
+        }
+    }
+
     GLuint mTexture;
     GLuint mFBO;
 };
@@ -1205,6 +1239,27 @@ TEST_P(ReadPixelsErrorTest, ReadBufferIsNone)
     EXPECT_GL_ERROR(GL_INVALID_OPERATION);
 }
 
+// The test verifies that glReadPixels generates a GL_INVALID_OPERATION
+// error when reading signed 8-bit color buffers using incompatible types.
+TEST_P(ReadPixelsErrorTest, ColorBufferSnorm8)
+{
+    ANGLE_SKIP_TEST_IF(!IsGLExtensionEnabled("GL_EXT_render_snorm"));
+
+    testUnsupportedTypeConversions({GL_R8_SNORM, GL_RG8_SNORM, GL_RGBA8_SNORM},
+                                   {GL_UNSIGNED_BYTE, GL_SHORT, GL_UNSIGNED_SHORT});
+}
+
+// The test verifies that glReadPixels generates a GL_INVALID_OPERATION
+// error when reading signed 16-bit color buffers using incompatible types.
+TEST_P(ReadPixelsErrorTest, ColorBufferSnorm16)
+{
+    ANGLE_SKIP_TEST_IF(!IsGLExtensionEnabled("GL_EXT_render_snorm"));
+    ANGLE_SKIP_TEST_IF(!IsGLExtensionEnabled("GL_EXT_texture_norm16"));
+
+    testUnsupportedTypeConversions({GL_R16_SNORM_EXT, GL_RG16_SNORM_EXT, GL_RGBA16_SNORM_EXT},
+                                   {GL_BYTE, GL_UNSIGNED_BYTE, GL_UNSIGNED_SHORT});
+}
+
 // a test class to be used for error checking of glReadPixels with WebGLCompatibility
 class ReadPixelsWebGLErrorTest : public ReadPixelsTest
 {