birkeh/o3de
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fixed several material editor bugs related to file paths and hot reload prompts (#6374)

Browse Source 
* draft

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

* Removes automatic generation of relative paths for external references for materials
Updated material editor functions for creating new materials, creating or saving model or lighting presets, to save to the project asset folder instead of the material folder which is not included in the new templates
Changed function for getting saved file names to handle case where Qt save file dialog adds double extensions if the extension contains a dot

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

* Fixed problems with material editor hot reloading after documents or dependencies changed.
Triggering message boxes within the tick function, which is executed from the main application timer, caused the tick function to be called a second time recursively.
Switched from using the tick bus to a timer so that the documents re opening and dialogs are triggered outside of the main tick.

Signed-off-by: Guthrie Adams <guthadam@amazon.com>
This commit is contained in:
Guthrie Adams
2021-12-14 10:52:01 -06:00
committed by GitHub
parent f3d426e638
commit f8ca427954
10 changed files with 60 additions and 77 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Util/MaterialPropertyUtil.h
+3 -5
View File
@@ -53,14 +53,12 @@ namespace AtomToolsFramework
const AZ::RPI::MaterialTypeSourceData::PropertyDefinition& propertyDefinition,
AZ::RPI::MaterialPropertyValue& propertyValue);
//! Generate a file path from the exported file to the external reference.
//! This function returns a relative path from the export file to the reference file.
//! If the relative path is too different or distant from the export path then we return the asset folder relative path.
//! Generate a file path that is relative to either the source asset root or the export path
//! @param exportPath absolute path of the file being saved
//! @param referencePath absolute path of a file that will be treated as an external reference
//! @param maxPathDepth the maximum relative depth or number of parent or child folders between the export path and the reference path
//! @param relativeToExportPath specifies if the path is relative to the source asset root or the export path
AZStd::string GetExteralReferencePath(
const AZStd::string& exportPath, const AZStd::string& referencePath, const uint32_t maxPathDepth = 2);
const AZStd::string& exportPath, const AZStd::string& referencePath, const bool relativeToExportPath = false);
//! Traverse up the instance data node hierarchy to find the containing dynamic property object
const AtomToolsFramework::DynamicProperty* FindDynamicPropertyForInstanceDataNode(const AzToolsFramework::InstanceDataNode* pNode);
Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocumentSystemComponent.cpp
+14 -11
View File
@@ -24,6 +24,7 @@ AZ_PUSH_DISABLE_WARNING(4251 4800, "-Wunknown-warning-option") // disable warnin
#include <QApplication>
#include <QMessageBox>
#include <QString>
#include <QTimer>
AZ_POP_DISABLE_WARNING
namespace AtomToolsFramework
@@ -121,7 +122,6 @@ namespace AtomToolsFramework
void AtomToolsDocumentSystemComponent::Deactivate()
{
AZ::TickBus::Handler::BusDisconnect();
AtomToolsDocumentNotificationBus::Handler::BusDisconnect();
AtomToolsDocumentSystemRequestBus::Handler::BusDisconnect();
m_documentMap.clear();
@@ -160,25 +160,30 @@ namespace AtomToolsFramework
void AtomToolsDocumentSystemComponent::OnDocumentExternallyModified(const AZ::Uuid& documentId)
{
m_documentIdsWithExternalChanges.insert(documentId);
if (!AZ::TickBus::Handler::BusIsConnected())
{
AZ::TickBus::Handler::BusConnect();
}
QueueReopenDocuments();
}
void AtomToolsDocumentSystemComponent::OnDocumentDependencyModified(const AZ::Uuid& documentId)
{
m_documentIdsWithDependencyChanges.insert(documentId);
if (!AZ::TickBus::Handler::BusIsConnected())
QueueReopenDocuments();
}
void AtomToolsDocumentSystemComponent::QueueReopenDocuments()
{
if (!m_queueReopenDocuments)
{
AZ::TickBus::Handler::BusConnect();
m_queueReopenDocuments = true;
QTimer::singleShot(0, [this] { ReopenDocuments(); });
}
}
void AtomToolsDocumentSystemComponent::OnTick([[maybe_unused]] float deltaTime, [[maybe_unused]] AZ::ScriptTimePoint time)
void AtomToolsDocumentSystemComponent::ReopenDocuments()
{
for (const AZ::Uuid& documentId : m_documentIdsWithExternalChanges)
{
m_documentIdsWithDependencyChanges.erase(documentId);
AZStd::string documentPath;
AtomToolsDocumentRequestBus::EventResult(documentPath, documentId, &AtomToolsDocumentRequestBus::Events::GetAbsolutePath);
@@ -191,8 +196,6 @@ namespace AtomToolsFramework
continue;
}
m_documentIdsWithDependencyChanges.erase(documentId);
AtomToolsFramework::TraceRecorder traceRecorder(m_maxMessageBoxLineCount);
bool openResult = false;
@@ -235,7 +238,7 @@ namespace AtomToolsFramework
m_documentIdsWithDependencyChanges.clear();
m_documentIdsWithExternalChanges.clear();
AZ::TickBus::Handler::BusDisconnect();
m_queueReopenDocuments = false;
}
AZ::Uuid AtomToolsDocumentSystemComponent::OpenDocument(AZStd::string_view sourcePath)
Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocumentSystemComponent.h
+3 -6
View File
@@ -9,7 +9,6 @@
#pragma once
#include <AzCore/Component/Component.h>
#include <AzCore/Component/TickBus.h>
#include <AzCore/std/smart_ptr/shared_ptr.h>
#include <AzCore/Asset/AssetCommon.h>
@@ -28,7 +27,6 @@ namespace AtomToolsFramework
//! AtomToolsDocumentSystemComponent is the central component of the Material Editor Core gem
class AtomToolsDocumentSystemComponent
: public AZ::Component
, private AZ::TickBus::Handler
, private AtomToolsDocumentNotificationBus::Handler
, private AtomToolsDocumentSystemRequestBus::Handler
{
@@ -59,10 +57,8 @@ namespace AtomToolsFramework
void OnDocumentExternallyModified(const AZ::Uuid& documentId) override;
//////////////////////////////////////////////////////////////////////////
////////////////////////////////////////////////////////////////////////
// AZ::TickBus::Handler overrides...
void OnTick(float deltaTime, AZ::ScriptTimePoint time) override;
////////////////////////////////////////////////////////////////////////
void QueueReopenDocuments();
void ReopenDocuments();
////////////////////////////////////////////////////////////////////////
// AtomToolsDocumentSystemRequestBus::Handler overrides...
@@ -87,6 +83,7 @@ namespace AtomToolsFramework
AZStd::unordered_map<AZ::Uuid, AZStd::shared_ptr<AtomToolsDocument>> m_documentMap;
AZStd::unordered_set<AZ::Uuid> m_documentIdsWithExternalChanges;
AZStd::unordered_set<AZ::Uuid> m_documentIdsWithDependencyChanges;
bool m_queueReopenDocuments = false;
const size_t m_maxMessageBoxLineCount = 15;
};
} // namespace AtomToolsFramework
Gems/Atom/Tools/AtomToolsFramework/Code/Source/Util/MaterialPropertyUtil.cpp
+6 -20
View File
@@ -222,31 +222,15 @@ namespace AtomToolsFramework
return true;
}
AZStd::string GetExteralReferencePath(const AZStd::string& exportPath, const AZStd::string& referencePath, const uint32_t maxPathDepth)
AZStd::string GetExteralReferencePath(
const AZStd::string& exportPath, const AZStd::string& referencePath, const bool relativeToExportPath)
{
if (referencePath.empty())
{
return {};
}
AZ::IO::BasicPath<AZStd::string> exportFolder(exportPath);
exportFolder.RemoveFilename();
const AZStd::string relativePath = AZ::IO::PathView(referencePath).LexicallyRelative(exportFolder).StringAsPosix();
// Count the difference in depth between the export file path and the referenced file path.
uint32_t parentFolderCount = 0;
AZStd::string::size_type pos = 0;
const AZStd::string parentFolderToken = "..";
while ((pos = relativePath.find(parentFolderToken, pos)) != AZStd::string::npos)
{
parentFolderCount++;
pos += parentFolderToken.length();
}
// If the difference in depth is too great then revert to using the asset folder relative path.
// We could change this to only use relative paths for references in subfolders.
if (parentFolderCount > maxPathDepth)
if (!relativeToExportPath)
{
AZStd::string watchFolder;
AZ::Data::AssetInfo assetInfo;
@@ -260,7 +244,9 @@ namespace AtomToolsFramework
}
}
return relativePath;
AZ::IO::BasicPath<AZStd::string> exportFolder(exportPath);
exportFolder.RemoveFilename();
return AZ::IO::PathView(referencePath).LexicallyRelative(exportFolder).StringAsPosix();
}
const AtomToolsFramework::DynamicProperty* FindDynamicPropertyForInstanceDataNode(const AzToolsFramework::InstanceDataNode* pNode)
Gems/Atom/Tools/AtomToolsFramework/Code/Source/Util/Util.cpp
+5 -1
View File
@@ -62,6 +62,8 @@ namespace AtomToolsFramework
const QFileInfo initialFileInfo(initialPath);
const QString initialExt(initialFileInfo.completeSuffix());
// Instead of just passing in the absolute file path, we pass in the absolute folder path and the base name to prevent the file
// dialog from displaying multiple extensions when the extension contains a "."
const QFileInfo selectedFileInfo(AzQtComponents::FileDialog::GetSaveFileName(
QApplication::activeWindow(),
"Save File",
@@ -82,7 +84,9 @@ namespace AtomToolsFramework
return QFileInfo();
}
return selectedFileInfo;
// Reconstructing the file info from the absolute path and expected extension to compensate for an issue with the save file
// dialog adding the extension multiple times if it contains "." like *.lightingpreset.azasset
return QFileInfo(selectedFileInfo.absolutePath() + AZ_CORRECT_FILESYSTEM_SEPARATOR_STRING + selectedFileInfo.baseName() + "." + initialExt);
}
QFileInfo GetOpenFileInfo(const AZStd::vector<AZ::Data::AssetType>& assetTypes)
Gems/Atom/Tools/AtomToolsFramework/Code/Tests/AtomToolsFrameworkTest.cpp
+9 -9
View File
@@ -66,15 +66,15 @@ namespace UnitTest
TEST_F(AtomToolsFrameworkTest, GetExteralReferencePath_Succeeds)
{
ASSERT_EQ(AtomToolsFramework::GetExteralReferencePath("", "", 2), "");
ASSERT_EQ(AtomToolsFramework::GetExteralReferencePath("d:/project/assets/materials/condor.material", "", 2), "");
ASSERT_EQ(AtomToolsFramework::GetExteralReferencePath("d:/project/assets/materials/talisman.material", "", 2), "");
ASSERT_EQ(AtomToolsFramework::GetExteralReferencePath("d:/project/assets/materials/talisman.material", "d:/project/assets/textures/gold.png", 2), "../textures/gold.png");
ASSERT_EQ(AtomToolsFramework::GetExteralReferencePath("d:/project/assets/materials/talisman.material", "d:/project/assets/textures/gold.png", 0), "textures/gold.png");
ASSERT_EQ(AtomToolsFramework::GetExteralReferencePath("d:/project/assets/objects/upgrades/materials/supercondor.material", "d:/project/assets/materials/condor.material", 3), "../../../materials/condor.material");
ASSERT_EQ(AtomToolsFramework::GetExteralReferencePath("d:/project/assets/objects/upgrades/materials/supercondor.material", "d:/project/assets/materials/condor.material", 2), "materials/condor.material");
ASSERT_EQ(AtomToolsFramework::GetExteralReferencePath("d:/project/assets/objects/upgrades/materials/supercondor.material", "d:/project/assets/materials/condor.material", 1), "materials/condor.material");
ASSERT_EQ(AtomToolsFramework::GetExteralReferencePath("d:/project/assets/objects/upgrades/materials/supercondor.material", "d:/project/assets/materials/condor.material", 0), "materials/condor.material");
ASSERT_EQ(AtomToolsFramework::GetExteralReferencePath("", "", true), "");
ASSERT_EQ(AtomToolsFramework::GetExteralReferencePath("d:/project/assets/materials/condor.material", "", true), "");
ASSERT_EQ(AtomToolsFramework::GetExteralReferencePath("d:/project/assets/materials/talisman.material", "", false), "");
ASSERT_EQ(AtomToolsFramework::GetExteralReferencePath("d:/project/assets/materials/talisman.material", "d:/project/assets/textures/gold.png", true), "../textures/gold.png");
ASSERT_EQ(AtomToolsFramework::GetExteralReferencePath("d:/project/assets/materials/talisman.material", "d:/project/assets/textures/gold.png", false), "textures/gold.png");
ASSERT_EQ(AtomToolsFramework::GetExteralReferencePath("d:/project/assets/objects/upgrades/materials/supercondor.material", "d:/project/assets/materials/condor.material", true), "../../../materials/condor.material");
ASSERT_EQ(AtomToolsFramework::GetExteralReferencePath("d:/project/assets/objects/upgrades/materials/supercondor.material", "d:/project/assets/materials/condor.material", false), "materials/condor.material");
ASSERT_EQ(AtomToolsFramework::GetExteralReferencePath("d:/project/assets/objects/upgrades/materials/supercondor.material", "d:/project/assets/materials/condor.material", false), "materials/condor.material");
ASSERT_EQ(AtomToolsFramework::GetExteralReferencePath("d:/project/assets/objects/upgrades/materials/supercondor.material", "d:/project/assets/materials/condor.material", false), "materials/condor.material");
}
AZ_UNIT_TEST_HOOK(new AtomToolsFrameworkTestEnvironment);
Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp
+1 -1
View File
@@ -555,7 +555,7 @@ namespace MaterialEditor
void MaterialDocument::SourceFileChanged(AZStd::string relativePath, AZStd::string scanFolder, [[maybe_unused]] AZ::Uuid sourceUUID)
{
auto sourcePath = AZ::RPI::AssetUtils::ResolvePathReference(scanFolder, relativePath);
const auto sourcePath = AZ::RPI::AssetUtils::ResolvePathReference(scanFolder, relativePath);
if (m_absolutePath == sourcePath)
{
Gems/Atom/Tools/MaterialEditor/Code/Source/Window/CreateMaterialDialog/CreateMaterialDialog.cpp
+8 -12
View File
@@ -6,25 +6,21 @@
*
*/
#include <Window/CreateMaterialDialog/CreateMaterialDialog.h>
#include <AzFramework/Application/Application.h>
#include <AzFramework/StringFunc/StringFunc.h>
#include <AzQtComponents/Components/Widgets/FileDialog.h>
#include <AtomToolsFramework/Util/Util.h>
#include <Atom/Document/MaterialDocumentSettings.h>
#include <Atom/RPI.Edit/Common/AssetUtils.h>
#include <Atom/RPI.Edit/Material/MaterialSourceData.h>
#include <Atom/RPI.Edit/Material/MaterialTypeSourceData.h>
#include <Atom/Document/MaterialDocumentSettings.h>
#include <AtomToolsFramework/Util/Util.h>
#include <AzCore/Utils/Utils.h>
#include <AzFramework/Application/Application.h>
#include <AzFramework/StringFunc/StringFunc.h>
#include <AzQtComponents/Components/Widgets/FileDialog.h>
#include <Window/CreateMaterialDialog/CreateMaterialDialog.h>
namespace MaterialEditor
{
CreateMaterialDialog::CreateMaterialDialog(QWidget* parent)
: CreateMaterialDialog(QString(AZ::IO::FileIOBase::GetInstance()->GetAlias("@projectroot@")) + AZ_CORRECT_FILESYSTEM_SEPARATOR + "Materials", parent)
: CreateMaterialDialog(QString(AZ::Utils::GetProjectPath().c_str()) + AZ_CORRECT_FILESYSTEM_SEPARATOR + "Assets", parent)
{
}
Gems/Atom/Tools/MaterialEditor/Code/Source/Window/MaterialEditorBrowserInteractions.cpp
+5 -4
View File
@@ -12,6 +12,7 @@
#include <Atom/RPI.Reflect/Material/MaterialAsset.h>
#include <AtomToolsFramework/Document/AtomToolsDocumentSystemRequestBus.h>
#include <AtomToolsFramework/Util/Util.h>
#include <AzCore/Utils/Utils.h>
#include <AzCore/std/string/wildcard.h>
#include <AzQtComponents/Utilities/DesktopUtilities.h>
#include <AzToolsFramework/AssetBrowser/AssetBrowserBus.h>
@@ -106,8 +107,8 @@ namespace MaterialEditor
menu->addAction("Create Material...", [entry]()
{
const QString defaultPath = AtomToolsFramework::GetUniqueFileInfo(
QString(AZ::IO::FileIOBase::GetInstance()->GetAlias("@projectroot@")) +
AZ_CORRECT_FILESYSTEM_SEPARATOR + "Materials" +
QString(AZ::Utils::GetProjectPath().c_str()) +
AZ_CORRECT_FILESYSTEM_SEPARATOR + "Assets" +
AZ_CORRECT_FILESYSTEM_SEPARATOR + "untitled." +
AZ::RPI::MaterialSourceData::Extension).absoluteFilePath();
@@ -182,8 +183,8 @@ namespace MaterialEditor
menu->addAction("Create Child Material...", [entry]()
{
const QString defaultPath = AtomToolsFramework::GetUniqueFileInfo(
QString(AZ::IO::FileIOBase::GetInstance()->GetAlias("@projectroot@")) +
AZ_CORRECT_FILESYSTEM_SEPARATOR + "Materials" +
QString(AZ::Utils::GetProjectPath().c_str()) +
AZ_CORRECT_FILESYSTEM_SEPARATOR + "Assets" +
AZ_CORRECT_FILESYSTEM_SEPARATOR + "untitled." +
AZ::RPI::MaterialSourceData::Extension).absoluteFilePath();
Gems/Atom/Tools/MaterialEditor/Code/Source/Window/ViewportSettingsInspector/ViewportSettingsInspector.cpp
+6 -8
View File
@@ -6,10 +6,11 @@
*
*/
#include <Atom/RPI.Reflect/Model/ModelAsset.h>
#include <Atom/Viewport/MaterialViewportRequestBus.h>
#include <AtomToolsFramework/Inspector/InspectorPropertyGroupWidget.h>
#include <AtomToolsFramework/Util/Util.h>
#include <Atom/RPI.Reflect/Model/ModelAsset.h>
#include <AzCore/Utils/Utils.h>
#include <Window/PresetBrowserDialogs/LightingPresetBrowserDialog.h>
#include <Window/PresetBrowserDialogs/ModelPresetBrowserDialog.h>
#include <Window/ViewportSettingsInspector/ViewportSettingsInspector.h>
@@ -346,13 +347,10 @@ namespace MaterialEditor
AZStd::string ViewportSettingsInspector::GetDefaultUniqueSaveFilePath(const AZStd::string& baseName) const
{
AZStd::string savePath = AZ::IO::FileIOBase::GetInstance()->GetAlias("@projectroot@");
savePath += AZ_CORRECT_FILESYSTEM_SEPARATOR;
savePath += "Materials";
savePath += AZ_CORRECT_FILESYSTEM_SEPARATOR;
savePath += baseName;
savePath = AtomToolsFramework::GetUniqueFileInfo(savePath.c_str()).absoluteFilePath().toUtf8().constData();
return savePath;
return AtomToolsFramework::GetUniqueFileInfo(
QString(AZ::Utils::GetProjectPath().c_str()) +
AZ_CORRECT_FILESYSTEM_SEPARATOR + "Assets" +
AZ_CORRECT_FILESYSTEM_SEPARATOR + baseName.c_str()).absoluteFilePath().toUtf8().constData();
}
AZ::Crc32 ViewportSettingsInspector::GetGroupSaveStateKey(const AZStd::string& groupName) const