From 5656736db4ae72dea0813f6699564ada4977e8f0 Mon Sep 17 00:00:00 2001 From: Tommy Walton <82672795+amzn-tommy@users.noreply.github.com> Date: Mon, 28 Jun 2021 09:26:03 -0700 Subject: [PATCH] Fix for LYN-3726 : Actor Draw Character Doesn't Work (#1336) -Re-purposed an unused boolean in RPI::Cullable for previous frame's visiblity to instead represent objects that are hidden in the simulation. -Updated MeshFeatureProcessor::SetVisible to set this value on the cullable. -Updated the MeshComponent to handle visiblity changes by not rendering the mesh instead of deactivating and/or reactivating the component. -Updated the AtomActorInstance to handle changes to the visibility from the ActorComponent. Tested by creating two entities with static mesh components, on entity hidden and the other visible. Plus three entities with actor components, one where the actor is visible, one where the entity is visible but the 'render character' setting on the actor component is disabled, and one where the 'render character' setting is enabled, but the entity is not visible. For each of these 5 entities, I added them as 5 loose entities, 5 entities that were children to a parent entity, and a slice with all 5 as children to a parent entity, and tested toggling visibility of the parent entities. For each of these 3 sets of 5 entities, I added them directly to the level, added them all to a layer where the layer was visible, and added them all to a layer where the layer was not visible, and tested toggling the visibility of the layers. --- .../Atom/Feature/Mesh/MeshFeatureProcessor.h | 1 + .../Code/Source/Mesh/MeshFeatureProcessor.cpp | 8 +++++++- .../RPI/Code/Include/Atom/RPI.Public/Culling.h | 5 +++-- Gems/Atom/RPI/Code/Source/RPI.Public/Culling.cpp | 14 ++++---------- .../Code/Source/Mesh/EditorMeshComponent.cpp | 14 ++++++++++++++ .../Code/Source/Mesh/EditorMeshComponent.h | 6 ++++++ .../Code/Source/Mesh/MeshComponentController.cpp | 9 +++------ .../Code/Source/AtomActorInstance.cpp | 12 ++++++++++++ .../EMotionFXAtom/Code/Source/AtomActorInstance.h | 2 +- .../Integration/Components/ActorComponent.cpp | 2 +- 10 files changed, 52 insertions(+), 21 deletions(-) diff --git a/Gems/Atom/Feature/Common/Code/Include/Atom/Feature/Mesh/MeshFeatureProcessor.h b/Gems/Atom/Feature/Common/Code/Include/Atom/Feature/Mesh/MeshFeatureProcessor.h index 66c524fe58..da52d63ffa 100644 --- a/Gems/Atom/Feature/Common/Code/Include/Atom/Feature/Mesh/MeshFeatureProcessor.h +++ b/Gems/Atom/Feature/Common/Code/Include/Atom/Feature/Mesh/MeshFeatureProcessor.h @@ -69,6 +69,7 @@ namespace AZ void UpdateCullBounds(const TransformServiceFeatureProcessor* transformService); void UpdateObjectSrg(); bool MaterialRequiresForwardPassIblSpecular(Data::Instance material) const; + void SetVisible(bool isVisible); using DrawPacketList = AZStd::vector; diff --git a/Gems/Atom/Feature/Common/Code/Source/Mesh/MeshFeatureProcessor.cpp b/Gems/Atom/Feature/Common/Code/Source/Mesh/MeshFeatureProcessor.cpp index 67d150c5b6..7ea5e4f09e 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Mesh/MeshFeatureProcessor.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/Mesh/MeshFeatureProcessor.cpp @@ -398,7 +398,7 @@ namespace AZ { if (meshHandle.IsValid()) { - meshHandle->m_visible = visible; + meshHandle->SetVisible(visible); } } @@ -1181,5 +1181,11 @@ namespace AZ return false; } + + void MeshDataInstance::SetVisible(bool isVisible) + { + m_visible = isVisible; + m_cullable.m_isHidden = !isVisible; + } } // namespace Render } // namespace AZ diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Culling.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Culling.h index 274beac333..4ede292b2d 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Culling.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Culling.h @@ -91,8 +91,9 @@ namespace AZ }; LodData m_lodData; - //! Flag indicating if the object is visible, i.e., was not culled out in the last frame - bool m_isVisible = true; + //! Flag indicating if the object is hidden, i.e., was specifically marked as + //! something that shouldn't be rendered, regardless of its actual position relative to the camera + bool m_isHidden = false; void SetDebugName([[maybe_unused]] const AZ::Name& debugName) { diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Culling.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Culling.cpp index 8bdea71624..e687c00abe 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Culling.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Culling.cpp @@ -325,12 +325,10 @@ namespace AZ { Cullable* c = static_cast(visibleEntry->m_userData); - // reset visibility flag to false, update to true if all culling checks pass - c->m_isVisible = false; - if ((c->m_cullData.m_drawListMask & drawListMask).none() || c->m_cullData.m_hideFlags & viewFlags || - c->m_cullData.m_scene != m_jobData->m_scene) //[GFX_TODO][ATOM-13796] once the IVisibilitySystem supports multiple octree scenes, remove this + c->m_cullData.m_scene != m_jobData->m_scene || //[GFX_TODO][ATOM-13796] once the IVisibilitySystem supports multiple octree scenes, remove this + c->m_isHidden) { continue; } @@ -341,7 +339,6 @@ namespace AZ { numDrawPackets += AddLodDataToView(c->m_cullData.m_boundingSphere.GetCenter(), c->m_lodData, *m_jobData->m_view); ++numVisibleCullables; - c->m_isVisible = true; } } } @@ -356,12 +353,10 @@ namespace AZ { Cullable* c = static_cast(visibleEntry->m_userData); - // reset visibility flag to false, update to true if all culling checks pass - c->m_isVisible = false; - if ((c->m_cullData.m_drawListMask & drawListMask).none() || c->m_cullData.m_hideFlags & viewFlags || - c->m_cullData.m_scene != m_jobData->m_scene) //[GFX_TODO][ATOM-13796] once the IVisibilitySystem supports multiple octree scenes, remove this + c->m_cullData.m_scene != m_jobData->m_scene || //[GFX_TODO][ATOM-13796] once the IVisibilitySystem supports multiple octree scenes, remove this + c->m_isHidden) { continue; } @@ -379,7 +374,6 @@ namespace AZ { numDrawPackets += AddLodDataToView(c->m_cullData.m_boundingSphere.GetCenter(), c->m_lodData, *m_jobData->m_view); ++numVisibleCullables; - c->m_isVisible = true; } } } diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Mesh/EditorMeshComponent.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Mesh/EditorMeshComponent.cpp index d8a38421ff..8aedc79952 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Mesh/EditorMeshComponent.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Mesh/EditorMeshComponent.cpp @@ -204,6 +204,7 @@ namespace AZ &AzToolsFramework::ToolsApplicationEvents::InvalidatePropertyDisplay, AzToolsFramework::Refresh_EntireTree); } + AZ::u32 EditorMeshComponent::OnConfigurationChanged() { // temp variable is needed to hold reference to m_modelAsset while it's being loaded. @@ -213,5 +214,18 @@ namespace AZ auto temp = m_controller.m_configuration.m_modelAsset; return BaseClass::OnConfigurationChanged(); } + + void EditorMeshComponent::OnEntityVisibilityChanged(bool visibility) + { + m_controller.SetVisibility(visibility); + } + + bool EditorMeshComponent::ShouldActivateController() const + { + // By default, components using the EditorRenderComponentAdapter will only activate if the component is visible + // Since the mesh component handles visibility changes by not rendering the mesh, rather than deactivating the component entirely, + // it can be activated even if it is not visible + return true; + } } // namespace Render } // namespace AZ diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Mesh/EditorMeshComponent.h b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Mesh/EditorMeshComponent.h index 41dd13427e..a9d9321816 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Mesh/EditorMeshComponent.h +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Mesh/EditorMeshComponent.h @@ -58,6 +58,12 @@ namespace AZ // MeshComponentNotificationBus overrides ... void OnModelReady(const Data::Asset& modelAsset, const Data::Instance& model) override; + // AzToolsFramework::EditorEntityVisibilityNotificationBus::Handler overrides + void OnEntityVisibilityChanged(bool visibility) override; + + // AzToolsFramework::Components::EditorComponentAdapter overrides + bool ShouldActivateController() const override; + AZ::u32 OnConfigurationChanged() override; AZ::Crc32 AddEditorMaterialComponent(); diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Mesh/MeshComponentController.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Mesh/MeshComponentController.cpp index 77821a7e10..bd972c432d 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Mesh/MeshComponentController.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Mesh/MeshComponentController.cpp @@ -325,6 +325,7 @@ namespace AZ m_meshFeatureProcessor->SetSortKey(m_meshHandle, m_configuration.m_sortKey); m_meshFeatureProcessor->SetLodOverride(m_meshHandle, m_configuration.m_lodOverride); m_meshFeatureProcessor->SetExcludeFromReflectionCubeMaps(m_meshHandle, m_configuration.m_excludeFromReflectionCubeMaps); + m_meshFeatureProcessor->SetVisible(m_meshHandle, m_isVisible); // [GFX TODO] This should happen automatically. m_changeEventHandler should be passed to AcquireMesh // If the model instance or asset already exists, announce a model change to let others know it's loaded. @@ -428,13 +429,9 @@ namespace AZ { if (m_isVisible != visible) { - if (m_isVisible) + if (m_meshFeatureProcessor) { - UnregisterModel(); - } - else - { - RegisterModel(); + m_meshFeatureProcessor->SetVisible(m_meshHandle, visible); } m_isVisible = visible; } diff --git a/Gems/AtomLyIntegration/EMotionFXAtom/Code/Source/AtomActorInstance.cpp b/Gems/AtomLyIntegration/EMotionFXAtom/Code/Source/AtomActorInstance.cpp index 9275c6415b..e91e0e4708 100644 --- a/Gems/AtomLyIntegration/EMotionFXAtom/Code/Source/AtomActorInstance.cpp +++ b/Gems/AtomLyIntegration/EMotionFXAtom/Code/Source/AtomActorInstance.cpp @@ -237,6 +237,18 @@ namespace AZ return SkinningMethod::LinearSkinning; } + void AtomActorInstance::SetIsVisible(bool isVisible) + { + if (IsVisible() != isVisible) + { + RenderActorInstance::SetIsVisible(isVisible); + if (m_meshFeatureProcessor && m_meshHandle) + { + m_meshFeatureProcessor->SetVisible(*m_meshHandle, isVisible); + } + } + } + AtomActor* AtomActorInstance::GetRenderActor() const { EMotionFX::Integration::ActorAsset* actorAsset = m_actorAsset.Get(); diff --git a/Gems/AtomLyIntegration/EMotionFXAtom/Code/Source/AtomActorInstance.h b/Gems/AtomLyIntegration/EMotionFXAtom/Code/Source/AtomActorInstance.h index 78cbdef851..d8b306ef31 100644 --- a/Gems/AtomLyIntegration/EMotionFXAtom/Code/Source/AtomActorInstance.h +++ b/Gems/AtomLyIntegration/EMotionFXAtom/Code/Source/AtomActorInstance.h @@ -82,7 +82,6 @@ namespace AZ EMotionFX::Integration::SkinningMethod skinningMethod); ~AtomActorInstance() override; - // AtomActorInstanceRequestBusTEMP::Handler interface implementation // RenderActorInstance overrides ... void OnTick(float timeDelta) override; void UpdateBounds() override; @@ -90,6 +89,7 @@ namespace AZ void SetMaterials(const EMotionFX::Integration::ActorAsset::MaterialList& materialPerLOD) override { AZ_UNUSED(materialPerLOD); }; void SetSkinningMethod(EMotionFX::Integration::SkinningMethod emfxSkinningMethod); SkinningMethod GetAtomSkinningMethod() const; + void SetIsVisible(bool isVisible) override; // BoundsRequestBus overrides ... AZ::Aabb GetWorldBounds() override; diff --git a/Gems/EMotionFX/Code/Source/Integration/Components/ActorComponent.cpp b/Gems/EMotionFX/Code/Source/Integration/Components/ActorComponent.cpp index 93e2f7cc9c..66c7ff3570 100644 --- a/Gems/EMotionFX/Code/Source/Integration/Components/ActorComponent.cpp +++ b/Gems/EMotionFX/Code/Source/Integration/Components/ActorComponent.cpp @@ -477,7 +477,7 @@ namespace EMotionFX if (!m_configuration.m_forceUpdateJointsOOV) { const bool isInCameraFrustum = m_renderActorInstance->IsInCameraFrustum(); - m_actorInstance->SetIsVisible(isInCameraFrustum); + m_actorInstance->SetIsVisible(isInCameraFrustum && m_configuration.m_renderCharacter); } RenderActorInstance::DebugOptions debugOptions;