commit 1c93b6c6a7be42f6bb1f8f759d7cb3fa99dab0b4
author Alexis Hetu <sugoi@google.com> Mon Nov 20 14:52:03 2017 -0500
committer Alexis Hétu <sugoi@google.com> Wed Nov 22 14:51:21 2017 +0000
tree c86a27d7f64ab2d0fbc6421e8ceae14585e738bf
parent f89cd0b8a6eebeb55add7b09b2b2499d50f81e52

Framebuffer related validity checks

Added validity checks for RenderbufferStorageMultisample
and BlitFramebuffer, mostly missing checks for using
integer types or depth/stencil framebuffers with
multisampling, which is not allowed.

Fixes all failures in:
dEQP-GLES3.functional.negative_api.buffer*

Change-Id: Ie1db21a3b9f1ca71ed660a2758d43f24846acdf1
Reviewed-on: https://swiftshader-review.googlesource.com/14048
Tested-by: Alexis Hétu <sugoi@google.com>
Reviewed-by: Nicolas Capens <nicolascapens@google.com>

src/OpenGL/libGLESv2/Context.cpp

diff --git a/src/OpenGL/libGLESv2/Context.cpp b/src/OpenGL/libGLESv2/Context.cpp
index 49e6ba2..ede3df6 100644
--- a/src/OpenGL/libGLESv2/Context.cpp
+++ b/src/OpenGL/libGLESv2/Context.cpp
@@ -4111,6 +4111,52 @@
 			return error(GL_INVALID_OPERATION);
 		}
 
+		// The GL ES 3.0.2 spec (pg 193) states that:
+		// 1) If the read buffer is fixed point format, the draw buffer must be as well
+		// 2) If the read buffer is an unsigned integer format, the draw buffer must be
+		// as well
+		// 3) If the read buffer is a signed integer format, the draw buffer must be as
+		// well
+		es2::Renderbuffer *readRenderbuffer = readFramebuffer->getReadColorbuffer();
+		es2::Renderbuffer *drawRenderbuffer = drawFramebuffer->getColorbuffer(0);
+		sw::Format readFormat = readRenderbuffer->getInternalFormat();
+		sw::Format drawFormat = drawRenderbuffer->getInternalFormat();
+		GLenum readComponentType = sw2es::GetComponentType(readFormat, GL_COLOR_ATTACHMENT0);
+		GLenum drawComponentType = sw2es::GetComponentType(drawFormat, GL_COLOR_ATTACHMENT0);
+		bool readFixedPoint = ((readComponentType == GL_UNSIGNED_NORMALIZED) ||
+		                       (readComponentType == GL_SIGNED_NORMALIZED));
+		bool drawFixedPoint = ((drawComponentType == GL_UNSIGNED_NORMALIZED) ||
+		                       (drawComponentType == GL_SIGNED_NORMALIZED));
+		bool readFixedOrFloat = (readFixedPoint || (readComponentType == GL_FLOAT));
+		bool drawFixedOrFloat = (drawFixedPoint || (drawComponentType == GL_FLOAT));
+
+		if(readFixedOrFloat != drawFixedOrFloat)
+		{
+			return error(GL_INVALID_OPERATION);
+		}
+
+		if((readComponentType == GL_UNSIGNED_INT) && (drawComponentType != GL_UNSIGNED_INT))
+		{
+			return error(GL_INVALID_OPERATION);
+		}
+
+		if((readComponentType == GL_INT) && (drawComponentType != GL_INT))
+		{
+			return error(GL_INVALID_OPERATION);
+		}
+
+		// Cannot filter integer data
+		if(((readComponentType == GL_UNSIGNED_INT) || (readComponentType == GL_INT)) && filter)
+		{
+			return error(GL_INVALID_OPERATION);
+		}
+
+		if((readRenderbuffer->getSamples() > 0) &&
+		   (readRenderbuffer->getFormat() != drawRenderbuffer->getFormat()))
+		{
+			return error(GL_INVALID_OPERATION);
+		}
+
 		blitRenderTarget = true;
 	}
 
