xserver: Branch 'server-1.8-branch' - 4 commits

Peter Hutterer whot at kemper.freedesktop.org
Tue Apr 20 17:53:32 PDT 2010


 glx/glxcmds.c             |   39 ++++++++-----
 glx/glxdri2.c             |    5 -
 glx/glxext.c              |   11 +++
 glx/glxscreens.c          |   28 ---------
 glx/glxscreens.h          |    1 
 hw/xfree86/dri2/dri2.c    |  133 ++++++++++++----------------------------------
 hw/xfree86/dri2/dri2ext.c |   22 -------
 7 files changed, 74 insertions(+), 165 deletions(-)

New commits:
commit 6ab87eb04cf24619c17c1fa05daaeebb481a6a50
Author: Kristian Høgsberg <krh at bitplanet.net>
Date:   Fri Apr 16 05:55:35 2010 -0400

    glx: Drop DestroyWindow hook
    
    Now that glx doesn't call DRI2DestroyDrawable anymore, we don't need to
    force a specific resource destruction order in the DestroyWindow hook.
    
    Signed-off-by: Kristian Høgsberg <krh at bitplanet.net>
    Reviewed-by: Michel Dänzer <michel at daenzer.net>
    
    https://bugs.freedesktop.org/show_bug.cgi?id=26394
    Signed-off-by: Keith Packard <keithp at keithp.com>

diff --git a/glx/glxscreens.c b/glx/glxscreens.c
index 58d8ee0..b75aea6 100644
--- a/glx/glxscreens.c
+++ b/glx/glxscreens.c
@@ -215,7 +215,6 @@ glxCloseScreen (int index, ScreenPtr pScreen)
     __GLXscreen *pGlxScreen = glxGetScreen(pScreen);
 
     pScreen->CloseScreen = pGlxScreen->CloseScreen;
-    pScreen->DestroyWindow = pGlxScreen->DestroyWindow;
 
     pGlxScreen->destroy(pGlxScreen);
 
@@ -347,31 +346,6 @@ pickFBConfig(__GLXscreen *pGlxScreen, VisualPtr visual)
     return best;
 }
 
-static Bool
-glxDestroyWindow(WindowPtr pWin)
-{
-    ScreenPtr pScreen = pWin->drawable.pScreen;
-    __GLXscreen *pGlxScreen = glxGetScreen(pScreen);
-    Bool retval = TRUE;
-
-    FreeResource(pWin->drawable.id, FALSE);
-
-    /* call lower wrapped functions */
-    if (pGlxScreen->DestroyWindow) {
-	/* unwrap */
-	pScreen->DestroyWindow = pGlxScreen->DestroyWindow;
-
-	/* call lower layers */
-	retval = (*pScreen->DestroyWindow)(pWin);
-
-	/* rewrap */
-	pGlxScreen->DestroyWindow = pScreen->DestroyWindow;
-	pScreen->DestroyWindow = glxDestroyWindow;
-    }
-
-    return retval;
-}
-
 void __glXScreenInit(__GLXscreen *pGlxScreen, ScreenPtr pScreen)
 {
     __GLXconfig *m;
@@ -394,8 +368,6 @@ void __glXScreenInit(__GLXscreen *pGlxScreen, ScreenPtr pScreen)
 
     pGlxScreen->CloseScreen = pScreen->CloseScreen;
     pScreen->CloseScreen = glxCloseScreen;
-    pGlxScreen->DestroyWindow = pScreen->DestroyWindow;
-    pScreen->DestroyWindow = glxDestroyWindow;
 
     i = 0;
     for (m = pGlxScreen->fbconfigs; m != NULL; m = m->next) {
diff --git a/glx/glxscreens.h b/glx/glxscreens.h
index bff4363..d52099f 100644
--- a/glx/glxscreens.h
+++ b/glx/glxscreens.h
@@ -173,7 +173,6 @@ struct __GLXscreen {
     /*@}*/
 
     Bool (*CloseScreen)(int index, ScreenPtr pScreen);
-    Bool (*DestroyWindow)(WindowPtr pWindow);
 };
 
 
commit 0c499f2ee4ae2b7dc424009abb336fc81a8a2853
Author: Kristian Høgsberg <krh at bitplanet.net>
Date:   Fri Apr 16 05:55:34 2010 -0400

    DRI2: Track DRI2 drawables as resources, not privates
    
    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
    
    [Ported to 1.8 branch by ajax at redhat.com]
    
    Signed-off-by: Kristian Høgsberg <krh at bitplanet.net>
    Signed-off-by: Keith Packard <keithp at keithp.com>

diff --git a/glx/glxdri2.c b/glx/glxdri2.c
index edd29b0..bde519a 100644
--- a/glx/glxdri2.c
+++ b/glx/glxdri2.c
@@ -105,11 +105,6 @@ __glXDRIdrawableDestroy(__GLXdrawable *drawable)
     
     (*core->destroyDrawable)(private->driDrawable);
 
-    /* If the X window was destroyed, the dri DestroyWindow hook will
-     * aready have taken care of this, so only call if pDraw isn't NULL. */
-    if (drawable->pDraw != NULL)
-	DRI2DestroyDrawable(drawable->pDraw);
-
     __glXDrawableRelease(drawable);
 
     xfree(private);
diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
index 48618e1..63bef28 100644
--- a/hw/xfree86/dri2/dri2.c
+++ b/hw/xfree86/dri2/dri2.c
@@ -45,15 +45,14 @@
 
 #include "xf86.h"
 
-static int dri2ScreenPrivateKeyIndex;
+static int           dri2ScreenPrivateKeyIndex;
 static DevPrivateKey dri2ScreenPrivateKey = &dri2ScreenPrivateKeyIndex;
-static int dri2WindowPrivateKeyIndex;
-static DevPrivateKey dri2WindowPrivateKey = &dri2WindowPrivateKeyIndex;
-static int dri2PixmapPrivateKeyIndex;
-static DevPrivateKey dri2PixmapPrivateKey = &dri2PixmapPrivateKeyIndex;
+static RESTYPE       dri2DrawableRes;
+
+typedef struct _DRI2Screen *DRI2ScreenPtr;
 
 typedef struct _DRI2Drawable {
-    unsigned int	 refCount;
+    DRI2ScreenPtr        dri2_screen;
     int			 width;
     int			 height;
     DRI2BufferPtr	*buffers;
@@ -67,9 +66,8 @@ typedef struct _DRI2Drawable {
     int			 swap_limit; /* for N-buffering */
 } DRI2DrawableRec, *DRI2DrawablePtr;
 
-typedef struct _DRI2Screen *DRI2ScreenPtr;
-
 typedef struct _DRI2Screen {
+    ScreenPtr			 screen;
     unsigned int		 numDrivers;
     const char			**driverNames;
     const char			*deviceName;
@@ -95,43 +93,33 @@ DRI2GetScreen(ScreenPtr pScreen)
 static DRI2DrawablePtr
 DRI2GetDrawable(DrawablePtr pDraw)
 {
-    WindowPtr		  pWin;
-    PixmapPtr		  pPixmap;
-
-    if (!pDraw)
+    DRI2DrawablePtr pPriv;
+    int rc;
+ 
+    rc = dixLookupResourceByType((pointer *) &pPriv, pDraw->id,
+				 dri2DrawableRes, NULL, DixReadAccess);
+    if (rc != Success)
 	return NULL;
 
-    if (pDraw->type == DRAWABLE_WINDOW)
-    {
-	pWin = (WindowPtr) pDraw;
-	return dixLookupPrivate(&pWin->devPrivates, dri2WindowPrivateKey);
-    }
-    else
-    {
-	pPixmap = (PixmapPtr) pDraw;
-	return dixLookupPrivate(&pPixmap->devPrivates, dri2PixmapPrivateKey);
-    }
+    return pPriv;
 }
 
 int
 DRI2CreateDrawable(DrawablePtr pDraw)
 {
-    WindowPtr	    pWin;
-    PixmapPtr	    pPixmap;
     DRI2DrawablePtr pPriv;
+    int rc;
 
-    pPriv = DRI2GetDrawable(pDraw);
-    if (pPriv != NULL)
-    {
-	pPriv->refCount++;
-	return Success;
-    }
+    rc = dixLookupResourceByType((pointer *) &pPriv, pDraw->id,
+				 dri2DrawableRes, NULL, DixReadAccess);
+    if (rc == Success || rc != BadValue)
+	return rc;
 
     pPriv = xalloc(sizeof *pPriv);
     if (pPriv == NULL)
 	return BadAlloc;
 
-    pPriv->refCount = 1;
+    pPriv->dri2_screen = DRI2GetScreen(pDraw->pScreen);
     pPriv->width = pDraw->width;
     pPriv->height = pDraw->height;
     pPriv->buffers = NULL;
@@ -144,43 +132,30 @@ DRI2CreateDrawable(DrawablePtr pDraw)
     pPriv->last_swap_target = -1;
     pPriv->swap_limit = 1; /* default to double buffering */
 
-    if (pDraw->type == DRAWABLE_WINDOW)
-    {
-	pWin = (WindowPtr) pDraw;
-	dixSetPrivate(&pWin->devPrivates, dri2WindowPrivateKey, pPriv);
-    }
-    else
-    {
-	pPixmap = (PixmapPtr) pDraw;
-	dixSetPrivate(&pPixmap->devPrivates, dri2PixmapPrivateKey, pPriv);
-    }
+    if (!AddResource(pDraw->id, dri2DrawableRes, pPriv))
+	return BadAlloc;
 
     return Success;
 }
 
-static void
-DRI2FreeDrawable(DrawablePtr pDraw)
+static int DRI2DrawableGone(pointer p, XID id)
 {
-    DRI2DrawablePtr pPriv;
-    WindowPtr  	    pWin;
-    PixmapPtr	    pPixmap;
+    DRI2DrawablePtr pPriv = p;
+    DRI2ScreenPtr   ds = pPriv->dri2_screen;
+    DrawablePtr     root;
+    int i;
 
-    pPriv = DRI2GetDrawable(pDraw);
-    if (pPriv == NULL)
-	return;
+    root = &WindowTable[ds->screen->myNum]->drawable;
+    if (pPriv->buffers != NULL) {
+	for (i = 0; i < pPriv->bufferCount; i++)
+	    (*ds->DestroyBuffer)(root, pPriv->buffers[i]);
+
+	xfree(pPriv->buffers);
+    }
 
     xfree(pPriv);
 
-    if (pDraw->type == DRAWABLE_WINDOW)
-    {
-	pWin = (WindowPtr) pDraw;
-	dixSetPrivate(&pWin->devPrivates, dri2WindowPrivateKey, NULL);
-    }
-    else
-    {
-	pPixmap = (PixmapPtr) pDraw;
-	dixSetPrivate(&pPixmap->devPrivates, dri2PixmapPrivateKey, NULL);
-    }
+    return Success;
 }
 
 static int
@@ -534,13 +509,6 @@ DRI2SwapComplete(ClientPtr client, DrawablePtr pDraw, int frame,
 	return;
     }
 
-    if (pPriv->refCount == 0) {
-        xf86DrvMsg(pScreen->myNum, X_ERROR,
-		   "[DRI2] %s: bad drawable refcount\n", __func__);
-	DRI2FreeDrawable(pDraw);
-	return;
-    }
-
     ust = ((CARD64)tv_sec * 1000000) + tv_usec;
     if (swap_complete)
 	swap_complete(client, swap_data, type, ust, frame, pPriv->swap_count);
@@ -753,36 +721,6 @@ DRI2WaitSBC(ClientPtr client, DrawablePtr pDraw, CARD64 target_sbc,
     return Success;
 }
 
-void
-DRI2DestroyDrawable(DrawablePtr pDraw)
-{
-    DRI2ScreenPtr   ds = DRI2GetScreen(pDraw->pScreen);
-    DRI2DrawablePtr pPriv;
-
-    pPriv = DRI2GetDrawable(pDraw);
-    if (pPriv == NULL)
-	return;
-
-    pPriv->refCount--;
-    if (pPriv->refCount > 0)
-	return;
-
-    if (pPriv->buffers != NULL) {
-	int i;
-
-	for (i = 0; i < pPriv->bufferCount; i++)
-	    (*ds->DestroyBuffer)(pDraw, pPriv->buffers[i]);
-
-	xfree(pPriv->buffers);
-    }
-
-    /* If the window is destroyed while we have a swap pending, don't
-     * actually free the priv yet.  We'll need it in the DRI2SwapComplete()
-     * callback and we'll free it there once we're done. */
-    if (!pPriv->swapsPending)
-	DRI2FreeDrawable(pDraw);
-}
-
 Bool
 DRI2Connect(ScreenPtr pScreen, unsigned int driverType, int *fd,
 	    const char **driverName, const char **deviceName)
@@ -834,6 +772,7 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
     if (!ds)
 	return FALSE;
 
+    ds->screen         = pScreen;
     ds->fd	       = info->fd;
     ds->deviceName     = info->deviceName;
 
@@ -897,6 +836,8 @@ DRI2Setup(pointer module, pointer opts, int *errmaj, int *errmin)
 {
     static Bool setupDone = FALSE;
 
+    dri2DrawableRes = CreateNewResourceType(DRI2DrawableGone, "DRI2Drawable");
+
     if (!setupDone)
     {
 	setupDone = TRUE;
diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c
index bd92fd3..1ac4a5f 100644
--- a/hw/xfree86/dri2/dri2ext.c
+++ b/hw/xfree86/dri2/dri2ext.c
@@ -51,7 +51,6 @@
 #include "xf86Module.h"
 
 static ExtensionEntry	*dri2Extension;
-static RESTYPE		 dri2DrawableRes;
 
 static Bool
 validDrawable(ClientPtr client, XID drawable, Mask access_mode,
@@ -172,11 +171,6 @@ ProcDRI2CreateDrawable(ClientPtr client)
     if (status != Success)
 	return status;
 
-    if (!AddResource(stuff->drawable, dri2DrawableRes, pDrawable)) {
-	DRI2DestroyDrawable(pDrawable);
-	return BadAlloc;
-    }
-
     return client->noClientException;
 }
 
@@ -192,8 +186,6 @@ ProcDRI2DestroyDrawable(ClientPtr client)
 		       &pDrawable, &status))
 	return status;
 
-    FreeResourceByType(stuff->drawable, dri2DrawableRes, FALSE);
-
     return client->noClientException;
 }
 
@@ -620,25 +612,11 @@ SProcDRI2Dispatch (ClientPtr client)
     }
 }
 
-static int DRI2DrawableGone(pointer p, XID id)
-{
-    DrawablePtr pDrawable = p;
-
-    DRI2DestroyDrawable(pDrawable);
-
-    return Success;
-}
-
 int DRI2EventBase;
 
 static void
 DRI2ExtensionInit(void)
 {
-    dri2DrawableRes = CreateNewResourceType(DRI2DrawableGone, "DRI2Drawable");
-
-    if (!dri2DrawableRes)
-	return;
-
     dri2Extension = AddExtension(DRI2_NAME,
 				 DRI2NumberEvents,
 				 DRI2NumberErrors,
commit 0460a76b9ae25fe26f683f0cbff1e4157287cf56
Author: Kristian Høgsberg <krh at bitplanet.net>
Date:   Fri Apr 16 05:55:33 2010 -0400

    glx: Let the resource system destroy pixmaps
    
    GLX pbuffers are implemented using a pixmap allocated by the server.
    With the change to DRI2 to track DRI2 drawables as resources, we need to make
    sure that every drawable we create a DRI2 drawable for has an XID.  By
    using the XID of the pbuffer, the resource system will automatically
    reclaim the hidden pixmap and the DRI2 drawable when the pbuffer is
    destroyed or the client exits.
    
    Signed-off-by: Kristian Høgsberg <krh at bitplanet.net>
    Signed-off-by: Keith Packard <keithp at keithp.com>
    (cherry picked from commit 22da7aa9d743deee198aaf6df5d370a446db9763)

diff --git a/glx/glxcmds.c b/glx/glxcmds.c
index 04c6d40..087d52e 100644
--- a/glx/glxcmds.c
+++ b/glx/glxcmds.c
@@ -1101,14 +1101,6 @@ __glXDrawableInit(__GLXdrawable *drawable,
 void
 __glXDrawableRelease(__GLXdrawable *drawable)
 {
-    ScreenPtr pScreen = drawable->pDraw->pScreen;
-
-    switch (drawable->type) {
-    case GLX_DRAWABLE_PIXMAP:
-    case GLX_DRAWABLE_PBUFFER:
-	(*pScreen->DestroyPixmap)((PixmapPtr) drawable->pDraw);
-	break;
-    }
 }
 
 static int 
@@ -1117,8 +1109,6 @@ DoCreateGLXDrawable(ClientPtr client, __GLXscreen *pGlxScreen, __GLXconfig *conf
 {
     __GLXdrawable *pGlxDraw;
 
-    LEGAL_NEW_RESOURCE(glxDrawableId, client);
-
     if (pGlxScreen->pScreen != pDraw->pScreen)
 	return BadMatch;
 
@@ -1135,7 +1125,8 @@ DoCreateGLXDrawable(ClientPtr client, __GLXscreen *pGlxScreen, __GLXconfig *conf
     /* Add the glx drawable under the XID of the underlying X drawable
      * too.  That way we'll get a callback in DrawableGone and can
      * clean up properly when the drawable is destroyed. */
-    if (!AddResource(pDraw->id, __glXDrawableRes, pGlxDraw)) {
+    if (pDraw->id != glxDrawableId &&
+	!AddResource(pDraw->id, __glXDrawableRes, pGlxDraw)) {
 	pGlxDraw->destroy (pGlxDraw);
 	return BadAlloc;
     }
@@ -1150,6 +1141,8 @@ DoCreateGLXPixmap(ClientPtr client, __GLXscreen *pGlxScreen, __GLXconfig *config
     DrawablePtr pDraw;
     int err;
 
+    LEGAL_NEW_RESOURCE(glxDrawableId, client);
+
     err = dixLookupDrawable(&pDraw, drawableId, client, 0, DixAddAccess);
     if (err != Success) {
 	client->errorValue = drawableId;
@@ -1163,9 +1156,6 @@ DoCreateGLXPixmap(ClientPtr client, __GLXscreen *pGlxScreen, __GLXconfig *config
     err = DoCreateGLXDrawable(client, pGlxScreen, config, pDraw,
 			      glxDrawableId, GLX_DRAWABLE_PIXMAP);
 
-    if (err == Success)
-	((PixmapPtr) pDraw)->refcnt++;
-
     return err;
 }
 
@@ -1306,6 +1296,8 @@ DoCreatePbuffer(ClientPtr client, int screenNum, XID fbconfigId,
     PixmapPtr		 pPixmap;
     int			 err;
 
+    LEGAL_NEW_RESOURCE(glxDrawableId, client);
+
     if (!validGlxScreen(client, screenNum, &pGlxScreen, &err))
 	return err;
     if (!validGlxFBConfig(client, pGlxScreen, fbconfigId, &config, &err))
@@ -1316,6 +1308,13 @@ DoCreatePbuffer(ClientPtr client, int screenNum, XID fbconfigId,
 						    width, height, config->rgbBits, 0);
     __glXleaveServer(GL_FALSE);
 
+    /* Assign the pixmap the same id as the pbuffer and add it as a
+     * resource so it and the DRI2 drawable will be reclaimed when the
+     * pbuffer is destroyed. */
+    pPixmap->drawable.id = glxDrawableId;
+    if (!AddResource(pPixmap->drawable.id, RT_PIXMAP, pPixmap))
+	return BadAlloc;
+
     return DoCreateGLXDrawable(client, pGlxScreen, config, &pPixmap->drawable,
 			       glxDrawableId, GLX_DRAWABLE_PBUFFER);
 }
@@ -1423,6 +1422,8 @@ int __glXDisp_CreateWindow(__GLXclientState *cl, GLbyte *pc)
     DrawablePtr		 pDraw;
     int			 err;
 
+    LEGAL_NEW_RESOURCE(req->glxwindow, client);
+
     if (!validGlxScreen(client, req->screen, &pGlxScreen, &err))
 	return err;
     if (!validGlxFBConfig(client, pGlxScreen, req->fbconfig, &config, &err))
commit 86ca6baee221fc691c7828b078008314ac989864
Author: Kristian Høgsberg <krh at bitplanet.net>
Date:   Fri Apr 16 05:55:32 2010 -0400

    glx: Track GLX 1.3 style GLX drawables under their X drawable ID as well
    
    This ensures that the DrawableGone callback gets called as necessary
    when the X drawable goes away.  Otherwise, using a GLX drawable
    (say, glXSwapBuffers) in indirect mode after the X drawable has been
    destroyed will crash the server.
    
    Signed-off-by: Kristian Høgsberg <krh at bitplanet.net>
    Reviewed-by: Michel Dänzer <michel at daenzer.net>
    Signed-off-by: Keith Packard <keithp at keithp.com>
    (cherry picked from commit f0006aa58f6cf7552a239e169ff6e7e4fda532f4)

diff --git a/glx/glxcmds.c b/glx/glxcmds.c
index 77afbf4..04c6d40 100644
--- a/glx/glxcmds.c
+++ b/glx/glxcmds.c
@@ -161,7 +161,11 @@ validGlxDrawable(ClientPtr client, XID id, int type, int access_mode,
 	return FALSE;
     }
 
+    /* If the ID of the glx drawable we looked up doesn't match the id
+     * we looked for, it's because we looked it up under the X
+     * drawable ID (see DoCreateGLXDrawable). */
     if (rc == BadValue ||
+	(*drawable)->drawId != id ||
 	(type != GLX_DRAWABLE_ANY && type != (*drawable)->type)) {
 	client->errorValue = id;
 	switch (type) {
@@ -1128,6 +1132,14 @@ DoCreateGLXDrawable(ClientPtr client, __GLXscreen *pGlxScreen, __GLXconfig *conf
 	return BadAlloc;
     }
 
+    /* Add the glx drawable under the XID of the underlying X drawable
+     * too.  That way we'll get a callback in DrawableGone and can
+     * clean up properly when the drawable is destroyed. */
+    if (!AddResource(pDraw->id, __glXDrawableRes, pGlxDraw)) {
+	pGlxDraw->destroy (pGlxDraw);
+	return BadAlloc;
+    }
+
     return Success;
 }
 
diff --git a/glx/glxext.c b/glx/glxext.c
index 59bcfbe..89e58b0 100644
--- a/glx/glxext.c
+++ b/glx/glxext.c
@@ -126,6 +126,17 @@ static Bool DrawableGone(__GLXdrawable *glxPriv, XID xid)
 {
     __GLXcontext *c;
 
+    /* If this drawable was created using glx 1.3 drawable
+     * constructors, we added it as a glx drawable resource under both
+     * its glx drawable ID and it X drawable ID.  Remove the other
+     * resource now so we don't a callback for freed memory. */
+    if (glxPriv->drawId != glxPriv->pDraw->id) {
+	if (xid == glxPriv->drawId)
+	    FreeResourceByType(glxPriv->pDraw->id, __glXDrawableRes, TRUE);
+	else
+	    FreeResourceByType(glxPriv->drawId, __glXDrawableRes, TRUE);
+    }
+
     for (c = glxAllContexts; c; c = c->next) {
 	if (c->isCurrent && (c->drawPriv == glxPriv || c->readPriv == glxPriv)) {
 	    int i;


More information about the xorg-commit mailing list