[Mesa-dev] [PATCH] vbo: fix offset in minmax cache key

[Brian Paul]

On 08/30/2017 03:49 PM, Charmaine Lee wrote:
> Instead of saving primitive offset in the minmax cache key,
> save the actual buffer offset which is used in the cache lookup.
>
> Fixes rendering artifact seen with GoogleEarth when run with
> VMware driver.

Wow, nice working finding that!  Looks good to me.  Maybe Nicolai Hähnle 
should review too since he wrote the code.  I'm cc'ing him.

This patch should probably also be tagged for stable branches with:
     Cc: mesa-stable at lists.freedesktop.org


One nit below.


> ---
>   src/mesa/vbo/vbo_minmax_index.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/vbo/vbo_minmax_index.c b/src/mesa/vbo/vbo_minmax_index.c
> index 4c17a08..3bad158 100644
> --- a/src/mesa/vbo/vbo_minmax_index.c
> +++ b/src/mesa/vbo/vbo_minmax_index.c
> @@ -245,6 +245,7 @@ vbo_get_minmax_index(struct gl_context *ctx,
>         _mesa_primitive_restart_index(ctx, ib->index_size);
>      const char *indices;
>      GLuint i;
> +   GLintptr offset;

We should probably init offset to zero here just to silence potential 
compiler errors.  Even though the assignment to offset and the use of 
offset are predicated by the same condition, some compilers will warn 
about "possibly uninitialized read" (and probably only when 
optimizations are turned on, IIRC).

-Brian

>
>      indices = (char *) ib->ptr + prim->start * ib->index_size;
>      if (_mesa_is_bufferobj(ib->obj)) {
> @@ -254,7 +255,8 @@ vbo_get_minmax_index(struct gl_context *ctx,
>                                   count, min_index, max_index))
>            return;
>
> -      indices = ctx->Driver.MapBufferRange(ctx, (GLintptr) indices, size,
> +      offset = indices;
> +      indices = ctx->Driver.MapBufferRange(ctx, offset, size,
>                                              GL_MAP_READ_BIT, ib->obj,
>                                              MAP_INTERNAL);
>      }
> @@ -337,8 +339,8 @@ vbo_get_minmax_index(struct gl_context *ctx,
>      }
>
>      if (_mesa_is_bufferobj(ib->obj)) {
> -      vbo_minmax_cache_store(ctx, ib->obj, ib->index_size, prim->start, count,
> -                             *min_index, *max_index);
> +      vbo_minmax_cache_store(ctx, ib->obj, ib->index_size, offset,
> +                             count, *min_index, *max_index);
>         ctx->Driver.UnmapBuffer(ctx, ib->obj, MAP_INTERNAL);
>      }
>   }
>