From 45429872d60d79c1daa8b7957e967cc2526e642a Mon Sep 17 00:00:00 2001 From: santorac <55155825+santorac@users.noreply.github.com> Date: Tue, 18 Jan 2022 17:39:44 -0800 Subject: [PATCH] Switched back to making MaterialAsset::GetPropertyValues automatically finalize the material asset. I realized that it's too burdensome to expect client code to call Finalize on the MaterialAsset; every code that calls GetPropertyValues would have to call Finalize(). Instead of using const_cast in GetPropertyValues like I was doing before, I just changed GetPropertyValues to be a non-const function. There were a few places in Decal code I had to update to pass non-const MaterialAsset pointers. This isn't ideal, but I think it's better than the alternatives. Signed-off-by: santorac <55155825+santorac@users.noreply.github.com> --- .../Code/Source/Decals/DecalTextureArray.cpp | 6 +- .../Code/Source/Decals/DecalTextureArray.h | 2 +- .../DecalTextureArrayFeatureProcessor.cpp | 6 +- .../DecalTextureArrayFeatureProcessor.h | 2 +- .../Atom/RPI.Reflect/Material/MaterialAsset.h | 29 ++++----- .../RPI.Builders/Material/MaterialBuilder.cpp | 2 +- .../Model/MaterialAssetBuilderComponent.cpp | 2 +- .../Source/RPI.Public/Material/Material.cpp | 2 - .../RPI.Reflect/Material/MaterialAsset.cpp | 33 ++++++---- .../Material/MaterialAssetCreator.cpp | 2 + .../Tests/Material/MaterialAssetTests.cpp | 61 +++++++++++++++++-- .../Material/MaterialSourceDataTests.cpp | 41 +++++-------- 12 files changed, 117 insertions(+), 71 deletions(-) diff --git a/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArray.cpp b/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArray.cpp index 36a59bd07f..56ff1648d4 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArray.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArray.cpp @@ -49,7 +49,7 @@ namespace AZ } // Extract exactly which texture asset we need to load from the given material and map type (diffuse, normal, etc). - static AZ::Data::Asset GetStreamingImageAsset(const AZ::RPI::MaterialAsset& materialAsset, const AZ::Name& propertyName) + static AZ::Data::Asset GetStreamingImageAsset(AZ::RPI::MaterialAsset& materialAsset, const AZ::Name& propertyName) { if (!materialAsset.IsReady()) { @@ -84,7 +84,7 @@ namespace AZ static AZ::Data::Asset GetStreamingImageAsset(const AZ::Data::Asset materialAssetData, const AZ::Name& propertyName) { AZ_Assert(materialAssetData->IsReady(), "GetStreamingImageAsset() called with AssetData that is not ready."); - const AZ::RPI::MaterialAsset* materialAsset = materialAssetData.GetAs(); + AZ::RPI::MaterialAsset* materialAsset = materialAssetData.GetAs(); return GetStreamingImageAsset(*materialAsset, propertyName); } } @@ -141,7 +141,7 @@ namespace AZ return m_textureArrayPacked[mapType]; } - bool DecalTextureArray::IsValidDecalMaterial(const AZ::RPI::MaterialAsset& materialAsset) + bool DecalTextureArray::IsValidDecalMaterial(AZ::RPI::MaterialAsset& materialAsset) { return GetStreamingImageAsset(materialAsset, GetMapName(DecalMapType_Diffuse)).IsReady(); } diff --git a/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArray.h b/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArray.h index 97bd8b9cbe..39e66176fd 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArray.h +++ b/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArray.h @@ -57,7 +57,7 @@ namespace AZ // often different (BC5 for normals, BC7 for diffuse, etc) const Data::Instance& GetPackedTexture(const DecalMapType mapType) const; - static bool IsValidDecalMaterial(const RPI::MaterialAsset& materialAsset); + static bool IsValidDecalMaterial(RPI::MaterialAsset& materialAsset); private: diff --git a/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArrayFeatureProcessor.cpp b/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArrayFeatureProcessor.cpp index 393104907a..c6b4d4e754 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArrayFeatureProcessor.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArrayFeatureProcessor.cpp @@ -25,7 +25,7 @@ namespace AZ { namespace { - static AZ::RHI::Size GetTextureSizeFromMaterialAsset(const AZ::RPI::MaterialAsset* materialAsset) + static AZ::RHI::Size GetTextureSizeFromMaterialAsset(AZ::RPI::MaterialAsset* materialAsset) { for (const auto& elem : materialAsset->GetPropertyValues()) { @@ -375,7 +375,7 @@ namespace AZ } } - AZStd::optional DecalTextureArrayFeatureProcessor::AddMaterialToTextureArrays(const AZ::RPI::MaterialAsset* materialAsset) + AZStd::optional DecalTextureArrayFeatureProcessor::AddMaterialToTextureArrays(AZ::RPI::MaterialAsset* materialAsset) { const RHI::Size textureSize = GetTextureSizeFromMaterialAsset(materialAsset); @@ -410,7 +410,7 @@ namespace AZ AZ_PROFILE_SCOPE(AzRender, "DecalTextureArrayFeatureProcessor: OnAssetReady"); const Data::AssetId& assetId = asset->GetId(); - const RPI::MaterialAsset* materialAsset = asset.GetAs(); + RPI::MaterialAsset* materialAsset = asset.GetAs(); const bool validDecalMaterial = materialAsset && DecalTextureArray::IsValidDecalMaterial(*materialAsset); if (validDecalMaterial) { diff --git a/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArrayFeatureProcessor.h b/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArrayFeatureProcessor.h index fd535bbe64..bdd1739ebb 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArrayFeatureProcessor.h +++ b/Gems/Atom/Feature/Common/Code/Source/Decals/DecalTextureArrayFeatureProcessor.h @@ -111,7 +111,7 @@ namespace AZ void CacheShaderIndices(); // This call could fail (returning nullopt) if we run out of texture arrays - AZStd::optional AddMaterialToTextureArrays(const AZ::RPI::MaterialAsset* materialAsset); + AZStd::optional AddMaterialToTextureArrays(AZ::RPI::MaterialAsset* materialAsset); int FindTextureArrayWithSize(const RHI::Size& size) const; void RemoveMaterialFromDecal(const uint16_t decalIndex); 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 941fcd0fe9..b7cc51dcc4 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 @@ -102,16 +102,6 @@ namespace AZ //! Returns a layout that includes a list of MaterialPropertyDescriptors for each material property. const MaterialPropertiesLayout* GetMaterialPropertiesLayout() const; - //! Returns whether the material's properties are fully processed or not. - //! If true, property values can be accessed through GetPropertyValues(). - //! 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. @@ -122,19 +112,26 @@ namespace AZ //! //! Calling GetPropertyValues() will automatically finalize the material asset if it isn't finalized already. The //! MaterialTypeAsset must be loaded and ready. - const AZStd::vector& GetPropertyValues() const; - + const AZStd::vector& GetPropertyValues(); + + //! Returns true if material was created in a finalize state, as opposed to being finalized after loading from disk. + bool WasPreFinalized() const; + //! Returns the list of raw values for all properties in this material, as listed in the source .material file(s), before the material asset was Finalized. //! //! The MaterialAsset can be created in a "half-baked" state (see MaterialUtils::BuildersShouldFinalizeMaterialAssets) where //! minimal processing has been done because it did not yet have access to the MaterialTypeAsset. In that case, the list will //! be populated with values copied from the source .material file with little or no validation or other processing. It includes //! all parent .material files, with properties listed in low-to-high priority order. - //! This list will be empty however if the asset was finalized at build-time. + //! This list will be empty however if the asset was finalized at build-time (i.e. WasPreFinalized() returns true). const AZStd::vector>& GetRawPropertyValues() const; private: bool PostLoadInit() override; + + //! 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. + //! MaterialTypeAsset must be valid before this is called. + 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. @@ -159,7 +156,7 @@ namespace AZ //! Holds values for each material property, used to initialize Material instances. //! This is indexed by MaterialPropertyIndex and aligns with entries in m_materialPropertiesLayout. - AZStd::vector m_propertyValues; + mutable AZStd::vector m_propertyValues; //! The MaterialAsset can be created in a "half-baked" state where minimal processing has been done because it does //! not yet have access to the MaterialTypeAsset. In that case, this list will be populated with values copied from @@ -175,10 +172,10 @@ namespace AZ AZStd::vector> m_rawPropertyValues; //! Tracks whether Finalize() has been called, meaning m_propertyValues is populated with data matching the material type's property layout. - bool m_isFinalized = false; + //! (This value is intentionally not serialized, it is set by the Finalize() function) + mutable bool m_isFinalized = false; //! Tracks whether the MaterialAsset was already in a finalized state when it was loaded. - //! (This value is intentionally not serialized) bool m_wasPreFinalized = false; //! The materialTypeVersion this materialAsset was based off. If the versions do not match at runtime when a 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 cedbb1df6e..768890a29b 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 = 114; // material dependency improvements + materialBuilderDescriptor.m_version = 115; // material dependency improvements updated 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 a44b47ee6c..9a92e7b762 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Builders/Model/MaterialAssetBuilderComponent.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Builders/Model/MaterialAssetBuilderComponent.cpp @@ -128,7 +128,7 @@ namespace AZ if (auto* serialize = azrtti_cast(context)) { serialize->Class() - ->Version(20); // material dependency improvements + ->Version(21); // material dependency improvements updated } } 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 fe9dbef278..1f739c24f1 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Material/Material.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Material/Material.cpp @@ -62,8 +62,6 @@ 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 0325c3ac38..309daf8b62 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAsset.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAsset.cpp @@ -33,12 +33,12 @@ namespace AZ if (auto* serializeContext = azrtti_cast(context)) { serializeContext->Class() - ->Version(13) // added m_rawPropertyValues + ->Version(14) // added m_rawPropertyValues ->Field("materialTypeAsset", &MaterialAsset::m_materialTypeAsset) ->Field("materialTypeVersion", &MaterialAsset::m_materialTypeVersion) ->Field("propertyValues", &MaterialAsset::m_propertyValues) ->Field("rawPropertyValues", &MaterialAsset::m_rawPropertyValues) - ->Field("isFinalized", &MaterialAsset::m_isFinalized) + ->Field("finalized", &MaterialAsset::m_wasPreFinalized) ; } } @@ -104,19 +104,19 @@ namespace AZ return m_materialTypeAsset->GetMaterialPropertiesLayout(); } - bool MaterialAsset::IsFinalized() const + bool MaterialAsset::WasPreFinalized() const { - if (m_isFinalized) - { - AZ_Assert(GetMaterialPropertiesLayout() && m_propertyValues.size() == GetMaterialPropertiesLayout()->GetPropertyCount(), "MaterialAsset is marked as Finalized but does not have the right number of property values."); - } - - return m_isFinalized; + return m_wasPreFinalized; } void MaterialAsset::Finalize(AZStd::function reportWarning, AZStd::function reportError) { - if (IsFinalized()) + if (m_wasPreFinalized) + { + m_isFinalized = true; + } + + if (m_isFinalized) { return; } @@ -197,10 +197,18 @@ namespace AZ m_isFinalized = true; } - const AZStd::vector& MaterialAsset::GetPropertyValues() const + const AZStd::vector& MaterialAsset::GetPropertyValues() { - AZ_Error(s_debugTraceName, IsFinalized(), "MaterialAsset must be finalized before its property values can be accessed"); + // 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. + // And overriding AssetHandler::InitAsset in MaterialAssetHandler didn't work, because there seems to be non-determinism on the order + // of InitAsset calls when a ModelAsset references a MaterialAsset, the model gets initialized first and then fails to use the material. + // So we finalize just-in-time when properties are accessed. + // If we could solve the problem with InitAsset, that would be the ideal place to call Finalize() and we could make GetPropertyValues() const again. + Finalize(); + AZ_Assert(GetMaterialPropertiesLayout() && m_propertyValues.size() == GetMaterialPropertiesLayout()->GetPropertyCount(), "MaterialAsset should be finalized but does not have the right number of property values."); + return m_propertyValues; } @@ -334,7 +342,6 @@ namespace AZ if (Base::LoadAssetData(asset, stream, assetLoadFilterCB) == Data::AssetHandler::LoadResult::LoadComplete) { asset.GetAs()->AssetInitBus::Handler::BusConnect(); - asset.GetAs()->m_wasPreFinalized = asset.GetAs()->m_isFinalized; return Data::AssetHandler::LoadResult::LoadComplete; } 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 7d4ecf0b18..872e4ad754 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAssetCreator.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAssetCreator.cpp @@ -49,6 +49,8 @@ namespace AZ [this](const char* message) { ReportWarning("%s", message); }, [this](const char* message) { ReportError("%s", message); }); + m_asset->m_wasPreFinalized = true; + // Finalize() doesn't clear the raw property data because that's the same function used at runtime, which does need to maintain the raw data // to support hot reload. But here we are pre-baking with the assumption that AP build dependencies will keep the material type // and material asset in sync, so we can discard the raw property data and just rely on the data in the material type asset. diff --git a/Gems/Atom/RPI/Code/Tests/Material/MaterialAssetTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/MaterialAssetTests.cpp index fdb131d449..61e1283f52 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/MaterialAssetTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/MaterialAssetTests.cpp @@ -111,6 +111,10 @@ namespace UnitTest EXPECT_EQ(assetId, materialAsset->GetId()); EXPECT_EQ(Data::AssetData::AssetStatus::Ready, materialAsset->GetStatus()); + + EXPECT_TRUE(materialAsset->WasPreFinalized()); + EXPECT_EQ(0, materialAsset->GetRawPropertyValues().size()); + validate(materialAsset); // Also test serialization... @@ -123,6 +127,57 @@ namespace UnitTest Data::Asset serializedAsset = tester.SerializeIn(Data::AssetId(Uuid::CreateRandom()), noAssets); validate(serializedAsset); } + + TEST_F(MaterialAssetTests, DeferredFinalize) + { + Data::AssetId assetId(Uuid::CreateRandom()); + + MaterialAssetCreator creator; + bool shouldFinalize = false; + creator.Begin(assetId, m_testMaterialTypeAsset, shouldFinalize); + + 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 }); + creator.SetPropertyValue(Name{ "MyColor" }, Color{ 1.0f, 1.0f, 1.0f, 1.0f }); + creator.SetPropertyValue(Name{ "MyInt" }, -2); + creator.SetPropertyValue(Name{ "MyUInt" }, 12u); + creator.SetPropertyValue(Name{ "MyFloat" }, 1.5f); + creator.SetPropertyValue(Name{ "MyBool" }, true); + creator.SetPropertyValue(Name{ "MyImage" }, m_testImageAsset); + creator.SetPropertyValue(Name{ "MyEnum" }, 1u); + + Data::Asset materialAsset; + EXPECT_TRUE(creator.End(materialAsset)); + + EXPECT_FALSE(materialAsset->WasPreFinalized()); + EXPECT_EQ(10, materialAsset->GetRawPropertyValues().size()); + + // Also test serialization... + + SerializeTester tester(GetSerializeContext()); + tester.SerializeOut(materialAsset.Get()); + + // Using a filter that skips loading assets because we are using a dummy image asset + ObjectStream::FilterDescriptor noAssets{ AZ::Data::AssetFilterNoAssetLoading }; + Data::Asset serializedAsset = tester.SerializeIn(Data::AssetId(Uuid::CreateRandom()), noAssets); + + EXPECT_FALSE(materialAsset->WasPreFinalized()); + EXPECT_EQ(10, materialAsset->GetRawPropertyValues().size()); + + // GetPropertyValues() will automatically finalize the material asset, so we can go ahead and check the property values. + EXPECT_EQ(materialAsset->GetPropertyValues().size(), 10); + EXPECT_EQ(materialAsset->GetPropertyValues()[0].GetValue(), true); + EXPECT_EQ(materialAsset->GetPropertyValues()[1].GetValue(), -2); + EXPECT_EQ(materialAsset->GetPropertyValues()[2].GetValue(), 12); + EXPECT_EQ(materialAsset->GetPropertyValues()[3].GetValue(), 1.5f); + EXPECT_EQ(materialAsset->GetPropertyValues()[4].GetValue(), Vector2(0.1f, 0.2f)); + EXPECT_EQ(materialAsset->GetPropertyValues()[5].GetValue(), Vector3(1.1f, 1.2f, 1.3f)); + EXPECT_EQ(materialAsset->GetPropertyValues()[6].GetValue(), Vector4(2.1f, 2.2f, 2.3f, 2.4f)); + EXPECT_EQ(materialAsset->GetPropertyValues()[7].GetValue(), Color(1.0f, 1.0f, 1.0f, 1.0f)); + EXPECT_EQ(materialAsset->GetPropertyValues()[8].GetValue>(), m_testImageAsset); + EXPECT_EQ(materialAsset->GetPropertyValues()[9].GetValue(), 1u); + } TEST_F(MaterialAssetTests, PropertyDefaultValuesComeFromParentMaterial) { @@ -267,15 +322,13 @@ namespace UnitTest 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. MaterialPropertyIndex myIntIndex = materialAsset->GetMaterialPropertiesLayout()->FindPropertyIndex(Name{"MyIntRenamed"}); 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. diff --git a/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp index 29f0e7d101..ef89e0138c 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp @@ -182,7 +182,7 @@ namespace UnitTest Data::Asset materialAsset = materialAssetOutcome.GetValue(); - EXPECT_TRUE(materialAsset->IsFinalized()); + EXPECT_TRUE(materialAsset->WasPreFinalized()); EXPECT_EQ(0, materialAsset->GetRawPropertyValues().size()); // A pre-baked material has no need for the original raw property names and values // The order here is based on the order in the MaterialTypeSourceData, as added to the MaterialTypeAssetCreator. @@ -227,14 +227,10 @@ namespace UnitTest EXPECT_TRUE(materialAssetOutcome.IsSuccess()); Data::Asset materialAsset = materialAssetOutcome.GetValue(); - - ErrorMessageFinder expectNotFinalizedError("MaterialAsset must be finalized"); - EXPECT_FALSE(materialAsset->IsFinalized()); + EXPECT_FALSE(materialAsset->WasPreFinalized()); - expectNotFinalizedError.ResetCounts(); - EXPECT_TRUE(materialAsset->GetPropertyValues().empty()); - expectNotFinalizedError.CheckExpectedErrorsFound(); + // Note we avoid calling GetPropertyValues() because that will auto-finalize the material. We want to check its raw property values first. auto findRawPropertyValue = [materialAsset](const char* propertyId) { @@ -279,16 +275,10 @@ namespace UnitTest SerializeTester tester(GetSerializeContext()); tester.SerializeOut(materialAsset.Get()); materialAsset = tester.SerializeIn(Uuid::CreateRandom(), ObjectStream::FilterDescriptor{AZ::Data::AssetFilterNoAssetLoading}); - - // We check that everything is still in the original un-finalized state after going through the serialization process. - EXPECT_FALSE(materialAsset->IsFinalized()); - checkRawPropertyValues(); - expectNotFinalizedError.ResetCounts(); - EXPECT_TRUE(materialAsset->GetPropertyValues().empty()); - expectNotFinalizedError.CheckExpectedErrorsFound(); - materialAsset->Finalize(); - EXPECT_TRUE(materialAsset->IsFinalized()); + // We check that the asset is still in the original un-finalized state after going through the serialization process. + EXPECT_FALSE(materialAsset->WasPreFinalized()); + checkRawPropertyValues(); // Now all the property values should be available through the main GetPropertyValues() API. EXPECT_EQ(materialAsset->GetPropertyValues()[0].GetValue(), true); @@ -301,8 +291,9 @@ namespace UnitTest EXPECT_EQ(materialAsset->GetPropertyValues()[7].GetValue(), Color(0.1f, 0.2f, 0.3f, 0.4f)); 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) + EXPECT_FALSE(materialAsset->WasPreFinalized()); checkRawPropertyValues(); } @@ -659,19 +650,19 @@ namespace UnitTest auto materialAssetLevel1 = sourceDataLevel1.CreateMaterialAsset(Uuid::CreateRandom(), "", MaterialAssetProcessingMode::PreBake, true); EXPECT_TRUE(materialAssetLevel1.IsSuccess()); - EXPECT_TRUE(materialAssetLevel1.GetValue()->IsFinalized()); + EXPECT_TRUE(materialAssetLevel1.GetValue()->WasPreFinalized()); m_assetSystemStub.RegisterSourceInfo("level1.material", materialAssetLevel1.GetValue().GetId()); auto materialAssetLevel2 = sourceDataLevel2.CreateMaterialAsset(Uuid::CreateRandom(), "", MaterialAssetProcessingMode::PreBake, true); EXPECT_TRUE(materialAssetLevel2.IsSuccess()); - EXPECT_TRUE(materialAssetLevel2.GetValue()->IsFinalized()); + EXPECT_TRUE(materialAssetLevel2.GetValue()->WasPreFinalized()); m_assetSystemStub.RegisterSourceInfo("level2.material", materialAssetLevel2.GetValue().GetId()); auto materialAssetLevel3 = sourceDataLevel3.CreateMaterialAsset(Uuid::CreateRandom(), "", MaterialAssetProcessingMode::PreBake, true); EXPECT_TRUE(materialAssetLevel3.IsSuccess()); - EXPECT_TRUE(materialAssetLevel3.GetValue()->IsFinalized()); + EXPECT_TRUE(materialAssetLevel3.GetValue()->WasPreFinalized()); auto layout = m_testMaterialTypeAsset->GetMaterialPropertiesLayout(); MaterialPropertyIndex myFloat = layout->FindPropertyIndex(Name("general.MyFloat")); @@ -731,21 +722,21 @@ namespace UnitTest auto materialAssetLevel1Result = sourceDataLevel1.CreateMaterialAsset(Uuid::CreateRandom(), "", MaterialAssetProcessingMode::DeferredBake, true); EXPECT_TRUE(materialAssetLevel1Result.IsSuccess()); Data::Asset materialAssetLevel1 = materialAssetLevel1Result.TakeValue(); - EXPECT_FALSE(materialAssetLevel1->IsFinalized()); + EXPECT_FALSE(materialAssetLevel1->WasPreFinalized()); m_assetSystemStub.RegisterSourceInfo("level1.material", materialAssetLevel1.GetId()); auto materialAssetLevel2Result = sourceDataLevel2.CreateMaterialAsset(Uuid::CreateRandom(), "", MaterialAssetProcessingMode::DeferredBake, true); EXPECT_TRUE(materialAssetLevel2Result.IsSuccess()); Data::Asset materialAssetLevel2 = materialAssetLevel2Result.TakeValue(); - EXPECT_FALSE(materialAssetLevel2->IsFinalized()); + EXPECT_FALSE(materialAssetLevel2->WasPreFinalized()); m_assetSystemStub.RegisterSourceInfo("level2.material", materialAssetLevel2.GetId()); auto materialAssetLevel3Result = sourceDataLevel3.CreateMaterialAsset(Uuid::CreateRandom(), "", MaterialAssetProcessingMode::DeferredBake, true); EXPECT_TRUE(materialAssetLevel3Result.IsSuccess()); Data::Asset materialAssetLevel3 = materialAssetLevel3Result.TakeValue(); - EXPECT_FALSE(materialAssetLevel3->IsFinalized()); + EXPECT_FALSE(materialAssetLevel3->WasPreFinalized()); // Now we'll create the material type asset in memory so the materials will have what they need to finalize. Data::Asset testMaterialTypeAsset = CreateTestMaterialTypeAsset(materialTypeAssetId); @@ -766,9 +757,7 @@ namespace UnitTest tester.SerializeOut(materialAssetLevel3.Get()); materialAssetLevel3 = tester.SerializeIn(Uuid::CreateRandom(), ObjectStream::FilterDescriptor{AZ::Data::AssetFilterNoAssetLoading}); - materialAssetLevel1->Finalize(); - materialAssetLevel2->Finalize(); - materialAssetLevel3->Finalize(); + // The properties will finalize automatically when we call GetPropertyValues()... AZStd::array_view properties;