Changed how asset creator generates the asset instance. Instead of finding or creating the asset in the asset manager, one is directly instantiated and only added to the asset manager after creation is complete. This allows for reuse of previously loaded asset ids and will replace or “reload” a pre-existing asset with the newly created one. This also sends although correct notifications.

Changed material document to load a source data are for the parent material as well.  It was also a previously loading the parent material products asset which would be out of date compared to the source data.
Changed material document to track source file dependency changes instead of product asset changes.
Fixed a bug or copy paste error in the document manager that was using the same container to track documents the modified externally and from other dependency changes.
Returning source data dependencies when creating a material asset from source.

Signed-off-by: Guthrie Adams <guthadam@amazon.com>

Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialSourceData.h

@@ -99,7 +99,8 @@ namespace AZ
                 Data::AssetId assetId,
                 AZStd::string_view materialSourceFilePath = "",
                 bool elevateWarnings = true,
-                bool includeMaterialPropertyNames = true) const;
+                bool includeMaterialPropertyNames = true,
+                AZStd::unordered_set<AZStd::string>* sourceDependencies = nullptr) const;
 
         private:
             void ApplyPropertiesToAssetCreator(

Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/AssetCreator.h

@@ -118,7 +118,7 @@ namespace AZ
 
             ResetIssueCounts(); // Because the asset creator can be used multiple times
 
-            m_asset = Data::AssetManager::Instance().CreateAsset<AssetDataT>(assetId, AZ::Data::AssetLoadBehavior::PreLoad);
+            m_asset = Data::Asset<AssetDataT>(assetId, aznew AssetDataT, AZ::Data::AssetLoadBehavior::PreLoad);
             m_beginCalled = true;
 
             if (!m_asset)
@@ -138,6 +138,7 @@ namespace AZ
             }
             else
             {
+                Data::AssetManager::Instance().AssignAssetData(m_asset);
                 result = AZStd::move(m_asset);
                 success = true;
             }

Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceData.cpp

