Fix mouse capture behavior for Editor Viewport (#3417)

* first pass of fixes for cursor capture and context menu

Signed-off-by: hultonha <hultonha@amazon.co.uk>

* restore previous behavior of HandleMouseMoveEvent

Signed-off-by: hultonha <hultonha@amazon.co.uk>

* tidy-up from previous cursor/input changes

Signed-off-by: hultonha <hultonha@amazon.co.uk>

* add missing casts

Signed-off-by: hultonha <hultonha@amazon.co.uk>

* small updates to support tests

Signed-off-by: hultonha <hultonha@amazon.co.uk>

* additional tests and some tidy-up

Signed-off-by: hultonha <hultonha@amazon.co.uk>

* small updates before publishing PR (comment/naming updates)

Signed-off-by: hultonha <hultonha@amazon.co.uk>

* add missing parameter to MouseInteractionEvent constructor

Signed-off-by: hultonha <hultonha@amazon.co.uk>

Code/Framework/AzCore/AzCore/std/math.h

@@ -22,6 +22,8 @@ namespace AZStd
     using std::exp2;
     using std::floor;
     using std::fmod;
+    using std::llround;
+    using std::lround;
     using std::pow;
     using std::round;
     using std::sin;

Code/Framework/AzFramework/AzFramework/Viewport/CameraInput.cpp

@@ -144,6 +144,7 @@ namespace AzFramework
         if (const auto& cursor = AZStd::get_if<CursorEvent>(&event))
         {
             m_cursorState.SetCurrentPosition(cursor->m_position);
+            m_cursorState.SetCaptured(cursor->m_captured);
         }
         else if (const auto& horizontalMotion = AZStd::get_if<HorizontalMotionEvent>(&event))
         {
@@ -790,17 +791,24 @@ namespace AzFramework
                 const auto* position = inputChannel.GetCustomData<AzFramework::InputChannel::PositionData2D>();
                 AZ_Assert(position, "Expected PositionData2D but found nullptr");
 
-                return CursorEvent{ ScreenPoint(
-                    static_cast<int>(position->m_normalizedPosition.GetX() * windowSize.m_width),
-                    static_cast<int>(position->m_normalizedPosition.GetY() * windowSize.m_height)) };
+                auto currentCursorState = AzFramework::SystemCursorState::Unknown;
+                AzFramework::InputSystemCursorRequestBus::EventResult(
+                    currentCursorState, inputDeviceId, &AzFramework::InputSystemCursorRequestBus::Events::GetSystemCursorState);
+
+                const auto x = position->m_normalizedPosition.GetX() * aznumeric_cast<float>(windowSize.m_width);
+                const auto y = position->m_normalizedPosition.GetY() * aznumeric_cast<float>(windowSize.m_height);
+                return CursorEvent{ ScreenPoint(aznumeric_cast<int>(AZStd::lround(x)), aznumeric_cast<int>(AZStd::lround(y))),
+                                    currentCursorState == AzFramework::SystemCursorState::ConstrainedAndHidden };
             }
             else if (inputChannelId == InputDeviceMouse::Movement::X)
             {
-                return HorizontalMotionEvent{ aznumeric_cast<int>(inputChannel.GetValue()) };
+                const auto x = inputChannel.GetValue();
+                return HorizontalMotionEvent{ aznumeric_cast<int>(AZStd::lround(x)) };
             }
             else if (inputChannelId == InputDeviceMouse::Movement::Y)
             {
-                return VerticalMotionEvent{ aznumeric_cast<int>(inputChannel.GetValue()) };
+                const auto y = inputChannel.GetValue();
+                return VerticalMotionEvent{ aznumeric_cast<int>(AZStd::lround(y)) };
             }
             else if (inputChannelId == InputDeviceMouse::Movement::Z)
             {

Code/Framework/AzFramework/AzFramework/Viewport/CameraInput.h

@@ -88,6 +88,7 @@ namespace AzFramework
     struct CursorEvent
     {
         ScreenPoint m_position;
+        bool m_captured = false;
     };
 
     struct ScrollEvent

Code/Framework/AzFramework/AzFramework/Viewport/CursorState.h

@@ -21,6 +21,8 @@ namespace AzFramework
         [[nodiscard]] ScreenVector CursorDelta() const;
         //! Call this in a 'handle event' call to update the most recent cursor position.
         void SetCurrentPosition(const ScreenPoint& currentPosition);
+        //! Set whether the cursor is currently being constrained (and hidden).
+        void SetCaptured(bool captured);
         //! Call this in an 'update' call to copy the current cursor position to the last
         //! cursor position.
         void Update();
@@ -28,8 +30,14 @@ namespace AzFramework
     private:
         AZStd::optional<ScreenPoint> m_lastCursorPosition;
         AZStd::optional<ScreenPoint> m_currentCursorPosition;
+        bool m_captured = false;
     };
 
+    inline void CursorState::SetCaptured(const bool captured)
+    {
+        m_captured = captured;
+    }
+
     inline void CursorState::SetCurrentPosition(const ScreenPoint& currentPosition)
     {
         m_currentCursorPosition = currentPosition;
@@ -44,9 +52,16 @@ namespace AzFramework
 
     inline void CursorState::Update()
     {
-        if (m_currentCursorPosition.has_value())
+        if (!m_captured)
         {
-            m_lastCursorPosition = m_currentCursorPosition;
+            if (m_currentCursorPosition.has_value())
+            {
+                m_lastCursorPosition = m_currentCursorPosition;
+            }
+        }
+        else
+        {
+            m_currentCursorPosition = m_lastCursorPosition;
         }
     }
 } // namespace AzFramework

Code/Framework/AzManipulatorTestFramework/Source/AzManipulatorTestFrameworkUtils.cpp

@@ -122,7 +122,7 @@ namespace AzManipulatorTestFramework
 
     MouseInteractionEvent CreateMouseInteractionEvent(const MouseInteraction& mouseInteraction, MouseEvent event)
     {
-        return MouseInteractionEvent(mouseInteraction, event);
+        return MouseInteractionEvent(mouseInteraction, event, /*captured=*/false);
     }
 
     void DispatchMouseInteractionEvent(const MouseInteractionEvent& event)

Code/Framework/AzToolsFramework/AzToolsFramework/Input/QtEventToAzInputManager.cpp

@@ -158,6 +158,16 @@ namespace AzToolsFramework
         SetImplementation(nullptr);
     }
 
