Add customization point for begin/end of camera behaviors (hide cursor with RMB look) (#1758)

* add customization points for begin/end of camera input to all cursor customization

Signed-off-by: hultonha <[email protected]>

* remove cursor experiments, use cursor bus

Signed-off-by: hultonha <[email protected]>

* run clang-format

Signed-off-by: hultonha <[email protected]>

* add additional unit tests for new camera behaviors

Signed-off-by: hultonha <[email protected]>

* update test name to use snake_case to increase readability

Signed-off-by: hultonha <[email protected]>

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

@@ -1,6 +1,6 @@
 /*
  * Copyright (c) Contributors to the Open 3D Engine Project
- * 
+ *
  * SPDX-License-Identifier: Apache-2.0 OR MIT
  *
  */
@@ -188,7 +188,7 @@ namespace AzFramework
         m_idleCameraInputs.push_back(AZStd::move(cameraInput));
     }
 
-    bool Cameras::HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, float scrollDelta)
+    bool Cameras::HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, const float scrollDelta)
     {
         bool handling = false;
         for (auto& cameraInput : m_activeCameraInputs)
@@ -308,7 +308,7 @@ namespace AzFramework
         };
     }
 
-    bool RotateCameraInput::HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, [[maybe_unused]] float scrollDelta)
+    bool RotateCameraInput::HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, [[maybe_unused]] const float scrollDelta)
     {
         const ClickDetector::ClickEvent clickEvent = [&event, this]
         {
@@ -393,7 +393,7 @@ namespace AzFramework
     }
 
     bool PanCameraInput::HandleEvents(
-        const InputEvent& event, [[maybe_unused]] const ScreenVector& cursorDelta, [[maybe_unused]] float scrollDelta)
+        const InputEvent& event, [[maybe_unused]] const ScreenVector& cursorDelta, [[maybe_unused]] const float scrollDelta)
     {
         if (const auto& input = AZStd::get_if<DiscreteInputEvent>(&event))
         {
@@ -580,7 +580,7 @@ namespace AzFramework
         m_boost = false;
     }
 
-    bool OrbitCameraInput::HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, float scrollDelta)
+    bool OrbitCameraInput::HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, const float scrollDelta)
     {
         if (const auto* input = AZStd::get_if<DiscreteInputEvent>(&event))
         {
@@ -675,7 +675,7 @@ namespace AzFramework
     }
 
     bool OrbitDollyScrollCameraInput::HandleEvents(
-        const InputEvent& event, [[maybe_unused]] const ScreenVector& cursorDelta, [[maybe_unused]] float scrollDelta)
+        const InputEvent& event, [[maybe_unused]] const ScreenVector& cursorDelta, [[maybe_unused]] const float scrollDelta)
     {
         if (const auto* scroll = AZStd::get_if<ScrollEvent>(&event))
         {
@@ -707,7 +707,7 @@ namespace AzFramework
     }
 
     bool OrbitDollyCursorMoveCameraInput::HandleEvents(
-        const InputEvent& event, [[maybe_unused]] const ScreenVector& cursorDelta, [[maybe_unused]] float scrollDelta)
+        const InputEvent& event, [[maybe_unused]] const ScreenVector& cursorDelta, [[maybe_unused]] const float scrollDelta)
     {
         if (const auto& input = AZStd::get_if<DiscreteInputEvent>(&event))
         {
@@ -747,7 +747,7 @@ namespace AzFramework
     }
 
     bool ScrollTranslationCameraInput::HandleEvents(
-        const InputEvent& event, [[maybe_unused]] const ScreenVector& cursorDelta, [[maybe_unused]] float scrollDelta)
+        const InputEvent& event, [[maybe_unused]] const ScreenVector& cursorDelta, [[maybe_unused]] const float scrollDelta)
     {
         if (const auto* scroll = AZStd::get_if<ScrollEvent>(&event))
         {

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

@@ -1,6 +1,6 @@
 /*
  * Copyright (c) Contributors to the Open 3D Engine Project
- * 
+ *
  * SPDX-License-Identifier: Apache-2.0 OR MIT
  *
  */
@@ -104,6 +104,8 @@ namespace AzFramework
     class CameraInput
     {
     public:
+        using ActivateChangeFn = AZStd::function<void()>;
+
         //! The state of activation the camera input is currently in.
         //! State changes of Activation: Idle -> Beginning -> Active -> Ending -> Idle
         enum class Activation
@@ -148,11 +150,28 @@ namespace AzFramework
 
         void ContinueActivation()
         {
+            // continue activation is called after the first step of the camera input,
+            // activation began is called once, the first time before the state is set to active
+            if (m_activation == Activation::Beginning)
+            {
+                if (m_activationBeganFn)
+                {
+                    m_activationBeganFn();
+                }
+            }
+
             m_activation = Activation::Active;
         }
 
         void ClearActivation()
         {
+            // clear activation happens after an end has been requested, activation ended
+            // is then called before the camera input is returned to idle
+            if (m_activationEndedFn)
+            {
+                m_activationEndedFn();
+            }
+
             m_activation = Activation::Idle;
         }
 
@@ -177,6 +196,16 @@ namespace AzFramework
             return false;
         }
 
+        void SetActivationBeganFn(ActivateChangeFn activationBeganFn)
+        {
+            m_activationBeganFn = AZStd::move(activationBeganFn);
+        }
+
+        void SetActivationEndedFn(ActivateChangeFn activationEndedFn)
+        {
+            m_activationEndedFn = AZStd::move(activationEndedFn);
+        }
+
     protected:
         //! Handle any state reset that may be required for the camera input (optional).
         virtual void ResetImpl()
@@ -185,6 +214,8 @@ namespace AzFramework
 
     private:
         Activation m_activation = Activation::Idle; //!< Default all camera inputs to the idle state.
+        AZStd::function<void()> m_activationBeganFn; //!< Called when the camera input successfully makes it to the active state.
+        AZStd::function<void()> m_activationEndedFn; //!< Called when the camera input ends and returns to the idle state.
     };
 
     //! Properties to use to configure behavior across all types of camera.

Code/Framework/Tests/CameraInputTests.cpp

@@ -1,6 +1,6 @@
 /*
  * Copyright (c) Contributors to the Open 3D Engine Project
- * 
+ *
  * SPDX-License-Identifier: Apache-2.0 OR MIT
  *
  */
@@ -36,8 +36,8 @@ namespace UnitTest
 
             m_cameraSystem = AZStd::make_shared<AzFramework::CameraSystem>();
 
-            auto firstPersonRotateCamera = AZStd::make_shared<AzFramework::RotateCameraInput>(AzFramework::InputDeviceMouse::Button::Right);
-            auto firstPersonTranslateCamera = AZStd::make_shared<AzFramework::TranslateCameraInput>(AzFramework::LookTranslation);
+            m_firstPersonRotateCamera = AZStd::make_shared<AzFramework::RotateCameraInput>(AzFramework::InputDeviceMouse::Button::Right);
+            m_firstPersonTranslateCamera = AZStd::make_shared<AzFramework::TranslateCameraInput>(AzFramework::LookTranslation);
 
             auto orbitCamera = AZStd::make_shared<AzFramework::OrbitCameraInput>();
             auto orbitRotateCamera = AZStd::make_shared<AzFramework::RotateCameraInput>(AzFramework::InputDeviceMouse::Button::Left);
@@ -46,37 +46,174 @@ namespace UnitTest
             orbitCamera->m_orbitCameras.AddCamera(orbitRotateCamera);
             orbitCamera->m_orbitCameras.AddCamera(orbitTranslateCamera);
 
-            m_cameraSystem->m_cameras.AddCamera(firstPersonRotateCamera);
-            m_cameraSystem->m_cameras.AddCamera(firstPersonTranslateCamera);
+            m_cameraSystem->m_cameras.AddCamera(m_firstPersonRotateCamera);
+            m_cameraSystem->m_cameras.AddCamera(m_firstPersonTranslateCamera);
             m_cameraSystem->m_cameras.AddCamera(orbitCamera);
         }
 
         void TearDown() override
         {
+            m_firstPersonRotateCamera.reset();
+            m_firstPersonTranslateCamera.reset();
+
             m_cameraSystem->m_cameras.Clear();
             m_cameraSystem.reset();
 
             AllocatorsTestFixture::TearDown();
         }
