Refactor read pixel format/type validation.

src/OpenGL/libGLESv2/Context.cpp

@@ -3197,21 +3197,7 @@ void Context::readPixels(GLint x, GLint y, GLsizei width, GLsizei height, GLenum
 		return error(GL_INVALID_OPERATION);
 	}
 
-	GLenum readFormat = GL_NONE;
-	GLenum readType = GL_NONE;
-	switch(format)
-	{
-	case GL_DEPTH_COMPONENT:
-		readFormat = framebuffer->getDepthReadFormat();
-		readType = framebuffer->getDepthReadType();
-		break;
-	default:
-		readFormat = framebuffer->getImplementationColorReadFormat();
-		readType = framebuffer->getImplementationColorReadType();
-		break;
-	}
-
-	if(!(readFormat == format && readType == type) && !ValidReadPixelsFormatType(readFormat, readType, format, type, clientVersion))
+	if(!IsValidReadPixelsFormatType(framebuffer, format, type, clientVersion))
 	{
 		return error(GL_INVALID_OPERATION);
 	}
@@ -4072,7 +4058,7 @@ void Context::blitFramebuffer(GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1
 
 		// OpenGL ES 3.0.4 spec, p.199:
 		// ...an INVALID_OPERATION error is generated if the formats of the read
-		// and draw framebuffers are not identical or if the source and destination 
+		// and draw framebuffers are not identical or if the source and destination
 		// rectangles are not defined with the same(X0, Y 0) and (X1, Y 1) bounds.
 		// If SAMPLE_BUFFERS for the draw framebuffer is greater than zero, an
 		// INVALID_OPERATION error is generated.

src/OpenGL/libGLESv2/Framebuffer.cpp

@@ -213,23 +213,23 @@ egl::Image *Framebuffer::getStencilBuffer()
 	return nullptr;
 }
 
-Renderbuffer *Framebuffer::getColorbuffer(GLuint index)
+Renderbuffer *Framebuffer::getColorbuffer(GLuint index) const
 {
 	return (index < MAX_COLOR_ATTACHMENTS) ? mColorbufferPointer[index] : (Renderbuffer*)nullptr;
 }
 
-Renderbuffer *Framebuffer::getReadColorbuffer()
+Renderbuffer *Framebuffer::getReadColorbuffer() const
 {
 	Context *context = getContext();
 	return getColorbuffer(context->getReadFramebufferColorIndex());
 }
 
-Renderbuffer *Framebuffer::getDepthbuffer()
+Renderbuffer *Framebuffer::getDepthbuffer() const
 {
 	return mDepthbufferPointer;
 }
 
-Renderbuffer *Framebuffer::getStencilbuffer()
+Renderbuffer *Framebuffer::getStencilbuffer() const
 {
 	return mStencilbufferPointer;
 }
@@ -491,7 +491,7 @@ GLenum Framebuffer::completeness(int &width, int &height, int &samples)
 	return GL_FRAMEBUFFER_COMPLETE;
 }
 
-GLenum Framebuffer::getImplementationColorReadFormat()
+GLenum Framebuffer::getImplementationColorReadFormat() const
 {
 	Renderbuffer *colorbuffer = getReadColorbuffer();
 
@@ -555,7 +555,7 @@ GLenum Framebuffer::getImplementationColorReadFormat()
 	return GL_RGBA;
 }
 
-GLenum Framebuffer::getImplementationColorReadType()
+GLenum Framebuffer::getImplementationColorReadType() const
 {
 	Renderbuffer *colorbuffer = getReadColorbuffer();
 
@@ -619,7 +619,7 @@ GLenum Framebuffer::getImplementationColorReadType()
 	return GL_UNSIGNED_BYTE;
 }
 
-GLenum Framebuffer::getDepthReadFormat()
+GLenum Framebuffer::getDepthReadFormat() const
 {
 	Renderbuffer *depthbuffer = getDepthbuffer();
 
@@ -633,7 +633,7 @@ GLenum Framebuffer::getDepthReadFormat()
 	return GL_NONE;
 }
 
