Improve selection in the viewport (#720)

* improve selection in the viewport * remove debug code * updates following review feedback - update API comments from /// to //! from - add [[nodiscard]] attribute to member function - move constructor implementations to .cpp files * use lambda instead of ternary operator * fix unit test failure caused by typo
2021-05-13 16:19:53 +01:00
parent 4aff32e719
commit 795aa114e6
11 changed files with 573 additions and 143 deletions
@@ -15,6 +15,7 @@
 #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>
@@ -156,35 +157,25 @@ namespace AzFramework
        camera.m_lookAt = transform.GetTranslation() + (camera.Rotation().GetBasisY() * -camera.m_lookDist);
    }

-    static ScreenVector CursorDelta(const AZStd::optional<ScreenPoint>& currentPosition, const AZStd::optional<ScreenPoint>& lastPosition)
-    {
-        return currentPosition.has_value() && lastPosition.has_value() ? currentPosition.value() - lastPosition.value()
-                                                                       : ScreenVector(0, 0);
-    }
-
    bool CameraSystem::HandleEvents(const InputEvent& event)
    {
        if (const auto& cursor = AZStd::get_if<CursorEvent>(&event))
        {
-            m_currentCursorPosition = cursor->m_position;
+            m_cursorState.SetCurrentPosition(cursor->m_position);
        }
        else if (const auto& scroll = AZStd::get_if<ScrollEvent>(&event))
        {
            m_scrollDelta = scroll->m_delta;
        }

-        return m_cameras.HandleEvents(event, CursorDelta(m_currentCursorPosition, m_lastCursorPosition), m_scrollDelta);
+        return m_cameras.HandleEvents(event, m_cursorState.CursorDelta(), m_scrollDelta);
    }

    Camera CameraSystem::StepCamera(const Camera& targetCamera, const float deltaTime)
    {
-        const auto cursorDelta = CursorDelta(m_currentCursorPosition, m_lastCursorPosition);
-        if (m_currentCursorPosition.has_value())
-        {
-            m_lastCursorPosition = m_currentCursorPosition;
-        }
+        const auto nextCamera = m_cameras.StepCamera(targetCamera, m_cursorState.CursorDelta(), m_scrollDelta, deltaTime);

-        const auto nextCamera = m_cameras.StepCamera(targetCamera, cursorDelta, m_scrollDelta, deltaTime);
+        m_cursorState.Update();

        m_scrollDelta = 0.0f;

@@ -236,12 +227,12 @@ namespace AzFramework
            }
        }

-        // accumulate
-        Camera nextCamera = targetCamera;
-        for (auto& cameraInput : m_activeCameraInputs)
-        {
-            nextCamera = cameraInput->StepCamera(nextCamera, cursorDelta, scrollDelta, deltaTime);
-        }
+        const Camera nextCamera = AZStd::accumulate(
+            AZStd::begin(m_activeCameraInputs), AZStd::end(m_activeCameraInputs), targetCamera,
+            [cursorDelta, scrollDelta, deltaTime](Camera acc, auto& camera) {
+                acc = camera->StepCamera(acc, cursorDelta, scrollDelta, deltaTime);
+                return acc;
+            });

        for (int i = 0; i < m_activeCameraInputs.size();)
        {
@@ -275,34 +266,42 @@ namespace AzFramework
        }
    }

