Merge pull request #280 from aws-lumberyard-dev/Atom/santorac/MaterialEditorHandlesMissingTextures2

Changed the overall strategy for how to handle missing image references

Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/PropertyAssetCtrl.cpp

@@ -528,7 +528,7 @@ namespace AzToolsFramework
         }
     }
     
-    void PropertyAssetCtrl::UpdateErrorButton(const AZStd::string& errorLog)
+    void PropertyAssetCtrl::UpdateErrorButton()
     {
         if (m_errorButton)
         {
@@ -543,12 +543,17 @@ namespace AzToolsFramework
             m_errorButton->setSizePolicy(QSizePolicy::Fixed, QSizePolicy::Fixed);
             m_errorButton->setFixedSize(QSize(16, 16));
             m_errorButton->setMouseTracking(true);
-            m_errorButton->setIcon(QIcon("Icons/PropertyEditor/error_icon.png"));
+            m_errorButton->setIcon(QIcon(":/PropertyEditor/Resources/error_icon.png"));
             m_errorButton->setToolTip("Show Errors");
 
             // Insert the error button after the asset label
             qobject_cast<QHBoxLayout*>(layout())->insertWidget(1, m_errorButton);
         }
+    }
+
+    void PropertyAssetCtrl::UpdateErrorButtonWithLog(const AZStd::string& errorLog)
+    {
+        UpdateErrorButton();
 
         // Connect pressed to opening the error dialog
         // Must capture this for call to QObject::connect
@@ -588,6 +593,21 @@ namespace AzToolsFramework
         });
     }
     
+    void PropertyAssetCtrl::UpdateErrorButtonWithMessage(const AZStd::string& message)
+    {
+        UpdateErrorButton();
+        
+        connect(m_errorButton, &QPushButton::clicked, this, [this, message]() {
+            QMessageBox::critical(nullptr, "Error", message.c_str());
+
+            // Without this, the error button would maintain focus after clicking, which left the red error icon in a blue-highlighted state
+            if (parentWidget())
+            {
+                parentWidget()->setFocus();
+            }
+        });
+    }
+
     void PropertyAssetCtrl::ClearAssetInternal()
     {
         SetCurrentAssetHint(AZStd::string());
@@ -960,7 +980,6 @@ namespace AzToolsFramework
         else
         {
             const AZ::Data::AssetId assetID = GetCurrentAssetID();
-            m_currentAssetHint = "";
 
             AZ::Outcome<AssetSystem::JobInfoContainer> jobOutcome = AZ::Failure();
             AssetSystemJobRequestBus::BroadcastResult(jobOutcome, &AssetSystemJobRequestBus::Events::GetAssetJobsInfoByAssetID, assetID, false, false);
@@ -1018,7 +1037,7 @@ namespace AzToolsFramework
                         // In case of failure, render failure icon
                         case AssetSystem::JobStatus::Failed:
                         {
-                            UpdateErrorButton(errorLog);
+                            UpdateErrorButtonWithLog(errorLog);
                         }
                         break;
 
@@ -1043,6 +1062,10 @@ namespace AzToolsFramework
                     m_currentAssetHint = assetPath;
                 }
             }
+            else
+            {
+                UpdateErrorButtonWithMessage(AZStd::string::format("Asset is missing.\n\nID: %s\nHint:%s", assetID.ToString<AZStd::string>().c_str(), GetCurrentAssetHint().c_str()));
+            }
         }
 
         // Get the asset file name

Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/PropertyAssetCtrl.hxx

@@ -168,7 +168,9 @@ namespace AzToolsFramework
 
         bool IsCorrectMimeData(const QMimeData* pData, AZ::Data::AssetId* pAssetId = nullptr, AZ::Data::AssetType* pAssetType = nullptr) const;
         void ClearErrorButton();
-        void UpdateErrorButton(const AZStd::string& errorLog);
+        void UpdateErrorButton();
+        void UpdateErrorButtonWithLog(const AZStd::string& errorLog);
+        void UpdateErrorButtonWithMessage(const AZStd::string& message);
         virtual const AZStd::string GetFolderSelection() const { return AZStd::string(); }
         virtual void SetFolderSelection(const AZStd::string& /* folderPath */) {}
         virtual void ClearAssetInternal();

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

@@ -211,7 +211,7 @@ namespace AZ
 
             //! Convert the property value into the format that will be stored in the source data
             //! This is primarily needed to support conversions of special types like enums and images
-            bool ConvertPropertyValueToSourceDataFormat(const PropertyDefinition& propertyDefinition, MaterialPropertyValue& propertyValue) const;
+            bool ConvertPropertyValueToSourceDataFormat(const AZ::Name& propertyId, const PropertyDefinition& propertyDefinition, MaterialPropertyValue& propertyValue) const;
 
             Outcome<Data::Asset<MaterialTypeAsset>> CreateMaterialTypeAsset(Data::AssetId assetId, AZStd::string_view materialTypeSourceFilePath = "", bool elevateWarnings = true) const;
             

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

@@ -300,14 +300,14 @@ namespace AZ
             }
         }
 
