[virglrenderer-devel] [PATCH v2] vrend: Associate host UBOs with the correct gallium uniform buffers

[Elie Tournier]

On Thu, May 03, 2018 at 06:33:48PM +0300, Alexandros Frantzis wrote:
> Take into account the gallium uniform buffer indices when associating
> host UBOs with gallium uniform buffers.
> 
> Previously the code disregarded the gallium uniform buffer indices,
> leading, under specific circumstances, to the provision of incorrect
> data to the shaders. The problem manifested typically when a context
> contained active UBOs which were not accessed by a particular shader,
> but ended up being used instead of the correct UBOs for that shader.
> This occurred, for example, when running the dEQP-GLES3.functional.ubo.*
> in batch mode, in which case left over UBOs from previous tests would
> cause subsequent tests to fail.
> 
> Fixes:
> dEQP-GLES3.functional.ubo.* when run in batch mode
> 
Tested-by: Elie Tournier <elie.tournier at collabora.com>
> Signed-off-by: Alexandros Frantzis <alexandros.frantzis at collabora.com>
> ---
> Changes in v2:
>  - Fill in only the used part of the ubo_idx array in vrend_shader_info
> 
>  src/vrend_renderer.c | 22 +++++++++++++++++++---
>  src/vrend_shader.c   |  1 +
>  src/vrend_shader.h   |  1 +
>  3 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/src/vrend_renderer.c b/src/vrend_renderer.c
> index 8e55f63..ac9723c 100644
> --- a/src/vrend_renderer.c
> +++ b/src/vrend_renderer.c
> @@ -1046,7 +1046,8 @@ static struct vrend_linked_shader_program *add_shader_program(struct vrend_conte
>  
>           sprog->ubo_locs[id] = calloc(sprog->ss[id]->sel->sinfo.num_ubos, sizeof(uint32_t));
>           for (i = 0; i < sprog->ss[id]->sel->sinfo.num_ubos; i++) {
> -            snprintf(name, 32, "%subo%d", prefix, i + 1);
> +            int ubo_idx = sprog->ss[id]->sel->sinfo.ubo_idx[i];
> +            snprintf(name, 32, "%subo%d", prefix, ubo_idx);
>              sprog->ubo_locs[id][i] = glGetUniformBlockIndex(prog_id, name);
>           }
>        } else
> @@ -2858,25 +2859,40 @@ static void vrend_draw_bind_ubo(struct vrend_context *ctx)
>     ubo_id = 0;
>     for (shader_type = PIPE_SHADER_VERTEX; shader_type <= ctx->sub->last_shader_idx; shader_type++) {
>        uint32_t mask;
> -      int shader_ubo_idx = 0;
> +      int shader_ubo_idx;
>        struct pipe_constant_buffer *cb;
>        struct vrend_resource *res;
> +      struct vrend_shader_info* sinfo;
> +
>        if (!ctx->sub->const_bufs_used_mask[shader_type])
>           continue;
>  
>        if (!ctx->sub->prog->ubo_locs[shader_type])
>           continue;
>  
> +      sinfo = &ctx->sub->prog->ss[shader_type]->sel->sinfo;
> +
>        mask = ctx->sub->const_bufs_used_mask[shader_type];
>        while (mask) {
> +         /* The const_bufs_used_mask stores the gallium uniform buffer indices */
>           i = u_bit_scan(&mask);
>  
> +         /* The cbs array is indexed using the gallium uniform buffer index */
>           cb = &ctx->sub->cbs[shader_type][i];
>           res = (struct vrend_resource *)cb->buffer;
> +
> +         /* Find the index of the uniform buffer in the array of shader ubo data */
> +         for (shader_ubo_idx = 0; shader_ubo_idx < sinfo->num_ubos; shader_ubo_idx++) {
> +            if (sinfo->ubo_idx[shader_ubo_idx] == i)
> +               break;
> +         }
> +         if (shader_ubo_idx == sinfo->num_ubos)
> +             continue;
> +
>           glBindBufferRange(GL_UNIFORM_BUFFER, ubo_id, res->id,
>                             cb->buffer_offset, cb->buffer_size);
> +         /* The ubo_locs array is indexed using the shader ubo index */
>           glUniformBlockBinding(ctx->sub->prog->id, ctx->sub->prog->ubo_locs[shader_type][shader_ubo_idx], ubo_id);
> -         shader_ubo_idx++;
>           ubo_id++;
>        }
>     }
> diff --git a/src/vrend_shader.c b/src/vrend_shader.c
> index bab603e..ba2c9c0 100644
> --- a/src/vrend_shader.c
> +++ b/src/vrend_shader.c
> @@ -2830,6 +2830,7 @@ char *vrend_convert_shader(struct vrend_shader_cfg *cfg,
>     sinfo->samplers_used_mask = ctx.samplers_used;
>     sinfo->num_consts = ctx.num_consts;
>     sinfo->num_ubos = ctx.num_ubo;
> +   memcpy(sinfo->ubo_idx, ctx.ubo_idx, ctx.num_ubo * sizeof(*ctx.ubo_idx));
>     sinfo->num_inputs = ctx.num_inputs;
>     sinfo->num_interps = ctx.num_interps;
>     sinfo->num_outputs = ctx.num_outputs;
> diff --git a/src/vrend_shader.h b/src/vrend_shader.h
> index e54a85c..681bfe2 100644
> --- a/src/vrend_shader.h
> +++ b/src/vrend_shader.h
> @@ -41,6 +41,7 @@ struct vrend_shader_info {
>     int num_interps;
>     int num_outputs;
>     int num_ubos;
> +   int ubo_idx[32];
>     int num_ucp;
>     int glsl_ver;
>     bool has_pervertex_out;
> -- 
> 2.17.0
> 
> _______________________________________________
> virglrenderer-devel mailing list
> virglrenderer-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/virglrenderer-devel