[Mesa-dev] [PATCH] mesa: Fix error code generation in glReadPixels()

[Ian Romanick]

On 02/14/2014 04:49 PM, Anuj Phogat wrote:
> Section 4.3.1, page 220, of OpenGL 3.3 specification explains
> the error conditions for glreadPixels():
> 
>    "If the format is DEPTH_STENCIL, then values are taken from
>     both the depth buffer and the stencil buffer. If there is
>     no depth buffer or if there is no stencil buffer, then the
>     error INVALID_OPERATION occurs. If the type parameter is
>     not UNSIGNED_INT_24_8 or FLOAT_32_UNSIGNED_INT_24_8_REV,
>     then the error INVALID_ENUM occurs."

Add this quotation to the code with an extra comment "OpenGL ES still
generates GL_INVALID_OPERATION because glReadPixels cannot be used to
read depth or stencil in that API."

> Fixes failing Khronos CTS test packed_depth_stencil_error.test
> 
> Cc: <mesa-stable at lists.freedesktop.org>
> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
> ---
>  src/mesa/main/glformats.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
> index 77cf263..b797900 100644
> --- a/src/mesa/main/glformats.c
> +++ b/src/mesa/main/glformats.c
> @@ -1257,6 +1257,9 @@ _mesa_error_check_format_and_type(const struct gl_context *ctx,
>            ctx->Extensions.ARB_texture_rgb10_a2ui) {
>           break; /* OK */
>        }
> +      if (format == GL_DEPTH_STENCIL && _mesa_is_desktop_gl(ctx)) {
> +         return GL_INVALID_ENUM;
> +      }

Wouldn't it be easier to just add the following before the
switch-statement?

   /* Section 4.3.1, page 220, of OpenGL 3.3 specification explains
    * the error conditions for glreadPixels():
    *
    *     "If the format is DEPTH_STENCIL, then values are taken from both
    *     the depth buffer and the stencil buffer. If there is no depth
    *     buffer or if there is no stencil buffer, then the error
    *     INVALID_OPERATION occurs. If the type parameter is not
    *     UNSIGNED_INT_24_8 or FLOAT_32_UNSIGNED_INT_24_8_REV, then the
    *     error INVALID_ENUM occurs."
    *
    * OpenGL ES still generates GL_INVALID_OPERATION because glReadPixels
    * cannot be used to read depth or stencil in that API.
    */
   if (_mesa_is_desktop_gl(ctx) && format == GL_DEPTH_STENCIL
       && type != GL_UNSIGNED_INT_24_8 
       && type != GL_FLOAT_32_UNSIGNED_INT_24_8_REV)
      return GL_INVALID_ENUM;

>        return GL_INVALID_OPERATION;
>  
>     case GL_UNSIGNED_SHORT_4_4_4_4:
> @@ -1280,6 +1283,9 @@ _mesa_error_check_format_and_type(const struct gl_context *ctx,
>            ctx->API == API_OPENGLES2) {
>           break; /* OK by GL_EXT_texture_type_2_10_10_10_REV */
>        }
> +      if (format == GL_DEPTH_STENCIL && _mesa_is_desktop_gl(ctx)) {
> +         return GL_INVALID_ENUM;
> +      }
>        return GL_INVALID_OPERATION;
>  
>     case GL_UNSIGNED_INT_24_8:
> @@ -1298,7 +1304,8 @@ _mesa_error_check_format_and_type(const struct gl_context *ctx,
>        return GL_NO_ERROR;
>  
>     case GL_UNSIGNED_INT_10F_11F_11F_REV:
> -      if (!ctx->Extensions.EXT_packed_float) {
> +      if (!ctx->Extensions.EXT_packed_float ||
> +          (format == GL_DEPTH_STENCIL && _mesa_is_desktop_gl(ctx))) {
>           return GL_INVALID_ENUM;
>        }
>        if (format != GL_RGB) {
>