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, gurchetansi...@chromium.org <gurchetansi...@chromium.org> wrote: > From: Gurchetan Singh <gurchetansi...@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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev