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

[Ian Romanick]

On 03/12/2014 06:07 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."
> 
> Fixes failing Khronos CTS test packed_depth_stencil_error.test
> 
> V2: Avoid code duplication
> 
> Cc: <mesa-stable at lists.freedesktop.org>
> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>

One comment below.  With that fixed,

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

> ---
>  src/mesa/main/glformats.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
> index 77cf263..32c4f42 100644
> --- a/src/mesa/main/glformats.c
> +++ b/src/mesa/main/glformats.c
> @@ -1238,6 +1238,22 @@ GLenum
>  _mesa_error_check_format_and_type(const struct gl_context *ctx,
>                                    GLenum format, GLenum type)
>  {
> +   /* From OpenGL 3.3 spec, page 220:
> +    *    "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.

Unindent the last two lines because they're not part of the spec quote.

> +    */
> +   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;
> +
>     /* special type-based checks (see glReadPixels, glDrawPixels error lists) */
>     switch (type) {
>     case GL_BITMAP:
>