+    void QtEventToAzInputMapper::EditorQtMouseDevice::SetSystemCursorState(const AzFramework::SystemCursorState systemCursorState)
+    {
+        m_systemCursorState = systemCursorState;
+    }
+
+    AzFramework::SystemCursorState QtEventToAzInputMapper::EditorQtMouseDevice::GetSystemCursorState() const
+    {
+        return m_systemCursorState;
+    }
+
     QtEventToAzInputMapper::QtEventToAzInputMapper(QWidget* sourceWidget, int syntheticDeviceId)
         : QObject(sourceWidget)
         , m_sourceWidget(sourceWidget)
@@ -210,12 +220,15 @@ namespace AzToolsFramework
         if (m_capturingCursor != enabled)
         {
             m_capturingCursor = enabled;
+
             if (m_capturingCursor)
             {
+                m_mouseDevice->SetSystemCursorState(AzFramework::SystemCursorState::ConstrainedAndHidden);
                 qApp->setOverrideCursor(Qt::BlankCursor);
             }
             else
             {
+                m_mouseDevice->SetSystemCursorState(AzFramework::SystemCursorState::UnconstrainedAndVisible);
                 qApp->restoreOverrideCursor();
             }
         }
@@ -238,10 +251,22 @@ namespace AzToolsFramework
             return false;
         }
 
