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?
Thanks,
- Pierre-Loup
On 04/06/2010 08:20 AM, Tiago Vignatti wrote:
On Tue, Apr 06, 2010 at 03:52:07AM +0200, ext Pierre-Loup A. Griffais wrote:
The DC code is broken for setups with several screens. Devs only have one pSave
pixmap and there's no code to thrash them like p[Save|Restore]GC.
That means if you have two X screens and force SW cursor on both, the server
will end up passing a bunch of CopyAreas with mismatching screens to the driver,
which can fail horribly if the driver does a good job abstracting screens away
>from each other.
If helps you, I can confirm this happening here also.
Tiago
From e7c5552be9cc217866ea7362c44884baa2964d85 Mon Sep 17 00:00:00 2001
From: Pierre-Loup A. Griffais<[email protected]>
Date: Tue, 6 Apr 2010 15:30:30 -0700
Subject: [PATCH] [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]>
---
mi/midispcur.c | 263 +++++++++++++++++++++++---------------------------------
1 files changed, 108 insertions(+), 155 deletions(-)
diff --git a/mi/midispcur.c b/mi/midispcur.c
index 3fb7e02..7d8f876 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;
@@ -77,8 +77,8 @@ typedef struct {
#define MIDCBUFFER(dev) \
((DevHasCursor(dev)) ? \
- (miDCBufferPtr)dixLookupPrivate(&dev->devPrivates, miDCSpriteKey) : \
- (miDCBufferPtr)dixLookupPrivate(&dev->u.master->devPrivates, miDCSpriteKey))
+ (miDCBufferPtr)dixLookupPrivate(&dev->devPrivates, miDCSpriteKey +
(pScreen)->myNum) : \
+ (miDCBufferPtr)dixLookupPrivate(&dev->u.master->devPrivates, miDCSpriteKey +
(pScreen)->myNum))
I think it's better to change the macro to take (dev, pScreen) instead of
magically relying on pScreen to be there.
Good point, done.
/*
* 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(ScreenPtr pScreen)
{
GCPtr pGC;
int status;
@@ -428,10 +419,9 @@ miDCMakeGC(
gcvals[0] = IncludeInferiors;
gcvals[1] = FALSE;
- pGC = CreateGC((DrawablePtr)pWin,
+ pGC = CreateGC((DrawablePtr)WindowTable[pScreen->myNum],
GCSubwindowMode|GCGraphicsExposures, gcvals,&status,
(XID)0, serverClient);
- *ppGC = pGC;
return pGC;
}
I don't really see the need for this change - why not let the window be
passed in instead of accessing the global WindowTable. Especially since
looking at the caller below you don't gain much from it (see my comment
in place there)
Sure.
@@ -461,17 +451,6 @@ miDCPutUpCursor (DeviceIntPtr pDev, ScreenPtr pScreen,
CursorPtr pCursor,
#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,
@@ -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);
@@ -578,14 +523,7 @@ miDCRestoreUnderCursor (DeviceIntPtr pDev, ScreenPtr
pScreen,
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);
@@ -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);
@@ -770,7 +694,7 @@ miDCMoveCursor (DeviceIntPtr pDev, ScreenPtr pScreen,
CursorPtr pCursor,
pTemp = pBuffer->pTemp;
if (!pTemp ||
- pTemp->drawable.width != pBuffer->pSave->drawable.width ||
+ pTemp->drawable.width != pBuffer->pSave->drawable.width ||
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,114 @@ 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);
+
+ pBuffer->pSourceGC = miDCMakeGC(pScreen);
+ if (!pBuffer->pSourceGC)
+ goto failure;
+
+ pBuffer->pMaskGC = miDCMakeGC(pScreen);
+ if (!pBuffer->pMaskGC)
+ goto failure;
+
+ pBuffer->pSaveGC = miDCMakeGC(pScreen);
+ if (!pBuffer->pSaveGC)
+ goto failure;
+
+ pBuffer->pRestoreGC = miDCMakeGC(pScreen);
+ if (!pBuffer->pRestoreGC)
+ goto failure;
+
+ pWin = WindowTable[pScreen->myNum];
if you move this line up to after the dixSetPrivate, you can just leave the
miDCMakeGC API as-is and the code is equally readable.
Done.
+
+ 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);
+
+ 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).
+
#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
other than this, looks good to me in principle
Thanks.
Cheers,
Peter
>From 69f15bf20f17a92f64a99b0bcbd4e1744b3d861c Mon Sep 17 00:00:00 2001
From: Pierre-Loup A. Griffais <[email protected]>
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 <[email protected]>
---
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 ||
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
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
