Prevent the camera from easily being set to an invalid orientation (#6203)

* add temporary debug logging Signed-off-by: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com> * improvement for box select sometimes getting stuck on Signed-off-by: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com> * add temporary debug logging Signed-off-by: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com> * improvement for box select sometimes getting stuck on Signed-off-by: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com> * remove temporary logging Signed-off-by: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com> * fixes for camera pitch issues Signed-off-by: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com> * missed file with camera pitch fixes Signed-off-by: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com> * remove debug logs Signed-off-by: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com> * add some tests for new pitch constraint updates Signed-off-by: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com>
2021-12-08 09:42:40 +00:00
parent a8ef23e4ae
commit b54215552c
8 changed files with 121 additions and 25 deletions
@@ -356,7 +356,8 @@ namespace SandboxEditor
            AZ::TransformBus::EventResult(worldFromLocal, viewEntityId, &AZ::TransformBus::Events::GetWorldTM);

            AtomToolsFramework::ModularViewportCameraControllerRequestBus::Event(
-                m_viewportId, &AtomToolsFramework::ModularViewportCameraControllerRequestBus::Events::StartTrackingTransform, worldFromLocal);
+                m_viewportId, &AtomToolsFramework::ModularViewportCameraControllerRequestBus::Events::StartTrackingTransform,
+                worldFromLocal);
        }
        else
        {
@@ -367,8 +368,10 @@ namespace SandboxEditor

    void EditorModularViewportCameraComposer::OnTick(const float deltaTime, [[maybe_unused]] AZ::ScriptTimePoint time)
    {
-        const float delta = [duration = &ed_cameraDefaultOrbitFadeDuration, deltaTime] {
-            if (*duration == 0.0f) {
+        const float delta = [duration = &ed_cameraDefaultOrbitFadeDuration, deltaTime]
+        {
+            if (*duration == 0.0f)
+            {
                return 1.0f;
            }
            return deltaTime / *duration;
@@ -6,7 +6,6 @@
 *
 */

-
 #include "GotoPositionDlg.h"
 #include "EditorDefs.h"

@@ -25,6 +24,17 @@ AZ_PUSH_DISABLE_DLL_EXPORT_MEMBER_WARNING
 #include <ui_GotoPositionDlg.h>
 AZ_POP_DISABLE_DLL_EXPORT_MEMBER_WARNING

+void GotoPositionPitchConstraints::DeterminePitchRange(const AngleRangeConfigureFn& configurePitchRangeFn) const
+{
+    const auto [pitchMinRadians, pitchMaxRadians] = AzFramework::CameraPitchMinMaxRadians();
+    configurePitchRangeFn(AZ::RadToDeg(pitchMinRadians), AZ::RadToDeg(pitchMaxRadians));
+}
+
+float GotoPositionPitchConstraints::PitchClampedRadians(float pitchDegrees) const
+{
+    return AzFramework::ClampPitchRotation(AZ::DegToRad(pitchDegrees));
+}
+
 GotoPositionDialog::GotoPositionDialog(QWidget* parent)
    : QDialog(parent)
    , m_ui(new Ui::GotoPositionDialog)
@@ -55,20 +65,23 @@ void GotoPositionDialog::OnInitDialog()
    const auto yawDegrees = AZ::RadToDeg(cameraRotation.GetZ());

    // position
-    m_ui->m_dymX->setRange(-64000.0, 64000.0);
+    const double CameraPositionExtent = 64000.0;
+    m_ui->m_dymX->setRange(-CameraPositionExtent, CameraPositionExtent);
    m_ui->m_dymX->setValue(cameraTranslation.GetX());
-
-    m_ui->m_dymY->setRange(-64000.0, 64000.0);
+    m_ui->m_dymY->setRange(-CameraPositionExtent, CameraPositionExtent);
    m_ui->m_dymY->setValue(cameraTranslation.GetY());
-
-    m_ui->m_dymZ->setRange(-64000.0, 64000.0);
+    m_ui->m_dymZ->setRange(-CameraPositionExtent, CameraPositionExtent);
    m_ui->m_dymZ->setValue(cameraTranslation.GetZ());

    // rotation
-    m_ui->m_dymAnglePitch->setRange(-180.0, 180.0);
+    m_gotoPositionPitchConstraints.DeterminePitchRange(
+        [this](const float minPitchDegrees, const float maxPitchDegrees)
+        {
+            m_ui->m_dymAnglePitch->setRange(minPitchDegrees, maxPitchDegrees);
+        });
    m_ui->m_dymAnglePitch->setValue(pitchDegrees);

-    m_ui->m_dymAngleYaw->setRange(-180.0, 180.0);
+    m_ui->m_dymAngleYaw->setRange(-360, 360);
    m_ui->m_dymAngleYaw->setValue(yawDegrees);

    // ensure the goto button is highlighted correctly.
@@ -108,12 +121,13 @@ void GotoPositionDialog::OnUpdateNumbers()

 void GotoPositionDialog::accept()
 {
-    SandboxEditor::InterpolateDefaultViewportCameraToTransform(
-        AZ::Vector3(
-            aznumeric_cast<float>(m_ui->m_dymX->value()), aznumeric_cast<float>(m_ui->m_dymY->value()),
-            aznumeric_cast<float>(m_ui->m_dymZ->value())),
-        AZ::DegToRad(aznumeric_cast<float>(m_ui->m_dymAnglePitch->value())),
-        AZ::DegToRad(aznumeric_cast<float>(m_ui->m_dymAngleYaw->value())));
+    const auto position = AZ::Vector3(
+        aznumeric_cast<float>(m_ui->m_dymX->value()), aznumeric_cast<float>(m_ui->m_dymY->value()),
+        aznumeric_cast<float>(m_ui->m_dymZ->value()));
+    const auto pitchRadians = m_gotoPositionPitchConstraints.PitchClampedRadians(aznumeric_cast<float>(m_ui->m_dymAnglePitch->value()));
+    const auto yawRadians = AZ::DegToRad(aznumeric_cast<float>(m_ui->m_dymAngleYaw->value()));
+
+    SandboxEditor::InterpolateDefaultViewportCameraToTransform(position, pitchRadians, yawRadians);

    QDialog::accept();
 }
@@ -6,21 +6,33 @@
 *
 */

-
 #pragma once

 #if !defined(Q_MOC_RUN)
 #include <QDialog>
 #endif

+#include <SandboxAPI.h>
+
+#include <AzCore/std/functional.h>
+
 namespace Ui
 {
    class GotoPositionDialog;
 }

+//! Utility to deal with ensuring camera pitch values are in the expected range.
+struct GotoPositionPitchConstraints
+{
+    using AngleRangeConfigureFn = AZStd::function<void(float, float)>;
+    //! Notify a callback with the min and max camera pitch constraints (no tolerance included).
+    SANDBOX_API void DeterminePitchRange(const AngleRangeConfigureFn& configurePitchRangeFn) const;
+    //! Returns the clamped pitch value (including tolerance with range extents).
+    SANDBOX_API float PitchClampedRadians(float pitchDegrees) const;
+};
+
 //! GotoPositionDialog for setting camera position and rotation.
-class GotoPositionDialog
-    : public QDialog
+class GotoPositionDialog : public QDialog
 {
    Q_OBJECT

@@ -39,5 +51,6 @@ public:
    QString m_transform;

 private:
+    GotoPositionPitchConstraints m_gotoPositionPitchConstraints;
    QScopedPointer<Ui::GotoPositionDialog> m_ui;
 };
@@ -15,6 +15,8 @@
 #include <AzToolsFramework/ToolsComponents/TransformComponent.h>
 #include <EditorModularViewportCameraComposer.h>

+#include <GotoPositionDlg.h>
+
 namespace UnitTest
 {
    class EditorCameraFixture : public ::testing::Test
@@ -257,4 +259,35 @@ namespace UnitTest
        EXPECT_THAT(interpolating, ::testing::IsFalse());
        EXPECT_THAT(nextInterpolationBegan, ::testing::IsTrue());
    }
+
+    TEST(GotoPositionPitchConstraints, GoToPositionPitchIsSetToPlusOrMinusNinetyDegrees)
+    {
+        float minPitch = 0.0f;
+        float maxPitch = 0.0f;
+
+        GotoPositionPitchConstraints m_gotoPositionContraints;
+        m_gotoPositionContraints.DeterminePitchRange(
+            [&minPitch, &maxPitch](const float minPitchDegrees, const float maxPitchDegrees)
+            {
+                minPitch = minPitchDegrees;
+                maxPitch = maxPitchDegrees;
+            });
+
+        using ::testing::FloatNear;
+        EXPECT_THAT(minPitch, FloatNear(-90.0f, AZ::Constants::FloatEpsilon));
+        EXPECT_THAT(maxPitch, FloatNear(90.0f, AZ::Constants::FloatEpsilon));
+    }
+
+    TEST(GotoPositionPitchConstraints, GoToPositionPitchClampsFinalPitchValueWithTolerance)
+    {
+        const auto [expectedMinPitchRadians, expectedMaxPitchRadians] = AzFramework::CameraPitchMinMaxRadiansWithTolerance();
+
+        GotoPositionPitchConstraints m_gotoPositionContraints;
+        const float minClampedPitchRadians = m_gotoPositionContraints.PitchClampedRadians(-90.0f);
+        const float maxClampedPitchRadians = m_gotoPositionContraints.PitchClampedRadians(90.0f);
+
+        using ::testing::FloatNear;
+        EXPECT_THAT(minClampedPitchRadians, FloatNear(expectedMinPitchRadians, AZ::Constants::FloatEpsilon));
+        EXPECT_THAT(maxClampedPitchRadians, FloatNear(expectedMaxPitchRadians, AZ::Constants::FloatEpsilon));
+    }
 } // namespace UnitTest
@@ -335,10 +335,13 @@ namespace AzFramework
        Camera nextCamera = targetCamera;

        const float rotateSpeed = m_rotateSpeedFn();
-        nextCamera.m_pitch -= float(cursorDelta.m_y) * rotateSpeed * Invert(m_invertPitchFn());
-        nextCamera.m_yaw -= float(cursorDelta.m_x) * rotateSpeed * Invert(m_invertYawFn());
+        const float deltaPitch = aznumeric_cast<float>(cursorDelta.m_y) * rotateSpeed * Invert(m_invertPitchFn());
+        const float deltaYaw = aznumeric_cast<float>(cursorDelta.m_x) * rotateSpeed * Invert(m_invertYawFn());

+        nextCamera.m_pitch -= deltaPitch;
+        nextCamera.m_yaw -= deltaYaw;
        nextCamera.m_yaw = WrapYawRotation(nextCamera.m_yaw);
+
        if (m_constrainPitch())
        {
            nextCamera.m_pitch = ClampPitchRotation(nextCamera.m_pitch);
@@ -25,6 +25,9 @@ namespace AzFramework

    struct WindowSize;

+    //! Tolerance to use when limiting pitch to avoid reaching +/-Pi/2 exactly.
+    constexpr float CameraPitchTolerance = 1.0e-4f;
+
    //! Returns Euler angles (pitch, roll, yaw) for the incoming orientation.
    //! @note Order of rotation is Z, Y, X.
    AZ::Vector3 EulerAngles(const AZ::Matrix3x3& orientation);
@@ -318,11 +321,26 @@ namespace AzFramework
        return m_handlingEvents;
    }

-    //! Clamps pitch to be +/-90 degrees (-Pi/2, Pi/2).
+    //! Returns min/max values for camera pitch (in radians).
+    inline AZStd::tuple<float, float> CameraPitchMinMaxRadians()
+    {
+        return { -AZ::Constants::HalfPi, AZ::Constants::HalfPi };
+    }
+
+    //! Returns min/max values for camera pitch (in radians) including a small tolerance at each
+    //! extreme (looking directly up or down) to avoid floating point accuracy issues.
+    inline AZStd::tuple<float, float> CameraPitchMinMaxRadiansWithTolerance()
+    {
+        const auto [pitchMinRadians, pitchMaxRadians] = CameraPitchMinMaxRadians();
+        return { pitchMinRadians + CameraPitchTolerance, pitchMaxRadians - CameraPitchTolerance };
+    }
+
+    //! Clamps pitch to be +/-90 degrees (-Pi/2, Pi/2) with a minor tolerance at each extreme.
    //! @param pitch Pitch angle in radians.
    inline float ClampPitchRotation(const float pitch)
    {
-        return AZ::GetClamp(pitch, -AZ::Constants::HalfPi, AZ::Constants::HalfPi);
+        const auto [pitchMin, pitchMax] = CameraPitchMinMaxRadiansWithTolerance();
+        return AZ::GetClamp(pitch, pitchMin, pitchMax);
    }

    //! Ensures yaw wraps between 0 and 360 degrees (0, 2Pi).
@@ -322,6 +322,17 @@ namespace UnitTest
        EXPECT_THAT(m_camera.m_offset, IsClose(AZ::Vector3::CreateZero()));
    }

+    TEST(CameraInput, CameraPitchIsClampedWithExpectedTolerance)
+    {
+        const auto [expectedMinPitch, expectedMaxPitch] = AzFramework::CameraPitchMinMaxRadiansWithTolerance();
+        const float minPitch = AzFramework::ClampPitchRotation(-AZ::Constants::HalfPi);
+        const float maxPitch = AzFramework::ClampPitchRotation(AZ::Constants::HalfPi);
+
+        using ::testing::FloatNear;
+        EXPECT_THAT(minPitch, FloatNear(expectedMinPitch, AzFramework::CameraPitchTolerance));
+        EXPECT_THAT(maxPitch, FloatNear(expectedMaxPitch, AzFramework::CameraPitchTolerance));
+    }
+
    TEST_F(CameraInputFixture, OrbitRotateCameraInputRotatesPitchOffsetByNinetyDegreesWithRequiredPixelDelta)
    {
        const auto cameraStartingPosition = AZ::Vector3::CreateAxisY(-20.0f);
@@ -64,7 +64,8 @@ namespace AzToolsFramework
                }
            }

-            if (clickOutcome == AzFramework::ClickDetector::ClickOutcome::Release)
+            if (clickOutcome == AzFramework::ClickDetector::ClickOutcome::Release ||
+                clickOutcome == AzFramework::ClickDetector::ClickOutcome::Click)
            {
                if (m_leftMouseUp)
                {