-        // If our focus changes, go ahead and reset all input devices.
         if (eventType == QEvent::FocusIn || eventType == QEvent::FocusOut)
         {
+            // If our focus changes, go ahead and reset all input devices.
             HandleFocusChange(event);
+
+            // If we focus in on the source widget and the mouse is contained in its
+            // bounds, refresh the cached cursor position to ensure it is up to date (this
+            // ensures cursor positions are refreshed correctly with context menu focus changes)
+            if (eventType == QEvent::FocusIn)
+            {
+                const auto widgetCursorPosition = m_sourceWidget->mapFromGlobal(QCursor::pos());
+                if (m_sourceWidget->geometry().contains(widgetCursorPosition))
+                {
+                    HandleMouseMoveEvent(widgetCursorPosition);
+                }
+            }
         }
         // Map key events to input channels.
         // ShortcutOverride is used in lieu of KeyPress for high priority input channels like Alt
@@ -249,7 +274,7 @@ namespace AzToolsFramework
         else if (
             eventType == QEvent::Type::KeyPress || eventType == QEvent::Type::KeyRelease || eventType == QEvent::Type::ShortcutOverride)
         {
-            QKeyEvent* keyEvent = static_cast<QKeyEvent*>(event);
+            auto keyEvent = static_cast<QKeyEvent*>(event);
             HandleKeyEvent(keyEvent);
         }
         // Map mouse events to input channels.
@@ -257,20 +282,20 @@ namespace AzToolsFramework
             eventType == QEvent::Type::MouseButtonPress || eventType == QEvent::Type::MouseButtonRelease ||
             eventType == QEvent::Type::MouseButtonDblClick)
         {
-            QMouseEvent* mouseEvent = static_cast<QMouseEvent*>(event);
+            auto mouseEvent = static_cast<QMouseEvent*>(event);
             HandleMouseButtonEvent(mouseEvent);
         }
         // Map mouse movement to the movement input channels.
         // This includes SystemCursorPosition alongside Movement::X and Movement::Y.
         else if (eventType == QEvent::Type::MouseMove)
         {
-            QMouseEvent* mouseEvent = static_cast<QMouseEvent*>(event);
-            HandleMouseMoveEvent(mouseEvent);
+            auto mouseEvent = static_cast<QMouseEvent*>(event);
+            HandleMouseMoveEvent(mouseEvent->pos());
         }
         // Map wheel events to the mouse Z movement channel.
         else if (eventType == QEvent::Type::Wheel)
         {
-            QWheelEvent* wheelEvent = static_cast<QWheelEvent*>(event);
+            auto wheelEvent = static_cast<QWheelEvent*>(event);
             HandleWheelEvent(wheelEvent);
         }
 
@@ -345,9 +370,8 @@ namespace AzToolsFramework
         return QPoint{ denormalizedX, denormalizedY };
     }
 
-    void QtEventToAzInputMapper::HandleMouseMoveEvent(QMouseEvent* mouseEvent)
+    void QtEventToAzInputMapper::HandleMouseMoveEvent(const QPoint& cursorPosition)
     {
-        const QPoint cursorPosition = mouseEvent->pos();
         const QPoint cursorDelta = cursorPosition - m_previousCursorPosition;
 
         m_mouseDevice->m_cursorPositionData2D->m_normalizedPosition = WidgetPositionToNormalizedPosition(cursorPosition);
@@ -357,17 +381,14 @@ namespace AzToolsFramework
 
         if (m_capturingCursor)
         {
-            // Reset our cursor position to the previous point.
-            const QPoint targetScreenPosition = m_sourceWidget->mapToGlobal(m_previousCursorPosition);
-            AzQtComponents::SetCursorPos(targetScreenPosition);
-
-            // Even though we just set the cursor position, there are edge cases such as remote desktop that will leave
-            // the cursor position unchanged. For safety, we re-cache our last cursor position for delta generation.
-            const QPoint actualWidgetPosition = m_sourceWidget->mapFromGlobal(QCursor::pos());
-            m_mouseDevice->m_cursorPositionData2D->m_normalizedPosition = WidgetPositionToNormalizedPosition(actualWidgetPosition);
+            // Reset our cursor position to the previous point
+            const QPoint screenCursorPosition = m_sourceWidget->mapToGlobal(m_previousCursorPosition);
+            AzQtComponents::SetCursorPos(screenCursorPosition);
+        }
+        else
+        {
+            m_previousCursorPosition = cursorPosition;
         }
-
-        m_previousCursorPosition = cursorPosition;
     }
 
     void QtEventToAzInputMapper::HandleKeyEvent(QKeyEvent* keyEvent)

