Fix for events that should have been consumed by manipulators (#3108)

* fix for events that should have been consumed by manipulators making their way to the main viewport handler

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

* add missing include for SANDBOX_API macro

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

* add dependency on Qt::Test for AzToolsFrameworkTestCommon

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

* fix order of buttons passed to QMouseEvent

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

* potential fix for vtable error on linux

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

* potential fix for vtable error on linux again

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

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

@@ -162,7 +162,6 @@ namespace AzToolsFramework
         : QObject(sourceWidget)
         , m_sourceWidget(sourceWidget)
         , m_keyboardModifiers(AZStd::make_shared<AzFramework::ModifierKeyStates>())
-        , m_cursorPosition(AZStd::make_shared<AzFramework::InputChannel::PositionData2D>())
     {
         InitializeKeyMappings();
         InitializeMouseButtonMappings();
@@ -230,24 +229,17 @@ namespace AzToolsFramework
             return false;
         }
 
-        // Because there's no "end" to mouse movement and wheel events, we reset mouse movement channels that have been opened
-        // during the next processed non-mouse event.
-        if (m_mouseChannelsNeedUpdate && event->type() != QEvent::Type::MouseMove && event->type() != QEvent::Type::Wheel)
-        {
-            m_cursorPosition->m_normalizedPositionDelta = AZ::Vector2::CreateZero();
-            ProcessPendingMouseEvents();
-            m_mouseChannelsNeedUpdate = false;
-        }
+        const auto eventType = event->type();
 
         // Only accept mouse & key release events that originate from an object that is not our target widget,
         // as we don't want to erroneously intercept user input meant for another component.
-        if (object != m_sourceWidget && event->type() != QEvent::Type::KeyRelease && event->type() != QEvent::Type::MouseButtonRelease)
+        if (object != m_sourceWidget && eventType != QEvent::Type::KeyRelease && eventType != QEvent::Type::MouseButtonRelease)
         {
             return false;
         }
 
         // If our focus changes, go ahead and reset all input devices.
-        if (event->type() == QEvent::FocusIn || event->type() == QEvent::FocusOut)
+        if (eventType == QEvent::FocusIn || eventType == QEvent::FocusOut)
         {
             HandleFocusChange(event);
         }
@@ -255,27 +247,28 @@ namespace AzToolsFramework
         // ShortcutOverride is used in lieu of KeyPress for high priority input channels like Alt
         // that need to be accepted and stopped before they bubble up and cause unintended behavior.
         else if (
-            event->type() == QEvent::Type::KeyPress || event->type() == QEvent::Type::KeyRelease ||
-            event->type() == QEvent::Type::ShortcutOverride)
+            eventType == QEvent::Type::KeyPress || eventType == QEvent::Type::KeyRelease || eventType == QEvent::Type::ShortcutOverride)
         {
             QKeyEvent* keyEvent = static_cast<QKeyEvent*>(event);
             HandleKeyEvent(keyEvent);
         }
         // Map mouse events to input channels.
-        else if (event->type() == QEvent::Type::MouseButtonPress || event->type() == QEvent::Type::MouseButtonRelease || event->type() == QEvent::Type::MouseButtonDblClick)
+        else if (
+            eventType == QEvent::Type::MouseButtonPress || eventType == QEvent::Type::MouseButtonRelease ||
+            eventType == QEvent::Type::MouseButtonDblClick)
         {
             QMouseEvent* 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 (event->type() == QEvent::Type::MouseMove)
+        else if (eventType == QEvent::Type::MouseMove)
         {
             QMouseEvent* mouseEvent = static_cast<QMouseEvent*>(event);
             HandleMouseMoveEvent(mouseEvent);
         }
         // Map wheel events to the mouse Z movement channel.
-        else if (event->type() == QEvent::Type::Wheel)
+        else if (eventType == QEvent::Type::Wheel)
         {
             QWheelEvent* wheelEvent = static_cast<QWheelEvent*>(event);
             HandleWheelEvent(wheelEvent);
@@ -303,14 +296,16 @@ namespace AzToolsFramework
         auto mouseWheelChannel =
             GetInputChannel<AzFramework::InputChannelDeltaWithSharedPosition2D>(AzFramework::InputDeviceMouse::Movement::Z);
 
-        systemCursorChannel->ProcessRawInputEvent(m_cursorPosition->m_normalizedPositionDelta.GetLength());
+        systemCursorChannel->ProcessRawInputEvent(m_mouseDevice->m_cursorPositionData2D->m_normalizedPositionDelta.GetLength());
         // Generate movement events based on the pixel delta divided by the DPI scaling factor, to calculate a rough approximation
         // of cursor movement velocity.
         movementXChannel->ProcessRawInputEvent(
-            m_cursorPosition->m_normalizedPositionDelta.GetX() * aznumeric_cast<float>(m_sourceWidget->width()) / m_sourceWidget->devicePixelRatioF());
+            m_mouseDevice->m_cursorPositionData2D->m_normalizedPositionDelta.GetX() * aznumeric_cast<float>(m_sourceWidget->width()) /
+            m_sourceWidget->devicePixelRatioF());
         movementYChannel->ProcessRawInputEvent(
-            m_cursorPosition->m_normalizedPositionDelta.GetY() * aznumeric_cast<float>(m_sourceWidget->height()) / m_sourceWidget->devicePixelRatioF());
-        mouseWheelChannel->ProcessRawInputEvent(0.f);
+            m_mouseDevice->m_cursorPositionData2D->m_normalizedPositionDelta.GetY() * aznumeric_cast<float>(m_sourceWidget->height()) /
+            m_sourceWidget->devicePixelRatioF());
+        mouseWheelChannel->ProcessRawInputEvent(0.0f);
 
         NotifyUpdateChannelIfNotIdle(systemCursorChannel, nullptr);
         NotifyUpdateChannelIfNotIdle(movementXChannel, nullptr);
@@ -358,14 +353,13 @@ namespace AzToolsFramework
 
     void QtEventToAzInputMapper::HandleMouseMoveEvent(QMouseEvent* mouseEvent)
     {
-        AZ::Vector2 lastCursorPosition = m_cursorPosition->m_normalizedPosition;
+        AZ::Vector2 lastCursorPosition = m_mouseDevice->m_cursorPositionData2D->m_normalizedPosition;
 
         const QPoint mousePos = mouseEvent->pos();
         const AZ::Vector2 normalizedPosition = WidgetPositionToNormalizedPosition(mousePos);
-        m_cursorPosition->m_normalizedPositionDelta = normalizedPosition - m_cursorPosition->m_normalizedPosition;
-        m_cursorPosition->m_normalizedPosition = normalizedPosition;
+        m_mouseDevice->m_cursorPositionData2D->m_normalizedPositionDelta = normalizedPosition - m_mouseDevice->m_cursorPositionData2D->m_normalizedPosition;
+        m_mouseDevice->m_cursorPositionData2D->m_normalizedPosition = normalizedPosition;
         ProcessPendingMouseEvents();
-        m_mouseChannelsNeedUpdate = true;
 
         if (m_capturingCursor)
         {
@@ -376,7 +370,7 @@ namespace AzToolsFramework
             // 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.
             QPoint actualWidgetPosition = m_sourceWidget->mapFromGlobal(QCursor::pos());
-            m_cursorPosition->m_normalizedPosition = WidgetPositionToNormalizedPosition(actualWidgetPosition);
+            m_mouseDevice->m_cursorPositionData2D->m_normalizedPosition = WidgetPositionToNormalizedPosition(actualWidgetPosition);
         }
     }
 
@@ -427,21 +421,18 @@ namespace AzToolsFramework
         }
         cursorZChannel->ProcessRawInputEvent(aznumeric_cast<float>(wheelAngle));
         NotifyUpdateChannelIfNotIdle(cursorZChannel, wheelEvent);
-        m_mouseChannelsNeedUpdate = true;
     }
 
     void QtEventToAzInputMapper::HandleFocusChange(QEvent* event)
     {
         for (auto& channelData : m_channels)
         {
-            // If resetting the input device changed the channel state, submit it to the mapped channel list
-            // for processing.
+            // If resetting the input device changed the channel state, submit it to the mapped channel list for processing.
             if (channelData.second->IsActive())
             {
                 channelData.second->UpdateState(false);
                 NotifyUpdateChannelIfNotIdle(channelData.second, event);
             }
         }
-        m_mouseChannelsNeedUpdate = false;
     }
 } // namespace AzToolsFramework

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

@@ -138,8 +138,6 @@ namespace AzToolsFramework
 
         // The current keyboard modifier state used by our synthetic key input channels.
         AZStd::shared_ptr<AzFramework::ModifierKeyStates> m_keyboardModifiers;
-        // The current normalized cursor position used by our synthetic system cursor event.
-        AZStd::shared_ptr<AzFramework::InputChannel::PositionData2D> m_cursorPosition;
         // A lookup table for Qt key -> AZ input channel.
         AZStd::unordered_map<Qt::Key, AzFramework::InputChannelId> m_keyMappings;
         // A lookup table for Qt mouse button -> AZ input channel.
@@ -152,8 +150,6 @@ namespace AzToolsFramework
         AZStd::unordered_map<AzFramework::InputChannelId, AzFramework::InputChannel*> m_channels;
         // The source widget to map events from, used to calculate the relative mouse position within the widget bounds.
         QWidget* m_sourceWidget;
-        // Flags when mouse movement channels have been opened and may need to be closed (as there are no movement ended events).
-        bool m_mouseChannelsNeedUpdate = false;
         // Flags whether or not Qt events should currently be processed.
         bool m_enabled = true;
         // Flags whether or not the cursor is being constrained to the source widget (for invisible mouse movement).

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

@@ -27,6 +27,35 @@ using namespace AzToolsFramework;
 
 namespace UnitTest
 {
+    void MousePressAndMove(
+        QWidget* widget, const QPoint& initialPositionWidget, const QPoint& mouseDelta, const Qt::MouseButton mouseButton)
+    {
+        QPoint position = widget->mapToGlobal(initialPositionWidget);
+        QTest::mousePress(widget, mouseButton, Qt::NoModifier, position);
+
+        MouseMove(widget, initialPositionWidget, mouseDelta, mouseButton);
+    }
+
+    // Note: There are a series of bugs in Qt that appear to be preventing mouseMove events
+    // firing when sent through the QTest framework. This is a work around for our version
+    // of Qt. In future this can hopefully be simplified. See ^1 for workaround.
+    // More info: Issues with mouse move in Qt
+    // - https://bugreports.qt.io/browse/QTBUG-5232
+    // - https://bugreports.qt.io/browse/QTBUG-69414
+    // - https://lists.qt-project.org/pipermail/development/2019-July/036873.html
+    void MouseMove(QWidget* widget, const QPoint& initialPositionWidget, const QPoint& mouseDelta, const Qt::MouseButton mouseButton)
+    {
+        QPoint nextPosition = widget->mapToGlobal(initialPositionWidget + mouseDelta);
+
+        // ^1 To ensure a mouse move event is fired we must call the test mouse move function
+        // and also send a mouse move event that matches. Each on their own do not appear to
+        // work - please see the links above for more context.
+        QTest::mouseMove(widget, nextPosition);
+        QMouseEvent mouseMoveEvent(
+            QEvent::MouseMove, QPointF(nextPosition), QPointF(nextPosition), Qt::NoButton, mouseButton, Qt::NoModifier);
+        QApplication::sendEvent(widget, &mouseMoveEvent);
+    }
+
     bool TestWidget::eventFilter(QObject* watched, QEvent* event)
     {
         AZ_UNUSED(watched);

Code/Framework/AzToolsFramework/AzToolsFramework/UnitTest/AzToolsFrameworkTestHelpers.h

@@ -59,6 +59,21 @@ namespace UnitTest
 {
     constexpr AZStd::string_view prefabSystemSetting = "/Amazon/Preferences/EnablePrefabSystem";
 
+    /// Performs a mouse press and move event on the provided widget.
+    /// @param widget The widget to perform the mouse press and move on.
+    /// @param initialPositionWidget The position of the mouse relative to the widget (will be remapped to a global position internally).
+    /// @param mouseDelta How far to move the mouse.
+    /// @param mouseButton The button to be used during the press and move.
+    void MousePressAndMove(
+        QWidget* widget, const QPoint& initialPositionWidget, const QPoint& mouseDelta, Qt::MouseButton mouseButton = Qt::LeftButton);
+
+    /// Performs a mouse move event on the provided widget.
+    /// @param widget The widget to perform the mouse move on.
+    /// @param initialPositionWidget The position of the mouse relative to the widget (will be remapped to a global position internally).
+    /// @param mouseDelta How far to move the mouse (note: mouseDelta may be zero and the mouse will only be moved to initialPosition).
+    /// @param mouseButton The button to be held during the move.
+    void MouseMove(QWidget* widget, const QPoint& initialPosition, const QPoint& mouseDelta, Qt::MouseButton mouseButton = Qt::NoButton);
+
     /// Test widget to store QActions generated by EditorTransformComponentSelection.
     class TestWidget : public QWidget
     {

Code/Framework/AzToolsFramework/AzToolsFramework/Viewport/ViewportMessages.h

@@ -313,7 +313,7 @@ namespace AzToolsFramework
     //! Utility function to return EntityContextId.
     inline AzFramework::EntityContextId GetEntityContextId()
     {
-        AzFramework::EntityContextId entityContextId;
+        auto entityContextId = AzFramework::EntityContextId::CreateNull();
         EditorEntityContextRequestBus::BroadcastResult(entityContextId, &EditorEntityContextRequests::GetEditorEntityContextId);
 
         return entityContextId;

Code/Framework/AzToolsFramework/CMakeLists.txt

@@ -60,6 +60,7 @@ if(PAL_TRAIT_BUILD_TESTS_SUPPORTED)
             PUBLIC
                 AZ::AzTestShared
             PRIVATE
+                3rdParty::Qt::Test
                 3rdParty::googletest::GMock
                 3rdParty::GoogleBenchmark
                 AZ::AzToolsFramework
@@ -76,8 +77,9 @@ if(PAL_TRAIT_BUILD_TESTS_SUPPORTED)
             PRIVATE
                 Tests
         BUILD_DEPENDENCIES
-            PRIVATE
+            PUBLIC
                 AZ::AzTestShared
+            PRIVATE
                 3rdParty::Qt::Test
                 AZ::AzFrameworkTestShared
                 AZ::AzToolsFramework

Code/Framework/AzToolsFramework/Tests/SpinBoxTests.cpp

@@ -107,31 +107,6 @@ namespace UnitTest
         EXPECT_THAT(m_doubleSpinBoxWithLineEdit, Ne(nullptr));
     }
 
-    // Note: There are a series of bugs in Qt that appear to be preventing mouseMove events
-    // firing when sent through the QTest framework. This is a work around for our version
-    // of Qt. In future this can hopefully be simplified. See ^1 for workaround.
-    // More info: Issues with mouse move in Qt
-    // - https://bugreports.qt.io/browse/QTBUG-5232
-    // - https://bugreports.qt.io/browse/QTBUG-69414
-    // - https://lists.qt-project.org/pipermail/development/2019-July/036873.html
-    void MousePressAndMove(
-        QWidget* widget, const QPoint& widgetScreenPosition, const QPoint& mouseDelta)
-    {
-        QPoint position = widget->mapToGlobal(widgetScreenPosition);
-        QPoint nextPosition = widget->mapToGlobal(widgetScreenPosition + mouseDelta);
-
-        QTest::mousePress(widget, Qt::LeftButton, Qt::NoModifier, position);
-
-        // ^1 To ensure a mouse move event is fired we must call the test mouse move function
-        // and also send a mouse move event that matches. Each on their own do not appear to
-        // work - please see the links above for more context.
-        QTest::mouseMove(widget, nextPosition);
-        QMouseEvent mouseMoveEvent(
-            QEvent::MouseMove, QPointF(nextPosition), QPointF(nextPosition),
-            Qt::NoButton, Qt::LeftButton, Qt::NoModifier);
-        QApplication::sendEvent(widget, &mouseMoveEvent);
-    }
-
     TEST_F(SpinBoxFixture, SpinBoxMousePressAndMoveRightScrollsValue)
     {
         m_doubleSpinBox->setValue(10.0);