[Mesa-dev] [PATCH] i965: pass wanted format to intel_miptree_create_for_dri_image
Tapani Pälli
tapani.palli at intel.com
Fri Oct 6 04:45:33 UTC 2017
On 10/06/2017 03:00 AM, Jason Ekstrand wrote:
> On Wed, Oct 4, 2017 at 6:32 AM, Tapani Pälli <tapani.palli at intel.com
> <mailto: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
> <mailto:tapani.palli at intel.com>>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102999
> <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,
ok
> Reviewed-by: Jason Ekstrand <jason at jlekstrand.net
> <mailto: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.
Yes, I will try to dig this issue further.
Thanks Jason!
> --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
>
>
More information about the mesa-dev
mailing list