@@ -188,14 +188,20 @@ namespace AZ
         }
 
         Outcome<Data::Asset<MaterialAsset>> MaterialSourceData::CreateMaterialAssetFromSourceData(
-            Data::AssetId assetId, AZStd::string_view materialSourceFilePath, bool elevateWarnings, bool includeMaterialPropertyNames) const
+            Data::AssetId assetId,
+            AZStd::string_view materialSourceFilePath,
+            bool elevateWarnings,
+            bool includeMaterialPropertyNames,
+            AZStd::unordered_set<AZStd::string>* sourceDependencies) const
         {
-            MaterialAssetCreator materialAssetCreator;
+            const auto materialTypeSourcePath = AssetUtils::ResolvePathReference(materialSourceFilePath, m_materialType);
-            materialAssetCreator.SetElevateWarnings(elevateWarnings);
+            const auto materialTypeAssetId = AssetUtils::MakeAssetId(materialTypeSourcePath, 0);
+            if (!materialTypeAssetId.IsSuccess())
+            {
+                return Failure();
+            }
 
-#if 1
             MaterialTypeSourceData materialTypeSourceData;
-            AZStd::string materialTypeSourcePath = AssetUtils::ResolvePathReference(materialSourceFilePath, m_materialType);
             if (!AZ::RPI::JsonUtils::LoadObjectFromFile(materialTypeSourcePath, materialTypeSourceData))
             {
                 return Failure();
@@ -203,21 +209,16 @@ namespace AZ
 
             materialTypeSourceData.ResolveUvEnums();
 
-            auto materialTypeAsset =
+            const auto materialTypeAsset =
-                materialTypeSourceData.CreateMaterialTypeAsset(AZ::Uuid::CreateRandom(), materialTypeSourcePath, elevateWarnings);
+                materialTypeSourceData.CreateMaterialTypeAsset(materialTypeAssetId.GetValue(), materialTypeSourcePath, elevateWarnings);
-            if (!materialTypeAsset.IsSuccess())
-            {
-                return Failure();
-            }
-#else
-            auto materialTypeAsset = AssetUtils::LoadAsset<MaterialTypeAsset>(materialSourceFilePath, m_materialType);
             if (!materialTypeAsset.IsSuccess())
             {
                 return Failure();
             }
-#endif
 
-            materialAssetCreator.Begin(assetId, *materialTypeAsset.GetValue().Get(), includeMaterialPropertyNames);
+            AZStd::unordered_set<AZStd::string> dependencies;
+            dependencies.insert(materialSourceFilePath);
+            dependencies.insert(materialTypeSourcePath);
 
             AZStd::vector<MaterialSourceData> parentSourceDataStack;
 
@@ -225,6 +226,13 @@ namespace AZ
             AZStd::string parentSourceAbsPath = AssetUtils::ResolvePathReference(materialSourceFilePath, parentSourceRelPath);
             while (!parentSourceRelPath.empty())
             {
+                if (dependencies.find(parentSourceAbsPath) != dependencies.end())
+                {
+                    return Failure();
+                }
+
+                dependencies.insert(parentSourceAbsPath);
+
                 MaterialSourceData parentSourceData;
                 if (!AZ::RPI::JsonUtils::LoadObjectFromFile(parentSourceAbsPath, parentSourceData))
                 {
@@ -232,20 +240,22 @@ namespace AZ
                 }
 
                 // Make sure the parent material has the same material type
-                auto materialTypeIdOutcome1 = AssetUtils::MakeAssetId(materialSourceFilePath, m_materialType, 0);
+                const auto parentTypeAssetId = AssetUtils::MakeAssetId(parentSourceAbsPath, parentSourceData.m_materialType, 0);
-                auto materialTypeIdOutcome2 = AssetUtils::MakeAssetId(parentSourceAbsPath, parentSourceData.m_materialType, 0);
+                if (!parentTypeAssetId || parentTypeAssetId.GetValue() != materialTypeAssetId.GetValue())
-                if (!materialTypeIdOutcome1.IsSuccess() || !materialTypeIdOutcome2.IsSuccess() ||
-                    materialTypeIdOutcome1.GetValue() != materialTypeIdOutcome2.GetValue())
                 {
                     AZ_Error("MaterialSourceData", false, "This material and its parent material do not share the same material type.");
                     return Failure();
                 }
 
-                parentSourceDataStack.push_back(parentSourceData);
                 parentSourceRelPath = parentSourceData.m_parentMaterial;
                 parentSourceAbsPath = AssetUtils::ResolvePathReference(parentSourceAbsPath, parentSourceRelPath);
+                parentSourceDataStack.emplace_back(AZStd::move(parentSourceData));
             }
 
+            MaterialAssetCreator materialAssetCreator;
+            materialAssetCreator.SetElevateWarnings(elevateWarnings);
+            materialAssetCreator.Begin(assetId, *materialTypeAsset.GetValue().Get(), includeMaterialPropertyNames);
+
             while (!parentSourceDataStack.empty())
             {
                 parentSourceDataStack.back().ApplyPropertiesToAssetCreator(materialAssetCreator, materialSourceFilePath);
@@ -257,12 +267,15 @@ namespace AZ
             Data::Asset<MaterialAsset> material;
             if (materialAssetCreator.End(material))
             {
+                if (sourceDependencies)
+                {
+                    sourceDependencies->insert(dependencies.begin(), dependencies.end());
+                }
+
                 return Success(material);
             }
-            else
+
-            {
+            return Failure();
-                return Failure();
-            }
         }
 
         void MaterialSourceData::ApplyPropertiesToAssetCreator(

Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp

@@ -398,8 +398,6 @@ namespace AZ
                 if (shaderAssetResult)
                 {
                     auto shaderAsset = shaderAssetResult.GetValue();
-                    shaderAsset.SetAutoLoadBehavior(Data::AssetLoadBehavior::PreLoad);
-
                     auto optionsLayout = shaderAsset->GetShaderOptionGroupLayout();
                     ShaderOptionGroup options{ optionsLayout };
                     for (auto& iter : shaderRef.m_shaderOptionValues)
@@ -501,7 +499,6 @@ namespace AZ
                             if (imageAssetResult)
                             {
                                 auto imageAsset = imageAssetResult.GetValue();
-                                imageAsset.SetAutoLoadBehavior(Data::AssetLoadBehavior::PreLoad);
                                 materialTypeAssetCreator.SetPropertyValue(propertyId.GetFullName(), imageAsset);
                             }
                             else

Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocumentSystemComponent.cpp

@@ -159,7 +159,7 @@ namespace AtomToolsFramework
 
     void AtomToolsDocumentSystemComponent::OnDocumentExternallyModified(const AZ::Uuid& documentId)
     {
-        m_documentIdsToReopen.insert(documentId);
+        m_documentIdsWithExternalChanges.insert(documentId);
         if (!AZ::TickBus::Handler::BusIsConnected())
         {
             AZ::TickBus::Handler::BusConnect();
@@ -168,7 +168,7 @@ namespace AtomToolsFramework
 
     void AtomToolsDocumentSystemComponent::OnDocumentDependencyModified(const AZ::Uuid& documentId)
     {
-        m_documentIdsToReopen.insert(documentId);
+        m_documentIdsWithDependencyChanges.insert(documentId);
         if (!AZ::TickBus::Handler::BusIsConnected())
         {
             AZ::TickBus::Handler::BusConnect();
@@ -177,7 +177,7 @@ namespace AtomToolsFramework
 
     void AtomToolsDocumentSystemComponent::OnTick([[maybe_unused]] float deltaTime, [[maybe_unused]] AZ::ScriptTimePoint time)
     {
-        for (const AZ::Uuid& documentId : m_documentIdsToReopen)
+        for (const AZ::Uuid& documentId : m_documentIdsWithExternalChanges)
         {
             AZStd::string documentPath;
             AtomToolsDocumentRequestBus::EventResult(documentPath, documentId, &AtomToolsDocumentRequestBus::Events::GetAbsolutePath);
@@ -191,6 +191,8 @@ namespace AtomToolsFramework
                 continue;
             }
 
+            m_documentIdsWithDependencyChanges.erase(documentId);
+
             AtomToolsFramework::TraceRecorder traceRecorder(m_maxMessageBoxLineCount);
 
             bool openResult = false;
@@ -204,7 +206,7 @@ namespace AtomToolsFramework
             }
         }
 
-        for (const AZ::Uuid& documentId : m_documentIdsToReopen)
+        for (const AZ::Uuid& documentId : m_documentIdsWithDependencyChanges)
         {
             AZStd::string documentPath;
             AtomToolsDocumentRequestBus::EventResult(documentPath, documentId, &AtomToolsDocumentRequestBus::Events::GetAbsolutePath);
@@ -231,8 +233,8 @@ namespace AtomToolsFramework
             }
         }
 
-        m_documentIdsToReopen.clear();
+        m_documentIdsWithDependencyChanges.clear();
-        m_documentIdsToReopen.clear();
+        m_documentIdsWithExternalChanges.clear();
         AZ::TickBus::Handler::BusDisconnect();
     }
 

Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocumentSystemComponent.h

@@ -85,8 +85,8 @@ namespace AtomToolsFramework
         AZStd::intrusive_ptr<AtomToolsDocumentSystemSettings> m_settings;
         AZStd::function<AtomToolsDocument*()> m_documentCreator;
         AZStd::unordered_map<AZ::Uuid, AZStd::shared_ptr<AtomToolsDocument>> m_documentMap;
-        AZStd::unordered_set<AZ::Uuid> m_documentIdsToRebuild;
+        AZStd::unordered_set<AZ::Uuid> m_documentIdsWithExternalChanges;
-        AZStd::unordered_set<AZ::Uuid> m_documentIdsToReopen;
+        AZStd::unordered_set<AZ::Uuid> m_documentIdsWithDependencyChanges;
         const size_t m_maxMessageBoxLineCount = 15;
     };
 } // namespace AtomToolsFramework

Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp

@@ -567,26 +567,26 @@ namespace MaterialEditor
         }
     }
 
-    void MaterialDocument::SourceFileChanged(AZStd::string relativePath, AZStd::string scanFolder, AZ::Uuid sourceUUID)
+    void MaterialDocument::SourceFileChanged(AZStd::string relativePath, AZStd::string scanFolder, [[maybe_unused]] AZ::Uuid sourceUUID)
     {
-        if (m_sourceAssetId.m_guid == sourceUUID)
+        auto sourcePath = AZ::RPI::AssetUtils::ResolvePathReference(scanFolder, relativePath);
+
+        if (m_absolutePath == sourcePath)
         {
             // ignore notifications caused by saving the open document
             if (!m_saveTriggeredInternally)
             {
                 AZ_TracePrintf("MaterialDocument", "Material document changed externally: '%s'.\n", m_absolutePath.c_str());
-                AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast(&AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentExternallyModified, m_id);
+                AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast(
+                    &AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentExternallyModified, m_id);
             }
             m_saveTriggeredInternally = false;
         }
-    }
+        else if (m_sourceDependencies.find(sourcePath) != m_sourceDependencies.end())
-
-    void MaterialDocument::OnAssetReloaded(AZ::Data::Asset<AZ::Data::AssetData> asset)
-    {
-        if (m_dependentAssetIds.find(asset->GetId()) != m_dependentAssetIds.end())
         {
             AZ_TracePrintf("MaterialDocument", "Material document dependency changed: '%s'.\n", m_absolutePath.c_str());
-            AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast(&AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentDependencyModified, m_id);
+            AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast(
+                &AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentDependencyModified, m_id);
         }
     }
 
@@ -655,7 +655,6 @@ namespace MaterialEditor
             return false;
         }
 
