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>
2021-11-04 12:27:18 -05:00
parent 48f3bb7d7a
commit c0acbe7bd4
8 changed files with 86 additions and 76 deletions
@@ -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(
@@ -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;
            }
@@ -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;
-            materialAssetCreator.SetElevateWarnings(elevateWarnings);
+            const auto materialTypeSourcePath = AssetUtils::ResolvePathReference(materialSourceFilePath, m_materialType);
+            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 =
-                materialTypeSourceData.CreateMaterialTypeAsset(AZ::Uuid::CreateRandom(), materialTypeSourcePath, elevateWarnings);
+            const auto materialTypeAsset =
+                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);
-                auto materialTypeIdOutcome2 = AssetUtils::MakeAssetId(parentSourceAbsPath, parentSourceData.m_materialType, 0);
-                if (!materialTypeIdOutcome1.IsSuccess() || !materialTypeIdOutcome2.IsSuccess() ||
-                    materialTypeIdOutcome1.GetValue() != materialTypeIdOutcome2.GetValue())
+                const auto parentTypeAssetId = AssetUtils::MakeAssetId(parentSourceAbsPath, parentSourceData.m_materialType, 0);
+                if (!parentTypeAssetId || parentTypeAssetId.GetValue() != materialTypeAssetId.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(
@@ -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
@@ -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_documentIdsToReopen.clear();
+        m_documentIdsWithDependencyChanges.clear();
+        m_documentIdsWithExternalChanges.clear();
        AZ::TickBus::Handler::BusDisconnect();
    }

@@ -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_documentIdsToReopen;
+        AZStd::unordered_set<AZ::Uuid> m_documentIdsWithExternalChanges;
+        AZStd::unordered_set<AZ::Uuid> m_documentIdsWithDependencyChanges;
        const size_t m_maxMessageBoxLineCount = 15;
    };
 } // namespace AtomToolsFramework
@@ -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;
        }
-    }
-
-    void MaterialDocument::OnAssetReloaded(AZ::Data::Asset<AZ::Data::AssetData> asset)
-    {
-        if (m_dependentAssetIds.find(asset->GetId()) != m_dependentAssetIds.end())
+        else if (m_sourceDependencies.find(sourcePath) != m_sourceDependencies.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);
-        if (!createResult)
+        auto materialAssetResult =
+            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
-            auto parentMaterialResult = AssetUtils::LoadAsset<MaterialAsset>(m_absolutePath, m_materialSourceData.m_parentMaterial);
-            if (!parentMaterialResult)
+            AZ::RPI::MaterialSourceData parentMaterialSourceData;
+            const auto parentMaterialFilePath = AssetUtils::ResolvePathReference(m_absolutePath, m_materialSourceData.m_parentMaterial);
+            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();
-            parentPropertyValues = parentMaterialAsset->GetPropertyValues();
+            const auto parentMaterialAssetIdResult = AssetUtils::MakeAssetId(parentMaterialFilePath, 0);
+            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
-            m_dependentAssetIds.insert(parentMaterialAsset->GetId());
-            AZ::Data::AssetBus::MultiHandler::BusConnect(parentMaterialAsset->GetId());
+            auto parentMaterialAssetResult = m_materialSourceData.CreateMaterialAssetFromSourceData(
+                parentMaterialAssetIdResult.GetValue(), parentMaterialFilePath, true, true, &m_sourceDependencies);
+            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_dependentAssetIds.clear();
+        m_sourceDependencies.clear();
        m_saveTriggeredInternally = {};
        m_compilePending = {};
        m_properties.clear();
@@ -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;