From dc598a8b3cadd4aeaf60b0ad8ef3441185a8e079 Mon Sep 17 00:00:00 2001 From: Guthrie Adams Date: Sun, 30 Jan 2022 00:33:02 -0600 Subject: [PATCH 1/7] =?UTF-8?q?Atom=20Tools:=20move=20boilerplate=20docume?= =?UTF-8?q?nt=20management=20code=20to=20atom=20tools=20framework=20?= =?UTF-8?q?=E2=80=A2=20Moved=20all=20of=20the=20common=20save=20and=20load?= =?UTF-8?q?=20code=20to=20the=20document=20base=20class=20=E2=80=A2=20Move?= =?UTF-8?q?d=20undo=20and=20redo=20support=20to=20document=20base=20class?= =?UTF-8?q?=20but=20will=20probably=20extract=20to=20its=20own=20class=20o?= =?UTF-8?q?r=20replace=20with=20one=20from=20AzTF=20=E2=80=A2=20Streamline?= =?UTF-8?q?d=20material=20editor,=20shader=20management=20console,=20and?= =?UTF-8?q?=20other=20tools=20with=20updated=20document=20code=20=E2=80=A2?= =?UTF-8?q?=20Cleaned=20up=20some=20of=20shader=20management=20console=20l?= =?UTF-8?q?oading=20code,=20added=20support=20for=20saving,=20as=20well=20?= =?UTF-8?q?as=20getting=20and=20setting=20the=20shader=20variant=20list=20?= =?UTF-8?q?source=20data=20structure?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Guthrie Adams --- .../Document/AtomToolsDocument.h | 54 +- .../Document/AtomToolsDocumentRequestBus.h | 3 - .../DynamicProperty/DynamicProperty.h | 2 +- .../Source/Document/AtomToolsDocument.cpp | 269 ++++++++-- .../AtomToolsDocumentSystemComponent.cpp | 1 - .../Code/Source/Document/MaterialDocument.cpp | 486 +++++------------- .../Code/Source/Document/MaterialDocument.h | 61 +-- .../Viewport/MaterialViewportComponent.cpp | 24 +- .../MaterialInspector/MaterialInspector.h | 3 +- .../ViewportSettingsInspector.h | 3 +- .../ShaderManagementConsoleDocument.cpp | 143 +++--- .../ShaderManagementConsoleDocument.h | 37 +- ...haderManagementConsoleDocumentRequestBus.h | 14 +- .../ShaderManagementConsoleApplication.cpp | 2 + .../Window/ShaderManagementConsoleWindow.cpp | 2 - 15 files changed, 548 insertions(+), 556 deletions(-) diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocument.h b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocument.h index bf18d36c2c..b7188286a1 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocument.h +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocument.h @@ -7,8 +7,9 @@ */ #pragma once -#include #include +#include +#include namespace AtomToolsFramework { @@ -17,6 +18,7 @@ namespace AtomToolsFramework */ class AtomToolsDocument : public AtomToolsDocumentRequestBus::Handler + , private AzToolsFramework::AssetSystemBus::Handler { public: AZ_RTTI(AtomToolsDocument, "{8992DF74-88EC-438C-B280-6E71D4C0880B}"); @@ -28,10 +30,8 @@ namespace AtomToolsFramework const AZ::Uuid& GetId() const; - //////////////////////////////////////////////////////////////////////// - // AtomToolsDocumentRequestBus::Handler implementation + // AtomToolsDocumentRequestBus::Handler overrides... AZStd::string_view GetAbsolutePath() const override; - AZStd::string_view GetRelativePath() const override; const AZStd::any& GetPropertyValue(const AZ::Name& propertyId) const override; const AtomToolsFramework::DynamicProperty& GetProperty(const AZ::Name& propertyId) const override; bool IsPropertyGroupVisible(const AZ::Name& propertyGroupFullName) const override; @@ -51,21 +51,59 @@ namespace AtomToolsFramework bool Redo() override; bool BeginEdit() override; bool EndEdit() override; - //////////////////////////////////////////////////////////////////////// protected: + virtual void Clear(); + + virtual bool OpenSucceeded(); + virtual bool OpenFailed(); + + virtual bool ReopenRecordState(); + virtual bool ReopenRestoreState(); + + virtual bool SaveSucceeded(); + virtual bool SaveFailed(); // Unique id of this document AZ::Uuid m_id = AZ::Uuid::CreateRandom(); - // Relative path to the material source file - AZStd::string m_relativePath; - // Absolute path to the material source file AZStd::string m_absolutePath; + AZStd::string m_savePathNormalized; + AZStd::any m_invalidValue; AtomToolsFramework::DynamicProperty m_invalidProperty; + + // Set of assets that can trigger a document reload + AZStd::unordered_set m_sourceDependencies; + + // Track if document saved itself last to skip external modification notification + bool m_saveTriggeredInternally = false; + + // Variables needed for tracking the undo and redo state of this document + + // Function to be bound for undo and redo + using UndoRedoFunction = AZStd::function; + + // A pair of functions, where first is the undo operation and second is the redo operation + using UndoRedoFunctionPair = AZStd::pair; + + // Container for all of the active undo and redo functions and state + using UndoRedoHistory = AZStd::vector; + + // Container of undo commands + UndoRedoHistory m_undoHistory; + UndoRedoHistory m_undoHistoryBeforeReopen; + + // The current position in the undo redo history + int m_undoHistoryIndex = {}; + int m_undoHistoryIndexBeforeReopen = {}; + + void AddUndoRedoHistory(const UndoRedoFunction& undoCommand, const UndoRedoFunction& redoCommand); + + // AzToolsFramework::AssetSystemBus::Handler overrides... + void SourceFileChanged(AZStd::string relativePath, AZStd::string scanFolder, AZ::Uuid sourceUUID) override; }; } // namespace AtomToolsFramework diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocumentRequestBus.h b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocumentRequestBus.h index 6a21a8a2df..b6b3f65110 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocumentRequestBus.h +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocumentRequestBus.h @@ -25,9 +25,6 @@ namespace AtomToolsFramework //! Get absolute path of document virtual AZStd::string_view GetAbsolutePath() const = 0; - //! Get relative path of document - virtual AZStd::string_view GetRelativePath() const = 0; - //! Return property value //! If the document is not open or the id can't be found, an invalid value is returned instead. virtual const AZStd::any& GetPropertyValue(const AZ::Name& propertyFullName) const = 0; diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/DynamicProperty/DynamicProperty.h b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/DynamicProperty/DynamicProperty.h index 68e5e38cbd..5af0fcc845 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/DynamicProperty/DynamicProperty.h +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/DynamicProperty/DynamicProperty.h @@ -111,7 +111,7 @@ namespace AtomToolsFramework AZStd::string GetDescription() const; AZStd::vector> GetEnumValues() const; - // Handles changes from the ReflectedPropertyEditor and sends notification to the material document. + // Handles changes from the ReflectedPropertyEditor and sends notification. AZ::u32 OnDataChanged() const; template diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocument.cpp b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocument.cpp index 1e2c685c3c..5941360b5d 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocument.cpp +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocument.cpp @@ -6,8 +6,10 @@ * */ +#include #include #include +#include namespace AtomToolsFramework { @@ -19,6 +21,7 @@ namespace AtomToolsFramework AtomToolsDocument::~AtomToolsDocument() { + AzToolsFramework::AssetSystemBus::Handler::BusDisconnect(); AtomToolsDocumentNotificationBus::Broadcast(&AtomToolsDocumentNotificationBus::Events::OnDocumentDestroyed, m_id); AtomToolsDocumentRequestBus::Handler::BusDisconnect(); } @@ -33,11 +36,6 @@ namespace AtomToolsFramework return m_absolutePath; } - AZStd::string_view AtomToolsDocument::GetRelativePath() const - { - return m_relativePath; - } - const AZStd::any& AtomToolsDocument::GetPropertyValue([[maybe_unused]] const AZ::Name& propertyId) const { AZ_UNUSED(propertyId); @@ -66,49 +64,140 @@ namespace AtomToolsFramework AZ_Error("AtomToolsDocument", false, "%s not implemented.", __FUNCTION__); } - bool AtomToolsDocument::Open([[maybe_unused]] AZStd::string_view loadPath) + bool AtomToolsDocument::Open(AZStd::string_view loadPath) { - AZ_UNUSED(loadPath); - AZ_Error("AtomToolsDocument", false, "%s not implemented.", __FUNCTION__); - return false; + Clear(); + + m_absolutePath = loadPath; + if (!AzFramework::StringFunc::Path::Normalize(m_absolutePath)) + { + AZ_Error("AtomToolsDocument", false, "Document path could not be normalized: '%s'.", m_absolutePath.c_str()); + return OpenFailed(); + } + + if (AzFramework::StringFunc::Path::IsRelative(m_absolutePath.c_str())) + { + AZ_Error("AtomToolsDocument", false, "Document path must be absolute: '%s'.", m_absolutePath.c_str()); + return OpenFailed(); + } + + return true; } bool AtomToolsDocument::Reopen() { - AZ_Error("AtomToolsDocument", false, "%s not implemented.", __FUNCTION__); - return false; + if (!ReopenRecordState()) + { + return false; + } + + const auto loadPath = m_absolutePath; + if (!Open(loadPath)) + { + return false; + } + + if (!ReopenRestoreState()) + { + return false; + } + + return true; } bool AtomToolsDocument::Save() { - AZ_Error("AtomToolsDocument", false, "%s not implemented.", __FUNCTION__); - return false; + m_savePathNormalized = m_absolutePath; + if (!AzFramework::StringFunc::Path::Normalize(m_savePathNormalized)) + { + AZ_Error("AtomToolsDocument", false, "Document save path could not be normalized: '%s'.", m_savePathNormalized.c_str()); + return SaveFailed(); + } + + if (!IsOpen()) + { + AZ_Error("AtomToolsDocument", false, "Document is not open to be saved: '%s'.", m_absolutePath.c_str()); + return SaveFailed(); + } + + if (!IsSavable()) + { + AZ_Error("AtomToolsDocument", false, "Material types can only be saved as a child: '%s'.", m_absolutePath.c_str()); + return SaveFailed(); + } + + return true; } - bool AtomToolsDocument::SaveAsCopy([[maybe_unused]] AZStd::string_view savePath) + bool AtomToolsDocument::SaveAsCopy(AZStd::string_view savePath) { - AZ_UNUSED(savePath); - AZ_Error("AtomToolsDocument", false, "%s not implemented.", __FUNCTION__); - return false; - } + m_savePathNormalized = savePath; + if (!AzFramework::StringFunc::Path::Normalize(m_savePathNormalized)) + { + AZ_Error("AtomToolsDocument", false, "Document save path could not be normalized: '%s'.", m_savePathNormalized.c_str()); + return SaveFailed(); + } + + if (!IsOpen()) + { + AZ_Error("AtomToolsDocument", false, "Document is not open to be saved: '%s'.", m_absolutePath.c_str()); + return SaveFailed(); + } + if (!IsSavable()) + { + AZ_Error("AtomToolsDocument", false, "Material types can only be saved as a child: '%s'.", m_absolutePath.c_str()); + return SaveFailed(); + } + + return true; + } - bool AtomToolsDocument::SaveAsChild([[maybe_unused]] AZStd::string_view savePath) + bool AtomToolsDocument::SaveAsChild(AZStd::string_view savePath) { - AZ_UNUSED(savePath); - AZ_Error("AtomToolsDocument", false, "%s not implemented.", __FUNCTION__); - return false; + m_savePathNormalized = savePath; + if (!AzFramework::StringFunc::Path::Normalize(m_savePathNormalized)) + { + AZ_Error("AtomToolsDocument", false, "Document save path could not be normalized: '%s'.", m_savePathNormalized.c_str()); + return SaveFailed(); + } + + if (!IsOpen()) + { + AZ_Error("AtomToolsDocument", false, "Document is not open to be saved: '%s'.", m_absolutePath.c_str()); + return SaveFailed(); + } + + if (m_absolutePath == m_savePathNormalized || m_sourceDependencies.find(m_savePathNormalized) != m_sourceDependencies.end()) + { + AZ_Error("AtomToolsDocument", false, "Document can't be saved over a dependancy: '%s'.", m_savePathNormalized.c_str()); + return SaveFailed(); + } + + return true; } bool AtomToolsDocument::Close() { - AZ_Error("AtomToolsDocument", false, "%s not implemented.", __FUNCTION__); - return false; + if (!IsOpen()) + { + AZ_Error("AtomToolsDocument", false, "Document is not open."); + return false; + } + + AZ_TracePrintf("AtomToolsDocument", "Document closed: '%s'.\n", m_absolutePath.c_str()); + + AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast( + &AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentClosed, m_id); + + // Clearing after notification so paths are still available + Clear(); + return true; } bool AtomToolsDocument::IsOpen() const { - return false; + return !m_id.IsNull() && !m_absolutePath.empty(); } bool AtomToolsDocument::IsModified() const @@ -123,23 +212,41 @@ namespace AtomToolsFramework bool AtomToolsDocument::CanUndo() const { - return false; + // Undo will only be allowed if something has been recorded and we're not at the beginning of history + return IsOpen() && !m_undoHistory.empty() && m_undoHistoryIndex > 0; } bool AtomToolsDocument::CanRedo() const { - return false; + // Redo will only be allowed if something has been recorded and we're not at the end of history + return IsOpen() && !m_undoHistory.empty() && m_undoHistoryIndex < m_undoHistory.size(); } bool AtomToolsDocument::Undo() { - AZ_Error("AtomToolsDocument", false, "%s not implemented.", __FUNCTION__); + if (CanUndo()) + { + // The history index is one beyond the last executed command. Decrement the index then execute undo. + m_undoHistory[--m_undoHistoryIndex].first(); + AZ_TracePrintf("AtomToolsDocument", "Document undo: '%s'.\n", m_absolutePath.c_str()); + AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast( + &AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentUndoStateChanged, m_id); + return true; + } return false; } bool AtomToolsDocument::Redo() { - AZ_Error("AtomToolsDocument", false, "%s not implemented.", __FUNCTION__); + if (CanRedo()) + { + // Execute the current redo command then move the history index to the next position. + m_undoHistory[m_undoHistoryIndex++].second(); + AZ_TracePrintf("AtomToolsDocument", "Document redo: '%s'.\n", m_absolutePath.c_str()); + AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast( + &AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentUndoStateChanged, m_id); + return true; + } return false; } @@ -154,4 +261,108 @@ namespace AtomToolsFramework AZ_Error("AtomToolsDocument", false, "%s not implemented.", __FUNCTION__); return false; } + + void AtomToolsDocument::Clear() + { + AzToolsFramework::AssetSystemBus::Handler::BusDisconnect(); + + m_absolutePath.clear(); + m_sourceDependencies.clear(); + m_saveTriggeredInternally = {}; + m_undoHistory.clear(); + m_undoHistoryIndex = {}; + } + + bool AtomToolsDocument::OpenSucceeded() + { + AZ_TracePrintf("AtomToolsDocument", "Document opened: '%s'.\n", m_absolutePath.c_str()); + AzToolsFramework::AssetSystemBus::Handler::BusConnect(); + AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast( + &AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentOpened, m_id); + return true; + } + + bool AtomToolsDocument::OpenFailed() + { + AZ_TracePrintf("AtomToolsDocument", "Document could not opened: '%s'.\n", m_absolutePath.c_str()); + Clear(); + return false; + } + + bool AtomToolsDocument::ReopenRecordState() + { + // Store history and property changes that should be reapplied after reload + m_undoHistoryBeforeReopen = m_undoHistory; + m_undoHistoryIndexBeforeReopen = m_undoHistoryIndex; + return true; + } + + bool AtomToolsDocument::ReopenRestoreState() + { + m_undoHistory = m_undoHistoryBeforeReopen; + m_undoHistoryIndex = m_undoHistoryIndexBeforeReopen; + m_undoHistoryBeforeReopen = {}; + m_undoHistoryIndexBeforeReopen = {}; + return true; + } + + bool AtomToolsDocument::SaveSucceeded() + { + m_saveTriggeredInternally = true; + + AZ_TracePrintf("AtomToolsDocument", "Document saved: '%s'.\n", m_savePathNormalized.c_str()); + + // Auto add or checkout saved file + AzToolsFramework::SourceControlCommandBus::Broadcast( + &AzToolsFramework::SourceControlCommandBus::Events::RequestEdit, m_savePathNormalized.c_str(), true, + [](bool, const AzToolsFramework::SourceControlFileInfo&) {}); + + AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast( + &AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentSaved, m_id); + return true; + } + + bool AtomToolsDocument::SaveFailed() + { + AZ_TracePrintf("AtomToolsDocument", "Document not saved: '%s'.\n", m_savePathNormalized.c_str()); + return false; + } + + void AtomToolsDocument::AddUndoRedoHistory(const UndoRedoFunction& undoCommand, const UndoRedoFunction& redoCommand) + { + // Wipe any state beyond the current history index + m_undoHistory.erase(m_undoHistory.begin() + m_undoHistoryIndex, m_undoHistory.end()); + + // Add undo and redo operations using functions that capture state and restore it when executed + m_undoHistory.emplace_back(undoCommand, redoCommand); + + // Assign the index to the end of history + m_undoHistoryIndex = aznumeric_cast(m_undoHistory.size()); + AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast( + &AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentUndoStateChanged, m_id); + } + + void AtomToolsDocument::SourceFileChanged(AZStd::string relativePath, AZStd::string scanFolder, [[maybe_unused]] AZ::Uuid sourceUUID) + { + const 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("AtomToolsDocument", "Document changed externally: '%s'.\n", m_absolutePath.c_str()); + AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast( + &AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentExternallyModified, m_id); + } + m_saveTriggeredInternally = false; + } + else if (m_sourceDependencies.find(sourcePath) != m_sourceDependencies.end()) + { + AZ_TracePrintf("AtomToolsDocument", "Document dependency changed: '%s'.\n", m_absolutePath.c_str()); + AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast( + &AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentDependencyModified, m_id); + } + } + } // namespace AtomToolsFramework diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocumentSystemComponent.cpp b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocumentSystemComponent.cpp index d554c68451..b5a3f5322c 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocumentSystemComponent.cpp +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocumentSystemComponent.cpp @@ -77,7 +77,6 @@ namespace AtomToolsFramework ->Attribute(AZ::Script::Attributes::Category, "Editor") ->Attribute(AZ::Script::Attributes::Module, "atomtools") ->Event("GetAbsolutePath", &AtomToolsDocumentRequestBus::Events::GetAbsolutePath) - ->Event("GetRelativePath", &AtomToolsDocumentRequestBus::Events::GetRelativePath) ->Event("GetPropertyValue", &AtomToolsDocumentRequestBus::Events::GetPropertyValue) ->Event("SetPropertyValue", &AtomToolsDocumentRequestBus::Events::SetPropertyValue) ->Event("Open", &AtomToolsDocumentRequestBus::Events::Open) diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp index c441762710..a8404baec1 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp @@ -12,16 +12,11 @@ #include #include #include -#include -#include -#include #include #include #include #include #include -#include -#include #include namespace MaterialEditor @@ -30,14 +25,12 @@ namespace MaterialEditor : AtomToolsFramework::AtomToolsDocument() { MaterialDocumentRequestBus::Handler::BusConnect(m_id); - AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast(&AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentCreated, m_id); } MaterialDocument::~MaterialDocument() { - AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast(&AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentDestroyed, m_id); MaterialDocumentRequestBus::Handler::BusDisconnect(); - Clear(); + AZ::TickBus::Handler::BusDisconnect(); } AZ::Data::Asset MaterialDocument::GetAsset() const @@ -62,19 +55,16 @@ namespace MaterialEditor const AZStd::any& MaterialDocument::GetPropertyValue(const AZ::Name& propertyId) const { - using namespace AZ; - using namespace RPI; - if (!IsOpen()) { - AZ_Error("MaterialDocument", false, "Material document is not open."); + AZ_Error("MaterialDocument", false, "Document is not open."); return m_invalidValue; } const auto it = m_properties.find(propertyId); if (it == m_properties.end()) { - AZ_Error("MaterialDocument", false, "Material document property could not be found: '%s'.", propertyId.GetCStr()); + AZ_Error("MaterialDocument", false, "Document property could not be found: '%s'.", propertyId.GetCStr()); return m_invalidValue; } @@ -86,14 +76,14 @@ namespace MaterialEditor { if (!IsOpen()) { - AZ_Error("MaterialDocument", false, "Material document is not open."); + AZ_Error("MaterialDocument", false, "Document is not open."); return m_invalidProperty; } const auto it = m_properties.find(propertyId); if (it == m_properties.end()) { - AZ_Error("MaterialDocument", false, "Material document property could not be found: '%s'.", propertyId.GetCStr()); + AZ_Error("MaterialDocument", false, "Document property could not be found: '%s'.", propertyId.GetCStr()); return m_invalidProperty; } @@ -105,14 +95,14 @@ namespace MaterialEditor { if (!IsOpen()) { - AZ_Error("MaterialDocument", false, "Material document is not open."); + AZ_Error("MaterialDocument", false, "Document is not open."); return false; } const auto it = m_propertyGroupVisibility.find(propertyGroupFullName); if (it == m_propertyGroupVisibility.end()) { - AZ_Error("MaterialDocument", false, "Material document property group could not be found: '%s'.", propertyGroupFullName.GetCStr()); + AZ_Error("MaterialDocument", false, "Document property group could not be found: '%s'.", propertyGroupFullName.GetCStr()); return false; } @@ -121,25 +111,21 @@ namespace MaterialEditor void MaterialDocument::SetPropertyValue(const AZ::Name& propertyId, const AZStd::any& value) { - using namespace AZ; - using namespace RPI; - if (!IsOpen()) { - AZ_Error("MaterialDocument", false, "Material document is not open."); + AZ_Error("MaterialDocument", false, "Document is not open."); return; } const auto it = m_properties.find(propertyId); if (it == m_properties.end()) { - AZ_Error("MaterialDocument", false, "Material document property could not be found: '%s'.", propertyId.GetCStr()); + AZ_Error("MaterialDocument", false, "Document property could not be found: '%s'.", propertyId.GetCStr()); return; } // This first converts to an acceptable runtime type in case the value came from script - const AZ::RPI::MaterialPropertyValue propertyValue = - AtomToolsFramework::ConvertToRuntimeType(value); + const AZ::RPI::MaterialPropertyValue propertyValue = AtomToolsFramework::ConvertToRuntimeType(value); AtomToolsFramework::DynamicProperty& property = it->second; property.SetValue(AtomToolsFramework::ConvertToEditableType(propertyValue)); @@ -149,88 +135,41 @@ namespace MaterialEditor { if (m_materialInstance->SetPropertyValue(propertyIndex, propertyValue)) { - MaterialPropertyFlags dirtyFlags = m_materialInstance->GetPropertyDirtyFlags(); + AZ::RPI::MaterialPropertyFlags dirtyFlags = m_materialInstance->GetPropertyDirtyFlags(); Recompile(); EditorMaterialFunctorResult result = RunEditorMaterialFunctors(dirtyFlags); - for (const Name& changedPropertyGroupName : result.m_updatedPropertyGroups) + for (const AZ::Name& changedPropertyGroupName : result.m_updatedPropertyGroups) { - AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast(&AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentPropertyGroupVisibilityChanged, m_id, changedPropertyGroupName, IsPropertyGroupVisible(changedPropertyGroupName)); + AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast( + &AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentPropertyGroupVisibilityChanged, m_id, + changedPropertyGroupName, IsPropertyGroupVisible(changedPropertyGroupName)); } - for (const Name& changedPropertyName : result.m_updatedProperties) + for (const AZ::Name& changedPropertyName : result.m_updatedProperties) { - AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast(&AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentPropertyConfigModified, m_id, GetProperty(changedPropertyName)); + AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast( + &AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentPropertyConfigModified, m_id, + GetProperty(changedPropertyName)); } } } - AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast(&AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentPropertyValueModified, m_id, property); - AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast(&AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentModified, m_id); - } - - bool MaterialDocument::Open(AZStd::string_view loadPath) - { - if (!OpenInternal(loadPath)) - { - Clear(); - AZ_Error("MaterialDocument", false, "Material document could not be opened: '%s'.", loadPath.data()); - return false; - } - - AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast(&AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentOpened, m_id); - return true; - } - - bool MaterialDocument::Reopen() - { - // Store history and property changes that should be reapplied after reload - auto undoHistoryToRestore = m_undoHistory; - auto undoHistoryIndexToRestore = m_undoHistoryIndex; - PropertyValueMap propertyValuesToRestore; - for (const auto& propertyPair : m_properties) - { - const AtomToolsFramework::DynamicProperty& property = propertyPair.second; - if (!AtomToolsFramework::ArePropertyValuesEqual(property.GetValue(), property.GetConfig().m_parentValue)) - { - propertyValuesToRestore[property.GetId()] = property.GetValue(); - } - } - - // Reopen the same document - const AZStd::string loadPath = m_absolutePath; - if (!OpenInternal(loadPath)) - { - Clear(); - return false; - } - - RestorePropertyValues(propertyValuesToRestore); - AZStd::swap(undoHistoryToRestore, m_undoHistory); - AZStd::swap(undoHistoryIndexToRestore, m_undoHistoryIndex); - AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast(&AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentOpened, m_id); - return true; + AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast( + &AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentPropertyValueModified, m_id, property); + AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast( + &AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentModified, m_id); } bool MaterialDocument::Save() { - using namespace AZ; - using namespace RPI; - - if (!IsOpen()) + if (!AtomToolsDocument::Save()) { - AZ_Error("MaterialDocument", false, "Material document is not open to be saved: '%s'.", m_absolutePath.c_str()); - return false; - } - - if (!IsSavable()) - { - AZ_Error("MaterialDocument", false, "Material types can only be saved as a child: '%s'.", m_absolutePath.c_str()); return false; } // create source data from properties - MaterialSourceData sourceData; + AZ::RPI::MaterialSourceData sourceData; sourceData.m_materialTypeVersion = m_materialAsset->GetMaterialTypeAsset()->GetVersion(); sourceData.m_materialType = AtomToolsFramework::GetExteralReferencePath(m_absolutePath, m_materialSourceData.m_materialType); sourceData.m_parentMaterial = AtomToolsFramework::GetExteralReferencePath(m_absolutePath, m_materialSourceData.m_parentMaterial); @@ -243,14 +182,14 @@ namespace MaterialEditor if (!savedProperties) { - return false; + return SaveFailed(); } // write sourceData to .material file if (!AZ::RPI::JsonUtils::SaveObjectToFile(m_absolutePath, sourceData)) { - AZ_Error("MaterialDocument", false, "Material document could not be saved: '%s'.", m_absolutePath.c_str()); - return false; + AZ_Error("MaterialDocument", false, "Document could not be saved: '%s'.", m_absolutePath.c_str()); + return SaveFailed(); } // after saving, reset to a clean state @@ -262,181 +201,97 @@ namespace MaterialEditor property.SetConfig(propertyConfig); } - // Auto add or checkout saved file - AzToolsFramework::SourceControlCommandBus::Broadcast(&AzToolsFramework::SourceControlCommandBus::Events::RequestEdit, - m_absolutePath.c_str(), true, [](bool, const AzToolsFramework::SourceControlFileInfo&) {}); - - AZ_TracePrintf("MaterialDocument", "Material document saved: '%s'.\n", m_absolutePath.data()); - - AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast(&AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentSaved, m_id); - - m_saveTriggeredInternally = true; - return true; + return SaveSucceeded(); } bool MaterialDocument::SaveAsCopy(AZStd::string_view savePath) { - using namespace AZ; - using namespace RPI; - - if (!IsOpen()) - { - AZ_Error("MaterialDocument", false, "Material document is not open to be saved: '%s'.", m_absolutePath.c_str()); - return false; - } - - if (!IsSavable()) + if (!AtomToolsDocument::SaveAsCopy(savePath)) { - AZ_Error("MaterialDocument", false, "Material types can only be saved as a child: '%s'.", m_absolutePath.c_str()); - return false; - } - - AZStd::string normalizedSavePath = savePath; - if (!AzFramework::StringFunc::Path::Normalize(normalizedSavePath)) - { - AZ_Error("MaterialDocument", false, "Material document save path could not be normalized: '%s'.", normalizedSavePath.c_str()); return false; } // create source data from properties - MaterialSourceData sourceData; + AZ::RPI::MaterialSourceData sourceData; sourceData.m_materialTypeVersion = m_materialAsset->GetMaterialTypeAsset()->GetVersion(); - sourceData.m_materialType = AtomToolsFramework::GetExteralReferencePath(normalizedSavePath, m_materialSourceData.m_materialType); - sourceData.m_parentMaterial = AtomToolsFramework::GetExteralReferencePath(normalizedSavePath, m_materialSourceData.m_parentMaterial); + sourceData.m_materialType = AtomToolsFramework::GetExteralReferencePath(m_savePathNormalized, m_materialSourceData.m_materialType); + sourceData.m_parentMaterial = AtomToolsFramework::GetExteralReferencePath(m_savePathNormalized, m_materialSourceData.m_parentMaterial); // populate sourceData with modified or overwritten properties - const bool savedProperties = SavePropertiesToSourceData(normalizedSavePath, sourceData, [](const AtomToolsFramework::DynamicProperty& property) + const bool savedProperties = SavePropertiesToSourceData(m_savePathNormalized, sourceData, [](const AtomToolsFramework::DynamicProperty& property) { return !AtomToolsFramework::ArePropertyValuesEqual(property.GetValue(), property.GetConfig().m_parentValue); }); if (!savedProperties) { - return false; + return SaveFailed(); } // write sourceData to .material file - if (!AZ::RPI::JsonUtils::SaveObjectToFile(normalizedSavePath, sourceData)) + if (!AZ::RPI::JsonUtils::SaveObjectToFile(m_savePathNormalized, sourceData)) { - AZ_Error("MaterialDocument", false, "Material document could not be saved: '%s'.", normalizedSavePath.c_str()); - return false; + AZ_Error("MaterialDocument", false, "Document could not be saved: '%s'.", m_savePathNormalized.c_str()); + return SaveFailed(); } - // Auto add or checkout saved file - AzToolsFramework::SourceControlCommandBus::Broadcast(&AzToolsFramework::SourceControlCommandBus::Events::RequestEdit, - normalizedSavePath.c_str(), true, [](bool, const AzToolsFramework::SourceControlFileInfo&) {}); - - AZ_TracePrintf("MaterialDocument", "Material document saved: '%s'.\n", normalizedSavePath.c_str()); - - AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast(&AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentSaved, m_id); - // If the document is saved to a new file we need to reopen the new document to update assets, paths, property deltas. - if (!Open(normalizedSavePath)) + if (!Open(m_savePathNormalized)) { - return false; + return SaveFailed(); } - // Setting flag after reopening becausse it's cleared on open - m_saveTriggeredInternally = true; - return true; + return SaveSucceeded(); } bool MaterialDocument::SaveAsChild(AZStd::string_view savePath) { - using namespace AZ; - using namespace RPI; - - if (!IsOpen()) - { - AZ_Error("MaterialDocument", false, "Material document is not open to be saved: '%s'.", m_absolutePath.c_str()); - return false; - } - - AZStd::string normalizedSavePath = savePath; - if (!AzFramework::StringFunc::Path::Normalize(normalizedSavePath)) + if (!AtomToolsDocument::SaveAsChild(savePath)) { - AZ_Error("MaterialDocument", false, "Material document save path could not be normalized: '%s'.", normalizedSavePath.c_str()); - return false; - } - - if (m_absolutePath == normalizedSavePath) - { - // ToDo: this should scan the entire hierarchy so we don't overwrite parent's parent, for example - AZ_Error("MaterialDocument", false, "Can't overwrite parent material with a child that depends on it."); return false; } // create source data from properties - MaterialSourceData sourceData; + AZ::RPI::MaterialSourceData sourceData; sourceData.m_materialTypeVersion = m_materialAsset->GetMaterialTypeAsset()->GetVersion(); - sourceData.m_materialType = AtomToolsFramework::GetExteralReferencePath(normalizedSavePath, m_materialSourceData.m_materialType); + sourceData.m_materialType = AtomToolsFramework::GetExteralReferencePath(m_savePathNormalized, m_materialSourceData.m_materialType); // Only assign a parent path if the source was a .material - if (AzFramework::StringFunc::Path::IsExtension(m_relativePath.c_str(), MaterialSourceData::Extension)) + if (AzFramework::StringFunc::Path::IsExtension(m_absolutePath.c_str(), AZ::RPI::MaterialSourceData::Extension)) { - sourceData.m_parentMaterial = AtomToolsFramework::GetExteralReferencePath(normalizedSavePath, m_absolutePath); + sourceData.m_parentMaterial = AtomToolsFramework::GetExteralReferencePath(m_savePathNormalized, m_absolutePath); } // populate sourceData with modified properties - const bool savedProperties = SavePropertiesToSourceData(normalizedSavePath, sourceData, [](const AtomToolsFramework::DynamicProperty& property) + const bool savedProperties = SavePropertiesToSourceData(m_savePathNormalized, sourceData, [](const AtomToolsFramework::DynamicProperty& property) { return !AtomToolsFramework::ArePropertyValuesEqual(property.GetValue(), property.GetConfig().m_originalValue); }); if (!savedProperties) { - return false; + return SaveFailed(); } // write sourceData to .material file - if (!AZ::RPI::JsonUtils::SaveObjectToFile(normalizedSavePath, sourceData)) + if (!AZ::RPI::JsonUtils::SaveObjectToFile(m_savePathNormalized, sourceData)) { - AZ_Error("MaterialDocument", false, "Material document could not be saved: '%s'.", normalizedSavePath.c_str()); - return false; + AZ_Error("MaterialDocument", false, "Document could not be saved: '%s'.", m_savePathNormalized.c_str()); + return SaveFailed(); } - // Auto add or checkout saved file - AzToolsFramework::SourceControlCommandBus::Broadcast(&AzToolsFramework::SourceControlCommandBus::Events::RequestEdit, - normalizedSavePath.c_str(), true, [](bool, const AzToolsFramework::SourceControlFileInfo&) {}); - - AZ_TracePrintf("MaterialDocument", "Material document saved: '%s'.\n", normalizedSavePath.c_str()); - - AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast(&AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentSaved, m_id); - // If the document is saved to a new file we need to reopen the new document to update assets, paths, property deltas. - if (!Open(normalizedSavePath)) + if (!Open(m_savePathNormalized)) { - return false; + return SaveFailed(); } - // Setting flag after reopening becausse it's cleared on open - m_saveTriggeredInternally = true; - return true; - } - - bool MaterialDocument::Close() - { - using namespace AZ; - using namespace RPI; - - if (!IsOpen()) - { - AZ_Error("MaterialDocument", false, "Material document is not open."); - return false; - } - - AZ_TracePrintf("MaterialDocument", "Material document closed: '%s'.\n", m_absolutePath.c_str()); - - AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast(&AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentClosed, m_id); - - // Clearing after notification so paths are still available - Clear(); - return true; + return SaveSucceeded(); } bool MaterialDocument::IsOpen() const { - return !m_absolutePath.empty() && !m_relativePath.empty() && m_materialAsset.IsReady() && m_materialInstance; + return AtomToolsDocument::IsOpen() && m_materialAsset.IsReady() && m_materialInstance; } bool MaterialDocument::IsModified() const @@ -454,44 +309,6 @@ namespace MaterialEditor return AzFramework::StringFunc::Path::IsExtension(m_absolutePath.c_str(), AZ::RPI::MaterialSourceData::Extension); } - bool MaterialDocument::CanUndo() const - { - // Undo will only be allowed if something has been recorded and we're not at the beginning of history - return IsOpen() && !m_undoHistory.empty() && m_undoHistoryIndex > 0; - } - - bool MaterialDocument::CanRedo() const - { - // Redo will only be allowed if something has been recorded and we're not at the end of history - return IsOpen() && !m_undoHistory.empty() && m_undoHistoryIndex < m_undoHistory.size(); - } - - bool MaterialDocument::Undo() - { - if (CanUndo()) - { - // The history index is one beyond the last executed command. Decrement the index then execute undo. - m_undoHistory[--m_undoHistoryIndex].first(); - AZ_TracePrintf("MaterialDocument", "Material document undo: '%s'.\n", m_absolutePath.c_str()); - AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast(&AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentUndoStateChanged, m_id); - return true; - } - return false; - } - - bool MaterialDocument::Redo() - { - if (CanRedo()) - { - // Execute the current redo command then move the history index to the next position. - m_undoHistory[m_undoHistoryIndex++].second(); - AZ_TracePrintf("MaterialDocument", "Material document redo: '%s'.\n", m_absolutePath.c_str()); - AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast(&AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentUndoStateChanged, m_id); - return true; - } - return false; - } - bool MaterialDocument::BeginEdit() { // Save the current properties as a momento for undo before any changes are applied @@ -524,17 +341,9 @@ namespace MaterialEditor if (!propertyValuesForUndo.empty() && !propertyValuesForRedo.empty()) { - // Wipe any state beyond the current history index - m_undoHistory.erase(m_undoHistory.begin() + m_undoHistoryIndex, m_undoHistory.end()); - - // Add undo and redo operations using lambdas that will capture property state and restore it when executed - m_undoHistory.emplace_back( + AddUndoRedoHistory( [this, propertyValuesForUndo]() { RestorePropertyValues(propertyValuesForUndo); }, [this, propertyValuesForRedo]() { RestorePropertyValues(propertyValuesForRedo); }); - - // Assign the index to the end of history - m_undoHistoryIndex = aznumeric_cast(m_undoHistory.size()); - AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast(&AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentUndoStateChanged, m_id); } m_propertyValuesBeforeEdit.clear(); @@ -553,51 +362,25 @@ namespace MaterialEditor } } - void MaterialDocument::SourceFileChanged(AZStd::string relativePath, AZStd::string scanFolder, [[maybe_unused]] AZ::Uuid sourceUUID) - { - const 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); - } - m_saveTriggeredInternally = false; - } - 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); - } - } - bool MaterialDocument::SavePropertiesToSourceData( const AZStd::string& exportPath, AZ::RPI::MaterialSourceData& sourceData, PropertyFilterFunction propertyFilter) const { - using namespace AZ; - using namespace RPI; - bool result = true; // populate sourceData with properties that meet the filter m_materialTypeSourceData.EnumerateProperties([&](const AZStd::string& propertyIdContext, const auto& propertyDefinition) { - Name propertyId{propertyIdContext + propertyDefinition->GetName()}; + AZ::Name propertyId{propertyIdContext + propertyDefinition->GetName()}; const auto it = m_properties.find(propertyId); if (it != m_properties.end() && propertyFilter(it->second)) { - MaterialPropertyValue propertyValue = AtomToolsFramework::ConvertToRuntimeType(it->second.GetValue()); + AZ::RPI::MaterialPropertyValue propertyValue = AtomToolsFramework::ConvertToRuntimeType(it->second.GetValue()); if (propertyValue.IsValid()) { if (!AtomToolsFramework::ConvertToExportFormat(exportPath, propertyId, *propertyDefinition, propertyValue)) { - AZ_Error("MaterialDocument", false, "Material document property could not be converted: '%s' in '%s'.", propertyId.GetCStr(), m_absolutePath.c_str()); + AZ_Error("MaterialDocument", false, "Document property could not be converted: '%s' in '%s'.", propertyId.GetCStr(), m_absolutePath.c_str()); result = false; return false; } @@ -613,52 +396,21 @@ namespace MaterialEditor return result; } - bool MaterialDocument::OpenInternal(AZStd::string_view loadPath) + bool MaterialDocument::Open(AZStd::string_view loadPath) { - using namespace AZ; - using namespace RPI; - - Clear(); - - m_absolutePath = loadPath; - if (!AzFramework::StringFunc::Path::Normalize(m_absolutePath)) + if (!AtomToolsDocument::Open(loadPath)) { - AZ_Error("MaterialDocument", false, "Material document path could not be normalized: '%s'.", m_absolutePath.c_str()); - return false; - } - - if (AzFramework::StringFunc::Path::IsRelative(m_absolutePath.c_str())) - { - AZ_Error("MaterialDocument", false, "Material document path must be absolute: '%s'.", m_absolutePath.c_str()); - return false; - } - - bool result = false; - Data::AssetInfo sourceAssetInfo; - AZStd::string watchFolder; - AzToolsFramework::AssetSystemRequestBus::BroadcastResult(result, &AzToolsFramework::AssetSystem::AssetSystemRequest::GetSourceInfoBySourcePath, - m_absolutePath.c_str(), sourceAssetInfo, watchFolder); - if (!result) - { - AZ_Error("MaterialDocument", false, "Could not find source material: '%s'.", m_absolutePath.c_str()); - return false; - } - - m_relativePath = sourceAssetInfo.m_relativePath; - if (!AzFramework::StringFunc::Path::Normalize(m_relativePath)) - { - AZ_Error("MaterialDocument", false, "Material document path could not be normalized: '%s'.", m_relativePath.c_str()); return false; } // The material document and inspector are constructed from source data - if (AzFramework::StringFunc::Path::IsExtension(m_absolutePath.c_str(), MaterialSourceData::Extension)) + if (AzFramework::StringFunc::Path::IsExtension(m_absolutePath.c_str(), AZ::RPI::MaterialSourceData::Extension)) { // Load the material source data so that we can check properties and create a material asset from it if (!AZ::RPI::JsonUtils::LoadObjectFromFile(m_absolutePath, m_materialSourceData)) { AZ_Error("MaterialDocument", false, "Material source data could not be loaded: '%s'.", m_absolutePath.c_str()); - return false; + return OpenFailed(); } // We always need the absolute path for the material type and parent material to load source data and resolving @@ -666,33 +418,34 @@ namespace MaterialEditor if (!m_materialSourceData.m_parentMaterial.empty()) { m_materialSourceData.m_parentMaterial = - AssetUtils::ResolvePathReference(m_absolutePath, m_materialSourceData.m_parentMaterial); + AZ::RPI::AssetUtils::ResolvePathReference(m_absolutePath, m_materialSourceData.m_parentMaterial); } if (!m_materialSourceData.m_materialType.empty()) { - m_materialSourceData.m_materialType = AssetUtils::ResolvePathReference(m_absolutePath, m_materialSourceData.m_materialType); + m_materialSourceData.m_materialType = + AZ::RPI::AssetUtils::ResolvePathReference(m_absolutePath, m_materialSourceData.m_materialType); } // Load the material type source data which provides the layout and default values of all of the properties - auto materialTypeOutcome = MaterialUtils::LoadMaterialTypeSourceData(m_materialSourceData.m_materialType); + auto materialTypeOutcome = AZ::RPI::MaterialUtils::LoadMaterialTypeSourceData(m_materialSourceData.m_materialType); if (!materialTypeOutcome.IsSuccess()) { AZ_Error("MaterialDocument", false, "Material type source data could not be loaded: '%s'.", m_materialSourceData.m_materialType.c_str()); - return false; + return OpenFailed(); } m_materialTypeSourceData = materialTypeOutcome.TakeValue(); } - else if (AzFramework::StringFunc::Path::IsExtension(m_absolutePath.c_str(), MaterialTypeSourceData::Extension)) + else if (AzFramework::StringFunc::Path::IsExtension(m_absolutePath.c_str(), AZ::RPI::MaterialTypeSourceData::Extension)) { // A material document can be created or loaded from material or material type source data. If we are attempting to load // material type source data then the material source data object can be created just by referencing the document path as the // material type path. - auto materialTypeOutcome = MaterialUtils::LoadMaterialTypeSourceData(m_absolutePath); + auto materialTypeOutcome = AZ::RPI::MaterialUtils::LoadMaterialTypeSourceData(m_absolutePath); if (!materialTypeOutcome.IsSuccess()) { AZ_Error("MaterialDocument", false, "Material type source data could not be loaded: '%s'.", m_absolutePath.c_str()); - return false; + return OpenFailed(); } m_materialTypeSourceData = materialTypeOutcome.TakeValue(); @@ -702,8 +455,8 @@ namespace MaterialEditor } else { - AZ_Error("MaterialDocument", false, "Material document extension not supported: '%s'.", m_absolutePath.c_str()); - return false; + AZ_Error("MaterialDocument", false, "Document extension not supported: '%s'.", m_absolutePath.c_str()); + return OpenFailed(); } const bool elevateWarnings = false; @@ -714,44 +467,44 @@ 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 materialAssetResult = - m_materialSourceData.CreateMaterialAssetFromSourceData(Uuid::CreateRandom(), m_absolutePath, elevateWarnings, &m_sourceDependencies); + auto materialAssetResult = m_materialSourceData.CreateMaterialAssetFromSourceData( + AZ::Uuid::CreateRandom(), m_absolutePath, elevateWarnings, &m_sourceDependencies); if (!materialAssetResult) { AZ_Error("MaterialDocument", false, "Material asset could not be created from source data: '%s'.", m_absolutePath.c_str()); - return false; + return OpenFailed(); } m_materialAsset = materialAssetResult.GetValue(); if (!m_materialAsset.IsReady()) { AZ_Error("MaterialDocument", false, "Material asset is not ready: '%s'.", m_absolutePath.c_str()); - return false; + return OpenFailed(); } const auto& materialTypeAsset = m_materialAsset->GetMaterialTypeAsset(); if (!materialTypeAsset.IsReady()) { AZ_Error("MaterialDocument", false, "Material type asset is not ready: '%s'.", m_absolutePath.c_str()); - return false; + return OpenFailed(); } AZStd::span parentPropertyValues = materialTypeAsset->GetDefaultPropertyValues(); - AZ::Data::Asset parentMaterialAsset; + AZ::Data::Asset parentMaterialAsset; if (!m_materialSourceData.m_parentMaterial.empty()) { AZ::RPI::MaterialSourceData parentMaterialSourceData; if (!AZ::RPI::JsonUtils::LoadObjectFromFile(m_materialSourceData.m_parentMaterial, parentMaterialSourceData)) { AZ_Error("MaterialDocument", false, "Material parent source data could not be loaded for: '%s'.", m_materialSourceData.m_parentMaterial.c_str()); - return false; + return OpenFailed(); } - const auto parentMaterialAssetIdResult = AssetUtils::MakeAssetId(m_materialSourceData.m_parentMaterial, 0); + const auto parentMaterialAssetIdResult = AZ::RPI::AssetUtils::MakeAssetId(m_materialSourceData.m_parentMaterial, 0); if (!parentMaterialAssetIdResult) { AZ_Error("MaterialDocument", false, "Material parent asset ID could not be created: '%s'.", m_materialSourceData.m_parentMaterial.c_str()); - return false; + return OpenFailed(); } auto parentMaterialAssetResult = parentMaterialSourceData.CreateMaterialAssetFromSourceData( @@ -759,7 +512,7 @@ namespace MaterialEditor if (!parentMaterialAssetResult) { AZ_Error("MaterialDocument", false, "Material parent asset could not be created from source data: '%s'.", m_materialSourceData.m_parentMaterial.c_str()); - return false; + return OpenFailed(); } parentMaterialAsset = parentMaterialAssetResult.GetValue(); @@ -767,11 +520,11 @@ namespace MaterialEditor } // Creating a material from a material asset will fail if a texture is referenced but not loaded - m_materialInstance = Material::Create(m_materialAsset); + m_materialInstance = AZ::RPI::Material::Create(m_materialAsset); if (!m_materialInstance) { AZ_Error("MaterialDocument", false, "Material instance could not be created: '%s'.", m_absolutePath.c_str()); - return false; + return OpenFailed(); } // Pipeline State Object changes are always allowed in the material editor because it only runs on developer systems @@ -781,7 +534,7 @@ namespace MaterialEditor // Populate the property map from a combination of source data and assets // Assets must still be used for now because they contain the final accumulated value after all other materials // in the hierarchy are applied - m_materialTypeSourceData.EnumeratePropertyGroups([this, &parentPropertyValues](const AZStd::string& propertyIdContext, const MaterialTypeSourceData::PropertyGroup* propertyGroup) + m_materialTypeSourceData.EnumeratePropertyGroups([this, &parentPropertyValues](const AZStd::string& propertyIdContext, const AZ::RPI::MaterialTypeSourceData::PropertyGroup* propertyGroup) { AtomToolsFramework::DynamicPropertyConfig propertyConfig; @@ -813,7 +566,7 @@ namespace MaterialEditor // Populate the property group visibility map // TODO: Support populating the Material Editor with nested property groups, not just the top level. - for (const AZStd::unique_ptr& propertyGroup : m_materialTypeSourceData.GetPropertyLayout().m_propertyGroups) + for (const AZStd::unique_ptr& propertyGroup : m_materialTypeSourceData.GetPropertyLayout().m_propertyGroups) { m_propertyGroupVisibility[AZ::Name{propertyGroup->GetName()}] = true; } @@ -856,15 +609,15 @@ namespace MaterialEditor m_properties[propertyConfig.m_id] = AtomToolsFramework::DynamicProperty(propertyConfig); //Add UV name customization properties - const RPI::MaterialUvNameMap& uvNameMap = materialTypeAsset->GetUvNameMap(); - for (const RPI::UvNamePair& uvNamePair : uvNameMap) + const AZ::RPI::MaterialUvNameMap& uvNameMap = materialTypeAsset->GetUvNameMap(); + for (const AZ::RPI::UvNamePair& uvNamePair : uvNameMap) { const AZStd::string shaderInput = uvNamePair.m_shaderInput.ToString(); const AZStd::string uvName = uvNamePair.m_uvName.GetStringView(); propertyConfig = {}; propertyConfig.m_dataType = AtomToolsFramework::DynamicPropertyType::String; - propertyConfig.m_id = MaterialPropertyId(UvGroupName, shaderInput); + propertyConfig.m_id = AZ::RPI::MaterialPropertyId(UvGroupName, shaderInput); propertyConfig.m_name = shaderInput; propertyConfig.m_displayName = shaderInput; propertyConfig.m_groupName = "UV Sets"; @@ -878,15 +631,15 @@ namespace MaterialEditor } // Add material functors that are in the top-level functors list. - const MaterialFunctorSourceData::EditorContext editorContext = - MaterialFunctorSourceData::EditorContext(m_materialSourceData.m_materialType, m_materialAsset->GetMaterialPropertiesLayout()); - for (Ptr functorData : m_materialTypeSourceData.m_materialFunctorSourceData) + const AZ::RPI::MaterialFunctorSourceData::EditorContext editorContext = + AZ::RPI::MaterialFunctorSourceData::EditorContext(m_materialSourceData.m_materialType, m_materialAsset->GetMaterialPropertiesLayout()); + for (Ptr functorData : m_materialTypeSourceData.m_materialFunctorSourceData) { - MaterialFunctorSourceData::FunctorResult result2 = functorData->CreateFunctor(editorContext); + AZ::RPI::MaterialFunctorSourceData::FunctorResult result2 = functorData->CreateFunctor(editorContext); if (result2.IsSuccess()) { - Ptr& functor = result2.GetValue(); + Ptr& functor = result2.GetValue(); if (functor != nullptr) { m_editorFunctors.push_back(functor); @@ -895,24 +648,24 @@ namespace MaterialEditor else { AZ_Error("MaterialDocument", false, "Material functors were not created: '%s'.", m_absolutePath.c_str()); - return false; + return OpenFailed(); } } // Add any material functors that are located inside each property group. bool enumerateResult = m_materialTypeSourceData.EnumeratePropertyGroups( - [this](const AZStd::string&, const MaterialTypeSourceData::PropertyGroup* propertyGroup) + [this](const AZStd::string&, const AZ::RPI::MaterialTypeSourceData::PropertyGroup* propertyGroup) { - const MaterialFunctorSourceData::EditorContext editorContext = MaterialFunctorSourceData::EditorContext( + const AZ::RPI::MaterialFunctorSourceData::EditorContext editorContext = AZ::RPI::MaterialFunctorSourceData::EditorContext( m_materialSourceData.m_materialType, m_materialAsset->GetMaterialPropertiesLayout()); - for (Ptr functorData : propertyGroup->GetFunctors()) + for (Ptr functorData : propertyGroup->GetFunctors()) { - MaterialFunctorSourceData::FunctorResult result = functorData->CreateFunctor(editorContext); + AZ::RPI::MaterialFunctorSourceData::FunctorResult result = functorData->CreateFunctor(editorContext); if (result.IsSuccess()) { - Ptr& functor = result.GetValue(); + Ptr& functor = result.GetValue(); if (functor != nullptr) { m_editorFunctors.push_back(functor); @@ -930,18 +683,35 @@ namespace MaterialEditor if (!enumerateResult) { - return false; + return OpenFailed(); } AZ::RPI::MaterialPropertyFlags dirtyFlags; dirtyFlags.set(); // Mark all properties as dirty since we just loaded the material and need to initialize property visibility RunEditorMaterialFunctors(dirtyFlags); - // Connecting to bus to monitor external changes - AzToolsFramework::AssetSystemBus::Handler::BusConnect(); + return OpenSucceeded(); + } - AZ_TracePrintf("MaterialDocument", "Material document opened: '%s'.\n", m_absolutePath.c_str()); - return true; + bool MaterialDocument::ReopenRecordState() + { + m_propertyValuesBeforeReopen.clear(); + for (const auto& propertyPair : m_properties) + { + const AtomToolsFramework::DynamicProperty& property = propertyPair.second; + if (!AtomToolsFramework::ArePropertyValuesEqual(property.GetValue(), property.GetConfig().m_parentValue)) + { + m_propertyValuesBeforeReopen[property.GetId()] = property.GetValue(); + } + } + return AtomToolsDocument::ReopenRecordState(); + } + + bool MaterialDocument::ReopenRestoreState() + { + RestorePropertyValues(m_propertyValuesBeforeReopen); + m_propertyValuesBeforeReopen.clear(); + return AtomToolsDocument::ReopenRestoreState(); } void MaterialDocument::Recompile() @@ -955,23 +725,18 @@ namespace MaterialEditor void MaterialDocument::Clear() { + AtomToolsFramework::AtomToolsDocument::Clear(); + AZ::TickBus::Handler::BusDisconnect(); - AzToolsFramework::AssetSystemBus::Handler::BusDisconnect(); m_materialAsset = {}; m_materialInstance = {}; - m_absolutePath.clear(); - m_relativePath.clear(); - m_sourceDependencies.clear(); - m_saveTriggeredInternally = {}; m_compilePending = {}; m_properties.clear(); m_editorFunctors.clear(); m_materialTypeSourceData = AZ::RPI::MaterialTypeSourceData(); m_materialSourceData = AZ::RPI::MaterialSourceData(); m_propertyValuesBeforeEdit.clear(); - m_undoHistory.clear(); - m_undoHistoryIndex = {}; } void MaterialDocument::RestorePropertyValues(const PropertyValueMap& propertyValues) @@ -1040,5 +805,4 @@ namespace MaterialEditor return result; } - } // namespace MaterialEditor diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.h b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.h index c975824e22..6557b5a326 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.h +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.h @@ -10,7 +10,6 @@ #include #include #include -#include #include #include @@ -28,7 +27,6 @@ namespace MaterialEditor : public AtomToolsFramework::AtomToolsDocument , public MaterialDocumentRequestBus::Handler , private AZ::TickBus::Handler - , private AzToolsFramework::AssetSystemBus::Handler { public: AZ_RTTI(MaterialDocument, "{DBA269AE-892B-415C-8FA1-166B94B0E045}"); @@ -38,37 +36,26 @@ namespace MaterialEditor MaterialDocument(); virtual ~MaterialDocument(); - //////////////////////////////////////////////////////////////////////// - // AtomToolsFramework::AtomToolsDocument - //////////////////////////////////////////////////////////////////////// + // AtomToolsFramework::AtomToolsDocument overrides... const AZStd::any& GetPropertyValue(const AZ::Name& propertyId) const override; const AtomToolsFramework::DynamicProperty& GetProperty(const AZ::Name& propertyId) const override; bool IsPropertyGroupVisible(const AZ::Name& propertyGroupFullName) const override; void SetPropertyValue(const AZ::Name& propertyId, const AZStd::any& value) override; bool Open(AZStd::string_view loadPath) override; - bool Reopen() override; bool Save() override; bool SaveAsCopy(AZStd::string_view savePath) override; bool SaveAsChild(AZStd::string_view savePath) override; - bool Close() override; bool IsOpen() const override; bool IsModified() const override; bool IsSavable() const override; - bool CanUndo() const override; - bool CanRedo() const override; - bool Undo() override; - bool Redo() override; bool BeginEdit() override; bool EndEdit() override; - //////////////////////////////////////////////////////////////////////// - //////////////////////////////////////////////////////////////////////// - // MaterialDocumentRequestBus::Handler implementation + // MaterialDocumentRequestBus::Handler overrides... AZ::Data::Asset GetAsset() const override; AZ::Data::Instance GetInstance() const override; const AZ::RPI::MaterialSourceData* GetMaterialSourceData() const override; const AZ::RPI::MaterialTypeSourceData* GetMaterialTypeSourceData() const override; - //////////////////////////////////////////////////////////////////////// private: @@ -84,33 +71,18 @@ namespace MaterialEditor // Map of document's property group visibility flags using PropertyGroupVisibilityMap = AZStd::unordered_map; - // Function to be bound for undo and redo - using UndoRedoFunction = AZStd::function; - - // A pair of functions, where first is the undo operation and second is the redo operation - using UndoRedoFunctionPair = AZStd::pair; - - // Container for all of the active undo and redo functions and state - using UndoRedoHistory = AZStd::vector; - - //////////////////////////////////////////////////////////////////////// - // AZ::TickBus interface implementation + // AZ::TickBus overrides... void OnTick(float deltaTime, AZ::ScriptTimePoint time) override; - //////////////////////////////////////////////////////////////////////// - - ////////////////////////////////////////////////////////////////////////// - // AzToolsFramework::AssetSystemBus::Handler overrides... - void SourceFileChanged(AZStd::string relativePath, AZStd::string scanFolder, AZ::Uuid sourceUUID) override; - ////////////////////////////////////////////////////////////////////////// bool SavePropertiesToSourceData( const AZStd::string& exportPath, AZ::RPI::MaterialSourceData& sourceData, PropertyFilterFunction propertyFilter) const; - bool OpenInternal(AZStd::string_view loadPath); + void Clear() override; - void Recompile(); + bool ReopenRecordState() override; + bool ReopenRestoreState() override; - void Clear(); + void Recompile(); void RestorePropertyValues(const PropertyValueMap& propertyValues); @@ -131,12 +103,6 @@ namespace MaterialEditor // Material instance being edited AZ::Data::Instance m_materialInstance; - // Set of assets that can trigger a document reload - AZStd::unordered_set m_sourceDependencies; - - // Track if document saved itself last to skip external modification notification - bool m_saveTriggeredInternally = false; - // If material instance value(s) were modified, do we need to recompile on next tick? bool m_compilePending = false; @@ -155,19 +121,10 @@ namespace MaterialEditor // Source data for material AZ::RPI::MaterialSourceData m_materialSourceData; - // Variables needed for tracking the undo and redo state of this document - // State of property values prior to an edit, used for restoration during undo PropertyValueMap m_propertyValuesBeforeEdit; - // Container of undo commands - UndoRedoHistory m_undoHistory; - - // The current position in the undo redo history - int m_undoHistoryIndex = 0; - - AZStd::any m_invalidValue; - - AtomToolsFramework::DynamicProperty m_invalidProperty; + // State of property values prior to reopen + PropertyValueMap m_propertyValuesBeforeReopen; }; } // namespace MaterialEditor diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Viewport/MaterialViewportComponent.cpp b/Gems/Atom/Tools/MaterialEditor/Code/Source/Viewport/MaterialViewportComponent.cpp index b2f1cdc9c1..a7b1fcc0c9 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Viewport/MaterialViewportComponent.cpp +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Viewport/MaterialViewportComponent.cpp @@ -170,11 +170,14 @@ namespace MaterialEditor { m_lightingPresetAssets[info.m_assetId] = { info.m_assetId, info.m_assetType }; AZ::Data::AssetBus::MultiHandler::BusConnect(info.m_assetId); + return; } - else if (AZ::StringFunc::EndsWith(info.m_relativePath.c_str(), ".modelpreset.azasset")) + + if (AZ::StringFunc::EndsWith(info.m_relativePath.c_str(), ".modelpreset.azasset")) { m_modelPresetAssets[info.m_assetId] = { info.m_assetId, info.m_assetType }; AZ::Data::AssetBus::MultiHandler::BusConnect(info.m_assetId); + return; } }; @@ -436,24 +439,20 @@ namespace MaterialEditor auto ReloadLightingAndModelPresets = [this, &assetId](AZ::Data::AssetCatalogRequests* assetCatalogRequests) { AZ::Data::AssetInfo assetInfo = assetCatalogRequests->GetAssetInfoById(assetId); - AZ::Data::Asset* modifiedPresetAsset{}; if (AZ::StringFunc::EndsWith(assetInfo.m_relativePath.c_str(), ".lightingpreset.azasset")) { m_lightingPresetAssets[assetInfo.m_assetId] = { assetInfo.m_assetId, assetInfo.m_assetType }; AZ::Data::AssetBus::MultiHandler::BusConnect(assetInfo.m_assetId); - modifiedPresetAsset = &m_lightingPresetAssets[assetInfo.m_assetId]; + m_lightingPresetAssets[assetInfo.m_assetId].QueueLoad(); + return; } - else if (AzFramework::StringFunc::EndsWith(assetInfo.m_relativePath.c_str(), ".modelpreset.azasset")) + + if (AzFramework::StringFunc::EndsWith(assetInfo.m_relativePath.c_str(), ".modelpreset.azasset")) { m_modelPresetAssets[assetInfo.m_assetId] = { assetInfo.m_assetId, assetInfo.m_assetType }; AZ::Data::AssetBus::MultiHandler::BusConnect(assetInfo.m_assetId); - modifiedPresetAsset = &m_modelPresetAssets[assetInfo.m_assetId]; - } - - // Queue a load on the changed asset - if (modifiedPresetAsset != nullptr) - { - modifiedPresetAsset->QueueLoad(); + m_modelPresetAssets[assetInfo.m_assetId].QueueLoad(); + return; } }; AZ::Data::AssetCatalogRequestBus::Broadcast(AZStd::move(ReloadLightingAndModelPresets)); @@ -470,11 +469,14 @@ namespace MaterialEditor { AZ::Data::AssetBus::MultiHandler::BusDisconnect(assetInfo.m_assetId); m_lightingPresetAssets.erase(assetId); + return; } + if (AZ::StringFunc::EndsWith(assetInfo.m_relativePath.c_str(), ".modelpreset.azasset")) { AZ::Data::AssetBus::MultiHandler::BusDisconnect(assetInfo.m_assetId); m_modelPresetAssets.erase(assetId); + return; } } } diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Window/MaterialInspector/MaterialInspector.h b/Gems/Atom/Tools/MaterialEditor/Code/Source/Window/MaterialInspector/MaterialInspector.h index a4cd2b7616..690e8add86 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Window/MaterialInspector/MaterialInspector.h +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Window/MaterialInspector/MaterialInspector.h @@ -19,8 +19,7 @@ namespace MaterialEditor { - //! Provides controls for viewing and editing a material document settings. - //! The settings can be divided into cards, with each one showing a subset of properties. + //! Provides controls for viewing and editing document settings. class MaterialInspector : public AtomToolsFramework::InspectorWidget , public AtomToolsFramework::AtomToolsDocumentNotificationBus::Handler diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Window/ViewportSettingsInspector/ViewportSettingsInspector.h b/Gems/Atom/Tools/MaterialEditor/Code/Source/Window/ViewportSettingsInspector/ViewportSettingsInspector.h index 464a55aa7f..ad4629550f 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Window/ViewportSettingsInspector/ViewportSettingsInspector.h +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Window/ViewportSettingsInspector/ViewportSettingsInspector.h @@ -21,8 +21,7 @@ namespace MaterialEditor { - //! Provides controls for viewing and editing a material document settings. - //! The settings can be divided into cards, with each one showing a subset of properties. + //! Provides controls for viewing and editing lighting and model preset settings. class ViewportSettingsInspector : public AtomToolsFramework::InspectorWidget , private AzToolsFramework::IPropertyEditorNotify diff --git a/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Document/ShaderManagementConsoleDocument.cpp b/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Document/ShaderManagementConsoleDocument.cpp index 2b7767635e..be5adfe4bf 100644 --- a/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Document/ShaderManagementConsoleDocument.cpp +++ b/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Document/ShaderManagementConsoleDocument.cpp @@ -10,8 +10,6 @@ #include #include #include -#include -#include #include namespace ShaderManagementConsole @@ -20,28 +18,32 @@ namespace ShaderManagementConsole : AtomToolsFramework::AtomToolsDocument() { ShaderManagementConsoleDocumentRequestBus::Handler::BusConnect(m_id); - AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast(&AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentCreated, m_id); } ShaderManagementConsoleDocument::~ShaderManagementConsoleDocument() { - AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast(&AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentDestroyed, m_id); ShaderManagementConsoleDocumentRequestBus::Handler::BusDisconnect(); - Clear(); } - size_t ShaderManagementConsoleDocument::GetShaderOptionCount() const + void ShaderManagementConsoleDocument::SetShaderVariantListSourceData(const AZ::RPI::ShaderVariantListSourceData& sourceData) { - auto layout = m_shaderAsset->GetShaderOptionGroupLayout(); - auto& shaderOptionDescriptors = layout->GetShaderOptions(); - return shaderOptionDescriptors.size(); + m_shaderVariantListSourceData = sourceData; + AZStd::string shaderPath = m_shaderVariantListSourceData.m_shaderFilePath; + AzFramework::StringFunc::Path::ReplaceExtension(shaderPath, AZ::RPI::ShaderAsset::Extension); + + m_shaderAsset = AZ::RPI::AssetUtils::LoadAssetByProductPath(shaderPath.c_str()); + if (!m_shaderAsset) + { + AZ_Error("ShaderManagementConsoleDocument", false, "Could not load shader asset: %s.", shaderPath.c_str()); + } + + AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast( + &AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentModified, m_id); } - const AZ::RPI::ShaderOptionDescriptor& ShaderManagementConsoleDocument::GetShaderOptionDescriptor(size_t index) const + const AZ::RPI::ShaderVariantListSourceData& ShaderManagementConsoleDocument::GetShaderVariantListSourceData() const { - auto layout = m_shaderAsset->GetShaderOptionGroupLayout(); - auto& shaderOptionDescriptors = layout->GetShaderOptions(); - return shaderOptionDescriptors[index]; + return m_shaderVariantListSourceData; } size_t ShaderManagementConsoleDocument::GetShaderVariantCount() const @@ -54,92 +56,115 @@ namespace ShaderManagementConsole return m_shaderVariantListSourceData.m_shaderVariants[index]; } - bool ShaderManagementConsoleDocument::Open(AZStd::string_view loadPath) + size_t ShaderManagementConsoleDocument::GetShaderOptionCount() const { - Clear(); + if (IsOpen()) + { + const auto& layout = m_shaderAsset->GetShaderOptionGroupLayout(); + const auto& shaderOptionDescriptors = layout->GetShaderOptions(); + return shaderOptionDescriptors.size(); + } + return 0; + } - m_absolutePath = loadPath; - if (!AzFramework::StringFunc::Path::Normalize(m_absolutePath)) + const AZ::RPI::ShaderOptionDescriptor& ShaderManagementConsoleDocument::GetShaderOptionDescriptor(size_t index) const + { + if (IsOpen()) { - AZ_Error("ShaderManagementConsoleDocument", false, "Document path could not be normalized: '%s'.", m_absolutePath.c_str()); - return false; + const auto& layout = m_shaderAsset->GetShaderOptionGroupLayout(); + const auto& shaderOptionDescriptors = layout->GetShaderOptions(); + return shaderOptionDescriptors.at(index); } + return m_invalidDescriptor; + } - if (AzFramework::StringFunc::Path::IsRelative(m_absolutePath.c_str())) + bool ShaderManagementConsoleDocument::Open(AZStd::string_view loadPath) + { + if (!AtomToolsDocument::Open(loadPath)) { - AZ_Error("ShaderManagementConsoleDocument", false, "Document path must be absolute: '%s'.", m_absolutePath.c_str()); return false; } - if (AzFramework::StringFunc::Path::IsExtension(m_absolutePath.c_str(), AZ::RPI::ShaderVariantListSourceData::Extension)) + if (!AzFramework::StringFunc::Path::IsExtension(m_absolutePath.c_str(), AZ::RPI::ShaderVariantListSourceData::Extension)) { - // Load the shader config data and create a shader config asset from it - if (!AZ::RPI::JsonUtils::LoadObjectFromFile(m_absolutePath, m_shaderVariantListSourceData)) - { - AZ_Error("ShaderManagementConsoleDocument", false, "Failed loading shader variant list data: '%s.'", m_absolutePath.c_str()); - return false; - } + AZ_Error("ShaderManagementConsoleDocument", false, "Document extension is not supported: '%s.'", m_absolutePath.c_str()); + return OpenFailed(); } - bool result = false; - AZ::Data::AssetInfo sourceAssetInfo; - AZStd::string watchFolder; - AzToolsFramework::AssetSystemRequestBus::BroadcastResult( - result, &AzToolsFramework::AssetSystem::AssetSystemRequest::GetSourceInfoBySourcePath, m_absolutePath.c_str(), sourceAssetInfo, - watchFolder); - if (!result) + // Load the shader config data and create a shader config asset from it + AZ::RPI::ShaderVariantListSourceData sourceData; + if (!AZ::RPI::JsonUtils::LoadObjectFromFile(m_absolutePath, sourceData)) { - AZ_Error("ShaderManagementConsoleDocument", false, "Could not find source data: '%s'.", m_absolutePath.c_str()); - return false; + AZ_Error("ShaderManagementConsoleDocument", false, "Failed loading shader variant list data: '%s.'", m_absolutePath.c_str()); + return OpenFailed(); } - m_relativePath = m_shaderVariantListSourceData.m_shaderFilePath; - if (!AzFramework::StringFunc::Path::Normalize(m_relativePath)) + SetShaderVariantListSourceData(sourceData); + return IsOpen() ? OpenSucceeded() : OpenFailed(); + } + + bool ShaderManagementConsoleDocument::Save() + { + if (!AtomToolsDocument::Save()) { - AZ_Error("ShaderManagementConsoleDocument", false, "Shader path could not be normalized: '%s'.", m_relativePath.c_str()); return false; } - AZStd::string shaderPath = m_relativePath; - AzFramework::StringFunc::Path::ReplaceExtension(shaderPath, AZ::RPI::ShaderAsset::Extension); + return SaveSourceData(); + } - m_shaderAsset = AZ::RPI::AssetUtils::LoadAssetByProductPath(shaderPath.c_str()); - if (!m_shaderAsset) + bool ShaderManagementConsoleDocument::SaveAsCopy(AZStd::string_view savePath) + { + if (!AtomToolsDocument::SaveAsCopy(savePath)) { - AZ_Error("ShaderManagementConsoleDocument", false, "Could not load shader asset: %s.", shaderPath.c_str()); return false; } - AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast(&AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentOpened, m_id); - - AZ_TracePrintf("ShaderManagementConsoleDocument", "Document opened: '%s'\n", m_absolutePath.c_str()); - return true; + return SaveSourceData(); } - bool ShaderManagementConsoleDocument::Close() + bool ShaderManagementConsoleDocument::SaveAsChild(AZStd::string_view savePath) { - if (!IsOpen()) + if (!AtomToolsDocument::SaveAsChild(savePath)) { - AZ_Error("ShaderManagementConsoleDocument", false, "Document is not open"); return false; } - Clear(); - AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast(&AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentClosed, m_id); - AZ_TracePrintf("ShaderManagementConsoleDocument", "Document closed\n"); - return true; + return SaveSourceData(); } bool ShaderManagementConsoleDocument::IsOpen() const { - return !m_absolutePath.empty() && !m_relativePath.empty(); + return AtomToolsDocument::IsOpen() && m_shaderAsset.IsReady(); + } + + bool ShaderManagementConsoleDocument::IsModified() const + { + return false; + } + + bool ShaderManagementConsoleDocument::IsSavable() const + { + return true; } void ShaderManagementConsoleDocument::Clear() { - m_absolutePath.clear(); - m_relativePath.clear(); + AtomToolsFramework::AtomToolsDocument::Clear(); + m_shaderVariantListSourceData = {}; m_shaderAsset = {}; } + + bool ShaderManagementConsoleDocument::SaveSourceData() + { + if (!AZ::RPI::JsonUtils::SaveObjectToFile(m_savePathNormalized, m_shaderVariantListSourceData)) + { + AZ_Error("ShaderManagementConsoleDocument", false, "Document could not be saved: '%s'.", m_savePathNormalized.c_str()); + return SaveFailed(); + } + + m_absolutePath = m_savePathNormalized; + return SaveSucceeded(); + } } // namespace ShaderManagementConsole diff --git a/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Document/ShaderManagementConsoleDocument.h b/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Document/ShaderManagementConsoleDocument.h index bd51616f7a..2c22880ffe 100644 --- a/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Document/ShaderManagementConsoleDocument.h +++ b/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Document/ShaderManagementConsoleDocument.h @@ -29,40 +29,35 @@ namespace ShaderManagementConsole AZ_DISABLE_COPY(ShaderManagementConsoleDocument); ShaderManagementConsoleDocument(); - virtual ~ShaderManagementConsoleDocument(); + ~ShaderManagementConsoleDocument(); - //////////////////////////////////////////////////////////////////////// - // AtomToolsFramework::AtomToolsDocument - //////////////////////////////////////////////////////////////////////// + // AtomToolsFramework::AtomToolsDocument overrides... bool Open(AZStd::string_view loadPath) override; - bool Close() override; + bool Save() override; + bool SaveAsCopy(AZStd::string_view savePath) override; + bool SaveAsChild(AZStd::string_view savePath) override; bool IsOpen() const override; - //////////////////////////////////////////////////////////////////////// + bool IsModified() const override; + bool IsSavable() const override; - //////////////////////////////////////////////////////////////////////// - // ShaderManagementConsoleDocumentRequestBus::Handler implementation - size_t GetShaderOptionCount() const override; - const AZ::RPI::ShaderOptionDescriptor& GetShaderOptionDescriptor(size_t index) const override; + // ShaderManagementConsoleDocumentRequestBus::Handler overridfes... + void SetShaderVariantListSourceData(const AZ::RPI::ShaderVariantListSourceData& sourceData) override; + const AZ::RPI::ShaderVariantListSourceData& GetShaderVariantListSourceData() const override; size_t GetShaderVariantCount() const override; const AZ::RPI::ShaderVariantListSourceData::VariantInfo& GetShaderVariantInfo(size_t index) const override; - //////////////////////////////////////////////////////////////////////// + size_t GetShaderOptionCount() const override; + const AZ::RPI::ShaderOptionDescriptor& GetShaderOptionDescriptor(size_t index) const override; private: - // Function to be bound for undo and redo - using UndoRedoFunction = AZStd::function; - - // A pair of functions, where first is the undo operation and second is the redo operation - using UndoRedoFunctionPair = AZStd::pair; - - // Container for all of the active undo and redo functions and state - using UndoRedoHistory = AZStd::vector; - - void Clear(); + void Clear() override; + bool SaveSourceData(); // Source data for shader variant list AZ::RPI::ShaderVariantListSourceData m_shaderVariantListSourceData; // Shader asset for the corresponding shader variant list AZ::Data::Asset m_shaderAsset; + + const AZ::RPI::ShaderOptionDescriptor m_invalidDescriptor; }; } // namespace ShaderManagementConsole diff --git a/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Document/ShaderManagementConsoleDocumentRequestBus.h b/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Document/ShaderManagementConsoleDocumentRequestBus.h index d0e6aea5b9..e58ee34ed1 100644 --- a/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Document/ShaderManagementConsoleDocumentRequestBus.h +++ b/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Document/ShaderManagementConsoleDocumentRequestBus.h @@ -23,17 +23,23 @@ namespace ShaderManagementConsole static const AZ::EBusAddressPolicy AddressPolicy = AZ::EBusAddressPolicy::ById; typedef AZ::Uuid BusIdType; - //! Get the number of options - virtual size_t GetShaderOptionCount() const = 0; + //! Set the shader variant list + virtual void SetShaderVariantListSourceData(const AZ::RPI::ShaderVariantListSourceData& sourceData) = 0; - //! Get the descriptor for the shader option at the specified index - virtual const AZ::RPI::ShaderOptionDescriptor& GetShaderOptionDescriptor(size_t index) const = 0; + //! Get the shader variant list + virtual const AZ::RPI::ShaderVariantListSourceData& GetShaderVariantListSourceData() const = 0; //! Get the number of shader variants virtual size_t GetShaderVariantCount() const = 0; //! Get the information for the shader variant at the specified index virtual const AZ::RPI::ShaderVariantListSourceData::VariantInfo& GetShaderVariantInfo(size_t index) const = 0; + + //! Get the number of options + virtual size_t GetShaderOptionCount() const = 0; + + //! Get the descriptor for the shader option at the specified index + virtual const AZ::RPI::ShaderOptionDescriptor& GetShaderOptionDescriptor(size_t index) const = 0; }; using ShaderManagementConsoleDocumentRequestBus = AZ::EBus; diff --git a/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/ShaderManagementConsoleApplication.cpp b/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/ShaderManagementConsoleApplication.cpp index ba8038319b..aa0c378ff0 100644 --- a/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/ShaderManagementConsoleApplication.cpp +++ b/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/ShaderManagementConsoleApplication.cpp @@ -84,6 +84,8 @@ namespace ShaderManagementConsole ->Attribute(AZ::Script::Attributes::Scope, AZ::Script::Attributes::ScopeFlags::Common) ->Attribute(AZ::Script::Attributes::Category, "Editor") ->Attribute(AZ::Script::Attributes::Module, "shadermanagementconsole") + ->Event("SetShaderVariantListSourceData", &ShaderManagementConsoleDocumentRequestBus::Events::SetShaderVariantListSourceData) + ->Event("GetShaderVariantListSourceData", &ShaderManagementConsoleDocumentRequestBus::Events::GetShaderVariantListSourceData) ->Event("GetShaderOptionCount", &ShaderManagementConsoleDocumentRequestBus::Events::GetShaderOptionCount) ->Event("GetShaderOptionDescriptor", &ShaderManagementConsoleDocumentRequestBus::Events::GetShaderOptionDescriptor) ->Event("GetShaderVariantCount", &ShaderManagementConsoleDocumentRequestBus::Events::GetShaderVariantCount) diff --git a/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Window/ShaderManagementConsoleWindow.cpp b/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Window/ShaderManagementConsoleWindow.cpp index f4672d670e..04a8a71191 100644 --- a/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Window/ShaderManagementConsoleWindow.cpp +++ b/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Window/ShaderManagementConsoleWindow.cpp @@ -61,8 +61,6 @@ namespace ShaderManagementConsole m_actionNew->setEnabled(false); m_actionSaveAsChild->setVisible(false); m_actionSaveAsChild->setEnabled(false); - m_actionSaveAll->setVisible(false); - m_actionSaveAll->setEnabled(false); OnDocumentOpened(AZ::Uuid::CreateNull()); } From e273f3ef6a1f73e6f9ba31df3e5b35f0efbdbb31 Mon Sep 17 00:00:00 2001 From: Guthrie Adams Date: Sun, 30 Jan 2022 01:01:10 -0600 Subject: [PATCH 2/7] Updating comments Signed-off-by: Guthrie Adams --- .../Application/AtomToolsApplication.h | 2 +- .../Document/AtomToolsDocument.h | 24 ++++++---- .../Source/Document/AtomToolsDocument.cpp | 47 +++++++++---------- .../AtomToolsDocumentSystemComponent.h | 2 +- .../PreviewRendererCaptureState.h | 4 +- .../Code/Source/Document/MaterialDocument.h | 1 + .../Viewport/MaterialViewportComponent.cpp | 4 +- .../ShaderManagementConsoleDocument.h | 4 +- 8 files changed, 49 insertions(+), 39 deletions(-) diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Application/AtomToolsApplication.h b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Application/AtomToolsApplication.h index 06a9f65ec1..016bc5c5e2 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Application/AtomToolsApplication.h +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Application/AtomToolsApplication.h @@ -111,7 +111,7 @@ namespace AtomToolsFramework AZStd::unique_ptr m_styleManager; - //! Local user settings are used to store material browser tree expansion state + //! Local user settings are used to store asset browser tree expansion state AZ::UserSettingsProvider m_localUserSettings; //! Are local settings loaded diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocument.h b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocument.h index b7188286a1..c2ef7f9594 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocument.h +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocument.h @@ -58,29 +58,36 @@ namespace AtomToolsFramework virtual bool OpenSucceeded(); virtual bool OpenFailed(); - virtual bool ReopenRecordState(); - virtual bool ReopenRestoreState(); - virtual bool SaveSucceeded(); virtual bool SaveFailed(); - // Unique id of this document + //! Record state that needs to be restored after a document is reopened. + //! This can be overridden to record additional data. + virtual bool ReopenRecordState(); + + //! Restore state that was recorded prior to document being reloaded. + //! This can be overridden to restore additional data. + virtual bool ReopenRestoreState(); + + //! The unique id of this document, used for all bus notifications and requests. AZ::Uuid m_id = AZ::Uuid::CreateRandom(); - // Absolute path to the material source file + //! The absolute path to the document source file. AZStd::string m_absolutePath; + //! The normalized, absolute path where the document will be saved. AZStd::string m_savePathNormalized; AZStd::any m_invalidValue; AtomToolsFramework::DynamicProperty m_invalidProperty; - // Set of assets that can trigger a document reload + //! This contains absolute paths of other source files that affect this document. + //! If any of the source files in this container are modified, the document system will he notified to reload this document. AZStd::unordered_set m_sourceDependencies; - // Track if document saved itself last to skip external modification notification - bool m_saveTriggeredInternally = false; + //! If this flag is true then the next source file change notification for this document will be ignored. + bool m_ignoreSourceFileChangeToSelf = false; // Variables needed for tracking the undo and redo state of this document @@ -101,6 +108,7 @@ namespace AtomToolsFramework int m_undoHistoryIndex = {}; int m_undoHistoryIndexBeforeReopen = {}; + //! Add new undo redo command functions at the current position in the undo history. void AddUndoRedoHistory(const UndoRedoFunction& undoCommand, const UndoRedoFunction& redoCommand); // AzToolsFramework::AssetSystemBus::Handler overrides... diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocument.cpp b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocument.cpp index 5941360b5d..1daf578679 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocument.cpp +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocument.cpp @@ -122,7 +122,7 @@ namespace AtomToolsFramework if (!IsSavable()) { - AZ_Error("AtomToolsDocument", false, "Material types can only be saved as a child: '%s'.", m_absolutePath.c_str()); + AZ_Error("AtomToolsDocument", false, "Document type can not be saved: '%s'.", m_absolutePath.c_str()); return SaveFailed(); } @@ -146,7 +146,7 @@ namespace AtomToolsFramework if (!IsSavable()) { - AZ_Error("AtomToolsDocument", false, "Material types can only be saved as a child: '%s'.", m_absolutePath.c_str()); + AZ_Error("AtomToolsDocument", false, "Document type can not be saved: '%s'.", m_absolutePath.c_str()); return SaveFailed(); } @@ -170,7 +170,7 @@ namespace AtomToolsFramework if (m_absolutePath == m_savePathNormalized || m_sourceDependencies.find(m_savePathNormalized) != m_sourceDependencies.end()) { - AZ_Error("AtomToolsDocument", false, "Document can't be saved over a dependancy: '%s'.", m_savePathNormalized.c_str()); + AZ_Error("AtomToolsDocument", false, "Document can not be saved over a dependancy: '%s'.", m_savePathNormalized.c_str()); return SaveFailed(); } @@ -268,7 +268,7 @@ namespace AtomToolsFramework m_absolutePath.clear(); m_sourceDependencies.clear(); - m_saveTriggeredInternally = {}; + m_ignoreSourceFileChangeToSelf = {}; m_undoHistory.clear(); m_undoHistoryIndex = {}; } @@ -289,26 +289,9 @@ namespace AtomToolsFramework return false; } - bool AtomToolsDocument::ReopenRecordState() - { - // Store history and property changes that should be reapplied after reload - m_undoHistoryBeforeReopen = m_undoHistory; - m_undoHistoryIndexBeforeReopen = m_undoHistoryIndex; - return true; - } - - bool AtomToolsDocument::ReopenRestoreState() - { - m_undoHistory = m_undoHistoryBeforeReopen; - m_undoHistoryIndex = m_undoHistoryIndexBeforeReopen; - m_undoHistoryBeforeReopen = {}; - m_undoHistoryIndexBeforeReopen = {}; - return true; - } - bool AtomToolsDocument::SaveSucceeded() { - m_saveTriggeredInternally = true; + m_ignoreSourceFileChangeToSelf = true; AZ_TracePrintf("AtomToolsDocument", "Document saved: '%s'.\n", m_savePathNormalized.c_str()); @@ -328,6 +311,22 @@ namespace AtomToolsFramework return false; } + bool AtomToolsDocument::ReopenRecordState() + { + m_undoHistoryBeforeReopen = m_undoHistory; + m_undoHistoryIndexBeforeReopen = m_undoHistoryIndex; + return true; + } + + bool AtomToolsDocument::ReopenRestoreState() + { + m_undoHistory = m_undoHistoryBeforeReopen; + m_undoHistoryIndex = m_undoHistoryIndexBeforeReopen; + m_undoHistoryBeforeReopen = {}; + m_undoHistoryIndexBeforeReopen = {}; + return true; + } + void AtomToolsDocument::AddUndoRedoHistory(const UndoRedoFunction& undoCommand, const UndoRedoFunction& redoCommand) { // Wipe any state beyond the current history index @@ -349,13 +348,13 @@ namespace AtomToolsFramework if (m_absolutePath == sourcePath) { // ignore notifications caused by saving the open document - if (!m_saveTriggeredInternally) + if (!m_ignoreSourceFileChangeToSelf) { AZ_TracePrintf("AtomToolsDocument", "Document changed externally: '%s'.\n", m_absolutePath.c_str()); AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast( &AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentExternallyModified, m_id); } - m_saveTriggeredInternally = false; + m_ignoreSourceFileChangeToSelf = false; } else if (m_sourceDependencies.find(sourcePath) != m_sourceDependencies.end()) { diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocumentSystemComponent.h b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocumentSystemComponent.h index 58470e3582..5c7454e5c7 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocumentSystemComponent.h +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocumentSystemComponent.h @@ -24,7 +24,7 @@ AZ_POP_DISABLE_WARNING namespace AtomToolsFramework { - //! AtomToolsDocumentSystemComponent is the central component of the Material Editor Core gem + //! AtomToolsDocumentSystemComponent is the central component for managing documents class AtomToolsDocumentSystemComponent : public AZ::Component , private AtomToolsDocumentNotificationBus::Handler diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRendererCaptureState.h b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRendererCaptureState.h index 74195ab396..55f0507014 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRendererCaptureState.h +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRendererCaptureState.h @@ -14,7 +14,7 @@ namespace AtomToolsFramework { - //! PreviewRendererCaptureState renders a thumbnail to a pixmap and notifies MaterialOrModelThumbnail once finished + //! PreviewRendererCaptureState renders a preview to an image class PreviewRendererCaptureState final : public PreviewRendererState , public AZ::TickBus::Handler @@ -31,7 +31,7 @@ namespace AtomToolsFramework //! AZ::Render::FrameCaptureNotificationBus::Handler overrides... void OnCaptureFinished(AZ::Render::FrameCaptureResult result, const AZStd::string& info) override; - //! This is necessary to suspend capture to allow a frame for Material and Mesh components to assign materials + //! This is necessary to suspend capture until preview scene is ready int m_ticksToCapture = 1; }; } // namespace AtomToolsFramework diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.h b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.h index 6557b5a326..99e2af7a73 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.h +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.h @@ -77,6 +77,7 @@ namespace MaterialEditor bool SavePropertiesToSourceData( const AZStd::string& exportPath, AZ::RPI::MaterialSourceData& sourceData, PropertyFilterFunction propertyFilter) const; + // AtomToolsFramework::AtomToolsDocument overrides... void Clear() override; bool ReopenRecordState() override; diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Viewport/MaterialViewportComponent.cpp b/Gems/Atom/Tools/MaterialEditor/Code/Source/Viewport/MaterialViewportComponent.cpp index a7b1fcc0c9..f25d83556d 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Viewport/MaterialViewportComponent.cpp +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Viewport/MaterialViewportComponent.cpp @@ -442,16 +442,16 @@ namespace MaterialEditor if (AZ::StringFunc::EndsWith(assetInfo.m_relativePath.c_str(), ".lightingpreset.azasset")) { m_lightingPresetAssets[assetInfo.m_assetId] = { assetInfo.m_assetId, assetInfo.m_assetType }; - AZ::Data::AssetBus::MultiHandler::BusConnect(assetInfo.m_assetId); m_lightingPresetAssets[assetInfo.m_assetId].QueueLoad(); + AZ::Data::AssetBus::MultiHandler::BusConnect(assetInfo.m_assetId); return; } if (AzFramework::StringFunc::EndsWith(assetInfo.m_relativePath.c_str(), ".modelpreset.azasset")) { m_modelPresetAssets[assetInfo.m_assetId] = { assetInfo.m_assetId, assetInfo.m_assetType }; - AZ::Data::AssetBus::MultiHandler::BusConnect(assetInfo.m_assetId); m_modelPresetAssets[assetInfo.m_assetId].QueueLoad(); + AZ::Data::AssetBus::MultiHandler::BusConnect(assetInfo.m_assetId); return; } }; diff --git a/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Document/ShaderManagementConsoleDocument.h b/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Document/ShaderManagementConsoleDocument.h index 2c22880ffe..4ad951e463 100644 --- a/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Document/ShaderManagementConsoleDocument.h +++ b/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Document/ShaderManagementConsoleDocument.h @@ -49,7 +49,9 @@ namespace ShaderManagementConsole const AZ::RPI::ShaderOptionDescriptor& GetShaderOptionDescriptor(size_t index) const override; private: + // AtomToolsFramework::AtomToolsDocument overrides... void Clear() override; + bool SaveSourceData(); // Source data for shader variant list @@ -58,6 +60,6 @@ namespace ShaderManagementConsole // Shader asset for the corresponding shader variant list AZ::Data::Asset m_shaderAsset; - const AZ::RPI::ShaderOptionDescriptor m_invalidDescriptor; + AZ::RPI::ShaderOptionDescriptor m_invalidDescriptor; }; } // namespace ShaderManagementConsole From 9ded52b1413ec077707232d768551ed7f159d796 Mon Sep 17 00:00:00 2001 From: Guthrie Adams Date: Mon, 31 Jan 2022 10:55:59 -0600 Subject: [PATCH 3/7] Wrapping redundant save logic into function in the material editor document class Signed-off-by: Guthrie Adams --- .../Code/Source/Document/MaterialDocument.cpp | 69 ++++++------------- .../Code/Source/Document/MaterialDocument.h | 3 +- 2 files changed, 23 insertions(+), 49 deletions(-) diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp index a8404baec1..c5b5bec8df 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp @@ -168,27 +168,17 @@ namespace MaterialEditor return false; } - // create source data from properties + // populate sourceData with modified or overridden properties and save object AZ::RPI::MaterialSourceData sourceData; sourceData.m_materialTypeVersion = m_materialAsset->GetMaterialTypeAsset()->GetVersion(); sourceData.m_materialType = AtomToolsFramework::GetExteralReferencePath(m_absolutePath, m_materialSourceData.m_materialType); sourceData.m_parentMaterial = AtomToolsFramework::GetExteralReferencePath(m_absolutePath, m_materialSourceData.m_parentMaterial); - - // populate sourceData with modified or overwritten properties - const bool savedProperties = SavePropertiesToSourceData(m_absolutePath, sourceData, [](const AtomToolsFramework::DynamicProperty& property) - { + auto propertyFilter = [](const AtomToolsFramework::DynamicProperty& property) { return !AtomToolsFramework::ArePropertyValuesEqual(property.GetValue(), property.GetConfig().m_parentValue); - }); - - if (!savedProperties) - { - return SaveFailed(); - } + }; - // write sourceData to .material file - if (!AZ::RPI::JsonUtils::SaveObjectToFile(m_absolutePath, sourceData)) + if (!SaveSourceData(sourceData, propertyFilter)) { - AZ_Error("MaterialDocument", false, "Document could not be saved: '%s'.", m_absolutePath.c_str()); return SaveFailed(); } @@ -211,27 +201,17 @@ namespace MaterialEditor return false; } - // create source data from properties + // populate sourceData with modified or overridden properties and save object AZ::RPI::MaterialSourceData sourceData; sourceData.m_materialTypeVersion = m_materialAsset->GetMaterialTypeAsset()->GetVersion(); sourceData.m_materialType = AtomToolsFramework::GetExteralReferencePath(m_savePathNormalized, m_materialSourceData.m_materialType); sourceData.m_parentMaterial = AtomToolsFramework::GetExteralReferencePath(m_savePathNormalized, m_materialSourceData.m_parentMaterial); - - // populate sourceData with modified or overwritten properties - const bool savedProperties = SavePropertiesToSourceData(m_savePathNormalized, sourceData, [](const AtomToolsFramework::DynamicProperty& property) - { + auto propertyFilter = [](const AtomToolsFramework::DynamicProperty& property) { return !AtomToolsFramework::ArePropertyValuesEqual(property.GetValue(), property.GetConfig().m_parentValue); - }); - - if (!savedProperties) - { - return SaveFailed(); - } + }; - // write sourceData to .material file - if (!AZ::RPI::JsonUtils::SaveObjectToFile(m_savePathNormalized, sourceData)) + if (!SaveSourceData(sourceData, propertyFilter)) { - AZ_Error("MaterialDocument", false, "Document could not be saved: '%s'.", m_savePathNormalized.c_str()); return SaveFailed(); } @@ -251,7 +231,7 @@ namespace MaterialEditor return false; } - // create source data from properties + // populate sourceData with modified or overridden properties and save object AZ::RPI::MaterialSourceData sourceData; sourceData.m_materialTypeVersion = m_materialAsset->GetMaterialTypeAsset()->GetVersion(); sourceData.m_materialType = AtomToolsFramework::GetExteralReferencePath(m_savePathNormalized, m_materialSourceData.m_materialType); @@ -262,21 +242,12 @@ namespace MaterialEditor sourceData.m_parentMaterial = AtomToolsFramework::GetExteralReferencePath(m_savePathNormalized, m_absolutePath); } - // populate sourceData with modified properties - const bool savedProperties = SavePropertiesToSourceData(m_savePathNormalized, sourceData, [](const AtomToolsFramework::DynamicProperty& property) - { + auto propertyFilter = [](const AtomToolsFramework::DynamicProperty& property) { return !AtomToolsFramework::ArePropertyValuesEqual(property.GetValue(), property.GetConfig().m_originalValue); - }); - - if (!savedProperties) - { - return SaveFailed(); - } + }; - // write sourceData to .material file - if (!AZ::RPI::JsonUtils::SaveObjectToFile(m_savePathNormalized, sourceData)) + if (!SaveSourceData(sourceData, propertyFilter)) { - AZ_Error("MaterialDocument", false, "Document could not be saved: '%s'.", m_savePathNormalized.c_str()); return SaveFailed(); } @@ -362,13 +333,12 @@ namespace MaterialEditor } } - bool MaterialDocument::SavePropertiesToSourceData( - const AZStd::string& exportPath, AZ::RPI::MaterialSourceData& sourceData, PropertyFilterFunction propertyFilter) const + bool MaterialDocument::SaveSourceData(AZ::RPI::MaterialSourceData& sourceData, PropertyFilterFunction propertyFilter) const { - bool result = true; + bool addPropertiesResult = true; // populate sourceData with properties that meet the filter - m_materialTypeSourceData.EnumerateProperties([&](const AZStd::string& propertyIdContext, const auto& propertyDefinition) { + m_materialTypeSourceData.EnumerateProperties([&, this](const AZStd::string& propertyIdContext, const auto& propertyDefinition) { AZ::Name propertyId{propertyIdContext + propertyDefinition->GetName()}; @@ -381,7 +351,7 @@ namespace MaterialEditor if (!AtomToolsFramework::ConvertToExportFormat(exportPath, propertyId, *propertyDefinition, propertyValue)) { AZ_Error("MaterialDocument", false, "Document property could not be converted: '%s' in '%s'.", propertyId.GetCStr(), m_absolutePath.c_str()); - result = false; + addPropertiesResult = false; return false; } @@ -393,7 +363,12 @@ namespace MaterialEditor return true; }); - return result; + if (!addPropertiesResult || !AZ::RPI::JsonUtils::SaveObjectToFile(m_savePathNormalized, sourceData)) + { + AZ_Error("MaterialDocument", false, "Document could not be saved: '%s'.", m_savePathNormalized.c_str()); + return false; + } + return true; } bool MaterialDocument::Open(AZStd::string_view loadPath) diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.h b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.h index 99e2af7a73..b03d0943fa 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.h +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.h @@ -74,8 +74,7 @@ namespace MaterialEditor // AZ::TickBus overrides... void OnTick(float deltaTime, AZ::ScriptTimePoint time) override; - bool SavePropertiesToSourceData( - const AZStd::string& exportPath, AZ::RPI::MaterialSourceData& sourceData, PropertyFilterFunction propertyFilter) const; + bool SaveSourceData(AZ::RPI::MaterialSourceData& sourceData, PropertyFilterFunction propertyFilter) const; // AtomToolsFramework::AtomToolsDocument overrides... void Clear() override; From 8617d34724d459e728ea96c97ad528501ab308aa Mon Sep 17 00:00:00 2001 From: Guthrie Adams Date: Mon, 31 Jan 2022 15:38:20 -0600 Subject: [PATCH 4/7] updated comment Signed-off-by: Guthrie Adams --- .../Include/AtomToolsFramework/Document/AtomToolsDocument.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocument.h b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocument.h index c2ef7f9594..cb64d23cd0 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocument.h +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocument.h @@ -83,7 +83,7 @@ namespace AtomToolsFramework AtomToolsFramework::DynamicProperty m_invalidProperty; //! This contains absolute paths of other source files that affect this document. - //! If any of the source files in this container are modified, the document system will he notified to reload this document. + //! If any of the source files in this container are modified, the document system is notified to reload this document. AZStd::unordered_set m_sourceDependencies; //! If this flag is true then the next source file change notification for this document will be ignored. From 66be8543cdf31e75c753210d68bfba6f9293028c Mon Sep 17 00:00:00 2001 From: Guthrie Adams Date: Mon, 31 Jan 2022 16:21:32 -0600 Subject: [PATCH 5/7] fixing merge issues Signed-off-by: Guthrie Adams --- .../Code/Source/Document/MaterialDocument.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp index c5b5bec8df..f25400721e 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp @@ -348,7 +348,7 @@ namespace MaterialEditor AZ::RPI::MaterialPropertyValue propertyValue = AtomToolsFramework::ConvertToRuntimeType(it->second.GetValue()); if (propertyValue.IsValid()) { - if (!AtomToolsFramework::ConvertToExportFormat(exportPath, propertyId, *propertyDefinition, propertyValue)) + if (!AtomToolsFramework::ConvertToExportFormat(m_savePathNormalized, propertyId, *propertyDefinition, propertyValue)) { AZ_Error("MaterialDocument", false, "Document property could not be converted: '%s' in '%s'.", propertyId.GetCStr(), m_absolutePath.c_str()); addPropertiesResult = false; @@ -608,13 +608,13 @@ namespace MaterialEditor // Add material functors that are in the top-level functors list. const AZ::RPI::MaterialFunctorSourceData::EditorContext editorContext = AZ::RPI::MaterialFunctorSourceData::EditorContext(m_materialSourceData.m_materialType, m_materialAsset->GetMaterialPropertiesLayout()); - for (Ptr functorData : m_materialTypeSourceData.m_materialFunctorSourceData) + for (AZ::RPI::Ptr functorData : m_materialTypeSourceData.m_materialFunctorSourceData) { AZ::RPI::MaterialFunctorSourceData::FunctorResult result2 = functorData->CreateFunctor(editorContext); if (result2.IsSuccess()) { - Ptr& functor = result2.GetValue(); + AZ::RPI::Ptr& functor = result2.GetValue(); if (functor != nullptr) { m_editorFunctors.push_back(functor); @@ -634,13 +634,13 @@ namespace MaterialEditor const AZ::RPI::MaterialFunctorSourceData::EditorContext editorContext = AZ::RPI::MaterialFunctorSourceData::EditorContext( m_materialSourceData.m_materialType, m_materialAsset->GetMaterialPropertiesLayout()); - for (Ptr functorData : propertyGroup->GetFunctors()) + for (AZ::RPI::Ptr functorData : propertyGroup->GetFunctors()) { AZ::RPI::MaterialFunctorSourceData::FunctorResult result = functorData->CreateFunctor(editorContext); if (result.IsSuccess()) { - Ptr& functor = result.GetValue(); + AZ::RPI::Ptr& functor = result.GetValue(); if (functor != nullptr) { m_editorFunctors.push_back(functor); From ca0006f57090f2fa203ecceed59c12926ef98ef6 Mon Sep 17 00:00:00 2001 From: Guthrie Adams Date: Tue, 1 Feb 2022 13:32:49 -0600 Subject: [PATCH 6/7] Minor changes and comments after PR feedback Signed-off-by: Guthrie Adams --- .../Code/Source/Document/MaterialDocument.cpp | 15 ++++++++++++++- .../Document/ShaderManagementConsoleDocument.cpp | 6 ++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp index f25400721e..283b7a8a7f 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp @@ -165,6 +165,8 @@ namespace MaterialEditor { if (!AtomToolsDocument::Save()) { + // SaveFailed has already been called so just forward the result without additional notifications. + // TODO Replace bool return value with enum for open and save states. return false; } @@ -198,6 +200,8 @@ namespace MaterialEditor { if (!AtomToolsDocument::SaveAsCopy(savePath)) { + // SaveFailed has already been called so just forward the result without additional notifications. + // TODO Replace bool return value with enum for open and save states. return false; } @@ -228,6 +232,8 @@ namespace MaterialEditor { if (!AtomToolsDocument::SaveAsChild(savePath)) { + // SaveFailed has already been called so just forward the result without additional notifications. + // TODO Replace bool return value with enum for open and save states. return false; } @@ -363,11 +369,18 @@ namespace MaterialEditor return true; }); - if (!addPropertiesResult || !AZ::RPI::JsonUtils::SaveObjectToFile(m_savePathNormalized, sourceData)) + if (!addPropertiesResult) + { + AZ_Error("MaterialDocument", false, "Document properties could not be saved: '%s'.", m_savePathNormalized.c_str()); + return false; + } + + if (!AZ::RPI::JsonUtils::SaveObjectToFile(m_savePathNormalized, sourceData)) { AZ_Error("MaterialDocument", false, "Document could not be saved: '%s'.", m_savePathNormalized.c_str()); return false; } + return true; } diff --git a/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Document/ShaderManagementConsoleDocument.cpp b/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Document/ShaderManagementConsoleDocument.cpp index be5adfe4bf..99a3aea347 100644 --- a/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Document/ShaderManagementConsoleDocument.cpp +++ b/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Document/ShaderManagementConsoleDocument.cpp @@ -107,6 +107,8 @@ namespace ShaderManagementConsole { if (!AtomToolsDocument::Save()) { + // SaveFailed has already been called so just forward the result without additional notifications. + // TODO Replace bool return value with enum for open and save states. return false; } @@ -117,6 +119,8 @@ namespace ShaderManagementConsole { if (!AtomToolsDocument::SaveAsCopy(savePath)) { + // SaveFailed has already been called so just forward the result without additional notifications. + // TODO Replace bool return value with enum for open and save states. return false; } @@ -127,6 +131,8 @@ namespace ShaderManagementConsole { if (!AtomToolsDocument::SaveAsChild(savePath)) { + // SaveFailed has already been called so just forward the result without additional notifications. + // TODO Replace bool return value with enum for open and save states. return false; } From 670d22cb5b32404fd95cfd446e8a976661f64de5 Mon Sep 17 00:00:00 2001 From: Guthrie Adams Date: Tue, 1 Feb 2022 15:34:20 -0600 Subject: [PATCH 7/7] Added notifications after re opening a document Signed-off-by: Guthrie Adams --- .../Code/Source/Document/AtomToolsDocument.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocument.cpp b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocument.cpp index 1daf578679..84638eefc4 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocument.cpp +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocument.cpp @@ -102,6 +102,10 @@ namespace AtomToolsFramework return false; } + AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast( + &AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentModified, m_id); + AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast( + &AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentUndoStateChanged, m_id); return true; }