[Mesa-dev] [PATCH] st/mesa: update the blend before a full color clear

[Marek Olšák]

This is not the correct fix.

clear_with_quad calls cso_set_blend. pipe->clear ignores (and should
ignore) the blend state. There is no scenario where you would have to
call st_update_blend for pipe->clear. I think this is a virgl bug in
pipe->clear.

Marek

On Fri, Feb 9, 2018 at 1:28 AM, gurchetansingh at chromium.org
<gurchetansingh at chromium.org> wrote:
> From: Gurchetan Singh <gurchetansingh at chromium.org>
>
> Consider this series of events:
>
> glColorMask(GL_FALSE, GL_TRUE, GL_TRUE, GL_TRUE);
> glClearColor(0.125f, 0.25f, 0.5f, 1.0f);
> glClear(GL_COLOR_BUFFER_BIT);
> glColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE);
> glClearColor(0.1f, 0.1f, 0.1f, 1.0f);
> glClear(GL_COLOR_BUFFER_BIT);
>
> With virgl, this may render an incorrect result.  This is because
> there our two paths for clears in Gallium -- one for full clears
> (st->pipe->clear) and another for color-masked/scissored clears
> (clear_with_quad).  We only encode the color mask in the
> virgl_encode_blend_state in the clear_with_quad case.
>
> We should set the colormask the case of full clears as well, since
> we need to update the GL state on the host driver.
>
> With this patch, the number of dEQP GLES2 failures on Virgl with a
> nvidia host drivers goes from 260 to 136 with no regressions.
> Two representative test cases are:
>
> dEQP-GLES2.functional.color_clear.masked_scissored_rgb
> dEQP-GLES2.functional.color_clear.masked_scissored_rgba
>
> Note: I tried a virgl specific solution (crrev.com/c/909961), however
> that caused a regression in dEQP-GLES2.functional.depth_stencil_clear.depth
> since the cso_cache was not updated.
> ---
>  src/mesa/state_tracker/st_cb_clear.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/src/mesa/state_tracker/st_cb_clear.c b/src/mesa/state_tracker/st_cb_clear.c
> index 68677182ab..d8ad2b3b4a 100644
> --- a/src/mesa/state_tracker/st_cb_clear.c
> +++ b/src/mesa/state_tracker/st_cb_clear.c
> @@ -444,6 +444,13 @@ st_Clear(struct gl_context *ctx, GLbitfield mask)
>        clear_with_quad(ctx, quad_buffers);
>     }
>     if (clear_buffers) {
> +      /* The colormask may go from a masked value to being completely unmasked.
> +       * In that case, we should notify the driver via st_update_blend. The cso
> +       * cache should take care of not emitting old states.
> +       */
> +      if (clear_buffers & PIPE_CLEAR_COLOR)
> +         st_update_blend(st);
> +
>        /* We can't translate the clear color to the colorbuffer format,
>         * because different colorbuffers may have different formats.
>         */
> --
> 2.13.5
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev