On Wed, Apr 21, 2010 at 04:51:17PM -0700, Pierre-Loup A. Griffais wrote: > Since the revert is already pushed, here's a new version of the > change as Peter pushed it including the teardown crash fix. > > Thanks, > - Pierre-Loup >
> From 7ffae8aaa4ac445a727e663e1e1bf05a2510cd35 Mon Sep 17 00:00:00 2001 > From: Pierre-Loup A. Griffais <[email protected]> > Date: Wed, 21 Apr 2010 16:46:17 -0700 > Subject: [PATCH] mi: don't thrash resources when displaying the software > cursor across screens > > This changes the DC layer to maintain a persistent set of GCs/pixmaps/pictures > for each pScreen instead of failing to thrash between them when changing > screens. > > Signed-off-by: Pierre-Loup A. Griffais <[email protected]> > Reviewed-by: Peter Hutterer <[email protected]> > Signed-off-by: Peter Hutterer <[email protected]> > --- > mi/midispcur.c | 269 ++++++++++++++++++++++--------------------------------- > 1 files changed, 108 insertions(+), 161 deletions(-) > > diff --git a/mi/midispcur.c b/mi/midispcur.c > index 3fb7e02..9041630 100644 > --- a/mi/midispcur.c > +++ b/mi/midispcur.c > @@ -59,9 +59,9 @@ static DevPrivateKey miDCScreenKey = &miDCScreenKeyIndex; > > static Bool miDCCloseScreen(int index, ScreenPtr pScreen); > > -/* per device private data */ > -static int miDCSpriteKeyIndex; > -static DevPrivateKey miDCSpriteKey = &miDCSpriteKeyIndex; > +/* per device per-screen private data */ > +static int miDCSpriteKeyIndex[MAXSCREENS]; > +static DevPrivateKey miDCSpriteKey = miDCSpriteKeyIndex; > > typedef struct { > GCPtr pSourceGC, pMaskGC; > @@ -75,10 +75,10 @@ typedef struct { > #endif > } miDCBufferRec, *miDCBufferPtr; > > -#define MIDCBUFFER(dev) \ > +#define MIDCBUFFER(dev, screen) \ > ((DevHasCursor(dev)) ? \ > - (miDCBufferPtr)dixLookupPrivate(&dev->devPrivates, miDCSpriteKey) : \ > - (miDCBufferPtr)dixLookupPrivate(&dev->u.master->devPrivates, > miDCSpriteKey)) > + (miDCBufferPtr)dixLookupPrivate(&dev->devPrivates, miDCSpriteKey + > (screen)->myNum) : \ > + (miDCBufferPtr)dixLookupPrivate(&dev->u.master->devPrivates, miDCSpriteKey > + (screen)->myNum)) > > /* > * The core pointer buffer will point to the index of the virtual core > pointer > @@ -158,10 +158,6 @@ miDCInitialize (ScreenPtr pScreen, > miPointerScreenFuncPtr screenFuncs) > return TRUE; > } > > -#define tossGC(gc) (gc ? FreeGC (gc, (GContext) 0) : 0) > -#define tossPix(pix) (pix ? (*pScreen->DestroyPixmap) (pix) : TRUE) > -#define tossPict(pict) (pict ? FreePicture (pict, 0) : 0) > - > static Bool > miDCCloseScreen (int index, ScreenPtr pScreen) > { > @@ -183,7 +179,6 @@ miDCRealizeCursor (ScreenPtr pScreen, CursorPtr pCursor) > } > > #ifdef ARGB_CURSOR > -#define EnsurePicture(picture,draw,win) (picture || > miDCMakePicture(&picture,draw,win)) > > static VisualPtr > miDCGetWindowVisual (WindowPtr pWin) > @@ -415,12 +410,8 @@ miDCPutBits ( > (*maskGC->ops->PushPixels) (maskGC, pPriv->maskBits, pDrawable, w, h, x, > y); > } > > -#define EnsureGC(gc,win) (gc || miDCMakeGC(&gc, win)) > - > static GCPtr > -miDCMakeGC( > - GCPtr *ppGC, > - WindowPtr pWin) > +miDCMakeGC(WindowPtr pWin) > { > GCPtr pGC; > int status; > @@ -431,7 +422,6 @@ miDCMakeGC( > pGC = CreateGC((DrawablePtr)pWin, > GCSubwindowMode|GCGraphicsExposures, gcvals, &status, > (XID)0, serverClient); > - *ppGC = pGC; > return pGC; > } > > @@ -456,22 +446,11 @@ miDCPutUpCursor (DeviceIntPtr pDev, ScreenPtr pScreen, > CursorPtr pCursor, > pScreenPriv = (miDCScreenPtr)dixLookupPrivate(&pScreen->devPrivates, > miDCScreenKey); > pWin = WindowTable[pScreen->myNum]; > - pBuffer = MIDCBUFFER(pDev); > + pBuffer = MIDCBUFFER(pDev, pScreen); > > #ifdef ARGB_CURSOR > if (pPriv->pPicture) > { > - /* see comment in miDCPutUpCursor */ > - if (pBuffer->pRootPicture && > - pBuffer->pRootPicture->pDrawable && > - pBuffer->pRootPicture->pDrawable->pScreen != pScreen) > - { > - tossPict(pBuffer->pRootPicture); > - pBuffer->pRootPicture = NULL; > - } > - > - if (!EnsurePicture(pBuffer->pRootPicture, &pWin->drawable, pWin)) > - return FALSE; > CompositePicture (PictOpOver, > pPriv->pPicture, > NULL, > @@ -484,33 +463,6 @@ miDCPutUpCursor (DeviceIntPtr pDev, ScreenPtr pScreen, > CursorPtr pCursor, > else > #endif > { > - /** > - * XXX: Before MPX, the sourceGC and maskGC were attached to the > - * screen, and would switch as the screen switches. With mpx we have > - * the GC's attached to the device now, so each time we switch screen > - * we need to make sure the GC's are allocated on the new screen. > - * This is ... not optimal. (whot) > - */ > - if (pBuffer->pSourceGC && pScreen != pBuffer->pSourceGC->pScreen) > - { > - tossGC(pBuffer->pSourceGC); > - pBuffer->pSourceGC = NULL; > - } > - > - if (pBuffer->pMaskGC && pScreen != pBuffer->pMaskGC->pScreen) > - { > - tossGC(pBuffer->pMaskGC); > - pBuffer->pMaskGC = NULL; > - } > - > - if (!EnsureGC(pBuffer->pSourceGC, pWin)) > - return FALSE; > - if (!EnsureGC(pBuffer->pMaskGC, pWin)) > - { > - FreeGC (pBuffer->pSourceGC, (GContext) 0); > - pBuffer->pSourceGC = 0; > - return FALSE; > - } > miDCPutBits ((DrawablePtr)pWin, pPriv, > pBuffer->pSourceGC, pBuffer->pMaskGC, > x, y, pCursor->bits->width, pCursor->bits->height, > @@ -531,7 +483,7 @@ miDCSaveUnderCursor (DeviceIntPtr pDev, ScreenPtr pScreen, > > pScreenPriv = (miDCScreenPtr)dixLookupPrivate(&pScreen->devPrivates, > miDCScreenKey); > - pBuffer = MIDCBUFFER(pDev); > + pBuffer = MIDCBUFFER(pDev, pScreen); > > pSave = pBuffer->pSave; > pWin = WindowTable[pScreen->myNum]; > @@ -544,14 +496,7 @@ miDCSaveUnderCursor (DeviceIntPtr pDev, ScreenPtr > pScreen, > if (!pSave) > return FALSE; > } > - /* see comment in miDCPutUpCursor */ > - if (pBuffer->pSaveGC && pBuffer->pSaveGC->pScreen != pScreen) > - { > - tossGC(pBuffer->pSaveGC); > - pBuffer->pSaveGC = NULL; > - } > - if (!EnsureGC(pBuffer->pSaveGC, pWin)) > - return FALSE; > + > pGC = pBuffer->pSaveGC; > if (pSave->drawable.serialNumber != pGC->serialNumber) > ValidateGC ((DrawablePtr) pSave, pGC); > @@ -572,20 +517,13 @@ miDCRestoreUnderCursor (DeviceIntPtr pDev, ScreenPtr > pScreen, > > pScreenPriv = (miDCScreenPtr)dixLookupPrivate(&pScreen->devPrivates, > miDCScreenKey); > - pBuffer = MIDCBUFFER(pDev); > + pBuffer = MIDCBUFFER(pDev, pScreen); > pSave = pBuffer->pSave; > > pWin = WindowTable[pScreen->myNum]; > if (!pSave) > return FALSE; > - /* see comment in miDCPutUpCursor */ > - if (pBuffer->pRestoreGC && pBuffer->pRestoreGC->pScreen != pScreen) > - { > - tossGC(pBuffer->pRestoreGC); > - pBuffer->pRestoreGC = NULL; > - } > - if (!EnsureGC(pBuffer->pRestoreGC, pWin)) > - return FALSE; > + > pGC = pBuffer->pRestoreGC; > if (pWin->drawable.serialNumber != pGC->serialNumber) > ValidateGC ((DrawablePtr) pWin, pGC); > @@ -607,7 +545,7 @@ miDCChangeSave (DeviceIntPtr pDev, ScreenPtr pScreen, > > pScreenPriv = (miDCScreenPtr)dixLookupPrivate(&pScreen->devPrivates, > miDCScreenKey); > - pBuffer = MIDCBUFFER(pDev); > + pBuffer = MIDCBUFFER(pDev, pScreen); > > pSave = pBuffer->pSave; > pWin = WindowTable[pScreen->myNum]; > @@ -616,14 +554,7 @@ miDCChangeSave (DeviceIntPtr pDev, ScreenPtr pScreen, > */ > if (!pSave) > return FALSE; > - /* see comment in miDCPutUpCursor */ > - if (pBuffer->pRestoreGC && pBuffer->pRestoreGC->pScreen != pScreen) > - { > - tossGC(pBuffer->pRestoreGC); > - pBuffer->pRestoreGC = NULL; > - } > - if (!EnsureGC(pBuffer->pRestoreGC, pWin)) > - return FALSE; > + > pGC = pBuffer->pRestoreGC; > if (pWin->drawable.serialNumber != pGC->serialNumber) > ValidateGC ((DrawablePtr) pWin, pGC); > @@ -662,14 +593,7 @@ miDCChangeSave (DeviceIntPtr pDev, ScreenPtr pScreen, > (*pGC->ops->CopyArea) ((DrawablePtr) pSave, (DrawablePtr) pWin, pGC, > 0, sourcey, -dx, copyh, x + dx, desty); > } > - /* see comment in miDCPutUpCursor */ > - if (pBuffer->pSaveGC && pBuffer->pSaveGC->pScreen != pScreen) > - { > - tossGC(pBuffer->pSaveGC); > - pBuffer->pSaveGC = NULL; > - } > - if (!EnsureGC(pBuffer->pSaveGC, pWin)) > - return FALSE; > + > pGC = pBuffer->pSaveGC; > if (pSave->drawable.serialNumber != pGC->serialNumber) > ValidateGC ((DrawablePtr) pSave, pGC); > @@ -766,7 +690,7 @@ miDCMoveCursor (DeviceIntPtr pDev, ScreenPtr pScreen, > CursorPtr pCursor, > pScreenPriv = (miDCScreenPtr)dixLookupPrivate(&pScreen->devPrivates, > miDCScreenKey); > pWin = WindowTable[pScreen->myNum]; > - pBuffer = MIDCBUFFER(pDev); > + pBuffer = MIDCBUFFER(pDev, pScreen); > > pTemp = pBuffer->pTemp; > if (!pTemp || > @@ -809,17 +733,9 @@ miDCMoveCursor (DeviceIntPtr pDev, ScreenPtr pScreen, > CursorPtr pCursor, > #ifdef ARGB_CURSOR > if (pPriv->pPicture) > { > - /* see comment in miDCPutUpCursor */ > - if (pBuffer->pTempPicture && > - pBuffer->pTempPicture->pDrawable && > - pBuffer->pTempPicture->pDrawable->pScreen != pScreen) > - { > - tossPict(pBuffer->pTempPicture); > - pBuffer->pTempPicture = NULL; > - } > + if (!pBuffer->pTempPicture) > + miDCMakePicture(&pBuffer->pTempPicture, &pTemp->drawable, pWin); > > - if (!EnsurePicture(pBuffer->pTempPicture, &pTemp->drawable, pWin)) > - return FALSE; > CompositePicture (PictOpOver, > pPriv->pPicture, > NULL, > @@ -832,38 +748,12 @@ miDCMoveCursor (DeviceIntPtr pDev, ScreenPtr pScreen, > CursorPtr pCursor, > else > #endif > { > - if (!pBuffer->pPixSourceGC) > - { > - pBuffer->pPixSourceGC = CreateGC ((DrawablePtr)pTemp, > - GCGraphicsExposures, &gcval, &status, (XID)0, serverClient); > - if (!pBuffer->pPixSourceGC) > - return FALSE; > - } > - if (!pBuffer->pPixMaskGC) > - { > - pBuffer->pPixMaskGC = CreateGC ((DrawablePtr)pTemp, > - GCGraphicsExposures, &gcval, &status, (XID)0, serverClient); > - if (!pBuffer->pPixMaskGC) > - return FALSE; > - } > miDCPutBits ((DrawablePtr)pTemp, pPriv, > pBuffer->pPixSourceGC, pBuffer->pPixMaskGC, > dx, dy, pCursor->bits->width, pCursor->bits->height, > source, mask); > } > > - /* see comment in miDCPutUpCursor */ > - if (pBuffer->pRestoreGC && pBuffer->pRestoreGC->pScreen != pScreen) > - { > - tossGC(pBuffer->pRestoreGC); > - pBuffer->pRestoreGC = NULL; > - } > - /* > - * copy the temporary pixmap onto the screen > - */ > - > - if (!EnsureGC(pBuffer->pRestoreGC, pWin)) > - return FALSE; > pGC = pBuffer->pRestoreGC; > if (pWin->drawable.serialNumber != pGC->serialNumber) > ValidateGC ((DrawablePtr) pWin, pGC); > @@ -877,51 +767,108 @@ miDCMoveCursor (DeviceIntPtr pDev, ScreenPtr pScreen, > CursorPtr pCursor, > static Bool > miDCDeviceInitialize(DeviceIntPtr pDev, ScreenPtr pScreen) > { > - miDCBufferPtr pBuffer; > - > - pBuffer = xalloc(sizeof(miDCBufferRec)); > - dixSetPrivate(&pDev->devPrivates, miDCSpriteKey, pBuffer); > - > - pBuffer->pSourceGC = > - pBuffer->pMaskGC = > - pBuffer->pSaveGC = > - pBuffer->pRestoreGC = > - pBuffer->pMoveGC = > - pBuffer->pPixSourceGC = > - pBuffer->pPixMaskGC = NULL; > + miDCBufferPtr pBuffer; > + WindowPtr pWin; > + XID gcval = FALSE; > + int status; > + int i; > + > + if (!DevHasCursor(pDev)) > + return TRUE; > + > + for (i = 0; i < screenInfo.numScreens; i++) > + { > + pScreen = screenInfo.screens[i]; > + > + pBuffer = xalloc(sizeof(miDCBufferRec)); > + if (!pBuffer) > + goto failure; > + > + dixSetPrivate(&pDev->devPrivates, miDCSpriteKey + pScreen->myNum, > pBuffer); > + pWin = WindowTable[pScreen->myNum]; > + > + pBuffer->pSourceGC = miDCMakeGC(pWin); > + if (!pBuffer->pSourceGC) > + goto failure; > + > + pBuffer->pMaskGC = miDCMakeGC(pWin); > + if (!pBuffer->pMaskGC) > + goto failure; > + > + pBuffer->pSaveGC = miDCMakeGC(pWin); > + if (!pBuffer->pSaveGC) > + goto failure; > + > + pBuffer->pRestoreGC = miDCMakeGC(pWin); > + if (!pBuffer->pRestoreGC) > + goto failure; > + > + pBuffer->pMoveGC = CreateGC ((DrawablePtr)pWin, > + GCGraphicsExposures, &gcval, &status, (XID)0, serverClient); > + if (!pBuffer->pMoveGC) > + goto failure; > + > + pBuffer->pPixSourceGC = CreateGC ((DrawablePtr)pWin, > + GCGraphicsExposures, &gcval, &status, (XID)0, serverClient); > + if (!pBuffer->pPixSourceGC) > + goto failure; > + > + pBuffer->pPixMaskGC = CreateGC ((DrawablePtr)pWin, > + GCGraphicsExposures, &gcval, &status, (XID)0, serverClient); > + if (!pBuffer->pPixMaskGC) > + goto failure; > + > #ifdef ARGB_CURSOR > - pBuffer->pRootPicture = NULL; > - pBuffer->pTempPicture = NULL; > + miDCMakePicture(&pBuffer->pRootPicture, &pWin->drawable, pWin); > + if (!pBuffer->pRootPicture) > + goto failure; > + > + pBuffer->pTempPicture = NULL; > #endif > - pBuffer->pSave = pBuffer->pTemp = NULL; > + > + // these get (re)allocated lazily depending on the cursor size > + pBuffer->pSave = pBuffer->pTemp = NULL; > + } > > return TRUE; > + > +failure: > + > + miDCDeviceCleanup(pDev, pScreen); > + > + return FALSE; > } > > static void > miDCDeviceCleanup(DeviceIntPtr pDev, ScreenPtr pScreen) > { > miDCBufferPtr pBuffer; > + int i; > > if (DevHasCursor(pDev)) > { > - pBuffer = MIDCBUFFER(pDev); > - tossGC (pBuffer->pSourceGC); > - tossGC (pBuffer->pMaskGC); > - tossGC (pBuffer->pSaveGC); > - tossGC (pBuffer->pRestoreGC); > - tossGC (pBuffer->pMoveGC); > - tossGC (pBuffer->pPixSourceGC); > - tossGC (pBuffer->pPixMaskGC); > - tossPix (pBuffer->pSave); > - tossPix (pBuffer->pTemp); > -#ifdef ARGB_CURSOR > -#if 0 /* This has been free()d before */ > - tossPict (pScreenPriv->pRootPicture); > -#endif > - tossPict (pBuffer->pTempPicture); > -#endif > - xfree(pBuffer); > - dixSetPrivate(&pDev->devPrivates, miDCSpriteKey, NULL); > + for (i = 0; i < screenInfo.numScreens; i++) > + { > + pScreen = screenInfo.screens[i]; > + > + pBuffer = MIDCBUFFER(pDev, pScreen); > + > + if (pBuffer) > + { > + if (pBuffer->pSourceGC) FreeGC(pBuffer->pSourceGC, > (GContext) 0); > + if (pBuffer->pMaskGC) FreeGC(pBuffer->pMaskGC, (GContext) 0); > + if (pBuffer->pSaveGC) FreeGC(pBuffer->pSaveGC, (GContext) 0); > + if (pBuffer->pRestoreGC) FreeGC(pBuffer->pRestoreGC, > (GContext) 0); > + if (pBuffer->pMoveGC) FreeGC(pBuffer->pMoveGC, (GContext) 0); > + if (pBuffer->pPixSourceGC) FreeGC(pBuffer->pPixSourceGC, > (GContext) 0); > + if (pBuffer->pPixMaskGC) FreeGC(pBuffer->pPixMaskGC, > (GContext) 0); > + > + if (pBuffer->pSave) > (*pScreen->DestroyPixmap)(pBuffer->pSave); > + if (pBuffer->pTemp) > (*pScreen->DestroyPixmap)(pBuffer->pTemp); > + > + xfree(pBuffer); > + dixSetPrivate(&pDev->devPrivates, miDCSpriteKey + > pScreen->myNum, NULL); > + } > + } > } > } > -- > 1.7.0.4 confirming that this doesn't crash anymore. Tested-by: Peter Hutterer <[email protected]> though I didn't test the actual problem it is supposed to fix (the screen crossing) Cheers, Peter _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
