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 2fb55e5f40..2a992159b3 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 @@ -69,6 +69,7 @@ namespace AZ //! Finalizing during asset processing reduces load times and obfuscates the material data. //! Waiting to finalize at load time reduces dependencies on the material type data, resulting in fewer asset rebuilds and less time spent processing assets. bool BuildersShouldFinalizeMaterialAssets(); + } } } diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/AssetCreator.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/AssetCreator.h index 79d43d0b8d..257abcc785 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/AssetCreator.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/AssetCreator.h @@ -28,6 +28,7 @@ namespace AZ // [GFX TODO] We need to iterate on this concept at some point. We may want to expose it through cvars or something // like that, or we may not need this at all. For now it's helpful for testing. void SetElevateWarnings(bool elevated); + bool GetElevateWarnings() const { return m_warningsElevated; } int GetErrorCount() const { return m_errorCount; } int GetWarningCount() const { return m_warningCount; } diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialAsset.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialAsset.h index 34e51b40af..736d7c5b53 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialAsset.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialAsset.h @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -110,11 +111,6 @@ namespace AZ //! If false, property values can be accessed through GetRawPropertyValues(). bool IsFinalized() const; - //! If the material asset is not finalized yet, this does the final processing of m_rawPropertyValues to - //! get the material asset ready to be used. - //! Note m_materialTypeAsset must be valid before this is called. - void Finalize(); - //! Returns the list of values for all properties in this material. //! The entries in this list align with the entries in the MaterialPropertiesLayout. Each AZStd::any is guaranteed //! to have a value of type that matches the corresponding MaterialPropertyDescriptor. @@ -129,6 +125,12 @@ namespace AZ private: bool PostLoadInit() override; + //! If the material asset is not finalized yet, this does the final processing of m_rawPropertyValues to + //! get the material asset ready to be used. + //! Note m_materialTypeAsset must be valid before this is called. + //! @param elevateWarnings Indicates whether to treat warnings as errors + void Finalize(AZStd::function reportWarning = nullptr, AZStd::function reportError = nullptr); + //! Checks the material type version and potentially applies a series of property changes (most common are simple property renames) //! based on the MaterialTypeAsset's version update procedure. void ApplyVersionUpdates(); diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialAssetCreator.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialAssetCreator.h index b7b1f9705f..74bd909e08 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialAssetCreator.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialAssetCreator.h @@ -9,30 +9,38 @@ #include #include +#include +#include namespace AZ { namespace RPI { //! Use a MaterialAssetCreator to create and configure a new MaterialAsset. - //! The MaterialAsset will be based on a MaterialTypeAsset or another MaterialAsset. - //! Either way, the base provides the necessary data to define the layout - //! and behavior of the material. The MaterialAsset only provides property value overrides. - //! Note however that the MaterialTypeAsset does not have to be loaded and available yet; - //! only the AssetId is required. The resulting MaterialAsset will be in a non-finalized state; - //! it must be finalized afterwards when the MaterialTypeAsset is available before it can be used. + //! + //! There are two options for how to create the MaterialAsset, whether it should be finalized now or deferred. + //! - Finalized now: This requires the MaterialTypeAsset to be fully populated so it can read the property layout. + //! - Deferred finalize: This only requires the MaterialTypeAsset to have a valid AssetId; the data inside will not be used. MaterialAsset::Finalize() + //! will need to be called later when the final MaterialTypeAsset is available, presumably after loading the MaterialAsset at runtime. class MaterialAssetCreator : public AssetCreator { public: friend class MaterialSourceData; - - void Begin(const Data::AssetId& assetId, const Data::Asset& materialType); + + void Begin(const Data::AssetId& assetId, const Data::Asset& materialType, bool shouldFinalize); bool End(Data::Asset& result); void SetMaterialTypeVersion(uint32_t version); void SetPropertyValue(const Name& name, const MaterialPropertyValue& value); + + void SetPropertyValue(const Name& name, const Data::Asset& imageAsset); + void SetPropertyValue(const Name& name, const Data::Asset& imageAsset); + void SetPropertyValue(const Name& name, const Data::Asset& imageAsset); + + private: + bool m_shouldFinalize = false; }; } // namespace RPI } // namespace AZ diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialAssetCreatorCommon.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialAssetCreatorCommon.h index 312739a0f0..c7fa9c2a4c 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialAssetCreatorCommon.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialAssetCreatorCommon.h @@ -52,12 +52,6 @@ namespace AZ private: bool PropertyCheck(TypeId typeId, const Name& name); - //! Returns the MaterialPropertyDataType value that corresponds to typeId - MaterialPropertyDataType GetMaterialPropertyDataType(TypeId typeId) const; - - //! Checks that the TypeId typeId matches the type expected by materialPropertyDescriptor - bool ValidateDataType(TypeId typeId, const Name& propertyName, const MaterialPropertyDescriptor* materialPropertyDescriptor); - const MaterialPropertiesLayout* m_propertyLayout = nullptr; //! Points to the m_propertyValues list in a MaterialAsset or MaterialTypeAsset AZStd::vector* m_propertyValues = nullptr; diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialPropertyDescriptor.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialPropertyDescriptor.h index 76bb2a6113..bd18c98780 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialPropertyDescriptor.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialPropertyDescriptor.h @@ -17,6 +17,8 @@ namespace AZ { namespace RPI { + class MaterialPropertyDescriptor; + struct MaterialPropertyIndexType { AZ_TYPE_INFO(MaterialPropertyIndexType, "{cfc09268-f3f1-4474-bd8f-f2c8de27c5f1}"); }; @@ -76,6 +78,9 @@ namespace AZ const char* ToString(MaterialPropertyDataType materialPropertyDataType); AZStd::string GetMaterialPropertyDataTypeString(AZ::TypeId typeId); + + //! Checks that the TypeId matches the type expected by materialPropertyDescriptor + bool ValidateMaterialPropertyDataType(TypeId typeId, const Name& propertyName, const MaterialPropertyDescriptor* materialPropertyDescriptor, AZStd::function onError); //! A material property is any data input to a material, like a bool, float, Vector, Image, Buffer, etc. //! This descriptor defines a single input property, including it's name ID, and how it maps diff --git a/Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.cpp b/Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.cpp index 1b5635a1c1..3aa8728e08 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.cpp @@ -52,7 +52,7 @@ namespace AZ { AssetBuilderSDK::AssetBuilderDesc materialBuilderDescriptor; materialBuilderDescriptor.m_name = JobKey; - materialBuilderDescriptor.m_version = 111; // material dependency improvements + materialBuilderDescriptor.m_version = 112; // material dependency improvements materialBuilderDescriptor.m_patterns.push_back(AssetBuilderSDK::AssetBuilderPattern("*.material", AssetBuilderSDK::AssetBuilderPattern::PatternType::Wildcard)); materialBuilderDescriptor.m_patterns.push_back(AssetBuilderSDK::AssetBuilderPattern("*.materialtype", AssetBuilderSDK::AssetBuilderPattern::PatternType::Wildcard)); materialBuilderDescriptor.m_busId = azrtti_typeid(); diff --git a/Gems/Atom/RPI/Code/Source/RPI.Builders/Model/MaterialAssetBuilderComponent.cpp b/Gems/Atom/RPI/Code/Source/RPI.Builders/Model/MaterialAssetBuilderComponent.cpp index ef251b6848..6f71fb70d4 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Builders/Model/MaterialAssetBuilderComponent.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Builders/Model/MaterialAssetBuilderComponent.cpp @@ -127,7 +127,7 @@ namespace AZ if (auto* serialize = azrtti_cast(context)) { serialize->Class() - ->Version(17); // Optional material conversion + ->Version(18); // material dependency improvements } } 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 cd01544bfa..23657f4871 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceData.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceData.cpp @@ -114,7 +114,7 @@ namespace AZ } } - materialAssetCreator.Begin(assetId, materialTypeAsset); + materialAssetCreator.Begin(assetId, materialTypeAsset, processingMode == MaterialAssetProcessingMode::PreBake); if (!m_parentMaterial.empty()) { @@ -180,11 +180,6 @@ namespace AZ Data::Asset material; if (materialAssetCreator.End(material)) { - if (processingMode == MaterialAssetProcessingMode::PreBake) - { - material->Finalize(); - } - return Success(material); } else @@ -270,11 +265,18 @@ namespace AZ parentSourceAbsPath = AssetUtils::ResolvePathReference(parentSourceAbsPath, parentSourceRelPath); parentSourceDataStack.emplace_back(AZStd::move(parentSourceData)); } + + // Unlike CreateMaterialAsset(), we can always finalize the material here because we loaded created the MaterialTypeAsset from + // the source .materialtype file, so the necessary data is always available. + // (In case you are wondering why we don't use CreateMaterialAssetFromSourceData in MaterialBuilder: that would require a + // source dependency between the .materialtype and .material file, which would cause all .material files to rebuild when you + // edit the .materialtype; it's faster to not read the material type data at all ... until it's needed at runtime) + const bool finalize = true; // Create the material asset from all the previously loaded source data MaterialAssetCreator materialAssetCreator; materialAssetCreator.SetElevateWarnings(elevateWarnings); - materialAssetCreator.Begin(assetId, materialTypeAsset.GetValue()); + materialAssetCreator.Begin(assetId, materialTypeAsset.GetValue(), finalize); while (!parentSourceDataStack.empty()) { @@ -287,13 +289,6 @@ namespace AZ Data::Asset material; if (materialAssetCreator.End(material)) { - // Unlike CreateMaterialAsset(), we can always finalize the material here because we loaded created the MaterialTypeAsset from - // the source .materialtype file, so the necessary data is always available. - // (In case you are wondering why we don't use CreateMaterialAssetFromSourceData in MaterialBuilder: that would require a - // source dependency between the .materialtype and .material file, which would cause all .material files to rebuild when you - // edit the .materialtype; it's faster to not read the material type data at all ... until it's needed at runtime) - material->Finalize(); - if (sourceDependencies) { sourceDependencies->insert(dependencies.begin(), dependencies.end()); 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 475c6ca219..2fe30f632f 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialUtils.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialUtils.cpp @@ -150,6 +150,7 @@ namespace AZ return shouldFinalize; } + } } } diff --git a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAsset.cpp b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAsset.cpp index 7962844736..ce5847d12f 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAsset.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAsset.cpp @@ -114,13 +114,29 @@ namespace AZ return m_isFinalized; } - void MaterialAsset::Finalize() + void MaterialAsset::Finalize(AZStd::function reportWarning, AZStd::function reportError) { if (IsFinalized()) { return; } + if (!reportWarning) + { + reportWarning = [](const char* message) + { + AZ_Warning(s_debugTraceName, false, "%s", message); + }; + } + + if (!reportError) + { + reportError = [](const char* message) + { + AZ_Error(s_debugTraceName, false, "%s", message); + }; + } + const uint32_t materialTypeVersion = m_materialTypeAsset->GetVersion(); if (m_materialTypeVersion < materialTypeVersion) { @@ -147,7 +163,7 @@ namespace AZ uint32_t enumValue = propertyDescriptor->GetEnumValue(enumName); if (enumValue == MaterialPropertyDescriptor::InvalidEnumValue) { - AZ_Error(s_debugTraceName, false, "Material property name \"%s\" has invalid enum value \"%s\".", name.GetCStr(), enumName.GetCStr()); + reportWarning(AZStd::string::format("Material property name \"%s\" has invalid enum value \"%s\".", name.GetCStr(), enumName.GetCStr()).c_str()); } else { @@ -164,12 +180,15 @@ namespace AZ } else { - finalizedPropertyValues[propertyIndex.GetIndex()] = value; + if (ValidateMaterialPropertyDataType(value.GetTypeId(), name, propertyDescriptor, reportError)) + { + finalizedPropertyValues[propertyIndex.GetIndex()] = value; + } } } else { - AZ_Warning(s_debugTraceName, false, "Material property name \"%s\" is not found in the material properties layout and will not be used.", name.GetCStr()); + reportWarning(AZStd::string::format("Material property name \"%s\" is not found in the material properties layout and will not be used.", name.GetCStr()).c_str()); } } diff --git a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAssetCreator.cpp b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAssetCreator.cpp index 4bee663791..f05fb8d848 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAssetCreator.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAssetCreator.cpp @@ -16,16 +16,21 @@ namespace AZ { namespace RPI { - void MaterialAssetCreator::Begin(const Data::AssetId& assetId, const Data::Asset& materialType) + void MaterialAssetCreator::Begin(const Data::AssetId& assetId, const Data::Asset& materialType, bool shouldFinalize) { BeginCommon(assetId); if (ValidateIsReady()) { - m_asset->m_materialTypeAsset = materialType; + m_shouldFinalize = shouldFinalize; + m_asset->m_materialTypeAsset = materialType; m_asset->m_materialTypeAsset.SetAutoLoadBehavior(AZ::Data::AssetLoadBehavior::PreLoad); - + + if (shouldFinalize && !m_asset->m_materialTypeAsset) + { + ReportError("MaterialTypeAsset is null, the MaterialAsset cannot be finalized"); + } } } @@ -37,6 +42,14 @@ namespace AZ } m_asset->SetReady(); + + if (m_shouldFinalize) + { + m_asset->Finalize( + [this](const char* message) { ReportWarning("%s", message); }, + [this](const char* message) { ReportError("%s", message); }); + } + return EndCommon(result); } @@ -71,6 +84,21 @@ namespace AZ m_asset->m_rawPropertyValues.emplace_back(name, value); } } + + void MaterialAssetCreator::SetPropertyValue(const Name& name, const Data::Asset& imageAsset) + { + SetPropertyValue(name, MaterialPropertyValue{imageAsset}); + } + + void MaterialAssetCreator::SetPropertyValue(const Name& name, const Data::Asset& imageAsset) + { + SetPropertyValue(name, Data::Asset(imageAsset)); + } + + void MaterialAssetCreator::SetPropertyValue(const Name& name, const Data::Asset& imageAsset) + { + SetPropertyValue(name, Data::Asset(imageAsset)); + } } // namespace RPI } // namespace AZ diff --git a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAssetCreatorCommon.cpp b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAssetCreatorCommon.cpp index 1ba74ce0b9..4b2f163f7c 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAssetCreatorCommon.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAssetCreatorCommon.cpp @@ -7,6 +7,7 @@ */ #include +#include namespace AZ { @@ -32,57 +33,6 @@ namespace AZ m_reportError = nullptr; } - MaterialPropertyDataType MaterialAssetCreatorCommon::GetMaterialPropertyDataType(TypeId typeId) const - { - if (typeId == azrtti_typeid()) { return MaterialPropertyDataType::Bool; } - if (typeId == azrtti_typeid()) { return MaterialPropertyDataType::Int; } - if (typeId == azrtti_typeid()) { return MaterialPropertyDataType::UInt; } - if (typeId == azrtti_typeid()) { return MaterialPropertyDataType::Float; } - if (typeId == azrtti_typeid()) { return MaterialPropertyDataType::Vector2; } - if (typeId == azrtti_typeid()) { return MaterialPropertyDataType::Vector3; } - if (typeId == azrtti_typeid()) { return MaterialPropertyDataType::Vector4; } - if (typeId == azrtti_typeid()) { return MaterialPropertyDataType::Color; } - if (typeId == azrtti_typeid>()) { return MaterialPropertyDataType::Image; } - else - { - return MaterialPropertyDataType::Invalid; - } - } - - bool MaterialAssetCreatorCommon::ValidateDataType(TypeId typeId, const Name& propertyName, const MaterialPropertyDescriptor* materialPropertyDescriptor) - { - auto expectedDataType = materialPropertyDescriptor->GetDataType(); - auto actualDataType = GetMaterialPropertyDataType(typeId); - - if (expectedDataType == MaterialPropertyDataType::Enum) - { - if (actualDataType != MaterialPropertyDataType::UInt) - { - m_reportError( - AZStd::string::format("Material property '%s' is a Enum type, can only accept UInt value, input value is %s", - propertyName.GetCStr(), - ToString(actualDataType) - ).data()); - return false; - } - } - else - { - if (expectedDataType != actualDataType) - { - m_reportError( - AZStd::string::format("Material property '%s': Type mismatch. Expected %s but was %s", - propertyName.GetCStr(), - ToString(expectedDataType), - ToString(actualDataType) - ).data()); - return false; - } - } - - return true; - } - bool MaterialAssetCreatorCommon::PropertyCheck(TypeId typeId, const Name& name) { if (!m_reportWarning || !m_reportError) @@ -108,7 +58,7 @@ namespace AZ return false; } - if (!ValidateDataType(typeId, name, materialPropertyDescriptor)) + if (!ValidateMaterialPropertyDataType(typeId, name, materialPropertyDescriptor, m_reportError)) { return false; } diff --git a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialPropertyDescriptor.cpp b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialPropertyDescriptor.cpp index 5d0a88a6d3..ddbb902761 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialPropertyDescriptor.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialPropertyDescriptor.cpp @@ -97,6 +97,57 @@ namespace AZ return AZStd::string::format("", typeId.ToString().c_str()); } } + + bool ValidateMaterialPropertyDataType(TypeId typeId, const Name& propertyName, const MaterialPropertyDescriptor* materialPropertyDescriptor, AZStd::function onError) + { + auto toMaterialPropertyDataType = [](TypeId typeId) + { + if (typeId == azrtti_typeid()) { return MaterialPropertyDataType::Bool; } + if (typeId == azrtti_typeid()) { return MaterialPropertyDataType::Int; } + if (typeId == azrtti_typeid()) { return MaterialPropertyDataType::UInt; } + if (typeId == azrtti_typeid()) { return MaterialPropertyDataType::Float; } + if (typeId == azrtti_typeid()) { return MaterialPropertyDataType::Vector2; } + if (typeId == azrtti_typeid()) { return MaterialPropertyDataType::Vector3; } + if (typeId == azrtti_typeid()) { return MaterialPropertyDataType::Vector4; } + if (typeId == azrtti_typeid()) { return MaterialPropertyDataType::Color; } + if (typeId == azrtti_typeid>()) { return MaterialPropertyDataType::Image; } + else + { + return MaterialPropertyDataType::Invalid; + } + }; + + auto expectedDataType = materialPropertyDescriptor->GetDataType(); + auto actualDataType = toMaterialPropertyDataType(typeId); + + if (expectedDataType == MaterialPropertyDataType::Enum) + { + if (actualDataType != MaterialPropertyDataType::UInt) + { + onError( + AZStd::string::format("Material property '%s' is a Enum type, can only accept UInt value, input value is %s", + propertyName.GetCStr(), + ToString(actualDataType) + ).data()); + return false; + } + } + else + { + if (expectedDataType != actualDataType) + { + onError( + AZStd::string::format("Material property '%s': Type mismatch. Expected %s but was %s", + propertyName.GetCStr(), + ToString(expectedDataType), + ToString(actualDataType) + ).data()); + return false; + } + } + + return true; + } void MaterialPropertyOutputId::Reflect(ReflectContext* context) { diff --git a/Gems/Atom/RPI/Code/Tests/Material/LuaMaterialFunctorTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/LuaMaterialFunctorTests.cpp index 2608ac3a9b..5016f8303e 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/LuaMaterialFunctorTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/LuaMaterialFunctorTests.cpp @@ -112,7 +112,7 @@ namespace UnitTest Data::Asset materialAsset; MaterialAssetCreator materialCreator; - materialCreator.Begin(Uuid::CreateRandom(), *m_materialTypeAsset); + materialCreator.Begin(Uuid::CreateRandom(), m_materialTypeAsset, true); EXPECT_TRUE(materialCreator.End(materialAsset)); m_material = Material::Create(materialAsset); @@ -138,7 +138,7 @@ namespace UnitTest Data::Asset materialAsset; MaterialAssetCreator materialCreator; - materialCreator.Begin(Uuid::CreateRandom(), *m_materialTypeAsset); + materialCreator.Begin(Uuid::CreateRandom(),m_materialTypeAsset, true); EXPECT_TRUE(materialCreator.End(materialAsset)); m_material = Material::Create(materialAsset); @@ -165,7 +165,7 @@ namespace UnitTest Data::Asset materialAsset; MaterialAssetCreator materialCreator; - materialCreator.Begin(Uuid::CreateRandom(), *m_materialTypeAsset); + materialCreator.Begin(Uuid::CreateRandom(), m_materialTypeAsset, true); EXPECT_TRUE(materialCreator.End(materialAsset)); m_material = Material::Create(materialAsset); @@ -194,7 +194,7 @@ namespace UnitTest Data::Asset materialAsset; MaterialAssetCreator materialCreator; - materialCreator.Begin(Uuid::CreateRandom(), *m_materialTypeAsset); + materialCreator.Begin(Uuid::CreateRandom(), m_materialTypeAsset, true); EXPECT_TRUE(materialCreator.End(materialAsset)); m_material = Material::Create(materialAsset); diff --git a/Gems/Atom/RPI/Code/Tests/Material/MaterialAssetTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/MaterialAssetTests.cpp index 58a852f176..633953cecc 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/MaterialAssetTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/MaterialAssetTests.cpp @@ -94,7 +94,7 @@ namespace UnitTest Data::AssetId assetId(Uuid::CreateRandom()); MaterialAssetCreator creator; - creator.Begin(assetId, *m_testMaterialTypeAsset); + creator.Begin(assetId, m_testMaterialTypeAsset, true); creator.SetPropertyValue(Name{ "MyFloat2" }, Vector2{ 0.1f, 0.2f }); creator.SetPropertyValue(Name{ "MyFloat3" }, Vector3{ 1.1f, 1.2f, 1.3f }); creator.SetPropertyValue(Name{ "MyFloat4" }, Vector4{ 2.1f, 2.2f, 2.3f, 2.4f }); @@ -129,7 +129,7 @@ namespace UnitTest Data::AssetId assetId(Uuid::CreateRandom()); MaterialAssetCreator creator; - creator.Begin(assetId, *m_testMaterialTypeAsset); + creator.Begin(assetId, m_testMaterialTypeAsset, true); creator.SetPropertyValue(Name{ "MyFloat" }, 3.14f); Data::Asset materialAsset; @@ -171,7 +171,7 @@ namespace UnitTest Data::Asset materialAsset; MaterialAssetCreator materialCreator; - materialCreator.Begin(Uuid::CreateRandom(), *emptyMaterialTypeAsset); + materialCreator.Begin(Uuid::CreateRandom(), emptyMaterialTypeAsset, true); EXPECT_TRUE(materialCreator.End(materialAsset)); EXPECT_EQ(emptyMaterialTypeAsset, materialAsset->GetMaterialTypeAsset()); EXPECT_EQ(materialAsset->GetPropertyValues().size(), 0); @@ -189,7 +189,7 @@ namespace UnitTest Data::AssetId assetId(Uuid::CreateRandom()); MaterialAssetCreator creator; - creator.Begin(assetId, *m_testMaterialTypeAsset); + creator.Begin(assetId, m_testMaterialTypeAsset, true); creator.SetPropertyValue(Name{ "MyImage" }, streamingImageAsset); Data::Asset materialAsset; @@ -231,8 +231,8 @@ namespace UnitTest Data::AssetId assetId(Uuid::CreateRandom()); MaterialAssetCreator creator; - const bool includePropertyNames = true; - creator.Begin(assetId, *testMaterialTypeAssetV1, includePropertyNames); + const bool shouldFinalize = false; + creator.Begin(assetId, testMaterialTypeAssetV1, shouldFinalize); creator.SetPropertyValue(Name{ "MyInt" }, 7); creator.SetPropertyValue(Name{ "MyUInt" }, 8u); creator.SetPropertyValue(Name{ "MyFloat" }, 9.0f); @@ -307,26 +307,68 @@ namespace UnitTest // We use local functions to easily start a new MaterialAssetCreator for each test case because // the AssetCreator would just skip subsequent operations after the first failure is detected. - auto expectCreatorError = [this](AZStd::function passBadInput) + auto expectCreatorError = [this](const char* expectedErrorMessage, AZStd::function passBadInput) { - MaterialAssetCreator creator; - creator.Begin(Uuid::CreateRandom(), *m_testMaterialTypeAsset); + // Test with finalizing enabled + { + MaterialAssetCreator creator; + creator.Begin(Uuid::CreateRandom(), m_testMaterialTypeAsset, true); + + ErrorMessageFinder errorMessageFinder; + errorMessageFinder.AddExpectedErrorMessage(expectedErrorMessage); + errorMessageFinder.AddIgnoredErrorMessage("Failed to build", true); + + passBadInput(creator); - AZ_TEST_START_ASSERTTEST; - passBadInput(creator); - AZ_TEST_STOP_ASSERTTEST(1); + Data::Asset materialAsset; + EXPECT_FALSE(creator.End(materialAsset)); + + errorMessageFinder.CheckExpectedErrorsFound(); + + EXPECT_TRUE(creator.GetErrorCount() > 0); + } + + // Test with finalizing disabled, so no validation occurs because the MaterialTypeAsset data is not used. + { + MaterialAssetCreator creator; + creator.Begin(Uuid::CreateRandom(), m_testMaterialTypeAsset, false); - EXPECT_EQ(1, creator.GetErrorCount()); + passBadInput(creator); + + Data::Asset materialAsset; + EXPECT_TRUE(creator.End(materialAsset)); + + EXPECT_EQ(creator.GetErrorCount(), 0); + } }; auto expectCreatorWarning = [this](AZStd::function passBadInput) { - MaterialAssetCreator creator; - creator.Begin(Uuid::CreateRandom(), *m_testMaterialTypeAsset); + // Test with finalizing enabled + { + MaterialAssetCreator creator; + creator.Begin(Uuid::CreateRandom(), m_testMaterialTypeAsset, true); + + passBadInput(creator); + + Data::Asset material; + creator.End(material); - passBadInput(creator); + EXPECT_EQ(1, creator.GetWarningCount()); + } + + // Test with finalizing disabled, so no validation occurs because the MaterialTypeAsset data is not used. + { + MaterialAssetCreator creator; + creator.Begin(Uuid::CreateRandom(), m_testMaterialTypeAsset, false); + + passBadInput(creator); + + Data::Asset material; + creator.End(material); - EXPECT_EQ(1, creator.GetWarningCount()); + EXPECT_EQ(0, creator.GetWarningCount()); + } }; // Invalid input ID @@ -343,55 +385,65 @@ namespace UnitTest // Test data type mismatches... - expectCreatorError([this](MaterialAssetCreator& creator) - { - creator.SetPropertyValue(Name{ "MyBool" }, m_testImageAsset); - }); + expectCreatorError("Type mismatch", + [this](MaterialAssetCreator& creator) + { + creator.SetPropertyValue(Name{ "MyBool" }, m_testImageAsset); + }); - expectCreatorError([](MaterialAssetCreator& creator) - { - creator.SetPropertyValue(Name{ "MyInt" }, 0.0f); - }); + expectCreatorError("Type mismatch", + [](MaterialAssetCreator& creator) + { + creator.SetPropertyValue(Name{ "MyInt" }, 0.0f); + }); - expectCreatorError([](MaterialAssetCreator& creator) - { - creator.SetPropertyValue(Name{ "MyUInt" }, -1); - }); + expectCreatorError("Type mismatch", + [](MaterialAssetCreator& creator) + { + creator.SetPropertyValue(Name{ "MyUInt" }, -1); + }); - expectCreatorError([](MaterialAssetCreator& creator) - { - creator.SetPropertyValue(Name{ "MyFloat" }, 10u); - }); + expectCreatorError("Type mismatch", + [](MaterialAssetCreator& creator) + { + creator.SetPropertyValue(Name{ "MyFloat" }, 10u); + }); - expectCreatorError([](MaterialAssetCreator& creator) - { - creator.SetPropertyValue(Name{ "MyFloat2" }, 1.0f); - }); + expectCreatorError("Type mismatch", + [](MaterialAssetCreator& creator) + { + creator.SetPropertyValue(Name{ "MyFloat2" }, 1.0f); + }); - expectCreatorError([](MaterialAssetCreator& creator) - { - creator.SetPropertyValue(Name{ "MyFloat3" }, AZ::Vector4{}); - }); + expectCreatorError("Type mismatch", + [](MaterialAssetCreator& creator) + { + creator.SetPropertyValue(Name{ "MyFloat3" }, AZ::Vector4{}); + }); - expectCreatorError([](MaterialAssetCreator& creator) - { - creator.SetPropertyValue(Name{ "MyFloat4" }, AZ::Vector3{}); - }); + expectCreatorError("Type mismatch", + [](MaterialAssetCreator& creator) + { + creator.SetPropertyValue(Name{ "MyFloat4" }, AZ::Vector3{}); + }); - expectCreatorError([](MaterialAssetCreator& creator) - { - creator.SetPropertyValue(Name{ "MyColor" }, MaterialPropertyValue(false)); - }); + expectCreatorError("Type mismatch", + [](MaterialAssetCreator& creator) + { + creator.SetPropertyValue(Name{ "MyColor" }, MaterialPropertyValue(false)); + }); - expectCreatorError([](MaterialAssetCreator& creator) - { - creator.SetPropertyValue(Name{ "MyImage" }, true); - }); + expectCreatorError("Type mismatch", + [](MaterialAssetCreator& creator) + { + creator.SetPropertyValue(Name{ "MyImage" }, true); + }); - expectCreatorError([](MaterialAssetCreator& creator) - { - creator.SetPropertyValue(Name{ "MyEnum" }, -1); - }); + expectCreatorError("can only accept UInt value", + [](MaterialAssetCreator& creator) + { + creator.SetPropertyValue(Name{ "MyEnum" }, -1); + }); } } diff --git a/Gems/Atom/RPI/Code/Tests/Material/MaterialFunctorTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/MaterialFunctorTests.cpp index ff5d24ff9a..3373646c6e 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/MaterialFunctorTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/MaterialFunctorTests.cpp @@ -275,7 +275,7 @@ namespace UnitTest materialTypeCreator.End(m_testMaterialTypeAsset); MaterialAssetCreator materialCreator; - materialCreator.Begin(Uuid::CreateRandom(), *m_testMaterialTypeAsset); + materialCreator.Begin(Uuid::CreateRandom(), m_testMaterialTypeAsset, true); materialCreator.SetPropertyValue(registedPropertyName, 42); materialCreator.SetPropertyValue(unregistedPropertyName, 42); materialCreator.SetPropertyValue(unrelatedPropertyName, 42); diff --git a/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp index d4bf3e5eaa..73aeacf818 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp @@ -175,7 +175,7 @@ namespace UnitTest AddProperty(sourceData, "general", "MyImage", AZStd::string("@exefolder@/Temp/test.streamingimage")); AddProperty(sourceData, "general", "MyEnum", AZStd::string("Enum1")); - auto materialAssetOutcome = sourceData.CreateMaterialAsset(Uuid::CreateRandom(), "", true); + auto materialAssetOutcome = sourceData.CreateMaterialAsset(Uuid::CreateRandom(), "", MaterialAssetProcessingMode::PreBake, true); EXPECT_TRUE(materialAssetOutcome.IsSuccess()); Data::Asset materialAsset = materialAssetOutcome.GetValue(); @@ -544,17 +544,17 @@ namespace UnitTest AddPropertyGroup(sourceDataLevel3, "general"); AddProperty(sourceDataLevel3, "general", "MyFloat", 3.5f); - auto materialAssetLevel1 = sourceDataLevel1.CreateMaterialAsset(Uuid::CreateRandom(), "", true); + auto materialAssetLevel1 = sourceDataLevel1.CreateMaterialAsset(Uuid::CreateRandom(), "", MaterialAssetProcessingMode::PreBake, true); EXPECT_TRUE(materialAssetLevel1.IsSuccess()); m_assetSystemStub.RegisterSourceInfo("level1.material", materialAssetLevel1.GetValue().GetId()); - auto materialAssetLevel2 = sourceDataLevel2.CreateMaterialAsset(Uuid::CreateRandom(), "", true); + auto materialAssetLevel2 = sourceDataLevel2.CreateMaterialAsset(Uuid::CreateRandom(), "", MaterialAssetProcessingMode::PreBake, true); EXPECT_TRUE(materialAssetLevel2.IsSuccess()); m_assetSystemStub.RegisterSourceInfo("level2.material", materialAssetLevel2.GetValue().GetId()); - auto materialAssetLevel3 = sourceDataLevel3.CreateMaterialAsset(Uuid::CreateRandom(), "", true); + auto materialAssetLevel3 = sourceDataLevel3.CreateMaterialAsset(Uuid::CreateRandom(), "", MaterialAssetProcessingMode::PreBake, true); EXPECT_TRUE(materialAssetLevel3.IsSuccess()); auto layout = m_testMaterialTypeAsset->GetMaterialPropertiesLayout(); @@ -604,18 +604,18 @@ namespace UnitTest sourceDataLevel3.m_materialType = "@exefolder@/Temp/otherBase.materialtype"; sourceDataLevel3.m_parentMaterial = "level2.material"; - auto materialAssetLevel1 = sourceDataLevel1.CreateMaterialAsset(Uuid::CreateRandom(), "", true); + auto materialAssetLevel1 = sourceDataLevel1.CreateMaterialAsset(Uuid::CreateRandom(), "", MaterialAssetProcessingMode::PreBake, true); EXPECT_TRUE(materialAssetLevel1.IsSuccess()); m_assetSystemStub.RegisterSourceInfo("level1.material", materialAssetLevel1.GetValue().GetId()); - auto materialAssetLevel2 = sourceDataLevel2.CreateMaterialAsset(Uuid::CreateRandom(), "", true); + auto materialAssetLevel2 = sourceDataLevel2.CreateMaterialAsset(Uuid::CreateRandom(), "", MaterialAssetProcessingMode::PreBake, true); EXPECT_TRUE(materialAssetLevel2.IsSuccess()); m_assetSystemStub.RegisterSourceInfo("level2.material", materialAssetLevel2.GetValue().GetId()); AZ_TEST_START_ASSERTTEST; - auto materialAssetLevel3 = sourceDataLevel3.CreateMaterialAsset(Uuid::CreateRandom(), "", true); + auto materialAssetLevel3 = sourceDataLevel3.CreateMaterialAsset(Uuid::CreateRandom(), "", MaterialAssetProcessingMode::PreBake, true); AZ_TEST_STOP_ASSERTTEST(1); EXPECT_FALSE(materialAssetLevel3.IsSuccess()); } @@ -625,7 +625,7 @@ namespace UnitTest // We use local functions to easily start a new MaterialAssetCreator for each test case because // the AssetCreator would just skip subsequent operations after the first failure is detected. - auto expectWarning = [](AZStd::function setOneBadInput, [[maybe_unused]] uint32_t expectedAsserts = 1) + auto expectWarning = [](const char* expectedErrorMessage, AZStd::function setOneBadInput, bool warningOccursBeforeFinalize = false) { MaterialSourceData sourceData; @@ -635,233 +635,69 @@ namespace UnitTest setOneBadInput(sourceData); - AZ_TEST_START_ASSERTTEST; - auto materialAssetOutcome = sourceData.CreateMaterialAsset(Uuid::CreateRandom(), "", true); - AZ_TEST_STOP_ASSERTTEST(expectedAsserts); // Usually just one for when End() is called + // Check with MaterialAssetProcessingMode::PreBake + { + ErrorMessageFinder errorFinder; + errorFinder.AddExpectedErrorMessage(expectedErrorMessage); + errorFinder.AddIgnoredErrorMessage("Failed to build", true); + auto materialAssetOutcome = sourceData.CreateMaterialAsset(Uuid::CreateRandom(), "", MaterialAssetProcessingMode::PreBake, true); + errorFinder.CheckExpectedErrorsFound(); - EXPECT_FALSE(materialAssetOutcome.IsSuccess()); + EXPECT_FALSE(materialAssetOutcome.IsSuccess()); + } + + // Check with MaterialAssetProcessingMode::DeferredBake, no validation occurs because the MaterialTypeAsset cannot be used and so the MaterialAsset is not finalized + if(!warningOccursBeforeFinalize) + { + auto materialAssetOutcome = sourceData.CreateMaterialAsset(Uuid::CreateRandom(), "", MaterialAssetProcessingMode::DeferredBake, true); + EXPECT_TRUE(materialAssetOutcome.IsSuccess()); + } }; // Test property does not exist... - expectWarning([](MaterialSourceData& materialSourceData) - { - AddProperty(materialSourceData, "general", "DoesNotExist", true); - }); + expectWarning("\"general.DoesNotExist\" is not found in the material properties layout", + [](MaterialSourceData& materialSourceData) + { + AddProperty(materialSourceData, "general", "DoesNotExist", true); + }); - expectWarning([](MaterialSourceData& materialSourceData) - { - AddProperty(materialSourceData, "general", "DoesNotExist", -10); - }); + expectWarning("\"general.DoesNotExist\" is not found in the material properties layout", + [](MaterialSourceData& materialSourceData) + { + AddProperty(materialSourceData, "general", "DoesNotExist", -10); + }); - expectWarning([](MaterialSourceData& materialSourceData) - { - AddProperty(materialSourceData, "general", "DoesNotExist", 25u); - }); + expectWarning("\"general.DoesNotExist\" is not found in the material properties layout", + [](MaterialSourceData& materialSourceData) + { + AddProperty(materialSourceData, "general", "DoesNotExist", 25u); + }); - expectWarning([](MaterialSourceData& materialSourceData) - { - AddProperty(materialSourceData, "general", "DoesNotExist", 1.5f); - }); + expectWarning("\"general.DoesNotExist\" is not found in the material properties layout", + [](MaterialSourceData& materialSourceData) + { + AddProperty(materialSourceData, "general", "DoesNotExist", 1.5f); + }); - expectWarning([](MaterialSourceData& materialSourceData) - { - AddProperty(materialSourceData, "general", "DoesNotExist", AZ::Color{ 0.1f, 0.2f, 0.3f, 0.4f }); - }); + expectWarning("\"general.DoesNotExist\" is not found in the material properties layout", + [](MaterialSourceData& materialSourceData) + { + AddProperty(materialSourceData, "general", "DoesNotExist", AZ::Color{ 0.1f, 0.2f, 0.3f, 0.4f }); + }); - expectWarning([](MaterialSourceData& materialSourceData) - { - AddProperty(materialSourceData, "general", "DoesNotExist", AZStd::string("@exefolder@/Temp/test.streamingimage")); - }); + expectWarning("\"general.DoesNotExist\" is not found in the material properties layout", + [](MaterialSourceData& materialSourceData) + { + AddProperty(materialSourceData, "general", "DoesNotExist", AZStd::string("@exefolder@/Temp/test.streamingimage")); + }); // Missing image reference - expectWarning([](MaterialSourceData& materialSourceData) - { - AddProperty(materialSourceData, "general", "MyImage", AZStd::string("doesNotExist.streamingimage")); - }); - } - - - TEST_F(MaterialSourceDataTests, Load_MaterialTypeVersionUpdate) - { - const AZStd::string inputJson = R"( - { - "materialType": "@exefolder@/Temp/test.materialtype", - "materialTypeVersion": 1, - "properties": { - "general": { - "testColorNameA": [0.1, 0.2, 0.3] - } - } - } - )"; - - MaterialSourceData material; - JsonTestResult loadResult = LoadTestDataFromJson(material, inputJson); - - EXPECT_EQ(AZ::JsonSerializationResult::Tasks::ReadField, loadResult.m_jsonResultCode.GetTask()); - EXPECT_EQ(AZ::JsonSerializationResult::Processing::Completed, loadResult.m_jsonResultCode.GetProcessing()); - - // Initially, the loaded material data will match the .material file exactly. This gives us the accurate representation of - // what's actually saved on disk. - - EXPECT_NE(material.m_properties["general"].find("testColorNameA"), material.m_properties["general"].end()); - EXPECT_EQ(material.m_properties["general"].find("testColorNameB"), material.m_properties["general"].end()); - EXPECT_EQ(material.m_properties["general"].find("testColorNameC"), material.m_properties["general"].end()); - EXPECT_EQ(material.m_properties["general"].find("MyColor"), material.m_properties["general"].end()); - - AZ::Color testColor = material.m_properties["general"]["testColorNameA"].m_value.GetValue(); - EXPECT_TRUE(AZ::Color(0.1f, 0.2f, 0.3f, 1.0f).IsClose(testColor, 0.01)); - - EXPECT_EQ(1, material.m_materialTypeVersion); - - // Then we force the material data to update to the latest material type version specification - ErrorMessageFinder warningFinder; // Note this finds errors and warnings, and we're looking for a warning. - warningFinder.AddExpectedErrorMessage("Automatic updates are available. Consider updating the .material source file"); - warningFinder.AddExpectedErrorMessage("This material is based on version '1'"); - warningFinder.AddExpectedErrorMessage("material type is now at version '10'"); - material.ApplyVersionUpdates(); - warningFinder.CheckExpectedErrorsFound(); - - // Now the material data should match the latest material type. - // Look for the property under the latest name in the material type, not the name used in the .material file. - - EXPECT_EQ(material.m_properties["general"].find("testColorNameA"), material.m_properties["general"].end()); - EXPECT_EQ(material.m_properties["general"].find("testColorNameB"), material.m_properties["general"].end()); - EXPECT_EQ(material.m_properties["general"].find("testColorNameC"), material.m_properties["general"].end()); - EXPECT_NE(material.m_properties["general"].find("MyColor"), material.m_properties["general"].end()); - - testColor = material.m_properties["general"]["MyColor"].m_value.GetValue(); - EXPECT_TRUE(AZ::Color(0.1f, 0.2f, 0.3f, 1.0f).IsClose(testColor, 0.01)); - - EXPECT_EQ(10, material.m_materialTypeVersion); - - // Calling ApplyVersionUpdates() again should not report the warning again, since the material has already been updated. - warningFinder.Reset(); - material.ApplyVersionUpdates(); - } - - TEST_F(MaterialSourceDataTests, Load_MaterialTypeVersionUpdate_MovePropertiesToAnotherGroup) - { - const AZStd::string inputJson = R"( - { - "materialType": "@exefolder@/Temp/test.materialtype", - "materialTypeVersion": 3, - "properties": { - "oldGroup": { - "MyFloat": 1.2, - "MyIntOldName": 5 - } - } - } - )"; - - MaterialSourceData material; - JsonTestResult loadResult = LoadTestDataFromJson(material, inputJson); - - EXPECT_EQ(AZ::JsonSerializationResult::Tasks::ReadField, loadResult.m_jsonResultCode.GetTask()); - EXPECT_EQ(AZ::JsonSerializationResult::Processing::Completed, loadResult.m_jsonResultCode.GetProcessing()); - - // Initially, the loaded material data will match the .material file exactly. This gives us the accurate representation of - // what's actually saved on disk. - - EXPECT_NE(material.m_properties["oldGroup"].find("MyFloat"), material.m_properties["oldGroup"].end()); - EXPECT_NE(material.m_properties["oldGroup"].find("MyIntOldName"), material.m_properties["oldGroup"].end()); - EXPECT_EQ(material.m_properties["general"].find("MyFloat"), material.m_properties["general"].end()); - EXPECT_EQ(material.m_properties["general"].find("MyInt"), material.m_properties["general"].end()); - - float myFloat = material.m_properties["oldGroup"]["MyFloat"].m_value.GetValue(); - EXPECT_EQ(myFloat, 1.2f); - - int32_t myInt = material.m_properties["oldGroup"]["MyIntOldName"].m_value.GetValue(); - EXPECT_EQ(myInt, 5); - - EXPECT_EQ(3, material.m_materialTypeVersion); - - // Then we force the material data to update to the latest material type version specification - ErrorMessageFinder warningFinder; // Note this finds errors and warnings, and we're looking for a warning. - warningFinder.AddExpectedErrorMessage("Automatic updates are available. Consider updating the .material source file"); - warningFinder.AddExpectedErrorMessage("This material is based on version '3'"); - warningFinder.AddExpectedErrorMessage("material type is now at version '10'"); - material.ApplyVersionUpdates(); - warningFinder.CheckExpectedErrorsFound(); - - // Now the material data should match the latest material type. - // Look for the property under the latest name in the material type, not the name used in the .material file. - - EXPECT_EQ(material.m_properties["oldGroup"].find("MyFloat"), material.m_properties["oldGroup"].end()); - EXPECT_EQ(material.m_properties["oldGroup"].find("MyIntOldName"), material.m_properties["oldGroup"].end()); - EXPECT_NE(material.m_properties["general"].find("MyFloat"), material.m_properties["general"].end()); - EXPECT_NE(material.m_properties["general"].find("MyInt"), material.m_properties["general"].end()); - - myFloat = material.m_properties["general"]["MyFloat"].m_value.GetValue(); - EXPECT_EQ(myFloat, 1.2f); - - myInt = material.m_properties["general"]["MyInt"].m_value.GetValue(); - EXPECT_EQ(myInt, 5); - - EXPECT_EQ(10, material.m_materialTypeVersion); - - // Calling ApplyVersionUpdates() again should not report the warning again, since the material has already been updated. - warningFinder.Reset(); - material.ApplyVersionUpdates(); - } - - TEST_F(MaterialSourceDataTests, Load_MaterialTypeVersionPartialUpdate) - { - // This case is similar to Load_MaterialTypeVersionUpdate but we start at a later - // version so only some of the version updates are applied. - - const AZStd::string inputJson = R"( - { - "materialType": "@exefolder@/Temp/test.materialtype", - "materialTypeVersion": 3, - "properties": { - "general": { - "testColorNameB": [0.1, 0.2, 0.3] - } - } - } - )"; - - MaterialSourceData material; - JsonTestResult loadResult = LoadTestDataFromJson(material, inputJson); - - EXPECT_EQ(AZ::JsonSerializationResult::Tasks::ReadField, loadResult.m_jsonResultCode.GetTask()); - EXPECT_EQ(AZ::JsonSerializationResult::Processing::Completed, loadResult.m_jsonResultCode.GetProcessing()); - - material.ApplyVersionUpdates(); - - AZ::Color testColor = material.m_properties["general"]["MyColor"].m_value.GetValue(); - EXPECT_TRUE(AZ::Color(0.1f, 0.2f, 0.3f, 1.0f).IsClose(testColor, 0.01)); - - EXPECT_EQ(10, material.m_materialTypeVersion); - } - - TEST_F(MaterialSourceDataTests, Load_Error_MaterialTypeVersionUpdateWithMismatchedVersion) - { - const AZStd::string inputJson = R"( - { - "materialType": "@exefolder@/Temp/test.materialtype", - "materialTypeVersion": 3, // At this version, the property should be testColorNameB not testColorNameA - "properties": { - "general": { - "testColorNameA": [0.1, 0.2, 0.3] - } - } - } - )"; - - MaterialSourceData material; - JsonTestResult loadResult = LoadTestDataFromJson(material, inputJson); - - loadResult.ContainsMessage("/properties/general/testColorNameA", "Property 'general.testColorNameA' not found in material type."); - - EXPECT_FALSE(material.m_properties["general"]["testColorNameA"].m_value.IsValid()); - - material.ApplyVersionUpdates(); - - EXPECT_FALSE(material.m_properties["general"]["MyColor"].m_value.IsValid()); + expectWarning("Could not find the image 'doesNotExist.streamingimage'", + [](MaterialSourceData& materialSourceData) + { + AddProperty(materialSourceData, "general", "MyImage", AZStd::string("doesNotExist.streamingimage")); + }, true); // In this case, the warning does happen even when the asset is not finalized, because the image path is checked earlier than that } } diff --git a/Gems/Atom/RPI/Code/Tests/Material/MaterialTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/MaterialTests.cpp index 477978875b..569f8f6df0 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/MaterialTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/MaterialTests.cpp @@ -85,7 +85,7 @@ namespace UnitTest m_testImage = StreamingImage::FindOrCreate(m_testImageAsset); MaterialAssetCreator materialCreator; - materialCreator.Begin(Uuid::CreateRandom(), *m_testMaterialTypeAsset); + materialCreator.Begin(Uuid::CreateRandom(), m_testMaterialTypeAsset, true); materialCreator.SetPropertyValue(Name{ "MyFloat2" }, Vector2{ 0.1f, 0.2f }); materialCreator.SetPropertyValue(Name{ "MyFloat3" }, Vector3{ 1.1f, 1.2f, 1.3f }); materialCreator.SetPropertyValue(Name{ "MyFloat4" }, Vector4{ 2.1f, 2.2f, 2.3f, 2.4f }); @@ -289,7 +289,7 @@ namespace UnitTest materialTypeCreator.End(materialTypeAsset); MaterialAssetCreator materialAssetCreator; - materialAssetCreator.Begin(Uuid::CreateRandom(), *materialTypeAsset); + materialAssetCreator.Begin(Uuid::CreateRandom(), materialTypeAsset, true); materialAssetCreator.End(materialAsset); Data::Instance material = Material::FindOrCreate(materialAsset); @@ -341,7 +341,7 @@ namespace UnitTest Data::Asset materialAssetWithEmptyImage; MaterialAssetCreator materialCreator; - materialCreator.Begin(Uuid::CreateRandom(), *m_testMaterialTypeAsset); + materialCreator.Begin(Uuid::CreateRandom(), m_testMaterialTypeAsset, true); materialCreator.SetPropertyValue(Name{"MyFloat2"}, Vector2{0.1f, 0.2f}); materialCreator.SetPropertyValue(Name{"MyFloat3"}, Vector3{1.1f, 1.2f, 1.3f}); materialCreator.SetPropertyValue(Name{"MyFloat4"}, Vector4{2.1f, 2.2f, 2.3f, 2.4f}); @@ -379,7 +379,7 @@ namespace UnitTest Data::Asset emptyMaterialAsset; MaterialAssetCreator materialCreator; - materialCreator.Begin(Uuid::CreateRandom(), *emptyMaterialTypeAsset); + materialCreator.Begin(Uuid::CreateRandom(), emptyMaterialTypeAsset, true); EXPECT_TRUE(materialCreator.End(emptyMaterialAsset)); Data::Instance material = Material::FindOrCreate(emptyMaterialAsset); @@ -450,7 +450,7 @@ namespace UnitTest materialTypeCreator.End(m_testMaterialTypeAsset); MaterialAssetCreator materialAssetCreator; - materialAssetCreator.Begin(Uuid::CreateRandom(), *m_testMaterialTypeAsset); + materialAssetCreator.Begin(Uuid::CreateRandom(), m_testMaterialTypeAsset, true); materialAssetCreator.End(m_testMaterialAsset); Data::Instance material = Material::FindOrCreate(m_testMaterialAsset); @@ -521,7 +521,7 @@ namespace UnitTest materialTypeCreator.End(m_testMaterialTypeAsset); MaterialAssetCreator materialAssetCreator; - materialAssetCreator.Begin(Uuid::CreateRandom(), *m_testMaterialTypeAsset); + materialAssetCreator.Begin(Uuid::CreateRandom(), m_testMaterialTypeAsset, true); materialAssetCreator.End(m_testMaterialAsset); Data::Instance material = Material::FindOrCreate(m_testMaterialAsset); @@ -591,7 +591,7 @@ namespace UnitTest materialTypeCreator.End(m_testMaterialTypeAsset); MaterialAssetCreator materialAssetCreator; - materialAssetCreator.Begin(Uuid::CreateRandom(), *m_testMaterialTypeAsset); + materialAssetCreator.Begin(Uuid::CreateRandom(), m_testMaterialTypeAsset, true); materialAssetCreator.End(m_testMaterialAsset); Data::Instance material = Material::FindOrCreate(m_testMaterialAsset); @@ -655,7 +655,7 @@ namespace UnitTest materialTypeCreator.End(m_testMaterialTypeAsset); MaterialAssetCreator materialAssetCreator; - materialAssetCreator.Begin(Uuid::CreateRandom(), *m_testMaterialTypeAsset); + materialAssetCreator.Begin(Uuid::CreateRandom(), m_testMaterialTypeAsset, true); materialAssetCreator.End(m_testMaterialAsset); Data::Instance material = Material::FindOrCreate(m_testMaterialAsset); @@ -669,7 +669,7 @@ namespace UnitTest { Data::Asset materialAsset; MaterialAssetCreator materialCreator; - materialCreator.Begin(Uuid::CreateRandom(), *m_testMaterialTypeAsset); + materialCreator.Begin(Uuid::CreateRandom(), m_testMaterialTypeAsset, true); materialCreator.SetPropertyValue(Name{ "MyFloat2" }, Vector2{ 0.1f, 0.2f }); materialCreator.SetPropertyValue(Name{ "MyFloat3" }, Vector3{ 1.1f, 1.2f, 1.3f }); materialCreator.SetPropertyValue(Name{ "MyFloat4" }, Vector4{ 2.1f, 2.2f, 2.3f, 2.4f }); @@ -778,7 +778,7 @@ namespace UnitTest materialTypeCreator.End(materialTypeAsset); MaterialAssetCreator materialAssetCreator; - materialAssetCreator.Begin(Uuid::CreateRandom(), *materialTypeAsset); + materialAssetCreator.Begin(Uuid::CreateRandom(), materialTypeAsset, true); materialAssetCreator.End(materialAsset); Data::Instance material = Material::FindOrCreate(materialAsset); @@ -859,7 +859,7 @@ namespace UnitTest materialTypeCreator.End(m_testMaterialTypeAsset); MaterialAssetCreator materialCreator; - materialCreator.Begin(Uuid::CreateRandom(), *m_testMaterialTypeAsset); + materialCreator.Begin(Uuid::CreateRandom(), m_testMaterialTypeAsset, true); materialCreator.End(m_testMaterialAsset); Data::Instance material = Material::FindOrCreate(m_testMaterialAsset);