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); + } } } }