Changeset 32817 in vbox for trunk/src/VBox/Main

Timestamp:

Sep 29, 2010 2:30:07 PM (14 years ago)

Author:

vboxsync

Message:

Main: some locking and readability clean-up of MouseImpl

Location:

trunk/src/VBox/Main

Files:

• MouseImpl.cpp (modified) (17 diffs)
• include/MouseImpl.h (modified) (1 diff)

trunk/src/VBox/Main/MouseImpl.cpp

@@ -110 +110 @@
     unconst(mParent) = parent;
 
-#ifdef RT_OS_L4
-    /* L4 console has no own mouse cursor */
-    uHostCaps = VMMDEV_MOUSE_HOST_CANNOT_HWPOINTER;
-#else
     uHostCaps = 0;
-#endif
 
     /* Confirm a successful initialization */
@@ -157 +152 @@
 {
     AssertPtrReturn(pfCaps, E_POINTER);
+    /** @todo does getting the VMMDev and the VMMDevPort like this guarantee
+     * they won't go away while we are using them? */
     VMMDev *pVMMDev = mParent->getVMMDev();
     ComAssertRet(pVMMDev, E_FAIL);
@@ -192 +189 @@
     if (FAILED(autoCaller.rc())) return autoCaller.rc();
 
-    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+    AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
     bool fAbs = false;
 
@@ -221 +218 @@
     if (FAILED(autoCaller.rc())) return autoCaller.rc();
 
-    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+    AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
     bool fRel = false;
 
@@ -245 +242 @@
     AutoCaller autoCaller(this);
     if (FAILED(autoCaller.rc())) return autoCaller.rc();
-
-    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
 
     *pfNeedsHostCursor = fVMMDevNeedsHostCursor;
@@ -283 +278 @@
     {
         PPDMIMOUSEPORT pUpPort = NULL;
-        for (unsigned i = 0; !pUpPort && i < MOUSE_MAX_DEVICES; ++i)
         {
-            if (mpDrv[i] && (mpDrv[i]->u32DevCaps & MOUSE_DEVCAP_RELATIVE))
-                pUpPort = mpDrv[i]->pUpPort;
+            AutoReadLock aLock(this COMMA_LOCKVAL_SRC_POS);
+
+            for (unsigned i = 0; !pUpPort && i < MOUSE_MAX_DEVICES; ++i)
+            {
+                if (mpDrv[i] && (mpDrv[i]->u32DevCaps & MOUSE_DEVCAP_RELATIVE))
+                    pUpPort = mpDrv[i]->pUpPort;
+            }
         }
         if (!pUpPort)
@@ -297 +296 @@
                             tr("Could not send the mouse event to the virtual mouse (%Rrc)"),
                             vrc);
+        mLastButtons = fButtons;
     }
     return S_OK;
@@ -314 +314 @@
     {
         PPDMIMOUSEPORT pUpPort = NULL;
-        for (unsigned i = 0; !pUpPort && i < MOUSE_MAX_DEVICES; ++i)
         {
-            if (mpDrv[i] && (mpDrv[i]->u32DevCaps & MOUSE_DEVCAP_ABSOLUTE))
-                pUpPort = mpDrv[i]->pUpPort;
+            AutoReadLock aLock(this COMMA_LOCKVAL_SRC_POS);
+
+            for (unsigned i = 0; !pUpPort && i < MOUSE_MAX_DEVICES; ++i)
+            {
+                if (mpDrv[i] && (mpDrv[i]->u32DevCaps & MOUSE_DEVCAP_ABSOLUTE))
+                    pUpPort = mpDrv[i]->pUpPort;
+            }
         }
         if (!pUpPort)
@@ -328 +332 @@
                             tr("Could not send the mouse event to the virtual mouse (%Rrc)"),
                             vrc);
+        mLastButtons = fButtons;
     }
     return S_OK;
@@ -357 +362 @@
 }
 
