Changed the overall strategy for how to handle missing image references. Instead of replacing it with one of the placeholder assets, we replace it with an random UUID which will be interpreted as a missing asset (unless some discovers discovers a UUID collision). This eventually gets replaced by one of the placeholder textures at runtime. This approach gives more consistent results in how missing texture are handled between Material Editor and Material Component.

I actually tried this approach before and it didn't seem to work the way we needed, but I realized that's because PropertyAssetCtrl wasn't handling missing assets properly. I fixed a few issues there including showing the error button when the asset can't be found, and fixing a broken reference to the error icon file.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>

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

@@ -527,8 +527,8 @@ namespace AzToolsFramework
             m_errorButton = nullptr;
         }
     }
-
-    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
@@ -587,6 +592,21 @@ namespace AzToolsFramework
             logDialog->show();
         });
     }
+    
+    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()
     {
@@ -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";
-                        imageAssetId = AssetUtils::MakeAssetId(ErrorMissingTexture, StreamingImageAsset::GetImageAssetSubId());
-                        
-                        if (imageAssetId.IsSuccess())
-                        {
-                            imageAsset = Data::Asset<ImageAsset>{imageAssetId.GetValue(), azrtti_typeid<StreamingImageAsset>(), imageFilePath};
-                            return GetImageAssetResult::Missing;
-                        }
-                        else
-                        {
-                            return GetImageAssetResult::MissingNoFallback;
-                        }
+                        static const Uuid InvalidAssetPlaceholderId = "{BADA55E7-1A1D-4940-B655-9D08679BD62F}";
+                        imageAsset = Data::Asset<ImageAsset>{InvalidAssetPlaceholderId, azrtti_typeid<StreamingImageAsset>(), imageFilePath};
+                        return GetImageAssetResult::Missing;
                     }
                     
                     imageAsset = Data::Asset<ImageAsset>{imageAssetId.GetValue(), azrtti_typeid<StreamingImageAsset>(), imageFilePath};

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;