Updates to fix BoxSelect when using Orbit with the new Camera (#825)

* update camera controller to block box select during orbit

* simplify update for modern viewport camera controller

* wip working lmb box select with orbit

* add test for changes to click detector

* add unit test for camera system to validate events

* remove debugging code, tidy-up changes for PR

* small updates before posting PR

* fix for linux build failure

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

@@ -193,13 +193,12 @@ namespace AzFramework
         bool handling = false;
         for (auto& cameraInput : m_activeCameraInputs)
         {
-            cameraInput->HandleEvents(event, cursorDelta, scrollDelta);
-            handling = !cameraInput->Idle() || handling;
+            handling = cameraInput->HandleEvents(event, cursorDelta, scrollDelta) || handling;
         }
 
         for (auto& cameraInput : m_idleCameraInputs)
         {
-            cameraInput->HandleEvents(event, cursorDelta, scrollDelta);
+            handling = cameraInput->HandleEvents(event, cursorDelta, scrollDelta) || handling;
         }
 
         return handling;
@@ -262,17 +261,26 @@ namespace AzFramework
         {
             m_activeCameraInputs[i]->Reset();
             m_idleCameraInputs.push_back(m_activeCameraInputs[i]);
-            m_activeCameraInputs[i] = m_activeCameraInputs[m_activeCameraInputs.size() - 1];
+            using AZStd::swap;
+            swap(m_activeCameraInputs[i], m_activeCameraInputs[m_activeCameraInputs.size() - 1]);
             m_activeCameraInputs.pop_back();
         }
     }
 
+    void Cameras::Clear()
+    {
+        Reset();
+        AZ_Assert(m_activeCameraInputs.empty(), "Active Camera Inputs is not empty");
+
+        m_idleCameraInputs.clear();
+    }
+
     RotateCameraInput::RotateCameraInput(const InputChannelId rotateChannelId)
         : m_rotateChannelId(rotateChannelId)
     {
     }
 
-    void RotateCameraInput::HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, [[maybe_unused]] float scrollDelta)
+    bool RotateCameraInput::HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, [[maybe_unused]] float scrollDelta)
     {
         const ClickDetector::ClickEvent clickEvent = [&event, this] {
             if (const auto& input = AZStd::get_if<DiscreteInputEvent>(&event))
@@ -304,6 +312,11 @@ namespace AzFramework
             // noop
             break;
         }
+
+        // note - must also check !ending to ensure the mouse up (release) event
+        // is not consumed and can be propagated to other systems.
+        // (don't swallow mouse up events)
+        return !Idle() && !Ending();
     }
 
     Camera RotateCameraInput::StepCamera(
@@ -330,7 +343,7 @@ namespace AzFramework
     {
     }
 
-    void PanCameraInput::HandleEvents(
+    bool PanCameraInput::HandleEvents(
         const InputEvent& event, [[maybe_unused]] const ScreenVector& cursorDelta, [[maybe_unused]] float scrollDelta)
     {
         if (const auto& input = AZStd::get_if<DiscreteInputEvent>(&event))
@@ -347,6 +360,8 @@ namespace AzFramework
                 }
             }
         }
+
+        return !Idle();
     }
 
     Camera PanCameraInput::StepCamera(
@@ -411,7 +426,7 @@ namespace AzFramework
     {
     }
 
-    void TranslateCameraInput::HandleEvents(
+    bool TranslateCameraInput::HandleEvents(
         const InputEvent& event, [[maybe_unused]] const ScreenVector& cursorDelta, [[maybe_unused]] float scrollDelta)
     {
         if (const auto& input = AZStd::get_if<DiscreteInputEvent>(&event))
@@ -429,7 +444,8 @@ namespace AzFramework
                     m_boost = true;
                 }
             }
-            else if (input->m_state == InputChannel::State::Ended)
+            // ensure we don't process end events in the idle state
+            else if (input->m_state == InputChannel::State::Ended && !Idle())
             {
                 m_translation &= ~(translationFromKey(input->m_channelId));
                 if (m_translation == TranslationType::Nil)
@@ -442,6 +458,8 @@ namespace AzFramework
                 }
             }
         }
+
+        return !Idle();
     }
 
     Camera TranslateCameraInput::StepCamera(
@@ -503,7 +521,7 @@ namespace AzFramework
         m_boost = false;
     }
 
-    void OrbitCameraInput::HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, float scrollDelta)
+    bool OrbitCameraInput::HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, float scrollDelta)
     {
         if (const auto* input = AZStd::get_if<DiscreteInputEvent>(&event))
         {
@@ -522,8 +540,10 @@ namespace AzFramework
 
         if (Active())
         {
-            m_orbitCameras.HandleEvents(event, cursorDelta, scrollDelta);
+            return m_orbitCameras.HandleEvents(event, cursorDelta, scrollDelta);
         }
+
+        return !Idle();
     }
 
     Camera OrbitCameraInput::StepCamera(
@@ -533,7 +553,7 @@ namespace AzFramework
 
         if (Beginning())
         {
-            const auto hasLookAt = [&nextCamera, &targetCamera, lookAtFn = m_lookAtFn] {
+            const auto hasLookAt = [&nextCamera, &targetCamera, &lookAtFn = m_lookAtFn] {
                 if (lookAtFn)
                 {
                     if (const auto lookAt = lookAtFn())
@@ -585,13 +605,15 @@ namespace AzFramework
         return nextCamera;
     }
 
-    void OrbitDollyScrollCameraInput::HandleEvents(
+    bool OrbitDollyScrollCameraInput::HandleEvents(
         const InputEvent& event, [[maybe_unused]] const ScreenVector& cursorDelta, [[maybe_unused]] float scrollDelta)
     {
         if (const auto* scroll = AZStd::get_if<ScrollEvent>(&event))
         {
             BeginActivation();
         }
+
+        return !Idle();
     }
 
     Camera OrbitDollyScrollCameraInput::StepCamera(
@@ -609,7 +631,7 @@ namespace AzFramework
     {
     }
 
-    void OrbitDollyCursorMoveCameraInput::HandleEvents(
+    bool OrbitDollyCursorMoveCameraInput::HandleEvents(
         const InputEvent& event, [[maybe_unused]] const ScreenVector& cursorDelta, [[maybe_unused]] float scrollDelta)
     {
         if (const auto& input = AZStd::get_if<DiscreteInputEvent>(&event))
@@ -626,6 +648,8 @@ namespace AzFramework
                 }
             }
         }
+
+        return !Idle();
     }
 
     Camera OrbitDollyCursorMoveCameraInput::StepCamera(
@@ -637,13 +661,15 @@ namespace AzFramework
         return nextCamera;
     }
 
-    void ScrollTranslationCameraInput::HandleEvents(
+    bool ScrollTranslationCameraInput::HandleEvents(
         const InputEvent& event, [[maybe_unused]] const ScreenVector& cursorDelta, [[maybe_unused]] float scrollDelta)
     {
         if (const auto* scroll = AZStd::get_if<ScrollEvent>(&event))
         {
             BeginActivation();
         }
+
+        return !Idle();
     }
 
     Camera ScrollTranslationCameraInput::StepCamera(

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

@@ -149,7 +149,7 @@ namespace AzFramework
             ResetImpl();
         }
 
-        virtual void HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, float scrollDelta) = 0;
+        virtual bool HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, float scrollDelta) = 0;
         virtual Camera StepCamera(const Camera& targetCamera, const ScreenVector& cursorDelta, float scrollDelta, float deltaTime) = 0;
 
         virtual bool Exclusive() const
@@ -171,16 +171,29 @@ namespace AzFramework
     class Cameras
     {
     public:
-        void AddCamera(AZStd::shared_ptr<CameraInput> cameraInput);
         bool HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, float scrollDelta);
         Camera StepCamera(const Camera& targetCamera, const ScreenVector& cursorDelta, float scrollDelta, float deltaTime);
+
+        void AddCamera(AZStd::shared_ptr<CameraInput> cameraInput);
+        //! Reset the state of all cameras.
         void Reset();
+        //! Remove all cameras that were added.
+        void Clear();
+        //! Is one of the cameras in the active camera inputs marked as 'exclusive'.
+        //! @note This implies no other sibling cameras can begin while the exclusive camera is running.
+        bool Exclusive() const;
 
     private:
         AZStd::vector<AZStd::shared_ptr<CameraInput>> m_activeCameraInputs;
         AZStd::vector<AZStd::shared_ptr<CameraInput>> m_idleCameraInputs;
     };
 
+    inline bool Cameras::Exclusive() const
+    {
+        return AZStd::any_of(
+            m_activeCameraInputs.begin(), m_activeCameraInputs.end(), [](const auto& cameraInput) { return cameraInput->Exclusive(); });
+    }
+
     class CameraSystem
     {
     public:
@@ -200,7 +213,7 @@ namespace AzFramework
         explicit RotateCameraInput(InputChannelId rotateChannelId);
 
         // CameraInput overrides ...
-        void HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, float scrollDelta) override;
+        bool HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, float scrollDelta) override;
         Camera StepCamera(const Camera& targetCamera, const ScreenVector& cursorDelta, float scrollDelta, float deltaTime) override;
 
     private:
@@ -241,7 +254,7 @@ namespace AzFramework
         PanCameraInput(InputChannelId panChannelId, PanAxesFn panAxesFn);
 
         // CameraInput overrides ...
-        void HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, float scrollDelta) override;
+        bool HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, float scrollDelta) override;
         Camera StepCamera(const Camera& targetCamera, const ScreenVector& cursorDelta, float scrollDelta, float deltaTime) override;
 
     private:
@@ -282,7 +295,7 @@ namespace AzFramework
         explicit TranslateCameraInput(TranslationAxesFn translationAxesFn);
 
         // CameraInput overrides ...
-        void HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, float scrollDelta) override;
+        bool HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, float scrollDelta) override;
         Camera StepCamera(const Camera& targetCamera, const ScreenVector& cursorDelta, float scrollDelta, float deltaTime) override;
         void ResetImpl() override;
 
@@ -352,7 +365,7 @@ namespace AzFramework
     {
     public:
         // CameraInput overrides ...
-        void HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, float scrollDelta) override;
+        bool HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, float scrollDelta) override;
         Camera StepCamera(const Camera& targetCamera, const ScreenVector& cursorDelta, float scrollDelta, float deltaTime) override;
     };
 
@@ -362,7 +375,7 @@ namespace AzFramework
         explicit OrbitDollyCursorMoveCameraInput(InputChannelId dollyChannelId);
 
         // CameraInput overrides ...
-        void HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, float scrollDelta) override;
+        bool HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, float scrollDelta) override;
         Camera StepCamera(const Camera& targetCamera, const ScreenVector& cursorDelta, float scrollDelta, float deltaTime) override;
 
     private:
@@ -373,7 +386,7 @@ namespace AzFramework
     {
     public:
         // CameraInput overrides ...
-        void HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, float scrollDelta) override;
+        bool HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, float scrollDelta) override;
         Camera StepCamera(const Camera& targetCamera, const ScreenVector& cursorDelta, float scrollDelta, float deltaTime) override;
     };
 
@@ -383,7 +396,7 @@ namespace AzFramework
         using LookAtFn = AZStd::function<AZStd::optional<AZ::Vector3>()>;
 
         // CameraInput overrides ...
-        void HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, float scrollDelta) override;
+        bool HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, float scrollDelta) override;
         Camera StepCamera(const Camera& targetCamera, const ScreenVector& cursorDelta, float scrollDelta, float deltaTime) override;
         bool Exclusive() const override;
 

Code/Framework/AzFramework/AzFramework/Viewport/ClickDetector.cpp

