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

[Gurchetan Singh]

On Thu, Feb 8, 2018 at 4:46 PM, Marek Olšák <maraeo at gmail.com> wrote:

> 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.
>

clear_with_quad is only called if a color mask exists
(see is_color_masked).  When the state transitions from color-mask exists
--> completely unmasked (the clear_buffers case), how is the driver
supposed to be notified?


> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180208/1e2f4bad/attachment.html>