Fix for GHI-9598: Vertex welding not functioning (#10096)

* Fix mesh optimization by making the remapped node the model builder searches for match the naming in the mesh optimizer

Signed-off-by: Tommy Walton <[email protected]>

* Initial commit for a mesh optimization hydra test

Signed-off-by: Tommy Walton <[email protected]>

* Hydra test updates

Signed-off-by: Tommy Walton <[email protected]>

* Switch to using CustomPropertyData

Signed-off-by: Tommy Walton <[email protected]>

* Use a pointer with the Outcome so that getting the PropertyMap can be wrapped in a function

Signed-off-by: Tommy Walton <[email protected]>

* Move vertex welding test into the existing P0 null renderer mesh component tests

Signed-off-by: Tommy Walton <[email protected]>

* Use new GetImmediateChildOfType utility in PrefabGroupBehavior instead of duplicating code for looking it up

Signed-off-by: Tommy Walton <[email protected]>

* Switch to AZStd::string::format instead of append

Signed-off-by: Tommy Walton <[email protected]>

* Fix cloth by adding a mapping from the optimized mesh node back to the original unoptimized mesh node, because cloth relies on knowing the path of the original, unoptimized mesh node

Signed-off-by: amzn-tommy <[email protected]>

* Fix hydra test to use a model from TestData, since Valena doesn't exist in the Automated Testing project. Add a test case for vertices that shouldn't be welded if they have different skin influences

Signed-off-by: Tommy Walton <[email protected]>

* PR feedback

Signed-off-by: Tommy Walton <[email protected]>

* Bump model builder version

Signed-off-by: Tommy Walton <[email protected]>

* Adding missing test asset

Signed-off-by: Tommy Walton <[email protected]>

Signed-off-by: Tommy Walton <[email protected]>
Signed-off-by: amzn-tommy <[email protected]>

AutomatedTesting/Gem/PythonTests/Atom/atom_utils/atom_constants.py

@@ -982,6 +982,7 @@ class AtomComponentProperties:
           - 'Minimum Screen Coverage' portion of the screen at which the mesh is culled; 0 (never culled) to 1
           - 'Quality Decay Rate' rate at which the mesh degrades; 0 (never) to 1 (lowest quality imediately)
           - 'Lod Override' which specific LOD to always use; default or other named LOD
+          - 'Vertex Count LOD0' the vertices in LOD 0 of the model
         :param property: From the last element of the property tree path. Default 'name' for component name string.
         :return: Full property path OR component name if no property specified.
         :rtype: str
@@ -998,6 +999,7 @@ class AtomComponentProperties:
             'Minimum Screen Coverage': 'Controller|Configuration|Lod Configuration|Minimum Screen Coverage',
             'Quality Decay Rate': 'Controller|Configuration|Lod Configuration|Quality Decay Rate',
             'Lod Override': 'Controller|Configuration|Lod Configuration|Lod Override',
+            'Vertex Count LOD0': 'Model Stats|Mesh Stats|LOD 0|Vert Count'
         }
         return properties[property]
 

AutomatedTesting/Gem/PythonTests/Atom/tests/hydra_AtomEditorComponents_MeshAdded.py

@@ -78,6 +78,12 @@ class Tests:
     mesh_component_removed = (
         "Mesh component removed successfully",
         "P1: Mesh component was not correctly removed from the entity")
+    model_asset_is_optimized = (
+        "tube.azmodel has <= 66 vertices",
+        "P0: Model has not been fully optimized")
+    model_different_bone_ids_same_position_should_weld_vertices = (
+        "sameposition_differentboneIds_shouldnotweldvertices.azmodel has 48 vertices",
+        "P0: Vertices were welded when they shouldnt be")
 
 
 def AtomEditorComponents_Mesh_AddedToEntity():
@@ -113,11 +119,13 @@ def AtomEditorComponents_Mesh_AddedToEntity():
     17) Remove Mesh component then UNDO the remove
     18) Enter/Exit game mode.
     19) Test IsHidden.