+
+        AZStd::shared_ptr<AzFramework::RotateCameraInput> m_firstPersonRotateCamera;
+        AZStd::shared_ptr<AzFramework::TranslateCameraInput> m_firstPersonTranslateCamera;
     };
 
-    TEST_F(CameraInputFixture, BeginEndOrbitCameraConsumesCorrectEvents)
+    TEST_F(CameraInputFixture, Begin_and_end_orbit_camera_consumes_correct_events)
     {
         // begin orbit camera
-        const bool consumed1 = HandleEventAndUpdate(
-            AzFramework::DiscreteInputEvent{AzFramework::InputDeviceKeyboard::Key::ModifierAltL, AzFramework::InputChannel::State::Began});
+        const bool consumed1 = HandleEventAndUpdate(AzFramework::DiscreteInputEvent{ AzFramework::InputDeviceKeyboard::Key::ModifierAltL,
+                                                                                     AzFramework::InputChannel::State::Began });
         // begin listening for orbit rotate (click detector) - event is not consumed
         const bool consumed2 = HandleEventAndUpdate(
-            AzFramework::DiscreteInputEvent{AzFramework::InputDeviceMouse::Button::Left, AzFramework::InputChannel::State::Began});
+            AzFramework::DiscreteInputEvent{ AzFramework::InputDeviceMouse::Button::Left, AzFramework::InputChannel::State::Began });
         // begin orbit rotate (mouse has moved sufficient distance to initiate)
