[Mesa-dev] [PATCH] mesa: Return INVALID_ENUM for glReadPixels(..., GL_DEPTH_*, ...) on ES 3

[Anuj Phogat]

On Fri, Nov 30, 2012 at 3:13 PM, Matt Turner <mattst88 at gmail.com> wrote:
> I'm not sure if this is the correct fix. The
> _mesa_es_error_check_format_and_type function (used above in the ES 1
> and 2 cases) was originally added for glTexImage checking and allows
> GL_DEPTH_STENCIL/GL_UNSIGNED_INT_24_8 combinations. Using it in ES 3
> causes other tests to regress.
>
> Fixes es3conform's packed_depth_stencil_error test.
> ---
>  src/mesa/main/readpix.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
> index d6d105b..6eea561 100644
> --- a/src/mesa/main/readpix.c
> +++ b/src/mesa/main/readpix.c
> @@ -709,36 +709,45 @@ _mesa_ReadnPixelsARB( GLint x, GLint y, GLsizei width, GLsizei height,
>      */
>     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;
>        }
> +   } else if (_mesa_is_gles3(ctx)) {
> +      if (format == GL_DEPTH_COMPONENT
> +          || format == GL_DEPTH_STENCIL) {
> +         err = GL_INVALID_ENUM;
> +         _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;
> +      }
>     }

Can't we test this condition under if(_mesa_is_gles(ctx)). Ideally API_OPENGLES3
should be included in _mesa_is_gles() function.

Below code should work fine:
GLenum err  = GL_NO_ERROR; (initialized at beginning of function)

   if (_mesa_is_gles(ctx)) {
      if (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;
            }
         }
      }

      if (err == GL_NO_ERROR && (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);
>
>     err = _mesa_error_check_format_and_type(ctx, format, type);
>     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->ReadBuffer->_Status != GL_FRAMEBUFFER_COMPLETE_EXT) {
>        _mesa_error(ctx, GL_INVALID_FRAMEBUFFER_OPERATION_EXT,
>                    "glReadPixels(incomplete framebuffer)" );
>        return;
>     }
> --
> 1.7.8.6
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev