From 0863a2449086d9ee4a5d7e2ad40dfc2ad348d603 Mon Sep 17 00:00:00 2001 From: Benjamin Jillich <43751992+amzn-jillich@users.noreply.github.com> Date: Thu, 15 Jul 2021 03:18:10 -0700 Subject: [PATCH] Revert "[MeshOptimizer] Determine the original vertex index based on the position (#2008)" (#2188) This reverts commit a077a88d3f7b066fd26446b7e6e821f66dcc9b9a due to assets failing on Jenkins. Signed-off-by: Benjamin Jillich --- .../SceneAPI/SceneData/Groups/MeshGroup.h | 24 +- Gems/SceneProcessing/Code/CMakeLists.txt | 1 - .../MeshOptimizer/MeshOptimizerComponent.cpp | 127 ++-------- .../MeshOptimizerComponentTests.cpp | 221 ------------------ .../sceneprocessing_editor_tests_files.cmake | 1 - 5 files changed, 34 insertions(+), 340 deletions(-) delete mode 100644 Gems/SceneProcessing/Code/Tests/MeshBuilder/MeshOptimizerComponentTests.cpp diff --git a/Code/Tools/SceneAPI/SceneData/Groups/MeshGroup.h b/Code/Tools/SceneAPI/SceneData/Groups/MeshGroup.h index 5b4c4612ad..c06c91a808 100644 --- a/Code/Tools/SceneAPI/SceneData/Groups/MeshGroup.h +++ b/Code/Tools/SceneAPI/SceneData/Groups/MeshGroup.h @@ -28,27 +28,27 @@ namespace AZ } namespace SceneData { - class SCENE_DATA_CLASS MeshGroup + class MeshGroup : public DataTypes::IMeshGroup { public: AZ_RTTI(MeshGroup, "{07B356B7-3635-40B5-878A-FAC4EFD5AD86}", DataTypes::IMeshGroup); AZ_CLASS_ALLOCATOR(MeshGroup, SystemAllocator, 0) - SCENE_DATA_API MeshGroup(); - SCENE_DATA_API ~MeshGroup() override = default; + MeshGroup(); + ~MeshGroup() override = default; - SCENE_DATA_API const AZStd::string& GetName() const override; - SCENE_DATA_API void SetName(const AZStd::string& name); - SCENE_DATA_API void SetName(AZStd::string&& name) override; - SCENE_DATA_API const Uuid& GetId() const override; - SCENE_DATA_API void OverrideId(const Uuid& id) override; + const AZStd::string& GetName() const override; + void SetName(const AZStd::string& name); + void SetName(AZStd::string&& name) override; + const Uuid& GetId() const override; + void OverrideId(const Uuid& id) override; - SCENE_DATA_API Containers::RuleContainer& GetRuleContainer() override; - SCENE_DATA_API const Containers::RuleContainer& GetRuleContainerConst() const override; + Containers::RuleContainer& GetRuleContainer() override; + const Containers::RuleContainer& GetRuleContainerConst() const override; - SCENE_DATA_API DataTypes::ISceneNodeSelectionList& GetSceneNodeSelectionList() override; - SCENE_DATA_API const DataTypes::ISceneNodeSelectionList& GetSceneNodeSelectionList() const override; + DataTypes::ISceneNodeSelectionList& GetSceneNodeSelectionList() override; + const DataTypes::ISceneNodeSelectionList& GetSceneNodeSelectionList() const override; static void Reflect(AZ::ReflectContext* context); static bool VersionConverter(SerializeContext& context, SerializeContext::DataElementNode& classElement); diff --git a/Gems/SceneProcessing/Code/CMakeLists.txt b/Gems/SceneProcessing/Code/CMakeLists.txt index 57fc458d70..b33e157e51 100644 --- a/Gems/SceneProcessing/Code/CMakeLists.txt +++ b/Gems/SceneProcessing/Code/CMakeLists.txt @@ -112,7 +112,6 @@ if(PAL_TRAIT_BUILD_TESTS_SUPPORTED) PRIVATE Gem::SceneProcessing.Editor.Static AZ::AzTest - AZ::SceneData ) ly_add_googletest( NAME Gem::SceneProcessing.Editor.Tests diff --git a/Gems/SceneProcessing/Code/Source/Generation/Components/MeshOptimizer/MeshOptimizerComponent.cpp b/Gems/SceneProcessing/Code/Source/Generation/Components/MeshOptimizer/MeshOptimizerComponent.cpp index 634dafa99c..e3eb617601 100644 --- a/Gems/SceneProcessing/Code/Source/Generation/Components/MeshOptimizer/MeshOptimizerComponent.cpp +++ b/Gems/SceneProcessing/Code/Source/Generation/Components/MeshOptimizer/MeshOptimizerComponent.cpp @@ -96,85 +96,6 @@ namespace AZ::SceneGenerationComponents namespace Containers = AZ::SceneAPI::Containers; namespace Views = Containers::Views; - // @brief A class to map from a mesh's vertex index to it's welded vertex index - // - // When the mesh optimizer runs, it welds nearby vertices (if there are no blendshapes). This class provides a - // constant time lookup to map from an unwelded vertex index to the welded one. - // The welding works by rounding the vertex's position to the given position tolerance, then uses that rounded - // Vector3 as a key into a unordered_map. - template - class Vector3Map - : private AZStd::unordered_map - { - public: - Vector3Map(const MeshDataType* meshData, bool hasBlendShapes, float positionTolerance) - : m_meshData(meshData) - , m_hasBlendShapes(hasBlendShapes) - , m_positionTolerance(positionTolerance) - , m_positionToleranceReciprocal(1.0f / positionTolerance) - { - } - - using AZStd::unordered_map::reserve; - - AZ::u32 operator[](const AZ::u32 vertexIndex) - { - if (m_hasBlendShapes) - { - // Don't attempt to weld similar vertices if there's blendshapes - // Welding the vertices here based on position could cause the vertices of a base shape to be welded, - // and the vertices of the blendshape to not be welded, resulting in a vertex count mismatch between - // the two - return m_meshData->GetUsedPointIndexForControlPoint(m_meshData->GetControlPointIndex(vertexIndex)); - } - - const auto& [iter, didInsert] = try_emplace(GetPositionForIndex(vertexIndex), m_currentOriginalVertexIndex); - if (didInsert) - { - ++m_currentOriginalVertexIndex; - } - return iter->second; - } - - [[nodiscard]] AZ::u32 at(const AZ::u32 vertexIndex) const - { - if (m_hasBlendShapes) - { - // Don't attempt to weld similar vertices if there's blendshapes - // Welding the vertices here based on position could cause the vertices of a base shape to be welded, - // and the vertices of the blendshape to not be welded, resulting in a vertex count mismatch between - // the two - return m_meshData->GetUsedPointIndexForControlPoint(m_meshData->GetControlPointIndex(vertexIndex)); - } - - auto iter = find(GetPositionForIndex(vertexIndex)); - AZSTD_CONTAINER_ASSERT(iter != end(), "Element with key is not present"); - return iter->second; - } - - private: - - AZ::Vector3 GetPositionForIndex(const AZ::u32 vertexIndex) const - { - // Round the vertex position so that a float comparison can be made with entires in the map - // pos = floor( x * 10 + 0.5) * 0.1 - return AZ::Vector3( - AZ::Simd::Vec3::Floor( - (m_meshData->GetPosition(vertexIndex) * m_positionToleranceReciprocal + AZ::Vector3(0.5f)).GetSimdValue() - ) - ) * m_positionTolerance; - } - - const MeshDataType* m_meshData; - bool m_hasBlendShapes; - float m_positionTolerance; - float m_positionToleranceReciprocal; - AZ::u32 m_currentOriginalVertexIndex = 0; - }; - - template - Vector3Map(const MeshDataType*) -> Vector3Map; - MeshOptimizerComponent::MeshOptimizerComponent() { BindToCall(&MeshOptimizerComponent::OptimizeMeshes); @@ -185,7 +106,7 @@ namespace AZ::SceneGenerationComponents auto* serializeContext = azrtti_cast(context); if (serializeContext) { - serializeContext->Class()->Version(4); + serializeContext->Class()->Version(2); } } @@ -194,8 +115,7 @@ namespace AZ::SceneGenerationComponents const MeshDataType* meshData, const SkinWeightDataView& skinWeights, AZ::u32 maxWeightsPerVertex, - float weightThreshold, - const Vector3Map& positionMap) + float weightThreshold) { if (skinWeights.empty()) { @@ -221,12 +141,15 @@ namespace AZ::SceneGenerationComponents for (size_t linkIndex = 0; linkIndex < linkCount; ++linkIndex) { const ISkinWeightData::Link& link = skinData.get().GetLink(controlPointIndex, linkIndex); - skinningInfo->AddInfluence(positionMap.at(usedPointIndex), {aznumeric_caster(link.boneId), link.weight}); + skinningInfo->AddInfluence(usedPointIndex, {aznumeric_caster(link.boneId), link.weight}); } } } - skinningInfo->Optimize(maxWeightsPerVertex, weightThreshold); + if (skinningInfo) + { + skinningInfo->Optimize(maxWeightsPerVertex, weightThreshold); + } return skinningInfo; } @@ -269,12 +192,17 @@ namespace AZ::SceneGenerationComponents const AZStd::vector> meshes = [](const SceneGraph& graph) { AZStd::vector> meshes; - const auto meshNodes = Containers::MakeDerivedFilterView(graph.GetContentStorage()); - for (auto it = meshNodes.cbegin(); it != meshNodes.cend(); ++it) + for (auto it = graph.GetContentStorage().cbegin(); it != graph.GetContentStorage().cend(); ++it) { + // Skip anything that isn't a mesh. + const auto* mesh = azdynamic_cast(it->get()); + if (!mesh) + { + continue; + } + // Get the mesh data and node index and store them in the vector as a pair, so we can iterate over them later. - // The sequential calls to GetBaseIterator unwrap the layers of FilterIterators from the MakeDerivedFilterView - meshes.emplace_back(&(*it), graph.ConvertToNodeIndex(it.GetBaseIterator().GetBaseIterator().GetBaseIterator())); + meshes.emplace_back(mesh, graph.ConvertToNodeIndex(it)); } return meshes; }(graph); @@ -359,12 +287,6 @@ namespace AZ::SceneGenerationComponents auto [optimizedMesh, optimizedUVs, optimizedTangents, optimizedBitangents, optimizedVertexColors, optimizedSkinWeights] = OptimizeMesh(mesh, mesh, uvDatas, tangentDatas, bitangentDatas, colorDatas, skinWeightDatas, meshGroup, hasBlendShapes); - AZ_TracePrintf(AZ::SceneAPI::Utilities::LogWindow, "Base mesh: %zu vertices, optimized mesh: %zu vertices, %0.02f%% of the original", - mesh->GetUsedControlPointCount(), - optimizedMesh->GetUsedControlPointCount(), - ((float)optimizedMesh->GetUsedControlPointCount() / (float)mesh->GetUsedControlPointCount()) * 100.0f - ); - const NodeIndex optimizedMeshNodeIndex = graph.AddChild(graph.GetNodeParent(nodeIndex), name.c_str(), AZStd::move(optimizedMesh)); auto addOptimizedNodes = [&graph, &optimizedMeshNodeIndex](const auto& originalNodeIndexes, auto& optimizedNodes) @@ -506,9 +428,10 @@ namespace AZ::SceneGenerationComponents const AZStd::vector bitangentLayers = makeLayersForData(bitangents); const AZStd::vector vertexColorLayers = makeLayersForData(vertexColors); - constexpr float positionTolerance = 0.0001f; - Vector3Map positionMap(meshData, hasBlendShapes, positionTolerance); - positionMap.reserve(vertexCount); + const auto* skinRule = meshGroup.GetRuleContainerConst().FindFirstByType().get(); + const AZ::u32 maxWeightsPerVertex = skinRule ? skinRule->GetMaxWeightsPerVertex() : 4; + const float weightThreshold = skinRule ? skinRule->GetWeightThreshold() : 0.001f; + meshBuilder.SetSkinningInfo(ExtractSkinningInfo(meshData, skinWeights, maxWeightsPerVertex, weightThreshold)); // Add the vertex data to all the layers const AZ::u32 faceCount = meshData->GetFaceCount(); @@ -517,8 +440,8 @@ namespace AZ::SceneGenerationComponents meshBuilder.BeginPolygon(baseMesh->GetFaceMaterialId(faceIndex)); for (const AZ::u32 vertexIndex : meshData->GetFaceInfo(faceIndex).vertexIndex) { - const AZ::u32 orgVertexNumber = positionMap[vertexIndex]; - + const int orgVertexNumber = meshData->GetUsedPointIndexForControlPoint(meshData->GetControlPointIndex(vertexIndex)); + AZ_Assert(orgVertexNumber >= 0, "Invalid vertex number"); orgVtxLayer->SetCurrentVertexValue(orgVertexNumber); posLayer->SetCurrentVertexValue(meshData->GetPosition(vertexIndex)); @@ -548,12 +471,6 @@ namespace AZ::SceneGenerationComponents meshBuilder.EndPolygon(); } - - const auto* skinRule = meshGroup.GetRuleContainerConst().FindFirstByType().get(); - const AZ::u32 maxWeightsPerVertex = skinRule ? skinRule->GetMaxWeightsPerVertex() : 4; - const float weightThreshold = skinRule ? skinRule->GetWeightThreshold() : 0.001f; - meshBuilder.SetSkinningInfo(ExtractSkinningInfo(meshData, skinWeights, maxWeightsPerVertex, weightThreshold, positionMap)); - meshBuilder.GenerateSubMeshVertexOrders(); // Create the resulting nodes diff --git a/Gems/SceneProcessing/Code/Tests/MeshBuilder/MeshOptimizerComponentTests.cpp b/Gems/SceneProcessing/Code/Tests/MeshBuilder/MeshOptimizerComponentTests.cpp deleted file mode 100644 index 81528fc3e4..0000000000 --- a/Gems/SceneProcessing/Code/Tests/MeshBuilder/MeshOptimizerComponentTests.cpp +++ /dev/null @@ -1,221 +0,0 @@ -/* - * Copyright (c) Contributors to the Open 3D Engine Project. For complete copyright and license terms please see the LICENSE at the root of this distribution. - * - * SPDX-License-Identifier: Apache-2.0 OR MIT - * - */ - -#include - -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#include - -namespace AZ::SceneAPI::DataTypes -{ - void PrintTo(const ISkinWeightData::Link& link, ::std::ostream* os) - { - *os << '{' << link.boneId << ", " << link.weight << '}'; - } -} - -namespace SceneProcessing -{ - class VertexDeduplicationFixture - : public SceneProcessing::InitSceneAPIFixture - { - public: - void SetUp() override - { - SceneProcessing::InitSceneAPIFixture::SetUp(); - - m_systemEntity = m_app.Create({}, {}); - m_systemEntity->AddComponent(aznew AZ::MemoryComponent()); - m_systemEntity->AddComponent(aznew AZ::JobManagerComponent()); - m_systemEntity->Init(); - m_systemEntity->Activate(); - } - void TearDown() override - { - m_systemEntity->Deactivate(); - SceneProcessing::InitSceneAPIFixture::TearDown(); - } - - static AZStd::unique_ptr MakePlaneMesh() - { - // Create a simple plane with 2 triangles, 6 total vertices, 2 shared vertices - // 0 --- 1 - // | / | - // | / | - // | / | - // 2 --- 3 - const AZStd::array planeVertexPositions = { - AZ::Vector3{0.0f, 0.0f, 0.0f}, - AZ::Vector3{0.0f, 0.0f, 1.0f}, - AZ::Vector3{1.0f, 0.0f, 1.0f}, - AZ::Vector3{1.0f, 0.0f, 1.0f}, - AZ::Vector3{1.0f, 0.0f, 0.0f}, - AZ::Vector3{0.0f, 0.0f, 0.0f}, - }; - - auto mesh = AZStd::make_unique(); - - int i = 0; - for (const AZ::Vector3& position : planeVertexPositions) - { - mesh->AddPosition(position); - mesh->AddNormal(AZ::Vector3::CreateAxisY()); - - // This assumes that the data coming from the import process gives a unique control point - // index to every vertex. This follows the behavior of the AssImp library. - mesh->SetVertexIndexToControlPointIndexMap(i, i); - ++i; - } - - mesh->AddFace({0, 1, 2}, 0); - mesh->AddFace({3, 4, 5}, 0); - - return mesh; - } - - static AZStd::unique_ptr MakeSkinData() - { - auto skinWeights = AZStd::make_unique(); - - skinWeights->ResizeContainerSpace(6); - - // Add bones 0 and 1 to the skin weights - skinWeights->GetBoneId("0"); - skinWeights->GetBoneId("1"); - - skinWeights->AppendLink(0, {/*.boneId=*/0, /*.weight=*/1}); - skinWeights->AppendLink(1, {/*.boneId=*/0, /*.weight=*/1}); - skinWeights->AppendLink(2, {/*.boneId=*/0, /*.weight=*/1}); - skinWeights->AppendLink(3, {/*.boneId=*/1, /*.weight=*/1}); - skinWeights->AppendLink(4, {/*.boneId=*/1, /*.weight=*/1}); - skinWeights->AppendLink(5, {/*.boneId=*/1, /*.weight=*/1}); - - return skinWeights; - } - - private: - AZ::ComponentApplication m_app; - AZ::Entity* m_systemEntity; - }; - - TEST_F(VertexDeduplicationFixture, CanDeduplicateVertices) - { - AZ::SceneAPI::Containers::Scene scene("testScene"); - AZ::SceneAPI::Containers::SceneGraph& graph = scene.GetGraph(); - - const auto meshNodeIndex = graph.AddChild(graph.GetRoot(), "testMesh", MakePlaneMesh()); - - // The original source mesh should have 6 vertices - EXPECT_EQ(AZStd::rtti_pointer_cast(graph.GetNodeContent(meshNodeIndex))->GetVertexCount(), 6); - - auto meshGroup = AZStd::make_unique(); - meshGroup->GetSceneNodeSelectionList().AddSelectedNode("testMesh"); - scene.GetManifest().AddEntry(AZStd::move(meshGroup)); - - AZ::SceneGenerationComponents::MeshOptimizerComponent component; - AZ::SceneAPI::Events::GenerateSimplificationEventContext context(scene, "pc"); - component.OptimizeMeshes(context); - - AZ::SceneAPI::Containers::SceneGraph::NodeIndex optimizedNodeIndex = graph.Find(AZStd::string("testMesh").append(AZ::SceneAPI::Utilities::OptimizedMeshSuffix)); - ASSERT_TRUE(optimizedNodeIndex.IsValid()) << "Mesh optimizer did not add an optimized version of the mesh"; - - const auto& optimizedMesh = AZStd::rtti_pointer_cast(graph.GetNodeContent(optimizedNodeIndex)); - ASSERT_TRUE(optimizedMesh); - - // The optimized mesh should have 4 vertices, the 2 shared vertices are welded together - EXPECT_EQ(optimizedMesh->GetVertexCount(), 4); - } - - MATCHER(VectorOfLinksEq, "") - { - return testing::ExplainMatchResult( - testing::AllOf( - testing::Field(&AZ::SceneData::GraphData::SkinWeightData::Link::boneId, testing::Eq(testing::get<0>(arg).boneId)), - testing::Field(&AZ::SceneData::GraphData::SkinWeightData::Link::weight, testing::FloatEq(testing::get<0>(arg).weight)) - ), - testing::get<1>(arg), - result_listener - ); - } - - MATCHER(VectorOfVectorOfLinksEq, "") - { - return testing::ExplainMatchResult( - testing::UnorderedPointwise(VectorOfLinksEq(), testing::get<0>(arg)), - testing::get<1>(arg), - result_listener - ); - } - - TEST_F(VertexDeduplicationFixture, DeduplicatedVerticesRemapSkinning) - { - AZ::SceneAPI::Containers::Scene scene("testScene"); - AZ::SceneAPI::Containers::SceneGraph& graph = scene.GetGraph(); - - const auto meshNodeIndex = graph.AddChild(graph.GetRoot(), "testMesh", MakePlaneMesh()); - const auto skinDataNodeIndex = graph.AddChild(meshNodeIndex, "skinData", MakeSkinData()); - graph.MakeEndPoint(skinDataNodeIndex); - - // The original source mesh should have 6 vertices - EXPECT_EQ(AZStd::rtti_pointer_cast(graph.GetNodeContent(meshNodeIndex))->GetVertexCount(), 6); - - auto meshGroup = AZStd::make_unique(); - meshGroup->GetSceneNodeSelectionList().AddSelectedNode("testMesh"); - scene.GetManifest().AddEntry(AZStd::move(meshGroup)); - - AZ::SceneGenerationComponents::MeshOptimizerComponent component; - AZ::SceneAPI::Events::GenerateSimplificationEventContext context(scene, "pc"); - component.OptimizeMeshes(context); - - AZ::SceneAPI::Containers::SceneGraph::NodeIndex optimizedNodeIndex = graph.Find(AZStd::string("testMesh").append(AZ::SceneAPI::Utilities::OptimizedMeshSuffix)); - ASSERT_TRUE(optimizedNodeIndex.IsValid()) << "Mesh optimizer did not add an optimized version of the mesh"; - - const auto& optimizedMesh = AZStd::rtti_pointer_cast(graph.GetNodeContent(optimizedNodeIndex)); - ASSERT_TRUE(optimizedMesh); - - AZ::SceneAPI::Containers::SceneGraph::NodeIndex optimizedSkinDataNodeIndex = graph.Find(AZStd::string("testMesh").append(AZ::SceneAPI::Utilities::OptimizedMeshSuffix).append(".skinWeights")); - ASSERT_TRUE(optimizedSkinDataNodeIndex.IsValid()) << "Mesh optimizer did not add an optimized version of the skin data"; - - const auto& optimizedSkinWeights = AZStd::rtti_pointer_cast(graph.GetNodeContent(optimizedSkinDataNodeIndex)); - ASSERT_TRUE(optimizedSkinWeights); - - const AZStd::vector> expectedLinks - { - /*0*/ { {0, 0.5f}, {1, 0.5f} }, - /*1*/ { {0, 1.0f} }, - /*2*/ { {0, 0.5f}, {1, 0.5f} }, - /*3*/ { {1, 1.0f} }, - }; - - AZStd::vector> gotLinks(optimizedMesh->GetVertexCount()); - for (unsigned int vertexIndex = 0; vertexIndex < optimizedMesh->GetVertexCount(); ++vertexIndex) - { - for (size_t linkIndex = 0; linkIndex < optimizedSkinWeights->GetLinkCount(vertexIndex); ++linkIndex) - { - gotLinks[vertexIndex].emplace_back(optimizedSkinWeights->GetLink(vertexIndex, linkIndex)); - } - } - EXPECT_THAT(gotLinks, testing::Pointwise(VectorOfVectorOfLinksEq(), expectedLinks)); - } -} // namespace SceneProcessing diff --git a/Gems/SceneProcessing/Code/sceneprocessing_editor_tests_files.cmake b/Gems/SceneProcessing/Code/sceneprocessing_editor_tests_files.cmake index c06d21233d..9ebe2291a3 100644 --- a/Gems/SceneProcessing/Code/sceneprocessing_editor_tests_files.cmake +++ b/Gems/SceneProcessing/Code/sceneprocessing_editor_tests_files.cmake @@ -7,7 +7,6 @@ set(FILES Tests/InitSceneAPIFixture.h - Tests/MeshBuilder/MeshOptimizerComponentTests.cpp Tests/MeshBuilder/MeshBuilderTests.cpp Tests/MeshBuilder/MeshVerticesTests.cpp Tests/MeshBuilder/SkinInfluencesTests.cpp