-        const bool consumed3 = HandleEventAndUpdate(AzFramework::HorizontalMotionEvent{5});
+        const bool consumed3 = HandleEventAndUpdate(AzFramework::HorizontalMotionEvent{ 5 });
         // end orbit (mouse up) - event is not consumed
         const bool consumed4 = HandleEventAndUpdate(
-            AzFramework::DiscreteInputEvent{AzFramework::InputDeviceMouse::Button::Left, AzFramework::InputChannel::State::Ended});
+            AzFramework::DiscreteInputEvent{ AzFramework::InputDeviceMouse::Button::Left, AzFramework::InputChannel::State::Ended });
 
-        const auto allConsumed = AZStd::vector<bool>{consumed1, consumed2, consumed3, consumed4};
+        const auto allConsumed = AZStd::vector<bool>{ consumed1, consumed2, consumed3, consumed4 };
 
         using ::testing::ElementsAre;
         EXPECT_THAT(allConsumed, ElementsAre(true, false, true, false));
     }
+
+    TEST_F(CameraInputFixture, Begin_camera_input_notifies_activation_began_callback_for_translate_camera)
+    {
+        bool activationBegan = false;
+        m_firstPersonTranslateCamera->SetActivationBeganFn(
+            [&activationBegan]
+            {
+                activationBegan = true;
+            });
+
+        HandleEventAndUpdate(AzFramework::DiscreteInputEvent{ AzFramework::InputDeviceKeyboard::Key::AlphanumericW,
+                                                              AzFramework::InputChannel::State::Began });
+
+        EXPECT_TRUE(activationBegan);
+    }
+
+    TEST_F(CameraInputFixture, Begin_camera_input_notifies_activation_began_callback_after_delta_for_rotate_camera)
+    {
+        bool activationBegan = false;
+        m_firstPersonRotateCamera->SetActivationBeganFn(
+            [&activationBegan]
+            {
+                activationBegan = true;
+            });
+
+        HandleEventAndUpdate(
+            AzFramework::DiscreteInputEvent{ AzFramework::InputDeviceMouse::Button::Right, AzFramework::InputChannel::State::Began });
+        HandleEventAndUpdate(AzFramework::HorizontalMotionEvent{ 20 }); // must move input device
+
+        EXPECT_TRUE(activationBegan);
+    }
+
+    TEST_F(CameraInputFixture, Begin_camera_input_does_not_notify_activation_began_callback_with_no_delta_for_rotate_camera)
+    {
+        bool activationBegan = false;
+        m_firstPersonRotateCamera->SetActivationBeganFn(
+            [&activationBegan]
+            {
+                activationBegan = true;
+            });
+
+        HandleEventAndUpdate(
+            AzFramework::DiscreteInputEvent{ AzFramework::InputDeviceMouse::Button::Right, AzFramework::InputChannel::State::Began });
+
+        EXPECT_FALSE(activationBegan);
+    }
+
+    TEST_F(CameraInputFixture, End_camera_input_notifies_activation_end_callback_after_delta_for_rotate_camera)
+    {
+        bool activationEnded = false;
+        m_firstPersonRotateCamera->SetActivationEndedFn(
+            [&activationEnded]
+            {
+                activationEnded = true;
+            });
+
+        HandleEventAndUpdate(
+            AzFramework::DiscreteInputEvent{ AzFramework::InputDeviceMouse::Button::Right, AzFramework::InputChannel::State::Began });
+        HandleEventAndUpdate(AzFramework::HorizontalMotionEvent{ 20 });
+        HandleEventAndUpdate(
+            AzFramework::DiscreteInputEvent{ AzFramework::InputDeviceMouse::Button::Right, AzFramework::InputChannel::State::Ended });
+
+        EXPECT_TRUE(activationEnded);
+    }
+
+    TEST_F(CameraInputFixture, End_camera_input_does_not_notify_activation_began_or_end_callback_with_no_delta_for_rotate_camera)
+    {
+        bool activationBegan = false;
+        m_firstPersonRotateCamera->SetActivationBeganFn(
+            [&activationBegan]
+            {
+                activationBegan = true;
+            });
+
+        bool activationEnded = false;
+        m_firstPersonRotateCamera->SetActivationEndedFn(
+            [&activationEnded]
+            {
+                activationEnded = true;
+            });
+
+        HandleEventAndUpdate(
+            AzFramework::DiscreteInputEvent{ AzFramework::InputDeviceMouse::Button::Right, AzFramework::InputChannel::State::Began });
+        HandleEventAndUpdate(
+            AzFramework::DiscreteInputEvent{ AzFramework::InputDeviceMouse::Button::Right, AzFramework::InputChannel::State::Ended });
+
+        EXPECT_FALSE(activationBegan);
+        EXPECT_FALSE(activationEnded);
+    }
+
+    TEST_F(CameraInputFixture, End_camera_input_notifies_activation_began_or_end_callback_with_translate_camera)
+    {
+        bool activationBegan = false;
+        m_firstPersonTranslateCamera->SetActivationBeganFn(
+            [&activationBegan]
+            {
+                activationBegan = true;
+            });
+
+        bool activationEnded = false;
+        m_firstPersonTranslateCamera->SetActivationEndedFn(
+            [&activationEnded]
+            {
+                activationEnded = true;
+            });
+
+        HandleEventAndUpdate(AzFramework::DiscreteInputEvent{ AzFramework::InputDeviceKeyboard::Key::AlphanumericW,
+                                                              AzFramework::InputChannel::State::Began });
+        HandleEventAndUpdate(AzFramework::DiscreteInputEvent{ AzFramework::InputDeviceKeyboard::Key::AlphanumericW,
+                                                              AzFramework::InputChannel::State::Ended });
+
+        EXPECT_TRUE(activationBegan);
+        EXPECT_TRUE(activationEnded);
+    }
+
+    TEST_F(CameraInputFixture, End_activation_called_for_camera_input_if_active_when_cameras_are_cleared)
+    {
+        bool activationEnded = false;
+        m_firstPersonTranslateCamera->SetActivationEndedFn(
+            [&activationEnded]
+            {
+                activationEnded = true;
+            });
+
+        HandleEventAndUpdate(AzFramework::DiscreteInputEvent{ AzFramework::InputDeviceKeyboard::Key::AlphanumericW,
+                                                              AzFramework::InputChannel::State::Began });
+
+        m_cameraSystem->m_cameras.Clear();
+
+        EXPECT_TRUE(activationEnded);
+    }
 } // namespace UnitTest