@@ -4134,6 +4180,11 @@
 				blitDepth = true;
 				readDSBuffer = readFramebuffer->getDepthbuffer();
 				drawDSBuffer = drawFramebuffer->getDepthbuffer();
+
+				if(readDSBuffer->getInternalFormat() != drawDSBuffer->getInternalFormat())
+				{
+					return error(GL_INVALID_OPERATION);
+				}
 			}
 		}
 
@@ -4152,6 +4203,11 @@
 				blitStencil = true;
 				readDSBuffer = readFramebuffer->getStencilbuffer();
 				drawDSBuffer = drawFramebuffer->getStencilbuffer();
+
+				if(readDSBuffer->getInternalFormat() != drawDSBuffer->getInternalFormat())
+				{
+					return error(GL_INVALID_OPERATION);
+				}
 			}
 		}
 

src/OpenGL/libGLESv2/libGLESv2.cpp

diff --git a/src/OpenGL/libGLESv2/libGLESv2.cpp b/src/OpenGL/libGLESv2/libGLESv2.cpp
index f16afed..c560600 100644
--- a/src/OpenGL/libGLESv2/libGLESv2.cpp
+++ b/src/OpenGL/libGLESv2/libGLESv2.cpp
@@ -4726,22 +4726,23 @@
 		return error(GL_INVALID_ENUM);
 	}
 
-	if(width < 0 || height < 0 || samples < 0)
+	if(width < 0 || height < 0 || samples < 0 ||
+	   width > es2::IMPLEMENTATION_MAX_RENDERBUFFER_SIZE ||
+	   height > es2::IMPLEMENTATION_MAX_RENDERBUFFER_SIZE)
 	{
 		return error(GL_INVALID_VALUE);
 	}
 
+	if(samples > es2::IMPLEMENTATION_MAX_SAMPLES ||
+	   (sw::Surface::isNonNormalizedInteger(es2sw::ConvertRenderbufferFormat(internalformat)) && samples > 0))
+	{
+		return error(GL_INVALID_OPERATION);
+	}
+
 	es2::Context *context = es2::getContext();
 
 	if(context)
 	{
-		if(width > es2::IMPLEMENTATION_MAX_RENDERBUFFER_SIZE ||
-		   height > es2::IMPLEMENTATION_MAX_RENDERBUFFER_SIZE ||
-		   samples > es2::IMPLEMENTATION_MAX_SAMPLES)
-		{
-			return error(GL_INVALID_VALUE);
-		}
-
 		GLuint handle = context->getRenderbufferName();
 		if(handle == 0)
 		{

src/OpenGL/libGLESv2/libGLESv3.cpp

diff --git a/src/OpenGL/libGLESv2/libGLESv3.cpp b/src/OpenGL/libGLESv2/libGLESv3.cpp
index 92813e2..aae9782 100644
--- a/src/OpenGL/libGLESv2/libGLESv3.cpp
+++ b/src/OpenGL/libGLESv2/libGLESv3.cpp
@@ -1498,7 +1498,12 @@
 	switch(filter)
 	{
 	case GL_NEAREST:
+		break;
 	case GL_LINEAR:
+		if((mask & GL_DEPTH_BUFFER_BIT) || (mask & GL_STENCIL_BUFFER_BIT))
+		{
+			return error(GL_INVALID_OPERATION);
+		}
 		break;
 	default:
 		return error(GL_INVALID_ENUM);
@@ -1547,7 +1552,7 @@
 		{
 			if(!textureObject)
 			{
-				return error(GL_INVALID_VALUE);
+				return error(GL_INVALID_OPERATION);
 			}
 
 			textarget = textureObject->getTarget();
@@ -1575,9 +1580,17 @@
 		{
 		case GL_DRAW_FRAMEBUFFER:
 		case GL_FRAMEBUFFER:
+			if(context->getDrawFramebufferName() == 0)
+			{
+				return error(GL_INVALID_OPERATION);
+			}
 			framebuffer = context->getDrawFramebuffer();
 			break;
 		case GL_READ_FRAMEBUFFER:
+			if(context->getReadFramebufferName() == 0)
+			{
+				return error(GL_INVALID_OPERATION);
+			}
 			framebuffer = context->getReadFramebuffer();
 			break;
 		default: