diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialSourceData.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialSourceData.h index a5fb0214e6..0a9371f722 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialSourceData.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialSourceData.h @@ -33,7 +33,7 @@ namespace AZ class MaterialAsset; class MaterialAssetCreator; - enum MaterialAssetProcessingMode + enum class MaterialAssetProcessingMode { PreBake, //!< all material asset processing is done in the Asset Processor, producing a finalized material asset DeferredBake //!< some material asset processing is deferred, and the material asset is finalized at runtime after loading 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 2a6c89a8fa..941fcd0fe9 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 @@ -107,6 +107,11 @@ 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 the raw property values to + //! get the material asset ready to be used. + //! Note the MaterialTypeAsset must be valid before this is called. + void Finalize(AZStd::function reportWarning = nullptr, AZStd::function reportError = nullptr); + //! 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. @@ -131,12 +136,6 @@ 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/Source/RPI.Builders/Material/MaterialBuilder.cpp b/Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.cpp index 6e50e58dd3..cedbb1df6e 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 = 113; // material dependency improvements + materialBuilderDescriptor.m_version = 114; // 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 1cc1925564..a44b47ee6c 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Builders/Model/MaterialAssetBuilderComponent.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Builders/Model/MaterialAssetBuilderComponent.cpp @@ -45,7 +45,8 @@ namespace AZ if (auto* serialize = azrtti_cast(context)) { serialize->Class() - ->Version(5) // <<<<< This probably is NOT the version number you want to bump. What you're looking for is MaterialAssetBuilderComponent::Reflect below + ->Version(5) // <<<<< If you have made changes to material code and need to force scene files to be reprocessed, this probably is + // NOT the version number you want to bump . What you're looking for is MaterialAssetBuilderComponent::Reflect below. ->Attribute(Edit::Attributes::SystemComponentTags, AZStd::vector({ AssetBuilderSDK::ComponentTags::AssetBuilder })); } } @@ -127,7 +128,7 @@ namespace AZ if (auto* serialize = azrtti_cast(context)) { serialize->Class() - ->Version(19); // material dependency improvements + ->Version(20); // material dependency improvements } } diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Material/Material.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Material/Material.cpp index 1f739c24f1..fe9dbef278 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Material/Material.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Material/Material.cpp @@ -62,6 +62,8 @@ namespace AZ m_materialAsset = { &materialAsset, AZ::Data::AssetLoadBehavior::PreLoad }; + m_materialAsset->Finalize(); + // Cache off pointers to some key data structures from the material type... auto srgLayout = m_materialAsset->GetMaterialSrgLayout(); if (srgLayout) 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 a40fa77019..569e2de81a 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAsset.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAsset.cpp @@ -199,10 +199,7 @@ namespace AZ const AZStd::vector& MaterialAsset::GetPropertyValues() const { - // This can't be done in MaterialAssetHandler::LoadAssetData because the MaterialTypeAsset isn't necessarily loaded at that point. - // And it can't be done in PostLoadInit() because that happens on the next frame which might be too late. So we finalize just-in-time - // when properties are accessed. - const_cast(this)->Finalize(); + AZ_Error(s_debugTraceName, IsFinalized(), "MaterialAsset must be finalized before its property values can be accessed"); return m_propertyValues; } diff --git a/Gems/Atom/RPI/Code/Tests/Material/MaterialAssetTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/MaterialAssetTests.cpp index 633953cecc..fdb131d449 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/MaterialAssetTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/MaterialAssetTests.cpp @@ -266,6 +266,10 @@ namespace UnitTest 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 '2'"); + + materialAsset->Finalize(); + + warningFinder.CheckExpectedErrorsFound(); // Even though this material was created using the old version of the material type, it's property values should get automatically // updated to align with the new property layout in the latest MaterialTypeAsset. @@ -273,8 +277,6 @@ namespace UnitTest EXPECT_EQ(2, myIntIndex.GetIndex()); EXPECT_EQ(7, materialAsset->GetPropertyValues()[myIntIndex.GetIndex()].GetValue()); - warningFinder.CheckExpectedErrorsFound(); - // Since the MaterialAsset has already been updated, and the warning reported once, we should not see the "consider updating" // warning reported again on subsequent property accesses. warningFinder.Reset(); diff --git a/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp index e4d3cc9768..29f0e7d101 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp @@ -228,8 +228,13 @@ namespace UnitTest Data::Asset materialAsset = materialAssetOutcome.GetValue(); + ErrorMessageFinder expectNotFinalizedError("MaterialAsset must be finalized"); + EXPECT_FALSE(materialAsset->IsFinalized()); - // Note we avoid calling GetPropertyValues() because that will auto-finalize the material. We want to check its raw property values first. + + expectNotFinalizedError.ResetCounts(); + EXPECT_TRUE(materialAsset->GetPropertyValues().empty()); + expectNotFinalizedError.CheckExpectedErrorsFound(); auto findRawPropertyValue = [materialAsset](const char* propertyId) { @@ -275,12 +280,14 @@ namespace UnitTest tester.SerializeOut(materialAsset.Get()); materialAsset = tester.SerializeIn(Uuid::CreateRandom(), ObjectStream::FilterDescriptor{AZ::Data::AssetFilterNoAssetLoading}); - // We check the raw property values again on the loaded data, showing that the same data is available in the original un-finalized state. - checkRawPropertyValues(); - - // The material will automatically finalize itself when the properties are accessed. + // We check that everything is still in the original un-finalized state after going through the serialization process. EXPECT_FALSE(materialAsset->IsFinalized()); - materialAsset->GetPropertyValues(); + checkRawPropertyValues(); + expectNotFinalizedError.ResetCounts(); + EXPECT_TRUE(materialAsset->GetPropertyValues().empty()); + expectNotFinalizedError.CheckExpectedErrorsFound(); + + materialAsset->Finalize(); EXPECT_TRUE(materialAsset->IsFinalized()); // Now all the property values should be available through the main GetPropertyValues() API. @@ -295,6 +302,8 @@ namespace UnitTest EXPECT_EQ(materialAsset->GetPropertyValues()[8].GetValue>(), m_testImageAsset); EXPECT_EQ(materialAsset->GetPropertyValues()[9].GetValue(), 1u); + // The raw property values are still available (because they are needed if a hot-reload of the MaterialTypeAsset occurs) + checkRawPropertyValues(); } void CheckEqual(MaterialSourceData& a, MaterialSourceData& b) @@ -757,8 +766,9 @@ namespace UnitTest tester.SerializeOut(materialAssetLevel3.Get()); materialAssetLevel3 = tester.SerializeIn(Uuid::CreateRandom(), ObjectStream::FilterDescriptor{AZ::Data::AssetFilterNoAssetLoading}); - - // The properties will finalize automatically when we call GetPropertyValues()... + materialAssetLevel1->Finalize(); + materialAssetLevel2->Finalize(); + materialAssetLevel3->Finalize(); AZStd::array_view properties; @@ -779,10 +789,6 @@ namespace UnitTest EXPECT_EQ(properties[myFloat.GetIndex()].GetValue(), 3.5f); EXPECT_EQ(properties[myFloat2.GetIndex()].GetValue(), Vector2(4.1f, 4.2f)); EXPECT_EQ(properties[myColor.GetIndex()].GetValue(), Color(0.15f, 0.25f, 0.35f, 0.45f)); - - EXPECT_TRUE(materialAssetLevel1->IsFinalized()); - EXPECT_TRUE(materialAssetLevel2->IsFinalized()); - EXPECT_TRUE(materialAssetLevel3->IsFinalized()); } TEST_F(MaterialSourceDataTests, CreateMaterialAsset_MultiLevelDataInheritance_Error_MaterialTypesDontMatch)