diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialUtils.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialUtils.h index d12e848a02..f39b8e7aaa 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialUtils.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialUtils.h @@ -28,7 +28,19 @@ namespace AZ namespace MaterialUtils { - Outcome> GetImageAssetReference(AZStd::string_view materialSourceFilePath, const AZStd::string imageFilePath); + enum class GetImageAssetResult + { + Empty, //! No image was actually requested, the path was empty + Found, //! The requested asset was found + Missing, //! The requested asset was not found, and a placeholder asset was used instead + MissingNoFallback //! The requested asset was not found, and a placeholder asset was not found either + }; + + //! Finds an ImageAsset referenced by a material file (or a placeholder) + //! @param imageAsset the resulting ImageAsset + //! @param materialSourceFilePath the full path to a material source file that is referenfing an image file + //! @param imageFilePath the path to an image source file, which could be relative to the asset root or relative to the material file + GetImageAssetResult GetImageAssetReference(Data::Asset& imageAsset, AZStd::string_view materialSourceFilePath, const AZStd::string imageFilePath); //! Resolve an enum to a uint32_t given its name and definition array (in MaterialPropertyDescriptor). //! @param propertyDescriptor it contains the definition of all enum names in an array. diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Image/ImageSystemInterface.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Image/ImageSystemInterface.h index f567881c50..920b763bc3 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Image/ImageSystemInterface.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Image/ImageSystemInterface.h @@ -29,6 +29,14 @@ namespace AZ Count }; + namespace DefaultImageAssetPaths + { + static constexpr char DefaultFallback[] = "textures/defaults/defaultfallback.png.streamingimage"; + static constexpr char Processing[] = "textures/defaults/processing.png.streamingimage"; + static constexpr char ProcessingFailed[] = "textures/defaults/processingfailed.png.streamingimage"; + static constexpr char Missing[] = "textures/defaults/missing.png.streamingimage"; + } + class ImageSystemInterface { public: diff --git a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceData.cpp b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceData.cpp index 40fc9e096f..a8d61465a5 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceData.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceData.cpp @@ -317,22 +317,20 @@ namespace AZ { case MaterialPropertyDataType::Image: { - Outcome> imageAssetResult = MaterialUtils::GetImageAssetReference( - materialSourceFilePath, property.second.m_value.GetValue()); + Data::Asset imageAsset; - if (imageAssetResult.IsSuccess()) - { - auto& imageAsset = imageAssetResult.GetValue(); - // Load referenced images when load material - imageAsset.SetAutoLoadBehavior(Data::AssetLoadBehavior::PreLoad); - materialAssetCreator.SetPropertyValue(propertyId.GetFullName(), imageAsset); - } - else + MaterialUtils::GetImageAssetResult result = MaterialUtils::GetImageAssetReference( + imageAsset, materialSourceFilePath, property.second.m_value.GetValue()); + + if (result == MaterialUtils::GetImageAssetResult::Missing || result == MaterialUtils::GetImageAssetResult::MissingNoFallback) { materialAssetCreator.ReportWarning( "Material property '%s': Could not find the image '%s'", propertyId.GetFullName().GetCStr(), property.second.m_value.GetValue().data()); } + + imageAsset.SetAutoLoadBehavior(Data::AssetLoadBehavior::PreLoad); + materialAssetCreator.SetPropertyValue(propertyId.GetFullName(), imageAsset); } break; case MaterialPropertyDataType::Enum: diff --git a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp index 396ba71e14..61f3d5282a 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp @@ -493,12 +493,13 @@ namespace AZ { case MaterialPropertyDataType::Image: { - auto imageAssetResult = MaterialUtils::GetImageAssetReference( - materialTypeSourceFilePath, property.m_value.GetValue()); + Data::Asset imageAsset; - if (imageAssetResult) + MaterialUtils::GetImageAssetResult result = MaterialUtils::GetImageAssetReference( + imageAsset, materialTypeSourceFilePath, property.m_value.GetValue()); + + if (result == MaterialUtils::GetImageAssetResult::Empty || result == MaterialUtils::GetImageAssetResult::Found) { - auto imageAsset = imageAssetResult.GetValue(); materialTypeAssetCreator.SetPropertyValue(propertyId.GetFullName(), imageAsset); } else diff --git a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialUtils.cpp b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialUtils.cpp index 90ce9e66ce..4bb4d75d40 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialUtils.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialUtils.cpp @@ -28,25 +28,37 @@ namespace AZ { namespace MaterialUtils { - Outcome> GetImageAssetReference(AZStd::string_view materialSourceFilePath, const AZStd::string imageFilePath) + GetImageAssetResult GetImageAssetReference(Data::Asset& imageAsset, AZStd::string_view materialSourceFilePath, const AZStd::string imageFilePath) { + imageAsset = {}; + if (imageFilePath.empty()) { // The image value was present but specified an empty string, meaning the texture asset should be explicitly cleared. - return AZ::Success(Data::Asset()); + return GetImageAssetResult::Empty; } else { Outcome imageAssetId = AssetUtils::MakeAssetId(materialSourceFilePath, imageFilePath, StreamingImageAsset::GetImageAssetSubId()); + if (!imageAssetId.IsSuccess()) { - return AZ::Failure(); - } - else - { - Data::Asset unloadedImageAssetReference(imageAssetId.GetValue(), azrtti_typeid(), imageFilePath); - return AZ::Success(unloadedImageAssetReference); + constexpr static char ErrorMissingTexture[] = "textures/defaults/missing.png"; + imageAssetId = AssetUtils::MakeAssetId(ErrorMissingTexture, StreamingImageAsset::GetImageAssetSubId()); + + if (imageAssetId.IsSuccess()) + { + imageAsset = Data::Asset{imageAssetId.GetValue(), azrtti_typeid(), imageFilePath}; + return GetImageAssetResult::Missing; + } + else + { + return GetImageAssetResult::MissingNoFallback; + } } + + imageAsset = Data::Asset{imageAssetId.GetValue(), azrtti_typeid(), imageFilePath}; + return GetImageAssetResult::Found; } } diff --git a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Image/StreamingImageAssetHandler.cpp b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Image/StreamingImageAssetHandler.cpp index f241ba6602..0b51962e86 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Image/StreamingImageAssetHandler.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Image/StreamingImageAssetHandler.cpp @@ -7,6 +7,7 @@ */ #include +#include #include #include @@ -53,7 +54,7 @@ namespace AZ // Generate asset info to use to register the fallback asset with the asset catalog Data::AssetInfo assetInfo; - assetInfo.m_relativePath = "textures/defaults/defaultfallback.png.streamingimage"; + assetInfo.m_relativePath = DefaultImageAssetPaths::DefaultFallback; assetInfo.m_assetType = azrtti_typeid(); bool useDebugFallbackImages = true; @@ -68,15 +69,15 @@ namespace AZ { case AzFramework::AssetSystem::AssetStatus::AssetStatus_Queued: case AzFramework::AssetSystem::AssetStatus::AssetStatus_Compiling: - assetInfo.m_relativePath = "textures/defaults/processing.png.streamingimage"; + assetInfo.m_relativePath = DefaultImageAssetPaths::Processing; break; case AzFramework::AssetSystem::AssetStatus::AssetStatus_Failed: - assetInfo.m_relativePath = "textures/defaults/processingfailed.png.streamingimage"; + assetInfo.m_relativePath = DefaultImageAssetPaths::ProcessingFailed; break; case AzFramework::AssetSystem::AssetStatus::AssetStatus_Missing: case AzFramework::AssetSystem::AssetStatus::AssetStatus_Unknown: case AzFramework::AssetSystem::AssetStatus::AssetStatus_Compiled: - assetInfo.m_relativePath = "textures/defaults/missing.png.streamingimage"; + assetInfo.m_relativePath = DefaultImageAssetPaths::Missing; break; } } 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..b385f21a5e 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocumentRequestBus.h +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocumentRequestBus.h @@ -73,6 +73,9 @@ namespace AtomToolsFramework //! Can the document be saved virtual bool IsSavable() const = 0; + //! Get a list of warnings about the data that would be good to know before saving + virtual AZStd::vector GetDataWarnings() const { return {}; } + //! Returns true if there are reversible modifications to the document virtual bool CanUndo() const = 0; diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocumentSystemComponent.cpp b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocumentSystemComponent.cpp index 3a392c1413..416babcbde 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocumentSystemComponent.cpp +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocumentSystemComponent.cpp @@ -365,6 +365,24 @@ namespace AtomToolsFramework return false; } + AZStd::vector dataWarnings; + AtomToolsDocumentRequestBus::EventResult(dataWarnings, documentId, &AtomToolsDocumentRequestBus::Events::GetDataWarnings); + if (!dataWarnings.empty()) + { + AZStd::string allWarnings; + AzFramework::StringFunc::Join(allWarnings, dataWarnings.begin(), dataWarnings.end(), "\n"); + + auto result = QMessageBox::warning( + QApplication::activeWindow(), QString("Data Warnings"), + QString("Are you sure you want to save with the following data warnings? \n\n%1").arg(allWarnings.c_str()), + QMessageBox::StandardButton::Yes, QMessageBox::StandardButton::No); + + if (result == QMessageBox::StandardButton::No) + { + return false; + } + } + AtomToolsFramework::TraceRecorder traceRecorder(m_maxMessageBoxLineCount); bool result = false; diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp index 6969e9f923..ea3d1fa319 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -467,6 +468,37 @@ namespace MaterialEditor { return AzFramework::StringFunc::Path::IsExtension(m_absolutePath.c_str(), AZ::RPI::MaterialSourceData::Extension); } + + AZStd::vector MaterialDocument::GetDataWarnings() const + { + AZStd::vector warnings; + + for (auto& [propertyName, dynamicProperty] : m_properties) + { + AZ::RPI::MaterialPropertyValue propertyValue = AtomToolsFramework::ConvertToRuntimeType(dynamicProperty.GetValue()); + if (propertyValue.Is>()) + { + auto isSameAsset = [&propertyValue](const char* path) + { + AZ::Data::AssetId assetId = propertyValue.GetValue>().GetId(); + AZ::Data::AssetId otherAssetId; + AZ::Data::AssetCatalogRequestBus::BroadcastResult(otherAssetId, &AZ::Data::AssetCatalogRequestBus::Events::GetAssetIdByPath, path, AZ::Data::AssetType{}, false); + return assetId == otherAssetId; + }; + + if (isSameAsset(AZ::RPI::DefaultImageAssetPaths::DefaultFallback) || + isSameAsset(AZ::RPI::DefaultImageAssetPaths::Missing) || + isSameAsset(AZ::RPI::DefaultImageAssetPaths::Processing) || + isSameAsset(AZ::RPI::DefaultImageAssetPaths::ProcessingFailed) + ) + { + warnings.push_back(AZStd::string::format("%s is using a placeholder image asset.", propertyName.GetCStr())); + } + } + } + + return warnings; + } bool MaterialDocument::CanUndo() const { diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.h b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.h index 452111f99a..d9f9dded0a 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.h +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.h @@ -55,6 +55,7 @@ namespace MaterialEditor bool IsOpen() const override; bool IsModified() const override; bool IsSavable() const override; + AZStd::vector GetDataWarnings() const override; bool CanUndo() const override; bool CanRedo() const override; bool Undo() override;