[Mesa-dev] [PATCH 1/4] mesa: only lock framebuffer in compat profile

[Nicolai Hähnle]

On 24.04.2017 07:28, Timothy Arceri wrote:
> From the EXT_framebuffer_object spec:
>
>    "Framebuffer objects created with the commands defined
>    by the  GL_EXT_framebuffer_object extension are defined
>    to be shared, while FBOs created with commands defined
>    by the OpenGL core or GL_ARB_framebuffer_object
>    extension are defined *not* to be shared.  However, the
>    following functions are viewed as aliases (in particular
>    the opcodes for X are also the same) between the functions
>    of GL_EXT_framebuffer_object and
>    GL_ARB_framebuffer_object:
>
>       ...
>
>    Since the above pairs are aliases, the functions of a
>    pair are equivalent.  Note that the functions
>    BindFramebuffer and BindFramebufferEXT are not aliases
>    and neither are the functions BindRenderbuffer and
>    BindRenderbufferEXT.  Because object creation  occurs
>    when the framebuffer object is bound for the first time,
>    a framebuffer object can be shared across contexts only
>    if it was first bound with BindFramebufferEXT.
>    Framebuffers first bound with BindFramebuffer may not
>    be shared across contexts.  Framebuffer objects created
>    with BindFramebufferEXT may subsequently be bound using
>    BindFramebuffer.  Framebuffer objects created with
>    BindFramebuffer may be bound with BindFramebufferEXT
>    provided they are bound to the same context they were
>    created on."
>
> So in theory we could have a flag that is set by the bind
> functions to decide if to lock or not. However we only
> expose GL_EXT_framebuffer_object in compat profile so this
> change just uses that to decide if we should lock or not.
> ---
>  src/mesa/main/fbobject.c    | 45 ++++++++++++++++++++++++++--------
>  src/mesa/main/framebuffer.c | 60 +++++++++++++++++++++++++++++++--------------
>  2 files changed, 76 insertions(+), 29 deletions(-)
[snip]
> diff --git a/src/mesa/main/framebuffer.c b/src/mesa/main/framebuffer.c
> index 9002020..b42bfba 100644
> --- a/src/mesa/main/framebuffer.c
> +++ b/src/mesa/main/framebuffer.c
> @@ -234,42 +234,64 @@ _mesa_free_framebuffer_data(struct gl_framebuffer *fb)
>
>  /**
>   * Set *ptr to point to fb, with refcounting and locking.
>   * This is normally only called from the _mesa_reference_framebuffer() macro
>   * when there's a real pointer change.
>   */
>  void
>  _mesa_reference_framebuffer_(struct gl_framebuffer **ptr,
>                               struct gl_framebuffer *fb)
>  {
> -   if (*ptr) {
> -      /* unreference old renderbuffer */
> -      GLboolean deleteFlag = GL_FALSE;
> -      struct gl_framebuffer *oldFb = *ptr;
> -
> -      mtx_lock(&oldFb->Mutex);
> -      assert(oldFb->RefCount > 0);
> -      oldFb->RefCount--;
> -      deleteFlag = (oldFb->RefCount == 0);
> -      mtx_unlock(&oldFb->Mutex);
> +   GET_CURRENT_CONTEXT(ctx);
> +
> +   if (!ctx || ctx->API != API_OPENGL_COMPAT) {

When does !ctx happen, and does it really guarantee not having a 
compatibility context?

BTW, what are the rules for having contexts of different GL versions / 
different compatibility vs. core type in the same share group?

Cheers,
Nicolai


> +      if (*ptr) {
> +         /* unreference old renderbuffer */
> +         struct gl_framebuffer *oldFb = *ptr;
> +
> +         assert(oldFb->RefCount > 0);
> +         oldFb->RefCount--;
>
> -      if (deleteFlag)
> -         oldFb->Delete(oldFb);
> +         if (oldFb->RefCount == 0)
> +            oldFb->Delete(oldFb);
>
> -      *ptr = NULL;
> -   }
> +         *ptr = NULL;
> +      }
>
> -   if (fb) {
> -      mtx_lock(&fb->Mutex);
> -      fb->RefCount++;
> -      mtx_unlock(&fb->Mutex);
> -      *ptr = fb;
> +      if (fb) {
> +         fb->RefCount++;
> +         *ptr = fb;
> +      }
> +   } else {
> +      if (*ptr) {
> +         /* unreference old renderbuffer */
> +         bool deleteFlag = false;
> +         struct gl_framebuffer *oldFb = *ptr;
> +
> +         mtx_lock(&oldFb->Mutex);
> +         assert(oldFb->RefCount > 0);
> +         oldFb->RefCount--;
> +         deleteFlag = (oldFb->RefCount == 0);
> +         mtx_unlock(&oldFb->Mutex);
> +
> +         if (deleteFlag)
> +            oldFb->Delete(oldFb);
> +
> +         *ptr = NULL;
> +      }
> +
> +      if (fb) {
> +         mtx_lock(&fb->Mutex);
> +         fb->RefCount++;
> +         mtx_unlock(&fb->Mutex);
> +         *ptr = fb;
> +      }
>     }
>  }
>
>
>  /**
>   * Resize the given framebuffer's renderbuffers to the new width and height.
>   * This should only be used for window-system framebuffers, not
>   * user-created renderbuffers (i.e. made with GL_EXT_framebuffer_object).
>   * This will typically be called directly from a device driver.
>   *
>


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