From eb31d90ad94da7cca7a13b8e1385f1edc4bc42b4 Mon Sep 17 00:00:00 2001 From: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com> Date: Thu, 20 May 2021 15:54:36 +0100 Subject: [PATCH] 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 --- .../AzFramework/Viewport/CameraInput.cpp | 54 ++++++++--- .../AzFramework/Viewport/CameraInput.h | 31 +++++-- .../AzFramework/Viewport/ClickDetector.cpp | 21 +++-- .../AzFramework/Viewport/ClickDetector.h | 9 ++ .../Viewport/ViewportMessages.h | 21 +++++ .../ViewportSelection/EditorBoxSelect.cpp | 13 ++- .../ViewportSelection/EditorBoxSelect.h | 38 ++++---- .../EditorTransformComponentSelection.cpp | 17 +--- Code/Framework/Tests/CameraInputTests.cpp | 90 +++++++++++++++++++ Code/Framework/Tests/ClickDetectorTests.cpp | 17 ++++ .../Tests/frameworktests_files.cmake | 1 + .../Editor/ModernViewportCameraController.cpp | 23 ++++- .../Editor/ModernViewportCameraController.h | 4 +- 13 files changed, 269 insertions(+), 70 deletions(-) create mode 100644 Code/Framework/Tests/CameraInputTests.cpp diff --git a/Code/Framework/AzFramework/AzFramework/Viewport/CameraInput.cpp b/Code/Framework/AzFramework/AzFramework/Viewport/CameraInput.cpp index 79c1a28e5d..bd826544a1 100644 --- a/Code/Framework/AzFramework/AzFramework/Viewport/CameraInput.cpp +++ b/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(&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(&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(&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(&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(&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(&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(&event)) { BeginActivation(); } + + return !Idle(); } Camera ScrollTranslationCameraInput::StepCamera( diff --git a/Code/Framework/AzFramework/AzFramework/Viewport/CameraInput.h b/Code/Framework/AzFramework/AzFramework/Viewport/CameraInput.h index b6b2bc1e6a..582fb5a6de 100644 --- a/Code/Framework/AzFramework/AzFramework/Viewport/CameraInput.h +++ b/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); 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); + //! 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> m_activeCameraInputs; AZStd::vector> 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()>; // 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; diff --git a/Code/Framework/AzFramework/AzFramework/Viewport/ClickDetector.cpp b/Code/Framework/AzFramework/AzFramework/Viewport/ClickDetector.cpp index 4b8fbca36a..c276463554 100644 --- a/Code/Framework/AzFramework/AzFramework/Viewport/ClickDetector.cpp +++ b/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; diff --git a/Code/Framework/AzFramework/AzFramework/Viewport/ClickDetector.h b/Code/Framework/AzFramework/AzFramework/Viewport/ClickDetector.h index 997ccd07d9..a595735d28 100644 --- a/Code/Framework/AzFramework/AzFramework/Viewport/ClickDetector.h +++ b/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 diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Viewport/ViewportMessages.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Viewport/ViewportMessages.h index ee95412376..8e91dc945d 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Viewport/ViewportMessages.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Viewport/ViewportMessages.h @@ -18,6 +18,7 @@ #include #include #include +#include #include #include @@ -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 diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorBoxSelect.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorBoxSelect.cpp index 2e467caa4c..531cffb561 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorBoxSelect.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorBoxSelect.cpp @@ -14,6 +14,7 @@ #include #include +#include #include @@ -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(); diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorBoxSelect.h b/Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorBoxSelect.h index 7f50b16325..c115220755 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorBoxSelect.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorBoxSelect.h @@ -14,6 +14,8 @@ #include #include +#include +#include #include #include @@ -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& leftMouseDown); - /// Set the mouse move callback. + //! Set the mouse move callback. void InstallMouseMove( const AZStd::function& mouseMove); - /// Set the left mouse up callback. + //! Set the left mouse up callback. void InstallLeftMouseUp( const AZStd::function& leftMouseUp); - /// Set the display scene callback. + //! Set the display scene callback. void InstallDisplayScene( const AZStd::function& 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& 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 m_displayScene; - AZStd::optional 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 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 diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorTransformComponentSelection.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorTransformComponentSelection.cpp index 6e49f7c601..91644dc6ac 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorTransformComponentSelection.cpp +++ b/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()); diff --git a/Code/Framework/Tests/CameraInputTests.cpp b/Code/Framework/Tests/CameraInputTests.cpp new file mode 100644 index 0000000000..6fe9837c22 --- /dev/null +++ b/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 +#include +#include +#include +#include +#include + +namespace UnitTest +{ + class CameraInputFixture : public AllocatorsTestFixture + { + public: + AzFramework::Camera m_camera; + AzFramework::Camera m_targetCamera; + AZStd::shared_ptr 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(); + + auto firstPersonRotateCamera = AZStd::make_shared(AzFramework::InputDeviceMouse::Button::Right); + auto firstPersonTranslateCamera = AZStd::make_shared(AzFramework::LookTranslation); + + auto orbitCamera = AZStd::make_shared(); + auto orbitRotateCamera = AZStd::make_shared(AzFramework::InputDeviceMouse::Button::Left); + auto orbitTranslateCamera = AZStd::make_shared(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{consumed1, consumed2, consumed3, consumed4, consumed5}; + + using ::testing::ElementsAre; + EXPECT_THAT(allConsumed, ElementsAre(false, true, false, true, false)); + } +} // namespace UnitTest diff --git a/Code/Framework/Tests/ClickDetectorTests.cpp b/Code/Framework/Tests/ClickDetectorTests.cpp index 7e6f9634c8..64f06ee66c 100644 --- a/Code/Framework/Tests/ClickDetectorTests.cpp +++ b/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 diff --git a/Code/Framework/Tests/frameworktests_files.cmake b/Code/Framework/Tests/frameworktests_files.cmake index e249cf6e64..197bcc9fce 100644 --- a/Code/Framework/Tests/frameworktests_files.cmake +++ b/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 diff --git a/Code/Sandbox/Editor/ModernViewportCameraController.cpp b/Code/Sandbox/Editor/ModernViewportCameraController.cpp index af161af493..83ab2ef0b5 100644 --- a/Code/Sandbox/Editor/ModernViewportCameraController.cpp +++ b/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; diff --git a/Code/Sandbox/Editor/ModernViewportCameraController.h b/Code/Sandbox/Editor/ModernViewportCameraController.h index 066c8efaa8..39e3c9cbb3 100644 --- a/Code/Sandbox/Editor/ModernViewportCameraController.h +++ b/Code/Sandbox/Editor/ModernViewportCameraController.h @@ -22,7 +22,9 @@ namespace SandboxEditor { class ModernViewportCameraControllerInstance; - class ModernViewportCameraController : public AzFramework::MultiViewportController + class ModernViewportCameraController + : public AzFramework::MultiViewportController< + ModernViewportCameraControllerInstance, AzFramework::ViewportControllerPriority::DispatchToAllPriorities> { public: using CameraListBuilder = AZStd::function;