[PATCH] mi: fix software cursor with several X screens

Peter Hutterer peter.hutterer at who-t.net
Wed Apr 7 18:49:12 PDT 2010


On Wed, Apr 07, 2010 at 02:08:18PM -0700, Pierre-Loup A. Griffais wrote:
> Thanks for the review; responses inline and attached revised version of the patch:
> 
> On 04/06/2010 06:52 PM, Peter Hutterer wrote:
> >On Tue, Apr 06, 2010 at 03:32:49PM -0700, Pierre-Loup A. Griffais wrote:
> >>Now with a proper GC cleanup sequence instead of freeing the same GC in a loop.
> >>
> >>Thanks,
> >>  - Pierre-Loup
> >>
> >>On 04/06/2010 02:44 PM, Pierre-Loup A. Griffais wrote:
> >>>(disregard previous message sent too soon)
> >>>
> >>>Attached is a tentative patch that cleans that particular code up and fixes the
> >>>issue.
> >>>
> >>>It would seem cleaner to perform the screen looping in ActivateDevice(), but
> >>>that would also mean changing miPointerDeviceInitialize and
> >>>miSpriteDeviceCursorInitialize to only perform their own setup once in that
> >>>case. Opinions?
> >
> >InitializeSprite is probably the better place since a device doesn't
> >necessarily get a sprite even when activated (attach SDs for example) and it
> >may get a sprite without being deactivated first (set floating).
> 
> Where do we hook the equivalent cleanup function in that case?
> Having these resources being allocated upfront and persisting across
> enable/disable seems more consistent in this context. It also means
> that toggling a device in a resource exhausted scenario will ensure
> that the existing storage remains optimal. Does that make sense?

actually, I was wrong with InitializeSprite, the code seems to be in
AttachDevice, both or init and cleanup. at least that's where the calls to
DeviceCursorInitialize and DeviceCursorCleanup are. and since that's called
for all attachments and detachments that seems like the most suitable place
if we want to do run-time allocation.

Pre-allocation: I don't think it would hurt us much in terms of memory, etc.
so I'd be fine with it. though Tiago may have a different opinion here.
Tiago - any comments?

> >>
> >>      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);
> >>+        for (i = 0; i<  screenInfo.numScreens; i++)
> >>+        {
> >>+            pScreen = screenInfo.screens[i];
> >>+
> >>+            pBuffer = MIDCBUFFER(pDev);
> >>+
> >>+            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);
> >
> >could we make FreeGC handle a null pointer?
> >that would clean the code up a bit.
> 
> I guess we could, but it seems inconsistent with the rest of the
> server (not to mention it would also broadens the scope of this
> change by a lot). I can bring back a macro that implicitely performs
> the check if you prefer? (even though I didn't like having macros
> like that with a local scope).

leave it like it is then, better than some (semi-)obscure macro.

> From 69f15bf20f17a92f64a99b0bcbd4e1744b3d861c Mon Sep 17 00:00:00 2001
> From: Pierre-Loup A. Griffais <pgriffais at nvidia.com>
> Date: Wed, 7 Apr 2010 13:52:47 -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 <pgriffais at nvidia.com>
> ---
>  mi/midispcur.c |  272 +++++++++++++++++++++++---------------------------------
>  1 files changed, 112 insertions(+), 160 deletions(-)
> 
> diff --git a/mi/midispcur.c b/mi/midispcur.c
> index 3fb7e02..c5d8f41 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,11 +690,11 @@ 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 ||
> -	pTemp->drawable.width != pBuffer->pSave->drawable.width ||
> +         pTemp->drawable.width != pBuffer->pSave->drawable.width ||

whitespace-only change.

>  	pTemp->drawable.height != pBuffer->pSave->drawable.height)
>      {
>  	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,113 @@ 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);
> +        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);
> +
>  #ifdef ARGB_CURSOR
> -#if 0				/* This has been free()d before */
> -        tossPict (pScreenPriv->pRootPicture);
> -#endif 
> -        tossPict (pBuffer->pTempPicture);
> +                if (pBuffer->pRootPicture) FreePicture(pBuffer->pRootPicture, 0);
> +                if (pBuffer->pTempPicture) FreePicture(pBuffer->pTempPicture, 0);
>  #endif
> -        xfree(pBuffer);
> -        dixSetPrivate(&pDev->devPrivates, miDCSpriteKey, NULL);
> +
> +                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

Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>

Cheers,
  Peter


More information about the xorg-devel mailing list