[PATCHv2] mi: Delete unused flicker-free MoveCursor code.
Peter Hutterer
peter.hutterer at who-t.net
Thu May 27 01:38:32 PDT 2010
On Mon, May 24, 2010 at 07:16:37PM -0700, Jamey Sharp wrote:
> It's been commented-out for three and a half years and nobody seems to
> be missing it enough to resurrect it.
>
> Besides deleting code that is untested and therefore buggy, this saves a
> little memory for each pointer device on each screen.
>
> Also, add a comment about why pRootPicture is not explicitly freed in
> miDCDeviceCleanup.
>
> Signed-off-by: Jamey Sharp <jamey at minilop.net>
> ---
> Oops, I tested after deleting some of the code, but forgot to retest
> after deleting the rest. The only difference from the previous version
> is that I deleted a couple local variables from miDCDeviceInitialize
> too, which the compiler warned me are unused now.
>
> On Mon, May 24, 2010 at 7:10 PM, Peter Hutterer <peter.hutterer at who-t.net> wrote:
> > you'd only see it with SW cursor which excludes most of the common
> > setups. It'd be a really nice thing to have though, create three MDs
> > and then move the newer two cursors (first one is a HW cursor) into
> > the same area and watch the flicker.
>
> I don't doubt it, but the code is in the git history for anyone who
> cares, and until somebody bothers, why keep maintaining it?
>
> mi/midispcur.c | 268 ++------------------------------------------------------
> mi/misprite.c | 70 +--------------
> mi/misprite.h | 7 --
> 3 files changed, 12 insertions(+), 333 deletions(-)
>
> diff --git a/mi/midispcur.c b/mi/midispcur.c
> index f2b2229..c279010 100644
> --- a/mi/midispcur.c
> +++ b/mi/midispcur.c
> @@ -66,12 +66,9 @@ static DevPrivateKey miDCSpriteKey = miDCSpriteKeyIndex;
> typedef struct {
> GCPtr pSourceGC, pMaskGC;
> GCPtr pSaveGC, pRestoreGC;
> - GCPtr pMoveGC;
> - GCPtr pPixSourceGC, pPixMaskGC;
> - PixmapPtr pSave, pTemp;
> + PixmapPtr pSave;
> #ifdef ARGB_CURSOR
> PicturePtr pRootPicture;
> - PicturePtr pTempPicture;
> #endif
> } miDCBufferRec, *miDCBufferPtr;
>
> @@ -498,243 +495,10 @@ miDCRestoreUnderCursor (DeviceIntPtr pDev, ScreenPtr pScreen,
> }
>
> Bool
> -miDCChangeSave (DeviceIntPtr pDev, ScreenPtr pScreen,
> - int x, int y, int w, int h, int dx, int dy)
> -{
> - miDCScreenPtr pScreenPriv;
> - miDCBufferPtr pBuffer;
> - PixmapPtr pSave;
> - WindowPtr pWin;
> - GCPtr pGC;
> - int sourcex, sourcey, destx, desty, copyw, copyh;
> -
> - pScreenPriv = (miDCScreenPtr)dixLookupPrivate(&pScreen->devPrivates,
> - miDCScreenKey);
> - pBuffer = MIDCBUFFER(pDev, pScreen);
> -
> - pSave = pBuffer->pSave;
> - pWin = WindowTable[pScreen->myNum];
> - /*
> - * restore the bits which are about to get trashed
> - */
> - if (!pSave)
> - return FALSE;
> -
> - pGC = pBuffer->pRestoreGC;
> - if (pWin->drawable.serialNumber != pGC->serialNumber)
> - ValidateGC ((DrawablePtr) pWin, pGC);
> - /*
> - * copy the old bits to the screen.
> - */
> - if (dy > 0)
> - {
> - (*pGC->ops->CopyArea) ((DrawablePtr) pSave, (DrawablePtr) pWin, pGC,
> - 0, h - dy, w, dy, x + dx, y + h);
> - }
> - else if (dy < 0)
> - {
> - (*pGC->ops->CopyArea) ((DrawablePtr) pSave, (DrawablePtr) pWin, pGC,
> - 0, 0, w, -dy, x + dx, y + dy);
> - }
> - if (dy >= 0)
> - {
> - desty = y + dy;
> - sourcey = 0;
> - copyh = h - dy;
> - }
> - else
> - {
> - desty = y;
> - sourcey = - dy;
> - copyh = h + dy;
> - }
> - if (dx > 0)
> - {
> - (*pGC->ops->CopyArea) ((DrawablePtr) pSave, (DrawablePtr) pWin, pGC,
> - w - dx, sourcey, dx, copyh, x + w, desty);
> - }
> - else if (dx < 0)
> - {
> - (*pGC->ops->CopyArea) ((DrawablePtr) pSave, (DrawablePtr) pWin, pGC,
> - 0, sourcey, -dx, copyh, x + dx, desty);
> - }
> -
> - pGC = pBuffer->pSaveGC;
> - if (pSave->drawable.serialNumber != pGC->serialNumber)
> - ValidateGC ((DrawablePtr) pSave, pGC);
> - /*
> - * move the bits that are still valid within the pixmap
> - */
> - if (dx >= 0)
> - {
> - sourcex = 0;
> - destx = dx;
> - copyw = w - dx;
> - }
> - else
> - {
> - destx = 0;
> - sourcex = - dx;
> - copyw = w + dx;
> - }
> - if (dy >= 0)
> - {
> - sourcey = 0;
> - desty = dy;
> - copyh = h - dy;
> - }
> - else
> - {
> - desty = 0;
> - sourcey = -dy;
> - copyh = h + dy;
> - }
> - (*pGC->ops->CopyArea) ((DrawablePtr) pSave, (DrawablePtr) pSave, pGC,
> - sourcex, sourcey, copyw, copyh, destx, desty);
> - /*
> - * copy the new bits from the screen into the remaining areas of the
> - * pixmap
> - */
> - if (dy > 0)
> - {
> - (*pGC->ops->CopyArea) ((DrawablePtr) pWin, (DrawablePtr) pSave, pGC,
> - x, y, w, dy, 0, 0);
> - }
> - else if (dy < 0)
> - {
> - (*pGC->ops->CopyArea) ((DrawablePtr) pWin, (DrawablePtr) pSave, pGC,
> - x, y + h + dy, w, -dy, 0, h + dy);
> - }
> - if (dy >= 0)
> - {
> - desty = dy;
> - sourcey = y + dy;
> - copyh = h - dy;
> - }
> - else
> - {
> - desty = 0;
> - sourcey = y;
> - copyh = h + dy;
> - }
> - if (dx > 0)
> - {
> - (*pGC->ops->CopyArea) ((DrawablePtr) pWin, (DrawablePtr) pSave, pGC,
> - x, sourcey, dx, copyh, 0, desty);
> - }
> - else if (dx < 0)
> - {
> - (*pGC->ops->CopyArea) ((DrawablePtr) pWin, (DrawablePtr) pSave, pGC,
> - x + w + dx, sourcey, -dx, copyh, w + dx, desty);
> - }
> - return TRUE;
> -}
> -
> -Bool
> -miDCMoveCursor (DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCursor,
> - int x, int y, int w, int h, int dx, int dy,
> - unsigned long source, unsigned long mask)
> -{
> - miDCCursorPtr pPriv;
> - miDCScreenPtr pScreenPriv;
> - miDCBufferPtr pBuffer;
> - int status;
> - WindowPtr pWin;
> - GCPtr pGC;
> - XID gcval = FALSE;
> - PixmapPtr pTemp;
> -
> - pPriv = (miDCCursorPtr)dixLookupPrivate(&pCursor->bits->devPrivates,
> - CursorScreenKey(pScreen));
> - if (!pPriv)
> - {
> - pPriv = miDCRealize(pScreen, pCursor);
> - if (!pPriv)
> - return FALSE;
> - }
> - pScreenPriv = (miDCScreenPtr)dixLookupPrivate(&pScreen->devPrivates,
> - miDCScreenKey);
> - pWin = WindowTable[pScreen->myNum];
> - pBuffer = MIDCBUFFER(pDev, pScreen);
> -
> - pTemp = pBuffer->pTemp;
> - if (!pTemp ||
> - pTemp->drawable.width != pBuffer->pSave->drawable.width ||
> - pTemp->drawable.height != pBuffer->pSave->drawable.height)
> - {
> - if (pTemp)
> - (*pScreen->DestroyPixmap) (pTemp);
> -#ifdef ARGB_CURSOR
> - if (pBuffer->pTempPicture)
> - {
> - FreePicture (pBuffer->pTempPicture, 0);
> - pBuffer->pTempPicture = 0;
> - }
> -#endif
> - pBuffer->pTemp = pTemp = (*pScreen->CreatePixmap)
> - (pScreen, w, h, pBuffer->pSave->drawable.depth, 0);
> - if (!pTemp)
> - return FALSE;
> - }
> - if (!pBuffer->pMoveGC)
> - {
> - pBuffer->pMoveGC = CreateGC ((DrawablePtr)pTemp,
> - GCGraphicsExposures, &gcval, &status, (XID)0, serverClient);
> - if (!pBuffer->pMoveGC)
> - return FALSE;
> - }
> - /*
> - * copy the saved area to a temporary pixmap
> - */
> - pGC = pBuffer->pMoveGC;
> - if (pGC->serialNumber != pTemp->drawable.serialNumber)
> - ValidateGC ((DrawablePtr) pTemp, pGC);
> - (*pGC->ops->CopyArea)((DrawablePtr)pBuffer->pSave,
> - (DrawablePtr)pTemp, pGC, 0, 0, w, h, 0, 0);
> -
> - /*
> - * draw the cursor in the temporary pixmap
> - */
> -#ifdef ARGB_CURSOR
> - if (pPriv->pPicture)
> - {
> - if (!EnsurePicture(pBuffer->pTempPicture, &pTemp->drawable, pWin))
> - return FALSE;
> - CompositePicture (PictOpOver,
> - pPriv->pPicture,
> - NULL,
> - pBuffer->pTempPicture,
> - 0, 0, 0, 0,
> - dx, dy,
> - pCursor->bits->width,
> - pCursor->bits->height);
> - }
> - else
> -#endif
> - {
> - miDCPutBits ((DrawablePtr)pTemp, pPriv,
> - pBuffer->pPixSourceGC, pBuffer->pPixMaskGC,
> - dx, dy, pCursor->bits->width, pCursor->bits->height,
> - source, mask);
> - }
> -
> - pGC = pBuffer->pRestoreGC;
> - if (pWin->drawable.serialNumber != pGC->serialNumber)
> - ValidateGC ((DrawablePtr) pWin, pGC);
> -
> - (*pGC->ops->CopyArea) ((DrawablePtr) pTemp, (DrawablePtr) pWin,
> - pGC,
> - 0, 0, w, h, x, y);
> - return TRUE;
> -}
> -
> -Bool
> miDCDeviceInitialize(DeviceIntPtr pDev, ScreenPtr pScreen)
> {
> miDCBufferPtr pBuffer;
> WindowPtr pWin;
> - XID gcval = FALSE;
> - int status;
> int i;
>
> if (!DevHasCursor(pDev))
> @@ -767,28 +531,12 @@ miDCDeviceInitialize(DeviceIntPtr pDev, ScreenPtr pScreen)
> 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;
> #endif
>
> - // these get (re)allocated lazily depending on the cursor size
> - pBuffer->pSave = pBuffer->pTemp = NULL;
> + /* (re)allocated lazily depending on the cursor size */
> + pBuffer->pSave = NULL;
> }
>
> return TRUE;
> @@ -820,12 +568,14 @@ miDCDeviceCleanup(DeviceIntPtr pDev, ScreenPtr pScreen)
> 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 a pRootPicture was allocated for a root window, it
> + * is freed when that root window is destroyed, so don't
> + * free it again here. */
> +#endif
this comment seems a bit odd. is this part of another patch?
>
> if (pBuffer->pSave) (*pScreen->DestroyPixmap)(pBuffer->pSave);
> - if (pBuffer->pTemp) (*pScreen->DestroyPixmap)(pBuffer->pTemp);
>
> free(pBuffer);
> dixSetPrivate(&pDev->devPrivates, miDCSpriteKey + pScreen->myNum, NULL);
> diff --git a/mi/misprite.c b/mi/misprite.c
> index 3d10bc8..2962abf 100644
> --- a/mi/misprite.c
> +++ b/mi/misprite.c
> @@ -798,73 +798,9 @@ miSpriteSetCursor (DeviceIntPtr pDev, ScreenPtr pScreen,
> miSpriteFindColors (pPointer, pScreen);
> }
> if (pPointer->isUp) {
> -#if 0
> - /* FIXME: Disabled for MPX, should be rewritten */
> - int sx, sy;
> - /*
> - * check to see if the old saved region
> - * encloses the new sprite, in which case we use
> - * the flicker-free MoveCursor primitive.
> - */
> - sx = pointer->x - (int)pCursor->bits->xhot;
> - sy = pointer->y - (int)pCursor->bits->yhot;
> - if (sx + (int) pCursor->bits->width >= pointer->saved.x1 &&
> - sx < pointer->saved.x2 &&
> - sy + (int) pCursor->bits->height >= pointer->saved.y1 &&
> - sy < pointer->saved.y2 &&
> - (int) pCursor->bits->width + (2 * SPRITE_PAD) ==
> - pointer->saved.x2 - pointer->saved.x1 &&
> - (int) pCursor->bits->height + (2 * SPRITE_PAD) ==
> - pointer->saved.y2 - pointer->saved.y1
> - )
> - {
> - DamageDrawInternal (pScreen, TRUE);
> - miSpriteIsDown(pCursorInfo);
> - if (!(sx >= pointer->saved.x1 &&
> - sx + (int)pCursor->bits->width < pointer->saved.x2
> - && sy >= pointer->saved.y1 &&
> - sy + (int)pCursor->bits->height <
> - pointer->saved.y2))
> - {
> - int oldx1, oldy1, dx, dy;
> -
> - oldx1 = pointer->saved.x1;
> - oldy1 = pointer->saved.y1;
> - dx = oldx1 - (sx - SPRITE_PAD);
> - dy = oldy1 - (sy - SPRITE_PAD);
> - pointer->saved.x1 -= dx;
> - pointer->saved.y1 -= dy;
> - pointer->saved.x2 -= dx;
> - pointer->saved.y2 -= dy;
> - (void) miDCChangeSave(pScreen,
> - pointer->saved.x1,
> - pointer->saved.y1,
> - pointer->saved.x2 -
> - pointer->saved.x1,
> - pointer->saved.y2 -
> - pointer->saved.y1,
> - dx, dy);
> - }
> - (void) miDCMoveCursor(pScreen, pCursor,
> - pointer->saved.x1,
> - pointer->saved.y1,
> - pointer->saved.x2 -
> - pointer->saved.x1,
> - pointer->saved.y2 -
> - pointer->saved.y1,
> - sx - pointer->saved.x1,
> - sy - pointer->saved.y1,
> - pointer->colors[SOURCE_COLOR].pixel,
> - pointer->colors[MASK_COLOR].pixel);
> - miSpriteIsUp(pCursorInfo);
> - DamageDrawInternal (pScreen, FALSE);
> - }
> - else
> -#endif
> - {
> - SPRITE_DEBUG (("SetCursor remove %d\n", pDev->id));
> - miSpriteRemoveCursor (pDev, pScreen);
> - }
> + /* TODO: reimplement flicker-free MoveCursor */
> + SPRITE_DEBUG (("SetCursor remove %d\n", pDev->id));
> + miSpriteRemoveCursor (pDev, pScreen);
> }
>
> if (!pPointer->isUp && pPointer->pCursor)
> diff --git a/mi/misprite.h b/mi/misprite.h
> index 78bf52c..632d207 100644
> --- a/mi/misprite.h
> +++ b/mi/misprite.h
> @@ -46,12 +46,5 @@ extern Bool miDCSaveUnderCursor(DeviceIntPtr pDev, ScreenPtr pScreen,
> int x, int y, int w, int h);
> extern Bool miDCRestoreUnderCursor(DeviceIntPtr pDev, ScreenPtr pScreen,
> int x, int y, int w, int h);
> -extern Bool miDCMoveCursor(DeviceIntPtr pDev, ScreenPtr pScreen,
> - CursorPtr pCursor, int x, int y,
> - int w, int h, int dx, int dy,
> - unsigned long source, unsigned long mask);
> -extern Bool miDCChangeSave(DeviceIntPtr pDev, ScreenPtr pScreen,
> - int x, int y, int w, int h,
> - int dx, int dy);
> extern Bool miDCDeviceInitialize(DeviceIntPtr pDev, ScreenPtr pScreen);
> extern void miDCDeviceCleanup(DeviceIntPtr pDev, ScreenPtr pScreen);
> --
> 1.7.0
if that comment has a good reason
Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
Cheers,
Peter
More information about the xorg-devel
mailing list