dix/mi: cleanup the device initialisation routines

Currently the code passes in pScreen 0 and then somewhere in the sublayers
loops across all the screens, this doesn't seem like the right way to do
things.

So instead loop across the screens at the top level, and remove lower level
loops, also be careful to avoid reinitialising the device privates in
misprite.c more than once.

This is just a cleanup from trying to understand the code.

Signed-off-by: Dave Airlie <airlied@redhat.com>

4 files changed, 74 insertions, 53 deletions

diff --git a/dix/devices.c b/dix/devices.c
index 9cf04ed3f..68161d986 100644
--- a/dix/devices.c
+++ b/dix/devices.c
@@ -362,7 +362,7 @@ EnableDevice(DeviceIntPtr dev, BOOL sendevent)
         if (IsMaster(dev)) {
             /* Sprites appear on first root window, so we can hardcode it */
             if (dev->spriteInfo->spriteOwner) {
-                InitializeSprite(dev, screenInfo.screens[0]->root);
+                InitializeSprite(dev);
                 /* mode doesn't matter */
                 EnterWindow(dev, screenInfo.screens[0]->root, NotifyAncestor);
             }
@@ -526,6 +526,46 @@ DisableAllDevices(void)
 }
 
 /**
+ * Initialize a device cursor on all screens
+ * In case of failure remove any cursors we've already created.
+ */
+static Bool
+InitializeDeviceCursor(DeviceIntPtr dev)
+{
+    int i;
+    ScreenPtr pScreen;
+    Bool ret;
+
+    for (i = 0; i < screenInfo.numScreens; i++) {
+        pScreen = screenInfo.screens[i];
+
+        ret = pScreen->DeviceCursorInitialize(dev, pScreen);
+        if (ret == FALSE)
+            goto cleanup;
+    }
+
+    return TRUE;
+ cleanup:
+    for (; i >= 0; i--) {
+        pScreen = screenInfo.screens[i];
+        (*pScreen->DeviceCursorCleanup)(dev, pScreen);
+    }
+    return FALSE;
+}
+
+static void
+DeviceCursorCleanup(DeviceIntPtr dev)
+{
+    int i;
+    ScreenPtr pScreen;
+
+    for (i = 0; i < screenInfo.numScreens; i++) {
+        pScreen = screenInfo.screens[i];
+        (*pScreen->DeviceCursorCleanup)(dev, pScreen);
+    }
+}
+
+/**
  * Initialise a new device through the driver and tell all clients about the
  * new device.
  *
@@ -539,7 +579,6 @@ int
 ActivateDevice(DeviceIntPtr dev, BOOL sendevent)
 {
     int ret = Success;
-    ScreenPtr pScreen = screenInfo.screens[0];
 
     if (!dev || !dev->deviceProc)
         return BadImplementation;
@@ -551,7 +590,7 @@ ActivateDevice(DeviceIntPtr dev, BOOL sendevent)
 
     /* Initialize memory for sprites. */
     if (IsMaster(dev) && dev->spriteInfo->spriteOwner)
-        if (!pScreen->DeviceCursorInitialize(dev, pScreen))
+        if (!InitializeDeviceCursor(dev))
             ret = BadAlloc;
 
     SendDevicePresenceEvent(dev->id, DeviceAdded);
@@ -913,7 +952,6 @@ FreeAllDeviceClasses(ClassesPtr classes)
 static void
 CloseDevice(DeviceIntPtr dev)
 {
-    ScreenPtr screen = screenInfo.screens[0];
     ClassesPtr classes;
     int j;
 
@@ -927,7 +965,7 @@ CloseDevice(DeviceIntPtr dev)
 
     /* free sprite memory */
     if (IsMaster(dev) && dev->spriteInfo->sprite)
-        screen->DeviceCursorCleanup(dev, screen);
+        DeviceCursorCleanup(dev);
 
     /* free acceleration info */
     if (dev->valuator && dev->valuator->accelScheme.AccelCleanupProc)
@@ -2536,8 +2574,6 @@ ReleaseButtonsAndKeys(DeviceIntPtr dev)
 int
 AttachDevice(ClientPtr client, DeviceIntPtr dev, DeviceIntPtr master)
 {
-    ScreenPtr screen;
-
     if (!dev || IsMaster(dev))
         return BadDevice;
 
@@ -2550,8 +2586,7 @@ AttachDevice(ClientPtr client, DeviceIntPtr dev, DeviceIntPtr master)
 
     /* free the existing sprite. */
     if (IsFloating(dev) && dev->spriteInfo->paired == dev) {
-        screen = miPointerGetScreen(dev);
-        screen->DeviceCursorCleanup(dev, screen);
+        DeviceCursorCleanup(dev);
         free(dev->spriteInfo->sprite);
     }
 
@@ -2564,18 +2599,9 @@ AttachDevice(ClientPtr client, DeviceIntPtr dev, DeviceIntPtr master)
      * alloc new memory but overwrite the previous one.
      */
     if (!master) {
-        WindowPtr currentRoot;
-
-        if (dev->spriteInfo->sprite)
-            currentRoot = GetCurrentRootWindow(dev);
-        else                    /* new device auto-set to floating */
-            currentRoot = screenInfo.screens[0]->root;
-
-        /* we need to init a fake sprite */
-        screen = currentRoot->drawable.pScreen;
-        screen->DeviceCursorInitialize(dev, screen);
+        InitializeDeviceCursor(dev);
         dev->spriteInfo->sprite = NULL;
-        InitializeSprite(dev, currentRoot);
+        InitializeSprite(dev);
         dev->spriteInfo->spriteOwner = FALSE;
         dev->spriteInfo->paired = dev;
     }
diff --git a/dix/events.c b/dix/events.c
index 3b40446bc..748486321 100644
--- a/dix/events.c
+++ b/dix/events.c
@@ -3096,8 +3096,9 @@ ReinitializeRootWindow(WindowPtr win, int xoff, int yoff)
  *
  */
 void
-InitializeSprite(DeviceIntPtr pDev, WindowPtr pWin)
+InitializeSprite(DeviceIntPtr pDev)
 {
+    WindowPtr pWin;
     SpritePtr pSprite;
     ScreenPtr pScreen;
     CursorPtr pCursor;
@@ -3121,7 +3122,10 @@ InitializeSprite(DeviceIntPtr pDev, WindowPtr pWin)
         }
         if (inputInfo.keyboard->spriteInfo->paired == pDev)
             inputInfo.keyboard->spriteInfo->sprite = pDev->spriteInfo->sprite;
-    }
+
+        pWin = screenInfo.screens[0]->root;
+    } else
+        pWin = GetCurrentRootWindow(pDev);
 
     pSprite = pDev->spriteInfo->sprite;
     pDev->spriteInfo->spriteOwner = TRUE;