-        m_sourceAssetId = sourceAssetInfo.m_assetId;
         m_relativePath = sourceAssetInfo.m_relativePath;
         if (!AzFramework::StringFunc::Path::Normalize(m_relativePath))
         {
@@ -722,14 +721,15 @@ namespace MaterialEditor
         // we can create the asset dynamically from the source data.
         // Long term, the material document should not be concerned with assets at all. The viewport window should be the
         // only thing concerned with assets or instances.
-        auto createResult = m_materialSourceData.CreateMaterialAssetFromSourceData(Uuid::CreateRandom(), m_absolutePath, true);
+        auto materialAssetResult =
-        if (!createResult)
+            m_materialSourceData.CreateMaterialAssetFromSourceData(Uuid::CreateRandom(), m_absolutePath, true, true, &m_sourceDependencies);
+        if (!materialAssetResult)
         {
             AZ_Error("MaterialDocument", false, "Material asset could not be created from source data: '%s'.", m_absolutePath.c_str());
             return false;
         }
 
-        m_materialAsset = createResult.GetValue();
+        m_materialAsset = materialAssetResult.GetValue();
         if (!m_materialAsset.IsReady())
         {
             AZ_Error("MaterialDocument", false, "Material asset is not ready: '%s'.", m_absolutePath.c_str());
@@ -743,28 +743,35 @@ namespace MaterialEditor
             return false;
         }
 
-        // track material type asset to notify when dependencies change
-        m_dependentAssetIds.insert(materialTypeAsset->GetId());
-        AZ::Data::AssetBus::MultiHandler::BusConnect(materialTypeAsset->GetId());
-
         AZStd::array_view<AZ::RPI::MaterialPropertyValue> parentPropertyValues = materialTypeAsset->GetDefaultPropertyValues();
         AZ::Data::Asset<MaterialAsset> parentMaterialAsset;
         if (!m_materialSourceData.m_parentMaterial.empty())
         {
-            // There is a parent for this material
+            AZ::RPI::MaterialSourceData parentMaterialSourceData;
-            auto parentMaterialResult = AssetUtils::LoadAsset<MaterialAsset>(m_absolutePath, m_materialSourceData.m_parentMaterial);
+            const auto parentMaterialFilePath = AssetUtils::ResolvePathReference(m_absolutePath, m_materialSourceData.m_parentMaterial);
-            if (!parentMaterialResult)
+            if (!AZ::RPI::JsonUtils::LoadObjectFromFile(parentMaterialFilePath, parentMaterialSourceData))
             {
-                AZ_Error("MaterialDocument", false, "Parent material asset could not be loaded: '%s'.", m_materialSourceData.m_parentMaterial.c_str());
+                AZ_Error("MaterialDocument", false, "Material parent source data could not be loaded for: '%s'.", parentMaterialFilePath.c_str());
                 return false;
             }
 
-            parentMaterialAsset = parentMaterialResult.GetValue();
+            const auto parentMaterialAssetIdResult = AssetUtils::MakeAssetId(parentMaterialFilePath, 0);
-            parentPropertyValues = parentMaterialAsset->GetPropertyValues();
+            if (!parentMaterialAssetIdResult)
+            {
+                AZ_Error("MaterialDocument", false, "Material parent asset ID could not be created: '%s'.", parentMaterialFilePath.c_str());
+                return false;
+            }
 
-            // track parent material asset to notify when dependencies change
+            auto parentMaterialAssetResult = m_materialSourceData.CreateMaterialAssetFromSourceData(
-            m_dependentAssetIds.insert(parentMaterialAsset->GetId());
+                parentMaterialAssetIdResult.GetValue(), parentMaterialFilePath, true, true, &m_sourceDependencies);
-            AZ::Data::AssetBus::MultiHandler::BusConnect(parentMaterialAsset->GetId());
+            if (!parentMaterialAssetResult)
+            {
+                AZ_Error("MaterialDocument", false, "Material parent asset could not be created from source data: '%s'.", parentMaterialFilePath.c_str());
+                return false;
+            }
+
+            parentMaterialAsset = parentMaterialAssetResult.GetValue();
+            parentPropertyValues = parentMaterialAsset->GetPropertyValues();
         }
 
         // Creating a material from a material asset will fail if a texture is referenced but not loaded 
@@ -913,15 +920,13 @@ namespace MaterialEditor
     void MaterialDocument::Clear()
     {
         AZ::TickBus::Handler::BusDisconnect();
-        AZ::Data::AssetBus::MultiHandler::BusDisconnect();
         AzToolsFramework::AssetSystemBus::Handler::BusDisconnect();
 
         m_materialAsset = {};
         m_materialInstance = {};
         m_absolutePath.clear();
         m_relativePath.clear();
-        m_sourceAssetId = {};
+        m_sourceDependencies.clear();
-        m_dependentAssetIds.clear();
         m_saveTriggeredInternally = {};
         m_compilePending = {};
         m_properties.clear();

Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.h

@@ -29,7 +29,6 @@ namespace MaterialEditor
         : public AtomToolsFramework::AtomToolsDocument
         , public MaterialDocumentRequestBus::Handler
         , private AZ::TickBus::Handler
-        , private AZ::Data::AssetBus::MultiHandler
         , private AzToolsFramework::AssetSystemBus::Handler
     {
     public:
@@ -105,11 +104,6 @@ namespace MaterialEditor
         void SourceFileChanged(AZStd::string relativePath, AZStd::string scanFolder, AZ::Uuid sourceUUID) override;
         //////////////////////////////////////////////////////////////////////////
 
-        //////////////////////////////////////////////////////////////////////////
-        // AZ::Data::AssetBus::Router overrides...
-        void OnAssetReloaded(AZ::Data::Asset<AZ::Data::AssetData> asset) override;
-        //////////////////////////////////////////////////////////////////////////
-
         bool SavePropertiesToSourceData(AZ::RPI::MaterialSourceData& sourceData, PropertyFilterFunction propertyFilter) const;
 
         bool OpenInternal(AZStd::string_view loadPath);
@@ -137,11 +131,8 @@ namespace MaterialEditor
         // Material instance being edited
         AZ::Data::Instance<AZ::RPI::Material> m_materialInstance;
 
-        // Asset used to open document
-        AZ::Data::AssetId m_sourceAssetId;
-
         // Set of assets that can trigger a document reload
-        AZStd::unordered_set<AZ::Data::AssetId> m_dependentAssetIds;
+        AZStd::unordered_set<AZStd::string> m_sourceDependencies;
 
         // Track if document saved itself last to skip external modification notification
         bool m_saveTriggeredInternally = false;