+    RotateCameraInput::RotateCameraInput(const InputChannelId rotateChannelId)
+        : m_rotateChannelId(rotateChannelId)
+    {
+    }
+
    void RotateCameraInput::HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, [[maybe_unused]] float scrollDelta)
    {
-        if (const auto& input = AZStd::get_if<DiscreteInputEvent>(&event))
-        {
-            if (input->m_channelId == m_rotateChannelId)
+        const ClickDetector::ClickEvent clickEvent = [&event, this] {
+            if (const auto& input = AZStd::get_if<DiscreteInputEvent>(&event))
            {
-                if (input->m_state == InputChannel::State::Began)
+                if (input->m_channelId == m_rotateChannelId)
                {
-                    m_tryingToBegin = true;
-                    m_moveAccumulator = 0.0f;
-                }
-                else if (input->m_state == InputChannel::State::Ended)
-                {
-                    m_tryingToBegin = false;
-                    EndActivation();
+                    if (input->m_state == InputChannel::State::Began)
+                    {
+                        return ClickDetector::ClickEvent::Down;
+                    }
+                    else if (input->m_state == InputChannel::State::Ended)
+                    {
+                        return ClickDetector::ClickEvent::Up;
+                    }
                }
            }
-        }
+            return ClickDetector::ClickEvent::Nil;
+        }();

-        if (m_tryingToBegin)
+        switch (const auto outcome = m_clickDetector.DetectClick(clickEvent, cursorDelta); outcome)
        {
-            // only allow the action to begin if the mouse has been moved a small amount
-            m_moveAccumulator += ScreenVectorLength(cursorDelta);
-            if (m_moveAccumulator > ed_cameraSystemLookDeadzone)
-            {
-                BeginActivation();
-                m_tryingToBegin = false;
-            }
+        case ClickDetector::ClickOutcome::Move:
+            BeginActivation();
+            break;
+        case ClickDetector::ClickOutcome::Release:
+            EndActivation();
+            break;
+        default:
+            // noop
+            break;
        }
    }

@@ -324,6 +323,12 @@ namespace AzFramework
        return nextCamera;
    }

+    PanCameraInput::PanCameraInput(const InputChannelId panChannelId, PanAxesFn panAxesFn)
+        : m_panAxesFn(AZStd::move(panAxesFn))
+        , m_panChannelId(panChannelId)
+    {
+    }
+
    void PanCameraInput::HandleEvents(
        const InputEvent& event, [[maybe_unused]] const ScreenVector& cursorDelta, [[maybe_unused]] float scrollDelta)
    {
@@ -400,6 +405,11 @@ namespace AzFramework
        return TranslationType::Nil;
    }

+    TranslateCameraInput::TranslateCameraInput(TranslationAxesFn translationAxesFn)
+        : m_translationAxesFn(AZStd::move(translationAxesFn))
+    {
+    }
+
    void TranslateCameraInput::HandleEvents(
        const InputEvent& event, [[maybe_unused]] const ScreenVector& cursorDelta, [[maybe_unused]] float scrollDelta)
    {
@@ -574,6 +584,11 @@ namespace AzFramework
        return nextCamera;
    }

+    OrbitDollyCursorMoveCameraInput::OrbitDollyCursorMoveCameraInput(const InputChannelId dollyChannelId)
+        : m_dollyChannelId(dollyChannelId)
+    {
+    }
+
    void OrbitDollyCursorMoveCameraInput::HandleEvents(
        const InputEvent& event, [[maybe_unused]] const ScreenVector& cursorDelta, [[maybe_unused]] float scrollDelta)
    {
@@ -17,6 +17,8 @@
 #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>

@@ -188,26 +190,21 @@ namespace AzFramework
        Cameras m_cameras;

    private:
+        CursorState m_cursorState;
        float m_scrollDelta = 0.0f;
-        AZStd::optional<ScreenPoint> m_lastCursorPosition;
-        AZStd::optional<ScreenPoint> m_currentCursorPosition;
    };

    class RotateCameraInput : public CameraInput
    {
    public:
-        explicit RotateCameraInput(const InputChannelId rotateChannelId)
-            : m_rotateChannelId(rotateChannelId)
-        {
-        }
+        explicit RotateCameraInput(InputChannelId rotateChannelId);

        void HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, float scrollDelta) override;
        Camera StepCamera(const Camera& targetCamera, const ScreenVector& cursorDelta, float scrollDelta, float deltaTime) override;

    private:
        InputChannelId m_rotateChannelId;
