[PATCHv2 1/2] DRI2: Track DRI2 drawables as resources, not privates

[Pauli Nieminen]

2010/4/19 Michel Dänzer <michel at daenzer.net>:
> On Fre, 2010-04-09 at 14:18 -0400, Kristian Høgsberg wrote:
>> The main motivation here is to have the resource system clean up the
>> DRI2 drawable automatically so glx doesn't have to.  Right now, the
>> glx drawable resource must be destroyed before the X drawable, so that
>> calling DRI2DestroyDrawable doesn't crash.  By making the DRI2
>> drawable a resource, GLX doesn't have to worry about that and the
>> resource destruction order becomes irrelevant.
>>
>> https://bugs.freedesktop.org/show_bug.cgi?id=26394
>>
>> Signed-off-by: Kristian Høgsberg <krh at bitplanet.net>
>
> There's a problem with this change (and potentially in DRI2SwapEvent()
> even before it): at least the pixmaps backing redirected windows still
> have ->drawable.id == 0. This can result in weird behaviour with a GLX
> compositing manager, e.g. when running the same app twice in a row: the
> second window will show a static snapshot of the first one, because the
> compositing manager gets the DRI2 front buffer from the first, since
> destroyed window.
>
> The patch below fixes that symptom, but I'm not sure it's correct. Right
> now I'm getting memory corruption when logging out or just restarting
> compiz, but then again people seem to be reporting such symptoms against
> server-1.8-branch anyway.
>
> BTW, shouldn't ProcDRI2DestroyDrawable() still destroy/unreference the
> DRI2 drawable resource?
>
>
> diff --git a/glx/glxdri2.c b/glx/glxdri2.c
> index 9752003..87918d9 100644
> --- a/glx/glxdri2.c
> +++ b/glx/glxdri2.c
> @@ -451,7 +451,7 @@ __glXDRIscreenCreateDrawable(__GLXscreen *screen,
>     private->base.waitGL       = __glXDRIdrawableWaitGL;
>     private->base.waitX                = __glXDRIdrawableWaitX;
>
> -    if (DRI2CreateDrawable(pDraw)) {
> +    if (DRI2CreateDrawable(pDraw, drawId)) {
>            xfree(private);
>            return NULL;
>     }
> diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
> index 642ea19..05da225 100644
> --- a/hw/xfree86/dri2/dri2.c
> +++ b/hw/xfree86/dri2/dri2.c
> @@ -125,13 +125,16 @@ DRI2GetDrawable(DrawablePtr pDraw)
>  }
>
>  int
> -DRI2CreateDrawable(DrawablePtr pDraw)
> +DRI2CreateDrawable(DrawablePtr pDraw, XID id)
>  {
>     DRI2ScreenPtr   ds = DRI2GetScreen(pDraw->pScreen);
>     DRI2DrawablePtr pPriv;
>     CARD64          ust;
>     int                    rc;
>
> +    if (!pDraw->id)
> +       pDraw->id = id;
> +

I have to always set pDraw->id to id here or there is some rendering
errors. But I don't yet know where the incorrect old value is coming
from.

>     rc = dixLookupResourceByType((pointer *) &pPriv, pDraw->id,
>                                 dri2DrawableRes, NULL, DixReadAccess);
>     if (rc == Success || rc != BadValue)
> diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h
> index a695c6d..42dd66e 100644
> --- a/hw/xfree86/dri2/dri2.h
> +++ b/hw/xfree86/dri2/dri2.h
> @@ -199,7 +199,7 @@ extern _X_EXPORT Bool DRI2Connect(ScreenPtr pScreen,
>
>  extern _X_EXPORT Bool DRI2Authenticate(ScreenPtr pScreen, drm_magic_t magic);
>
> -extern _X_EXPORT int DRI2CreateDrawable(DrawablePtr pDraw);
> +extern _X_EXPORT int DRI2CreateDrawable(DrawablePtr pDraw, XID id);
>
>  extern _X_EXPORT void DRI2DestroyDrawable(DrawablePtr pDraw);
>
> diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c
> index 78f897b..6015126 100644
> --- a/hw/xfree86/dri2/dri2ext.c
> +++ b/hw/xfree86/dri2/dri2ext.c
> @@ -167,7 +167,7 @@ ProcDRI2CreateDrawable(ClientPtr client)
>                       &pDrawable, &status))
>        return status;
>
> -    status = DRI2CreateDrawable(pDrawable);
> +    status = DRI2CreateDrawable(pDrawable, stuff->drawable);
>     if (status != Success)
>        return status;
>
>
>
> --
> Earthling Michel Dänzer           |                http://www.vmware.com
> Libre software enthusiast         |          Debian, X and DRI developer
> _______________________________________________
> 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