From 59409822446d896aed127c587eda277c1d47cde1 Mon Sep 17 00:00:00 2001 From: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com> Date: Mon, 24 Jan 2022 10:41:32 +0000 Subject: [PATCH] Ensure render geometry is refreshed when the mesh component controller is moved (#6452) * ensure render geometry is refreshed when the mesh component controller is moved Signed-off-by: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com> * cache bus pointer and send update notification when mesh changes Signed-off-by: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com> * add integration test to detect MeshComponentController notification to IntersectionNotificationBus Signed-off-by: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com> * remove optimize off Signed-off-by: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com> * update build visibility for AtomLyIntegration editor static lib Signed-off-by: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com> * move runtime dependencies to gem module Signed-off-by: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com> --- .../AzFramework/Render/Intersector.h | 26 ++-- .../CommonFeatures/Code/CMakeLists.txt | 50 +++++++- .../Source/Mesh/MeshComponentController.cpp | 36 ++++-- .../Source/Mesh/MeshComponentController.h | 3 +- .../CommonFeatures/Code/Tests/Main.cpp | 39 ++++++ .../Tests/MeshComponentControllerTests.cpp | 116 ++++++++++++++++++ ...egration_commonfeatures_editor_files.cmake | 1 - ...ion_commonfeatures_editor_test_files.cmake | 11 ++ 8 files changed, 253 insertions(+), 29 deletions(-) create mode 100644 Gems/AtomLyIntegration/CommonFeatures/Code/Tests/Main.cpp create mode 100644 Gems/AtomLyIntegration/CommonFeatures/Code/Tests/MeshComponentControllerTests.cpp create mode 100644 Gems/AtomLyIntegration/CommonFeatures/Code/atomlyintegration_commonfeatures_editor_test_files.cmake diff --git a/Code/Framework/AzFramework/AzFramework/Render/Intersector.h b/Code/Framework/AzFramework/AzFramework/Render/Intersector.h index acd6553c32..ba11996a65 100644 --- a/Code/Framework/AzFramework/AzFramework/Render/Intersector.h +++ b/Code/Framework/AzFramework/AzFramework/Render/Intersector.h @@ -5,25 +5,23 @@ * SPDX-License-Identifier: Apache-2.0 OR MIT * */ + #pragma once #include #include #include - #include +#include #include #include -#include - namespace AzFramework { - namespace RenderGeometry { - //! Implementation of IntersectorBus interface, this class contains a cached AABB list - //! of the connected entities to the intersection bus and calculates performant ray intersections against them + //! Implementation of IntersectorBus interface, this class contains a cached AABB list of the connected + //! entities to the intersection bus and calculates efficient ray intersections against them. class Intersector final : public IntersectorBus::Handler , protected IntersectionNotificationBus::Handler @@ -34,32 +32,32 @@ namespace AzFramework Intersector(AzFramework::EntityContextId contextId); ~Intersector(); - // IntersectorBus + // IntersectorBus overrides ... RayResult RayIntersect(const RayRequest& ray) override; - // IntersectionNotifications + // IntersectionNotificationBus overrides ... void OnEntityConnected(AZ::EntityId entityId) override; void OnEntityDisconnected(AZ::EntityId entityId) override; void OnGeometryChanged(AZ::EntityId entityId) override; private: - struct EntityData { EntityData(AZ::EntityId id, AZ::Aabb aabb) : m_id(id) , m_aabb(aabb) - , m_ref(1) {} + , m_ref(1) + { + } AZ::EntityId m_id; AZ::Aabb m_aabb; int m_ref; }; - + class EntityDataList { public: - int AddRef(AZ::EntityId entityId); int RemoveRef(AZ::EntityId entityId); void Update(const EntityData& newData); @@ -88,5 +86,5 @@ namespace AzFramework AZStd::vector> m_candidatesSortedByDist; AzFramework::EntityContextId m_contextId; }; - } -} + } // namespace RenderGeometry +} // namespace AzFramework diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/CMakeLists.txt b/Gems/AtomLyIntegration/CommonFeatures/Code/CMakeLists.txt index 3234db2292..7c31751421 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/CMakeLists.txt +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/CMakeLists.txt @@ -77,8 +77,7 @@ ly_create_alias(NAME AtomLyIntegration_CommonFeatures.Servers NAMESPACE Gem if(PAL_TRAIT_BUILD_HOST_TOOLS) ly_add_target( - NAME AtomLyIntegration_CommonFeatures.Editor GEM_MODULE - + NAME AtomLyIntegration_CommonFeatures.Editor.Static STATIC NAMESPACE Gem AUTOUIC AUTOMOC @@ -98,7 +97,7 @@ if(PAL_TRAIT_BUILD_HOST_TOOLS) PRIVATE ATOMLYINTEGRATION_FEATURE_COMMON_EDITOR BUILD_DEPENDENCIES - PRIVATE + PUBLIC Gem::AtomLyIntegration_CommonFeatures.Static Gem::Atom_RPI.Edit Gem::AtomToolsFramework.Static @@ -108,6 +107,24 @@ if(PAL_TRAIT_BUILD_HOST_TOOLS) Legacy::Editor.Headers Legacy::EditorCommon Legacy::CryCommon + ) + + ly_add_target( + NAME AtomLyIntegration_CommonFeatures.Editor GEM_MODULE + NAMESPACE Gem + FILES_CMAKE + atomlyintegration_commonfeatures_shared_files.cmake + INCLUDE_DIRECTORIES + PRIVATE + Source + PUBLIC + Include + COMPILE_DEFINITIONS + PRIVATE + ATOMLYINTEGRATION_FEATURE_COMMON_EDITOR + BUILD_DEPENDENCIES + PRIVATE + Gem::AtomLyIntegration_CommonFeatures.Editor.Static RUNTIME_DEPENDENCIES Gem::Atom_RPI.Editor Gem::Atom_Feature_Common.Editor @@ -128,6 +145,33 @@ if(PAL_TRAIT_BUILD_HOST_TOOLS) Gem::AtomLyIntegration_CommonFeatures.Editor Gem::GradientSignal.Tools ) + + ################################################################################ + # Tests + ################################################################################ + if(PAL_TRAIT_BUILD_TESTS_SUPPORTED) + ly_add_target( + NAME AtomLyIntegration_CommonFeatures.Editor.Tests ${PAL_TRAIT_TEST_TARGET_TYPE} + NAMESPACE Gem + FILES_CMAKE + atomlyintegration_commonfeatures_editor_test_files.cmake + INCLUDE_DIRECTORIES + PRIVATE + Tests + Source + BUILD_DEPENDENCIES + PRIVATE + AZ::AzTest + AZ::AzTestShared + AZ::AzToolsFramework + AZ::AzToolsFrameworkTestCommon + Gem::AtomLyIntegration_CommonFeatures.Static + Gem::AtomLyIntegration_CommonFeatures.Editor.Static + ) + ly_add_googletest( + NAME Gem::AtomLyIntegration_CommonFeatures.Editor.Tests + ) + endif() endif() # AtomLyIntegration_CommonFeatures gem targets are required as part of the Editor and AssetProcessor diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Mesh/MeshComponentController.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Mesh/MeshComponentController.cpp index a68ad24adf..e26d328e17 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Mesh/MeshComponentController.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Mesh/MeshComponentController.cpp @@ -82,7 +82,6 @@ namespace AZ ->Field("MinimumScreenCoverage", &MeshComponentConfig::m_minimumScreenCoverage) ->Field("QualityDecayRate", &MeshComponentConfig::m_qualityDecayRate); } - } bool MeshComponentConfig::IsAssetSet() @@ -225,33 +224,42 @@ namespace AZ { } + static AzFramework::EntityContextId FindOwningContextId(const AZ::EntityId entityId) + { + AzFramework::EntityContextId contextId = AzFramework::EntityContextId::CreateNull(); + AzFramework::EntityIdContextQueryBus::EventResult( + contextId, entityId, &AzFramework::EntityIdContextQueries::GetOwningContextId); + return contextId; + } + void MeshComponentController::Activate(const AZ::EntityComponentIdPair& entityComponentIdPair) { const AZ::EntityId entityId = entityComponentIdPair.GetEntityId(); m_entityComponentIdPair = entityComponentIdPair; m_transformInterface = TransformBus::FindFirstHandler(entityId); - AZ_Warning("MeshComponentController", m_transformInterface, "Unable to attach to a TransformBus handler. This mesh will always be rendered at the origin."); + AZ_Warning( + "MeshComponentController", m_transformInterface, + "Unable to attach to a TransformBus handler. This mesh will always be rendered at the origin."); m_meshFeatureProcessor = RPI::Scene::GetFeatureProcessorForEntity(entityId); AZ_Error("MeshComponentController", m_meshFeatureProcessor, "Unable to find a MeshFeatureProcessorInterface on the entityId."); m_cachedNonUniformScale = AZ::Vector3::CreateOne(); AZ::NonUniformScaleRequestBus::EventResult(m_cachedNonUniformScale, entityId, &AZ::NonUniformScaleRequests::GetScale); - AZ::NonUniformScaleRequestBus::Event(entityId, &AZ::NonUniformScaleRequests::RegisterScaleChangedEvent, - m_nonUniformScaleChangedHandler); + AZ::NonUniformScaleRequestBus::Event( + entityId, &AZ::NonUniformScaleRequests::RegisterScaleChangedEvent, m_nonUniformScaleChangedHandler); + const auto entityContextId = FindOwningContextId(entityId); MeshComponentRequestBus::Handler::BusConnect(entityId); TransformNotificationBus::Handler::BusConnect(entityId); MaterialReceiverRequestBus::Handler::BusConnect(entityId); MaterialComponentNotificationBus::Handler::BusConnect(entityId); AzFramework::BoundsRequestBus::Handler::BusConnect(entityId); - AzFramework::EntityContextId contextId; - AzFramework::EntityIdContextQueryBus::EventResult( - contextId, entityId, &AzFramework::EntityIdContextQueries::GetOwningContextId); - AzFramework::RenderGeometry::IntersectionRequestBus::Handler::BusConnect({entityId, contextId}); + AzFramework::RenderGeometry::IntersectionRequestBus::Handler::BusConnect({ entityId, entityContextId }); + AzFramework::RenderGeometry::IntersectionNotificationBus::Bind(m_intersectionNotificationBus, entityContextId); - //Buses must be connected before RegisterModel in case requests are made as a result of HandleModelChange + // Buses must be connected before RegisterModel in case requests are made as a result of HandleModelChange RegisterModel(); } @@ -291,6 +299,11 @@ namespace AZ { m_meshFeatureProcessor->SetTransform(m_meshHandle, world, m_cachedNonUniformScale); } + + // ensure the render geometry is kept in sync with any changes to the entity the mesh is on + AzFramework::RenderGeometry::IntersectionNotificationBus::Event( + m_intersectionNotificationBus, &AzFramework::RenderGeometry::IntersectionNotificationBus::Events::OnGeometryChanged, + m_entityComponentIdPair.GetEntityId()); } void MeshComponentController::HandleNonUniformScaleChange(const AZ::Vector3& nonUniformScale) @@ -301,7 +314,7 @@ namespace AZ m_meshFeatureProcessor->SetTransform(m_meshHandle, m_transformInterface->GetWorldTM(), m_cachedNonUniformScale); } } - + RPI::ModelMaterialSlotMap MeshComponentController::GetModelMaterialSlots() const { Data::Asset modelAsset = GetModelAsset(); @@ -369,6 +382,9 @@ namespace AZ MeshComponentNotificationBus::Event(entityId, &MeshComponentNotificationBus::Events::OnModelReady, m_configuration.m_modelAsset, model); MaterialReceiverNotificationBus::Event(entityId, &MaterialReceiverNotificationBus::Events::OnMaterialAssignmentsChanged); AZ::Interface::Get()->RefreshEntityLocalBoundsUnion(entityId); + AzFramework::RenderGeometry::IntersectionNotificationBus::Event( + m_intersectionNotificationBus, &AzFramework::RenderGeometry::IntersectionNotificationBus::Events::OnGeometryChanged, + m_entityComponentIdPair.GetEntityId()); } } diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Mesh/MeshComponentController.h b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Mesh/MeshComponentController.h index 76cf6a1b39..cecd744b80 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Mesh/MeshComponentController.h +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Mesh/MeshComponentController.h @@ -162,6 +162,8 @@ namespace AZ bool m_isVisible = true; MeshComponentConfig m_configuration; AZ::Vector3 m_cachedNonUniformScale = AZ::Vector3::CreateOne(); + //! Cached bus to use to notify RenderGeometry::Intersector the entity/component has changed. + AzFramework::RenderGeometry::IntersectionNotificationBus::BusPtr m_intersectionNotificationBus; MeshFeatureProcessorInterface::ModelChangedEvent::Handler m_changeEventHandler { @@ -173,6 +175,5 @@ namespace AZ [&](const AZ::Vector3& nonUniformScale) { HandleNonUniformScaleChange(nonUniformScale); } }; }; - } // namespace Render } // namespace AZ diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Tests/Main.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Tests/Main.cpp new file mode 100644 index 0000000000..2d75cf96f5 --- /dev/null +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Tests/Main.cpp @@ -0,0 +1,39 @@ +/* + * 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 +#include + +#include + +class AtomLyIntegrationHook : public AZ::Test::ITestEnvironment +{ +public: + void SetupEnvironment() override + { + AZ::AllocatorInstance::Create(); + } + + void TeardownEnvironment() override + { + AZ::AllocatorInstance::Destroy(); + } +}; + +// required to support running integration tests with Qt +AZTEST_EXPORT int AZ_UNIT_TEST_HOOK_NAME(int argc, char** argv) +{ + ::testing::InitGoogleMock(&argc, argv); + QApplication app(argc, argv); + AZ::Test::printUnusedParametersWarning(argc, argv); + AZ::Test::addTestEnvironments({ DEFAULT_UNIT_TEST_ENV, new AtomLyIntegrationHook }); + int result = RUN_ALL_TESTS(); + return result; +} + +IMPLEMENT_TEST_EXECUTABLE_MAIN(); diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Tests/MeshComponentControllerTests.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Tests/MeshComponentControllerTests.cpp new file mode 100644 index 0000000000..bc1bf3a247 --- /dev/null +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Tests/MeshComponentControllerTests.cpp @@ -0,0 +1,116 @@ +/* + * 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 +#include +#include + +#include +#include + +namespace UnitTest +{ + static AzFramework::EntityContextId FindOwningContextId(const AZ::EntityId entityId) + { + AzFramework::EntityContextId contextId = AzFramework::EntityContextId::CreateNull(); + AzFramework::EntityIdContextQueryBus::EventResult(contextId, entityId, &AzFramework::EntityIdContextQueries::GetOwningContextId); + return contextId; + } + + class IntersectionNotificationDetector : public AzFramework::RenderGeometry::IntersectionNotificationBus::Handler + { + public: + void Connect(const AzFramework::EntityContextId& entityContextId); + void Disconnect(); + + // IntersectionNotificationBus overrides ... + void OnEntityConnected(AZ::EntityId entityId) override; + void OnEntityDisconnected(AZ::EntityId entityId) override; + void OnGeometryChanged(AZ::EntityId entityId) override; + + AZ::EntityId m_lastEntityIdChanged; + }; + + void IntersectionNotificationDetector::Connect(const AzFramework::EntityContextId& entityContextId) + { + AzFramework::RenderGeometry::IntersectionNotificationBus::Handler::BusConnect(entityContextId); + } + + void IntersectionNotificationDetector::Disconnect() + { + AzFramework::RenderGeometry::IntersectionNotificationBus::Handler::BusDisconnect(); + } + + void IntersectionNotificationDetector::OnEntityConnected([[maybe_unused]] AZ::EntityId entityId) + { + } + + void IntersectionNotificationDetector::OnEntityDisconnected([[maybe_unused]] AZ::EntityId entityId) + { + } + + void IntersectionNotificationDetector::OnGeometryChanged(AZ::EntityId entityId) + { + m_lastEntityIdChanged = entityId; + } + + class MeshComponentControllerFixture : public ToolsApplicationFixture + { + public: + void SetUpEditorFixtureImpl() override + { + m_meshComponentDescriptor = AZStd::unique_ptr(AZ::Render::MeshComponent::CreateDescriptor()); + m_meshComponentDescriptor->Reflect(GetApplication()->GetSerializeContext()); + + m_editorMeshComponentDescriptor = + AZStd::unique_ptr(AZ::Render::EditorMeshComponent::CreateDescriptor()); + m_editorMeshComponentDescriptor->Reflect(GetApplication()->GetSerializeContext()); + + m_entityId1 = CreateDefaultEditorEntity("Entity1"); + m_entityIds.push_back(m_entityId1); + + m_intersectionNotificationDetector.Connect(FindOwningContextId(m_entityId1)); + } + + void TearDownEditorFixtureImpl() override + { + bool entityDestroyed = false; + AzToolsFramework::EditorEntityContextRequestBus::BroadcastResult( + entityDestroyed, &AzToolsFramework::EditorEntityContextRequestBus::Events::DestroyEditorEntity, m_entityId1); + + m_intersectionNotificationDetector.Disconnect(); + + m_meshComponentDescriptor.reset(); + m_editorMeshComponentDescriptor.reset(); + } + + AZ::EntityId m_entityId1; + AzToolsFramework::EntityIdList m_entityIds; + AZStd::unique_ptr m_meshComponentDescriptor; + AZStd::unique_ptr m_editorMeshComponentDescriptor; + IntersectionNotificationDetector m_intersectionNotificationDetector; + }; + + TEST_F(MeshComponentControllerFixture, IntersectionNotificationBusIsNotifiedWhenMeshComponentControllerTransformIsModified) + { + auto* entity1 = AzToolsFramework::GetEntityById(m_entityId1); + entity1->Deactivate(); + entity1->CreateComponent(); + // note: RPI::Scene::GetFeatureProcessorForEntity(...) returns nullptr + // and so m_meshFeatureProcessor is null + AZ_TEST_START_TRACE_SUPPRESSION; + entity1->Activate(); + AZ_TEST_STOP_TRACE_SUPPRESSION(1); + + AZ::TransformBus::Event( + m_entityId1, &AZ::TransformBus::Events::SetWorldTM, AZ::Transform::CreateTranslation(AZ::Vector3(1.0f, 2.0f, 3.0f))); + + using ::testing::Eq; + EXPECT_THAT(m_entityId1, Eq(m_intersectionNotificationDetector.m_lastEntityIdChanged)); + } +} // namespace UnitTest diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/atomlyintegration_commonfeatures_editor_files.cmake b/Gems/AtomLyIntegration/CommonFeatures/Code/atomlyintegration_commonfeatures_editor_files.cmake index 0dea725d70..018795dcf2 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/atomlyintegration_commonfeatures_editor_files.cmake +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/atomlyintegration_commonfeatures_editor_files.cmake @@ -10,7 +10,6 @@ set(FILES Include/AtomLyIntegration/CommonFeatures/Material/EditorMaterialSystemComponentNotificationBus.h Include/AtomLyIntegration/CommonFeatures/Material/EditorMaterialSystemComponentRequestBus.h Include/AtomLyIntegration/CommonFeatures/ReflectionProbe/EditorReflectionProbeBus.h - Source/Module.cpp Source/Animation/EditorAttachmentComponent.h Source/Animation/EditorAttachmentComponent.cpp Source/EditorCommonFeaturesSystemComponent.h diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/atomlyintegration_commonfeatures_editor_test_files.cmake b/Gems/AtomLyIntegration/CommonFeatures/Code/atomlyintegration_commonfeatures_editor_test_files.cmake new file mode 100644 index 0000000000..5a58124e0a --- /dev/null +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/atomlyintegration_commonfeatures_editor_test_files.cmake @@ -0,0 +1,11 @@ +# +# 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 +# +# + +set(FILES + Tests/Main.cpp + Tests/MeshComponentControllerTests.cpp)