From a9c55c1070dff2fbfd7c0c5d5afc03a40362bca2 Mon Sep 17 00:00:00 2001 From: yuriy0 Date: Tue, 15 Jun 2021 14:09:30 -0400 Subject: [PATCH] Update actor render bounding box (#991) * Extend MeshFeatureProcessor to allow changing the mesh bbox, which requires re-compute the culling data for that mesh * Update actor mesh bbox when EMFX actor instance bbox changes. Also use the actor instance global bbox to compute the local bbox for the skinned render mesh, instead of the using the static bounds based bbox, as not every actor instance is going to be using the static bounds bbox. * Store per-instance mesh AABB in the right place. In the MeshInstanceData, which is unique per instance, instead of in the Model, which is shared between all instances. For greater clarity, also remove Model::m_aabb and the corresponding getter and setter, as it isn't immediately obvious whether this gets the model asset bbox or the mesh instance bbox. Callers should instead be explicit about which bbox they want. * Bug fix: model asset is not necessarily ready in AcquireMesh * Remove now-unused forward declaration * Update MockMeshFeatureProcessor with SetLocalAabb/GetLocalAabb --- .../Atom/Feature/Mesh/MeshFeatureProcessor.h | 5 ++++ .../Mesh/MeshFeatureProcessorInterface.h | 4 +++ .../Code/Mocks/MockMeshFeatureProcessor.h | 2 ++ .../Code/Source/Mesh/MeshFeatureProcessor.cpp | 30 +++++++++++++++++-- .../Include/Atom/RPI.Public/Model/Model.h | 5 +--- .../Code/Source/RPI.Public/Model/Model.cpp | 16 +++++----- .../Source/RPI.Public/Model/ModelLodUtils.cpp | 2 +- .../Source/Mesh/MeshComponentController.cpp | 6 ++-- .../Code/Source/AtomActorInstance.cpp | 12 ++++++-- 9 files changed, 62 insertions(+), 20 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 0d61ef82d1..24c0658c61 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 @@ -95,6 +95,8 @@ namespace AZ TransformServiceFeatureProcessorInterface::ObjectId m_objectId; + Aabb m_aabb = Aabb::CreateNull(); + bool m_cullBoundsNeedsUpdate = false; bool m_cullableNeedsRebuild = false; bool m_objectSrgNeedsUpdate = true; @@ -160,6 +162,9 @@ namespace AZ Transform GetTransform(const MeshHandle& meshHandle) override; Vector3 GetNonUniformScale(const MeshHandle& meshHandle) override; + void SetLocalAabb(const MeshHandle& meshHandle, const AZ::Aabb& localAabb) override; + AZ::Aabb GetLocalAabb(const MeshHandle& meshHandle) const override; + void SetSortKey(const MeshHandle& meshHandle, RHI::DrawItemSortKey sortKey) override; RHI::DrawItemSortKey GetSortKey(const MeshHandle& meshHandle) override; diff --git a/Gems/Atom/Feature/Common/Code/Include/Atom/Feature/Mesh/MeshFeatureProcessorInterface.h b/Gems/Atom/Feature/Common/Code/Include/Atom/Feature/Mesh/MeshFeatureProcessorInterface.h index fb5bff5584..fdc6ea3cc8 100644 --- a/Gems/Atom/Feature/Common/Code/Include/Atom/Feature/Mesh/MeshFeatureProcessorInterface.h +++ b/Gems/Atom/Feature/Common/Code/Include/Atom/Feature/Mesh/MeshFeatureProcessorInterface.h @@ -86,6 +86,10 @@ namespace AZ virtual Transform GetTransform(const MeshHandle& meshHandle) = 0; //! Gets the non-uniform scale for a given mesh handle. virtual Vector3 GetNonUniformScale(const MeshHandle& meshHandle) = 0; + //! Sets the local space bbox for a given mesh handle. You don't need to call this for static models, only skinned/animated models + virtual void SetLocalAabb(const MeshHandle& meshHandle, const AZ::Aabb& localAabb) = 0; + //! Gets the local space bbox for a given mesh handle. Unless SetLocalAabb has been called before, this will be the bbox of the model asset + virtual AZ::Aabb GetLocalAabb(const MeshHandle& meshHandle) const = 0; //! Sets the sort key for a given mesh handle. virtual void SetSortKey(const MeshHandle& meshHandle, RHI::DrawItemSortKey sortKey) = 0; //! Gets the sort key for a given mesh handle. diff --git a/Gems/Atom/Feature/Common/Code/Mocks/MockMeshFeatureProcessor.h b/Gems/Atom/Feature/Common/Code/Mocks/MockMeshFeatureProcessor.h index 418ee0cfb8..e6635906e9 100644 --- a/Gems/Atom/Feature/Common/Code/Mocks/MockMeshFeatureProcessor.h +++ b/Gems/Atom/Feature/Common/Code/Mocks/MockMeshFeatureProcessor.h @@ -33,6 +33,8 @@ namespace UnitTest MOCK_METHOD2(SetMaterialAssignmentMap, void(const MeshHandle&, const AZ::Render::MaterialAssignmentMap&)); MOCK_METHOD1(GetTransform, AZ::Transform(const MeshHandle&)); MOCK_METHOD1(GetNonUniformScale, AZ::Vector3(const MeshHandle&)); + MOCK_METHOD2(SetLocalAabb, void(const MeshHandle&, const AZ::Aabb&)); + MOCK_CONST_METHOD1(GetLocalAabb, AZ::Aabb(const MeshHandle&)); MOCK_METHOD2(SetSortKey, void (const MeshHandle&, AZ::RHI::DrawItemSortKey)); MOCK_METHOD1(GetSortKey, AZ::RHI::DrawItemSortKey(const MeshHandle&)); MOCK_METHOD2(SetLodOverride, void(const MeshHandle&, AZ::RPI::Cullable::LodOverride)); diff --git a/Gems/Atom/Feature/Common/Code/Source/Mesh/MeshFeatureProcessor.cpp b/Gems/Atom/Feature/Common/Code/Source/Mesh/MeshFeatureProcessor.cpp index b087269089..6fe6cf1f88 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Mesh/MeshFeatureProcessor.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/Mesh/MeshFeatureProcessor.cpp @@ -304,6 +304,30 @@ namespace AZ } } + void MeshFeatureProcessor::SetLocalAabb(const MeshHandle& meshHandle, const AZ::Aabb& localAabb) + { + if (meshHandle.IsValid()) + { + MeshDataInstance& meshData = *meshHandle; + meshData.m_aabb = localAabb; + meshData.m_cullBoundsNeedsUpdate = true; + meshData.m_objectSrgNeedsUpdate = true; + } + }; + + AZ::Aabb MeshFeatureProcessor::GetLocalAabb(const MeshHandle& meshHandle) const + { + if (meshHandle.IsValid()) + { + return meshHandle->m_aabb; + } + else + { + AZ_Assert(false, "Invalid mesh handle"); + return Aabb::CreateNull(); + } + } + Transform MeshFeatureProcessor::GetTransform(const MeshHandle& meshHandle) { if (meshHandle.IsValid()) @@ -603,6 +627,8 @@ namespace AZ SetRayTracingData(); } + m_aabb = model->GetModelAsset()->GetAabb(); + m_cullableNeedsRebuild = true; m_cullBoundsNeedsUpdate = true; m_objectSrgNeedsUpdate = true; @@ -996,7 +1022,7 @@ namespace AZ RPI::Cullable::CullData& cullData = m_cullable.m_cullData; RPI::Cullable::LodData& lodData = m_cullable.m_lodData; - const Aabb& localAabb = m_model->GetAabb(); + const Aabb& localAabb = m_aabb; lodData.m_lodSelectionRadius = 0.5f*localAabb.GetExtents().GetMaxElement(); const size_t modelLodCount = m_model->GetLodCount(); @@ -1077,7 +1103,7 @@ namespace AZ Vector3 center; float radius; - Aabb localAabb = m_model->GetAabb(); + Aabb localAabb = m_aabb; localAabb.MultiplyByScale(nonUniformScale); localAabb.GetTransformedAabb(localToWorld).GetAsSphere(center, radius); diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Model/Model.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Model/Model.h index 514e3e37a5..b403a86f00 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Model/Model.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Model/Model.h @@ -32,6 +32,7 @@ namespace AZ : public Data::InstanceData { friend class ModelSystem; + public: AZ_INSTANCE_DATA(Model, "{C30F5522-B381-4B38-BBAF-6E0B1885C8B9}"); AZ_CLASS_ALLOCATOR(Model, AZ::SystemAllocator, 0); @@ -53,8 +54,6 @@ namespace AZ //! Returns whether a buffer upload is pending. bool IsUploadPending() const; - const AZ::Aabb& GetAabb() const; - const Data::Asset& GetModelAsset() const; //! Checks a ray for intersection against this model. The ray must be in the same coordinate space as the model. @@ -105,8 +104,6 @@ namespace AZ // Tracks whether buffers have all been streamed up to the GPU. bool m_isUploadPending = false; - - AZ::Aabb m_aabb; }; } // namespace RPI } // namespace AZ diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Model/Model.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Model/Model.cpp index 17ff2c64c8..8809225150 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Model/Model.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Model/Model.cpp @@ -62,8 +62,6 @@ namespace AZ { AZ_PROFILE_FUNCTION(Debug::ProfileCategory::AzRender); - m_aabb = modelAsset.GetAabb(); - m_lods.resize(modelAsset.GetLodAssets().size()); for (size_t lodIndex = 0; lodIndex < m_lods.size(); ++lodIndex) @@ -127,11 +125,6 @@ namespace AZ return m_isUploadPending; } - const AZ::Aabb& Model::GetAabb() const - { - return m_aabb; - } - const Data::Asset& Model::GetModelAsset() const { return m_modelAsset; @@ -140,9 +133,16 @@ namespace AZ bool Model::LocalRayIntersection(const AZ::Vector3& rayStart, const AZ::Vector3& rayDir, float& distanceNormalized, AZ::Vector3& normal) const { AZ_PROFILE_FUNCTION(Debug::ProfileCategory::AzRender); + + if (!GetModelAsset()) + { + AZ_Assert(false, "Invalid Model - not created from a ModelAsset?"); + return false; + } + float start; float end; - const int result = Intersect::IntersectRayAABB2(rayStart, rayDir.GetReciprocal(), m_aabb, start, end); + const int result = Intersect::IntersectRayAABB2(rayStart, rayDir.GetReciprocal(), GetModelAsset()->GetAabb(), start, end); if (Intersect::ISECT_RAY_AABB_NONE != result) { if (ModelAsset* modelAssetPtr = m_modelAsset.Get()) diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Model/ModelLodUtils.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Model/ModelLodUtils.cpp index b213e84bca..a4e5d1ba10 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Model/ModelLodUtils.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Model/ModelLodUtils.cpp @@ -49,7 +49,7 @@ namespace AZ With that percentage we can determine which Lod we want to use. */ - Aabb modelAabb = model.GetAabb(); + Aabb modelAabb = model.GetModelAsset()->GetAabb(); modelAabb.Translate(position); Vector3 center; diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Mesh/MeshComponentController.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Mesh/MeshComponentController.cpp index e7eecd3c7f..90d3763067 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Mesh/MeshComponentController.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Mesh/MeshComponentController.cpp @@ -460,10 +460,10 @@ namespace AZ Aabb MeshComponentController::GetLocalBounds() { - const Data::Instance model = GetModel(); - if (model) + if (m_meshHandle.IsValid() && m_meshFeatureProcessor) { - Aabb aabb = model->GetAabb(); + Aabb aabb = m_meshFeatureProcessor->GetLocalAabb(m_meshHandle); + aabb.MultiplyByScale(m_cachedNonUniformScale); return aabb; } diff --git a/Gems/AtomLyIntegration/EMotionFXAtom/Code/Source/AtomActorInstance.cpp b/Gems/AtomLyIntegration/EMotionFXAtom/Code/Source/AtomActorInstance.cpp index 532c8720b5..ff4b82ce92 100644 --- a/Gems/AtomLyIntegration/EMotionFXAtom/Code/Source/AtomActorInstance.cpp +++ b/Gems/AtomLyIntegration/EMotionFXAtom/Code/Source/AtomActorInstance.cpp @@ -83,12 +83,20 @@ namespace AZ void AtomActorInstance::UpdateBounds() { // Update RenderActorInstance world bounding box - // The bounding box is moving with the actor instance. It is static in the way that it does not change shape. + // The bounding box is moving with the actor instance. // The entity and actor transforms are kept in sync already. m_worldAABB = AZ::Aabb::CreateFromMinMax(m_actorInstance->GetAABB().GetMin(), m_actorInstance->GetAABB().GetMax()); // Update RenderActorInstance local bounding box - m_localAABB = AZ::Aabb::CreateFromMinMax(m_actorInstance->GetStaticBasedAABB().GetMin(), m_actorInstance->GetStaticBasedAABB().GetMax()); + // NB: computing the local bbox from the world bbox makes the local bbox artifically larger than it should be + // instead EMFX should support getting the local bbox from the actor instance directly + m_localAABB = m_worldAABB.GetTransformedAabb(m_transformInterface->GetWorldTM().GetInverse()); + + // Update bbox on mesh instance if it exists + if (m_meshFeatureProcessor && m_meshHandle && m_meshHandle->IsValid() && m_skinnedMeshInstance) + { + m_meshFeatureProcessor->SetLocalAabb(*m_meshHandle, m_localAABB); + } AZ::Interface::Get()->RefreshEntityLocalBoundsUnion(m_entityId); }