+HRESULT Mouse::reportAbsEvent(uint32_t mouseXAbs, uint32_t mouseYAbs,
+                              int32_t dz, int32_t dw, uint32_t fButtons,
+                              bool fUsesVMMDevEvent)
+{
+    HRESULT rc;
+    /** If we are using the VMMDev to report absolute position but without
+     * VMMDev IRQ support then we need to send a small "jiggle" to the emulated
+     * relative mouse device to alert the guest to changes. */
+    LONG cJiggle = 0;
+
+    if (fVMMDevCanAbs)
+    {
+        /*
+         * Send the absolute mouse position to the VMM device.
+         */
+        if (mouseXAbs != mLastAbsX || mouseYAbs != mLastAbsY)
+        {
+            rc = reportAbsEventToVMMDev(mouseXAbs, mouseYAbs);
+            cJiggle = 1;
+        }
+        rc = reportRelEventToMouseDev(cJiggle, 0, dz, dw, fButtons);
+    }
+    else
+        rc = reportAbsEventToMouseDev(mouseXAbs, mouseYAbs, dz, dw, fButtons);
+
+    mLastAbsX = mouseXAbs;
+    mLastAbsY = mouseYAbs;
+    return rc;
+}
+
 /**
  * Send a mouse event.
@@ -368 +403 @@
 STDMETHODIMP Mouse::PutMouseEvent(LONG dx, LONG dy, LONG dz, LONG dw, LONG buttonState)
 {
-    HRESULT rc = S_OK;
+    HRESULT rc;
+    /** Do we need to send updated capabilities to the VMM device? */
+    bool fUpdateCaps = FALSE;
+    uint32_t fButtons;
 
     AutoCaller autoCaller(this);
     if (FAILED(autoCaller.rc())) return autoCaller.rc();
 
-    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
-
-    LogRel3(("%s: dx=%d, dy=%d, dz=%d, dw=%d\n", __PRETTY_FUNCTION__,
-             dx, dy, dz, dw));
-    /*
-     * This method being called implies that the host no
-     * longer wants to use absolute coordinates. If the VMM
-     * device isn't aware of that yet, tell it.
-     */
-    if (uHostCaps & VMMDEV_MOUSE_HOST_CAN_ABSOLUTE)
-    {
-        uHostCaps &= ~VMMDEV_MOUSE_HOST_CAN_ABSOLUTE;
+    {
+        AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+
+        LogRel3(("%s: dx=%d, dy=%d, dz=%d, dw=%d\n", __PRETTY_FUNCTION__,
+                 dx, dy, dz, dw));
+        /* Make sure that the guest knows that we are sending real movement
+         * events to the PS/2 device and not just dummy wake-up ones. */
+        if (uHostCaps & VMMDEV_MOUSE_HOST_CAN_ABSOLUTE)
+        {
+            uHostCaps &= ~VMMDEV_MOUSE_HOST_CAN_ABSOLUTE;
+            fUpdateCaps = TRUE;
+        }
+
+        fButtons = mouseButtonsToPDM(buttonState);
+    }
+    if (fUpdateCaps)
         setVMMDevMouseCaps(uHostCaps);
-    }
-
-    uint32_t fButtons = mouseButtonsToPDM(buttonState);
     rc = reportRelEventToMouseDev(dx, dy, dz, dw, fButtons);
-    if (SUCCEEDED(rc))
-        mLastButtons = fButtons;
 
     return rc;
@@ -397 +434 @@
 
 /**
- * Convert an X value in screen co-ordinates (starting from 1) to a value
- * from 0 to 0xffff.
+ * Convert an (X, Y) value pair in screen co-ordinates (starting from 1) to a
+ * value from 0 to 0xffff.
  *
  * @returns   COM status value
  */
