Refactor ReadPixels validation.
Move ReadPixels error generation out of the implementation and into
the API level.
BUG=angle:571
Change-Id: I0b32294f359fedd13d1af2c95baf37a3e5ac1d5b
Reviewed-on: https://chromium-review.googlesource.com/188014
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Reviewed-by: Shannon Woods <shannonwoods@chromium.org>
Tested-by: Jamie Madill <jmadill@chromium.org>
diff --git a/src/libGLESv2/Context.cpp b/src/libGLESv2/Context.cpp
index 3199f95..0fe10c5 100644
--- a/src/libGLESv2/Context.cpp
+++ b/src/libGLESv2/Context.cpp
@@ -2933,31 +2933,11 @@
void Context::readPixels(GLint x, GLint y, GLsizei width, GLsizei height,
GLenum format, GLenum type, GLsizei *bufSize, void* pixels)
{
- Framebuffer *framebuffer = getReadFramebuffer();
+ gl::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);
-
- 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);
}
diff --git a/src/libGLESv2/libGLESv2.cpp b/src/libGLESv2/libGLESv2.cpp
index 20455aa..b0a3d06 100644
--- a/src/libGLESv2/libGLESv2.cpp
+++ b/src/libGLESv2/libGLESv2.cpp
@@ -4397,20 +4397,10 @@
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(¤tInternalFormat, ¤tFormat, ¤tType))
- 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 @@
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(¤tInternalFormat, ¤tFormat, ¤tType))
- 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);
diff --git a/src/libGLESv2/validationES.cpp b/src/libGLESv2/validationES.cpp
index 5ed852c..98b353e 100644
--- a/src/libGLESv2/validationES.cpp
+++ b/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 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(¤tInternalFormat, ¤tFormat, ¤tType))
+ 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;
+}
+
}
diff --git a/src/libGLESv2/validationES.h b/src/libGLESv2/validationES.h
index 4352901..1fa58c9 100644
--- a/src/libGLESv2/validationES.h
+++ b/src/libGLESv2/validationES.h
@@ -40,6 +40,9 @@
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