An imperfect fix for Xinerama state changes without a mode change

    This patch improves behaviour for Xinerama state changes (via the
    VMWARE_CTRL) extension that don't have an accompanying mode change.
    This will be the case if a new Xinerama monitor layout has a bounding
    box with an identical size to that of the previous layout.

    Prior to this patch, the behaviour was pretty bad. If you sent two
    Xinerama states with the same bounding box, the second state would
    be set as pending but no actual mode change would occur, because
    the X server would already be in the right video mode. This means
    that the pending mode stays pending.

    If another Xinerama state comes in after this, we would hit our
    "Aborting due to existing pending state" error, and the new state
    would be discarded. This means we'd drop the mode switch on the
    floor, plus we'd lie to the client and say it worked.

    One example of the user-visible symptoms from this: The user has
    four monitors of the same size. We'll call them A through D.
    The VM goes into full-screen mode, and they set it to use screens
    ABC. Now they switch to BCD. These have the same bounding box size,
    so no mode change occurs and a topology is still pending. Now they
    switch to monitors BC. This mode switch is dropped, so the guest
    is still in the ABC topology and the mode is too wide for BC.

    This patch is an incomplete fix. If we're setting a new topology
    with the same bounding box, we'll flush the Xinerama state
    immediately since we know the mode switch will never occur. This
    means we don't get stuck with xineramaNextState set when it
    shouldn't be, and we don't have the problem with dropping
    subsequent mode changes. We also do set the new Xinerama state,
    so apps that query it will see the updated state immediately.

    But the fix isn't perfect- as far as I can tell, there's no way
    to notify applications that the monitor layout changed without
    a mode switch. So even though we've set the new topology, most
    apps won't notice. There are ways we could hack around this,
    but none of them are pretty.

3 files changed, 105 insertions, 9 deletions

diff --git a/src/vmware.c b/src/vmware.c
index 069cf2c..5275441 100644
--- a/src/vmware.c
+++ b/src/vmware.c
@@ -306,7 +306,7 @@ vmwareSendSVGACmdUpdate(VMWAREPtr pVMWARE, BoxPtr pBB)
     vmwareWriteWordToFIFO(pVMWARE, pBB->y2 - pBB->y1);
 }
 
-static void
+void
 vmwareSendSVGACmdUpdateFullScreen(VMWAREPtr pVMWARE)
 {
     BoxRec BB;
@@ -1163,8 +1163,40 @@ VMWAREModeInit(ScrnInfoPtr pScrn, DisplayModePtr mode, Bool rebuildPixmap)
     vgaHWProtect(pScrn, FALSE);
 
     /*
-     * XXX -- If we want to check that we got the mode we asked for, this
-     * would be a good place.
+     * Push the new Xinerama state to X clients and the hardware,
+     * synchronously with the mode change. Note that this must happen
+     * AFTER we write the new width and height to the hardware
+     * registers, since updating the WIDTH and HEIGHT registers will
+     * reset the device's multimon topology.
+     */
+    vmwareNextXineramaState(pVMWARE);
+
+    return TRUE;
+}
+
+void
+vmwareNextXineramaState(VMWAREPtr pVMWARE)
+{
+    VMWARERegPtr vmwareReg = &pVMWARE->ModeReg;
+
+    /*
+     * Switch to the next Xinerama state (from pVMWARE->xineramaNextState).
+     *
+     * This new state will be available to X clients via the Xinerama
+     * extension, and we push the new state to the virtual hardware,
+     * in order to configure a number of virtual monitors within the
+     * device's framebuffer.
+     *
+     * This function can be called at any time, but it should usually be
+     * called just after a mode switch. This is for two reasons:
+     *
+     *   1) We don't want X clients to see a Xinerama topology and a video
+     *      mode that are inconsistent with each other, so we'd like to switch
+     *      both at the same time.
+     *
+     *   2) We must set the host's display topology registers after setting
+     *      the new video mode, since writes to WIDTH/HEIGHT will reset the
+     *      hardware display topology.
      */
 
     /*
@@ -1178,7 +1210,14 @@ VMWAREModeInit(ScrnInfoPtr pScrn, DisplayModePtr mode, Bool rebuildPixmap)
 
           pVMWARE->xineramaNextState = NULL;
           pVMWARE->xineramaNextNumOutputs = 0;
+
        } else {
+          /*
+           * There is no next state pending. Switch back to
+           * single-monitor mode. This is necessary for resetting the
+           * Xinerama state if we get a mode change which doesn't
+           * follow a VMwareCtrlDoSetTopology call.
+           */
           VMWAREXineramaPtr basicState =
              (VMWAREXineramaPtr)xcalloc(1, sizeof (VMWAREXineramaRec));
           if (basicState) {
@@ -1195,7 +1234,8 @@ VMWAREModeInit(ScrnInfoPtr pScrn, DisplayModePtr mode, Bool rebuildPixmap)
     }
 
     /*
-     * Update host's view of guest topology.
+     * Update host's view of guest topology. This tells the device
+     * how we're carving up its framebuffer into virtual screens.
      */
     if (pVMWARE->vmwareCapability & SVGA_CAP_DISPLAY_TOPOLOGY) {
         if (pVMWARE->xinerama) {
@@ -1223,14 +1263,13 @@ VMWAREModeInit(ScrnInfoPtr pScrn, DisplayModePtr mode, Bool rebuildPixmap)
             vmwareWriteReg(pVMWARE, SVGA_REG_DISPLAY_IS_PRIMARY, TRUE);
             vmwareWriteReg(pVMWARE, SVGA_REG_DISPLAY_POSITION_X, 0);
             vmwareWriteReg(pVMWARE, SVGA_REG_DISPLAY_POSITION_Y, 0);
-            vmwareWriteReg(pVMWARE, SVGA_REG_DISPLAY_WIDTH, mode->HDisplay);
-            vmwareWriteReg(pVMWARE, SVGA_REG_DISPLAY_HEIGHT, mode->VDisplay);
+            vmwareWriteReg(pVMWARE, SVGA_REG_DISPLAY_WIDTH, vmwareReg->svga_reg_width);
+            vmwareWriteReg(pVMWARE, SVGA_REG_DISPLAY_HEIGHT, vmwareReg->svga_reg_height);
         }
 
+        /* Done. */
         vmwareWriteReg(pVMWARE, SVGA_REG_DISPLAY_ID, SVGA_INVALID_DISPLAY_ID);
     }
-
-    return TRUE;
 }
 
 static void