Code/Framework/AzToolsFramework/AzToolsFramework/Input/QtEventToAzInputManager.h

@@ -105,7 +105,14 @@ namespace AzToolsFramework
         public:
             EditorQtMouseDevice(AzFramework::InputDeviceId id);
 
+            // AzFramework::InputDeviceMouse overrides ...
+            void SetSystemCursorState(AzFramework::SystemCursorState systemCursorState) override;
+            AzFramework::SystemCursorState GetSystemCursorState() const override;
+
             friend class QtEventToAzInputMapper;
+
+        private:
+            AzFramework::SystemCursorState m_systemCursorState = AzFramework::SystemCursorState::UnconstrainedAndVisible;
         };
 
         // Emits InputChannelUpdated if channel has transitioned in state (i.e. has gone from active to inactive or vice versa).
@@ -122,7 +129,7 @@ namespace AzToolsFramework
         // Handle mouse click events.
         void HandleMouseButtonEvent(QMouseEvent* mouseEvent);
         // Handle mouse move events.
-        void HandleMouseMoveEvent(QMouseEvent* mouseEvent);
+        void HandleMouseMoveEvent(const QPoint& cursorPosition);
         // Handles key press / release events (or ShortcutOverride events for keys listed in m_highPriorityKeys).
         void HandleKeyEvent(QKeyEvent* keyEvent);
         // Handles mouse wheel events.

Code/Framework/AzToolsFramework/AzToolsFramework/UnitTest/AzToolsFrameworkTestHelpers.cpp

@@ -115,7 +115,7 @@ namespace UnitTest
                     handled, AzToolsFramework::GetEntityContextId(),
                     &EditorInteractionSystemViewportSelectionRequestBus::Events::InternalHandleMouseViewportInteraction,
                     AzToolsFramework::ViewportInteraction::MouseInteractionEvent(
-                        mouseInteraction, AzToolsFramework::ViewportInteraction::MouseEvent::Down));
+                        mouseInteraction, AzToolsFramework::ViewportInteraction::MouseEvent::Down, /*captured=*/false));
                 return handled;
             }
         }

Code/Framework/AzToolsFramework/AzToolsFramework/Viewport/EditorContextMenu.cpp

@@ -41,7 +41,8 @@ namespace AzToolsFramework
                 ViewportInteraction::QPointFromScreenPoint(mouseInteraction.m_mouseInteraction.m_mousePick.m_screenCoordinates);
 
             // if the mouse hasn't moved, open the pop-up menu
-            if ((currentScreenCoords - contextMenu.m_clickPoint).manhattanLength() < ed_contextMenuDisplayThreshold)
+            if ((currentScreenCoords - contextMenu.m_clickPoint).manhattanLength() < ed_contextMenuDisplayThreshold &&
+                !mouseInteraction.m_captured)
             {
                 QWidget* parent = nullptr;
                 ViewportInteraction::MainEditorViewportInteractionRequestBus::EventResult(

Code/Framework/AzToolsFramework/AzToolsFramework/Viewport/ViewportTypes.h

@@ -212,9 +212,10 @@ namespace AzToolsFramework
             static void Reflect(AZ::SerializeContext& context);
 
             //! Constructor to create a default MouseInteractionEvent
-            MouseInteractionEvent(MouseInteraction mouseInteraction, const MouseEvent mouseEvent)
+            MouseInteractionEvent(MouseInteraction mouseInteraction, const MouseEvent mouseEvent, const bool captured)
                 : m_mouseInteraction(std::move(mouseInteraction))
                 , m_mouseEvent(mouseEvent)
+                , m_captured(captured)
             {
             }
 
@@ -228,6 +229,7 @@ namespace AzToolsFramework
 
             MouseInteraction m_mouseInteraction; //!< Mouse state.
             MouseEvent m_mouseEvent; //!< Mouse event.
+            bool m_captured = false; //!< Is the mouse cursor being captured during the event.
 
             //! Special friend function to return the mouse wheel delta (scroll amount)
             //! if the event was of type MouseEvent::Wheel.