Merge branch 'main' into non-uniform-scale-mesh

Code/Framework/AzCore/AzCore/Settings/SettingsRegistryMergeUtils.cpp

@@ -506,6 +506,7 @@ namespace AZ::SettingsRegistryMergeUtils
 
     void MergeSettingsToRegistry_AddRuntimeFilePaths(SettingsRegistryInterface& registry)
     {
+        using FixedValueString = AZ::SettingsRegistryInterface::FixedValueString;
         // Binary folder
         AZ::IO::FixedMaxPath path = AZ::Utils::GetExecutableDirectory();
         registry.Set(FilePathKey_BinaryFolder, path.LexicallyNormal().Native());
@@ -514,28 +515,25 @@ namespace AZ::SettingsRegistryMergeUtils
         AZ::IO::FixedMaxPath engineRoot = FindEngineRoot(registry);
         registry.Set(FilePathKey_EngineRootFolder, engineRoot.LexicallyNormal().Native());
 
-        constexpr size_t bufferSize = 64;
-        auto buffer = AZStd::fixed_string<bufferSize>::format("%s/project_path", BootstrapSettingsRootKey);
-
-        AZ::SettingsRegistryInterface::FixedValueString projectPathKey(buffer);
+        auto projectPathKey = FixedValueString::format("%s/project_path", BootstrapSettingsRootKey);
         SettingsRegistryInterface::FixedValueString projectPathValue;
         if (registry.Get(projectPathValue, projectPathKey))
         {
             // Cache folder
             // Get the name of the asset platform assigned by the bootstrap. First check for platform version such as "windows_assets"
             // and if that's missing just get "assets".
-            constexpr char platformName[] = AZ_TRAIT_OS_PLATFORM_CODENAME_LOWER;
-
-            buffer = AZStd::fixed_string<bufferSize>::format("%s/%s_assets", BootstrapSettingsRootKey, platformName);
-            AZStd::string_view assetPlatformKey(buffer);
-            // Use the platform codename to retrieve the default asset platform value
-            SettingsRegistryInterface::FixedValueString assetPlatform = AZ::OSPlatformToDefaultAssetPlatform(AZ_TRAIT_OS_PLATFORM_CODENAME);
-            if (!registry.Get(assetPlatform, assetPlatformKey))
+            FixedValueString assetPlatform;
+            if (auto assetPlatformKey = FixedValueString::format("%s/%s_assets", BootstrapSettingsRootKey, AZ_TRAIT_OS_PLATFORM_CODENAME_LOWER);
+                !registry.Get(assetPlatform, assetPlatformKey))
             {
-                buffer = AZStd::fixed_string<bufferSize>::format("%s/assets", BootstrapSettingsRootKey);
-                assetPlatformKey = AZStd::string_view(buffer);
+                assetPlatformKey = FixedValueString::format("%s/assets", BootstrapSettingsRootKey);
                 registry.Get(assetPlatform, assetPlatformKey);
             }
+            if (assetPlatform.empty())
+            {
+                // Use the platform codename to retrieve the default asset platform value
+                assetPlatform = AZ::OSPlatformToDefaultAssetPlatform(AZ_TRAIT_OS_PLATFORM_CODENAME);
+            }
 
             // Project path - corresponds to the @devassets@ alias
             // NOTE: Here we append to engineRoot, but if projectPathValue is absolute then engineRoot is discarded.
@@ -575,8 +573,7 @@ namespace AZ::SettingsRegistryMergeUtils
             {
                 // Cache: project root - no corresponding fileIO alias, but this is where the asset database lives.
                 // A registry override is accepted using the "project_cache_path" key.
-                buffer = AZStd::fixed_string<bufferSize>::format("%s/project_cache_path", BootstrapSettingsRootKey);
-                AZStd::string_view projectCacheRootOverrideKey(buffer);
+                auto projectCacheRootOverrideKey = FixedValueString::format("%s/project_cache_path", BootstrapSettingsRootKey);
                 // Clear path to make sure that the `project_cache_path` value isn't concatenated to the project path
                 path.clear();
                 if (registry.Get(path.Native(), projectCacheRootOverrideKey))

Code/Framework/AzToolsFramework/AzToolsFramework/Entity/EditorEntityHelpers.cpp

@@ -136,6 +136,48 @@ namespace AzToolsFramework
         return entity->GetName();
     }
 
+    EntityList EntityIdListToEntityList(const EntityIdList& inputEntityIds)
+    {
+        EntityList entities;
+        entities.reserve(inputEntityIds.size());
+
+        for (AZ::EntityId entityId : inputEntityIds)
+        {
+            if (!entityId.IsValid())
+            {
+                continue;
+            }
+
+            if (auto entity = GetEntityById(entityId))
+            {
+                entities.emplace_back(entity);
+            }
+        }
+
+        return entities;
+    }
+
+    EntityList EntityIdSetToEntityList(const EntityIdSet& inputEntityIds)
+    {
+        EntityList entities;
+        entities.reserve(inputEntityIds.size());
+
+        for (AZ::EntityId entityId : inputEntityIds)
+        {
+            if (!entityId.IsValid())
+            {
+                continue;
+            }
+
+            if (auto entity = GetEntityById(entityId))
+            {
+                entities.emplace_back(entity);
+            }
+        }
+
+        return entities;
+    }
+
     void GetAllComponentsForEntity(const AZ::Entity* entity, AZ::Entity::ComponentArrayType& componentsOnEntity)
     {
         if (entity)
@@ -1068,6 +1110,45 @@ namespace AzToolsFramework
         return !allEntityClonesContainer.m_entities.empty();
     }
 
+    EntityIdSet GetCulledEntityHierarchy(const EntityIdList& entities)
+    {
+        EntityIdSet culledEntities;
+
+        for (const AZ::EntityId& entityId : entities)
+        {
+            bool selectionIncludesTransformHeritage = false;
+            AZ::EntityId parentEntityId = entityId;
+            do
+            {
+                AZ::EntityId nextParentId;
+                AZ::TransformBus::EventResult(
+                    /*result*/ nextParentId,
+                    /*address*/ parentEntityId,
+                    &AZ::TransformBus::Events::GetParentId);
+                parentEntityId = nextParentId;
+                if (!parentEntityId.IsValid())
+                {
+                    break;
+                }
+                for (const AZ::EntityId& parentCheck : entities)
+                {
+                    if (parentCheck == parentEntityId)
+                    {
+                        selectionIncludesTransformHeritage = true;
+                        break;
+                    }
+                }
+            } while (parentEntityId.IsValid() && !selectionIncludesTransformHeritage);
+
+            if (!selectionIncludesTransformHeritage)
+            {
+                culledEntities.insert(entityId);
+            }
+        }
+
+        return culledEntities;
+    }
+
     namespace Internal
     {
         void CloneSliceEntitiesAndChildren(

Code/Framework/AzToolsFramework/AzToolsFramework/Entity/EditorEntityHelpers.h

@@ -47,6 +47,9 @@ namespace AzToolsFramework
 
     AZStd::string GetEntityName(const AZ::EntityId& entityId, const AZStd::string_view& nameOverride = {});
 
+    EntityList EntityIdListToEntityList(const EntityIdList& inputEntityIds);
+    EntityList EntityIdSetToEntityList(const EntityIdSet& inputEntityIds);
+
     template <typename... ComponentTypes>
     struct AddComponents
     {
@@ -202,4 +205,8 @@ namespace AzToolsFramework
     /// Wrap EBus SetSelectedEntities call.
     void SelectEntities(const AzToolsFramework::EntityIdList& entities);
 
+    /// Return a set of entities, culling any that have an ancestor in the list.
+    /// e.g. This is useful for getting a concise set of entities that need to be duplicated.
+    EntityIdSet GetCulledEntityHierarchy(const EntityIdList& entities);
+
 }; // namespace AzToolsFramework

Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp

@@ -61,8 +61,7 @@ namespace AzToolsFramework
         PrefabOperationResult PrefabPublicHandler::CreatePrefab(const AZStd::vector<AZ::EntityId>& entityIds, AZ::IO::PathView filePath)
         {
             // Retrieve entityList from entityIds
-            EntityList inputEntityList;
-            EntityIdListToEntityList(entityIds, inputEntityList);
+            EntityList inputEntityList = EntityIdListToEntityList(entityIds);
 
             // Find common root and top level entities
             bool entitiesHaveCommonRoot = false;
@@ -419,8 +418,7 @@ namespace AzToolsFramework
             InstanceOptionalReference instance = GetOwnerInstanceByEntityId(entityIds[0]);
 
             // Retrieve entityList from entityIds
-            EntityList inputEntityList;
-            EntityIdListToEntityList(entityIds, inputEntityList);
+            EntityList inputEntityList = EntityIdListToEntityList(entityIds);
 
             AZ_PROFILE_FUNCTION(AZ::Debug::ProfileCategory::AzToolsFramework);
 
@@ -767,18 +765,5 @@ namespace AzToolsFramework
 
             return true;
         }
-
-        void PrefabPublicHandler::EntityIdListToEntityList(const EntityIdList& inputEntityIds, EntityList& outEntities)
-        {
-            outEntities.reserve(inputEntityIds.size());
-
-            for (AZ::EntityId entityId : inputEntityIds)
-            {
-                if (entityId.IsValid())
-                {
-                    outEntities.emplace_back(GetEntityById(entityId));
-                }
-            }
-        }
     }
 }

Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h

@@ -70,7 +70,6 @@ namespace AzToolsFramework
             static Instance* GetParentInstance(Instance* instance);
             static Instance* GetAncestorOfInstanceThatIsChildOfRoot(const Instance* ancestor, Instance* descendant);
             static void GenerateContainerEntityTransform(const EntityList& topLevelEntities, AZ::Vector3& translation, AZ::Quaternion& rotation);
-            static void EntityIdListToEntityList(const EntityIdList& inputEntityIds, EntityList& outEntities);
 
             InstanceEntityMapperInterface* m_instanceEntityMapperInterface = nullptr;
             InstanceToTemplateInterface* m_instanceToTemplateInterface = nullptr;

Code/Framework/AzToolsFramework/AzToolsFramework/UI/Outliner/EntityOutlinerListModel.cpp

@@ -1344,14 +1344,15 @@ namespace AzToolsFramework
         emit EnableSelectionUpdates(false);
         auto parentIndex = GetIndexFromEntity(parentId);
         auto childIndex = GetIndexFromEntity(childId);
-        beginRemoveRows(parentIndex, childIndex.row(), childIndex.row());
+        beginResetModel();
     }
 
     void EntityOutlinerListModel::OnEntityInfoUpdatedRemoveChildEnd(AZ::EntityId parentId, AZ::EntityId childId)
     {
         (void)childId;
         AZ_PROFILE_FUNCTION(AZ::Debug::ProfileCategory::AzToolsFramework);
-        endRemoveRows();
+
+        endResetModel();
 
         //must refresh partial lock/visibility of parents
         m_isFilterDirty = true;

Code/Framework/AzToolsFramework/Tests/Entity/EditorEntityHelpersTests.cpp

@@ -0,0 +1,57 @@
+/*
+* All or portions of this file Copyright (c) Amazon.com, Inc. or its affiliates or
+* its licensors.
+*
+* For complete copyright and license terms please see the LICENSE at the root of this
+* distribution (the "License"). All use of this software is governed by the License,
+* or, if provided, by the license below or the license accompanying this file. Do not
+* remove or modify any license notices. This file is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+*
+*/
+
+#include <AzTest/AzTest.h>
+
+#include <AzToolsFramework/Application/ToolsApplication.h>
+#include <AzToolsFramework/Entity/EditorEntityHelpers.h>
+#include <AzToolsFramework/UnitTest/AzToolsFrameworkTestHelpers.h>
+
+namespace UnitTest
+{
+    class EditorEntityHelpersTest
+        : public ToolsApplicationFixture
+    {
+        void SetUpEditorFixtureImpl() override
+        {
+            m_parent1 = CreateDefaultEditorEntity("Parent1");
+            m_child1 = CreateDefaultEditorEntity("Child1");
+            m_child2 = CreateDefaultEditorEntity("Child2");
+            m_grandChild1 = CreateDefaultEditorEntity("GrandChild1");
+            m_parent2 = CreateDefaultEditorEntity("Parent2");
+
+            AZ::TransformBus::Event(m_child1, &AZ::TransformBus::Events::SetParent, m_parent1);
+            AZ::TransformBus::Event(m_child2, &AZ::TransformBus::Events::SetParent, m_parent1);
+            AZ::TransformBus::Event(m_grandChild1, &AZ::TransformBus::Events::SetParent, m_child1);
+        }
+
+    public:
+        AZ::EntityId m_parent1;
+        AZ::EntityId m_child1;
+        AZ::EntityId m_child2;
+        AZ::EntityId m_grandChild1;
+        AZ::EntityId m_parent2;
+    };
+
+    TEST_F(EditorEntityHelpersTest, EditorEntityHelpersTests_GetCulledEntityHierarchy)
+    {
+        AzToolsFramework::EntityIdList testEntityIds{ m_parent1, m_child1, m_child2, m_grandChild1, m_parent2 };
+
+        AzToolsFramework::EntityIdSet culledSet = AzToolsFramework::GetCulledEntityHierarchy(testEntityIds);
+
+        // There should only be two EntityIds returned (m_parent1, and m_parent2),
+        // since all the others should be culled out since they have a common ancestor
+        // in the list already
+        using ::testing::UnorderedElementsAre;
+        EXPECT_THAT(culledSet, UnorderedElementsAre(m_parent1, m_parent2));
+    }
+}

Code/Framework/AzToolsFramework/Tests/aztoolsframeworktests_files.cmake

@@ -85,7 +85,9 @@ set(FILES
     Prefab/SpawnableSortEntitiesTestFixture.cpp
     Prefab/SpawnableSortEntitiesTestFixture.h
     Entity/EditorEntityContextComponentTests.cpp
+    Entity/EditorEntityHelpersTests.cpp
     Entity/EditorEntitySearchComponentTests.cpp
+    Entity/EditorEntitySelectionTests.cpp
     SliceStabilityTests/SliceStabilityTestFramework.h
     SliceStabilityTests/SliceStabilityTestFramework.cpp
     SliceStabilityTests/SliceStabilityCreateTests.cpp

Code/Sandbox/Plugins/ComponentEntityEditorPlugin/SandboxIntegration.cpp

@@ -670,9 +670,13 @@ void SandboxIntegrationManager::PopulateEditorGlobalContextMenu(QMenu* menu, con
         action = menu->addAction(QObject::tr("Create layer"));
         QObject::connect(action, &QAction::triggered, [this] { ContextMenu_NewLayer(); });
 
+        AzToolsFramework::EntityIdList entities;
+        AzToolsFramework::ToolsApplicationRequests::Bus::BroadcastResult(
+            entities,
+            &AzToolsFramework::ToolsApplicationRequests::GetSelectedEntities);
+
         SetupLayerContextMenu(menu);
-        AzToolsFramework::EntityIdSet flattenedSelection;
-        GetSelectedEntitiesSetWithFlattenedHierarchy(flattenedSelection);
+        AzToolsFramework::EntityIdSet flattenedSelection = AzToolsFramework::GetCulledEntityHierarchy(entities);
         AzToolsFramework::SetupAddToLayerMenu(menu, flattenedSelection, [this] { return ContextMenu_NewLayer(); });
 
         SetupSliceContextMenu(menu);
@@ -1220,10 +1224,14 @@ void SandboxIntegrationManager::CloneSelection(bool& handled)
 {
     AZ_PROFILE_FUNCTION(AZ::Debug::ProfileCategory::AzToolsFramework);
 
-    AzToolsFramework::EntityIdSet duplicationSet;
-    GetSelectedEntitiesSetWithFlattenedHierarchy(duplicationSet);
+    AzToolsFramework::EntityIdList entities;
+    AzToolsFramework::ToolsApplicationRequests::Bus::BroadcastResult(
+        entities,
+        &AzToolsFramework::ToolsApplicationRequests::GetSelectedEntities);
 
-    if (duplicationSet.size() > 0)
+    AzToolsFramework::EntityIdSet duplicationSet = AzToolsFramework::GetCulledEntityHierarchy(entities);
+
+    if (!duplicationSet.empty())
     {
         AZStd::unordered_set<AZ::EntityId> clonedEntities;
         handled = AzToolsFramework::CloneInstantiatedEntities(duplicationSet, clonedEntities);

Code/Tools/SceneAPI/FbxSceneBuilder/Importers/AssImpAnimationImporter.cpp

@@ -42,7 +42,108 @@ namespace AZ
 
             // Downstream only supports 30 frames per second sample rate. Adjusting to 60 doubles the
             // length of the animations, they still play back at 30 frames per second.
-            const double AssImpAnimationImporter::s_defaultTimeStepSampleRate = 1.0 / 30.0;
+            const double AssImpAnimationImporter::s_defaultTimeStepBetweenFrames = 1.0 / 30.0;
+
+            AZ::u32 GetNumKeyFrames(AZ::u32 keysSize, double duration, double ticksPerSecond)
+            {
+                if (AZ::IsClose(ticksPerSecond, 0))
+                {
+                    AZ_Warning("AnimationImporter", false, "Animation ticks per second should not be zero, defaulting to %d keyframes for animation.", keysSize);
+                    return keysSize;
+                }
+                const double totalTicks = duration / ticksPerSecond;
+                AZ::u32 numKeys = keysSize;
+                // +1 because the animation is from [0, duration] - we have a keyframe at the end of the duration which needs to be included
+                double totalFramesAtDefaultTimeStep = totalTicks / AssImpAnimationImporter::s_defaultTimeStepBetweenFrames + 1;
+                if (!AZ::IsClose(totalFramesAtDefaultTimeStep, numKeys, 1))
+                {
+                    numKeys = AZStd::ceilf(totalFramesAtDefaultTimeStep);
+                }
+                return numKeys;
+            }
+
+            double GetTimeForFrame(AZ::u32 frame, double ticksPerSecond)
+            {
+                return frame * AssImpAnimationImporter::s_defaultTimeStepBetweenFrames * ticksPerSecond;
+            }
+
+            // Helper class to store key data, when translating from AssImp layout to the engine's scene format.
+            struct KeyData
+            {
+                KeyData(float value, float time) :
+                    mValue(value),
+                    mTime(time)
+                {
+
+                }
+
+                bool operator<(const KeyData& other) const
+                {
+                    return mTime < other.mTime;
+                }
+
+                float mValue = 0;
+                float mTime = 0;
+            };
+
+            template<class T>
+            void LerpTemplate(T& start, const T& end, float t)
+            {
+                start = start * (1.0f - t) + end * t;
+            }
+
+            template<>
+            void LerpTemplate(aiQuaternion& start, const aiQuaternion& end, float t)
+            {
+                aiQuaternion::Interpolate(start, start, end, t);
+            }
+
+            template<>
+            void LerpTemplate(float& start, const float& end, float t)
+            {
+                start = AZ::Lerp(start, end, t);
+            }
+
+            template<class KeyContainerType, class FrameValueType>
+            bool SampleKeyFrame(FrameValueType& result, const KeyContainerType& keys, AZ::u32 numKeys, double time, AZ::u32& lastIndex)
+            {
+                if (numKeys == 0)
+                {
+                    AZ_Error("AnimationImporter", numKeys > 0, "Animation key set must have at least 1 key");
+                    return false;
+                }
+                if (numKeys == 1)
+                {
+                    result = keys[0].mValue;
+                    return true;
+                }
+
+                while (lastIndex < numKeys - 1 && time >= keys[lastIndex + 1].mTime)
+                {
+                    ++lastIndex;
+                }
+                result = keys[lastIndex].mValue;
+                if (lastIndex < numKeys - 1)
+                {
+                    auto nextValue = keys[lastIndex + 1].mValue;
+                    float normalizedTimeBetweenFrames = 0;
+                    if (keys[lastIndex + 1].mTime != keys[lastIndex].mTime)
+                    {
+                        normalizedTimeBetweenFrames =
+                            (time - keys[lastIndex].mTime) / (keys[lastIndex + 1].mTime - keys[lastIndex].mTime);
+                    }
+                    else
+                    {
+                        AZ_Warning("AnimationImporter", false,
+                            "Animation has keys with duplicate time %5.5f, at indices %d and %d. The second will be ignored.",
+                            keys[lastIndex].mTime,
+                            lastIndex,
+                            lastIndex + 1);
+                    }
+                    LerpTemplate(result, nextValue, normalizedTimeBetweenFrames);
+                }
+                return true;
+            }
 
             AssImpAnimationImporter::AssImpAnimationImporter()
             {
@@ -199,6 +300,14 @@ namespace AZ
                 for (AZ::u32 animIndex = 0; animIndex < scene->mNumAnimations; ++animIndex)
                 {
                     const aiAnimation* animation = scene->mAnimations[animIndex];
+                    if (animation->mTicksPerSecond == 0)
+                    {
+                        AZ_Error(
+                            "AnimationImporter", false,
+                            "Animation name %s has a sample rate of 0 ticks per second and cannot be processed.",
+                            animation->mName.C_Str());
+                        return Events::ProcessingResult::Failure;
+                    }
                     
                     mapAnimationsFunc(animation->mNumChannels, animation->mChannels, animation, boneAnimations);
 
@@ -410,70 +519,38 @@ namespace AZ
                             anim->mNumPositionKeys, anim->mNumRotationKeys, anim->mNumScalingKeys);
                         return Events::ProcessingResult::Failure;
                     }
-                    
-                    auto sampleKeyFrame = [](const auto& keys, AZ::u32 numKeys, double time, AZ::u32& lastIndex)
-                    {
-                        AZ_Error("AnimationImporter", numKeys > 0, "Animation key set must have at least 1 key");
-
-                        if (numKeys == 1)
-                        {
-                            return keys[0].mValue;
-                        }
-
-                        auto returnValue = keys[0].mValue;
-
-                        for (AZ::u32 keyIndex = lastIndex; keyIndex < numKeys; ++keyIndex)
-                        {
-                            const auto& key = keys[keyIndex];
-                            lastIndex = keyIndex;
-
-                            // We want to return the key that exactly matches the time if possible, otherwise we'll keep track of the previous time
-                            // If we don't find an exact match and end up going past the desired time (or run out of keyframes) then we return the previous key
-                            if (key.mTime < time)
-                            {
-                                returnValue = key.mValue;
-                            }
-                            else if (AZ::IsClose(key.mTime, time))
-                            {
-                                return key.mValue;
-                            }
-                            else
-                            {
-                                return returnValue;
-                            }
-                        }
-                        
-                        return returnValue;
-                    };
 
                     // Resample the animations at a fixed time step. This matches the behaviour of
                     // the previous SDK used. Longer term, this could be data driven, or based on the
                     // smallest time step between key frames.
                     // AssImp has an animation->mTicksPerSecond and animation->mDuration, but those
                     // are less predictable than just using a fixed time step.
-                    const double duration = animation->mDuration / animation->mTicksPerSecond;
+                    // AssImp documentation claims animation->mDuration is the duration of the animation in ticks, but
+                    // not all animations we've tested follow that pattern. Sometimes duration is in seconds.
+                    const AZ::u32 numKeyFrames = GetNumKeyFrames(
+                        AZStd::max(AZStd::max(anim->mNumScalingKeys, anim->mNumPositionKeys), anim->mNumRotationKeys),
+                        animation->mDuration,
+                        animation->mTicksPerSecond);
 
-                    AZ::u32 numKeyFrames = AZStd::max(AZStd::max(anim->mNumScalingKeys, anim->mNumPositionKeys), anim->mNumRotationKeys);
-                    if (!AZ::IsClose(duration / s_defaultTimeStepSampleRate, numKeyFrames, 1))
-                    {
-                        double dT = duration / s_defaultTimeStepSampleRate;
-                        numKeyFrames = AZStd::ceilf(dT) + 1; // +1 because the animation is from [0, duration] - we have a keyframe at the end of the duration which needs to be included
-                    }
-                    
                     AZStd::shared_ptr<SceneData::GraphData::AnimationData> createdAnimationData =
                        AZStd::make_shared<SceneData::GraphData::AnimationData>();
                     createdAnimationData->ReserveKeyFrames(numKeyFrames);
-                    createdAnimationData->SetTimeStepBetweenFrames(s_defaultTimeStepSampleRate);
+                    createdAnimationData->SetTimeStepBetweenFrames(s_defaultTimeStepBetweenFrames);
 
                     AZ::u32 lastScaleIndex = 0;
                     AZ::u32 lastPositionIndex = 0;
                     AZ::u32 lastRotationIndex = 0;
                     for (AZ::u32 frame = 0; frame < numKeyFrames; ++frame)
                     {
-                        double time = frame * s_defaultTimeStepSampleRate * animation->mTicksPerSecond;
-                        aiVector3D scale = sampleKeyFrame(anim->mScalingKeys, anim->mNumScalingKeys, time, lastScaleIndex);
-                        aiVector3D position = sampleKeyFrame(anim->mPositionKeys, anim->mNumPositionKeys, time, lastPositionIndex);
-                        aiQuaternion rotation = sampleKeyFrame(anim->mRotationKeys, anim->mNumRotationKeys, time, lastRotationIndex);
+                        const double time = GetTimeForFrame(frame, animation->mTicksPerSecond);
+                        aiVector3D scale = aiVector3D(1.f, 1.f, 1.f), position = aiVector3D(0.f, 0.f, 0.f);
+                        aiQuaternion rotation(1.f, 0.f, 0.f, 0.f);
+                        if (!SampleKeyFrame(scale, anim->mScalingKeys, anim->mNumScalingKeys, time, lastScaleIndex) ||
+                            !SampleKeyFrame(position, anim->mPositionKeys, anim->mNumPositionKeys, time, lastPositionIndex) ||
+                            !SampleKeyFrame(rotation, anim->mRotationKeys, anim->mNumRotationKeys, time, lastRotationIndex))
+                        {
+                            return Events::ProcessingResult::Failure;
+                        }
                         
                         aiMatrix4x4 transform(scale, rotation, position);
                         
@@ -520,28 +597,6 @@ namespace AZ
                 //      SetTimeStepBetweenFrames set on the animation data
                 //      Keyframes. Weights (Values in FBX SDK) per key time.
                 //      Keyframes generated for every single frame of the animation.
-
-                // Helper class to store key data, when translating from AssImp layout to the engine's scene format.
-                struct KeyData
-                {
-                    KeyData(float weight, float time) :
-                        m_weight(weight),
-                        m_time(time)
-                    {
-
-                    }
-
-                    bool operator<(const KeyData& other) const
-                    {
-                        return m_time < other.m_time;
-                    }
-
-                    // Naming in the previous SDK (FBX SDK) and in the engine's scene format
-                    // doesn't match AssImp's naming convention.
-                    // weight here is the AssImp's name for the data, it was named value in FBX SDK.
-                    float m_weight = 0;
-                    float m_time = 0;
-                };
                 typedef AZStd::map<int, AZStd::vector<KeyData>> ValueToKeyDataMap;
                 ValueToKeyDataMap valueToKeyDataMap;
 
@@ -562,44 +617,27 @@ namespace AZ
                 {
                     AZStd::shared_ptr<SceneData::GraphData::BlendShapeAnimationData> morphAnimNode =
                         AZStd::make_shared<SceneData::GraphData::BlendShapeAnimationData>();
-                    morphAnimNode->ReserveKeyFrames(animation->mDuration + 1);
-                    morphAnimNode->SetTimeStepBetweenFrames(1.0 / animation->mTicksPerSecond);
+
+                    const AZ::u32 numKeyFrames = GetNumKeyFrames(keys.size(), animation->mDuration, animation->mTicksPerSecond);
+                    morphAnimNode->ReserveKeyFrames(numKeyFrames);
+                    morphAnimNode->SetTimeStepBetweenFrames(s_defaultTimeStepBetweenFrames);
 
                     aiAnimMesh* aiAnimMesh = mesh->mAnimMeshes[meshIdx];
                     AZStd::string_view nodeName(aiAnimMesh->mName.C_Str());
 
                     const AZ::u32 maxKeys = keys.size();
                     AZ::u32 keyIdx = 0;
-                    for (AZ::u32 time = 0; time <= animation->mDuration; ++time)
+                    for (AZ::u32 frame = 0; frame <= numKeyFrames; ++frame)
                     {
-                        if (keyIdx < maxKeys - 1 && time >= keys[keyIdx+1].m_time)
-                        {
-                            ++keyIdx;
-                        }
-                        float weight_value = keys[keyIdx].m_weight;
-                        if (keyIdx < maxKeys - 1)
-                        {
-                            float nextWeight = keys[keyIdx+1].m_weight;
-                            float normalizedTimeBetweenFrames = 0;
+                        const double time = GetTimeForFrame(frame, animation->mTicksPerSecond);
 
-                            if (keys[keyIdx + 1].m_time != keys[keyIdx].m_time)
-                            {
-                                normalizedTimeBetweenFrames =
-                                    (time - keys[keyIdx].m_time) / (keys[keyIdx + 1].m_time - keys[keyIdx].m_time);
-                            }
-                            else
-                            {
-                                AZ_Warning("AnimationImporter", false,
-                                    "Morph target mesh %s has keys with duplicate time, at indices %d and %d. The second will be ignored.",
-                                    nodeName.data(),
-                                    keyIdx,
-                                    keyIdx+1);
-                            }
-
-                            // AssImp and FBX both only support linear interpolation for blend shapes.
-                            weight_value = AZ::Lerp(weight_value, nextWeight, normalizedTimeBetweenFrames);
+                        float weight = 0;
+                        if (!SampleKeyFrame(weight, keys, keys.size(), time, keyIdx))
+                        {
+                            return Events::ProcessingResult::Failure;
                         }
-                        morphAnimNode->AddKeyFrame(weight_value);
+
+                        morphAnimNode->AddKeyFrame(weight);
                     }
 
 

Code/Tools/SceneAPI/FbxSceneBuilder/Importers/AssImpAnimationImporter.h

@@ -45,7 +45,7 @@ namespace AZ
                     const aiMeshMorphAnim* meshMorphAnim,
                     const aiMesh* mesh);
 
-                static const double s_defaultTimeStepSampleRate;
+                static const double s_defaultTimeStepBetweenFrames;
 
             protected:
                 static const char* s_animationNodeName;

Code/Tools/SceneAPI/SceneCore/Containers/SceneGraph.cpp

@@ -43,7 +43,7 @@ namespace AZ
                 AZ::BehaviorContext* behaviorContext = azrtti_cast<AZ::BehaviorContext*>(context);
                 if (behaviorContext)
                 {
-                    behaviorContext->Class<SceneGraph::NodeIndex>()
+                    behaviorContext->Class<SceneGraph::NodeIndex>("NodeIndex")
                         ->Attribute(AZ::Script::Attributes::Scope, AZ::Script::Attributes::ScopeFlags::Common)
                         ->Attribute(AZ::Script::Attributes::Module, "scene.graph")
                         ->Constructor<>()
@@ -57,7 +57,7 @@ namespace AZ
                             ->Attribute(AZ::Script::Attributes::Operator, AZ::Script::Attributes::OperatorType::ToString)
                         ;
 
-                    behaviorContext->Class<SceneGraph::Name>()
+                    behaviorContext->Class<SceneGraph::Name>("SceneGraphName")
                         ->Attribute(AZ::Script::Attributes::Scope, AZ::Script::Attributes::ScopeFlags::Common)
                         ->Attribute(AZ::Script::Attributes::Module, "scene.graph")
                         ->Constructor()