From 252b00d77c30ce39608c1a9de18523cbdcaca623 Mon Sep 17 00:00:00 2001 From: Gurchetan Singh Date: Mon, 12 Feb 2018 09:37:13 -0800 Subject: [PATCH] vrend: use full colormask before a clear, restore previous colormask afterwards 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) Signed-off-by: Dave Airlie --- 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)