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

[Michel Dänzer]

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