@@ -1436,6 +1475,7 @@ VMWAREAddDisplayMode(ScrnInfoPtr pScrn,
    DisplayModeRec *mode;
 
    mode = xalloc(sizeof(DisplayModeRec));
+   memset(mode, 0, sizeof *mode);
 
    mode->name = xalloc(strlen(name) + 1);
    strcpy(mode->name, name);
diff --git a/src/vmware.h b/src/vmware.h
index b906ff2..a3502dd 100644
--- a/src/vmware.h
+++ b/src/vmware.h
@@ -245,6 +245,10 @@ void vmwareSendSVGACmdUpdate(
    VMWAREPtr pVMWARE, BoxPtr pBB
    );
 
+void vmwareSendSVGACmdUpdateFullScreen(
+   VMWAREPtr pVMWARE
+   );
+
 DisplayModeRec *VMWAREAddDisplayMode(
     ScrnInfoPtr pScrn,
     const char *name,
@@ -257,6 +261,10 @@ Bool vmwareIsRegionEqual(
     const RegionPtr reg2
    );
 
+void vmwareNextXineramaState(
+   VMWAREPtr pVMWARE
+   );
+
 /* vmwarecurs.c */
 Bool vmwareCursorInit(
    ScreenPtr pScr
diff --git a/src/vmwarectrl.c b/src/vmwarectrl.c
index 634b9ca..d9ceaa4 100644
--- a/src/vmwarectrl.c
+++ b/src/vmwarectrl.c
@@ -282,11 +282,59 @@ VMwareCtrlDoSetTopology(ScrnInfoPtr pScrn,
       if (xineramaState) {
          memcpy(xineramaState, extents, number * sizeof (VMWAREXineramaRec));
 
+         /*
+          * Make this the new pending Xinerama state. Normally we'll
+          * wait until the next mode switch in order to synchronously
+          * push this state out to X clients and the virtual hardware.
+          *
+          * However, if we're already in the right video mode, there
+          * will be no mode change. In this case, push it out
+          * immediately.
+          */
          xfree(pVMWARE->xineramaNextState);
          pVMWARE->xineramaNextState = xineramaState;
          pVMWARE->xineramaNextNumOutputs = number;
 
-         return VMwareCtrlDoSetRes(pScrn, maxX, maxY, FALSE);
+         if (maxX == pVMWARE->ModeReg.svga_reg_width &&
+             maxY == pVMWARE->ModeReg.svga_reg_height) {
+
+            /*
+             * XXX:
+             *
+             * There are problems with trying to set a Xinerama state
+             * without a mode switch. The biggest one is that
+             * applications typically won't notice a topology change
+             * that occurs without a mode switch. If you run "xdpyinfo
+             * -ext XINERAMA" after one such topology change, it will
+             * report the new data, but apps (like the GNOME Panel)
+             * will not notice until the next mode change.
+             *
+             * I don't think there's any good solution to this... as
+             * far as I know, even on a non-virtualized machine
+             * there's no way for an app to find out if the Xinerama
+             * opology changes without a resolution change also
+             * occurring. There might be some cheats we can take, like
+             * swithcing to a new mode with the same resolution and a
+             * different (fake) refresh rate, or temporarily switching
+             * to an intermediate mode. Ick.
+             *
+             * The other annoyance here is that when we reprogram the
+             * SVGA device's monitor topology registers, it may
+             * rearrange those monitors on the host's screen, but they
+             * will still have the old contents. This might be
+             * correct, but it isn't guaranteed to match what's on X's
+             * framebuffer at the moment. So we'll send a
+             * full-framebuffer update rect afterwards.
+             */
+
+            vmwareNextXineramaState(pVMWARE);
+            vmwareSendSVGACmdUpdateFullScreen(pVMWARE);
+
+            return TRUE;
+         } else {
+            return VMwareCtrlDoSetRes(pScrn, maxX, maxY, FALSE);
+         }
+
       } else {
          return FALSE;
       }