[Mesa-dev] [PATCH 1/3] mesa: Fix handling of glCopyBufferSubData() for src == dst.

[Ian Romanick]

On 01/25/2012 05:51 PM, Eric Anholt wrote:
> Fixes piglit ARB_copy_buffer-overlap, which previously assertion failed.

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

One other orthogonal question below...

> ---
>   src/mesa/main/bufferobj.c |   25 +++++++++++++++++++------
>   1 files changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
> index 5b6db78..e4f964f 100644
> --- a/src/mesa/main/bufferobj.c
> +++ b/src/mesa/main/bufferobj.c
> @@ -526,11 +526,23 @@ _mesa_copy_buffer_subdata(struct gl_context *ctx,
>      assert(!_mesa_bufferobj_mapped(src));
>      assert(!_mesa_bufferobj_mapped(dst));
>
> -   srcPtr = ctx->Driver.MapBufferRange(ctx, readOffset, size,
> -                                       GL_MAP_READ_BIT, src);
> -   dstPtr = ctx->Driver.MapBufferRange(ctx, writeOffset, size,
> -                                       (GL_MAP_WRITE_BIT |
> -                                        GL_MAP_INVALIDATE_RANGE_BIT), dst);
> +   if (src == dst) {
> +      srcPtr = dstPtr = ctx->Driver.MapBufferRange(ctx, 0, src->Size,
> +						   GL_MAP_READ_BIT |
> +						   GL_MAP_WRITE_BIT, src);
> +
> +      if (!srcPtr)
> +	 return;
> +
> +      srcPtr += readOffset;
> +      dstPtr += writeOffset;
> +   } else {
> +      srcPtr = ctx->Driver.MapBufferRange(ctx, readOffset, size,
> +					  GL_MAP_READ_BIT, src);
> +      dstPtr = ctx->Driver.MapBufferRange(ctx, writeOffset, size,
> +					  (GL_MAP_WRITE_BIT |
> +					   GL_MAP_INVALIDATE_RANGE_BIT), dst);
> +   }
>
>      /* Note: the src and dst regions will never overlap.  Trying to do so
>       * would generate GL_INVALID_VALUE earlier.

So we have a test for the assertion in this comment?  That's the only 
case that concerns me in this code.

> @@ -539,7 +551,8 @@ _mesa_copy_buffer_subdata(struct gl_context *ctx,
>         memcpy(dstPtr, srcPtr, size);
>
>      ctx->Driver.UnmapBuffer(ctx, src);
> -   ctx->Driver.UnmapBuffer(ctx, dst);
> +   if (dst != src)
> +      ctx->Driver.UnmapBuffer(ctx, dst);
>   }
>
>