Initial fix for geometry being intersected when not visible (#6473)

* initial fix for geometry being intersected when not visible Signed-off-by: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com> * add tests for shape intersection with helpers enabled/disabled Signed-off-by: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com> * update moved cmake file after merge Signed-off-by: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com> * updates following PR feedback Signed-off-by: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com>
2022-01-25 16:56:58 +00:00
parent ad8b142230
commit ea55c6d5e5
8 changed files with 213 additions and 61 deletions
@@ -8,13 +8,11 @@

 #pragma once

-#include <AzCore/EBus/EBus.h>
 #include <AzCore/Component/EntityId.h>
+#include <AzCore/EBus/EBus.h>
 #include <AzFramework/Render/GeometryIntersectionStructures.h>
 #include <AzToolsFramework/ToolsComponents/EditorSelectionAccentSystemComponent.h>

-class CEntityObject;
-
 namespace AzFramework
 {
    struct ViewportInfo;
@@ -22,56 +20,68 @@ namespace AzFramework

 namespace AzToolsFramework
 {
-    /// Bus for customizing Entity selection logic from within the EditorComponents.
-    /// Used to provide with custom implementation for Ray intersection tests, specifying AABB, etc.
-    class EditorComponentSelectionRequests
-        : public AZ::ComponentBus
+    //! Bus for customizing Entity selection logic from within the EditorComponents.
+    //! Used to provide with custom implementation for Ray intersection tests, specifying AABB, etc.
+    class EditorComponentSelectionRequests : public AZ::ComponentBus
    {
    public:
-        /// @brief Returns an AABB that encompasses the object.
-        /// @return AABB that encompasses the object.
-        /// @note ViewportInfo may be necessary if the all or part of the object
-        /// stays at a constant size regardless of camera position.
-        virtual AZ::Aabb GetEditorSelectionBoundsViewport(
-            const AzFramework::ViewportInfo& /*viewportInfo*/)
+        //! @brief Returns an AABB that encompasses the object.
+        //! @return AABB that encompasses the object.
+        //! @note ViewportInfo may be necessary if the all or part of the object
+        //! stays at a constant size regardless of camera position.
+        virtual AZ::Aabb GetEditorSelectionBoundsViewport([[maybe_unused]] const AzFramework::ViewportInfo& viewportInfo)
        {
-            AZ_Assert(!SupportsEditorRayIntersect(),
+            AZ_Assert(
+                !SupportsEditorRayIntersect(),
                "Component claims to support ray intersection but GetEditorSelectionBoundsViewport "
                "has not been implemented in the derived class");

            return AZ::Aabb::CreateNull();
        }

-        /// @brief Returns true if editor selection ray intersects with the handler.
-        /// @return True if the editor selection ray intersects the handler.
-        /// @note ViewportInfo may be necessary if the all or part of the object
-        /// stays at a constant size regardless of camera position.
+        //! @brief Returns true if editor selection ray intersects with the handler.
+        //! @return True if the editor selection ray intersects the handler.
+        //! @note ViewportInfo may be necessary if the all or part of the object
+        //! stays at a constant size regardless of camera position.
        virtual bool EditorSelectionIntersectRayViewport(
-            const AzFramework::ViewportInfo& /*viewportInfo*/,
-            const AZ::Vector3& /*src*/, const AZ::Vector3& /*dir*/, float& /*distance*/)
+            [[maybe_unused]] const AzFramework::ViewportInfo& viewportInfo,
+            [[maybe_unused]] const AZ::Vector3& src,
+            [[maybe_unused]] const AZ::Vector3& dir,
+            [[maybe_unused]] float& distance)
        {
-            AZ_Assert(!SupportsEditorRayIntersect(),
+            AZ_Assert(
+                !SupportsEditorRayIntersect(),
                "Component claims to support ray intersection but EditorSelectionIntersectRayViewport "
                "has not been implemented in the derived class");

            return false;
        }

-        /// @brief Returns true if the component overrides EditorSelectionIntersectRay method,
-        /// otherwise selection will be based only on AABB test.
-        /// @return True if EditorSelectionIntersectRay method is implemented.
-        virtual bool SupportsEditorRayIntersect() { return false; }
+        //! @brief Returns if the component overrides EditorSelectionIntersectRay(Viewport) interface,
+        //! otherwise selection will be based only on an AABB test.
+        virtual bool SupportsEditorRayIntersect()
+        {
+            return false;
+        }
+
+        //! @brief Returns if the component overrides EditorSelectionIntersectRay(Viewport) interface,
+        //! otherwise selection will be based only on an AABB test.
+        //! @note Overload of SupportsEditorRayIntersect which accepts a ViewportInfo containing the ViewportId, this can be used to
+        //! lookup the intersection setting per viewport.
+        virtual bool SupportsEditorRayIntersectViewport([[maybe_unused]] const AzFramework::ViewportInfo& viewportInfo)
+        {
+            return SupportsEditorRayIntersect();
+        }

    protected:
        ~EditorComponentSelectionRequests() = default;
    };

-    /// Type to inherit to implement EditorComponentSelectionRequests.
+    //! Type to inherit to implement EditorComponentSelectionRequests.
    using EditorComponentSelectionRequestsBus = AZ::EBus<EditorComponentSelectionRequests>;

-    /// Bus that provides notifications about selection events of the parent Entity.
-    class EditorComponentSelectionNotifications
-        : public AZ::EBusTraits
+    //! Bus that provides notifications about selection events of the parent Entity.
+    class EditorComponentSelectionNotifications : public AZ::EBusTraits
    {
    public:
        // EBusTraits overrides
@@ -79,20 +89,21 @@ namespace AzToolsFramework
        static const AZ::EBusAddressPolicy AddressPolicy = AZ::EBusAddressPolicy::ById;
        typedef AZ::EntityId BusIdType;

-        /// @brief Notifies listeners about in-editor selection events (mouse hover, selected, etc.)
-        virtual void OnAccentTypeChanged(EntityAccentType /*accent*/) {}
+        //! @brief Notifies listeners about in-editor selection events (mouse hover, selected, etc.)
+        virtual void OnAccentTypeChanged([[maybe_unused]] EntityAccentType accent)
+        {
+        }

    protected:
        ~EditorComponentSelectionNotifications() = default;
    };

-    /// Type to inherit to implement EditorComponentSelectionNotifications.
+    //! Type to inherit to implement EditorComponentSelectionNotifications.
    using EditorComponentSelectionNotificationsBus = AZ::EBus<EditorComponentSelectionNotifications>;

-    /// Returns the union of all editor selection bounds on a given Entity.
-    /// @note The returned Aabb is in world space.
-    inline AZ::Aabb CalculateEditorEntitySelectionBounds(
-        const AZ::EntityId entityId, const AzFramework::ViewportInfo& viewportInfo)
+    //! Returns the union of all editor selection bounds on a given Entity.
+    //! @note The returned Aabb is in world space.
+    inline AZ::Aabb CalculateEditorEntitySelectionBounds(const AZ::EntityId entityId, const AzFramework::ViewportInfo& viewportInfo)
    {
        AZ::EBusReduceResult<AZ::Aabb, AzFramework::AabbUnionAggregator> aabbResult(AZ::Aabb::CreateNull());
        EditorComponentSelectionRequestsBus::EventResult(
@@ -95,11 +95,12 @@ namespace AzToolsFramework
            entityId,
            [mouseInteraction, &entityPicked, &closestDistance, viewportId](EditorComponentSelectionRequests* handler) -> bool
            {
-                if (handler->SupportsEditorRayIntersect())
+                const auto viewportInfo = AzFramework::ViewportInfo{ viewportId };
+                if (handler->SupportsEditorRayIntersectViewport(viewportInfo))
                {
                    float distance = std::numeric_limits<float>::max();
                    const bool intersection = handler->EditorSelectionIntersectRayViewport(
-                        { viewportId }, mouseInteraction.m_mousePick.m_rayOrigin, mouseInteraction.m_mousePick.m_rayDirection, distance);
+                        viewportInfo, mouseInteraction.m_mousePick.m_rayOrigin, mouseInteraction.m_mousePick.m_rayDirection, distance);

                    if (intersection && distance < closestDistance)
                    {
@@ -201,6 +201,7 @@ namespace AZ

            debugDisplay.PushMatrix(worldTM);

+            debugDisplay.SetColor(AZ::Colors::White);
            debugDisplay.DrawWireBox(localAabb.GetMin(), localAabb.GetMax());

            debugDisplay.PopMatrix();
@@ -7,8 +7,11 @@
 */

 #include "EditorBaseShapeComponent.h"
+
 #include <AzCore/Interface/Interface.h>
 #include <AzCore/Serialization/EditContext.h>
+#include <AzToolsFramework/Viewport/ViewportMessages.h>
+#include <AzToolsFramework/Viewport/ViewportSettings.h>
 #include <LmbrCentral/Shape/ShapeComponentBus.h>

 namespace LmbrCentral
@@ -35,22 +38,28 @@ namespace LmbrCentral
            ->Field("Visible", &EditorBaseShapeComponent::m_visibleInEditor)
            ->Field("GameView", &EditorBaseShapeComponent::m_visibleInGameView)
            ->Field("DisplayFilled", &EditorBaseShapeComponent::m_displayFilled)
-            ->Field("ShapeColor", &EditorBaseShapeComponent::m_shapeColor)
-            ;
+            ->Field("ShapeColor", &EditorBaseShapeComponent::m_shapeColor);

        if (auto editContext = context.GetEditContext())
        {
            editContext->Class<EditorBaseShapeComponent>("EditorBaseShapeComponent", "Editor base shape component")
                ->ClassElement(AZ::Edit::ClassElements::EditorData, "")
                ->Attribute(AZ::Edit::Attributes::Visibility, AZ::Edit::PropertyVisibility::ShowChildrenOnly)
-                ->DataElement(AZ::Edit::UIHandlers::CheckBox, &EditorBaseShapeComponent::m_visibleInEditor, "Visible", "Always display this shape in the editor viewport")
-                ->DataElement(AZ::Edit::UIHandlers::CheckBox, &EditorBaseShapeComponent::m_visibleInGameView, "Game View", "Display the shape while in Game View")
-                ->DataElement(AZ::Edit::UIHandlers::CheckBox, &EditorBaseShapeComponent::m_displayFilled, "Filled", "Display the shape as either filled or wireframe") // hidden before selection is resolved
-                     ->Attribute(AZ::Edit::Attributes::ChangeNotify, &EditorBaseShapeComponent::OnDisplayFilledChanged)
-                ->DataElement(AZ::Edit::UIHandlers::Default, &EditorBaseShapeComponent::m_shapeColor, "Shape Color", "The color to use when rendering the faces of the shape object")
-                    ->Attribute(AZ::Edit::Attributes::ChangeNotify, &EditorBaseShapeComponent::OnShapeColorChanged)
-                    ->Attribute(AZ::Edit::Attributes::Visibility, &EditorBaseShapeComponent::GetShapeColorIsEditable)
-                ;
+                ->DataElement(
+                    AZ::Edit::UIHandlers::CheckBox, &EditorBaseShapeComponent::m_visibleInEditor, "Visible",
+                    "Always display this shape in the editor viewport")
+                ->DataElement(
+                    AZ::Edit::UIHandlers::CheckBox, &EditorBaseShapeComponent::m_visibleInGameView, "Game View",
+                    "Display the shape while in Game View")
+                ->DataElement(
+                    AZ::Edit::UIHandlers::CheckBox, &EditorBaseShapeComponent::m_displayFilled, "Filled",
+                    "Display the shape as either filled or wireframe") // hidden before selection is resolved
+                ->Attribute(AZ::Edit::Attributes::ChangeNotify, &EditorBaseShapeComponent::OnDisplayFilledChanged)
+                ->DataElement(
+                    AZ::Edit::UIHandlers::Default, &EditorBaseShapeComponent::m_shapeColor, "Shape Color",
+                    "The color to use when rendering the faces of the shape object")
+                ->Attribute(AZ::Edit::Attributes::ChangeNotify, &EditorBaseShapeComponent::OnShapeColorChanged)
+                ->Attribute(AZ::Edit::Attributes::Visibility, &EditorBaseShapeComponent::GetShapeColorIsEditable);
        }
    }

@@ -84,7 +93,8 @@ namespace LmbrCentral
    void EditorBaseShapeComponent::SetShapeColor(const AZ::Color& shapeColor)
    {
        m_shapeColor = shapeColor;
-        AzToolsFramework::ToolsApplicationEvents::Bus::Broadcast(&AzToolsFramework::ToolsApplicationEvents::InvalidatePropertyDisplay, AzToolsFramework::Refresh_Values);
+        AzToolsFramework::ToolsApplicationEvents::Bus::Broadcast(
+            &AzToolsFramework::ToolsApplicationEvents::InvalidatePropertyDisplay, AzToolsFramework::Refresh_Values);
    }

    void EditorBaseShapeComponent::SetShapeWireframeColor(const AZ::Color& wireColor)
@@ -97,7 +107,7 @@ namespace LmbrCentral
        if (m_shapeColorIsEditable != editable)
        {
            m_shapeColorIsEditable = editable;
-            
+
            if (editable)
            {
                // Restore the color to the value from when it was previously editable.
@@ -110,7 +120,8 @@ namespace LmbrCentral
            }

            // This changes the visibility of a property so a request to refresh the entire tree must be sent.
-            AzToolsFramework::ToolsApplicationEvents::Bus::Broadcast(&AzToolsFramework::ToolsApplicationEvents::InvalidatePropertyDisplay, AzToolsFramework::Refresh_EntireTree);
+            AzToolsFramework::ToolsApplicationEvents::Bus::Broadcast(
+                &AzToolsFramework::ToolsApplicationEvents::InvalidatePropertyDisplay, AzToolsFramework::Refresh_EntireTree);
        }
    }

@@ -129,15 +140,13 @@ namespace LmbrCentral
        m_shapeConfig = shapeConfig;
    }

-    AZ::Aabb EditorBaseShapeComponent::GetEditorSelectionBoundsViewport(
-        const AzFramework::ViewportInfo& /*viewportInfo*/)
+    AZ::Aabb EditorBaseShapeComponent::GetEditorSelectionBoundsViewport([[maybe_unused]] const AzFramework::ViewportInfo& viewportInfo)
    {
        return GetWorldBounds();
    }

    bool EditorBaseShapeComponent::EditorSelectionIntersectRayViewport(
-        const AzFramework::ViewportInfo& /*viewportInfo*/,
-        const AZ::Vector3& src, const AZ::Vector3& dir, float& distance)
+        [[maybe_unused]] const AzFramework::ViewportInfo& viewportInfo, const AZ::Vector3& src, const AZ::Vector3& dir, float& distance)
    {
        // if we are not drawing this or it is wireframe, do not allow selection
        if (!CanDraw() || !m_displayFilled)
@@ -145,19 +154,33 @@ namespace LmbrCentral
            return false;
        }

-        // Don't intersect with shapes when the camera is inside them
+        // don't intersect with shapes when the camera is inside them
        bool isInside = false;
        ShapeComponentRequestsBus::EventResult(isInside, GetEntityId(), &ShapeComponentRequests::IsPointInside, src);
        if (isInside)
        {
            return false;
        }
-        
+
        bool rayHit = false;
        ShapeComponentRequestsBus::EventResult(rayHit, GetEntityId(), &ShapeComponentRequests::IntersectRay, src, dir, distance);
        return rayHit;
    }

+    bool EditorBaseShapeComponent::SupportsEditorRayIntersect()
+    {
+        return AzToolsFramework::HelpersVisible();
+    }
+
+    bool EditorBaseShapeComponent::SupportsEditorRayIntersectViewport(const AzFramework::ViewportInfo& viewportInfo)
+    {
+        bool helpersVisible = false;
+        AzToolsFramework::ViewportInteraction::ViewportSettingsRequestBus::EventResult(
+            helpersVisible, viewportInfo.m_viewportId,
+            &AzToolsFramework::ViewportInteraction::ViewportSettingsRequestBus::Events::HelpersVisible);
+        return helpersVisible;
+    }
+
    void EditorBaseShapeComponent::OnAccentTypeChanged(AzToolsFramework::EntityAccentType accent)
    {
        if (accent == AzToolsFramework::EntityAccentType::Hover || IsSelected())
@@ -200,8 +223,7 @@ namespace LmbrCentral
        AZ::Transform unused;
        AZ::Aabb resultBounds = AZ::Aabb::CreateNull();
        LmbrCentral::ShapeComponentRequestsBus::Event(
-            GetEntityId(), &LmbrCentral::ShapeComponentRequestsBus::Events::GetTransformAndLocalBounds,
-            unused, resultBounds);
+            GetEntityId(), &LmbrCentral::ShapeComponentRequestsBus::Events::GetTransformAndLocalBounds, unused, resultBounds);
        return resultBounds;
    }

@@ -72,7 +72,8 @@ namespace LmbrCentral
        bool EditorSelectionIntersectRayViewport(
            const AzFramework::ViewportInfo& viewportInfo,
            const AZ::Vector3& src, const AZ::Vector3& dir, float& distance) override;
-        bool SupportsEditorRayIntersect() override { return true; }
+        bool SupportsEditorRayIntersect() override;
+        bool SupportsEditorRayIntersectViewport(const AzFramework::ViewportInfo& viewportInfo) override;

        // EditorComponentSelectionNotificationsBus overrides ... 
        void OnAccentTypeChanged(AzToolsFramework::EntityAccentType accent) override;
@@ -0,0 +1,114 @@
+/*
+ * 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 <AzManipulatorTestFramework/AzManipulatorTestFrameworkTestHelpers.h>
+#include <AzTest/AzTest.h>
+#include <AzToolsFramework/Entity/EditorEntityHelpers.h>
+#include <AzToolsFramework/UnitTest/AzToolsFrameworkTestHelpers.h>
+#include <AzToolsFramework/Viewport/ViewportSettings.h>
+#include <AzToolsFramework/Viewport/ViewportTypes.h>
+#include <AzToolsFramework/ViewportSelection/EditorSelectionUtil.h>
+
+#include "Shape/EditorSphereShapeComponent.h"
+
+namespace LmbrCentral
+{
+    using AzToolsFramework::ViewportInteraction::BuildMouseButtons;
+    using AzToolsFramework::ViewportInteraction::BuildMouseInteraction;
+    using AzToolsFramework::ViewportInteraction::BuildMousePick;
+
+    class EditorSphereShapeComponentFixture : public UnitTest::ToolsApplicationFixture
+    {
+    public:
+        void SetUpEditorFixtureImpl() override
+        {
+            AZ::SerializeContext* serializeContext = nullptr;
+            AZ::ComponentApplicationBus::BroadcastResult(serializeContext, &AZ::ComponentApplicationBus::Events::GetSerializeContext);
+
+            m_editorSphereShapeComponentDescriptor =
+                AZStd::unique_ptr<AZ::ComponentDescriptor>(EditorSphereShapeComponent::CreateDescriptor());
+            m_editorSphereShapeComponentDescriptor->Reflect(serializeContext);
+
+            m_entityId1 = UnitTest::CreateDefaultEditorEntity("Entity1");
+        }
+
+        void TearDownEditorFixtureImpl() override
+        {
+            bool entityDestroyed = false;
+            AzToolsFramework::EditorEntityContextRequestBus::BroadcastResult(
+                entityDestroyed, &AzToolsFramework::EditorEntityContextRequestBus::Events::DestroyEditorEntity, m_entityId1);
+
+            m_editorSphereShapeComponentDescriptor.reset();
+        }
+
+        AZ::EntityId m_entityId1;
+        AZStd::unique_ptr<AZ::ComponentDescriptor> m_editorSphereShapeComponentDescriptor;
+    };
+
+    struct IntersectionQueryOutcome
+    {
+        bool m_helpersVisible;
+        bool m_expectedIntersection;
+    };
+
+    using ShapeComponentIndirectCallManipulatorViewportInteractionFixture =
+        UnitTest::IndirectCallManipulatorViewportInteractionFixtureMixin<EditorSphereShapeComponentFixture>;
+
+    class ShapeComponentIndirectCallManipulatorViewportInteractionFixtureParam
+        : public ShapeComponentIndirectCallManipulatorViewportInteractionFixture
+        , public ::testing::WithParamInterface<IntersectionQueryOutcome>
+    {
+    public:
+        void SetUpEditorFixtureImpl() override
+        {
+            ShapeComponentIndirectCallManipulatorViewportInteractionFixture::SetUpEditorFixtureImpl();
+
+            auto* entity1 = AzToolsFramework::GetEntityById(m_entityId1);
+            AZ_Assert(entity1, "Entity1 could not be found");
+            entity1->Deactivate();
+            entity1->CreateComponent<EditorSphereShapeComponent>();
+            entity1->Activate();
+
+            AZ::TransformBus::Event(
+                m_entityId1, &AZ::TransformBus::Events::SetWorldTM, AZ::Transform::CreateTranslation(AZ::Vector3(0.0f, 2.0f, 0.0f)));
+            LmbrCentral::SphereShapeComponentRequestsBus::Event(
+                m_entityId1, &LmbrCentral::SphereShapeComponentRequestsBus::Events::SetRadius, 1.0f);
+
+            m_cameraState = AzFramework::CreateDefaultCamera(AZ::Transform::CreateIdentity(), AZ::Vector2(1024.0f, 768.0f));
+        }
+    };
+
+    TEST_P(ShapeComponentIndirectCallManipulatorViewportInteractionFixtureParam, ShapeIntersectionOnlyHappensWithHelpersEnabled)
+    {
+        // given
+        m_viewportManipulatorInteraction->GetViewportInteraction().SetHelpersVisible(GetParam().m_helpersVisible);
+
+        const auto entity1ScreenPosition = AzFramework::WorldToScreen(AzToolsFramework::GetWorldTranslation(m_entityId1), m_cameraState);
+        const auto viewportId = m_viewportManipulatorInteraction->GetViewportInteraction().GetViewportId();
+        const auto mouseInteraction = BuildMouseInteraction(
+            BuildMousePick(m_cameraState, entity1ScreenPosition),
+            BuildMouseButtons(AzToolsFramework::ViewportInteraction::MouseButton::None),
+            AzToolsFramework::ViewportInteraction::InteractionId(AZ::EntityId(), viewportId),
+            AzToolsFramework::ViewportInteraction::KeyboardModifiers());
+
+        // mimic mouse move
+        m_actionDispatcher->CameraState(m_cameraState)->MousePosition(entity1ScreenPosition);
+
+        // when
+        float closestDistance = AZStd::numeric_limits<float>::max();
+        const bool entityPicked = AzToolsFramework::PickEntity(m_entityId1, mouseInteraction, closestDistance, viewportId);
+
+        // then
+        EXPECT_THAT(entityPicked, ::testing::Eq(GetParam().m_expectedIntersection));
+    }
+
+    INSTANTIATE_TEST_CASE_P(
+        All,
+        ShapeComponentIndirectCallManipulatorViewportInteractionFixtureParam,
+        testing::Values(IntersectionQueryOutcome{ true, true }, IntersectionQueryOutcome{ false, false }));
+} // namespace LmbrCentral
@@ -1,5 +1,6 @@
 /*
- * 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.
+ * 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
 *
@@ -10,6 +10,7 @@ set(FILES
    LmbrCentralEditorTest.cpp
    LmbrCentralReflectionTest.h
    LmbrCentralReflectionTest.cpp
+    EditorShapeComponentIntersectionTests.cpp
    EditorBoxShapeComponentTests.cpp
    EditorSphereShapeComponentTests.cpp
    EditorCapsuleShapeComponentTests.cpp