[Mesa-dev] [PATCH] mesa: fix errors when reading depth with glReadPixels

[Anuj Phogat]

On Thu, Sep 17, 2015 at 3:28 AM, Tapani Pälli <tapani.palli at intel.com>
wrote:

> OpenGL ES 3.0 spec 3.7.2 "Transfer of Pixel Rectangles" specifies
> DEPTH_COMPONENT, UNSIGNED_INT as a valid couple, validation for
> internal format is checked by is_float_depth().
>
> Fix regression caused by 81d2fd91a90e5b2fd9fd74792a7a7c329f0e4d29 in:
>    ES3-CTS.gtf.GL3Tests.packed_pixels.packed_pixels
>
> Test uses GL_DEPTH_COMPONENT, UNSIGNED_INT only when GL_NV_read_depth
> extension is present.
>
> v2: change check in _mesa_error_check_format_and_type to be explicit
>     for ES 2.0+, desktop OpenGL does not allow this behaviour + uses
>     this function for both glReadPixels and glDrawPixels validation.
>     (No Piglit regressions seen with v2.)
>
> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92009
> Cc: "10.7 11.0" <mesa-stable at lists.freedesktop.org>
> ---
>  src/mesa/main/glformats.c | 7 ++++++-
>  src/mesa/main/readpix.c   | 1 +
>  2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
> index c1d3c7d..eb4a02a 100644
> --- a/src/mesa/main/glformats.c
> +++ b/src/mesa/main/glformats.c
> @@ -494,7 +494,8 @@ _mesa_bytes_per_pixel(GLenum format, GLenum type)
>        else
>           return -1;
>     case GL_UNSIGNED_INT_24_8_EXT:
> -      if (format == GL_DEPTH_STENCIL_EXT)
> +      if (format == GL_DEPTH_COMPONENT ||
> +          format == GL_DEPTH_STENCIL_EXT)
>           return sizeof(GLuint);
>        else
>           return -1;
> @@ -1789,6 +1790,10 @@ _mesa_error_check_format_and_type(const struct
> gl_context *ctx,
>        return GL_INVALID_OPERATION;
>
>     case GL_UNSIGNED_INT_24_8:
> +      /* Depth buffer OK to read in OpenGL ES (NV_read_depth). */
> +      if (ctx->API == API_OPENGLES2 && format == GL_DEPTH_COMPONENT)
> +         return GL_NO_ERROR;
> +
>
Verified that  NV_read_depth extension is an OpenGL ES extension and allows
GL_DEPTH_COMPONENT format in ReadPixels.

       if (format != GL_DEPTH_STENCIL) {
>           return GL_INVALID_OPERATION;
>        }
> diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
> index 76ef8ee..81bb912 100644
> --- a/src/mesa/main/readpix.c
> +++ b/src/mesa/main/readpix.c
> @@ -963,6 +963,7 @@ read_pixels_es3_error_check(GLenum format, GLenum type,
>              return GL_NO_ERROR;
>           break;
>        case GL_UNSIGNED_SHORT:
> +      case GL_UNSIGNED_INT:
>        case GL_UNSIGNED_INT_24_8:
>           if (!is_float_depth)
>              return GL_NO_ERROR;
> --
> 2.4.3
>
>
Nice catch before merging. Updated patch is:
Reviewed-by: Anuj Phogat <anuj.phogat at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150917/5f1f4af6/attachment.html>