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

[Pauli Nieminen]

On Mon, Apr 26, 2010 at 3:44 PM, Pauli Nieminen <suokkos at gmail.com> wrote:
> 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.
>

Sorry. I can't reproduce the problem after recompilation. It looks
like some compilation problem in my original binary instead of code
bug.

>>     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
>