diff --git a/include/dix.h b/include/dix.h
index 74123b51b..9dbe9bf0f 100644
--- a/include/dix.h
+++ b/include/dix.h
@@ -395,8 +395,8 @@ DeliverTouchEvents(DeviceIntPtr /* dev */ ,
                    XID /* resource */ );
 
 extern void
-InitializeSprite(DeviceIntPtr /* pDev */ ,
-                 WindowPtr /* pWin */ );
+InitializeSprite(DeviceIntPtr /* pDev */);
+
 extern void
 FreeSprite(DeviceIntPtr pDev);
 
diff --git a/mi/misprite.c b/mi/misprite.c
index 03c753e2d..695947e57 100644
--- a/mi/misprite.c
+++ b/mi/misprite.c
@@ -63,6 +63,7 @@ typedef struct {
     Bool isInCacheWin;
     Bool checkPixels;           /* check colormap collision */
     ScreenPtr pScreen;
+    Bool initialized;
 } miCursorInfoRec, *miCursorInfoPtr;
 
 /*
@@ -793,42 +794,36 @@ static Bool
 miSpriteDeviceCursorInitialize(DeviceIntPtr pDev, ScreenPtr pScreen)
 {
     int ret = FALSE;
-    int i;
 
     if (!DevHasCursor(pDev))
         return TRUE;
 
-    for (i = 0; i < screenInfo.numScreens; i++) {
-        WindowPtr pWin;
-        pScreen = screenInfo.screens[i];
-        pWin = pScreen->root;
-        ret = miDCDeviceInitializeOne(pDev, pScreen, &pWin->drawable);
-        if (ret == FALSE)
-            goto failure;
-    }
+    ret = miDCDeviceInitializeOne(pDev, pScreen, &pScreen->root->drawable);
+    if (ret == FALSE)
+        goto failure;
 
     if (ret) {
         miCursorInfoPtr pCursorInfo;
 
         pCursorInfo =
             dixLookupPrivate(&pDev->devPrivates, miSpriteDevPrivatesKey);
-        pCursorInfo->pCursor = NULL;
-        pCursorInfo->x = 0;
-        pCursorInfo->y = 0;
-        pCursorInfo->isUp = FALSE;
-        pCursorInfo->shouldBeUp = FALSE;
-        pCursorInfo->pCacheWin = NullWindow;
-        pCursorInfo->isInCacheWin = FALSE;
-        pCursorInfo->checkPixels = TRUE;
-        pCursorInfo->pScreen = FALSE;
+        if (!pCursorInfo->initialized) {
+            pCursorInfo->pCursor = NULL;
+            pCursorInfo->x = 0;
+            pCursorInfo->y = 0;
+            pCursorInfo->isUp = FALSE;
+            pCursorInfo->shouldBeUp = FALSE;
+            pCursorInfo->pCacheWin = NullWindow;
+            pCursorInfo->isInCacheWin = FALSE;
+            pCursorInfo->checkPixels = TRUE;
+            pCursorInfo->pScreen = NULL;
+            pCursorInfo->initialized = TRUE;
+        }
     }
 
     return ret;
 failure:
-    for (i = 0; i < screenInfo.numScreens; i++) {
-        pScreen = screenInfo.screens[i];
-        miDCDeviceCleanupOne(pDev, pScreen);
-    }
+    miDCDeviceCleanupOne(pDev, pScreen);
     return ret;
 }
 
@@ -839,15 +834,11 @@ miSpriteDeviceCursorCleanup(DeviceIntPtr pDev, ScreenPtr pScreen)
         dixLookupPrivate(&pDev->devPrivates, miSpriteDevPrivatesKey);
 
     if (DevHasCursor(pDev)) {
-        int i;
-        for (i = 0; i < screenInfo.numScreens; i++) {
-            pScreen = screenInfo.screens[i];
-
-            miDCDeviceCleanupOne(pDev, pScreen);
-        }
+        miDCDeviceCleanupOne(pDev, pScreen);
     }
 
-    memset(pCursorInfo, 0, sizeof(miCursorInfoRec));
+    if (pCursorInfo->initialized)
+        memset(pCursorInfo, 0, sizeof(miCursorInfoRec));
 }
 
 /*