[PATCH v2 05/10] dri2: Keep DRI2Drawable resource allocated until creator frees it

[Ville Syrjälä]

On Tue, Feb 08, 2011 at 11:42:51PM +0200, ext 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    |   41 +++++++++++++++++++++++++++++------------
>  hw/xfree86/dri2/dri2.h    |    4 ++++
>  hw/xfree86/dri2/dri2ext.c |   15 +++++++++------
>  3 files changed, 42 insertions(+), 18 deletions(-)
> 
> diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
> index dcc53c0..1693546 100644
> --- a/hw/xfree86/dri2/dri2.c
> +++ b/hw/xfree86/dri2/dri2.c
> @@ -243,6 +243,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)(uintptr_t)cdata;
> +    DRI2DrawableRefPtr ref = DRI2LookupDrawableRef(pPriv, id);
> +    return ref != NULL;
> +}
> +
> +int
> +DRI2LookupDrawableComplex(ClientPtr client, XID id, DRI2DrawablePtr *pPriv)
> +{
> +    DRI2DrawablePtr tmp = LookupClientResourceComplex(client, dri2DrawableRes,
> +	    DRI2MatchDrawableRef, (pointer)(uintptr_t)id);
> +    if (!tmp)
> +	return BadDrawable;
> +    *pPriv = tmp;
> +    return Success;
> +}
> +
>  static int
>  DRI2AddDrawableRef(DRI2DrawablePtr pPriv, XID id, XID dri2_id,
>  		   DRI2InvalidateProcPtr invalidate, void *priv)
> @@ -312,9 +332,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) {
> @@ -329,11 +347,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;
>  	}
>      }
>  
> @@ -348,11 +365,11 @@ static int DRI2DrawableGone(pointer p, XID id)
>  
>      pDraw = pPriv->drawable;
>  
> -    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 3dec65e..4616dc5 100644
> --- a/hw/xfree86/dri2/dri2.h
> +++ b/hw/xfree86/dri2/dri2.h
> @@ -342,4 +342,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 a8c5c50..2851573 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;
> @@ -239,11 +237,16 @@ send_buffers_reply(ClientPtr client, DRI2DrawablePtr pPriv,
>      int skip = 0;
>      int i;
>      DrawablePtr pDrawable = DRI2DrawableGetDrawable(pPriv);
> +    unsigned char type = DRAWABLE_WINDOW;
>  
>      if (buffers == NULL)
>  	    return BadAlloc;
>  
> -    if (pDrawable->type == DRAWABLE_WINDOW) {
> +    /* If drawable is NULL that means drawable was a window */
> +    if (pDrawable)
> +	type = pDrawable->type;

Maybe it would be cleaner to add 'type' also to DRI2Drawable? That would
make the structure bigger though. Or maybe add a small helper eg.
'int DRI2DrawableType(DRI2DrawablePtr pPriv)' that would hide this subtle
detail about 'NULL drawable == window' from the caller.

> +
> +    if (type == DRAWABLE_WINDOW) {
>  	for (i = 0; i < count; i++) {
>  	    /* Do not send the real front buffer of a window to the client.
>  	     */
> @@ -267,7 +270,7 @@ send_buffers_reply(ClientPtr client, DRI2DrawablePtr pPriv,
>  
>  	/* Do not send the real front buffer of a window to the client.
>  	 */
> -	if (pDrawable->type == DRAWABLE_WINDOW
> +	if (type == DRAWABLE_WINDOW
>  	    && (buffers[i]->attachment == DRI2BufferFrontLeft)) {
>  	    continue;
>  	}
> -- 
> 1.7.0.4
> 
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel

-- 
Ville Syrjälä