On Don, 2010-12-16 at 11:54 -0500, Kristian Høgsberg wrote:
> From: Chris Wilson <[email protected]>
>
> Although there may be more than one resource handles pointing to the
> Drawable, we only want to destroy it once and only reference the
> resource which may have just been deleted on the first instance.
>
> v2: Apply fixes and combine with another bug fix from Michel Dänzer,
> https://bugs.freedesktop.org/show_bug.cgi?id=28181
> v3: Just use the refcnt and don't try to free other resources in the
> DrawableGone callback.
>
> Signed-off-by: Kristian Høgsberg <[email protected]>
>
> Can we just do this instead? Didn't test the patch, but this should do
> the same and actually simplifies the code.
>
> Kristian
>
> ---
> glx/glxcmds.c | 13 +++++++++----
> glx/glxdrawable.h | 2 ++
> glx/glxext.c | 13 +++----------
> 3 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/glx/glxcmds.c b/glx/glxcmds.c
> index 8d13c15..1e8044b 100644
> --- a/glx/glxcmds.c
> +++ b/glx/glxcmds.c
> @@ -1131,14 +1133,17 @@ DoCreateGLXDrawable(ClientPtr client, __GLXscreen
> *pGlxScreen,
>
> /* Add the glx drawable under the XID of the underlying X drawable
> * too. That way we'll get a callback in DrawableGone and can
> * clean up properly when the drawable is destroyed. */
> - if (drawableId != glxDrawableId &&
> - !AddResource(pDraw->id, __glXDrawableRes, pGlxDraw)) {
> - pGlxDraw->destroy (pGlxDraw);
> - return BadAlloc;
> + if (drawableId != glxDrawableId) {
> + if (!AddResource(pDraw->id, __glXDrawableRes, pGlxDraw)) {
> + pGlxDraw->destroy (pGlxDraw);
> + return BadAlloc;
> + }
> + pGlxDraw->refcnt++;
The fixed version of this part from Chris' v2 patch is needed.
> diff --git a/glx/glxext.c b/glx/glxext.c
> index f5ebe4f..499567a 100644
> --- a/glx/glxext.c
> +++ b/glx/glxext.c
> @@ -126,16 +126,9 @@ static Bool DrawableGone(__GLXdrawable *glxPriv, XID xid)
> {
> __GLXcontext *c, *next;
>
> - /* If this drawable was created using glx 1.3 drawable
> - * constructors, we added it as a glx drawable resource under both
> - * its glx drawable ID and it X drawable ID. Remove the other
> - * resource now so we don't a callback for freed memory. */
> - if (glxPriv->drawId != glxPriv->pDraw->id) {
> - if (xid == glxPriv->drawId)
> - FreeResourceByType(glxPriv->pDraw->id, __glXDrawableRes, TRUE);
> - else
> - FreeResourceByType(glxPriv->drawId, __glXDrawableRes, TRUE);
> - }
> + glxPriv->refcnt--;
> + if (glxPriv->refcnt > 0)
> + return True;
If the IDs are different, doesn't this mean the GLX drawable will never
actually be destroyed before the X drawable?
--
Earthling Michel Dänzer | http://www.vmware.com
Libre software enthusiast | Debian, X and DRI developer
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
