[Mesa-dev] [PATCH 4/4] virgl: work around bad assumptions in virglrenderer

[Gert Wollny]

This fixes rendering in Supertuxkart, Serious Sam 3, and Shadow Warrior
via virgl for me. So for the series:

Tested-By: Gert Wollny <gert.wollny at collabora.com> 

Am Dienstag, den 11.12.2018, 15:26 +0100 schrieb Erik Faye-Lund:
> Virglrenderer does the wrong thing when given an instance divisor;
> it tries to use the element-index rather than the binding-index as
> the argument to glVertexBindingDivisor(). This worked fine as long
> as there was a 1:1 relationship between elements and bindings,
> which was the case util 19a91841c34 "st/mesa: Use Array._DrawVAO in
> st_atom_array.c.".
> 
> So let's detect instance divisors, and restore a 1:1 relationship in
> that case. This will make old versions of virglrenderer behave
> correctly. For newer versions, we can consider making a better
> interface, where the instance divisor isn't specified per element,
> but rather per binding. But let's save that for another day.
> 
> Signed-off-by: Erik Faye-Lund <erik.faye-lund at collabora.com>
> ---
>  src/gallium/drivers/virgl/virgl_context.c | 29
> ++++++++++++++++++++++-
>  1 file changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/src/gallium/drivers/virgl/virgl_context.c
> b/src/gallium/drivers/virgl/virgl_context.c
> index 121d9139f28..ce72f73a0f6 100644
> --- a/src/gallium/drivers/virgl/virgl_context.c
> +++ b/src/gallium/drivers/virgl/virgl_context.c
> @@ -50,6 +50,8 @@
>  
>  struct virgl_vertex_elements_state {
>     uint32_t handle;
> +   uint8_t binding_map[PIPE_MAX_ATTRIBS];
> +   uint8_t num_bindings;
>  };
>  
>  static uint32_t next_handle;
> @@ -390,10 +392,24 @@ static void
> *virgl_create_vertex_elements_state(struct pipe_context *ctx,
>                                                          unsigned
> num_elements,
>                                                          const struct
> pipe_vertex_element *elements)
>  {
> +   struct pipe_vertex_element new_elements[PIPE_MAX_ATTRIBS];
>     struct virgl_context *vctx = virgl_context(ctx);
>     struct virgl_vertex_elements_state *state =
>        CALLOC_STRUCT(virgl_vertex_elements_state);
>  
> +   for (int i = 0; i < num_elements; ++i) {
> +      if (elements[i].instance_divisor) {
> +	 for (int j = 0; j < num_elements; ++j) {
> +            new_elements[j] = elements[j];
> +            new_elements[j].vertex_buffer_index = j;
> +            state->binding_map[j] = elements[j].vertex_buffer_index;
> +	 }
> +	 elements = new_elements;
> +	 state->num_bindings = num_elements;
> +	 break;
> +      }
> +   }
> +
>     state->handle = virgl_object_assign_handle();
>     virgl_encoder_create_vertex_elements(vctx, state->handle,
>                                         num_elements, elements);
> @@ -419,6 +435,7 @@ static void
> virgl_bind_vertex_elements_state(struct pipe_context *ctx,
>     vctx->vertex_elements = state;
>     virgl_encode_bind_object(vctx, state ? state->handle : 0,
>                              VIRGL_OBJECT_VERTEX_ELEMENTS);
> +   vctx->vertex_array_dirty = TRUE;
>  }
>  
>  static void virgl_set_vertex_buffers(struct pipe_context *ctx,
> @@ -438,7 +455,17 @@ static void virgl_set_vertex_buffers(struct
> pipe_context *ctx,
>  static void virgl_hw_set_vertex_buffers(struct virgl_context *vctx)
>  {
>     if (vctx->vertex_array_dirty) {
> -      virgl_encoder_set_vertex_buffers(vctx, vctx-
> >num_vertex_buffers, vctx->vertex_buffer);
> +      struct virgl_vertex_elements_state *ve = vctx-
> >vertex_elements;
> +
> +      if (ve->num_bindings) {
> +         struct pipe_vertex_buffer vertex_buffers[PIPE_MAX_ATTRIBS];
> +         for (int i = 0; i < ve->num_bindings; ++i)
> +            vertex_buffers[i] = vctx->vertex_buffer[ve-
> >binding_map[i]];
> +
> +         virgl_encoder_set_vertex_buffers(vctx, ve->num_bindings,
> vertex_buffers);
> +      } else
> +         virgl_encoder_set_vertex_buffers(vctx, vctx-
> >num_vertex_buffers, vctx->vertex_buffer);
> +
>        virgl_attach_res_vertex_buffers(vctx);
>     }
>  }