From 243532c5debc7c65c20b11cb066eee593cbd8485 Mon Sep 17 00:00:00 2001 From: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com> Date: Fri, 22 Oct 2021 14:33:36 -0500 Subject: [PATCH] Addressed feedback from PR 4874. (#4915) * Addressed feedback from PR 4874. * Removed second copy of HeightfieldProviderBus.h from cmake file * Changed CookedMeshShapeConfiguration and HeightfieldShapeConfiguration to have less messy implementations, instead opting for the slightly less messy const_cast inside of Utils.cpp and DebugDraw.cpp. * Changed InitHeightfieldShapeConfiguraiton to CreateHeightfieldShapeConfiguration with a better API signature. Signed-off-by: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com> * Fixed indentation Signed-off-by: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com> --- .../Physics/ShapeConfiguration.cpp | 21 +++++++++--- .../AzFramework/Physics/ShapeConfiguration.h | 14 ++++---- .../AzFramework/azframework_files.cmake | 1 - Gems/PhysX/Code/Editor/DebugDraw.cpp | 6 +++- .../EditorHeightfieldColliderComponent.cpp | 4 +-- .../Source/HeightfieldColliderComponent.cpp | 2 +- Gems/PhysX/Code/Source/Utils.cpp | 32 +++++++++++++------ Gems/PhysX/Code/Source/Utils.h | 2 +- 8 files changed, 54 insertions(+), 28 deletions(-) diff --git a/Code/Framework/AzFramework/AzFramework/Physics/ShapeConfiguration.cpp b/Code/Framework/AzFramework/AzFramework/Physics/ShapeConfiguration.cpp index 9bf00d9707..db8004d83a 100644 --- a/Code/Framework/AzFramework/AzFramework/Physics/ShapeConfiguration.cpp +++ b/Code/Framework/AzFramework/AzFramework/Physics/ShapeConfiguration.cpp @@ -286,12 +286,17 @@ namespace Physics return m_type; } - void* CookedMeshShapeConfiguration::GetCachedNativeMesh() const + const void* CookedMeshShapeConfiguration::GetCachedNativeMesh() const { return m_cachedNativeMesh; } - void CookedMeshShapeConfiguration::SetCachedNativeMesh(void* cachedNativeMesh) const + void* CookedMeshShapeConfiguration::GetCachedNativeMesh() + { + return m_cachedNativeMesh; + } + + void CookedMeshShapeConfiguration::SetCachedNativeMesh(void* cachedNativeMesh) { m_cachedNativeMesh = cachedNativeMesh; } @@ -353,12 +358,17 @@ namespace Physics return *this; } - void* HeightfieldShapeConfiguration::GetCachedNativeHeightfield() const + const void* HeightfieldShapeConfiguration::GetCachedNativeHeightfield() const { return m_cachedNativeHeightfield; } - void HeightfieldShapeConfiguration::SetCachedNativeHeightfield(void* cachedNativeHeightfield) const + void* HeightfieldShapeConfiguration::GetCachedNativeHeightfield() + { + return m_cachedNativeHeightfield; + } + + void HeightfieldShapeConfiguration::SetCachedNativeHeightfield(void* cachedNativeHeightfield) { if (m_cachedNativeHeightfield) { @@ -427,4 +437,5 @@ namespace Physics { m_maxHeightBounds = maxBounds; } -} +} // namespace Physics + diff --git a/Code/Framework/AzFramework/AzFramework/Physics/ShapeConfiguration.h b/Code/Framework/AzFramework/AzFramework/Physics/ShapeConfiguration.h index 3bcf336af6..bd9d6a6aa7 100644 --- a/Code/Framework/AzFramework/AzFramework/Physics/ShapeConfiguration.h +++ b/Code/Framework/AzFramework/AzFramework/Physics/ShapeConfiguration.h @@ -187,8 +187,9 @@ namespace Physics MeshType GetMeshType() const; - void* GetCachedNativeMesh() const; - void SetCachedNativeMesh(void* cachedNativeMesh) const; + void* GetCachedNativeMesh(); + const void* GetCachedNativeMesh() const; + void SetCachedNativeMesh(void* cachedNativeMesh); private: void ReleaseCachedNativeMesh(); @@ -197,7 +198,7 @@ namespace Physics MeshType m_type = MeshType::TriangleMesh; //! Cached native mesh object (e.g. PxConvexMesh or PxTriangleMesh). This data is not serialized. - mutable void* m_cachedNativeMesh = nullptr; + void* m_cachedNativeMesh = nullptr; }; class HeightfieldShapeConfiguration @@ -217,8 +218,9 @@ namespace Physics return ShapeType::Heightfield; } - void* GetCachedNativeHeightfield() const; - void SetCachedNativeHeightfield(void* cachedNativeHeightfield) const; + const void* GetCachedNativeHeightfield() const; + void* GetCachedNativeHeightfield(); + void SetCachedNativeHeightfield(void* cachedNativeHeightfield); AZ::Vector2 GetGridResolution() const; void SetGridResolution(const AZ::Vector2& gridSpacing); int32_t GetNumColumns() const; @@ -246,6 +248,6 @@ namespace Physics //! The grid of sample points for the heightfield. AZStd::vector m_samples; //! An optional storage pointer for the physics system to cache its native heightfield representation. - mutable void* m_cachedNativeHeightfield{ nullptr }; + void* m_cachedNativeHeightfield{ nullptr }; }; } // namespace Physics diff --git a/Code/Framework/AzFramework/AzFramework/azframework_files.cmake b/Code/Framework/AzFramework/AzFramework/azframework_files.cmake index d3196e8517..e03d166cfc 100644 --- a/Code/Framework/AzFramework/AzFramework/azframework_files.cmake +++ b/Code/Framework/AzFramework/AzFramework/azframework_files.cmake @@ -252,7 +252,6 @@ set(FILES Physics/Shape.h Physics/ShapeConfiguration.h Physics/ShapeConfiguration.cpp - Physics/HeightfieldProviderBus.h Physics/SystemBus.h Physics/ColliderComponentBus.h Physics/RagdollPhysicsBus.h diff --git a/Gems/PhysX/Code/Editor/DebugDraw.cpp b/Gems/PhysX/Code/Editor/DebugDraw.cpp index 734557205a..90a1eb6004 100644 --- a/Gems/PhysX/Code/Editor/DebugDraw.cpp +++ b/Gems/PhysX/Code/Editor/DebugDraw.cpp @@ -264,7 +264,11 @@ namespace PhysX case Physics::ShapeType::CookedMesh: { const auto& cookedMeshConfig = static_cast(shapeConfig); - physx::PxBase* meshData = static_cast(cookedMeshConfig.GetCachedNativeMesh()); + const physx::PxBase* constMeshData = static_cast(cookedMeshConfig.GetCachedNativeMesh()); + + // Specifically removing the const from the meshData pointer because the physx APIs expect this pointer to be non-const. + physx::PxBase* meshData = const_cast(constMeshData); + if (meshData) { if (meshData->is()) diff --git a/Gems/PhysX/Code/Source/EditorHeightfieldColliderComponent.cpp b/Gems/PhysX/Code/Source/EditorHeightfieldColliderComponent.cpp index 96b1bd51f5..0f09258524 100644 --- a/Gems/PhysX/Code/Source/EditorHeightfieldColliderComponent.cpp +++ b/Gems/PhysX/Code/Source/EditorHeightfieldColliderComponent.cpp @@ -201,9 +201,7 @@ namespace PhysX void EditorHeightfieldColliderComponent::InitHeightfieldShapeConfiguration() { - Physics::HeightfieldShapeConfiguration& configuration = static_cast(*m_shapeConfig); - - Utils::InitHeightfieldShapeConfiguration(GetEntityId(), configuration); + *m_shapeConfig = Utils::CreateHeightfieldShapeConfiguration(GetEntityId()); } void EditorHeightfieldColliderComponent::RefreshHeightfield() diff --git a/Gems/PhysX/Code/Source/HeightfieldColliderComponent.cpp b/Gems/PhysX/Code/Source/HeightfieldColliderComponent.cpp index 9bc209982d..c1fa09f21f 100644 --- a/Gems/PhysX/Code/Source/HeightfieldColliderComponent.cpp +++ b/Gems/PhysX/Code/Source/HeightfieldColliderComponent.cpp @@ -139,7 +139,7 @@ namespace PhysX { Physics::HeightfieldShapeConfiguration& configuration = static_cast(*m_shapeConfig.second); - Utils::InitHeightfieldShapeConfiguration(GetEntityId(), configuration); + configuration = Utils::CreateHeightfieldShapeConfiguration(GetEntityId()); } void HeightfieldColliderComponent::RefreshHeightfield() diff --git a/Gems/PhysX/Code/Source/Utils.cpp b/Gems/PhysX/Code/Source/Utils.cpp index 72e17aff70..b4e1e768c3 100644 --- a/Gems/PhysX/Code/Source/Utils.cpp +++ b/Gems/PhysX/Code/Source/Utils.cpp @@ -67,7 +67,7 @@ namespace PhysX } void CreatePxGeometryFromHeightfield( - const Physics::HeightfieldShapeConfiguration& heightfieldConfig, physx::PxGeometryHolder& pxGeometry) + Physics::HeightfieldShapeConfiguration& heightfieldConfig, physx::PxGeometryHolder& pxGeometry) { physx::PxHeightField* heightfield = nullptr; @@ -264,9 +264,14 @@ namespace PhysX } case Physics::ShapeType::CookedMesh: { - const Physics::CookedMeshShapeConfiguration& cookedMeshShapeConfig = + const Physics::CookedMeshShapeConfiguration& constCookedMeshShapeConfig = static_cast(shapeConfiguration); + // We are deliberately removing the const off of the ShapeConfiguration here because we're going to change the cached + // native mesh pointer that gets stored in the configuration. + Physics::CookedMeshShapeConfiguration& cookedMeshShapeConfig = + const_cast(constCookedMeshShapeConfig); + physx::PxBase* nativeMeshObject = nullptr; // Use the cached mesh object if it is there, otherwise create one and save in the shape configuration @@ -303,13 +308,18 @@ namespace PhysX return false; } case Physics::ShapeType::Heightfield: - { - const Physics::HeightfieldShapeConfiguration& heightfieldConfig = - static_cast(shapeConfiguration); + { + const Physics::HeightfieldShapeConfiguration& constHeightfieldConfig = + static_cast(shapeConfiguration); - CreatePxGeometryFromHeightfield(heightfieldConfig, pxGeometry); - break; - } + // We are deliberately removing the const off of the ShapeConfiguration here because we're going to change the cached + // native heightfield pointer that gets stored in the configuration. + Physics::HeightfieldShapeConfiguration& heightfieldConfig = + const_cast(constHeightfieldConfig); + + CreatePxGeometryFromHeightfield(heightfieldConfig, pxGeometry); + break; + } default: AZ_Warning("PhysX Rigid Body", false, "Shape not supported in PhysX. Shape Type: %d", shapeType); return false; @@ -1518,9 +1528,9 @@ namespace PhysX return entityWorldTransformWithoutScale * jointLocalTransformWithoutScale; } - void InitHeightfieldShapeConfiguration(AZ::EntityId entityId, Physics::HeightfieldShapeConfiguration& configuration) + Physics::HeightfieldShapeConfiguration CreateHeightfieldShapeConfiguration(AZ::EntityId entityId) { - configuration = Physics::HeightfieldShapeConfiguration(); + Physics::HeightfieldShapeConfiguration configuration; AZ::Vector2 gridSpacing(1.0f); Physics::HeightfieldProviderRequestsBus::EventResult( @@ -1549,6 +1559,8 @@ namespace PhysX samples, entityId, &Physics::HeightfieldProviderRequestsBus::Events::GetHeightsAndMaterials); configuration.SetSamples(samples); + + return configuration; } } // namespace Utils diff --git a/Gems/PhysX/Code/Source/Utils.h b/Gems/PhysX/Code/Source/Utils.h index 54a81bb28d..525db03315 100644 --- a/Gems/PhysX/Code/Source/Utils.h +++ b/Gems/PhysX/Code/Source/Utils.h @@ -188,7 +188,7 @@ namespace PhysX //! Returns defaultValue if the input is infinite or NaN, otherwise returns the input unchanged. const AZ::Vector3& Sanitize(const AZ::Vector3& input, const AZ::Vector3& defaultValue = AZ::Vector3::CreateZero()); - void InitHeightfieldShapeConfiguration(AZ::EntityId entityId, Physics::HeightfieldShapeConfiguration& configuration); + Physics::HeightfieldShapeConfiguration CreateHeightfieldShapeConfiguration(AZ::EntityId entityId); namespace Geometry {