-        float m_moveAccumulator = 0.0f;
-        bool m_tryingToBegin = false;
+        ClickDetector m_clickDetector;
    };

    struct PanAxes
@@ -240,11 +237,8 @@ namespace AzFramework
    class PanCameraInput : public CameraInput
    {
    public:
-        PanCameraInput(const InputChannelId panChannelId, PanAxesFn panAxesFn)
-            : m_panAxesFn(AZStd::move(panAxesFn))
-            , m_panChannelId(panChannelId)
-        {
-        }
+        PanCameraInput(InputChannelId panChannelId, PanAxesFn panAxesFn);
+
        void HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, float scrollDelta) override;
        Camera StepCamera(const Camera& targetCamera, const ScreenVector& cursorDelta, float scrollDelta, float deltaTime) override;

@@ -283,10 +277,8 @@ namespace AzFramework
    class TranslateCameraInput : public CameraInput
    {
    public:
-        explicit TranslateCameraInput(TranslationAxesFn translationAxesFn)
-            : m_translationAxesFn(AZStd::move(translationAxesFn))
-        {
-        }
+        explicit TranslateCameraInput(TranslationAxesFn translationAxesFn);
+
        void 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;
@@ -363,8 +355,7 @@ namespace AzFramework
    class OrbitDollyCursorMoveCameraInput : public CameraInput
    {
    public:
-        explicit OrbitDollyCursorMoveCameraInput(const InputChannelId dollyChannelId)
-            : m_dollyChannelId(dollyChannelId) {}
+        explicit OrbitDollyCursorMoveCameraInput(InputChannelId dollyChannelId);

        void HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, float scrollDelta) override;
        Camera StepCamera(const Camera& targetCamera, const ScreenVector& cursorDelta, float scrollDelta, float deltaTime) override;
@@ -0,0 +1,68 @@
+/*
+ * 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 <AzFramework/Viewport/ClickDetector.h>
+#include <AzFramework/Viewport/ScreenGeometry.h>
+
+namespace AzFramework
+{
+    ClickDetector::ClickOutcome ClickDetector::DetectClick(const ClickEvent clickEvent, const ScreenVector& cursorDelta)
+    {
+        if (clickEvent == ClickEvent::Down)
+        {
+            const auto now = std::chrono::steady_clock::now();
+            if (m_tryBeginTime)
+            {
+                const std::chrono::duration<float> diff = now - m_tryBeginTime.value();
+                if (diff.count() < m_doubleClickInterval)
+                {
+                    return ClickOutcome::Nil;
+                }
+            }
+
+            m_detectionState = DetectionState::WaitingForMove;
+            m_moveAccumulator = 0.0f;
+
+            m_tryBeginTime = now;
+        }
+        else if (clickEvent == ClickEvent::Up)
+        {
+            const auto clickOutcome = [detectionState = m_detectionState] {
+                if (detectionState == DetectionState::WaitingForMove)
+                {
+                    return ClickOutcome::Click;
+                }
+                if (detectionState == DetectionState::Moved)
+                {
+                    return ClickOutcome::Release;
+                }
+                return ClickOutcome::Nil;
+            }();
+
+            m_detectionState = DetectionState::Nil;
+            return clickOutcome;
+        }
+
+        if (m_detectionState == 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;
+                return ClickOutcome::Move;
+            }
+        }
+
+        return ClickOutcome::Nil;
+    }
+} // namespace AzFramework
@@ -0,0 +1,75 @@
+/*
+ * 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.
+ *
+ */
+
+#pragma once
+
+#include <AzCore/std/optional.h>
+
+#include <chrono>
+
+namespace AzFramework
+{
+    struct ScreenVector;
+
+    //! Utility class to help detect different types of mouse click (mouse down and up with
+    //! no movement), mouse move (down and initial move after some threshold) and mouse release
+    //! (mouse down with movement and then mouse up).
+    class ClickDetector
+    {
+        //! Alias for recording time of mouse down events
+        using Time = std::chrono::time_point<std::chrono::steady_clock>;
+
+    public:
+        //! Internal representation of click event (map from external event for this when
+        //! calling DetectClick).
+        enum class ClickEvent
+        {
+            Nil,
+            Down,
+            Up
+        };
+
+        //! The type of mouse click.
+        enum class ClickOutcome
+        {
+            Nil, //!< Not recognized.
+            Move, //!< Initial move after mouse down.
+            Click, //!< Mouse down and up with no intermediate movement.
+            Release //!< Mouse down with movement and then mouse up.
+        };
+
+        //! Called from any type of 'handle event' function.
+        ClickOutcome DetectClick(ClickEvent clickEvent, const ScreenVector& cursorDelta);
+
+        void SetDoubleClickInterval(float doubleClickInterval);
+
+    private:
+        //! Internal state of ClickDetector based on incoming events.
+        enum class DetectionState
+        {
+            Nil, //!< Initial state
+            WaitingForMove, //! Mouse down has happened but mouse hasn't yet moved.
+            Moved //! Mouse has moved, no longer will be counted as a click.
+        };
+
+        float m_moveAccumulator = 0.0f; //!< How far the mouse has moved after mouse down.
+        float m_deadZone = 2.0f; //!< How far to move before a click is cancelled (when Move will fire).
+        float m_doubleClickInterval = 0.4f; //!< Default double click interval, can be overridden.
+        DetectionState m_detectionState; //!< Internal state of ClickDetector.
+        AZStd::optional<Time> m_tryBeginTime; //!< Mouse down time (happens each mouse down, helps with double click handling).
+    };
+
+    inline void ClickDetector::SetDoubleClickInterval(const float doubleClickInterval)
+    {
+        m_doubleClickInterval = doubleClickInterval;
+    }
+} // namespace AzFramework
@@ -0,0 +1,56 @@
+/*
+ * 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.
+ *
+ */
+
+#pragma once
+
+#include <AzFramework/Viewport/ScreenGeometry.h>
+
+#include <AzCore/std/optional.h>
+
+namespace AzFramework
+{
+    //! Utility type to wrap a current and last cursor position.
+    struct CursorState
+    {
+        //! Returns the delta between the current and last cursor position.
+        [[nodiscard]] ScreenVector CursorDelta() const;
+        //! Call this in a 'handle event' call to update the most recent cursor position.
+        void SetCurrentPosition(const ScreenPoint& currentPosition);
+        //! Call this in an 'update' call to copy the current cursor position to the last
+        //! cursor position.
+        void Update();
+
+    private:
+        AZStd::optional<ScreenPoint> m_lastCursorPosition;
+        AZStd::optional<ScreenPoint> m_currentCursorPosition;
+    };
+
+    inline void CursorState::SetCurrentPosition(const ScreenPoint& currentPosition)
+    {
+        m_currentCursorPosition = currentPosition;
+    }
+
+    inline ScreenVector CursorState::CursorDelta() const
+    {
+        return m_currentCursorPosition.has_value() && m_lastCursorPosition.has_value()
+            ? m_currentCursorPosition.value() - m_lastCursorPosition.value()
+            : ScreenVector(0, 0);
+    }
+
+    inline void CursorState::Update()
+    {
+        if (m_currentCursorPosition.has_value())
+        {
+            m_lastCursorPosition = m_currentCursorPosition;
+        }
+    }
+} // namespace AzFramework
@@ -103,6 +103,9 @@ set(FILES
    Viewport/CameraState.cpp
    Viewport/CameraInput.h
    Viewport/CameraInput.cpp
+    Viewport/ClickDetector.h
+    Viewport/ClickDetector.cpp
+    Viewport/CursorState.h
    Viewport/DisplayContextRequestBus.h
    Entity/BehaviorEntity.cpp
    Entity/BehaviorEntity.h