From c31e3c020898816b1a2eee355af202e7c952c56c Mon Sep 17 00:00:00 2001 From: Sergey Pereslavtsev Date: Thu, 20 Jan 2022 19:36:33 +0000 Subject: [PATCH 1/3] LYN-7640 Terrain Physics Materials hooked up all the way down to PhysX Signed-off-by: Sergey Pereslavtsev --- .../EditorHeightfieldColliderComponent.cpp | 3 + .../Source/HeightfieldColliderComponent.cpp | 4 + Gems/PhysX/Code/Source/Utils.cpp | 132 ++++++----- Gems/PhysX/Code/Source/Utils.h | 7 + ...ditorHeightfieldColliderComponentTests.cpp | 208 ++++++++++++++++-- Gems/PhysX/Code/Tests/EditorTestUtilities.cpp | 18 ++ Gems/PhysX/Code/Tests/EditorTestUtilities.h | 3 + 7 files changed, 302 insertions(+), 73 deletions(-) diff --git a/Gems/PhysX/Code/Source/EditorHeightfieldColliderComponent.cpp b/Gems/PhysX/Code/Source/EditorHeightfieldColliderComponent.cpp index fb0a38fa18..c6def5f0ab 100644 --- a/Gems/PhysX/Code/Source/EditorHeightfieldColliderComponent.cpp +++ b/Gems/PhysX/Code/Source/EditorHeightfieldColliderComponent.cpp @@ -189,6 +189,9 @@ namespace PhysX configuration.m_entityId = GetEntityId(); configuration.m_debugName = GetEntity()->GetName(); + // Update material selection from the mapping + Utils::SetMaterialsFromHeightfieldProvider(GetEntityId(), m_colliderConfig.m_materialSelection); + AzPhysics::ShapeColliderPairList colliderShapePairs; colliderShapePairs.emplace_back(AZStd::make_shared(m_colliderConfig), m_shapeConfig); configuration.m_colliderAndShapeData = colliderShapePairs; diff --git a/Gems/PhysX/Code/Source/HeightfieldColliderComponent.cpp b/Gems/PhysX/Code/Source/HeightfieldColliderComponent.cpp index c1fa09f21f..b219253b6f 100644 --- a/Gems/PhysX/Code/Source/HeightfieldColliderComponent.cpp +++ b/Gems/PhysX/Code/Source/HeightfieldColliderComponent.cpp @@ -140,6 +140,10 @@ namespace PhysX Physics::HeightfieldShapeConfiguration& configuration = static_cast(*m_shapeConfig.second); configuration = Utils::CreateHeightfieldShapeConfiguration(GetEntityId()); + + // Update material selection from the mapping + Physics::ColliderConfiguration* colliderConfig = m_shapeConfig.first.get(); + Utils::SetMaterialsFromHeightfieldProvider(GetEntityId(), colliderConfig->m_materialSelection); } void HeightfieldColliderComponent::RefreshHeightfield() diff --git a/Gems/PhysX/Code/Source/Utils.cpp b/Gems/PhysX/Code/Source/Utils.cpp index fecea57d9c..a988f6791c 100644 --- a/Gems/PhysX/Code/Source/Utils.cpp +++ b/Gems/PhysX/Code/Source/Utils.cpp @@ -66,6 +66,67 @@ namespace PhysX } } + AZStd::pair GetPhysXMaterialIndicesFromHeightfieldSamples( + const AZStd::vector& samples, + const int32_t row, const int32_t col, + const int32_t numRows, const int32_t numCols) + { + + uint8_t materialIndex0 = 0; + uint8_t materialIndex1 = 0; + + const bool lastRowIndex = (row == (numRows - 1)); + const bool lastColumnIndex = (col == (numCols - 1)); + + // In PhysX, the material indices refer to the quad down and to the right of the sample. + // If we're in the last row or last column, there aren't any quads down or to the right, + // so just clear these out. + + if (!lastRowIndex && !lastColumnIndex) + { + auto GetIndex = [numCols](int32_t row, int32_t col) + { + return (row * numCols) + col; + }; + + // Our source data is providing one material index per vertex, but PhysX wants one material index + // per triangle. The heuristic that we'll go with for selecting the material index is to choose + // the material for the vertex that's not on the diagonal of each triangle. + // Ex: A *---* B + // | / | For this, we'll use A for index0 and D for index1. + // C *---* D + // + // Ex: A *---* B + // | \ | For this, we'll use C for index0 and B for index1. + // C *---* D + // + // This is a pretty arbitrary choice, so the heuristic might need to be revisited over time if this + // causes incorrect or unpredictable physics material mappings. + + const Physics::HeightMaterialPoint& currentSample = samples[GetIndex(row, col)]; + + switch (currentSample.m_quadMeshType) + { + case Physics::QuadMeshType::SubdivideUpperLeftToBottomRight: + materialIndex0 = samples[GetIndex(row + 1, col)].m_materialIndex; + materialIndex1 = samples[GetIndex(row, col + 1)].m_materialIndex; + break; + case Physics::QuadMeshType::SubdivideBottomLeftToUpperRight: + materialIndex0 = currentSample.m_materialIndex; + materialIndex1 = samples[GetIndex(row + 1, col + 1)].m_materialIndex; + break; + case Physics::QuadMeshType::Hole: + materialIndex0 = physx::PxHeightFieldMaterial::eHOLE; + materialIndex1 = physx::PxHeightFieldMaterial::eHOLE; + break; + default: + AZ_Assert(false, "Unhandled case in GetPhysXMaterialIndicesFromHeightfieldSamples"); + break; + } + } + return { materialIndex0, materialIndex1 }; + } + void CreatePxGeometryFromHeightfield( Physics::HeightfieldShapeConfiguration& heightfieldConfig, physx::PxGeometryHolder& pxGeometry) { @@ -116,12 +177,8 @@ namespace PhysX for (int32_t row = 0; row < numRows; row++) { - const bool lastRowIndex = (row == (numRows - 1)); - for (int32_t col = 0; col < numCols; col++) { - const bool lastColumnIndex = (col == (numCols - 1)); - auto GetIndex = [numCols](int32_t row, int32_t col) { return (row * numCols) + col; @@ -134,52 +191,15 @@ namespace PhysX AZ_Assert(currentSample.m_materialIndex < physxMaximumMaterialIndex, "MaterialIndex must be less than 128"); currentPhysxSample.height = azlossy_cast( AZ::GetClamp(currentSample.m_height, minHeightBounds, maxHeightBounds) * scaleFactor); - if (lastRowIndex || lastColumnIndex) - { - // In PhysX, the material indices refer to the quad down and to the right of the sample. - // If we're in the last row or last column, there aren't any quads down or to the right, - // so just clear these out. - currentPhysxSample.materialIndex0 = 0; - currentPhysxSample.materialIndex1 = 0; - } - else - { - // Our source data is providing one material index per vertex, but PhysX wants one material index - // per triangle. The heuristic that we'll go with for selecting the material index is to choose - // the material for the vertex that's not on the diagonal of each triangle. - // Ex: A *---* B - // | / | For this, we'll use A for index0 and D for index1. - // C *---* D - // - // Ex: A *---* B - // | \ | For this, we'll use C for index0 and B for index1. - // C *---* D - // - // This is a pretty arbitrary choice, so the heuristic might need to be revisited over time if this - // causes incorrect or unpredictable physics material mappings. - switch (currentSample.m_quadMeshType) - { - case Physics::QuadMeshType::SubdivideUpperLeftToBottomRight: - currentPhysxSample.materialIndex0 = samples[GetIndex(row + 1, col)].m_materialIndex; - currentPhysxSample.materialIndex1 = samples[GetIndex(row, col + 1)].m_materialIndex; - // Set the tesselation flag to say that we need to go from UL to BR - currentPhysxSample.materialIndex0.setBit(); - break; - case Physics::QuadMeshType::SubdivideBottomLeftToUpperRight: - currentPhysxSample.materialIndex0 = currentSample.m_materialIndex; - currentPhysxSample.materialIndex1 = samples[GetIndex(row + 1, col + 1)].m_materialIndex; - break; - case Physics::QuadMeshType::Hole: - currentPhysxSample.materialIndex0 = physx::PxHeightFieldMaterial::eHOLE; - currentPhysxSample.materialIndex1 = physx::PxHeightFieldMaterial::eHOLE; - break; - default: - AZ_Warning("PhysX Heightfield", false, "Unhandled case in CreatePxGeometryFromConfig"); - currentPhysxSample.materialIndex0 = 0; - currentPhysxSample.materialIndex1 = 0; - break; - } + auto [materialIndex0, materialIndex1] = GetPhysXMaterialIndicesFromHeightfieldSamples(samples, row, col, numRows, numCols); + currentPhysxSample.materialIndex0 = materialIndex0; + currentPhysxSample.materialIndex1 = materialIndex1; + + if (currentSample.m_quadMeshType == Physics::QuadMeshType::SubdivideUpperLeftToBottomRight) + { + // Set the tesselation flag to say that we need to go from UL to BR + currentPhysxSample.setTessFlag(); } } } @@ -1562,6 +1582,20 @@ namespace PhysX return configuration; } + + void SetMaterialsFromHeightfieldProvider(const AZ::EntityId& heightfieldProviderId, Physics::MaterialSelection& materialSelection) + { + AZStd::vector materialList; + Physics::HeightfieldProviderRequestsBus::EventResult( + materialList, heightfieldProviderId, &Physics::HeightfieldProviderRequestsBus::Events::GetMaterialList); + + materialSelection.SetMaterialSlots(Physics::MaterialSelection::SlotsArray(materialList.size(), "")); + + for (int i = 0; i < materialList.size(); ++i) + { + materialSelection.SetMaterialId(materialList[i], i); + } + } } // namespace Utils namespace ReflectionUtils diff --git a/Gems/PhysX/Code/Source/Utils.h b/Gems/PhysX/Code/Source/Utils.h index 525db03315..5244926aa2 100644 --- a/Gems/PhysX/Code/Source/Utils.h +++ b/Gems/PhysX/Code/Source/Utils.h @@ -188,8 +188,15 @@ 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()); + AZStd::pair GetPhysXMaterialIndicesFromHeightfieldSamples( + const AZStd::vector& samples, + const int32_t row, const int32_t col, + const int32_t numRows, const int32_t numCols); + Physics::HeightfieldShapeConfiguration CreateHeightfieldShapeConfiguration(AZ::EntityId entityId); + void SetMaterialsFromHeightfieldProvider(const AZ::EntityId& heightfieldProviderId, Physics::MaterialSelection& materialSelection); + namespace Geometry { using PointList = AZStd::vector; diff --git a/Gems/PhysX/Code/Tests/EditorHeightfieldColliderComponentTests.cpp b/Gems/PhysX/Code/Tests/EditorHeightfieldColliderComponentTests.cpp index ef112d8623..9bdc63e60d 100644 --- a/Gems/PhysX/Code/Tests/EditorHeightfieldColliderComponentTests.cpp +++ b/Gems/PhysX/Code/Tests/EditorHeightfieldColliderComponentTests.cpp @@ -19,6 +19,7 @@ #include #include #include +#include using ::testing::NiceMock; using ::testing::Return; @@ -27,18 +28,28 @@ namespace PhysXEditorTests { AZStd::vector GetSamples() { - AZStd::vector samples{ { 3.0f, Physics::QuadMeshType::SubdivideUpperLeftToBottomRight }, - { 2.0f, Physics::QuadMeshType::SubdivideUpperLeftToBottomRight }, - { 1.5f, Physics::QuadMeshType::SubdivideUpperLeftToBottomRight }, - { 1.0f, Physics::QuadMeshType::SubdivideUpperLeftToBottomRight }, - { 3.0f, Physics::QuadMeshType::SubdivideUpperLeftToBottomRight }, - { 1.0f, Physics::QuadMeshType::SubdivideUpperLeftToBottomRight }, - { 3.0f, Physics::QuadMeshType::SubdivideUpperLeftToBottomRight }, - { 0.0f, Physics::QuadMeshType::SubdivideUpperLeftToBottomRight }, - { 3.0f, Physics::QuadMeshType::SubdivideUpperLeftToBottomRight } }; + AZStd::vector samples{ { 3.0f, Physics::QuadMeshType::SubdivideUpperLeftToBottomRight, 0 }, + { 2.0f, Physics::QuadMeshType::SubdivideUpperLeftToBottomRight, 1 }, + { 1.5f, Physics::QuadMeshType::SubdivideUpperLeftToBottomRight, 2 }, + { 1.0f, Physics::QuadMeshType::SubdivideUpperLeftToBottomRight, 0 }, + { 3.0f, Physics::QuadMeshType::SubdivideUpperLeftToBottomRight, 1 }, + { 1.0f, Physics::QuadMeshType::SubdivideUpperLeftToBottomRight, 2 }, + { 3.0f, Physics::QuadMeshType::SubdivideUpperLeftToBottomRight, 0 }, + { 0.0f, Physics::QuadMeshType::SubdivideUpperLeftToBottomRight, 1 }, + { 3.0f, Physics::QuadMeshType::SubdivideUpperLeftToBottomRight, 2 } }; return samples; } + AZStd::vector GetMaterialList() + { + AZStd::vector materials{ + {Physics::MaterialId::FromUUID("{EC976D51-2C26-4C1E-BBF2-75BAAAFA162C}")}, + {Physics::MaterialId::FromUUID("{B9836F51-A235-4781-95E3-A6302BEE9EFF}")}, + {Physics::MaterialId::FromUUID("{7E060707-BB03-47EB-B046-4503C7145B6E}")} + }; + return materials; + } + EntityPtr SetupHeightfieldComponent() { // create an editor entity with a shape collider component and a box shape component @@ -78,6 +89,7 @@ namespace PhysXEditorTests x = -3.0f; y = 3.0f; }); + ON_CALL(mockShapeRequests, GetMaterialList).WillByDefault(Return(GetMaterialList())); } EntityPtr TestCreateActiveGameEntityFromEditorEntity(AZ::Entity* editorEntity) @@ -89,6 +101,89 @@ namespace PhysXEditorTests return gameEntity; } + class PhysXEditorHeightfieldFixture : public PhysXEditorFixture + { + public: + void SetUp() override + { + PhysXEditorFixture::SetUp(); + PopulateDefaultMaterialLibrary(); + + m_editorEntity = SetupHeightfieldComponent(); + m_editorMockShapeRequests = AZStd::make_unique>(m_editorEntity->GetId()); + SetupMockMethods(*m_editorMockShapeRequests.get()); + m_editorEntity->Activate(); + + m_gameEntity = TestCreateActiveGameEntityFromEditorEntity(m_editorEntity.get()); + m_gameMockShapeRequests = AZStd::make_unique>(m_gameEntity->GetId()); + SetupMockMethods(*m_gameMockShapeRequests.get()); + m_gameEntity->Activate(); + } + + void TearDown() override + { + CleanupHeightfieldComponent(); + + m_editorEntity = nullptr; + m_gameEntity = nullptr; + m_editorMockShapeRequests = nullptr; + m_gameMockShapeRequests = nullptr; + + PhysXEditorFixture::TearDown(); + } + + void PopulateDefaultMaterialLibrary() + { + AZ::Data::AssetId assetId = AZ::Data::AssetId(AZ::Uuid::Create()); + + // Create an asset out of our Script Event + Physics::MaterialLibraryAsset* matLibAsset = aznew Physics::MaterialLibraryAsset; + { + AZStd::vector matIds = GetMaterialList(); + + for (Physics::MaterialId matId : matIds) + { + Physics::MaterialFromAssetConfiguration matConfig; + matConfig.m_id = matId; + matConfig.m_configuration.m_surfaceType = matId.GetUuid().ToString(); + matLibAsset->AddMaterialData(matConfig); + } + } + + // Note: There is no interface to simply update material library asset. It has to go via updating the entire configuration which causes assets reloading. + // It makes sense as a safety mechanism in the Editor but makes it harder to write tests. + // Hence have to work around it via const_cast here to be able to simply set the generated asset into configuration. + AzPhysics::SystemConfiguration* sysConfig = const_cast(AZ::Interface::Get()->GetConfiguration()); + + AZ::Data::Asset assetData(assetId, matLibAsset, AZ::Data::AssetLoadBehavior::Default); + sysConfig->m_materialLibraryAsset = assetData; + } + + Physics::Material* GetMaterialFromRaycast(float x, float y) + { + AzPhysics::RayCastRequest request; + request.m_start = AZ::Vector3(x, y, 5.0f); + request.m_direction = AZ::Vector3(0.0f, 0.0f, -1.0f); + request.m_distance = 10.0f; + + //query the scene + auto* sceneInterface = AZ::Interface::Get(); + AzPhysics::SceneQueryHits result = sceneInterface->QueryScene(m_defaultSceneHandle, &request); + EXPECT_EQ(result.m_hits.size(), 1); + + if (result) + { + return result.m_hits[0].m_material; + } + + return nullptr; + }; + + EntityPtr m_editorEntity; + EntityPtr m_gameEntity; + AZStd::unique_ptr> m_editorMockShapeRequests; + AZStd::unique_ptr> m_gameMockShapeRequests; + }; TEST_F(PhysXEditorFixture, EditorHeightfieldColliderComponentDependenciesSatisfiedEntityIsValid) { @@ -149,21 +244,13 @@ namespace PhysXEditorTests CleanupHeightfieldComponent(); } - TEST_F(PhysXEditorFixture, EditorHeightfieldColliderComponentHeightfieldColliderWithAABoxCorrectRuntimeGeometry) + TEST_F(PhysXEditorHeightfieldFixture, EditorHeightfieldColliderComponentHeightfieldColliderWithAABoxCorrectRuntimeGeometry) { - EntityPtr editorEntity = SetupHeightfieldComponent(); - NiceMock mockShapeRequests(editorEntity->GetId()); - SetupMockMethods(mockShapeRequests); - editorEntity->Activate(); - - EntityPtr gameEntity = TestCreateActiveGameEntityFromEditorEntity(editorEntity.get()); - NiceMock mockShapeRequests2(gameEntity->GetId()); - SetupMockMethods(mockShapeRequests2); - gameEntity->Activate(); + AZ::EntityId gameEntityId = m_gameEntity->GetId(); AzPhysics::SimulatedBody* staticBody = nullptr; AzPhysics::SimulatedBodyComponentRequestsBus::EventResult( - staticBody, gameEntity->GetId(), &AzPhysics::SimulatedBodyComponentRequests::GetSimulatedBody); + staticBody, gameEntityId, &AzPhysics::SimulatedBodyComponentRequests::GetSimulatedBody); const auto* pxRigidStatic = static_cast(staticBody->GetNativePointer()); PHYSX_SCENE_READ_LOCK(pxRigidStatic->getScene()); @@ -183,7 +270,7 @@ namespace PhysXEditorTests int32_t numRows{ 0 }; int32_t numColumns{ 0 }; Physics::HeightfieldProviderRequestsBus::Event( - gameEntity->GetId(), &Physics::HeightfieldProviderRequestsBus::Events::GetHeightfieldGridSize, numColumns, numRows); + gameEntityId, &Physics::HeightfieldProviderRequestsBus::Events::GetHeightfieldGridSize, numColumns, numRows); EXPECT_EQ(numColumns, heightfield->getNbColumns()); EXPECT_EQ(numRows, heightfield->getNbRows()); @@ -194,12 +281,12 @@ namespace PhysXEditorTests float minHeightBounds{ 0.0f }; float maxHeightBounds{ 0.0f }; Physics::HeightfieldProviderRequestsBus::Event( - gameEntity->GetId(), &Physics::HeightfieldProviderRequestsBus::Events::GetHeightfieldHeightBounds, minHeightBounds, + gameEntityId, &Physics::HeightfieldProviderRequestsBus::Events::GetHeightfieldHeightBounds, minHeightBounds, maxHeightBounds); AZStd::vector samples; Physics::HeightfieldProviderRequestsBus::EventResult( - samples, gameEntity->GetId(), &Physics::HeightfieldProviderRequestsBus::Events::GetHeightsAndMaterials); + samples, gameEntityId, &Physics::HeightfieldProviderRequestsBus::Events::GetHeightsAndMaterials); const float halfBounds{ (maxHeightBounds - minHeightBounds) / 2.0f }; const float scaleFactor = (maxHeightBounds <= minHeightBounds) ? 1.0f : AZStd::numeric_limits::max() / halfBounds; @@ -208,7 +295,80 @@ namespace PhysXEditorTests EXPECT_EQ(samplePhysX.height, azlossy_cast(samplePhysics.m_height * scaleFactor)); } } - CleanupHeightfieldComponent(); + } + + TEST_F(PhysXEditorHeightfieldFixture, EditorHeightfieldColliderComponentHeightfieldColliderCorrectMaterials) + { + AZ::EntityId gameEntityId = m_gameEntity->GetId(); + + int32_t numRows{ 0 }; + int32_t numColumns{ 0 }; + Physics::HeightfieldProviderRequestsBus::Event( + gameEntityId, &Physics::HeightfieldProviderRequestsBus::Events::GetHeightfieldGridSize, numColumns, numRows); + + EXPECT_EQ(numRows, 3); + EXPECT_EQ(numColumns, 3); + + AZStd::vector samples; + Physics::HeightfieldProviderRequestsBus::EventResult( + samples, gameEntityId, &Physics::HeightfieldProviderRequestsBus::Events::GetHeightsAndMaterials); + + AzPhysics::SimulatedBody* staticBody = nullptr; + AzPhysics::SimulatedBodyComponentRequestsBus::EventResult( + staticBody, gameEntityId, &AzPhysics::SimulatedBodyComponentRequests::GetSimulatedBody); + + const auto* pxRigidStatic = static_cast(staticBody->GetNativePointer()); + PHYSX_SCENE_READ_LOCK(pxRigidStatic->getScene()); + + physx::PxShape* shape = nullptr; + pxRigidStatic->getShapes(&shape, 1, 0); + + physx::PxHeightFieldGeometry heightfieldGeometry; + shape->getHeightFieldGeometry(heightfieldGeometry); + + physx::PxHeightField* heightfield = heightfieldGeometry.heightField; + + AZStd::vector physicsSurfaceTypes; + for (Physics::MaterialId materialId : GetMaterialList()) + { + physicsSurfaceTypes.emplace_back(materialId.GetUuid().ToString()); + } + + // PhysX Heightfield cooking doesn't map 1-1 sample material indices to triangle material indices + // Hence hardcoding the expected material indices in the test + const int physicsMaterialsValidationDataIndex[] = {0, 2, 1, 1}; + + for (int sampleRow = 0; sampleRow < numRows; ++sampleRow) + { + for (int sampleColumn = 0; sampleColumn < numColumns; ++sampleColumn) + { + physx::PxHeightFieldSample samplePhysX = heightfield->getSample(sampleRow, sampleColumn); + Physics::HeightMaterialPoint samplePhysics = samples[sampleRow * numColumns + sampleColumn]; + + auto [materialIndex0, materialIndex1] = PhysX::Utils::GetPhysXMaterialIndicesFromHeightfieldSamples(samples, sampleRow, sampleColumn, numRows, numColumns); + EXPECT_EQ(samplePhysX.materialIndex0, materialIndex0); + EXPECT_EQ(samplePhysX.materialIndex1, materialIndex1); + + if (sampleRow != numRows - 1 && sampleColumn != numColumns - 1) + { + const float x_offset = -0.25f; + const float y_offset = 0.75f; + const float secondRayOffset = 0.5f; + + float rayX = x_offset + sampleColumn; + float rayY = y_offset + sampleRow; + + Physics::Material* mat1 = GetMaterialFromRaycast(rayX, rayY); + EXPECT_NE(mat1, nullptr); + + Physics::Material* mat2 = GetMaterialFromRaycast(rayX + secondRayOffset, rayY + secondRayOffset); + EXPECT_NE(mat2, nullptr); + + AZStd::string expectedMaterialName = physicsSurfaceTypes[physicsMaterialsValidationDataIndex[sampleRow * 2 + sampleColumn]]; + EXPECT_EQ(mat1->GetSurfaceTypeName(), expectedMaterialName); + } + } + } } } // namespace PhysXEditorTests diff --git a/Gems/PhysX/Code/Tests/EditorTestUtilities.cpp b/Gems/PhysX/Code/Tests/EditorTestUtilities.cpp index fa794c58a5..272d9157b2 100644 --- a/Gems/PhysX/Code/Tests/EditorTestUtilities.cpp +++ b/Gems/PhysX/Code/Tests/EditorTestUtilities.cpp @@ -70,6 +70,24 @@ namespace PhysXEditorTests } } + void PhysXEditorFixture::ConnectToPVD() + { + auto* debug = AZ::Interface::Get(); + if (debug) + { + debug->ConnectToPvd(); + } + } + + void PhysXEditorFixture::DisconnectFromPVD() + { + auto* debug = AZ::Interface::Get(); + if (debug) + { + debug->DisconnectFromPvd(); + } + } + // DefaultWorldBus AzPhysics::SceneHandle PhysXEditorFixture::GetDefaultSceneHandle() const { diff --git a/Gems/PhysX/Code/Tests/EditorTestUtilities.h b/Gems/PhysX/Code/Tests/EditorTestUtilities.h index 9dfe40f366..26566f724a 100644 --- a/Gems/PhysX/Code/Tests/EditorTestUtilities.h +++ b/Gems/PhysX/Code/Tests/EditorTestUtilities.h @@ -48,6 +48,9 @@ namespace PhysXEditorTests void SetUp() override; void TearDown() override; + void ConnectToPVD(); + void DisconnectFromPVD(); + // DefaultWorldBus AzPhysics::SceneHandle GetDefaultSceneHandle() const override; From 56c6f81627fcc957f30a46f5e818cad7558c79c8 Mon Sep 17 00:00:00 2001 From: Sergey Pereslavtsev Date: Fri, 21 Jan 2022 13:21:57 +0000 Subject: [PATCH 2/3] PR addressing. Removed RTTI from HeightMaterialPoint since it's just a struct of data with no intention being a part of class hierarchy Signed-off-by: Sergey Pereslavtsev --- .../Physics/HeightfieldProviderBus.h | 5 +- Gems/PhysX/Code/Source/Utils.cpp | 85 ++++++++++--------- ...ditorHeightfieldColliderComponentTests.cpp | 13 +-- 3 files changed, 53 insertions(+), 50 deletions(-) diff --git a/Code/Framework/AzFramework/AzFramework/Physics/HeightfieldProviderBus.h b/Code/Framework/AzFramework/AzFramework/Physics/HeightfieldProviderBus.h index da361f0a2b..585fb59ad6 100644 --- a/Code/Framework/AzFramework/AzFramework/Physics/HeightfieldProviderBus.h +++ b/Code/Framework/AzFramework/AzFramework/Physics/HeightfieldProviderBus.h @@ -35,16 +35,15 @@ namespace Physics { } - virtual ~HeightMaterialPoint() = default; + ~HeightMaterialPoint() = default; static void Reflect(AZ::ReflectContext* context); - AZ_RTTI(HeightMaterialPoint, "{DF167ED4-24E6-4F7B-8AB7-42622F7DBAD3}"); + AZ_TYPE_INFO(HeightMaterialPoint, "{DF167ED4-24E6-4F7B-8AB7-42622F7DBAD3}"); float m_height{ 0.0f }; //!< Holds the height of this point in the heightfield relative to the heightfield entity location. QuadMeshType m_quadMeshType{ QuadMeshType::SubdivideUpperLeftToBottomRight }; //!< By default, create two triangles like this |\|, where this point is in the upper left corner. uint8_t m_materialIndex{ 0 }; //!< The surface material index for the upper left corner of this quad. uint16_t m_padding{ 0 }; //!< available for future use. - }; //! An interface to provide heightfield values. diff --git a/Gems/PhysX/Code/Source/Utils.cpp b/Gems/PhysX/Code/Source/Utils.cpp index a988f6791c..950404820d 100644 --- a/Gems/PhysX/Code/Source/Utils.cpp +++ b/Gems/PhysX/Code/Source/Utils.cpp @@ -71,7 +71,6 @@ namespace PhysX const int32_t row, const int32_t col, const int32_t numRows, const int32_t numCols) { - uint8_t materialIndex0 = 0; uint8_t materialIndex1 = 0; @@ -81,49 +80,51 @@ namespace PhysX // In PhysX, the material indices refer to the quad down and to the right of the sample. // If we're in the last row or last column, there aren't any quads down or to the right, // so just clear these out. - - if (!lastRowIndex && !lastColumnIndex) + if (lastRowIndex || lastColumnIndex) { - auto GetIndex = [numCols](int32_t row, int32_t col) - { - return (row * numCols) + col; - }; - - // Our source data is providing one material index per vertex, but PhysX wants one material index - // per triangle. The heuristic that we'll go with for selecting the material index is to choose - // the material for the vertex that's not on the diagonal of each triangle. - // Ex: A *---* B - // | / | For this, we'll use A for index0 and D for index1. - // C *---* D - // - // Ex: A *---* B - // | \ | For this, we'll use C for index0 and B for index1. - // C *---* D - // - // This is a pretty arbitrary choice, so the heuristic might need to be revisited over time if this - // causes incorrect or unpredictable physics material mappings. - - const Physics::HeightMaterialPoint& currentSample = samples[GetIndex(row, col)]; - - switch (currentSample.m_quadMeshType) - { - case Physics::QuadMeshType::SubdivideUpperLeftToBottomRight: - materialIndex0 = samples[GetIndex(row + 1, col)].m_materialIndex; - materialIndex1 = samples[GetIndex(row, col + 1)].m_materialIndex; - break; - case Physics::QuadMeshType::SubdivideBottomLeftToUpperRight: - materialIndex0 = currentSample.m_materialIndex; - materialIndex1 = samples[GetIndex(row + 1, col + 1)].m_materialIndex; - break; - case Physics::QuadMeshType::Hole: - materialIndex0 = physx::PxHeightFieldMaterial::eHOLE; - materialIndex1 = physx::PxHeightFieldMaterial::eHOLE; - break; - default: - AZ_Assert(false, "Unhandled case in GetPhysXMaterialIndicesFromHeightfieldSamples"); - break; - } + return { materialIndex0, materialIndex1 }; } + + auto GetIndex = [numCols](int32_t row, int32_t col) + { + return (row * numCols) + col; + }; + + // Our source data is providing one material index per vertex, but PhysX wants one material index + // per triangle. The heuristic that we'll go with for selecting the material index is to choose + // the material for the vertex that's not on the diagonal of each triangle. + // Ex: A *---* B + // | / | For this, we'll use A for index0 and D for index1. + // C *---* D + // + // Ex: A *---* B + // | \ | For this, we'll use C for index0 and B for index1. + // C *---* D + // + // This is a pretty arbitrary choice, so the heuristic might need to be revisited over time if this + // causes incorrect or unpredictable physics material mappings. + + const Physics::HeightMaterialPoint& currentSample = samples[GetIndex(row, col)]; + + switch (currentSample.m_quadMeshType) + { + case Physics::QuadMeshType::SubdivideUpperLeftToBottomRight: + materialIndex0 = samples[GetIndex(row + 1, col)].m_materialIndex; + materialIndex1 = samples[GetIndex(row, col + 1)].m_materialIndex; + break; + case Physics::QuadMeshType::SubdivideBottomLeftToUpperRight: + materialIndex0 = currentSample.m_materialIndex; + materialIndex1 = samples[GetIndex(row + 1, col + 1)].m_materialIndex; + break; + case Physics::QuadMeshType::Hole: + materialIndex0 = physx::PxHeightFieldMaterial::eHOLE; + materialIndex1 = physx::PxHeightFieldMaterial::eHOLE; + break; + default: + AZ_Assert(false, "Unhandled case in GetPhysXMaterialIndicesFromHeightfieldSamples"); + break; + } + return { materialIndex0, materialIndex1 }; } diff --git a/Gems/PhysX/Code/Tests/EditorHeightfieldColliderComponentTests.cpp b/Gems/PhysX/Code/Tests/EditorHeightfieldColliderComponentTests.cpp index 9bdc63e60d..9a216b7d80 100644 --- a/Gems/PhysX/Code/Tests/EditorHeightfieldColliderComponentTests.cpp +++ b/Gems/PhysX/Code/Tests/EditorHeightfieldColliderComponentTests.cpp @@ -139,9 +139,9 @@ namespace PhysXEditorTests // Create an asset out of our Script Event Physics::MaterialLibraryAsset* matLibAsset = aznew Physics::MaterialLibraryAsset; { - AZStd::vector matIds = GetMaterialList(); + const AZStd::vector matIds = GetMaterialList(); - for (Physics::MaterialId matId : matIds) + for (const Physics::MaterialId& matId : matIds) { Physics::MaterialFromAssetConfiguration matConfig; matConfig.m_id = matId; @@ -336,7 +336,7 @@ namespace PhysXEditorTests // PhysX Heightfield cooking doesn't map 1-1 sample material indices to triangle material indices // Hence hardcoding the expected material indices in the test - const int physicsMaterialsValidationDataIndex[] = {0, 2, 1, 1}; + const AZStd::array physicsMaterialsValidationDataIndex = {0, 2, 1, 1}; for (int sampleRow = 0; sampleRow < numRows; ++sampleRow) { @@ -364,8 +364,11 @@ namespace PhysXEditorTests Physics::Material* mat2 = GetMaterialFromRaycast(rayX + secondRayOffset, rayY + secondRayOffset); EXPECT_NE(mat2, nullptr); - AZStd::string expectedMaterialName = physicsSurfaceTypes[physicsMaterialsValidationDataIndex[sampleRow * 2 + sampleColumn]]; - EXPECT_EQ(mat1->GetSurfaceTypeName(), expectedMaterialName); + if (mat1) + { + AZStd::string expectedMaterialName = physicsSurfaceTypes[physicsMaterialsValidationDataIndex[sampleRow * 2 + sampleColumn]]; + EXPECT_EQ(mat1->GetSurfaceTypeName(), expectedMaterialName); + } } } } From b21878d04e78e02f430c660b4cc9c9c5bda361a8 Mon Sep 17 00:00:00 2001 From: Sergey Pereslavtsev Date: Fri, 21 Jan 2022 19:57:28 +0000 Subject: [PATCH 3/3] Removed unused variable Signed-off-by: Sergey Pereslavtsev --- .../PhysX/Code/Tests/EditorHeightfieldColliderComponentTests.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/Gems/PhysX/Code/Tests/EditorHeightfieldColliderComponentTests.cpp b/Gems/PhysX/Code/Tests/EditorHeightfieldColliderComponentTests.cpp index 9a216b7d80..5f830acc8a 100644 --- a/Gems/PhysX/Code/Tests/EditorHeightfieldColliderComponentTests.cpp +++ b/Gems/PhysX/Code/Tests/EditorHeightfieldColliderComponentTests.cpp @@ -343,7 +343,6 @@ namespace PhysXEditorTests for (int sampleColumn = 0; sampleColumn < numColumns; ++sampleColumn) { physx::PxHeightFieldSample samplePhysX = heightfield->getSample(sampleRow, sampleColumn); - Physics::HeightMaterialPoint samplePhysics = samples[sampleRow * numColumns + sampleColumn]; auto [materialIndex0, materialIndex1] = PhysX::Utils::GetPhysXMaterialIndicesFromHeightfieldSamples(samples, sampleRow, sampleColumn, numRows, numColumns); EXPECT_EQ(samplePhysX.materialIndex0, materialIndex0);