diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialTypeSourceData.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialTypeSourceData.h index cca7d41e91..6d7c9b556c 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialTypeSourceData.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialTypeSourceData.h @@ -129,7 +129,8 @@ namespace AZ AZStd::string m_renameTo; }; - // TODO: Support script operations. We will only be supporting rename for now. + // TODO: Support script operations--At that point, we'll likely need to replace VersionUpdatesRenameOperationDefinition with a more generic + // data structure that has a custom JSON serialize. We will only be supporting rename for now. using VersionUpdateActions = AZStd::vector; struct VersionUpdateDefinition @@ -146,9 +147,6 @@ namespace AZ { AZ_TYPE_INFO(AZ::RPI::MaterialTypeSourceData::PropertyLayout, "{AE53CF3F-5C3B-44F5-B2FB-306F0EB06393}"); - //! Indicates the version of the set of available properties. Can be used to detect materials that might need to be updated. - uint32_t m_version = 0; - //! List of groups that will contain the available properties AZStd::vector m_groups; 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 2d42cc13ea..e14ff30f87 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 @@ -22,6 +22,7 @@ namespace UnitTest { class MaterialTests; + class MaterialAssetTests; } namespace AZ @@ -42,10 +43,12 @@ namespace AZ , public MaterialReloadNotificationBus::Handler , public AssetInitBus::Handler { + friend class MaterialVersionUpdate; friend class MaterialAssetCreator; friend class MaterialAssetHandler; friend class MaterialAssetCreatorCommon; friend class UnitTest::MaterialTests; + friend class UnitTest::MaterialAssetTests; public: AZ_RTTI(MaterialAsset, "{522C7BE0-501D-463E-92C6-15184A2B7AD8}", AZ::Data::AssetData); @@ -119,10 +122,8 @@ namespace AZ //! from m_materialTypeAsset. void RealignPropertyValuesAndNames(); - //! Renames properties in m_propertyNames based on the MaterialTypeAsset's version update. - void RenamePropertyNames(); - template - void RenamePropertyNames(iteratorType start, iteratorType end); + //! Renames properties in m_propertyNames based on the MaterialTypeAsset's version update. Note that only version upgrades are supported. + void ApplyVersionUpdates(); //! Called by asset creators to assign the asset to a ready state. void SetReady(); diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialTypeAsset.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialTypeAsset.h index ca7de621a4..d0f0609c02 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialTypeAsset.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialTypeAsset.h @@ -126,10 +126,10 @@ namespace AZ //! Returns the version of the MaterialTypeAsset. uint32_t GetVersion() const; - //! Returns the version update containing the actions to perform. If a version - //! is not defined in m_materialVersionUpdatesMap, a material version with the specified + //! Returns the toVersion update containing the actions to perform. If a toVersion + //! is not defined in m_materialVersionUpdatesMap, a material toVersion with the specified //! version but empty actions will be returned. - const MaterialVersionUpdate GetMaterialVersionUpdate(uint32_t version) const; + MaterialVersionUpdate GetMaterialVersionUpdate(uint32_t toVersion) const; private: bool PostLoadInit() override; diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialTypeAssetCreator.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialTypeAssetCreator.h index 953b201960..64327ecd03 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialTypeAssetCreator.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialTypeAssetCreator.h @@ -41,7 +41,7 @@ namespace AZ //! Sets the version of the MaterialTypeAsset void SetVersion(uint32_t version); //! Adds a version update object into the MaterialTypeAsset - void AddVersionUpdate(uint32_t toVersion, MaterialVersionUpdate materialVersionUpdate); + void AddVersionUpdate(uint32_t toVersion, const MaterialVersionUpdate& materialVersionUpdate); //! Indicates that this MaterialType will own the specified shader option. //! Material-owned shader options can be connected to material properties (either directly or through functors). diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialVersionUpdate.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialVersionUpdate.h index 9147ec8d82..ee15ebfdcf 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialVersionUpdate.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialVersionUpdate.h @@ -11,13 +11,18 @@ #include #include #include +#include namespace AZ { namespace RPI { - struct MaterialVersionUpdate + class MaterialAsset; + + // This class contains a toVersion and a list of actions to specify what operations were performed to upgrade a materialType. + class MaterialVersionUpdate { + public: AZ_TYPE_INFO(AZ::RPI::MaterialVersionUpdate, "{B36E7712-AED8-46AA-AFE0-01F8F884C44A}"); static void Reflect(ReflectContext* context); @@ -26,20 +31,28 @@ namespace AZ { AZ_TYPE_INFO(AZ::RPI::MaterialVersionUpdate::Action, "{A1FBEB19-EA05-40F0-9700-57D048DF572B}"); - AZStd::string m_operation; - AZStd::map m_argsMap; + AZ::Name m_operation; + AZStd::unordered_map m_argsMap; Action() = default; - Action(const AZStd::string& operation, const AZStd::initializer_list>& args); - void AddArgs(const AZStd::string& key, const AZStd::string& argument); + Action(const AZ::Name& operation, const AZStd::initializer_list>& args); + void AddArg(const AZ::Name& key, const AZ::Name& argument); }; - MaterialVersionUpdate() = default; - MaterialVersionUpdate(uint32_t toVersion); + explicit MaterialVersionUpdate() = default; + explicit MaterialVersionUpdate(uint32_t toVersion); - uint32_t m_toVersion; + uint32_t GetVersion() const; + void SetVersion(uint32_t toVersion); + + void ApplyVersionUpdates(MaterialAsset& materialAsset) const; using Actions = AZStd::vector; + const Actions& GetActions() const; + void AddAction(const Action& action); + + private: + uint32_t m_toVersion; Actions m_actions; }; diff --git a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp index c8ce26d4e7..4d9d40cde2 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp @@ -95,8 +95,7 @@ namespace AZ ; serializeContext->Class() - ->Version(1) - ->Field("version", &PropertyLayout::m_version) + ->Version(2) ->Field("groups", &PropertyLayout::m_groups) ->Field("properties", &PropertyLayout::m_properties) ; @@ -317,8 +316,9 @@ namespace AZ MaterialVersionUpdate materialVersionUpdate; for (const auto& action : versionUpdate.m_actions) { - materialVersionUpdate.m_actions.push_back( - MaterialVersionUpdate::Action("rename", { { "from", action.m_renameFrom }, { "to", action.m_renameTo } })); + materialVersionUpdate.AddAction(MaterialVersionUpdate::Action(AZ::Name{ "rename" }, { + { AZ::Name{ "from" }, AZ::Name{ action.m_renameFrom } }, + { AZ::Name{ "to" }, AZ::Name{ action.m_renameTo } } })); } materialTypeAssetCreator.AddVersionUpdate(versionUpdate.m_toVersion, materialVersionUpdate); } 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 eefa259ed3..1695319a64 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAsset.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAsset.cpp @@ -6,6 +6,8 @@ * */ +#pragma optimize("", off) + #include #include #include @@ -107,10 +109,9 @@ namespace AZ if (!m_propertyNames.empty()) { const uint32_t materialTypeVersion = m_materialTypeAsset->GetVersion(); - if (m_materialTypeVersion != materialTypeVersion) + if (m_materialTypeVersion < materialTypeVersion) { - const_cast(this)->RenamePropertyNames(); - const_cast(m_materialTypeVersion) = materialTypeVersion; + const_cast(this)->ApplyVersionUpdates(); } if (m_isDirty) @@ -194,50 +195,16 @@ namespace AZ m_isDirty = false; } - template - void MaterialAsset::RenamePropertyNames(iteratorType start, iteratorType end) + void MaterialAsset::ApplyVersionUpdates() { - for (iteratorType it = start; it != end; ++it) + for (int i = 0; i < static_cast(m_materialTypeAsset->GetVersion() - m_materialTypeVersion); ++i) { - for (auto& propertyName : m_propertyNames) - { - const auto toFromIterator = it->find(propertyName.GetCStr()); - if (toFromIterator != it->end()) - { - propertyName = AZ::Name{ toFromIterator->second }; - } - } - } - } + const auto& versionUpdate = m_materialTypeAsset->GetMaterialVersionUpdate(m_materialTypeVersion + i + 1); - void MaterialAsset::RenamePropertyNames() - { - // construct toFrom map in ascending order of version. - AZStd::vector> versionToFrom; - - const bool ascending = m_materialTypeVersion < m_materialTypeAsset->GetVersion(); - for (int i = 0; i < AZStd::abs(static_cast(m_materialTypeVersion - m_materialTypeAsset->GetVersion())); ++i) - { - const auto& versionUpdate = m_materialTypeAsset->GetMaterialVersionUpdate((m_materialTypeVersion < m_materialTypeAsset->GetVersion() ? m_materialTypeVersion : m_materialTypeAsset->GetVersion()) + i + 1); - - versionToFrom.push_back(); - for (const auto& action : versionUpdate.m_actions) - { - const AZStd::string from = ascending ? action.m_argsMap.find("from")->second : action.m_argsMap.find("to")->second; - const AZStd::string to = ascending ? action.m_argsMap.find("to")->second : action.m_argsMap.find("from")->second; - - versionToFrom[i][from] = to; - } + versionUpdate.ApplyVersionUpdates(*this); } - if (ascending) - { - RenamePropertyNames(versionToFrom.cbegin(), versionToFrom.cend()); - } - else - { - RenamePropertyNames(versionToFrom.crbegin(), versionToFrom.crend()); - } + m_materialTypeVersion = m_materialTypeAsset->GetVersion(); } void MaterialAsset::ReinitializeMaterialTypeAsset(Data::Asset asset) @@ -287,3 +254,5 @@ namespace AZ } // namespace RPI } // namespace AZ + +#pragma optimize("", on) diff --git a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialTypeAsset.cpp b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialTypeAsset.cpp index 9209942f28..a876028599 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialTypeAsset.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialTypeAsset.cpp @@ -169,10 +169,10 @@ namespace AZ return m_version; } - const AZ::RPI::MaterialVersionUpdate MaterialTypeAsset::GetMaterialVersionUpdate(uint32_t version) const + MaterialVersionUpdate MaterialTypeAsset::GetMaterialVersionUpdate(uint32_t toVersion) const { - const auto it = m_materialVersionUpdateMap.find(version); - return it != m_materialVersionUpdateMap.end() ? it->second : MaterialVersionUpdate(version); + const auto it = m_materialVersionUpdateMap.find(toVersion); + return it != m_materialVersionUpdateMap.end() ? it->second : MaterialVersionUpdate(toVersion); } void MaterialTypeAsset::SetReady() diff --git a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialTypeAssetCreator.cpp b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialTypeAssetCreator.cpp index f10ae42266..79336692ec 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialTypeAssetCreator.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialTypeAssetCreator.cpp @@ -102,18 +102,18 @@ namespace AZ bool MaterialTypeAssetCreator::ValidateMaterialVersion() { - AZStd::vector renamedPropertieNames; + AZStd::vector renamedPropertyNames; const auto& materialVersionUpdate = m_asset->GetMaterialVersionUpdate(m_asset->m_version); - for (const auto& action : materialVersionUpdate.m_actions) + for (const auto& action : materialVersionUpdate.GetActions()) { - const auto it = action.m_argsMap.find("to"); + const auto it = action.m_argsMap.find(AZ::Name{ "to" }); if (it != action.m_argsMap.end()) { - renamedPropertieNames.push_back(it->second); + renamedPropertyNames.push_back(it->second); } } - for (const auto& propertyName : renamedPropertieNames) + for (const auto& propertyName : renamedPropertyNames) { const auto propertyIndex = m_asset->m_materialPropertiesLayout->FindPropertyIndex(AZ::Name{ propertyName }); if (!propertyIndex.IsValid()) @@ -121,7 +121,7 @@ namespace AZ ReportError(AZStd::string::format( "Renamed property '%s' not found in material property layout. Check that the property name has been " "upgraded to the correct version", - propertyName.c_str()) + propertyName.GetCStr()) .c_str()); return false; } @@ -157,7 +157,7 @@ namespace AZ m_asset->m_version = version; } - void MaterialTypeAssetCreator::AddVersionUpdate(uint32_t toVersion, MaterialVersionUpdate materialVersionUpdate) + void MaterialTypeAssetCreator::AddVersionUpdate(uint32_t toVersion, const MaterialVersionUpdate& materialVersionUpdate) { m_asset->m_materialVersionUpdateMap[toVersion] = materialVersionUpdate; } diff --git a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialVersionUpdate.cpp b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialVersionUpdate.cpp index fb2143d020..faa571e403 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialVersionUpdate.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialVersionUpdate.cpp @@ -7,6 +7,7 @@ */ #include +#include #include namespace AZ @@ -40,16 +41,59 @@ namespace AZ { } - MaterialVersionUpdate::Action::Action(const AZStd::string& operation, const AZStd::initializer_list>& args) + uint32_t MaterialVersionUpdate::GetVersion() const + { + return m_toVersion; + } + + void MaterialVersionUpdate::SetVersion(uint32_t toVersion) + { + m_toVersion = toVersion; + } + + void MaterialVersionUpdate::ApplyVersionUpdates(MaterialAsset& materialAsset) const + { + // collect all renames within this version update + AZStd::unordered_map renameToFrom; + for (const auto& action : m_actions) + { + const AZ::Name from = action.m_argsMap.find(AZ::Name{ "from" })->second; + const AZ::Name to = action.m_argsMap.find(AZ::Name{ "to" })->second; + + renameToFrom[from] = to; + } + + // apply rename actions + for (auto& propertyName : materialAsset.m_propertyNames) + { + const auto toFromIterator = renameToFrom.find(propertyName); + if (toFromIterator != renameToFrom.end()) + { + propertyName = AZ::Name{ toFromIterator->second }; + } + } + } + + const AZ::RPI::MaterialVersionUpdate::Actions& MaterialVersionUpdate::GetActions() const + { + return m_actions; + } + + void MaterialVersionUpdate::AddAction(const Action& action) + { + m_actions.push_back(action); + } + + MaterialVersionUpdate::Action::Action(const AZ::Name& operation, const AZStd::initializer_list>& args) : m_operation(operation) { for (const auto& arg : args) { - AddArgs(arg.first, arg.second); + AddArg(arg.first, arg.second); } } - void MaterialVersionUpdate::Action::AddArgs(const AZStd::string& key, const AZStd::string& argument) + void MaterialVersionUpdate::Action::AddArg(const AZ::Name& key, const AZ::Name& argument) { m_argsMap[key] = argument; } diff --git a/Gems/Atom/RPI/Code/Tests/Material/MaterialAssetTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/MaterialAssetTests.cpp index ce223ceb35..3191797ded 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/MaterialAssetTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/MaterialAssetTests.cpp @@ -63,6 +63,17 @@ namespace UnitTest RPITestFixture::TearDown(); } + + void UpgradeAndValidateMaterialAsset(Data::Asset materialAsset, Data::Asset upgradedMaterialTypeAsset) + { + // Set materialTypeAsset to the upgraded version. + EXPECT_EQ(1, materialAsset->m_materialTypeVersion); + materialAsset->m_materialTypeAsset = upgradedMaterialTypeAsset; + materialAsset->ApplyVersionUpdates(); + + EXPECT_EQ(2, materialAsset->m_materialTypeVersion); + EXPECT_EQ(AZ::Name{ "MyBoolNext" }, materialAsset->m_propertyNames[0]); + } }; TEST_F(MaterialAssetTests, Basic) @@ -202,6 +213,48 @@ namespace UnitTest EXPECT_EQ(serializedAsset->GetPropertyValues()[8].GetValue>(), streamingImageAsset); } + TEST_F(MaterialAssetTests, UpgradeMaterialAsset) + { + auto materialSrgLayout = CreateCommonTestMaterialSrgLayout(); + + auto shaderAsset = CreateTestShaderAsset(Uuid::CreateRandom(), materialSrgLayout); + + Data::Asset testMaterialTypeAssetV1; + MaterialTypeAssetCreator materialTypeCreator; + materialTypeCreator.Begin(Uuid::CreateRandom()); + materialTypeCreator.AddShader(shaderAsset); + AddMaterialPropertyForSrg(materialTypeCreator, Name{ "MyBool" }, MaterialPropertyDataType::Bool, Name{ "m_bool" }); + materialTypeCreator.SetPropertyValue(Name{ "MyBool" }, true); + EXPECT_TRUE(materialTypeCreator.End(testMaterialTypeAssetV1)); + + // Construct the material asset with materialTypeAsset version 1 + Data::AssetId assetId(Uuid::CreateRandom()); + + MaterialAssetCreator creator; + creator.Begin(assetId, *testMaterialTypeAssetV1); + creator.SetPropertyValue(Name{ "MyBool" }, true); + Data::Asset materialAsset; + EXPECT_TRUE(creator.End(materialAsset)); + + // Prepare material type asset version 2 with the update actions + MaterialVersionUpdate versionUpdate(2); + versionUpdate.AddAction(MaterialVersionUpdate::Action(AZ::Name{ "rename" }, { + { Name{ "from" }, Name{ "MyBool" } }, + { Name{ "to" }, Name{ "MyBoolNext" } } })); + + Data::Asset testMaterialTypeAssetV2; + materialTypeCreator.Begin(Uuid::CreateRandom()); + materialTypeCreator.SetVersion(versionUpdate.GetVersion()); + materialTypeCreator.AddVersionUpdate(versionUpdate.GetVersion(), versionUpdate); + materialTypeCreator.AddShader(shaderAsset); + AddMaterialPropertyForSrg(materialTypeCreator, Name{ "MyBoolNext" }, MaterialPropertyDataType::Bool, Name{ "m_bool" }); + materialTypeCreator.SetPropertyValue(Name{ "MyBoolNext" }, true); + EXPECT_TRUE(materialTypeCreator.End(testMaterialTypeAssetV2)); + + // Upgrade the material asset with the materialTypeAsset v2 and verify + UpgradeAndValidateMaterialAsset(materialAsset, testMaterialTypeAssetV2); + } + TEST_F(MaterialAssetTests, Error_NoBegin) { Data::AssetId assetId(Uuid::CreateRandom()); diff --git a/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeAssetTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeAssetTests.cpp index b9774c84d9..6329ef8d48 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeAssetTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeAssetTests.cpp @@ -14,6 +14,7 @@ #include #include +#include #include #include #include @@ -153,6 +154,14 @@ namespace UnitTest MaterialTypeAssetCreator materialTypeCreator; materialTypeCreator.Begin(assetId); + // Version updates + MaterialVersionUpdate versionUpdate(2); + versionUpdate.AddAction(MaterialVersionUpdate::Action(AZ::Name{ "rename" }, { + { Name{ "from" }, Name{ "EnableSpecialPassPrev" } }, + { Name{ "to" }, Name{ "EnableSpecialPass" } } })); + materialTypeCreator.SetVersion(versionUpdate.GetVersion()); + materialTypeCreator.AddVersionUpdate(versionUpdate.GetVersion(), versionUpdate); + // Built-in shader materialTypeCreator.AddShader(m_testShaderAsset); @@ -198,7 +207,7 @@ namespace UnitTest { EXPECT_EQ(m_testMaterialSrgLayout, materialTypeAsset->GetMaterialSrgLayout()); EXPECT_EQ(5, materialTypeAsset->GetMaterialPropertiesLayout()->GetPropertyCount()); - + //EXPECT_EQ(2, materialTypeAsset->GetVersion()); // Check aliased properties const MaterialPropertyIndex colorIndex = materialTypeAsset->GetMaterialPropertiesLayout()->FindPropertyIndex(Name{ "MyColor" }); @@ -490,6 +499,32 @@ namespace UnitTest }); } + TEST_F(MaterialTypeAssetTests, Error_InvalidMaterialVersionUpdate) + { + Data::Asset materialTypeAsset; + + Data::AssetId assetId(Uuid::CreateRandom()); + + MaterialTypeAssetCreator materialTypeCreator; + materialTypeCreator.Begin(assetId); + + // Invalid version updates + MaterialVersionUpdate versionUpdate(2); + versionUpdate.AddAction(MaterialVersionUpdate::Action(AZ::Name{ "rename" }, { + { Name{ "from" }, Name{ "EnableSpecialPassPrev" } }, + { Name{ "to" }, Name{ "InvalidPropertyName" } } })); + materialTypeCreator.SetVersion(versionUpdate.GetVersion()); + materialTypeCreator.AddVersionUpdate(versionUpdate.GetVersion(), versionUpdate); + materialTypeCreator.AddShader(m_testShaderAsset); + + materialTypeCreator.BeginMaterialProperty(Name{ "EnableSpecialPass" }, MaterialPropertyDataType::Bool); + materialTypeCreator.EndMaterialProperty(); + + AZ_TEST_START_ASSERTTEST; + EXPECT_FALSE(materialTypeCreator.End(materialTypeAsset)); + AZ_TEST_STOP_ASSERTTEST(1); + EXPECT_EQ(1, materialTypeCreator.GetErrorCount()); + } TEST_F(MaterialTypeAssetTests, MaterialTypeWithNoSRGOrProperties) { diff --git a/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeSourceDataTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeSourceDataTests.cpp index 179dc7c966..7766c1f8d8 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeSourceDataTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeSourceDataTests.cpp @@ -978,8 +978,16 @@ namespace UnitTest const AZStd::string inputJson = R"( { "description": "This is a general description about the material", + "version": 2, + "versionUpdates": [ + { + "toVersion": 2, + "actions": [ + { "op": "rename", "from": "groupA.fooPrev", "to": "groupA.foo" } + ] + } + ], "propertyLayout": { - "version": 2, "groups": [ { "id": "groupA", @@ -1061,9 +1069,12 @@ namespace UnitTest JsonTestResult loadResult = LoadTestDataFromJson(material, inputJson); EXPECT_EQ(material.m_description, "This is a general description about the material"); - - EXPECT_EQ(material.m_propertyLayout.m_version, 2); - + EXPECT_EQ(material.m_version, 2); + EXPECT_EQ(material.m_versionUpdates.size(), 1); + EXPECT_EQ(material.m_versionUpdates[0].m_toVersion, 2); + EXPECT_EQ(material.m_versionUpdates[0].m_actions[0].m_operation, "rename"); + EXPECT_EQ(material.m_versionUpdates[0].m_actions[0].m_renameFrom, "groupA.fooPrev"); + EXPECT_EQ(material.m_versionUpdates[0].m_actions[0].m_renameTo, "groupA.foo"); EXPECT_EQ(material.m_propertyLayout.m_groups.size(), 2); EXPECT_TRUE(material.FindGroup("groupA") != nullptr); EXPECT_TRUE(material.FindGroup("groupB") != nullptr); diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp index d032f35e38..ab8f9ff1a4 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp @@ -231,7 +231,6 @@ namespace MaterialEditor // create source data from properties MaterialSourceData sourceData; - sourceData.m_propertyLayoutVersion = m_materialTypeSourceData.m_propertyLayout.m_version; sourceData.m_materialType = m_materialSourceData.m_materialType; sourceData.m_parentMaterial = m_materialSourceData.m_parentMaterial; @@ -303,7 +302,6 @@ namespace MaterialEditor // create source data from properties MaterialSourceData sourceData; - sourceData.m_propertyLayoutVersion = m_materialTypeSourceData.m_propertyLayout.m_version; sourceData.m_materialType = m_materialSourceData.m_materialType; sourceData.m_parentMaterial = m_materialSourceData.m_parentMaterial; @@ -374,7 +372,6 @@ namespace MaterialEditor // create source data from properties MaterialSourceData sourceData; - sourceData.m_propertyLayoutVersion = m_materialTypeSourceData.m_propertyLayout.m_version; sourceData.m_materialType = m_materialSourceData.m_materialType; // Only assign a parent path if the source was a .material diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentUtil.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentUtil.cpp index e7059ccdf7..65ddef47e5 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentUtil.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentUtil.cpp @@ -99,7 +99,6 @@ namespace AZ { // Construct the material source data object that will be exported AZ::RPI::MaterialSourceData exportData; - exportData.m_propertyLayoutVersion = editData.m_materialTypeSourceData.m_propertyLayout.m_version; // Converting absolute material paths to relative paths bool result = false;