[Mesa-dev] [PATCH] i965: pass wanted format to intel_miptree_create_for_dri_image

Jason Ekstrand jason at jlekstrand.net
Fri Oct 6 00:00:14 UTC 2017


On Wed, Oct 4, 2017 at 6:32 AM, Tapani Pälli <tapani.palli at intel.com> wrote:

> Change b3a44ae7a4 caused regressions on Android where DRI and renderbuffer
> can disagree on the format being used. This patch removes the colorspace
> parameter and instead we pass renderbuffer format. For non-winsys images we
> still do srgb/linear modification in same manner as change b3a44ae7a4
> wanted
> but take format from renderbuffer instead of DRI image.
>
> This patch fixes regressions seen with following test sets:
>
>    dEQP-EGL.functional.color_clears*
>    dEQP-EGL.functional.render*
>
> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102999
> ---
>  src/mesa/drivers/dri/i965/brw_context.c       | 14 +----------
>  src/mesa/drivers/dri/i965/intel_fbo.c         |  2 +-
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 34
> ++++++++++-----------------
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.h |  2 +-
>  src/mesa/drivers/dri/i965/intel_tex_image.c   |  4 ++--
>  5 files changed, 18 insertions(+), 38 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c
> b/src/mesa/drivers/dri/i965/brw_context.c
> index 1fd967e424..751b026439 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -1596,21 +1596,9 @@ intel_update_image_buffer(struct brw_context
> *intel,
>     if (last_mt && last_mt->bo == buffer->bo)
>        return;
>
> -   enum isl_colorspace colorspace;
> -   switch (_mesa_get_format_color_encoding(intel_rb_format(rb))) {
> -   case GL_SRGB:
> -      colorspace = ISL_COLORSPACE_SRGB;
> -      break;
> -   case GL_LINEAR:
> -      colorspace = ISL_COLORSPACE_LINEAR;
> -      break;
> -   default:
> -      unreachable("Invalid color encoding");
> -   }
> -
>     struct intel_mipmap_tree *mt =
>        intel_miptree_create_for_dri_image(intel, buffer, GL_TEXTURE_2D,
> -                                         colorspace, true);
> +                                         intel_rb_format(rb), true);
>     if (!mt)
>        return;
>
> diff --git a/src/mesa/drivers/dri/i965/intel_fbo.c
> b/src/mesa/drivers/dri/i965/intel_fbo.c
> index 46f140c028..4a592f37ef 100644
> --- a/src/mesa/drivers/dri/i965/intel_fbo.c
> +++ b/src/mesa/drivers/dri/i965/intel_fbo.c
> @@ -364,7 +364,7 @@ intel_image_target_renderbuffer_storage(struct
> gl_context *ctx,
>      * content.
>      */
>     irb->mt = intel_miptree_create_for_dri_image(brw, image,
> GL_TEXTURE_2D,
> -                                                ISL_COLORSPACE_NONE,
> false);
> +                                                image->format, false);
>     if (!irb->mt)
>        return;
>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 5b7cde82f6..9870748711 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -959,34 +959,26 @@ create_ccs_buf_for_image(struct brw_context *brw,
>  struct intel_mipmap_tree *
>  intel_miptree_create_for_dri_image(struct brw_context *brw,
>                                     __DRIimage *image, GLenum target,
> -                                   enum isl_colorspace colorspace,
> +                                   mesa_format format,
>                                     bool is_winsys_image)
>  {
> -   if (image->planar_format && image->planar_format->nplanes > 1) {
> -      assert(colorspace == ISL_COLORSPACE_NONE ||
> -             colorspace == ISL_COLORSPACE_YUV);
> +   if (image->planar_format && image->planar_format->nplanes > 1)
>        return miptree_create_for_planar_image(brw, image, target);
> -   }
>
>     if (image->planar_format)
>        assert(image->planar_format->planes[0].dri_format ==
> image->dri_format);
>
> -   mesa_format format = image->format;
> -   switch (colorspace) {
> -   case ISL_COLORSPACE_NONE:
> -      /* Keep the image format unmodified */
> -      break;
> -
> -   case ISL_COLORSPACE_LINEAR:
> -      format =_mesa_get_srgb_format_linear(format);
> -      break;
> -
> -   case ISL_COLORSPACE_SRGB:
> -      format =_mesa_get_linear_format_srgb(format);
> -      break;
> -
> -   default:
> -      unreachable("Inalid colorspace for non-planar image");
> +   if (!is_winsys_image) {
> +      switch(_mesa_get_format_color_encoding(format)) {
> +      case GL_SRGB:
> +         format =_mesa_get_linear_format_srgb(format);
> +         break;
> +      case GL_LINEAR:
> +         format =_mesa_get_srgb_format_linear(format);
> +         break;
> +      default:
> +         unreachable("Invalid color encoding");
> +      }
>

This switch statement does nothing since it's switching on the current
format's encoding.  Let's just drop it.  With that,

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>

We should still figure out why Android is giving us the wrong image format
but let's focus on getting the bug fixed first.  This shouldn't be any more
wrong than we were before I did all the colorspace stuff.

--Jason


>     }
>
>     if (!brw->ctx.TextureFormatSupported[format]) {
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> index 2fce28c524..439b0f66ae 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> @@ -407,7 +407,7 @@ struct intel_mipmap_tree *
>  intel_miptree_create_for_dri_image(struct brw_context *brw,
>                                     __DRIimage *image,
>                                     GLenum target,
> -                                   enum isl_colorspace colorspace,
> +                                   mesa_format format,
>                                     bool is_winsys_image);
>
>  bool
> diff --git a/src/mesa/drivers/dri/i965/intel_tex_image.c
> b/src/mesa/drivers/dri/i965/intel_tex_image.c
> index 7712f7085f..7396597d9f 100644
> --- a/src/mesa/drivers/dri/i965/intel_tex_image.c
> +++ b/src/mesa/drivers/dri/i965/intel_tex_image.c
> @@ -524,8 +524,8 @@ intel_image_target_texture_2d(struct gl_context *ctx,
> GLenum target,
>        return;
>     }
>
> -   mt = intel_miptree_create_for_dri_image(brw, image, target,
> -                                           ISL_COLORSPACE_NONE, false);
> +   mt = intel_miptree_create_for_dri_image(brw, image, target,
> image->format,
> +                                           false);
>     if (mt == NULL)
>        return;
>
> --
> 2.13.5
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20171005/b7f57cc6/attachment-0001.html>


More information about the mesa-dev mailing list