-HRESULT Mouse::convertDisplayWidth(LONG x, uint32_t *pcX)
+HRESULT Mouse::convertDisplayRes(LONG x, LONG y, uint32_t *pcX, uint32_t *pcY)
 {
     AssertPtrReturn(pcX, E_POINTER);
-    Display *pDisplay = mParent->getDisplay();
-    ComAssertRet(pDisplay, E_FAIL);
-
-    ULONG displayWidth;
-    HRESULT rc = pDisplay->GetScreenResolution (0, &displayWidth, NULL, NULL);
-    if (FAILED(rc)) return rc;
-
-    *pcX = displayWidth ? ((x - 1) * 0xFFFF) / displayWidth: 0;
-    return S_OK;
-}
-
-/**
- * Convert a Y value in screen co-ordinates (starting from 1) to a value
- * from 0 to 0xffff.
- *
- * @returns   COM status value
- */
-HRESULT Mouse::convertDisplayHeight(LONG y, uint32_t *pcY)
-{
     AssertPtrReturn(pcY, E_POINTER);
     Display *pDisplay = mParent->getDisplay();
     ComAssertRet(pDisplay, E_FAIL);
 
-    ULONG displayHeight;
-    HRESULT rc = pDisplay->GetScreenResolution (0, NULL, &displayHeight, NULL);
-    if (FAILED(rc)) return rc;
-
+    ULONG displayWidth, displayHeight;
+    /* Takes the display lock */
+    HRESULT rc = pDisplay->GetScreenResolution (0, &displayWidth, &displayHeight,
+                                                NULL);
+    if (FAILED(rc))
+        return rc;
+
+    *pcX = displayWidth ? ((x - 1) * 0xFFFF) / displayWidth: 0;
     *pcY = displayHeight ? ((y - 1) * 0xFFFF) / displayHeight: 0;
     return S_OK;
@@ -438 +460 @@
 
 /**
- * Send an absolute mouse event to the VM. This only works
- * when the required guest support has been installed.
+ * Send an absolute mouse event to the VM. This requires either VirtualBox-
+ * specific drivers installed in the guest or absolute pointing device
+ * emulation.
  *
  * @returns COM status code
@@ -453 +476 @@
     if (FAILED(autoCaller.rc())) return autoCaller.rc();
 
-    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
-
     LogRel3(("%s: x=%d, y=%d, dz=%d, dw=%d, buttonState=0x%x\n",
              __PRETTY_FUNCTION__, x, y, dz, dw, buttonState));
 
-    uint32_t mouseXAbs;
+    uint32_t mouseXAbs, mouseYAbs;
+    /** Do we need to send updated capabilities to the VMM device? */
+    bool fUpdateCaps = FALSE;
+
     /** @todo the front end should do this conversion to avoid races */
-    HRESULT rc = convertDisplayWidth(x, &mouseXAbs);
+    /** @note Or maybe not... races are pretty inherent in everything done in
+     *        this object and not really bad as far as I can see. */
+    HRESULT rc = convertDisplayRes(x, y, &mouseXAbs, &mouseYAbs);
     if (FAILED(rc)) return rc;
 
-    /**
-     * @todo multi-monitor Windows guests expect this to be unbounded.
-     * Understand the issues involved and fix for the rest.
-     */
+    /** @todo multi-monitor Windows guests expect this to be unbounded.
+     * Understand the issues involved and fix for the rest. */
     /* if (mouseXAbs > 0xffff)
-        mouseXAbs = mLastAbsX; */
-
-    uint32_t mouseYAbs;
-    rc = convertDisplayHeight(y, &mouseYAbs);
-    if (FAILED(rc)) return rc;
-    /* if (mouseYAbs > 0xffff)
+        mouseXAbs = mLastAbsX;
+    if (mouseYAbs > 0xffff)
         mouseYAbs = mLastAbsY; */
-
-    uint32_t fButtons = mouseButtonsToPDM(buttonState);
 
     uint32_t mouseCaps;
     rc = getVMMDevMouseCaps(&mouseCaps);
     if (FAILED(rc)) return rc;
-
-    /*
-     * This method being called implies that the host wants
-     * to use absolute coordinates. If the VMM device isn't
-     * aware of that yet, tell it.
-     */
-    if (!(mouseCaps & VMMDEV_MOUSE_HOST_CAN_ABSOLUTE))
-    {
-        uHostCaps |= VMMDEV_MOUSE_HOST_CAN_ABSOLUTE;
+    uint32_t fButtons = mouseButtonsToPDM(buttonState);
+
+    /* If we are doing old-style (IRQ-less) absolute reporting to the VMM
+     * device then make sure the guest is aware of it, so that it knows to
+     * ignore relative movement on the PS/2 device. */
+    {
+        AutoWriteLock aLock(this COMMA_LOCKVAL_SRC_POS);
+
+        /** @todo rename that capability to VMMDEV_MOUSE_HOST_WANTS_ABSOLUTE */
+        if (fVMMDevCanAbs && !(uHostCaps & VMMDEV_MOUSE_HOST_CAN_ABSOLUTE))
+        {
+            uHostCaps |= VMMDEV_MOUSE_HOST_CAN_ABSOLUTE;
+            fUpdateCaps = TRUE;
+        }
+    }
+    if (fUpdateCaps)
         setVMMDevMouseCaps(uHostCaps);
-    }
-
-    if (fVMMDevCanAbs)
-    {
-        /*
-         * Send the absolute mouse position to the VMM device.
-         */
-        rc = reportAbsEventToVMMDev(mouseXAbs, mouseYAbs);
-        /* We may need to send an additional event for button information or
-         * to wake up older guests to the changed absolute co-ordinates.  If
-         * the event is a pure wake up one, we make sure it contains some
-         * (possibly phony) event data to make sure it isn't just discarded
-         * on the way. */
-        bool fNeedsJiggle = !(mouseCaps & VMMDEV_MOUSE_GUEST_USES_VMMDEV);
-        if (fNeedsJiggle || fButtons != mLastButtons || dz || dw)
-        {
-            rc = reportRelEventToMouseDev(fNeedsJiggle ? 1 : 0, 0, dz, dw,
-                                          fButtons);
-        }
-    }
-    else
-        rc = reportAbsEventToMouseDev(mouseXAbs, mouseYAbs, dz, dw, fButtons);
-
-    if (FAILED(rc)) return rc;
-
-    mLastAbsX = mouseXAbs;
-    mLastAbsY = mouseYAbs;
-
-    mLastButtons = fButtons;
+
+    /** @todo rename that capability to VMMDEV_MOUSE_GUEST_USES_EVENT */
+    rc = reportAbsEvent(mouseXAbs, mouseYAbs, dz, dw, fButtons,
+                        mouseCaps & VMMDEV_MOUSE_GUEST_USES_VMMDEV);
+
     return rc;
 }