-    10) Test IsVisible.
-    21) Delete Mesh entity.
-    22) UNDO deletion.
-    23) REDO deletion.
-    24) Look for errors.
+    20) Test IsVisible.
+    21) Verify that vertex welding is functioning
+    22) Verify that vertices with the same position but different joint ids aren't welded
+    23) Delete Mesh entity.
+    24) UNDO deletion.
+    25) REDO deletion.
+    26) Look for errors.
 
     :return: None
     """
@@ -126,6 +134,8 @@ def AtomEditorComponents_Mesh_AddedToEntity():
 
     from PySide2 import QtWidgets
 
+    import azlmbr.bus
+    from functools import partial
     import azlmbr.legacy.general as general
     from Atom.atom_utils.atom_constants import (MESH_LOD_TYPE,
                                                 AtomComponentProperties)
@@ -134,6 +144,35 @@ def AtomEditorComponents_Mesh_AddedToEntity():
     from editor_python_test_tools.editor_entity_utils import EditorEntity
     from editor_python_test_tools.utils import Report, TestHelper, Tracer
 
+    class OnModelReadyHelper:
+        def __init__(self):
+            self.isModelReady = False
+        
+        def model_is_ready_predicate(self):
+            """
+            A predicate function what will be used in wait_for_condition.
+            """
+            return self.isModelReady
+
+        def on_model_ready(self, parameters):
+            self.isModelReady = True
+
+        def wait_for_on_model_ready(self, entityId, mesh_component, model_id):
+            self.isModelReady = False
+            # Connect to the MeshNotificationBus
+            # Listen for notifications when entities are created/deleted
+            self.onModelReadyHandler = azlmbr.bus.NotificationHandler('MeshComponentNotificationBus')
+            self.onModelReadyHandler.connect(entityId)
+            self.onModelReadyHandler.add_callback('OnModelReady', self.on_model_ready)
+            
+            waitCondition = partial(self.model_is_ready_predicate)
+            
+            mesh_component.set_component_property_value(AtomComponentProperties.mesh('Model Asset'), model_id)
+            if TestHelper.wait_for_condition(waitCondition, 20.0):
+                return True
+            else:
+                return False
+
     with Tracer() as error_tracer:
         # Test setup begins.
         # Setup: Wait for Editor idle loop before executing Python hydra scripts then open "Base" level.
@@ -285,22 +324,48 @@ def AtomEditorComponents_Mesh_AddedToEntity():
         mesh_entity.set_visibility_state(True)
         general.idle_wait_frames(1)
         Report.result(Tests.is_visible, mesh_entity.is_visible() is True)
+        
+        # 21. Test that vertex welding is functioning
+        model_path = os.path.join('testdata', 'objects', 'tube.azmodel')
+        model = Asset.find_asset_by_path(model_path)
+        onModelReadyHelper = OnModelReadyHelper()
+        onModelReadyHelper.wait_for_on_model_ready(mesh_entity.id, mesh_component, model.id)
+        Report.result(Tests.model_asset_specified,
+                      mesh_component.get_component_property_value(
+                          AtomComponentProperties.mesh('Model Asset')) == model.id)
+
+        Report.result(Tests.model_asset_is_optimized,
+                      mesh_component.get_component_property_value(
+                          AtomComponentProperties.mesh('Vertex Count LOD0')) <= 66)
+
+        # 22. Test that vertices with the same position but different boneId's aren't unintentionally welded
+        model_path = os.path.join('testdata', 'objects', 'skinnedmesh', 'meshoptimization', 'sameposition_differentjointids_shouldnotweldvertices.azmodel')
+        model = Asset.find_asset_by_path(model_path)
+        onModelReadyHelper = OnModelReadyHelper()
+        onModelReadyHelper.wait_for_on_model_ready(mesh_entity.id, mesh_component, model.id)
+        Report.result(Tests.model_asset_specified,
+                      mesh_component.get_component_property_value(
+                          AtomComponentProperties.mesh('Model Asset')) == model.id)
+
+        Report.result(Tests.model_different_bone_ids_same_position_should_weld_vertices,
+                      mesh_component.get_component_property_value(
+                          AtomComponentProperties.mesh('Vertex Count LOD0')) == 48)
 
-        # 21. Delete Mesh entity.
+        # 23. Delete Mesh entity.
         mesh_entity.delete()
         Report.result(Tests.entity_deleted, not mesh_entity.exists())
 
-        # 22. UNDO deletion.
+        # 24. UNDO deletion.
         general.undo()
         general.idle_wait_frames(1)
         Report.result(Tests.deletion_undo, mesh_entity.exists())
 
-        # 23. REDO deletion.
+        # 25. REDO deletion.
         general.redo()
         general.idle_wait_frames(1)
         Report.result(Tests.deletion_redo, not mesh_entity.exists())
 
-        # 24. Look for errors or asserts.
+        # 26. Look for errors or asserts.
         TestHelper.wait_for_condition(lambda: error_tracer.has_errors or error_tracer.has_asserts, 1.0)
         for error_info in error_tracer.errors:
             Report.info(f"Error: {error_info.filename} {error_info.function} | {error_info.message}")

Code/Tools/SceneAPI/SceneCore/Containers/Utilities/SceneGraphUtilities.cpp

@@ -59,6 +59,23 @@ namespace AZ
                 }
 
                 return outTransform;
+            }            
+
+            Containers::SceneGraph::NodeIndex GetImmediateChildOfType(
+                const Containers::SceneGraph& graph, const Containers::SceneGraph::NodeIndex& nodeIndex, const AZ::TypeId& typeId)
+            {
+                auto childIndex = graph.GetNodeChild(nodeIndex);
+                while (childIndex.IsValid())
+                {
+                    const auto nodeContent = graph.GetNodeContent(childIndex);
+                    if (nodeContent && azrtti_istypeof(typeId, nodeContent.get()))
+                    {
+                        break;
+                    }
+                    childIndex = graph.GetNodeSibling(childIndex);
+                }
+
+                return childIndex;
             }
         } // Utilities
     } // SceneAPI

Code/Tools/SceneAPI/SceneCore/Containers/Utilities/SceneGraphUtilities.h

@@ -27,6 +27,10 @@ namespace AZ
             template<typename T>
             bool DoesSceneGraphContainDataLike(const Containers::Scene& scene, bool checkVirtualTypes);
             SCENE_CORE_API DataTypes::MatrixType BuildWorldTransform(const Containers::SceneGraph& graph, Containers::SceneGraph::NodeIndex nodeIndex);
+            // Searches only the immediate children of nodeIndex for a child node that matches the specified type.
+            // Returns the NodeIndex if it exists, or and invalid NodeIndex if it doesn't
+            SCENE_CORE_API Containers::SceneGraph::NodeIndex GetImmediateChildOfType(
+                const Containers::SceneGraph& graph, const Containers::SceneGraph::NodeIndex& nodeIndex, const AZ::TypeId& typeId);
         } // Utilities
     } // SceneAPI
 } // AZ

Code/Tools/SceneAPI/SceneCore/DataTypes/Rules/IClothRule.h

@@ -39,6 +39,10 @@ namespace AZ
                 virtual AZStd::vector<AZ::Color> ExtractClothData(const Containers::SceneGraph& graph, const size_t numVertices) const = 0;
 
                 /// Finds the cloth rule affecting a mesh node and extracts cloth data.
+                /// @param graph The scene graph
+                /// @param meshNodeIndex The index of the original, unoptimized mesh node
+                /// @param numVertices The vertex count used when extracting the cloth data
+                /// @param rules The rules for the scene, which may include the cloth rule
                 static AZStd::vector<AZ::Color> FindClothData(
                     const AZ::SceneAPI::Containers::SceneGraph& graph,
                     const AZ::SceneAPI::Containers::SceneGraph::NodeIndex& meshNodeIndex,
@@ -47,13 +51,10 @@ namespace AZ
                 {
                     AZStd::vector<AZ::Color> clothData;
 
+                    // Note: it is important the original mesh node index is used, not a generated optimized mesh node,
+                    // because the cloth rule tracks the mesh node based on the original node paths
                     AZStd::string_view meshNodeName = graph.GetNodeName(meshNodeIndex).GetPath();
 
-                    if (meshNodeName.ends_with(Utilities::OptimizedMeshSuffix))
-                    {
-                        meshNodeName.remove_suffix(Utilities::OptimizedMeshSuffix.size());
-                    }
-
                     for (size_t ruleIndex = 0; ruleIndex < rules.GetRuleCount(); ++ruleIndex)
                     {
                         const IClothRule* clothRule = azrtti_cast<const IClothRule*>(rules.GetRule(ruleIndex).get());

Code/Tools/SceneAPI/SceneCore/Utilities/SceneGraphSelector.cpp

@@ -6,13 +6,16 @@
  *
  */
 
+#include <SceneAPI/SceneCore/Containers/Utilities/SceneGraphUtilities.h>
 #include <SceneAPI/SceneCore/Containers/Views/SceneGraphDownwardsIterator.h>
 #include <SceneAPI/SceneCore/Containers/Views/PairIterator.h>
 #include <SceneAPI/SceneCore/Utilities/SceneGraphSelector.h>
 #include <AzCore/std/containers/set.h>
 #include <AzCore/Math/Transform.h>
 #include <SceneAPI/SceneCore/DataTypes/ManifestBase/ISceneNodeSelectionList.h>
+#include <SceneAPI/SceneCore/DataTypes/GraphData/ICustomPropertyData.h>
 #include <SceneAPI/SceneCore/DataTypes/GraphData/IMeshData.h>
+#include <SceneAPI/SceneCore/DataTypes/Groups/ISceneNodeGroup.h>
 
 namespace AZ
 {
@@ -40,18 +43,67 @@ namespace AZ
                 return object && object->RTTI_IsTypeOf(DataTypes::IMeshData::TYPEINFO_Uuid());
             }
 
-            Containers::SceneGraph::NodeIndex SceneGraphSelector::RemapToOptimizedMesh(const Containers::SceneGraph& graph, const Containers::SceneGraph::NodeIndex& index)
+            Containers::SceneGraph::NodeIndex SceneGraphSelector::RemapToOptimizedMesh(
+                const Containers::SceneGraph& graph, const Containers::SceneGraph::NodeIndex& unoptimizedMeshNodeIndex)
             {
-                const auto& nodeName = graph.GetNodeName(index);
-                const AZStd::string optimizedName = AZStd::string(nodeName.GetPath(), nodeName.GetPathLength()).append(OptimizedMeshSuffix);
-                if (auto optimizedIndex = graph.Find(optimizedName); optimizedIndex.IsValid())
+                return RemapNodeIndex(graph, unoptimizedMeshNodeIndex, SceneAPI::Utilities::OptimizedMeshPropertyMapKey);
+            }
+
+            Containers::SceneGraph::NodeIndex SceneGraphSelector::RemapToOriginalUnoptimizedMesh(
+                const Containers::SceneGraph& graph, const Containers::SceneGraph::NodeIndex& optimizedMeshNodeIndex)
+            {
+                return RemapNodeIndex(graph, optimizedMeshNodeIndex, SceneAPI::Utilities::OriginalUnoptimizedMeshPropertyMapKey);
+            }
+
+            Containers::SceneGraph::NodeIndex SceneGraphSelector::RemapNodeIndex(
+                const Containers::SceneGraph& graph,
+                const Containers::SceneGraph::NodeIndex& index,
+                const AZStd::string_view customPropertyKey)
+            {
+                // Search the immediate children for an ICustomPropertyData node to lookup the optimized mesh index
+                const Containers::SceneGraph::NodeIndex customPropertyIndex =
+                    GetImmediateChildOfType(graph, index, azrtti_typeid<AZ::SceneAPI::DataTypes::ICustomPropertyData>());
+
+                if (customPropertyIndex.IsValid())
                 {
-                    return optimizedIndex;
+                    const DataTypes::ICustomPropertyData* customPropertyDataNode =
+                        azrtti_cast<const DataTypes::ICustomPropertyData*>(graph.GetNodeContent(customPropertyIndex).get());
+
+                    // Now look up the optimized index
+                    const auto& propertyMap = customPropertyDataNode->GetPropertyMap();
+                    const auto iter = propertyMap.find(customPropertyKey);
+
+                    if (iter != propertyMap.end())
+                    {
+                        const AZStd::any& remappedNodeIndex = iter->second;
+
+                        if (!remappedNodeIndex.empty() && remappedNodeIndex.is<Containers::SceneGraph::NodeIndex>())
+                        {
+                            return AZStd::any_cast<Containers::SceneGraph::NodeIndex>(remappedNodeIndex);
+                        }
+                    }
                 }
+
+                // Return the original index if there is no optimized mesh node
                 return index;
             }
 
-            AZStd::vector<AZStd::string> SceneGraphSelector::GenerateTargetNodes(const Containers::SceneGraph& graph, const DataTypes::ISceneNodeSelectionList& list, NodeFilterFunction nodeFilter, NodeRemapFunction nodeRemap)
+            AZStd::string SceneGraphSelector::GenerateOptimizedMeshNodeName(
+                const Containers::SceneGraph& graph,
+                const Containers::SceneGraph::NodeIndex& unoptimizedMeshNodeIndex,
+                const DataTypes::ISceneNodeGroup& sceneNodeGroup)
+            {
+                const auto& nodeName = graph.GetNodeName(unoptimizedMeshNodeIndex);
+
+                return AZStd::string::format(
+                    "%s_%s%s", nodeName.GetName(), sceneNodeGroup.GetName().c_str(), SceneAPI::Utilities::OptimizedMeshSuffix.data());
+            }
+
+            AZStd::vector<AZStd::string> SceneGraphSelector::GenerateTargetNodes(
+                const Containers::SceneGraph& graph,
+                const DataTypes::ISceneNodeSelectionList& list,
+                NodeFilterFunction nodeFilter,
+                NodeRemapFunction nodeRemap)
             {
                 AZStd::vector<AZStd::string> targetNodes;
                 AZStd::set<AZStd::string> selectedNodesSet;
@@ -273,4 +325,5 @@ namespace AZ
             }
         }
     }
-}
+} // namespace AZ
+

Code/Tools/SceneAPI/SceneCore/Utilities/SceneGraphSelector.h

@@ -14,11 +14,16 @@
 #include <AzCore/std/containers/unordered_set.h>
 #include <SceneAPI/SceneCore/Containers/SceneGraph.h>
 
-namespace AZ::SceneAPI::DataTypes { class ISceneNodeSelectionList; }
+namespace AZ::SceneAPI::DataTypes {
+    class ISceneNodeSelectionList;
+    class ISceneNodeGroup;
+}
 
 namespace AZ::SceneAPI::Utilities
 {
     inline constexpr AZStd::string_view OptimizedMeshSuffix = "_optimized";
+    inline constexpr AZStd::string_view OptimizedMeshPropertyMapKey = "o3de_optimized_mesh_node";
+    inline constexpr AZStd::string_view OriginalUnoptimizedMeshPropertyMapKey = "o3de_original_unoptimized_mesh_node";
 
     // SceneGraphSelector provides utilities including converting selected and unselected node lists
     // in the MeshGroup into the final target node list.
@@ -28,8 +33,11 @@ namespace AZ::SceneAPI::Utilities
         using NodeFilterFunction = bool(const Containers::SceneGraph& graph, Containers::SceneGraph::NodeIndex& index);
         using NodeRemapFunction = Containers::SceneGraph::NodeIndex(const Containers::SceneGraph& graph, const Containers::SceneGraph::NodeIndex& index);
 
-        SCENE_CORE_API static AZStd::vector<AZStd::string> GenerateTargetNodes(const Containers::SceneGraph& graph, const DataTypes::ISceneNodeSelectionList& list,
-            NodeFilterFunction nodeFilter, NodeRemapFunction nodeRemap = NoRemap);
+        SCENE_CORE_API static AZStd::vector<AZStd::string> GenerateTargetNodes(
+            const Containers::SceneGraph& graph,
+            const DataTypes::ISceneNodeSelectionList& list,
+            NodeFilterFunction nodeFilter,
+            NodeRemapFunction nodeRemap = NoRemap);
         SCENE_CORE_API static void SelectAll(const Containers::SceneGraph& graph, DataTypes::ISceneNodeSelectionList& list);
         SCENE_CORE_API static void UnselectAll(const Containers::SceneGraph& graph, DataTypes::ISceneNodeSelectionList& list);
         SCENE_CORE_API static void UpdateNodeSelection(const Containers::SceneGraph& graph, DataTypes::ISceneNodeSelectionList& list);
@@ -40,11 +48,46 @@ namespace AZ::SceneAPI::Utilities
         SCENE_CORE_API static bool IsMesh(const Containers::SceneGraph& graph, Containers::SceneGraph::NodeIndex& index);
         SCENE_CORE_API static bool IsMeshObject(const AZStd::shared_ptr<const DataTypes::IGraphObject>& object);
 
-        SCENE_CORE_API static Containers::SceneGraph::NodeIndex NoRemap(const Containers::SceneGraph& /*graph*/, const Containers::SceneGraph::NodeIndex& index)
+        //! Returns the index that is passed in without remapping it.
+        //! GenerateTargetNodes takes a NodeRemapFunction as input. NoRemap is used as the default
+        //! if you want to call GenerateTargetNodes without doing any re-mapping
+        SCENE_CORE_API static Containers::SceneGraph::NodeIndex NoRemap(
+            [[maybe_unused]] const Containers::SceneGraph& /*graph*/,
+            const Containers::SceneGraph::NodeIndex& index)
         {
             return index;
         }
-        SCENE_CORE_API static Containers::SceneGraph::NodeIndex RemapToOptimizedMesh(const Containers::SceneGraph& graph, const Containers::SceneGraph::NodeIndex& index);
+
+        //! Remaps unoptimizedMeshNodeIndex to the optimized version of the mesh, if it exists.
+        //! @param graph The scene graph
+        //! @param unoptimizedMeshNodeIndex The node index of the unoptimized mesh
+        //! @return Returns the node index of the optimized mesh if it exists, or unoptimizedMeshNodeIndex if it doesn't exist
+        SCENE_CORE_API static Containers::SceneGraph::NodeIndex RemapToOptimizedMesh(
+            const Containers::SceneGraph& graph, const Containers::SceneGraph::NodeIndex& unoptimizedMeshNodeIndex);
+
+        //! Remap optimizedMeshNodeIndex to the original unoptimized version of the mesh, if it exists
+        //! @param graph The scene graph
+        //! @param optimizedMeshNodeIndex The node index of the optimized mesh
+        //! @return Returns the node index of the original unoptimized mesh if it exists. Returns optimizedMeshNodeIndex if it doesn't exist.
+        SCENE_CORE_API static Containers::SceneGraph::NodeIndex RemapToOriginalUnoptimizedMesh(
+            const Containers::SceneGraph& graph, const Containers::SceneGraph::NodeIndex& optimizedMeshNodeIndex);
+
+        //! Look for a ICustomPropertyData child node. If it exists, use the property map
+        //! to look for an entry that matches customPropertyKey, and return the result
+        //! @param graph The scene graph
+        //! @param index The node index that is being remapped
+        //! @param customPropertyKey The key to use to look up the remapped index in the property map
+        //! @return Returns the remapped node index if one matching customPropertyKey is found. Returns the input index if it doesn't exist.
+        SCENE_CORE_API static Containers::SceneGraph::NodeIndex RemapNodeIndex(
+            const Containers::SceneGraph& graph,
+            const Containers::SceneGraph::NodeIndex& index,
+            const AZStd::string_view customPropertyKey);
+
+        //! Generate a name for an optimized mesh node based on the name of the original node and the mesh group it belongs to
+        SCENE_CORE_API static AZStd::string GenerateOptimizedMeshNodeName(
+            const Containers::SceneGraph& graph,
+            const Containers::SceneGraph::NodeIndex& unoptimizedMeshNodeIndex,
+            const DataTypes::ISceneNodeGroup& meshGroup);
 
     private:
         static void CopySelectionToSet(AZStd::set<AZStd::string>& selected, AZStd::set<AZStd::string>& unselected, const DataTypes::ISceneNodeSelectionList& list);

Gems/Atom/RPI/Code/Source/RPI.Builders/Model/ModelAssetBuilderComponent.cpp

@@ -114,7 +114,7 @@ namespace AZ
             if (auto* serialize = azrtti_cast<SerializeContext*>(context))
             {
                 serialize->Class<ModelAssetBuilderComponent, SceneAPI::SceneCore::ExportingComponent>()
-                    ->Version(33);  // Fix parent-child relationship in scene
+                    ->Version(34);  // Fix vertex welding
             }
         }
 
@@ -212,16 +212,18 @@ namespace AZ
                 selectedMeshPathsByLod.resize(lodRule->GetLodCount());
                 for (size_t lod = 0; lod < lodRule->GetLodCount(); ++lod)
                 {
-                    selectedMeshPathsByLod[lod] = SceneAPI::Utilities::SceneGraphSelector::GenerateTargetNodes(sceneGraph,
-                        lodRule->GetSceneNodeSelectionList(lod), isNonOptimizedMesh, SceneAPI::Utilities::SceneGraphSelector::RemapToOptimizedMesh);
+                    selectedMeshPathsByLod[lod] = SceneAPI::Utilities::SceneGraphSelector::GenerateTargetNodes(
+                        sceneGraph, lodRule->GetSceneNodeSelectionList(lod), isNonOptimizedMesh,
+                        SceneAPI::Utilities::SceneGraphSelector::RemapToOptimizedMesh);
                 }
             }
 
             // Gather the list of nodes in the graph that are selected as part of this
             // MeshGroup defined in context.m_group, then remap to the optimized mesh
             // nodes, if they exist.
-            AZStd::vector<AZStd::string> selectedMeshPaths = SceneAPI::Utilities::SceneGraphSelector::GenerateTargetNodes(sceneGraph,
-                context.m_group.GetSceneNodeSelectionList(), isNonOptimizedMesh, SceneAPI::Utilities::SceneGraphSelector::RemapToOptimizedMesh);
+            AZStd::vector<AZStd::string> selectedMeshPaths = SceneAPI::Utilities::SceneGraphSelector::GenerateTargetNodes(
+                sceneGraph, context.m_group.GetSceneNodeSelectionList(), isNonOptimizedMesh,
+                SceneAPI::Utilities::SceneGraphSelector::RemapToOptimizedMesh);
 
             // Iterate over the downwards, breadth-first view into the scene.
             // First we have to split the source mesh data up by lod.
@@ -310,18 +312,17 @@ namespace AZ
                     // Gather mesh content
                     SourceMeshContent sourceMesh;
 
-                    // Although the nodes used to gather mesh content are the optimized ones (when found), to make
-                    // this process transparent for the end-asset generated, the name assigned to the source mesh
-                    // content will not include the "_optimized" prefix.
-                    AZStd::string_view sourceMeshName = meshName;
-                    if (sourceMeshName.ends_with(SceneAPI::Utilities::OptimizedMeshSuffix))
-                    {
-                        sourceMeshName.remove_suffix(SceneAPI::Utilities::OptimizedMeshSuffix.size());
-                    }
-                    sourceMesh.m_name = sourceMeshName;
+                    
 
                     const auto node = sceneGraph.Find(meshPath);
                     sourceMesh.m_worldTransform = AZ::SceneAPI::Utilities::DetermineWorldTransform(scene, node, context.m_group.GetRuleContainerConst());
+                    
+                    SceneAPI::Containers::SceneGraph::NodeIndex originalUnoptimizedMeshIndex =
+                        SceneAPI::Utilities::SceneGraphSelector::RemapToOriginalUnoptimizedMesh(sceneGraph, node);
+                    // Although the nodes used to gather mesh content are the optimized ones (when found), to make
+                    // this process transparent for the end-asset generated, the name assigned to the source mesh
+                    // content will not include the "_optimized" prefix or the group name.
+                    sourceMesh.m_name = sceneGraph.GetNodeName(originalUnoptimizedMeshIndex).GetName();
 
                     // Add the MeshData to the source mesh
                     AddToMeshContent(viewIt.second, sourceMesh);
@@ -355,7 +356,10 @@ namespace AZ
                     // Get the cloth data (only for full mesh LOD 0).
                     sourceMesh.m_meshClothData = (lodIndex == 0)
                         ? SceneAPI::DataTypes::IClothRule::FindClothData(
-                            sceneGraph, node, sourceMesh.m_meshData->GetVertexCount(), context.m_group.GetRuleContainerConst())
+                                                                       sceneGraph,
+                                                                       originalUnoptimizedMeshIndex,
+                                                                       sourceMesh.m_meshData->GetVertexCount(),
+                                                                       context.m_group.GetRuleContainerConst())
                         : AZStd::vector<AZ::Color>{};
 
                     // We've traversed this node and all its children that hold

Gems/Atom/TestData/TestData/Objects/SkinnedMesh/MeshOptimization/sameposition_differentjointids_shouldnotweldvertices.fbx

@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:8bde2ef8e912c41dd993f4b5eac3c160f553e5962aa624a1770797ec48c1c3a5
+size 52956

Gems/NvCloth/Code/Source/Pipeline/SceneAPIExt/ClothRule.cpp

@@ -12,6 +12,7 @@
 
 #include <SceneAPI/SceneCore/DataTypes/GraphData/IMeshData.h>
 #include <SceneAPI/SceneCore/DataTypes/GraphData/IMeshVertexColorData.h>
+#include <SceneAPI/SceneData/Groups/MeshGroup.h>
 
 #include <Pipeline/SceneAPIExt/ClothRule.h>
 
@@ -39,11 +40,9 @@ namespace NvCloth
         {
             const AZ::SceneAPI::Containers::SceneGraph::NodeIndex meshNodeIndex = [this, &graph]()
             {
-                if (const auto index = graph.Find(GetMeshNodeName() + AZStd::string(AZ::SceneAPI::Utilities::OptimizedMeshSuffix)); index.IsValid())
-                {
-                    return index;
-                }
-                return graph.Find(GetMeshNodeName());
+                const auto originalMeshIndex = graph.Find(GetMeshNodeName());
+                return AZ::SceneAPI::Utilities::SceneGraphSelector::RemapToOptimizedMesh(
+                    graph, originalMeshIndex);
             }();
 
             if (!meshNodeIndex.IsValid())

Gems/SceneProcessing/Code/Source/Generation/Components/MeshOptimizer/MeshOptimizerComponent.cpp

@@ -20,6 +20,7 @@
 #include <AzCore/std/iterator.h>
 #include <AzCore/std/limits.h>
 #include <AzCore/std/reference_wrapper.h>
+#include <AzCore/std/smart_ptr/make_shared.h>
 #include <AzCore/std/smart_ptr/shared_ptr.h>
 #include <AzCore/std/smart_ptr/unique_ptr.h>
 #include <AzCore/std/string/string_view.h>
@@ -33,6 +34,7 @@
 #include <SceneAPI/SceneCore/Containers/Views/PairIterator.h>
 #include <SceneAPI/SceneCore/Containers/Views/SceneGraphChildIterator.h>
 #include <SceneAPI/SceneCore/Containers/Utilities/Filters.h>
+#include <SceneAPI/SceneCore/Containers/Utilities/SceneGraphUtilities.h>
 #include <SceneAPI/SceneCore/Containers/Views/ConvertIterator.h>
 #include <SceneAPI/SceneCore/Containers/Views/FilterIterator.h>
 #include <SceneAPI/SceneCore/Containers/Views/View.h>
@@ -52,6 +54,7 @@
 #include <SceneAPI/SceneCore/Utilities/Reporting.h>
 #include <SceneAPI/SceneCore/Utilities/SceneGraphSelector.h>
 #include <SceneAPI/SceneData/GraphData/BlendShapeData.h>
+#include <SceneAPI/SceneData/GraphData/CustomPropertyData.h>
 #include <SceneAPI/SceneData/GraphData/MeshData.h>
 #include <SceneAPI/SceneData/GraphData/MeshVertexBitangentData.h>
 #include <SceneAPI/SceneData/GraphData/MeshVertexColorData.h>
@@ -87,6 +90,7 @@ namespace AZ::SceneGenerationComponents
     using AZ::SceneAPI::DataTypes::IMeshVertexUVData;
     using AZ::SceneAPI::DataTypes::IMeshVertexColorData;
     using AZ::SceneAPI::DataTypes::ISkinWeightData;
+    using AZ::SceneAPI::DataTypes::ICustomPropertyData;
     using AZ::SceneAPI::Events::ProcessingResult;
     using AZ::SceneAPI::Events::GenerateSimplificationEventContext;
     using AZ::SceneAPI::SceneCore::GenerationComponent;
@@ -209,7 +213,7 @@ namespace AZ::SceneGenerationComponents
         auto* serializeContext = azrtti_cast<AZ::SerializeContext*>(context);
         if (serializeContext)
         {
-            serializeContext->Class<MeshOptimizerComponent, GenerationComponent>()->Version(11);
+            serializeContext->Class<MeshOptimizerComponent, GenerationComponent>()->Version(12); // Fix vertex welding
         }
     }
 
@@ -261,6 +265,42 @@ namespace AZ::SceneGenerationComponents
         ).empty();
     }
 
+    static ICustomPropertyData::PropertyMap& FindOrCreateCustomPropertyData(
+        Containers::SceneGraph& graph, const Containers::SceneGraph::NodeIndex& nodeIndex)
+    {
+        NodeIndex customPropertyIndex =
+            SceneAPI::Utilities::GetImmediateChildOfType(graph, nodeIndex, azrtti_typeid<ICustomPropertyData>());
+
+        if (!customPropertyIndex.IsValid())
+        {
+            // If no custom property data node exists, insert one
+            AZStd::shared_ptr<SceneData::GraphData::CustomPropertyData> createdCustumPropertyData =
+                AZStd::make_shared<SceneData::GraphData::CustomPropertyData>();
+            customPropertyIndex = graph.AddChild(nodeIndex, "custom_properties", AZStd::move(createdCustumPropertyData));
+        }
+
+        ICustomPropertyData* customPropertyDataNode =
+            azrtti_cast<ICustomPropertyData*>(graph.GetNodeContent(customPropertyIndex).get());
+
+        return customPropertyDataNode->GetPropertyMap();
+    }
+
+    static bool HasOptimizedMeshNode(ICustomPropertyData::PropertyMap& propertyMap)
+    {
+        // Now look up the optimized index
+        auto iter = propertyMap.find(SceneAPI::Utilities::OptimizedMeshPropertyMapKey);
+        if (iter != propertyMap.end())
+        {
+            const auto& [key, optimizedAnyIndex] = *iter;
+            if (!optimizedAnyIndex.empty() && optimizedAnyIndex.is<NodeIndex>())
+            {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     ProcessingResult MeshOptimizerComponent::OptimizeMeshes(GenerateSimplificationEventContext& context) const
     {
         // Iterate over all graph content and filter out all meshes.
@@ -348,14 +388,11 @@ namespace AZ::SceneGenerationComponents
                     continue;
                 }
 
-                const AZStd::string name =
-                    AZStd::string(graph.GetNodeName(nodeIndex).GetName(), graph.GetNodeName(nodeIndex).GetNameLength())
-                        .append("_")
-                        .append(meshGroup.GetName())
-                        .append(SceneAPI::Utilities::OptimizedMeshSuffix);
-                if (graph.Find(name).IsValid())
+                ICustomPropertyData::PropertyMap& unoptimizedPropertyMap = FindOrCreateCustomPropertyData(graph, nodeIndex);
+                if (HasOptimizedMeshNode(unoptimizedPropertyMap))
                 {
-                    AZ_TracePrintf(AZ::SceneAPI::Utilities::LogWindow, "Optimized mesh already exists at '%s', there must be multiple mesh groups that have selected this mesh. Skipping the additional ones.", name.c_str());
+                    // There is already an optimized mesh node for this mesh, so skip it.
+                    // There must be another mesh group already referencing this mesh node.
                     continue;
                 }
 
@@ -371,7 +408,23 @@ namespace AZ::SceneGenerationComponents
                     hasBlendShapes ? "Yes" : "No"
                 );
 
-                const NodeIndex optimizedMeshNodeIndex = graph.AddChild(graph.GetNodeParent(nodeIndex), name.c_str(), AZStd::move(optimizedMesh));
+                // Insert a new node for the optimized mesh
+                const AZStd::string name =
+                    SceneAPI::Utilities::SceneGraphSelector::GenerateOptimizedMeshNodeName(graph, nodeIndex, meshGroup);
+                const NodeIndex optimizedMeshNodeIndex =
+                    graph.AddChild(graph.GetNodeParent(nodeIndex), name.c_str(), AZStd::move(optimizedMesh));            
+
+                // Copy any custom properties from the original mesh to the optimized mesh
+                ICustomPropertyData::PropertyMap& optimizedPropertyMap = FindOrCreateCustomPropertyData(graph, optimizedMeshNodeIndex);
+                optimizedPropertyMap = unoptimizedPropertyMap;
+
+                // Add a mapping from the optimized node back to the original node so it can also be looked up later
+                optimizedPropertyMap[SceneAPI::Utilities::OriginalUnoptimizedMeshPropertyMapKey] =
+                    AZStd::make_any<NodeIndex>(nodeIndex);
+
+                // Add the optimized node index to the original mesh's custom property map so it can be looked up later
+                unoptimizedPropertyMap[SceneAPI::Utilities::OptimizedMeshPropertyMapKey] =
+                    AZStd::make_any<NodeIndex>(optimizedMeshNodeIndex);    
 
                 auto addOptimizedNodes = [&graph, &optimizedMeshNodeIndex](const auto& originalNodeIndexes, auto& optimizedNodes)
                 {
@@ -411,7 +464,9 @@ namespace AZ::SceneGenerationComponents
                     }
                 }
 
-                const AZStd::array optimizedChildTypes {
+                const AZStd::array skippedChildTypes {
+                    // Skip copying the optimized nodes since we've already
+                    // populated those nodes with the optimized data
                     azrtti_typeid<IMeshData>(),
                     azrtti_typeid<IMeshVertexUVData>(),
                     azrtti_typeid<IMeshVertexTangentData>(),
@@ -419,12 +474,22 @@ namespace AZ::SceneGenerationComponents
                     azrtti_typeid<IMeshVertexColorData>(),
                     azrtti_typeid<ISkinWeightData>(),
                     azrtti_typeid<IBlendShapeData>(),
+                    // Skip copying the custom property data because we've already copied it above
+                    azrtti_typeid<ICustomPropertyData>()
                 };
+
+                // Copy the children of the original mesh node, but skip any nodes we have already populated
                 for (const NodeIndex& childNodeIndex : nodeIndexes(childNodes(nodeIndex)))
                 {
                     const AZStd::shared_ptr<SceneAPI::DataTypes::IGraphObject>& childNode = graph.GetNodeContent(childNodeIndex);
 
-                    if (!AZStd::any_of(optimizedChildTypes.begin(), optimizedChildTypes.end(), [&childNode](const AZ::Uuid& typeId) { return AZ::RttiIsTypeOf(typeId, childNode.get()); }))
+                    if (!AZStd::any_of(
+                            skippedChildTypes.begin(),
+                            skippedChildTypes.end(),
+                            [&childNode](const AZ::Uuid& typeId)
+                            {
+                                return AZ::RttiIsTypeOf(typeId, childNode.get());
+                            }))
                     {
                         const AZStd::string optimizedName {graph.GetNodeName(childNodeIndex).GetName(), graph.GetNodeName(childNodeIndex).GetNameLength()};
                         const NodeIndex optimizedNodeIndex = graph.AddChild(optimizedMeshNodeIndex, optimizedName.c_str(), childNode);