@@ -17,6 +17,17 @@ namespace AzFramework
 {
     ClickDetector::ClickOutcome ClickDetector::DetectClick(const ClickEvent clickEvent, const ScreenVector& cursorDelta)
     {
+        const auto previousDetectionState = m_detectionState;
+        if (previousDetectionState == DetectionState::WaitingForMove)
+        {
+            // only allow the action to begin if the mouse has been moved a small amount
+            m_moveAccumulator += ScreenVectorLength(cursorDelta);
+            if (m_moveAccumulator > m_deadZone)
+            {
+                m_detectionState = DetectionState::Moved;
+            }
+        }
+
         if (clickEvent == ClickEvent::Down)
         {
             const auto now = std::chrono::steady_clock::now();
@@ -52,15 +63,9 @@ namespace AzFramework
             return clickOutcome;
         }
 
-        if (m_detectionState == DetectionState::WaitingForMove)
+        if (previousDetectionState == DetectionState::WaitingForMove && m_detectionState == DetectionState::Moved)
         {
-            // only allow the action to begin if the mouse has been moved a small amount
-            m_moveAccumulator += ScreenVectorLength(cursorDelta);
-            if (m_moveAccumulator > m_deadZone)
-            {
-                m_detectionState = DetectionState::Moved;
-                return ClickOutcome::Move;
-            }
+            return ClickOutcome::Move;
         }
 
         return ClickOutcome::Nil;

Code/Framework/AzFramework/AzFramework/Viewport/ClickDetector.h

@@ -50,7 +50,11 @@ namespace AzFramework
         //! Called from any type of 'handle event' function.
         ClickOutcome DetectClick(ClickEvent clickEvent, const ScreenVector& cursorDelta);
 
+        //! Override the default double click interval.
+        //! @note Default is 400ms - system default.
         void SetDoubleClickInterval(float doubleClickInterval);
+        //! Override the dead zone before a 'move' outcome will be triggered.
+        void SetDeadZone(float deadZone);
 
     private:
         //! Internal state of ClickDetector based on incoming events.
@@ -72,4 +76,9 @@ namespace AzFramework
     {
         m_doubleClickInterval = doubleClickInterval;
     }
+
+    inline void ClickDetector::SetDeadZone(const float deadZone)
+    {
+        m_deadZone = deadZone;
+    }
 } // namespace AzFramework

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

@@ -18,6 +18,7 @@
 #include <AzFramework/Entity/EntityDebugDisplayBus.h>
 #include <AzFramework/Viewport/CameraState.h>
 #include <AzFramework/Viewport/ViewportId.h>
+#include <AzFramework/Viewport/ClickDetector.h>
 #include <AzToolsFramework/Entity/EditorEntityContextBus.h>
 #include <AzToolsFramework/Viewport/ViewportTypes.h>
 
@@ -304,4 +305,24 @@ namespace AzToolsFramework
 
         return entityContextId;
     }
+
+    //! Maps a mouse interaction event to a ClickDetector event.
+    //! @note Function only cares about up or down events, all other events are mapped to Nil (ignored).
+    inline AzFramework::ClickDetector::ClickEvent ClickDetectorEventFromViewportInteraction(
+        const ViewportInteraction::MouseInteractionEvent& mouseInteraction)
+    {
+        if (mouseInteraction.m_mouseInteraction.m_mouseButtons.Left())
+        {
+            if (mouseInteraction.m_mouseEvent == ViewportInteraction::MouseEvent::Down)
+            {
+                return AzFramework::ClickDetector::ClickEvent::Down;
+            }
+
+            if (mouseInteraction.m_mouseEvent == ViewportInteraction::MouseEvent::Up)
+            {
+                return AzFramework::ClickDetector::ClickEvent::Up;
+            }
+        }
+        return AzFramework::ClickDetector::ClickEvent::Nil;
+    }
 } // namespace AzToolsFramework

Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorBoxSelect.cpp

@@ -14,6 +14,7 @@
 
 #include <AzFramework/Entity/EntityDebugDisplayBus.h>
 #include <AzToolsFramework/ViewportSelection/EditorSelectionUtil.h>
+#include <AzToolsFramework/Viewport/ViewportMessages.h>
 
 #include <QApplication>
 