@@ -532 +533 @@
     bool fRelDev = false;
     uint32_t u32MouseCaps;
-    for (unsigned i = 0; i < MOUSE_MAX_DEVICES; ++i)
-        if (mpDrv[i])
-        {
-           if (mpDrv[i]->u32DevCaps & MOUSE_DEVCAP_ABSOLUTE)
-               fAbsDev = true;
-           if (mpDrv[i]->u32DevCaps & MOUSE_DEVCAP_RELATIVE)
-               fRelDev = true;
-        }
+    
+    {
+        AutoWriteLock aLock(this COMMA_LOCKVAL_SRC_POS);
+
+        for (unsigned i = 0; i < MOUSE_MAX_DEVICES; ++i)
+            if (mpDrv[i])
+            {
+               if (mpDrv[i]->u32DevCaps & MOUSE_DEVCAP_ABSOLUTE)
+                   fAbsDev = true;
+               if (mpDrv[i]->u32DevCaps & MOUSE_DEVCAP_RELATIVE)
+                   fRelDev = true;
+            }
+        if (fAbsDev && !(uHostCaps & VMMDEV_MOUSE_HOST_HAS_ABS_DEV))
+            uHostCaps |= VMMDEV_MOUSE_HOST_HAS_ABS_DEV;
+        if (!fAbsDev && (uHostCaps & VMMDEV_MOUSE_HOST_HAS_ABS_DEV))
+            uHostCaps &= ~VMMDEV_MOUSE_HOST_HAS_ABS_DEV;
+    }
     if (SUCCEEDED(getVMMDevMouseCaps(&u32MouseCaps)))
         fVMMDevCanAbs =    (u32MouseCaps & VMMDEV_MOUSE_GUEST_CAN_ABSOLUTE)
@@ -545 +555 @@
     else
         fVMMDevCanAbs = false;
+    /** @todo this call takes the Console lock in order to update the cached
+     * callback data atomically.  However I can't see any sign that the cached
+     * data is ever used again. */
     mParent->onMouseCapabilityChange(fAbsDev || fVMMDevCanAbs, fRelDev,
                                      fVMMDevNeedsHostCursor);
     /** @todo if this gets called during device initialisation we get an
      * error due to VMMDev not being initialised yet. */
-    if (fAbsDev && !(uHostCaps & VMMDEV_MOUSE_HOST_HAS_ABS_DEV))
-        uHostCaps |= VMMDEV_MOUSE_HOST_HAS_ABS_DEV;
-    if (!fAbsDev && (uHostCaps & VMMDEV_MOUSE_HOST_HAS_ABS_DEV))
-        uHostCaps &= ~VMMDEV_MOUSE_HOST_HAS_ABS_DEV;
     setVMMDevMouseCaps(uHostCaps);
 }
@@ -664 +673 @@
     pData->pMouse = (Mouse *)pv;        /** @todo Check this cast! */
     unsigned cDev;
-    for (cDev = 0; cDev < MOUSE_MAX_DEVICES; ++cDev)
-        if (!pData->pMouse->mpDrv[cDev])
-        {
-            pData->pMouse->mpDrv[cDev] = pData;
-            break;
-        }
+    {
+        AutoReadLock mouseLock(pData->pMouse COMMA_LOCKVAL_SRC_POS);
+
+        for (cDev = 0; cDev < MOUSE_MAX_DEVICES; ++cDev)
+            if (!pData->pMouse->mpDrv[cDev])
+            {
+                pData->pMouse->mpDrv[cDev] = pData;
+                break;
+            }
+    }
     if (cDev == MOUSE_MAX_DEVICES)
         return VERR_NO_MORE_HANDLES;

trunk/src/VBox/Main/include/MouseImpl.h

@@ -120 +120 @@
                                  int32_t dz, int32_t dw, uint32_t fButtons);
     HRESULT reportAbsEventToVMMDev(uint32_t mouseXAbs, uint32_t mouseYAbs);
-    HRESULT convertDisplayWidth(LONG x, uint32_t *pcX);
-    HRESULT convertDisplayHeight(LONG y, uint32_t *pcY);
+    HRESULT reportAbsEvent(uint32_t mouseXAbs, uint32_t mouseYAbs,
+                           int32_t dz, int32_t dw, uint32_t fButtons,
+                           bool fUsesVMMDevEvent);
+    HRESULT convertDisplayRes(LONG x, LONG y, uint32_t *pcX, uint32_t *pcY);
 
     void sendMouseCapsNotifications(void);