Fix for rotation matching and resetting also scaling the entity transform (#1856)

* fix issue with rotation matching (ditto) Signed-off-by: hultonha <hultonha@amazon.co.uk> * fix for context menu appearing Signed-off-by: hultonha <hultonha@amazon.co.uk> * minor tidy-up in EditorContextMenu Signed-off-by: hultonha <hultonha@amazon.co.uk> * add option to disable cursor during free-look Signed-off-by: hultonha <hultonha@amazon.co.uk> * small fixes after PR comments Signed-off-by: hultonha <hultonha@amazon.co.uk>
2021-07-07 15:48:33 +01:00
parent 4f523e496f
commit 51733f3809
8 changed files with 373 additions and 289 deletions
@@ -33,6 +33,61 @@ namespace AzFramework
        return Dir[aznumeric_cast<int>(invert)];
    };

+    // maps a discrete motion input to a click detector click event (e.g. button down or up event)
+    static ClickDetector::ClickEvent ClickFromInput(const InputEvent& event, const AzFramework::InputChannelId& inputChannelId)
+    {
+        if (const auto& input = AZStd::get_if<DiscreteInputEvent>(&event))
+        {
+            if (input->m_channelId == inputChannelId)
+            {
+                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;
+    }
+
+    // begins a camera input after a sufficient movement has occurred and ends a
+    // camera input once the initiating button is released
+    static void HandleActivationEvents(
+        const InputEvent& event,
+        const AzFramework::InputChannelId& inputChannelId,
+        const ScreenVector& cursorDelta,
+        ClickDetector& clickDetector,
+        CameraInput& cameraInput)
+    {
+        const auto clickEvent = ClickFromInput(event, inputChannelId);
+        switch (const auto outcome = clickDetector.DetectClick(clickEvent, cursorDelta); outcome)
+        {
+        case ClickDetector::ClickOutcome::Move:
+            cameraInput.BeginActivation();
+            break;
+        case ClickDetector::ClickOutcome::Release:
+            cameraInput.EndActivation();
+            break;
+        default:
+            // noop
+            break;
+        }
+    }
+
+    // returns true if a camera input is being updated after having been initiated from a
+    // motion input (e.g. mouse move while button held)
+    static bool CameraInputUpdatingAfterMotion(const CameraInput& cameraInput)
+    {
+        // 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 !cameraInput.Idle() && !cameraInput.Ending();
+    }
+
    // Based on paper by David Eberly - https://www.geometrictools.com/Documentation/EulerAngles.pdf
    AZ::Vector3 EulerAngles(const AZ::Matrix3x3& orientation)
    {
@@ -231,42 +286,8 @@ namespace AzFramework

    bool RotateCameraInput::HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, [[maybe_unused]] const float scrollDelta)
    {
-        const ClickDetector::ClickEvent clickEvent = [&event, this]
-        {
-            if (const auto& input = AZStd::get_if<DiscreteInputEvent>(&event))
-            {
-                if (input->m_channelId == m_rotateChannelId)
-                {
-                    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;
-        }();
-
-        switch (const auto outcome = m_clickDetector.DetectClick(clickEvent, cursorDelta); outcome)
-        {
-        case ClickDetector::ClickOutcome::Move:
-            BeginActivation();
-            break;
-        case ClickDetector::ClickOutcome::Release:
-            EndActivation();
-            break;
-        default:
-            // 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();
+        HandleActivationEvents(event, m_rotateChannelId, cursorDelta, m_clickDetector, *this);
+        return CameraInputUpdatingAfterMotion(*this);
    }

    Camera RotateCameraInput::StepCamera(
@@ -316,22 +337,8 @@ namespace AzFramework
    bool PanCameraInput::HandleEvents(
        const InputEvent& event, [[maybe_unused]] const ScreenVector& cursorDelta, [[maybe_unused]] const float scrollDelta)
    {
-        if (const auto& input = AZStd::get_if<DiscreteInputEvent>(&event))
-        {
-            if (input->m_channelId == m_panChannelId)
-            {
-                if (input->m_state == InputChannel::State::Began)
-                {
-                    BeginActivation();
-                }
-                else if (input->m_state == InputChannel::State::Ended)
-                {
-                    EndActivation();
-                }
-            }
-        }
-
-        return !Idle();
+        HandleActivationEvents(event, m_panChannelId, cursorDelta, m_clickDetector, *this);
+        return CameraInputUpdatingAfterMotion(*this);
    }

    Camera PanCameraInput::StepCamera(
@@ -638,22 +645,8 @@ namespace AzFramework
    bool OrbitDollyCursorMoveCameraInput::HandleEvents(
        const InputEvent& event, [[maybe_unused]] const ScreenVector& cursorDelta, [[maybe_unused]] const float scrollDelta)
    {
-        if (const auto& input = AZStd::get_if<DiscreteInputEvent>(&event))
-        {
-            if (input->m_channelId == m_dollyChannelId)
-            {
-                if (input->m_state == InputChannel::State::Began)
-                {
-                    BeginActivation();
-                }
-                else if (input->m_state == InputChannel::State::Ended)
-                {
-                    EndActivation();
-                }
-            }
-        }
-
-        return !Idle();
+        HandleActivationEvents(event, m_dollyChannelId, cursorDelta, m_clickDetector, *this);
+        return CameraInputUpdatingAfterMotion(*this);
    }

    Camera OrbitDollyCursorMoveCameraInput::StepCamera(
@@ -285,7 +285,8 @@ namespace AzFramework

    private:
        InputChannelId m_rotateChannelId; //!< Input channel to begin the rotate camera input.
-        ClickDetector m_clickDetector; //!< Used to determine when a sufficient motion delta has occurred to begin the input.
+        ClickDetector m_clickDetector; //!< Used to determine when a sufficient motion delta has occurred after an initial discrete input
+                                       //!< event has started (press and move event).
    };

    //! Axes to use while panning the camera.
@@ -337,6 +338,8 @@ namespace AzFramework
    private:
        PanAxesFn m_panAxesFn; //!< Builder for the particular pan axes (provided in the constructor).
        InputChannelId m_panChannelId; //!< Input channel to begin the pan camera input.
+        ClickDetector m_clickDetector; //!< Used to determine when a sufficient motion delta has occurred after an initial discrete input
+                                       //!< event has started (press and move event).
    };

    //! Axes to use while translating the camera.
@@ -489,7 +492,9 @@ namespace AzFramework
        AZStd::function<float()> m_cursorSpeedFn;

    private:
-        InputChannelId m_dollyChannelId;
+        InputChannelId m_dollyChannelId; //!< Input channel to begin the dolly cursor camera input.
+        ClickDetector m_clickDetector; //!< Used to determine when a sufficient motion delta has occurred after an initial discrete input
+                                       //!< event has started (press and move event).
    };

    //! A camera input to handle discrete scroll events that can scroll (translate) the camera along its forward axis.
@@ -1,14 +1,22 @@
 /*
 * Copyright (c) Contributors to the Open 3D Engine Project. For complete copyright and license terms please see the LICENSE at the root of this distribution.
- * 
+ *
 * SPDX-License-Identifier: Apache-2.0 OR MIT
 *
 */

-#include "EditorContextMenu.h"
+#include <AzCore/Console/IConsole.h>
+#include <AzToolsFramework/Viewport/EditorContextMenu.h>
+#include <AzToolsFramework/Viewport/ViewportMessages.h>
+#include <Editor/EditorContextMenuBus.h>

-#include "AzToolsFramework/Viewport/ViewportMessages.h"
-#include "Editor/EditorContextMenuBus.h"
+AZ_CVAR(
+    int,
+    ed_contextMenuDisplayThreshold,
+    2,
+    nullptr,
+    AZ::ConsoleFunctorFlags::Null,
+    "The minimum 'Manhattan Distance' the mouse can move before the context menu will no longer trigger");

 namespace AzToolsFramework
 {
@@ -20,25 +28,19 @@ namespace AzToolsFramework
        if (mouseInteraction.m_mouseInteraction.m_mouseButtons.Right() &&
            mouseInteraction.m_mouseEvent == ViewportInteraction::MouseEvent::Down)
        {
-            contextMenu.m_shouldOpen = true;
            contextMenu.m_clickPoint =
                ViewportInteraction::QPointFromScreenPoint(mouseInteraction.m_mouseInteraction.m_mousePick.m_screenCoordinates);
        }

-        // disable shouldOpen if right clicking an moving the mouse
-        if (mouseInteraction.m_mouseEvent == ViewportInteraction::MouseEvent::Move)
-        {
-            const QPoint currentScreenCoords =
-                ViewportInteraction::QPointFromScreenPoint(mouseInteraction.m_mouseInteraction.m_mousePick.m_screenCoordinates);
-
-            contextMenu.m_shouldOpen = contextMenu.m_shouldOpen && (currentScreenCoords - contextMenu.m_clickPoint).manhattanLength() < 2;
-        }
-
        // do show the context menu
        if (mouseInteraction.m_mouseInteraction.m_mouseButtons.Right() &&
            mouseInteraction.m_mouseEvent == ViewportInteraction::MouseEvent::Up)
        {
-            if (contextMenu.m_shouldOpen)
+            const QPoint currentScreenCoords =
+                ViewportInteraction::QPointFromScreenPoint(mouseInteraction.m_mouseInteraction.m_mousePick.m_screenCoordinates);
+
+            // if the mouse hasn't moved, open the pop-up menu
+            if ((currentScreenCoords - contextMenu.m_clickPoint).manhattanLength() < ed_contextMenuDisplayThreshold)
            {
                QWidget* parent = nullptr;
                ViewportInteraction::MainEditorViewportInteractionRequestBus::EventResult(
@@ -49,15 +51,16 @@ namespace AzToolsFramework
                contextMenu.m_menu->setAttribute(Qt::WA_DeleteOnClose);
                contextMenu.m_menu->setParent(parent);

-                // Populate global context menu.
+                // populate global context menu.
                const int contextMenuFlag = 0;
-                AzToolsFramework::EditorContextMenuBus::Broadcast(&AzToolsFramework::EditorContextMenuEvents::PopulateEditorGlobalContextMenu, contextMenu.m_menu.data(),
+                AzToolsFramework::EditorContextMenuBus::Broadcast(
+                    &AzToolsFramework::EditorContextMenuEvents::PopulateEditorGlobalContextMenu, contextMenu.m_menu.data(),
                    AzFramework::Vector2FromScreenPoint(mouseInteraction.m_mouseInteraction.m_mousePick.m_screenCoordinates),
                    contextMenuFlag);

                if (!contextMenu.m_menu->isEmpty())
                {
-                    // Use popup instead of exec; this avoids blocking input event processing while the menu dialog is active
+                    // use popup instead of exec; this avoids blocking input event processing while the menu dialog is active
                    contextMenu.m_menu->popup(QCursor::pos());
                }
            }
@@ -7,8 +7,6 @@

 #pragma once

-#include <AzCore/std/smart_ptr/unique_ptr.h>
-
 #include <QMenu>
 #include <QPoint>
 #include <QPointer>
@@ -23,7 +21,6 @@ namespace AzToolsFramework
    //! State of when and where the right-click context menu should appear.
    struct EditorContextMenu final
    {
-        bool m_shouldOpen = false;
        QPoint m_clickPoint = QPoint(0, 0);
        QPointer<QMenu> m_menu;
    };
@@ -3078,17 +3078,10 @@ namespace AzToolsFramework
            // update orientations relative to initial
            for (AZ::EntityId entityId : manipulatorEntityIds.m_entityIds)
            {
-                ScopedUndoBatch::MarkEntityDirty(entityId);
-
-                const auto transformIt = transformsBefore.find(entityId);
-                if (transformIt != transformsBefore.end())
+                if (const auto transformIt = transformsBefore.find(entityId); transformIt != transformsBefore.end())
                {
-                    AZ::Transform newWorldFromLocal = transformIt->second;
-                    const float scale = newWorldFromLocal.GetUniformScale();
-                    newWorldFromLocal.SetRotation(orientation);
-                    newWorldFromLocal *= AZ::Transform::CreateUniformScale(scale);
-
-                    SetEntityWorldTransform(entityId, newWorldFromLocal);
+                    ScopedUndoBatch::MarkEntityDirty(entityId);
+                    SetEntityLocalRotation(entityId, orientation);
                }
            }

@@ -3714,6 +3707,11 @@ namespace AzToolsFramework
        ETCS::SetEntityLocalRotation(entityId, localRotation, m_transformChangedInternally);
    }

+    void EditorTransformComponentSelection::SetEntityLocalRotation(AZ::EntityId entityId, const AZ::Quaternion& localRotation)
+    {
+        ETCS::SetEntityLocalRotation(entityId, localRotation, m_transformChangedInternally);
+    }
+
    void EditorTransformComponentSelection::OnStartPlayInEditor()
    {
        SetAllViewportUiVisible(false);
@@ -3779,6 +3777,12 @@ namespace AzToolsFramework
            ScopeSwitch sw(internal);
            AZ::TransformBus::Event(entityId, &AZ::TransformBus::Events::SetLocalRotation, localRotation);
        }
+
+        void SetEntityLocalRotation(AZ::EntityId entityId, const AZ::Quaternion& localRotation, bool& internal)
+        {
+            ScopeSwitch sw(internal);
+            AZ::TransformBus::Event(entityId, &AZ::TransformBus::Events::SetLocalRotationQuaternion, localRotation);
+        }
    } // namespace ETCS

    // explicit instantiations
@@ -297,6 +297,7 @@ namespace AzToolsFramework
        void SetEntityWorldTransform(AZ::EntityId entityId, const AZ::Transform& worldTransform);
        void SetEntityLocalScale(AZ::EntityId entityId, float localScale);
        void SetEntityLocalRotation(AZ::EntityId entityId, const AZ::Vector3& localRotation);
+        void SetEntityLocalRotation(AZ::EntityId entityId, const AZ::Quaternion& localRotation);

        //! Responsible for keeping the space cluster in sync with the current reference frame.
        void UpdateSpaceCluster(ReferenceFrame referenceFrame);
@@ -376,5 +377,6 @@ namespace AzToolsFramework
        void SetEntityWorldTransform(AZ::EntityId entityId, const AZ::Transform& worldTransform, bool& internal);
        void SetEntityLocalScale(AZ::EntityId entityId, float localScale, bool& internal);
        void SetEntityLocalRotation(AZ::EntityId entityId, const AZ::Vector3& localRotation, bool& internal);
+        void SetEntityLocalRotation(AZ::EntityId entityId, const AZ::Quaternion& localRotation, bool& internal);
    } // namespace ETCS
 } // namespace AzToolsFramework