[Mesa-dev] [PATCH v2 2/3] mesa/glthread: add tracking of PBO binding

[Nicolai Hähnle]

On 01.04.2017 11:42, Gregory Hainaut wrote:
> In gl core, buffer must be reserved first by CreateBuffers/GenBuffers
> to be valid.
>
> Signed-off-by: Gregory Hainaut <gregory.hainaut at gmail.com>
> ---
>  src/mesa/main/marshal.c | 36 +++++++++++++++++++++++++++++++++---
>  1 file changed, 33 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/main/marshal.c b/src/mesa/main/marshal.c
> index 5a8354d..acbf6df 100644
> --- a/src/mesa/main/marshal.c
> +++ b/src/mesa/main/marshal.c
> @@ -290,7 +290,20 @@ _mesa_unmarshal_BindBufferBase(struct gl_context *ctx, const struct marshal_cmd_
>     CALL_BindBufferBase(ctx->CurrentServerDispatch, (target, index, buffer));
>  }
>
> -/** Tracks the current bindings for the vertex array and index array buffers.
> +/**
> + * Check that buffer is a valid buffer handle
> + * Always return false for ID 0.
> + */
> +static bool
> +is_bufferobj(struct gl_context *ctx, GLuint buffer)
> +{
> +   if (buffer == 0)
> +      return false;
> +   else
> +      return _mesa_HashLookup(ctx->Shared->ShadowBufferObjects, buffer) != NULL;
> +}
> +
> +/** Tracks the current bindings of GL buffer targets
>   *
>   * This is part of what we need to enable glthread on compat-GL contexts that
>   * happen to use VBOs, without also supporting the full tracking of VBO vs
> @@ -312,9 +325,14 @@ _mesa_unmarshal_BindBufferBase(struct gl_context *ctx, const struct marshal_cmd_
>   * instead of updating the binding.  However, compat GL has the ridiculous
>   * feature that if you pass a bad name, it just gens a buffer object for you,
>   * so we escape without having to know if things are valid or not.
> + *
> + * Code was extended to track pixel buffers so you know if pixel transfer
> + * goes to an user pointer (must be synchronous) or an GL buffer (could
> + * be asynchronous). Unlike VBO we need a stronger guarantee that buffer
> + * was reserved by GenBuffers or CreateBuffers.

Code comments should usually be written in a way that describes the 
present state, not the evolution of the code, except in unusual cases. 
Here, I'd suggest:

"Pixel buffers are tracked to decide whether pixel transfer goes to a 
user pointer (must be synchronous) or a GL buffer (can be asynchronous). 
Unlike for VBOs, we do need accurate tracking, since user pointers can 
be used in GL core contexts."

Cheers,
Nicolai


>   */
>  static void
> -track_vbo_binding(struct gl_context *ctx, GLenum target, GLuint buffer)
> +track_buffers_binding(struct gl_context *ctx, GLenum target, GLuint buffer)
>  {
>     struct glthread_state *glthread = ctx->GLThread;
>
> @@ -329,6 +347,18 @@ track_vbo_binding(struct gl_context *ctx, GLenum target, GLuint buffer)
>         */
>        glthread->element_array_is_vbo = (buffer != 0);
>        break;
> +   case GL_PIXEL_UNPACK_BUFFER:
> +      if (ctx->API == API_OPENGL_COMPAT || is_bufferobj(ctx, buffer))
> +         glthread->pixel_unpack_buffer_bound = buffer;
> +      else
> +         glthread->pixel_unpack_buffer_bound = 0;
> +      break;
> +   case GL_PIXEL_PACK_BUFFER:
> +      if (ctx->API == API_OPENGL_COMPAT || is_bufferobj(ctx, buffer))
> +         glthread->pixel_pack_buffer_bound = buffer;
> +      else
> +         glthread->pixel_pack_buffer_bound = 0;
> +      break;
>     }
>  }
>
> @@ -360,7 +390,7 @@ _mesa_marshal_BindBuffer(GLenum target, GLuint buffer)
>     struct marshal_cmd_BindBuffer *cmd;
>     debug_print_marshal("BindBuffer");
>
> -   track_vbo_binding(ctx, target, buffer);
> +   track_buffers_binding(ctx, target, buffer);
>
>     if (cmd_size <= MARSHAL_MAX_CMD_SIZE) {
>        cmd = _mesa_glthread_allocate_command(ctx, DISPATCH_CMD_BindBuffer,
>


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.