@@ -27,8 +28,11 @@ namespace AzToolsFramework
     {
         AZ_PROFILE_FUNCTION(AZ::Debug::ProfileCategory::AzToolsFramework);
 
-        if (mouseInteraction.m_mouseInteraction.m_mouseButtons.Left() &&
-            mouseInteraction.m_mouseEvent == ViewportInteraction::MouseEvent::Down)
+        m_cursorState.SetCurrentPosition(mouseInteraction.m_mouseInteraction.m_mousePick.m_screenCoordinates);
+
+        const auto selectClickEvent = ClickDetectorEventFromViewportInteraction(mouseInteraction);
+        const auto clickOutcome = m_clickDetector.DetectClick(selectClickEvent, m_cursorState.CursorDelta());
+        if (clickOutcome == AzFramework::ClickDetector::ClickOutcome::Move)
         {
             if (m_leftMouseDown)
             {
@@ -58,8 +62,7 @@ namespace AzToolsFramework
                 }
             }
 
-            if (mouseInteraction.m_mouseInteraction.m_mouseButtons.Left() &&
-                mouseInteraction.m_mouseEvent == ViewportInteraction::MouseEvent::Up)
+            if (clickOutcome == AzFramework::ClickDetector::ClickOutcome::Release)
             {
                 if (m_leftMouseUp)
                 {
@@ -77,6 +80,8 @@ namespace AzToolsFramework
     {
         AZ_PROFILE_FUNCTION(AZ::Debug::ProfileCategory::AzToolsFramework);
 
+        m_cursorState.Update();
+
         if (m_boxSelectRegion)
         {
             debugDisplay.DepthTestOff();

Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorBoxSelect.h

@@ -14,6 +14,8 @@
 
 #include <AzCore/std/functional.h>
 #include <AzCore/std/optional.h>
+#include <AzFramework/Viewport/ClickDetector.h>
+#include <AzFramework/Viewport/CursorState.h>
 #include <AzToolsFramework/Viewport/ViewportTypes.h>
 
 #include <QRect>
@@ -26,49 +28,49 @@ namespace AzFramework
 
 namespace AzToolsFramework
 {
-    /// Utility to provide box select (click and drag) support for viewport types.
-    /// Users can override the mouse event callbacks and display scene function to customize behavior.
+    //! Utility to provide box select (click and drag) support for viewport types.
+    //! Users can override the mouse event callbacks and display scene function to customize behavior.
     class EditorBoxSelect
     {
     public:
         EditorBoxSelect() = default;
 
-        /// Return if a box select action is currently taking place.
+        //! Return if a box select action is currently taking place.
         bool Active() const { return m_boxSelectRegion.has_value(); }
 
-        /// Update the box select for various mouse events.
-        /// Call HandleMouseInteraction from type/system implementing MouseViewportRequests interface.
+        //! Update the box select for various mouse events.
+        //! Call HandleMouseInteraction from type/system implementing MouseViewportRequests interface.
         void HandleMouseInteraction(
             const ViewportInteraction::MouseInteractionEvent& mouseInteraction);
 
-        /// Responsible for drawing the 2d box representing the selection in screen space.
+        //! Responsible for drawing the 2d box representing the selection in screen space.
         void Display2d(const AzFramework::ViewportInfo& viewportInfo, AzFramework::DebugDisplayRequests& debugDisplay);
 
-        /// Custom drawing behavior to happen during a box select.
+        //! Custom drawing behavior to happen during a box select.
         void DisplayScene(
             const AzFramework::ViewportInfo& viewportInfo, AzFramework::DebugDisplayRequests& debugDisplay);
 
-        /// Set the left mouse down callback.
+        //! Set the left mouse down callback.
         void InstallLeftMouseDown(
             const AZStd::function<void(const ViewportInteraction::MouseInteractionEvent& mouseInteraction)>& leftMouseDown);
-        /// Set the mouse move callback.
+        //! Set the mouse move callback.
         void InstallMouseMove(
             const AZStd::function<void(const ViewportInteraction::MouseInteractionEvent& mouseInteraction)>& mouseMove);
-        /// Set the left mouse up callback.
+        //! Set the left mouse up callback.
         void InstallLeftMouseUp(
             const AZStd::function<void()>& leftMouseUp);
-        /// Set the display scene callback.
+        //! Set the display scene callback.
         void InstallDisplayScene(
             const AZStd::function<void(
                 const AzFramework::ViewportInfo& viewportInfo,
                 AzFramework::DebugDisplayRequests& debugDisplay)>& displayScene);
 
-        /// Return the box select region.
-        /// If a box selection is being made, return the current rectangle representing the area.
-        /// If there is currently no active box select, then the Maybe type will be empty (there will be no region/area).
+        //! Return the box select region.
+        //! If a box selection is being made, return the current rectangle representing the area.
+        //! If there is currently no active box select, then the Maybe type will be empty (there will be no region/area).
         const AZStd::optional<QRect>& BoxRegion() const { return m_boxSelectRegion; }
 
-        /// Return the active modifiers from the previous frame.
+        //! Return the active modifiers from the previous frame.
         ViewportInteraction::KeyboardModifiers PreviousModifiers() const { return m_previousModifiers; }
 
     private:
@@ -79,7 +81,9 @@ namespace AzToolsFramework
         AZStd::function<void(
             const AzFramework::ViewportInfo& viewportInfo, AzFramework::DebugDisplayRequests& debugDisplay)> m_displayScene;
 
-        AZStd::optional<QRect> m_boxSelectRegion; ///< Maybe/optional value to store box select region while active.
-        ViewportInteraction::KeyboardModifiers m_previousModifiers; ///< Modifier keys active on the previous frame.
+        AZStd::optional<QRect> m_boxSelectRegion; //!< Maybe/optional value to store box select region while active.
+        ViewportInteraction::KeyboardModifiers m_previousModifiers; //!< Modifier keys active on the previous frame.
+        AzFramework::ClickDetector m_clickDetector; //!< Utility type to detect if a mouse click or move has occurred.
+        AzFramework::CursorState m_cursorState; //!< Utility type to track the current cursor position (and movement/delta).
     };
 } // namespace AzToolsFramework

Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorTransformComponentSelection.cpp

@@ -1782,22 +1782,7 @@ namespace AzToolsFramework
 
         m_cachedEntityIdUnderCursor = m_editorHelpers->HandleMouseInteraction(cameraState, mouseInteraction);
 
-        const AzFramework::ClickDetector::ClickEvent selectClickEvent = [&mouseInteraction] {
-            if (mouseInteraction.m_mouseInteraction.m_mouseButtons.Left())
-            {
-                if (mouseInteraction.m_mouseEvent == ViewportInteraction::MouseEvent::Down)
-                {
-                    return AzFramework::ClickDetector::ClickEvent::Down;
-                }
-
-                if (mouseInteraction.m_mouseEvent == ViewportInteraction::MouseEvent::Up)
-                {
-                    return AzFramework::ClickDetector::ClickEvent::Up;
-                }
-            }
-            return AzFramework::ClickDetector::ClickEvent::Nil;
-        }();
-
+        const auto selectClickEvent = ClickDetectorEventFromViewportInteraction(mouseInteraction);
         m_cursorState.SetCurrentPosition(mouseInteraction.m_mouseInteraction.m_mousePick.m_screenCoordinates);
         const auto clickOutcome = m_clickDetector.DetectClick(selectClickEvent, m_cursorState.CursorDelta());
 

Code/Framework/Tests/CameraInputTests.cpp

@@ -0,0 +1,90 @@
+/*
+ * All or portions of this file Copyright (c) Amazon.com, Inc. or its affiliates or
+ * its licensors.
+ *
+ * For complete copyright and license terms please see the LICENSE at the root of this
+ * distribution (the "License"). All use of this software is governed by the License,
+ * or, if provided, by the license below or the license accompanying this file. Do not
+ * remove or modify any license notices. This file is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *
+ */
+
+#include <AzCore/UnitTest/TestTypes.h>
+#include <AzCore/std/smart_ptr/make_shared.h>
+#include <AzFramework/Input/Devices/Keyboard/InputDeviceKeyboard.h>
+#include <AzFramework/Input/Devices/Mouse/InputDeviceMouse.h>
+#include <AzFramework/Viewport/CameraInput.h>
+#include <AzFramework/Windowing/WindowBus.h>
+
+namespace UnitTest
+{
+    class CameraInputFixture : public AllocatorsTestFixture
+    {
+    public:
+        AzFramework::Camera m_camera;
+        AzFramework::Camera m_targetCamera;
+        AZStd::shared_ptr<AzFramework::CameraSystem> m_cameraSystem;
+
+        bool HandleEventAndUpdate(const AzFramework::InputEvent& event)
+        {
+            constexpr float deltaTime = 0.01666f; // 60fps
+            const bool consumed = m_cameraSystem->HandleEvents(event);
+            m_camera = m_cameraSystem->StepCamera(m_targetCamera, deltaTime);
+            return consumed;
+        }
+
+        void SetUp() override
+        {
+            AllocatorsTestFixture::SetUp();
+
+            AzFramework::ReloadCameraKeyBindings();
+
+            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);
+
+            auto orbitCamera = AZStd::make_shared<AzFramework::OrbitCameraInput>();
+            auto orbitRotateCamera = AZStd::make_shared<AzFramework::RotateCameraInput>(AzFramework::InputDeviceMouse::Button::Left);
+            auto orbitTranslateCamera = AZStd::make_shared<AzFramework::TranslateCameraInput>(AzFramework::OrbitTranslation);
+
+            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(orbitCamera);
+        }
+
+        void TearDown() override
+        {
+            m_cameraSystem->m_cameras.Clear();
+            m_cameraSystem.reset();
+
+            AllocatorsTestFixture::TearDown();
+        }
+    };
+
+    TEST_F(CameraInputFixture, BeginEndOrbitCameraConsumesCorrectEvents)
+    {
+        // set initial mouse position
+        const bool consumed1 = HandleEventAndUpdate(AzFramework::CursorEvent{AzFramework::ScreenPoint(5, 5)});
+        // begin orbit camera
+        const bool consumed2 = HandleEventAndUpdate(
+            AzFramework::DiscreteInputEvent{AzFramework::InputDeviceKeyboard::Key::ModifierAltL, AzFramework::InputChannel::State::Began});
+        // begin listening for orbit rotate (click detector) - event is not consumed
+        const bool consumed3 = HandleEventAndUpdate(
+            AzFramework::DiscreteInputEvent{AzFramework::InputDeviceMouse::Button::Left, AzFramework::InputChannel::State::Began});
+        // begin orbit rotate (mouse has moved sufficient distance to initiate)
+        const bool consumed4 = HandleEventAndUpdate(AzFramework::CursorEvent{AzFramework::ScreenPoint(10, 10)});
+        // end orbit (mouse up) - event is not consumed
+        const bool consumed5 = HandleEventAndUpdate(
+            AzFramework::DiscreteInputEvent{AzFramework::InputDeviceMouse::Button::Left, AzFramework::InputChannel::State::Ended});
+
+        const auto allConsumed = AZStd::vector<bool>{consumed1, consumed2, consumed3, consumed4, consumed5};
+
+        using ::testing::ElementsAre;
+        EXPECT_THAT(allConsumed, ElementsAre(false, true, false, true, false));
+    }
+} // namespace UnitTest

Code/Framework/Tests/ClickDetectorTests.cpp

@@ -139,4 +139,21 @@ namespace UnitTest
         EXPECT_THAT(secondaryDownOutcome, Eq(ClickDetector::ClickOutcome::Nil)); // ignored double click
         EXPECT_THAT(secondaryUpOutcome, Eq(ClickDetector::ClickOutcome::Nil)); // click not registered
     }
+
+    // if the click detector registers a mouse down event, but then all intermediate calls are ignored
+    // (another system may start intercepting events and swallowing them) then when we do receive a mouse
+    // up event we should ensure we take into account the current delta - if the delta is large, then the
+    // outcome will be release
+    TEST_F(ClickDetectorFixture, ClickIsNotRegisteredAfterIgnoringMouseMovesBeforeMouseUpWithLargeDelta)
+    {
+        using ::testing::Eq;
+
+        const ClickDetector::ClickOutcome downOutcome =
+            m_clickDetector.DetectClick(ClickDetector::ClickEvent::Down, ScreenVector(0, 0));
+        const ClickDetector::ClickOutcome upOutcome =
+            m_clickDetector.DetectClick(ClickDetector::ClickEvent::Up, ScreenVector(50, 50));
+
+        EXPECT_THAT(downOutcome, Eq(ClickDetector::ClickOutcome::Nil));
+        EXPECT_THAT(upOutcome, Eq(ClickDetector::ClickOutcome::Release));
+    }
 } // namespace UnitTest

Code/Framework/Tests/frameworktests_files.cmake

@@ -17,6 +17,7 @@ set(FILES
     BinToTextEncode.cpp
     ComponentAddRemove.cpp
     ComponentAdapterTests.cpp
+    CameraInputTests.cpp
     ClickDetectorTests.cpp
     CursorStateTests.cpp
     EntityContext.cpp

Code/Sandbox/Editor/ModernViewportCameraController.cpp

@@ -97,17 +97,38 @@ namespace SandboxEditor
         AzFramework::ViewportDebugDisplayEventBus::Handler::BusDisconnect();
     }
 
+    // should the camera system respond to this particular event
+    static bool ShouldHandle(const AzFramework::ViewportControllerPriority priority, const bool exclusive)
+    {
+        // ModernViewportCameraControllerInstance receives events at all priorities, it should only respond
+        // to normal priority events if it is not in 'exclusive' mode and when in 'exclusive' mode it should
+        // only respond to the highest priority events
+        return !exclusive && priority == AzFramework::ViewportControllerPriority::Normal ||
+            exclusive && priority == AzFramework::ViewportControllerPriority::Highest;
+    }
+
     bool ModernViewportCameraControllerInstance::HandleInputChannelEvent(const AzFramework::ViewportControllerInputEvent& event)
     {
         AzFramework::WindowSize windowSize;
         AzFramework::WindowRequestBus::EventResult(
             windowSize, event.m_windowHandle, &AzFramework::WindowRequestBus::Events::GetClientAreaSize);
 
-        return m_cameraSystem.HandleEvents(AzFramework::BuildInputEvent(event.m_inputChannel, windowSize));
+        if (ShouldHandle(event.m_priority, m_cameraSystem.m_cameras.Exclusive()))
+        {
+            return m_cameraSystem.HandleEvents(AzFramework::BuildInputEvent(event.m_inputChannel, windowSize));
+        }
+
+        return false;
     }
 
     void ModernViewportCameraControllerInstance::UpdateViewport(const AzFramework::ViewportControllerUpdateEvent& event)
     {
+        // only update for a single priority (normal is the default)
+        if (event.m_priority != AzFramework::ViewportControllerPriority::Normal)
+        {
+            return;
+        }
+
         if (auto viewportContext = RetrieveViewportContext(GetViewportId()))
         {
             m_updatingTransform = true;

Code/Sandbox/Editor/ModernViewportCameraController.h

@@ -22,7 +22,9 @@
 namespace SandboxEditor
 {
     class ModernViewportCameraControllerInstance;
-    class ModernViewportCameraController : public AzFramework::MultiViewportController<ModernViewportCameraControllerInstance>
+    class ModernViewportCameraController
+        : public AzFramework::MultiViewportController<
+              ModernViewportCameraControllerInstance, AzFramework::ViewportControllerPriority::DispatchToAllPriorities>
     {
     public:
         using CameraListBuilder = AZStd::function<void(AzFramework::Cameras&)>;