Refactor ReadPixels validation.

src/libGLESv2/Context.cpp

@@ -2933,31 +2933,11 @@ void Context::clearBufferfi(GLenum buffer, int drawbuffer, float depth, int sten
 void Context::readPixels(GLint x, GLint y, GLsizei width, GLsizei height,
                          GLenum format, GLenum type, GLsizei *bufSize, void* pixels)
 {
-    Framebuffer *framebuffer = getReadFramebuffer();
-
-    if (framebuffer->completeness() != GL_FRAMEBUFFER_COMPLETE)
-    {
-        return gl::error(GL_INVALID_FRAMEBUFFER_OPERATION);
-    }
-
-    if (getReadFramebufferHandle() != 0 && framebuffer->getSamples() != 0)
-    {
-        return gl::error(GL_INVALID_OPERATION);
-    }
-
-    GLenum sizedInternalFormat = IsSizedInternalFormat(format, mClientVersion) ? format
-                                                                               : GetSizedInternalFormat(format, type, mClientVersion);
+    gl::Framebuffer *framebuffer = getReadFramebuffer();
 
-    GLsizei outputPitch = GetRowPitch(sizedInternalFormat, type, mClientVersion, width, getPackAlignment());
-    // sized query sanity check
-    if (bufSize)
-    {
-        int requiredSize = outputPitch * height;
-        if (requiredSize > *bufSize)
-        {
-            return gl::error(GL_INVALID_OPERATION);
-        }
-    }
+    bool isSized = IsSizedInternalFormat(format, mClientVersion);
+    GLenum sizedInternalFormat = (isSized ? format : GetSizedInternalFormat(format, type, mClientVersion));
+    GLuint outputPitch = GetRowPitch(sizedInternalFormat, type, mClientVersion, width, getPackAlignment());
 
     mRenderer->readPixels(framebuffer, x, y, width, height, format, type, outputPitch, getPackReverseRowOrder(), getPackAlignment(), pixels);
 }

src/libGLESv2/libGLESv2.cpp

@@ -4397,20 +4397,10 @@ void __stdcall glReadnPixelsEXT(GLint x, GLint y, GLsizei width, GLsizei height,
 
         if (context)
         {
-            GLenum currentInternalFormat, currentFormat, currentType;
-
-            // Failure in getCurrentReadFormatType indicates that no color attachment is currently bound,
-            // and attempting to read back if that's the case is an error. The error will be registered
-            // by getCurrentReadFormat.
-            if (!context->getCurrentReadFormatType(&currentInternalFormat, &currentFormat, &currentType))
-                return;
-
-            bool validReadFormat = (context->getClientVersion() < 3) ? gl::ValidES2ReadFormatType(format, type) :
-                                                                       gl::ValidES3ReadFormatType(currentInternalFormat, format, type);
-
-            if (!(currentFormat == format && currentType == type) && !validReadFormat)
+            if (!gl::ValidateReadPixelsParameters(context, x, y, width, height,
+                                                  format, type, &bufSize, data))
             {
-                return gl::error(GL_INVALID_OPERATION);
+                return;
             }
 
             context->readPixels(x, y, width, height, format, type, &bufSize, data);
@@ -4440,20 +4430,10 @@ void __stdcall glReadPixels(GLint x, GLint y, GLsizei width, GLsizei height,
 
         if (context)
         {
-            GLenum currentInternalFormat, currentFormat, currentType;
-
-            // Failure in getCurrentReadFormatType indicates that no color attachment is currently bound,
-            // and attempting to read back if that's the case is an error. The error will be registered
-            // by getCurrentReadFormat.
-            if (!context->getCurrentReadFormatType(&currentInternalFormat, &currentFormat, &currentType))
-                return;
-
-            bool validReadFormat = (context->getClientVersion() < 3) ? gl::ValidES2ReadFormatType(format, type) :
-                                                                       gl::ValidES3ReadFormatType(currentInternalFormat, format, type);
-
-            if (!(currentFormat == format && currentType == type) && !validReadFormat)
+            if (!gl::ValidateReadPixelsParameters(context, x, y, width, height,
+                                                  format, type, NULL, pixels))
             {
-                return gl::error(GL_INVALID_OPERATION);
+                return;
             }
 
             context->readPixels(x, y, width, height, format, type, NULL, pixels);

src/libGLESv2/validationES.cpp

@@ -8,6 +8,8 @@
 // validationES.h: Validation functions for generic OpenGL ES entry point parameters
 
 #include "libGLESv2/validationES.h"
+#include "libGLESv2/validationES2.h"
+#include "libGLESv2/validationES3.h"
 #include "libGLESv2/Context.h"
 #include "libGLESv2/Texture.h"
 #include "libGLESv2/Framebuffer.h"
@@ -771,4 +773,55 @@ bool ValidateSamplerObjectParameter(GLenum pname)
     }
 }
 
+bool ValidateReadPixelsParameters(gl::Context *context, GLint x, GLint y, GLsizei width, GLsizei height,
+                                  GLenum format, GLenum type, GLsizei *bufSize, GLvoid *pixels)
+{
+    gl::Framebuffer *framebuffer = context->getReadFramebuffer();
+
+    if (framebuffer->completeness() != GL_FRAMEBUFFER_COMPLETE)
+    {
+        return gl::error(GL_INVALID_FRAMEBUFFER_OPERATION, false);
+    }
+
+    if (context->getReadFramebufferHandle() != 0 && framebuffer->getSamples() != 0)
+    {
+        return gl::error(GL_INVALID_OPERATION, false);
+    }
+
+    GLenum currentInternalFormat, currentFormat, currentType;
+    int clientVersion = context->getClientVersion();
+
+    // Failure in getCurrentReadFormatType indicates that no color attachment is currently bound,
+    // and attempting to read back if that's the case is an error. The error will be registered
+    // by getCurrentReadFormat.
+    // Note: we need to explicitly check for framebuffer completeness here, before we call
+    // getCurrentReadFormatType, because it generates a different (wrong) error for incomplete FBOs
+    if (!context->getCurrentReadFormatType(&currentInternalFormat, &currentFormat, &currentType))
+        return false;
+
+    bool validReadFormat = (clientVersion < 3) ? ValidES2ReadFormatType(format, type) :
+                                                 ValidES3ReadFormatType(currentInternalFormat, format, type);
+
+    if (!(currentFormat == format && currentType == type) && !validReadFormat)
+    {
+        return gl::error(GL_INVALID_OPERATION, false);
+    }
+
+    GLenum sizedInternalFormat = IsSizedInternalFormat(format, clientVersion) ? format :
+                                 GetSizedInternalFormat(format, type, clientVersion);
+
+    GLsizei outputPitch = GetRowPitch(sizedInternalFormat, type, clientVersion, width, context->getPackAlignment());
+    // sized query sanity check
+    if (bufSize)
+    {
+        int requiredSize = outputPitch * height;
+        if (requiredSize > *bufSize)
+        {
+            return gl::error(GL_INVALID_OPERATION, false);
+        }
+    }
+
+    return true;
+}
+
 }

src/libGLESv2/validationES.h

@@ -40,6 +40,9 @@ bool ValidateTexParamParameters(gl::Context *context, GLenum pname, GLint param)
 
 bool ValidateSamplerObjectParameter(GLenum pname);
 
+bool ValidateReadPixelsParameters(gl::Context *context, GLint x, GLint y, GLsizei width, GLsizei height,
+                                  GLenum format, GLenum type, GLsizei *bufSize, GLvoid *pixels);
+
 }
 
 #endif // LIBGLESV2_VALIDATION_ES_H