[PATCH 5/9] dri2: Keep DRI2Drawable resource allocated until creator frees it

[Pauli Nieminen]

On 04/02/11 13:33 +1100, ext Christopher James Halse Rogers wrote:
> On Thu, 2011-02-03 at 19:48 +0200, Pauli wrote:
> > From: Pauli Nieminen <ext-pauli.nieminen at nokia.com>
> > 
> > EGLImage requires that image siblings stay valid until all of them has
> > been freed. Base EGLImage is only required for creating new siblings.
> > 
> > http://www.khronos.org/registry/egl/extensions/KHR/EGL_KHR_image_base.txt
> > 
> > To keep DRI2Drawable until all siblings has been destroyed we need to
> > extent life time of DRI2Drawable beyond the Drawable.
> > 
> > We can keep the fake dri2 resource allocated. DRI2 can then searched
> > for fake resource  if client tries to use DRI2Drawable after Drawable was
> > destroyed.
> > 
> > Signed-off-by: Pauli Nieminen <ext-pauli.nieminen at nokia.com>
> > ---
> >  hw/xfree86/dri2/dri2.c    |   43 +++++++++++++++++++++++++++++--------------
> >  hw/xfree86/dri2/dri2.h    |    4 ++++
> >  hw/xfree86/dri2/dri2ext.c |    6 ++----
> >  3 files changed, 35 insertions(+), 18 deletions(-)
> > 
> > diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
> > index 21ee982..7746a1d 100644
> > --- a/hw/xfree86/dri2/dri2.c
> > +++ b/hw/xfree86/dri2/dri2.c
> > @@ -238,6 +238,26 @@ DRI2LookupClientDrawableRef(DRI2DrawablePtr pPriv, ClientPtr client, XID id)
> >      return NULL;
> >  }
> >  
> > +static Bool
> > +DRI2MatchDrawableRef(pointer data, XID fake_id, pointer cdata)
> > +{
> > +    DRI2DrawablePtr pPriv = data;
> > +    XID id = (XID)cdata;
> 
> This leads to (the technically correct) warning:
> dri2.c: In function ‘DRI2MatchDrawableRef’:
> dri2.c:245:14: warning: cast from pointer to integer of different size
> 
> I can't think off the top of my head how to silence that warning
> correctly, though.

64bit vs 32bit. Casting the pointer to uintptr_t would silence warning if I
remember correctly. Too bad older gcc that I have here didn't warn me. I will
try to fix the warning in weekend with newer gcc.

> 
> > +    DRI2DrawableRefPtr ref = DRI2LookupDrawableRef(pPriv, id);
> > +    return ref != NULL;
> > +}
> > +
> > +int
> > +DRI2LookupDrawableComplex(ClientPtr client, XID id, DRI2DrawablePtr *pPriv)
> > +{
> > +    DRI2DrawablePtr tmp = LookupClientResourceComplex(client, dri2DrawableRes,
> > +	    DRI2MatchDrawableRef, (pointer)id);
> 
> And similarly here.
> 
> > +    if (!tmp)
> > +	return BadDrawable;
> > +    *pPriv = tmp;
> > +    return Success;
> > +}
> > +
> >  static int
> >  DRI2AddDrawableRef(DRI2DrawablePtr pPriv, XID id, XID dri2_id,
> >  		   DRI2InvalidateProcPtr invalidate, void *priv)
> > @@ -307,9 +327,7 @@ static int DRI2DrawableGone(pointer p, XID id)
> >      DRI2DrawablePtr pPriv = p;
> >      DRI2ScreenPtr   ds = pPriv->dri2_screen;
> >      DRI2DrawableRefPtr ref, next;
> > -    WindowPtr pWin;
> > -    PixmapPtr pPixmap;
> > -    DrawablePtr pDraw;
> > +    DrawablePtr pDraw = pPriv->drawable;
> >      int i;
> >  
> >      list_for_each_entry_safe(ref, next, &pPriv->reference_list, link) {
> > @@ -324,11 +342,10 @@ static int DRI2DrawableGone(pointer p, XID id)
> >  	    break;
> >  	}
> >  
> > -	if (ref->id == id) {
> > -	    pPriv->refcnt--;
> > -	    list_del(&ref->link);
> > -	    FreeResourceByType(ref->dri2_id, dri2DrawableRes, TRUE);
> > -	    free(ref);
> > +	if (ref->id == id && pDraw && pDraw->type == DRAWABLE_WINDOW) {
> > +	    WindowPtr pWin = (WindowPtr) pDraw;
> > +	    dixSetPrivate(&pWin->devPrivates, dri2WindowPrivateKey, NULL);
> > +	    pPriv->drawable = NULL;
> >  	}
> >      }
> >  
> > @@ -336,8 +353,6 @@ static int DRI2DrawableGone(pointer p, XID id)
> >      if (pPriv->refcnt > 0)
> >  	return Success;
> >  
> > -    pDraw = pPriv->drawable;
> > -
> >      if (pPriv->buffers != NULL) {
> >  	for (i = 0; i < pPriv->bufferCount; i++)
> >  	    (*ds->DestroyBuffer)(pPriv, pPriv->buffers[i]);
> > @@ -345,11 +360,11 @@ static int DRI2DrawableGone(pointer p, XID id)
> >  	free(pPriv->buffers);
> >      }
> >  
> > -    if (pDraw->type == DRAWABLE_WINDOW) {
> > -	pWin = (WindowPtr) pDraw;
> > +    if (pDraw && pDraw->type == DRAWABLE_WINDOW) {
> > +	WindowPtr pWin = (WindowPtr) pDraw;
> >  	dixSetPrivate(&pWin->devPrivates, dri2WindowPrivateKey, NULL);
> > -    } else {
> > -	pPixmap = (PixmapPtr) pDraw;
> > +    } else if (pDraw) {
> > +	PixmapPtr pPixmap = (PixmapPtr) pDraw;
> >  	dixSetPrivate(&pPixmap->devPrivates, dri2PixmapPrivateKey, NULL);
> >  	pDraw->pScreen->DestroyPixmap(pPixmap);
> >      }
> > diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h
> > index 0182700..c324b8f 100644
> > --- a/hw/xfree86/dri2/dri2.h
> > +++ b/hw/xfree86/dri2/dri2.h
> > @@ -315,4 +315,8 @@ extern _X_EXPORT ScreenPtr DRI2DrawableGetScreen(DRI2DrawablePtr pPriv);
> >   * \return Valid DRI2DrawablePtr if DRI2Drawable exists. Otherwise NULL.
> >   */
> >  extern _X_EXPORT DRI2DrawablePtr DRI2GetDrawable(DrawablePtr pDraw);
> > +
> > +extern _X_EXPORT int DRI2LookupDrawableComplex(ClientPtr client,
> > +                                               XID id,
> > +                                               DRI2DrawablePtr *pPriv);
> >  #endif
> > diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c
> > index 639bbc5..be113cf 100644
> > --- a/hw/xfree86/dri2/dri2ext.c
> > +++ b/hw/xfree86/dri2/dri2ext.c
> > @@ -66,10 +66,8 @@ validDRI2Drawable(ClientPtr client, XID id, Mask access,
> >  
> >      rc = dixLookupResourceByType((pointer*)&pTmp, id, dri2DrawableRes, client, access);
> >  
> > -    if (rc == BadValue) {
> > -	*status = BadDrawable;
> > -	return FALSE;
> > -    }
> > +    if (rc == BadValue)
> > +	rc = DRI2LookupDrawableComplex(client, id, &pTmp);
> >      *status = rc;
> >      if (rc != Success)
> >  	return FALSE;
> 
> Reviewed-By: Christopher James Halse Rogers
> <christopher.halse.rogers at canonical.com>