[Mesa-dev] [PATCH 10/20] mesa/es: Validate glReadPixels format and type in Mesa code rather than the ES wrapper

[Ian Romanick]

From: Ian Romanick <ian.d.romanick at intel.com>

v2: Add proper GLES3 filtering.

Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
---
 src/mesa/main/APIspec.xml |   53 ---------------------------------------------
 src/mesa/main/readpix.c   |   33 ++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 53 deletions(-)

diff --git a/src/mesa/main/APIspec.xml b/src/mesa/main/APIspec.xml
index f1dae65..1bc7e2a 100644
--- a/src/mesa/main/APIspec.xml
+++ b/src/mesa/main/APIspec.xml
@@ -591,59 +591,6 @@
 		<param name="type" type="GLenum"/>
 		<param name="pixels" type="GLvoid *"/>
 	</proto>
-
-	<!-- Technically, only two combinations are actually allowed:
-	     GL_RGBA/GL_UNSIGNED_BYTE, and some implementation-specific
-	     internal preferred combination.  I don't know what that is, so I'm
-	     allowing any valid combination for now; the underlying support
-	     should fail when necessary.-->
-	<desc name="format">
-		<value name="GL_ALPHA"/>
-		<desc name="type" error="GL_INVALID_OPERATION">
-			<value name="GL_UNSIGNED_BYTE"/>
-		</desc>
-	</desc>
-
-	<desc name="format">
-		<value name="GL_RGB"/>
-		<desc name="type" error="GL_INVALID_OPERATION">
-			<value name="GL_UNSIGNED_BYTE"/>
-			<value name="GL_UNSIGNED_SHORT_5_6_5"/>
-		</desc>
-	</desc>
-
-	<desc name="format">
-		<value name="GL_RGBA"/>
-		<desc name="type" error="GL_INVALID_OPERATION">
-			<value name="GL_UNSIGNED_BYTE"/>
-			<value name="GL_UNSIGNED_SHORT_4_4_4_4"/>
-			<value name="GL_UNSIGNED_SHORT_5_5_5_1"/>
-		</desc>
-	</desc>
-
-	<desc name="format">
-		<value name="GL_LUMINANCE"/>
-		<desc name="type" error="GL_INVALID_OPERATION">
-			<value name="GL_UNSIGNED_BYTE"/>
-		</desc>
-	</desc>
-
-	<desc name="format">
-		<value name="GL_LUMINANCE_ALPHA"/>
-		<desc name="type" error="GL_INVALID_OPERATION">
-			<value name="GL_UNSIGNED_BYTE"/>
-		</desc>
-	</desc>
-
-	<desc name="format" category="EXT_read_format_bgra">
-		<value name="GL_BGRA_EXT"/>
-
-		<desc name="type" error="GL_INVALID_OPERATION">
-			<value name="GL_UNSIGNED_BYTE"/>
-			<value name="GL_UNSIGNED_SHORT_4_4_4_4_REV_EXT"/>
-			<value name="GL_UNSIGNED_SHORT_1_5_5_5_REV_EXT"/>
-		</desc>
-	</desc>
 </template>
 
 <template name="GetClipPlane" direction="get">
diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
index f0bc157..7dc7581 100644
--- a/src/mesa/main/readpix.c
+++ b/src/mesa/main/readpix.c
@@ -38,7 +38,13 @@
 #include "state.h"
 #include "glformats.h"
 #include "fbobject.h"
+#include "teximage.h"
 
+/* Inexplicably, GL_HALF_FLOAT_OES has a different value than GL_HALF_FLOAT.
+ */
+#ifndef GL_HALF_FLOAT_OES
+#define GL_HALF_FLOAT_OES 0x8D61
+#endif
 
 /**
  * Tries to implement glReadPixels() of GL_DEPTH_COMPONENT using memcpy of the
@@ -699,6 +705,33 @@ _mesa_ReadnPixelsARB( GLint x, GLint y, GLsizei width, GLsizei height,
       return;
    }
 
+   /* OpenGL ES 1.x and OpenGL ES 2.0 impose additional restrictions on the
+    * combinations of format and type that can be used.
+    *
+    * Technically, only two combinations are actually allowed:
+    * GL_RGBA/GL_UNSIGNED_BYTE, and some implementation-specific internal
+    * preferred combination.  This code doesn't know what that preferred
+    * combination is, and Mesa can handle anything valid.  Just work instead.
+    */
+   if (_mesa_is_gles(ctx) && ctx->Version < 30) {
+      err = _mesa_es_error_check_format_and_type(format, type, 2);
+      if (err == GL_NO_ERROR) {
+         if (type == GL_FLOAT || type == GL_HALF_FLOAT_OES) {
+            err = GL_INVALID_OPERATION;
+         } else if (format == GL_DEPTH_COMPONENT
+                    || format == GL_DEPTH_STENCIL) {
+            err = GL_INVALID_ENUM;
+         }
+      }
+
+      if (err != GL_NO_ERROR) {
+         _mesa_error(ctx, err, "glReadPixels(invalid format %s and/or type %s)",
+                     _mesa_lookup_enum_by_nr(format),
+                     _mesa_lookup_enum_by_nr(type));
+         return;
+      }
+   }
+
    if (ctx->NewState)
       _mesa_update_state(ctx);
 
-- 
1.7.6.5