[Mesa-dev] [PATCH] mesa: handle zero-size buffers in MapBuffer and ranges in MapBufferRange (v2)

[Brian Paul]

On 08/30/2011 02:34 PM, Marek Olšák wrote:
> ---
>   src/mesa/main/bufferobj.c                    |   14 ++++++++++-
>   src/mesa/state_tracker/st_cb_bufferobjects.c |   32 ++++++-------------------
>   2 files changed, 21 insertions(+), 25 deletions(-)
>
> diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
> index c453f9c..7e36809 100644
> --- a/src/mesa/main/bufferobj.c
> +++ b/src/mesa/main/bufferobj.c
> @@ -1004,6 +1004,12 @@ _mesa_MapBufferARB(GLenum target, GLenum access)
>         return NULL;
>      }
>
> +   if (!bufObj->Size) {
> +      _mesa_error(ctx, GL_OUT_OF_MEMORY,
> +                  "glMapBufferRange(buffer size = 0)");

The error string should be "glMapBuffer(buffer size = 0)"


> +      return NULL;
> +   }
> +
>      ASSERT(ctx->Driver.MapBufferRange);
>      map = ctx->Driver.MapBufferRange(ctx, 0, bufObj->Size, accessFlags, bufObj);
>      if (!map) {
> @@ -1410,7 +1416,13 @@ _mesa_MapBufferRange(GLenum target, GLintptr offset, GLsizeiptr length,
>                     "glMapBufferRange(buffer already mapped)");
>         return NULL;
>      }
> -
> +
> +   if (!length) {
> +      _mesa_error(ctx, GL_OUT_OF_MEMORY,
> +                  "glMapBufferRange(length = 0)");
> +      return NULL;
> +   }

This isn't quite right.  If the bufObj->Size == 0, we should raise the 
error.

If length==0, the mapping should succeed and return a non-null value. 
Of course, the caller can't legally read or write anything in the 
0-sized aperture.

That's what NVIDIA does.

-Brian

> +
>      ASSERT(ctx->Driver.MapBufferRange);
>      map = ctx->Driver.MapBufferRange(ctx, offset, length, access, bufObj);
>      if (!map) {
> diff --git a/src/mesa/state_tracker/st_cb_bufferobjects.c b/src/mesa/state_tracker/st_cb_bufferobjects.c
> index a451b44..aab7444 100644
> --- a/src/mesa/state_tracker/st_cb_bufferobjects.c
> +++ b/src/mesa/state_tracker/st_cb_bufferobjects.c
> @@ -229,13 +229,6 @@ st_bufferobj_data(struct gl_context *ctx,
>
>
>   /**
> - * Dummy data whose's pointer is used for zero size buffers or ranges.
> - */
> -static long st_bufferobj_zero_length = 0;
> -
> -
> -
> -/**
>    * Called via glMapBufferRange().
>    */
>   static void *
> @@ -280,24 +273,15 @@ st_bufferobj_map_range(struct gl_context *ctx,
>      assert(offset<  obj->Size);
>      assert(offset + length<= obj->Size);
>
> -   /*
> -    * We go out of way here to hide the degenerate yet valid case of zero
> -    * length range from the pipe driver.
> -    */
> -   if (!length) {
> -      obj->Pointer =&st_bufferobj_zero_length;
> -   }
> -   else {
> -      obj->Pointer = pipe_buffer_map_range(pipe,
> -                                           st_obj->buffer,
> -                                           offset, length,
> -                                           flags,
> -&st_obj->transfer);
> -      if (obj->Pointer) {
> -         obj->Pointer = (ubyte *) obj->Pointer + offset;
> -      }
> +   obj->Pointer = pipe_buffer_map_range(pipe,
> +                                        st_obj->buffer,
> +                                        offset, length,
> +                                        flags,
> +&st_obj->transfer);
> +   if (obj->Pointer) {
> +      obj->Pointer = (ubyte *) obj->Pointer + offset;
>      }
> -
> +
>      if (obj->Pointer) {
>         obj->Offset = offset;
>         obj->Length = length;

Reviewed-by: Brian Paul <brianp at vmware.com>