-        bool MaterialTypeSourceData::ConvertPropertyValueToSourceDataFormat(const PropertyDefinition& propertyDefinition, MaterialPropertyValue& propertyValue) const
+        bool MaterialTypeSourceData::ConvertPropertyValueToSourceDataFormat([[maybe_unused]] const AZ::Name& propertyId, const PropertyDefinition& propertyDefinition, MaterialPropertyValue& propertyValue) const
         {
             if (propertyDefinition.m_dataType == AZ::RPI::MaterialPropertyDataType::Enum && propertyValue.Is<uint32_t>())
             {
                 const uint32_t index = propertyValue.GetValue<uint32_t>();
                 if (index >= propertyDefinition.m_enumValues.size())
                 {
-                    AZ_Error("Material source data", false, "Invalid value for material enum property: '%s'.", propertyDefinition.m_name.c_str());
+                    AZ_Error("Material source data", false, "Invalid value for material enum property: '%s'.", propertyId.GetCStr());
                     return false;
                 }
 
@@ -330,7 +330,7 @@ namespace AZ
                         imageAsset.GetId(), imageAsset.GetType(), platformName, imageAssetInfo, rootFilePath);
                     if (!result)
                     {
-                        AZ_Error("Material source data", false, "Image asset could not be found for property: '%s'.", propertyDefinition.m_name.c_str());
+                        AZ_Error("Material source data", false, "Image asset could not be found for property: '%s'.", propertyId.GetCStr());
                         return false;
                     }
                 }

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

@@ -43,18 +43,9 @@ namespace AZ
                     
                     if (!imageAssetId.IsSuccess())
                     {
-                        constexpr static char ErrorMissingTexture[] = "textures/defaults/missing.png";
+                        static const Uuid InvalidAssetPlaceholderId = "{BADA55E7-1A1D-4940-B655-9D08679BD62F}";
-                        imageAssetId = AssetUtils::MakeAssetId(ErrorMissingTexture, StreamingImageAsset::GetImageAssetSubId());
+                        imageAsset = Data::Asset<ImageAsset>{InvalidAssetPlaceholderId, azrtti_typeid<StreamingImageAsset>(), imageFilePath};
-                        
+                        return GetImageAssetResult::Missing;
-                        if (imageAssetId.IsSuccess())
-                        {
-                            imageAsset = Data::Asset<ImageAsset>{imageAssetId.GetValue(), azrtti_typeid<StreamingImageAsset>(), imageFilePath};
-                            return GetImageAssetResult::Missing;
-                        }
-                        else
-                        {
-                            return GetImageAssetResult::MissingNoFallback;
-                        }
                     }
                     
                     imageAsset = Data::Asset<ImageAsset>{imageAssetId.GetValue(), azrtti_typeid<StreamingImageAsset>(), imageFilePath};

Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialPropertyValue.cpp

@@ -118,12 +118,16 @@ namespace AZ
             else if (value.is<Data::Asset<Data::AssetData>>())
             {
                 result.m_value = Data::Asset<RPI::ImageAsset>(
-                    AZStd::any_cast<Data::Asset<Data::AssetData>>(value).GetId(), azrtti_typeid<RPI::StreamingImageAsset>());
+                    AZStd::any_cast<Data::Asset<Data::AssetData>>(value).GetId(),
+                    azrtti_typeid<RPI::StreamingImageAsset>(),
+                    AZStd::any_cast<Data::Asset<Data::AssetData>>(value).GetHint());
             }
             else if (value.is<Data::Asset<StreamingImageAsset>>())
             {
                 result.m_value = Data::Asset<RPI::ImageAsset>(
-                    AZStd::any_cast<Data::Asset<StreamingImageAsset>>(value).GetId(), azrtti_typeid<RPI::StreamingImageAsset>());
+                    AZStd::any_cast<Data::Asset<StreamingImageAsset>>(value).GetId(),
+                    azrtti_typeid<RPI::StreamingImageAsset>(),
+                    AZStd::any_cast<Data::Asset<StreamingImageAsset>>(value).GetHint());
             }
             else if (value.is<Data::Asset<ImageAsset>>())
             {

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

@@ -640,7 +640,7 @@ namespace MaterialEditor
                 MaterialPropertyValue propertyValue = AtomToolsFramework::ConvertToRuntimeType(it->second.GetValue());
                 if (propertyValue.IsValid())
                 {
-                    if (!m_materialTypeSourceData.ConvertPropertyValueToSourceDataFormat(propertyDefinition, propertyValue))
+                    if (!m_materialTypeSourceData.ConvertPropertyValueToSourceDataFormat(propertyId.GetFullName(), propertyDefinition, propertyValue))
                     {
                         AZ_Error("MaterialDocument", false, "Material document property could not be converted: '%s' in '%s'.", propertyId.GetFullName().GetCStr(), m_absolutePath.c_str());
                         result = false;

Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentUtil.cpp

@@ -156,7 +156,7 @@ namespace AZ
                         propertyValue = AZ::RPI::MaterialPropertyValue::FromAny(propertyOverrideItr->second);
                     }
 
-                    if (!editData.m_materialTypeSourceData.ConvertPropertyValueToSourceDataFormat(propertyDefinition, propertyValue))
+                    if (!editData.m_materialTypeSourceData.ConvertPropertyValueToSourceDataFormat(propertyId.GetFullName(), propertyDefinition, propertyValue))
                     {
                         AZ_Error("AZ::Render::EditorMaterialComponentUtil", false, "Failed to export: %s", path.c_str());
                         result = false;