Code/Sandbox/Editor/EditorViewportWidget.cpp

@@ -45,6 +45,7 @@
 #include <AzToolsFramework/Manipulators/ManipulatorManager.h>
 #include <AzToolsFramework/ViewportSelection/EditorInteractionSystemViewportSelectionRequestBus.h>
 #include <AzToolsFramework/ViewportSelection/EditorTransformComponentSelectionRequestBus.h>
+#include <AzToolsFramework/Viewport/ViewportMessages.h>
 
 // AtomToolsFramework
 #include <AtomToolsFramework/Viewport/RenderViewportWidget.h>
@@ -1244,11 +1245,24 @@ AZStd::shared_ptr<AtomToolsFramework::ModularViewportCameraController> CreateMod
     controller->SetCameraListBuilderCallback(
         [viewportId](AzFramework::Cameras& cameras)
         {
+            const auto hideCursor = [viewportId]
+            {
+                AzToolsFramework::ViewportInteraction::ViewportMouseCursorRequestBus::Event(
+                    viewportId, &AzToolsFramework::ViewportInteraction::ViewportMouseCursorRequestBus::Events::BeginCursorCapture);
+            };
+            const auto showCursor = [viewportId]
+            {
+                AzToolsFramework::ViewportInteraction::ViewportMouseCursorRequestBus::Event(
+                    viewportId, &AzToolsFramework::ViewportInteraction::ViewportMouseCursorRequestBus::Events::EndCursorCapture);
+            };
+
             auto firstPersonRotateCamera = AZStd::make_shared<AzFramework::RotateCameraInput>(AzFramework::CameraFreeLookButton);
             firstPersonRotateCamera->m_rotateSpeedFn = []
             {
                 return SandboxEditor::CameraRotateSpeed();
             };
+            firstPersonRotateCamera->SetActivationBeganFn(hideCursor);
+            firstPersonRotateCamera->SetActivationEndedFn(showCursor);
 
             auto firstPersonPanCamera =
                 AZStd::make_shared<AzFramework::PanCameraInput>(AzFramework::CameraFreePanButton, AzFramework::LookPan);