On Thu, Feb 8, 2018 at 4:46 PM, Marek Olšák <mar...@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, 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