-GLenum Framebuffer::getDepthReadType()
+GLenum Framebuffer::getDepthReadType() const
 {
 	Renderbuffer *depthbuffer = getDepthbuffer();
 

src/OpenGL/libGLESv2/Framebuffer.h

@@ -56,10 +56,10 @@ public:
 	egl::Image *getDepthBuffer();
 	egl::Image *getStencilBuffer();
 
-	Renderbuffer *getColorbuffer(GLuint index);
-	Renderbuffer *getReadColorbuffer();
-	Renderbuffer *getDepthbuffer();
-	Renderbuffer *getStencilbuffer();
+	Renderbuffer *getColorbuffer(GLuint index) const;
+	Renderbuffer *getReadColorbuffer() const;
+	Renderbuffer *getDepthbuffer() const;
+	Renderbuffer *getStencilbuffer() const;
 
 	GLenum getColorbufferType(GLuint index);
 	GLenum getDepthbufferType();
@@ -78,10 +78,10 @@ public:
 	GLenum completeness();
 	GLenum completeness(int &width, int &height, int &samples);
 
-	GLenum getImplementationColorReadFormat();
-	GLenum getImplementationColorReadType();
-	GLenum getDepthReadFormat();
-	GLenum getDepthReadType();
+	GLenum getImplementationColorReadFormat() const;
+	GLenum getImplementationColorReadType() const;
+	GLenum getDepthReadFormat() const;
+	GLenum getDepthReadType() const;
 
 	virtual bool isDefaultFramebuffer() const { return false; }
 

src/OpenGL/libGLESv2/utilities.cpp

@@ -16,6 +16,7 @@
 
 #include "utilities.h"
 
+#include "Framebuffer.h"
 #include "main.h"
 #include "mathutil.h"
 #include "Context.h"
@@ -596,82 +597,107 @@ namespace es2
 		return GL_NONE;
 	}
 
-	bool ValidReadPixelsFormatType(GLenum internalFormat, GLenum internalType, GLenum format, GLenum type, GLint clientVersion)
+	bool IsValidReadPixelsFormatType(const Framebuffer *framebuffer, GLenum format, GLenum type, GLint clientVersion)
 	{
-		switch(format)
+		// GL_NV_read_depth
+		if(format == GL_DEPTH_COMPONENT)
 		{
-		case GL_RGBA:
+			Renderbuffer *depthbuffer = framebuffer->getDepthbuffer();
+
+			if(!depthbuffer)
+			{
+				return false;
+			}
+
 			switch(type)
 			{
-			case GL_UNSIGNED_BYTE:
-				break;
-			case GL_UNSIGNED_INT_2_10_10_10_REV:
-				return (clientVersion >= 3) && (internalFormat == GL_RGB10_A2);
+			case GL_UNSIGNED_SHORT:
 			case GL_FLOAT:
-				return (clientVersion >= 3) && (internalType == GL_FLOAT);
+				return true;
 			default:
+				UNIMPLEMENTED();
 				return false;
 			}
-			break;
-		case GL_RGBA_INTEGER:
-			if(clientVersion < 3)
+		}
+
+		Renderbuffer *colorbuffer = framebuffer->getColorbuffer(0);
+
+		if(!colorbuffer)
+		{
+			return false;
+		}
+
+		sw::Format internalformat = colorbuffer->getInternalFormat();
+
+		if(sw::Surface::isNormalizedInteger(internalformat))
+		{
+			// Combination always supported by normalized fixed-point rendering surfaces.
+			if(format == GL_RGBA && type == GL_UNSIGNED_BYTE)
 			{
-				return false;
+				return true;
 			}
-			switch(type)
+		}
+		else if(sw::Surface::isFloatFormat(internalformat))
+		{
+			// Combination always supported by floating-point rendering surfaces.
+			// Supported in OpenGL ES 2.0 due to GL_EXT_color_buffer_half_float.
+			if(format == GL_RGBA && type == GL_FLOAT)
 			{
-			case GL_INT:
-				if(internalType != GL_INT)
-				{
-					return false;
-				}
-				break;
-			case GL_UNSIGNED_INT:
-				if(internalType != GL_UNSIGNED_INT)
-				{
-					return false;
-				}
-				break;
-			default:
-				return false;
+				return true;
 			}
-			break;
-		case GL_BGRA_EXT:
-			switch(type)
+		}
+		else if(sw::Surface::isSignedNonNormalizedInteger(internalformat))
+		{
+			ASSERT(clientVersion >= 3);
+
+			if(format == GL_RGBA_INTEGER && type == GL_INT)
 			{
-			case GL_UNSIGNED_BYTE:
-			case GL_UNSIGNED_SHORT_4_4_4_4_REV_EXT:
-			case GL_UNSIGNED_SHORT_1_5_5_5_REV_EXT:
-				break;
-			default:
-				return false;
+				return true;
 			}
-			break;
-		case GL_RG_EXT:
-		case GL_RED_EXT:
-			return (clientVersion >= 3) && (type == GL_UNSIGNED_BYTE);
-		case GL_DEPTH_COMPONENT:
-			if(internalFormat != format)
+		}
+		else if(sw::Surface::isUnsignedNonNormalizedInteger(internalformat))
+		{
+			ASSERT(clientVersion >= 3);
+
+			if(format == GL_RGBA_INTEGER && type == GL_UNSIGNED_INT)
 			{
-				return false;
+				return true;
 			}
-			switch(type)
+		}
+		else UNREACHABLE(internalformat);
+
+		// GL_IMPLEMENTATION_COLOR_READ_FORMAT / GL_IMPLEMENTATION_COLOR_READ_TYPE
+		GLenum implementationReadFormat = GL_NONE;
+		GLenum implementationReadType = GL_NONE;
+		switch(format)
+		{
+		default:
+			implementationReadFormat = framebuffer->getImplementationColorReadFormat();
+			implementationReadType = framebuffer->getImplementationColorReadType();
+			break;
+		case GL_DEPTH_COMPONENT:
+			implementationReadFormat = framebuffer->getDepthReadFormat();
+			implementationReadType = framebuffer->getDepthReadType();
+			break;
+		}
+
+		if(format == implementationReadFormat && type == implementationReadType)
+		{
+			return true;
+		}
+
+		// Additional third combination accepted by OpenGL ES 3.0.
+		if(internalformat == sw::FORMAT_A2B10G10R10)
+		{
+			ASSERT(clientVersion >= 3);
+
+			if(format == GL_RGBA && type == GL_UNSIGNED_INT_2_10_10_10_REV)
 			{
-			case GL_UNSIGNED_SHORT:
-			case GL_FLOAT:
-				if(internalType != type)
-				{
-					return false;
-				}
-				break;
-			default:
-				return false;
+				return true;
 			}
-			break;
-		default:
-			return false;
 		}
-		return true;
+
+		return false;
 	}
 
 	bool IsDepthTexture(GLenum format)
