[Mesa-dev] [PATCH] i965: Fix misapplication of gles3 srgb workaround

[Ian Romanick]

On 08/21/2013 11:09 AM, Chad Versace wrote:
> Fixes inconsistent failure of gles2conform/GL2Tests/glUniform/glUniform.test
> under gnome-shell. What follows is a description of the bug and its fix.
>
> When intel_update_renderbuffers() allocates a miptree for a winsys
> renderbuffer, it propagates the renderbuffer's format to become also the
> miptree's format.
>
> If the winsys color buffer format is SARGB, then, in the first call to
> eglMakeCurrent, intel_gles3_srgb_workaround() changes the renderbuffer's
> format to ARGB. That is, it changes the format from sRGB to non-sRGB.
> However, it changes the renderbuffer's format *after*
> intel_update_renderbuffers() has allocated the renderbuffer's miptree.
> Therefore, when eglMakeCurrent returns, the miptree format (SARGB)
> differs from the renderbuffer format (ARGB).
>
> If the X server reallocates the color buffer,
> intel_update_renderbuffers() will create a new miptree for the
> renderbuffer. The new miptree's format (ARGB) will differ from old
> miptree's format (SARGB). This mismatch between old and new miptrees
> causes bugs.
>
> Fix the bug by moving intel_gles3_srgb_workaround() to occur *before*
> intel_update_renderbuffers().

This sounds reasonable... but why did the test fail intermittently?

> CC: "9.2" <mesa-stable at lists.freedesktop.org>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67934
> Signed-off-by: Chad Versace <chad.versace at linux.intel.com>
> ---
>   src/mesa/drivers/dri/i965/intel_context.c | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_context.c b/src/mesa/drivers/dri/i965/intel_context.c
> index 2e38475..37c1770 100644
> --- a/src/mesa/drivers/dri/i965/intel_context.c
> +++ b/src/mesa/drivers/dri/i965/intel_context.c
> @@ -755,11 +755,15 @@ intelMakeCurrent(__DRIcontext * driContextPriv,
>   	 driContextPriv->dri2.read_stamp = driReadPriv->dri2.stamp - 1;
>         }
>
> +      /* The sRGB workaround changes the renderbuffer's format. We must change
> +       * the format before the renderbuffer's miptree get's allocated, otherwise
> +       * the formats of the renderbuffer and its miptree will differ.
> +       */
> +      intel_gles3_srgb_workaround(brw, fb);
> +      intel_gles3_srgb_workaround(brw, readFb);
> +
>         intel_prepare_render(brw);
>         _mesa_make_current(ctx, fb, readFb);
> -
> -      intel_gles3_srgb_workaround(brw, ctx->WinSysDrawBuffer);
> -      intel_gles3_srgb_workaround(brw, ctx->WinSysReadBuffer);
>      }
>      else {
>         _mesa_make_current(NULL, NULL, NULL);
>