[Mesa-dev] [PATCH 6/7] glx: remove incorrect refcounting of DRIdrawable

Ian Romanick idr at freedesktop.org
Tue Feb 9 01:11:38 UTC 2016


On 02/05/2016 01:11 PM, Miklós Máté wrote:
> dri drawables must never be released when unbound from a context
> as long as their corresponding glx objects (window, pixmap, pbuffer)
> still exist

I'd really like to have Kristian weigh in, since DRI2 was his design,
and this is all his code being affected.  That said, I'm a little unsure
about this change.

The DRI drawables and associated resources will still get freed when the
application eventually calls glXDestroyPBuffer or glXDestroyWindow.

But I'm not 100% sure what will happen with GLX 1.2 applications...
there are no glXDestroy* functions in GLX 1.2.  In that case the
application will have a soft leak because calling XDestroyWindow won't
do anything on the client (or server?) for the DRI resources.  I suspect
that's what this code was trying to prevent.

> this fixes fd.o bug #93955 and disappearing characters in KotOR when
> soft shadows are enabled

I think this is also a candidate for stable.  Reference this in the
commit message like:

This fixes disappearing characters in KotOR when soft shadows are enabled.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93955
Cc: "11.1" <mesa-stable at lists.freedesktop.org>

> ---
>  src/glx/dri2_glx.c   |  4 ----
>  src/glx/dri3_glx.c   |  4 ----
>  src/glx/dri_common.c | 38 --------------------------------------
>  src/glx/dri_glx.c    |  4 ----
>  src/glx/drisw_glx.c  |  4 ----
>  src/glx/glxclient.h  |  1 -
>  6 files changed, 55 deletions(-)
> 
> diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c
> index 7710349..ebc878f 100644
> --- a/src/glx/dri2_glx.c
> +++ b/src/glx/dri2_glx.c
> @@ -122,8 +122,6 @@ dri2_destroy_context(struct glx_context *context)
>     struct dri2_context *pcp = (struct dri2_context *) context;
>     struct dri2_screen *psc = (struct dri2_screen *) context->psc;
>  
> -   driReleaseDrawables(&pcp->base);
> -
>     free((char *) context->extensions);
>  
>     (*psc->core->destroyContext) (pcp->driContext);
> @@ -145,8 +143,6 @@ dri2_bind_context(struct glx_context *context, struct glx_context *old,
>     pdraw = (struct dri2_drawable *) driFetchDrawable(context, draw);
>     pread = (struct dri2_drawable *) driFetchDrawable(context, read);
>  
> -   driReleaseDrawables(&pcp->base);
> -
>     if (pdraw)
>        dri_draw = pdraw->driDrawable;
>     else if (draw != None)
> diff --git a/src/glx/dri3_glx.c b/src/glx/dri3_glx.c
> index 6054ffc..38f5799 100644
> --- a/src/glx/dri3_glx.c
> +++ b/src/glx/dri3_glx.c
> @@ -186,8 +186,6 @@ dri3_destroy_context(struct glx_context *context)
>     struct dri3_context *pcp = (struct dri3_context *) context;
>     struct dri3_screen *psc = (struct dri3_screen *) context->psc;
>  
> -   driReleaseDrawables(&pcp->base);
> -
>     free((char *) context->extensions);
>  
>     (*psc->core->destroyContext) (pcp->driContext);
> @@ -206,8 +204,6 @@ dri3_bind_context(struct glx_context *context, struct glx_context *old,
>     pdraw = (struct dri3_drawable *) driFetchDrawable(context, draw);
>     pread = (struct dri3_drawable *) driFetchDrawable(context, read);
>  
> -   driReleaseDrawables(&pcp->base);
> -
>     if (pdraw == NULL || pread == NULL)
>        return GLXBadDrawable;
>  
> diff --git a/src/glx/dri_common.c b/src/glx/dri_common.c
> index 6728d38..2d334ab 100644
> --- a/src/glx/dri_common.c
> +++ b/src/glx/dri_common.c
> @@ -404,7 +404,6 @@ driFetchDrawable(struct glx_context *gc, GLXDrawable glxDrawable)
>        return NULL;
>  
>     if (__glxHashLookup(priv->drawHash, glxDrawable, (void *) &pdraw) == 0) {
> -      pdraw->refcount ++;
>        return pdraw;
>     }
>  
> @@ -420,47 +419,10 @@ driFetchDrawable(struct glx_context *gc, GLXDrawable glxDrawable)
>        (*pdraw->destroyDrawable) (pdraw);
>        return NULL;
>     }
> -   pdraw->refcount = 1;
>  
>     return pdraw;
>  }
>  
> -_X_HIDDEN void
> -driReleaseDrawables(struct glx_context *gc)
> -{
> -   const struct glx_display *priv = gc->psc->display;
> -   __GLXDRIdrawable *pdraw;
> -
> -   if (priv == NULL)
> -      return;
> -
> -   if (__glxHashLookup(priv->drawHash,
> -		       gc->currentDrawable, (void *) &pdraw) == 0) {
> -      if (pdraw->drawable == pdraw->xDrawable) {
> -	 pdraw->refcount --;
> -	 if (pdraw->refcount == 0) {
> -	    (*pdraw->destroyDrawable)(pdraw);
> -	    __glxHashDelete(priv->drawHash, gc->currentDrawable);
> -	 }
> -      }
> -   }
> -
> -   if (__glxHashLookup(priv->drawHash,
> -		       gc->currentReadable, (void *) &pdraw) == 0) {
> -      if (pdraw->drawable == pdraw->xDrawable) {
> -	 pdraw->refcount --;
> -	 if (pdraw->refcount == 0) {
> -	    (*pdraw->destroyDrawable)(pdraw);
> -	    __glxHashDelete(priv->drawHash, gc->currentReadable);
> -	 }
> -      }
> -   }
> -
> -   gc->currentDrawable = None;
> -   gc->currentReadable = None;
> -
> -}
> -
>  _X_HIDDEN bool
>  dri2_convert_glx_attribs(unsigned num_attribs, const uint32_t *attribs,
>                           unsigned *major_ver, unsigned *minor_ver,
> diff --git a/src/glx/dri_glx.c b/src/glx/dri_glx.c
> index d087751..69d7502 100644
> --- a/src/glx/dri_glx.c
> +++ b/src/glx/dri_glx.c
> @@ -526,8 +526,6 @@ dri_destroy_context(struct glx_context * context)
>     struct dri_context *pcp = (struct dri_context *) context;
>     struct dri_screen *psc = (struct dri_screen *) context->psc;
>  
> -   driReleaseDrawables(&pcp->base);
> -
>     free((char *) context->extensions);
>  
>     (*psc->core->destroyContext) (pcp->driContext);
> @@ -547,8 +545,6 @@ dri_bind_context(struct glx_context *context, struct glx_context *old,
>     pdraw = (struct dri_drawable *) driFetchDrawable(context, draw);
>     pread = (struct dri_drawable *) driFetchDrawable(context, read);
>  
> -   driReleaseDrawables(&pcp->base);
> -
>     if (pdraw == NULL || pread == NULL)
>        return GLXBadDrawable;
>  
> diff --git a/src/glx/drisw_glx.c b/src/glx/drisw_glx.c
> index 241ac7f..2350292 100644
> --- a/src/glx/drisw_glx.c
> +++ b/src/glx/drisw_glx.c
> @@ -234,8 +234,6 @@ drisw_destroy_context(struct glx_context *context)
>     struct drisw_context *pcp = (struct drisw_context *) context;
>     struct drisw_screen *psc = (struct drisw_screen *) context->psc;
>  
> -   driReleaseDrawables(&pcp->base);
> -
>     free((char *) context->extensions);
>  
>     (*psc->core->destroyContext) (pcp->driContext);
> @@ -254,8 +252,6 @@ drisw_bind_context(struct glx_context *context, struct glx_context *old,
>     pdraw = (struct drisw_drawable *) driFetchDrawable(context, draw);
>     pread = (struct drisw_drawable *) driFetchDrawable(context, read);
>  
> -   driReleaseDrawables(&pcp->base);
> -
>     if (pdraw == NULL || pread == NULL)
>        return GLXBadDrawable;
>  
> diff --git a/src/glx/glxclient.h b/src/glx/glxclient.h
> index 2776b44..9105be3 100644
> --- a/src/glx/glxclient.h
> +++ b/src/glx/glxclient.h
> @@ -134,7 +134,6 @@ struct __GLXDRIdrawableRec
>     GLenum textureTarget;
>     GLenum textureFormat;        /* EXT_texture_from_pixmap support */
>     unsigned long eventMask;
> -   int refcount;
>  };
>  
>  /*
> 



More information about the mesa-dev mailing list