[virglrenderer-devel] [PATCH] vrend: use full colormask before a clear, restore previous colormask afterwards

[Gurchetan Singh]

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 driver goes from 260 to 136 with no regressions.
Some representative test cases are:

dEQP-GLES2.functional.color_clear.masked_scissored_rgb
dEQP-GLES2.functional.color_clear.masked_scissored_rgba
dEQP-GLES2.functional.depth_stencil_clear.depth_stencil_scissored
dEQP-GLES2.functional.fragment_ops.random.0
dEQP-GLES2.functional.fragment_ops.interaction.basic_shader.0

v2: Revise comments, fix curly braces (Elie)
---
 src/vrend_renderer.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/src/vrend_renderer.c b/src/vrend_renderer.c
index ec37fc8..437fedf 100644
--- a/src/vrend_renderer.c
+++ b/src/vrend_renderer.c
@@ -2413,6 +2413,17 @@ void vrend_clear(struct vrend_context *ctx,
       } else {
          glClearColor(color->f[0], color->f[1], color->f[2], color->f[3]);
       }
+
+      /* This function implements Gallium's full clear callback (st->pipe->clear) on the host. This
+         callback requires no color component be masked. We must unmask all components before
+         calling glClear* and restore the previous colormask afterwards, as Gallium expects. */
+      if (ctx->sub->hw_blend_state.independent_blend_enable) {
+         /* ARB_draw_buffers_blend is required for this */
+         int i;
+         for (i = 0; i < PIPE_MAX_COLOR_BUFS; i++)
+            glColorMaskIndexedEXT(i, GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE);
+      } else
+         glColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE);
    }
 
    if (buffers & PIPE_CLEAR_DEPTH) {
@@ -2460,6 +2471,26 @@ void vrend_clear(struct vrend_context *ctx,
    if (buffers & PIPE_CLEAR_DEPTH)
       if (!ctx->sub->dsa_state.depth.writemask)
          glDepthMask(GL_FALSE);
+
+   /* Restore previous colormask */
+   if (buffers & PIPE_CLEAR_COLOR) {
+      if (ctx->sub->hw_blend_state.independent_blend_enable) {
+         /* ARB_draw_buffers_blend is required for this */
+         int i;
+         for (i = 0; i < PIPE_MAX_COLOR_BUFS; i++) {
+            struct pipe_blend_state *blend = &ctx->sub->hw_blend_state;
+            glColorMaskIndexedEXT(i, blend->rt[i].colormask & PIPE_MASK_R ? GL_TRUE : GL_FALSE,
+                                  blend->rt[i].colormask & PIPE_MASK_G ? GL_TRUE : GL_FALSE,
+                                  blend->rt[i].colormask & PIPE_MASK_B ? GL_TRUE : GL_FALSE,
+                                  blend->rt[i].colormask & PIPE_MASK_A ? GL_TRUE : GL_FALSE);
+         }
+      } else {
+         glColorMask(ctx->sub->hw_blend_state.rt[0].colormask & PIPE_MASK_R ? GL_TRUE : GL_FALSE,
+                     ctx->sub->hw_blend_state.rt[0].colormask & PIPE_MASK_G ? GL_TRUE : GL_FALSE,
+                     ctx->sub->hw_blend_state.rt[0].colormask & PIPE_MASK_B ? GL_TRUE : GL_FALSE,
+                     ctx->sub->hw_blend_state.rt[0].colormask & PIPE_MASK_A ? GL_TRUE : GL_FALSE);
+      }
+   }
 }
 
 static void vrend_update_scissor_state(struct vrend_context *ctx)
-- 
2.16.0.rc1.238.g530d649a79-goog