[Mesa-dev] [PATCH] virgl: clear render state before submitting clear command

[Gurchetan Singh]

In vrend_clear, we already save and restore colormasks and stencils:

https://cgit.freedesktop.org/virglrenderer/commit/?id=b75e0a1dabdfbda44c310a69026a9dbd7d980294
https://cgit.freedesktop.org/virglrenderer/commit/?id=252b00d77c30ce39608c1a9de18523cbdcaca623

It would be nice if we can put everything on the host side.  Wouldn't
the following code also solve the RASTERIZER_DISCARD problem?

if (ctx->sub->hw_rs_state.rasterizer_discard)
    glDisable(GL_RASTERIZER_DISCARD)

...

glClear(..)

...

if (ctx->sub->hw_rs_state.rasterizer_discard)
    glEnable(GL_RASTERIZER_DISCARD)


On Mon, May 7, 2018 at 1:18 AM, Gert Wollny <gert.wollny at collabora.com> wrote:
> Virgl/virglrenderer implements the gallium clear command via a call to
> glClear. However, gallium clear does not take render states like e.g.
> RASTERIZER_DISCARD into account, but glClear does, and gallium st_Clear
> does not updated the latest rendering state on the host. Hence, in order
> to properly emulate gallium clear with glClear, all rendering states that
> influence glClear must be set to their default values on the host before
> submitting the actual clear command and restore the original state
> afterwards (Thanks to Ilia Mirkin for helpint me understand this).
>
> This patch implements the according mechanism in virgl_clear, the clear state
> is defined like in mesa/st/st_cb_clear.c: st_init_clear.
>
> Fixes CTS:
>    dEQP-GLES3.functional.rasterizer_discard.fbo.*
>
> Signed-off-by: Gert Wollny <gert.wollny at collabora.com>
> ---
> PS : I have no mesa commits rights.
>
>  src/gallium/drivers/virgl/virgl_context.c | 24 ++++++++++++++++++-----
>  src/gallium/drivers/virgl/virgl_context.h |  2 ++
>  2 files changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/src/gallium/drivers/virgl/virgl_context.c b/src/gallium/drivers/virgl/virgl_context.c
> index 8d701bb8f4..55ddbcf7df 100644
> --- a/src/gallium/drivers/virgl/virgl_context.c
> +++ b/src/gallium/drivers/virgl/virgl_context.c
> @@ -305,9 +305,8 @@ static void virgl_bind_rasterizer_state(struct pipe_context *ctx,
>                                                  void *rs_state)
>  {
>     struct virgl_context *vctx = virgl_context(ctx);
> -   uint32_t handle = (unsigned long)rs_state;
> -
> -   virgl_encode_bind_object(vctx, handle, VIRGL_OBJECT_RASTERIZER);
> +   vctx->last_render_state = rs_state;
> +   virgl_encode_bind_object(vctx, (unsigned long)rs_state, VIRGL_OBJECT_RASTERIZER);
>  }
>
>  static void virgl_delete_rasterizer_state(struct pipe_context *ctx,
> @@ -568,8 +567,19 @@ static void virgl_clear(struct pipe_context *ctx,
>                                  double depth, unsigned stencil)
>  {
>     struct virgl_context *vctx = virgl_context(ctx);
> +   struct pipe_rasterizer_state *old_state = vctx->last_render_state;
>
> +   if (!vctx->clear_render_state) {
> +      struct pipe_rasterizer_state clear_state;
> +      memset(&clear_state, 0, sizeof(clear_state));
> +      clear_state.depth_clip = 1;
> +      clear_state.bottom_edge_rule = 1;
> +      clear_state.half_pixel_center = 1;
> +      vctx->clear_render_state = virgl_create_rasterizer_state(ctx, &clear_state);
> +   }
> +   virgl_bind_rasterizer_state(ctx, vctx->clear_render_state);
>     virgl_encode_clear(vctx, buffers, color, depth, stencil);
> +   virgl_bind_rasterizer_state(ctx, old_state);
>  }
>
>  static void virgl_draw_vbo(struct pipe_context *ctx,
> @@ -844,6 +854,9 @@ virgl_context_destroy( struct pipe_context *ctx )
>     struct virgl_context *vctx = virgl_context(ctx);
>     struct virgl_screen *rs = virgl_screen(ctx->screen);
>
> +   if (vctx->clear_render_state)
> +      virgl_delete_rasterizer_state(ctx, vctx->clear_render_state);
> +
>     vctx->framebuffer.zsbuf = NULL;
>     vctx->framebuffer.nr_cbufs = 0;
>     virgl_encoder_destroy_sub_ctx(vctx, vctx->hw_sub_ctx_id);
> @@ -948,6 +961,7 @@ struct pipe_context *virgl_context_create(struct pipe_screen *pscreen,
>     virgl_encoder_create_sub_ctx(vctx, vctx->hw_sub_ctx_id);
>
>     virgl_encoder_set_sub_ctx(vctx, vctx->hw_sub_ctx_id);
> +
>     return &vctx->base;
>  fail:
>     return NULL;
> diff --git a/src/gallium/drivers/virgl/virgl_context.h b/src/gallium/drivers/virgl/virgl_context.h
> index 3492dcfa49..a1c1d1a60c 100644
> --- a/src/gallium/drivers/virgl/virgl_context.h
> +++ b/src/gallium/drivers/virgl/virgl_context.h
> @@ -74,6 +74,8 @@ struct virgl_context {
>
>     struct primconvert_context *primconvert;
>     uint32_t hw_sub_ctx_id;
> +   void *last_render_state;
> +   void *clear_render_state;
>  };
>
>  static inline struct virgl_sampler_view *
> --
> 2.17.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev