Fix issue with mouse input for viewport camera (#3210)

* fix for drift accumulating in the viewport camera

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

* fix typo and update how events are stored

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

* respond to PR feedback and fix linux and windows build issues

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

* fix failing unit tests in camera input

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

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

@@ -8,12 +8,12 @@
 
 #include "CameraInput.h"
 
-#include <AzCore/Console/IConsole.h>
 #include <AzCore/Math/MathUtils.h>
 #include <AzCore/Math/Plane.h>
 #include <AzCore/std/numeric.h>
 #include <AzFramework/Input/Devices/Keyboard/InputDeviceKeyboard.h>
 #include <AzFramework/Input/Devices/Mouse/InputDeviceMouse.h>
+#include <AzFramework/Windowing/WindowBus.h>
 
 namespace AzFramework
 {
@@ -26,6 +26,13 @@ namespace AzFramework
         "The default height of the ground plane to do intersection tests against when orbiting");
     AZ_CVAR(float, ed_cameraSystemMinOrbitDistance, 10.0f, nullptr, AZ::ConsoleFunctorFlags::Null, "");
     AZ_CVAR(float, ed_cameraSystemMaxOrbitDistance, 50.0f, nullptr, AZ::ConsoleFunctorFlags::Null, "");
+    AZ_CVAR(
+        bool,
+        ed_cameraSystemUseCursor,
+        true,
+        nullptr,
+        AZ::ConsoleFunctorFlags::Null,
+        "Should the camera use cursor absolute positions or motion deltas");
 
     //! return -1.0f if inverted, 1.0f otherwise
     constexpr static float Invert(const bool invert)
@@ -134,9 +141,13 @@ namespace AzFramework
 
     bool CameraSystem::HandleEvents(const InputEvent& event)
     {
-        if (const auto& horizonalMotion = AZStd::get_if<HorizontalMotionEvent>(&event))
+        if (const auto& cursor = AZStd::get_if<CursorEvent>(&event))
         {
-            m_motionDelta.m_x = horizonalMotion->m_delta;
+            m_cursorState.SetCurrentPosition(cursor->m_position);
+        }
+        else if (const auto& horizontalMotion = AZStd::get_if<HorizontalMotionEvent>(&event))
+        {
+            m_motionDelta.m_x = horizontalMotion->m_delta;
         }
         else if (const auto& verticalMotion = AZStd::get_if<VerticalMotionEvent>(&event))
         {
@@ -147,15 +158,18 @@ namespace AzFramework
             m_scrollDelta = scroll->m_delta;
         }
 
-        m_handlingEvents = m_cameras.HandleEvents(event, m_motionDelta, m_scrollDelta);
+        m_handlingEvents =
+            m_cameras.HandleEvents(event, ed_cameraSystemUseCursor ? m_cursorState.CursorDelta() : m_motionDelta, m_scrollDelta);
 
         return m_handlingEvents;
     }
 
     Camera CameraSystem::StepCamera(const Camera& targetCamera, const float deltaTime)
     {
-        const auto nextCamera = m_cameras.StepCamera(targetCamera, m_motionDelta, m_scrollDelta, deltaTime);
+        const auto nextCamera = m_cameras.StepCamera(
+            targetCamera, ed_cameraSystemUseCursor ? m_cursorState.CursorDelta() : m_motionDelta, m_scrollDelta, deltaTime);
 
+        m_cursorState.Update();
         m_motionDelta = ScreenVector{ 0, 0 };
         m_scrollDelta = 0.0f;
 
@@ -727,18 +741,36 @@ namespace AzFramework
         Camera camera;
         // note: the math for the lerp smoothing implementation for camera rotation and translation was inspired by this excellent
         // article by Scott Lembcke: https://www.gamasutra.com/blogs/ScottLembcke/20180404/316046/Improved_Lerp_Smoothing.php
-        const float lookRate = AZStd::exp2(cameraProps.m_rotateSmoothnessFn());
-        const float lookT = AZStd::exp2(-lookRate * deltaTime);
-        camera.m_pitch = AZ::Lerp(targetCamera.m_pitch, currentCamera.m_pitch, lookT);
-        camera.m_yaw = AZ::Lerp(targetYaw, currentYaw, lookT);
-        const float moveRate = AZStd::exp2(cameraProps.m_translateSmoothnessFn());
-        const float moveT = AZStd::exp2(-moveRate * deltaTime);
-        camera.m_lookDist = AZ::Lerp(targetCamera.m_lookDist, currentCamera.m_lookDist, moveT);
-        camera.m_lookAt = targetCamera.m_lookAt.Lerp(currentCamera.m_lookAt, moveT);
+        if (cameraProps.m_rotateSmoothingEnabledFn())
+        {
+            const float lookRate = AZStd::exp2(cameraProps.m_rotateSmoothnessFn());
+            const float lookTime = AZStd::exp2(-lookRate * deltaTime);
+            camera.m_pitch = AZ::Lerp(targetCamera.m_pitch, currentCamera.m_pitch, lookTime);
+            camera.m_yaw = AZ::Lerp(targetYaw, currentYaw, lookTime);
+        }
+        else
+        {
+            camera.m_pitch = targetCamera.m_pitch;
+            camera.m_yaw = targetYaw;
+        }
+
+        if (cameraProps.m_translateSmoothingEnabledFn())
+        {
+            const float moveRate = AZStd::exp2(cameraProps.m_translateSmoothnessFn());
+            const float moveTime = AZStd::exp2(-moveRate * deltaTime);
+            camera.m_lookDist = AZ::Lerp(targetCamera.m_lookDist, currentCamera.m_lookDist, moveTime);
+            camera.m_lookAt = targetCamera.m_lookAt.Lerp(currentCamera.m_lookAt, moveTime);
+        }
+        else
+        {
+            camera.m_lookDist = targetCamera.m_lookDist;
+            camera.m_lookAt = targetCamera.m_lookAt;
+        }
+
         return camera;
     }
 
-    InputEvent BuildInputEvent(const InputChannel& inputChannel)
+    InputEvent BuildInputEvent(const InputChannel& inputChannel, const WindowSize& windowSize)
     {
         const auto& inputChannelId = inputChannel.GetInputChannelId();
         const auto& inputDeviceId = inputChannel.GetInputDevice().GetInputDeviceId();
@@ -753,7 +785,16 @@ namespace AzFramework
         // accept active mouse channel updates, inactive movement channels will just have a 0 delta
         if (inputChannel.IsActive())
         {
-            if (inputChannelId == InputDeviceMouse::Movement::X)
+            if (inputChannelId == InputDeviceMouse::SystemCursorPosition)
+            {
+                const auto* position = inputChannel.GetCustomData<AzFramework::InputChannel::PositionData2D>();
+                AZ_Assert(position, "Expected PositionData2D but found nullptr");
+
+                return CursorEvent{ ScreenPoint(
+                    position->m_normalizedPosition.GetX() * windowSize.m_width,
+                    position->m_normalizedPosition.GetY() * windowSize.m_height) };
+            }
+            else if (inputChannelId == InputDeviceMouse::Movement::X)
             {
                 return HorizontalMotionEvent{ aznumeric_cast<int>(inputChannel.GetValue()) };
             }
@@ -761,6 +802,7 @@ namespace AzFramework
             {
                 return VerticalMotionEvent{ aznumeric_cast<int>(inputChannel.GetValue()) };
             }
+
             else if (inputChannelId == InputDeviceMouse::Movement::Z)
             {
                 return ScrollEvent{ inputChannel.GetValue() };

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

@@ -8,17 +8,23 @@
 
 #pragma once
 
+#include <AzCore/Console/IConsole.h>
 #include <AzCore/Math/Matrix3x3.h>
 #include <AzCore/Math/Transform.h>
 #include <AzCore/std/containers/variant.h>
 #include <AzCore/std/optional.h>
 #include <AzFramework/Input/Channels/InputChannel.h>
 #include <AzFramework/Viewport/ClickDetector.h>
+#include <AzFramework/Viewport/CursorState.h>
 #include <AzFramework/Viewport/ScreenGeometry.h>
 #include <AzFramework/Viewport/ViewportId.h>
 
 namespace AzFramework
 {
+    AZ_CVAR_EXTERNED(bool, ed_cameraSystemUseCursor);
+
+    struct WindowSize;
+
     //! Returns Euler angles (pitch, roll, yaw) for the incoming orientation.
     //! @note Order of rotation is Z, Y, X.
     AZ::Vector3 EulerAngles(const AZ::Matrix3x3& orientation);
@@ -79,6 +85,11 @@ namespace AzFramework
     using HorizontalMotionEvent = MotionEvent<struct HorizontalMotionTag>;
     using VerticalMotionEvent = MotionEvent<struct VerticalMotionTag>;
 
+    struct CursorEvent
+    {
+        ScreenPoint m_position;
+    };
+
     struct ScrollEvent
     {
         float m_delta;
@@ -93,7 +104,8 @@ namespace AzFramework
     };
 
     //! Represents a type-safe union of input events that are handled by the camera system.
-    using InputEvent = AZStd::variant<AZStd::monostate, HorizontalMotionEvent, VerticalMotionEvent, ScrollEvent, DiscreteInputEvent>;
+    using InputEvent =
+        AZStd::variant<AZStd::monostate, HorizontalMotionEvent, VerticalMotionEvent, CursorEvent, ScrollEvent, DiscreteInputEvent>;
 
     //! Base class for all camera behaviors.
     //! The core interface consists of:
@@ -219,10 +231,14 @@ namespace AzFramework
     //! Properties to use to configure behavior across all types of camera.
     struct CameraProps
     {
-        AZStd::function<float()>
-            m_rotateSmoothnessFn; //!< Rotate smoothing value (useful approx range 3-6, higher values give sharper feel).
-        AZStd::function<float()>
-            m_translateSmoothnessFn; //!< Translate smoothing value (useful approx range 3-6, higher values give sharper feel).
+        //! Rotate smoothing value (useful approx range 3-6, higher values give sharper feel).
+        AZStd::function<float()> m_rotateSmoothnessFn;
+        //! Translate smoothing value (useful approx range 3-6, higher values give sharper feel).
+        AZStd::function<float()> m_translateSmoothnessFn;
+        //! Enable/disable rotation smoothing.
+        AZStd::function<bool()> m_rotateSmoothingEnabledFn;
+        //! Enable/disable translation smoothing.
+        AZStd::function<bool()> m_translateSmoothingEnabledFn;
     };
 
     //! An interpolation function to smoothly interpolate all camera properties from currentCamera to targetCamera.
@@ -262,12 +278,16 @@ namespace AzFramework
     public:
         bool HandleEvents(const InputEvent& event);
         Camera StepCamera(const Camera& targetCamera, float deltaTime);
-        bool HandlingEvents() const { return m_handlingEvents; }
+        bool HandlingEvents() const
+        {
+            return m_handlingEvents;
+        }
 
         Cameras m_cameras; //!< Represents a collection of camera inputs that together provide a camera controller.
 
     private:
         ScreenVector m_motionDelta; //!< The delta used for look/orbit/pan (rotation + translation) - two dimensional.
+        CursorState m_cursorState; //!< The current and previous position of the cursor (used to calculate movement delta).
         float m_scrollDelta = 0.0f; //!< The delta used for dolly/movement (translation) - one dimensional.
         bool m_handlingEvents = false; //!< Is the camera system currently handling events (events are consumed and not propagated).
     };
@@ -548,5 +568,5 @@ namespace AzFramework
     }
 
     //! Map from a generic InputChannel event to a camera specific InputEvent.
-    InputEvent BuildInputEvent(const InputChannel& inputChannel);
+    InputEvent BuildInputEvent(const InputChannel& inputChannel, const WindowSize& windowSize);
 } // namespace AzFramework

Code/Framework/AzFramework/CMakeLists.txt

@@ -70,6 +70,9 @@ if(PAL_TRAIT_BUILD_TESTS_SUPPORTED)
             PRIVATE
                 AZ::AzCore
                 AZ::AzFramework
+            PUBLIC
+                AZ::AzTest
+                AZ::AzTestShared
     )
 
     if(PAL_TRAIT_BUILD_HOST_TOOLS)

Code/Framework/AzFramework/Tests/CameraInputTests.cpp

@@ -59,10 +59,15 @@ namespace UnitTest
             m_cameraSystem->m_cameras.AddCamera(m_firstPersonRotateCamera);
             m_cameraSystem->m_cameras.AddCamera(m_firstPersonTranslateCamera);
             m_cameraSystem->m_cameras.AddCamera(orbitCamera);
+
+            // these tests rely on using motion delta, not cursor positions (default is true)
+            AzFramework::ed_cameraSystemUseCursor = false;
         }
 
         void TearDown() override
         {
+            AzFramework::ed_cameraSystemUseCursor = true;
+
             m_firstPersonRotateCamera.reset();
             m_firstPersonTranslateCamera.reset();
 

Code/Framework/AzFramework/Tests/Mocks/MockWindowRequests.h

@@ -0,0 +1,41 @@
+/*
+ * Copyright (c) Contributors to the Open 3D Engine Project.
+ * For complete copyright and license terms please see the LICENSE at the root of this distribution.
+ *
+ * SPDX-License-Identifier: Apache-2.0 OR MIT
+ *
+ */
+
+#pragma once
+
+#include <AzFramework/Windowing/WindowBus.h>
+
+#include <gmock/gmock.h>
+
+namespace UnitTest
+{
+    class MockWindowRequests : public AzFramework::WindowRequestBus::Handler
+    {
+    public:
+        void Connect(AzFramework::NativeWindowHandle handle)
+        {
+            AzFramework::WindowRequestBus::Handler::BusConnect(handle);
+        }
+        void Disconnect()
+        {
+            AzFramework::WindowRequestBus::Handler::BusDisconnect();
+        }
+
+        // AzFramework::WindowRequestBus overrides ...
+        MOCK_METHOD1(SetWindowTitle, void(const AZStd::string&));
+        MOCK_CONST_METHOD0(GetClientAreaSize, AzFramework::WindowSize());
+        MOCK_METHOD1(ResizeClientArea, void(AzFramework::WindowSize clientAreaSize));
+        MOCK_CONST_METHOD0(GetFullScreenState, bool());
+        MOCK_METHOD1(SetFullScreenState, void(bool));
+        MOCK_CONST_METHOD0(CanToggleFullScreenState, bool());
+        MOCK_METHOD0(ToggleFullScreenState, void());
+        MOCK_CONST_METHOD0(GetDpiScaleFactor, float());
+        MOCK_CONST_METHOD0(GetSyncInterval, uint32_t());
+        MOCK_CONST_METHOD0(GetDisplayRefreshRate, uint32_t());
+    };
+} // namespace UnitTest

Code/Framework/AzFramework/Tests/framework_shared_tests_files.cmake

@@ -8,6 +8,7 @@
 
 set(FILES
     Mocks/MockSpawnableEntitiesInterface.h
+    Mocks/MockWindowRequests.h
     Utils/Utils.h
     Utils/Utils.cpp
     FrameworkApplicationFixture.h