@@ -1878,7 +1904,7 @@ namespace sw2es
 				return GL_SIGNED_NORMALIZED;
 			default:
 				UNREACHABLE(format);
-				return 0;
+				return GL_NONE;
 			}
 		case GL_DEPTH_ATTACHMENT:
 		case GL_STENCIL_ATTACHMENT:
@@ -1886,7 +1912,7 @@ namespace sw2es
 			return GL_FLOAT;
 		default:
 			UNREACHABLE(attachment);
-			return 0;
+			return GL_NONE;
 		}
 	}
 

src/OpenGL/libGLESv2/utilities.h

@@ -29,6 +29,7 @@
 namespace es2
 {
 	struct Color;
+	class Framebuffer;
 
 	unsigned int UniformComponentCount(GLenum type);
 	GLenum UniformComponentType(GLenum type);
@@ -46,7 +47,7 @@ namespace es2
 	GLenum ValidateCompressedFormat(GLenum format, GLint clientVersion, bool expectCompressedFormats);
 	GLenum ValidateSubImageParams(bool compressed, GLsizei width, GLsizei height, GLint xoffset, GLint yoffset, GLenum target, GLint level, GLenum sizedInternalFormat, Texture *texture);
 	GLenum ValidateSubImageParams(bool compressed, GLsizei width, GLsizei height, GLsizei depth, GLint xoffset, GLint yoffset, GLint zoffset, GLenum target, GLint level, GLenum sizedInternalFormat, Texture *texture);
-	bool ValidReadPixelsFormatType(GLenum internalFormat, GLenum internalType, GLenum format, GLenum type, GLint clientVersion);
+	bool IsValidReadPixelsFormatType(const Framebuffer *framebuffer, GLenum format, GLenum type, GLint clientVersion);
 	bool IsDepthTexture(GLenum format);
 	bool IsStencilTexture(GLenum format);
 	bool IsCubemapTextureTarget(GLenum target);

src/Renderer/Surface.cpp

@@ -2941,7 +2941,7 @@ namespace sw
 		}
 	}
 
-	bool Surface::isNonNormalizedInteger(Format format)
+	bool Surface::isSignedNonNormalizedInteger(Format format)
 	{
 		switch(format)
 		{
@@ -2957,6 +2957,16 @@ namespace sw
 		case FORMAT_X16B16G16R16I:
 		case FORMAT_G16R16I:
 		case FORMAT_R16I:
+			return true;
+		default:
+			return false;
+		}
+	}
+
+	bool Surface::isUnsignedNonNormalizedInteger(Format format)
+	{
+		switch(format)
+		{
 		case FORMAT_A16B16G16R16UI:
 		case FORMAT_X16B16G16R16UI:
 		case FORMAT_G16R16UI:
@@ -2975,6 +2985,21 @@ namespace sw
 		}
 	}
 
+	bool Surface::isNonNormalizedInteger(Format format)
+	{
+		return isSignedNonNormalizedInteger(format) ||
+		       isUnsignedNonNormalizedInteger(format);
+	}
+
+	bool Surface::isNormalizedInteger(Format format)
+	{
+		return !isFloatFormat(format) &&
+		       !isNonNormalizedInteger(format) &&
+		       !isCompressed(format) &&
+		       !isDepth(format) &&
+		       !isStencil(format);
+	}
+
 	int Surface::componentCount(Format format)
 	{
 		switch(format)

src/Renderer/Surface.hpp

@@ -337,7 +337,10 @@ namespace sw
 		static bool isSRGBreadable(Format format);
 		static bool isSRGBwritable(Format format);
 		static bool isCompressed(Format format);
+		static bool isSignedNonNormalizedInteger(Format format);
+		static bool isUnsignedNonNormalizedInteger(Format format);
 		static bool isNonNormalizedInteger(Format format);
+		static bool isNormalizedInteger(Format format);
 		static int componentCount(Format format);
 
 		static void setTexturePalette(unsigned int *palette);