birkeh/o3de
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Simplified the code around MaterialAsset::ApplyVersionUpdates()

Browse Source 
Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
This commit is contained in:
santorac
2021-10-21 00:07:36 -07:00
parent 1633ced656
commit 46061eb302
10 changed files with 119 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialTypeAsset.h
+3 -5
View File
@@ -126,10 +126,8 @@ namespace AZ
//! Returns the version of the MaterialTypeAsset.
uint32_t GetVersion() const;
//! 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.
MaterialVersionUpdate GetMaterialVersionUpdate(uint32_t toVersion) const;
const AZStd::vector<MaterialVersionUpdate>& GetMaterialVersionUpdateList() const { return m_materialVersionUpdates; }
private:
bool PostLoadInit() override;
@@ -175,7 +173,7 @@ namespace AZ
uint32_t m_version = 1;
//! Contains actions to perform for each material update version.
MaterialVersionUpdateMap m_materialVersionUpdateMap;
AZStd::vector<MaterialVersionUpdate> m_materialVersionUpdates;
};
class MaterialTypeAssetHandler : public AssetHandler<MaterialTypeAsset>
Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialTypeAssetCreator.h
+1 -1
View File
@@ -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, const MaterialVersionUpdate& materialVersionUpdate);
void AddVersionUpdate(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).
Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialVersionUpdate.h
-1
View File
@@ -56,6 +56,5 @@ namespace AZ
Actions m_actions;
};
using MaterialVersionUpdateMap = AZStd::map<uint32_t, MaterialVersionUpdate>;
} // namespace RPI
} // namespace AZ
Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp
+2 -2
View File
@@ -358,7 +358,7 @@ namespace AZ
for (const auto& versionUpdate : m_versionUpdates)
{
MaterialVersionUpdate materialVersionUpdate;
MaterialVersionUpdate materialVersionUpdate{versionUpdate.m_toVersion};
for (const auto& action : versionUpdate.m_actions)
{
if (action.m_operation == rename.GetStringView())
@@ -373,7 +373,7 @@ namespace AZ
materialTypeAssetCreator.ReportWarning("Unsupported material version update operation '%s'", action.m_operation.c_str());
}
}
materialTypeAssetCreator.AddVersionUpdate(versionUpdate.m_toVersion, materialVersionUpdate);
materialTypeAssetCreator.AddVersionUpdate(materialVersionUpdate);
}
}
Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAsset.cpp
+7 -5
View File
@@ -210,13 +210,15 @@ namespace AZ
bool changesWereApplied = false;
for (int i = 0; i < aznumeric_cast<int>(m_materialTypeAsset->GetVersion() - m_materialTypeVersion); ++i)
for (const MaterialVersionUpdate& versionUpdate : m_materialTypeAsset->GetMaterialVersionUpdateList())
{
const auto& versionUpdate = m_materialTypeAsset->GetMaterialVersionUpdate(m_materialTypeVersion + i + 1);
if (versionUpdate.ApplyVersionUpdates(*this))
if (m_materialTypeVersion < versionUpdate.GetVersion())
{
changesWereApplied = true;
if (versionUpdate.ApplyVersionUpdates(*this))
{
changesWereApplied = true;
m_materialTypeVersion = versionUpdate.GetVersion();
}
}
}
Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialTypeAsset.cpp
+1 -7
View File
@@ -47,7 +47,7 @@ namespace AZ
serializeContext->Class<MaterialTypeAsset, AZ::Data::AssetData>()
->Version(5) // Material version update
->Field("Version", &MaterialTypeAsset::m_version)
->Field("VersionUpdates", &MaterialTypeAsset::m_materialVersionUpdateMap)
->Field("VersionUpdates", &MaterialTypeAsset::m_materialVersionUpdates)
->Field("ShaderCollection", &MaterialTypeAsset::m_shaderCollection)
->Field("MaterialFunctors", &MaterialTypeAsset::m_materialFunctors)
->Field("MaterialSrgShaderIndex", &MaterialTypeAsset::m_materialSrgShaderIndex)
@@ -169,12 +169,6 @@ namespace AZ
return m_version;
}
MaterialVersionUpdate MaterialTypeAsset::GetMaterialVersionUpdate(uint32_t toVersion) const
{
const auto it = m_materialVersionUpdateMap.find(toVersion);
return it != m_materialVersionUpdateMap.end() ? it->second : MaterialVersionUpdate(toVersion);
}
void MaterialTypeAsset::SetReady()
{
m_status = AssetStatus::Ready;
Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialTypeAssetCreator.cpp
+29 -9
View File
@@ -102,18 +102,38 @@ namespace AZ
bool MaterialTypeAssetCreator::ValidateMaterialVersion()
{
AZStd::vector<AZ::Name> renamedPropertyNames;
const auto& materialVersionUpdate = m_asset->GetMaterialVersionUpdate(m_asset->m_version);
for (const auto& action : materialVersionUpdate.GetActions())
if (m_asset->m_materialVersionUpdates.empty())
{
return true;
}
uint32_t prevVersion = 0;
for(const MaterialVersionUpdate& versionUpdate : m_asset->m_materialVersionUpdates)
{
if (versionUpdate.GetVersion() <= prevVersion)
{
ReportError("Version updates are not sequential. See version update '%u'.", versionUpdate.GetVersion());
return false;
}
if (versionUpdate.GetVersion() > m_asset->m_version)
{
ReportError("Version updates go beyond the current material type version. See version update '%u'.", versionUpdate.GetVersion());
return false;
}
prevVersion = versionUpdate.GetVersion();
}
const auto& lastMaterialVersionUpdate = m_asset->m_materialVersionUpdates.back();
for (const auto& action : lastMaterialVersionUpdate.GetActions())
{
const auto propertyIndex = m_asset->m_materialPropertiesLayout->FindPropertyIndex(AZ::Name{ action.m_toPropertyId });
if (!propertyIndex.IsValid())
{
ReportError(AZStd::string::format(
"Renamed property '%s' not found in material property layout. Check that the property name has been "
ReportError("Renamed property '%s' not found in material property layout. Check that the property name has been "
"upgraded to the correct version",
action.m_toPropertyId.GetCStr())
.c_str());
action.m_toPropertyId.GetCStr());
return false;
}
@@ -150,9 +170,9 @@ namespace AZ
m_asset->m_version = version;
}
void MaterialTypeAssetCreator::AddVersionUpdate(uint32_t toVersion, const MaterialVersionUpdate& materialVersionUpdate)
void MaterialTypeAssetCreator::AddVersionUpdate(const MaterialVersionUpdate& materialVersionUpdate)
{
m_asset->m_materialVersionUpdateMap[toVersion] = materialVersionUpdate;
m_asset->m_materialVersionUpdates.push_back(materialVersionUpdate);
}
void MaterialTypeAssetCreator::ClaimShaderOptionOwnership(const Name& shaderOptionName)
Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialVersionUpdate.cpp
-2
View File
@@ -31,8 +31,6 @@ namespace AZ
->Field("ToVersion", &MaterialVersionUpdate::m_toVersion)
->Field("Actions", &MaterialVersionUpdate::m_actions)
;
serializeContext->RegisterGenericType<MaterialVersionUpdateMap>();
}
}
Gems/Atom/RPI/Code/Tests/Material/MaterialAssetTests.cpp
+1 -1
View File
@@ -250,7 +250,7 @@ namespace UnitTest
Data::Asset<MaterialTypeAsset> testMaterialTypeAssetV2;
materialTypeCreator.Begin(Uuid::CreateRandom());
materialTypeCreator.SetVersion(versionUpdate.GetVersion());
materialTypeCreator.AddVersionUpdate(versionUpdate.GetVersion(), versionUpdate);
materialTypeCreator.AddVersionUpdate(versionUpdate);
materialTypeCreator.AddShader(shaderAsset);
// Now we add the properties in a different order from before, and use the new name for MyInt.
AddMaterialPropertyForSrg(materialTypeCreator, Name{ "MyUInt" }, MaterialPropertyDataType::UInt, Name{ "m_uint" });
Gems/Atom/RPI/Code/Tests/Material/MaterialTypeAssetTests.cpp
+75 -3
View File
@@ -162,7 +162,7 @@ namespace UnitTest
Name{ "EnableSpecialPass" }
}));
materialTypeCreator.SetVersion(versionUpdate.GetVersion());
materialTypeCreator.AddVersionUpdate(versionUpdate.GetVersion(), versionUpdate);
materialTypeCreator.AddVersionUpdate(versionUpdate);
// Built-in shader
@@ -501,7 +501,7 @@ namespace UnitTest
});
}
TEST_F(MaterialTypeAssetTests, Error_InvalidMaterialVersionUpdate)
TEST_F(MaterialTypeAssetTests, Error_InvalidMaterialVersionUpdate_WrongName)
{
Data::Asset<MaterialTypeAsset> materialTypeAsset;
@@ -518,7 +518,7 @@ namespace UnitTest
Name{ "InvalidPropertyName" }
}));
materialTypeCreator.SetVersion(versionUpdate.GetVersion());
materialTypeCreator.AddVersionUpdate(versionUpdate.GetVersion(), versionUpdate);
materialTypeCreator.AddVersionUpdate(versionUpdate);
materialTypeCreator.AddShader(m_testShaderAsset);
materialTypeCreator.BeginMaterialProperty(Name{ "EnableSpecialPass" }, MaterialPropertyDataType::Bool);
@@ -529,6 +529,78 @@ namespace UnitTest
AZ_TEST_STOP_ASSERTTEST(1);
EXPECT_EQ(1, materialTypeCreator.GetErrorCount());
}
TEST_F(MaterialTypeAssetTests, Error_InvalidMaterialVersionUpdate_WrongOrder)
{
MaterialTypeAssetCreator materialTypeCreator;
materialTypeCreator.Begin(Uuid::CreateRandom());
materialTypeCreator.SetVersion(4);
materialTypeCreator.AddShader(m_testShaderAsset);
materialTypeCreator.BeginMaterialProperty(Name{ "d" }, MaterialPropertyDataType::Bool);
materialTypeCreator.EndMaterialProperty();
ErrorMessageFinder errorMessageFinder;
errorMessageFinder.AddExpectedErrorMessage("Version updates are not sequential. See version update '3'");
{
MaterialVersionUpdate versionUpdate(2);
versionUpdate.AddAction(MaterialVersionUpdate::RenamePropertyAction({Name{ "a" },Name{ "b" }}));
materialTypeCreator.AddVersionUpdate(versionUpdate);
}
{
MaterialVersionUpdate versionUpdate(4);
versionUpdate.AddAction(MaterialVersionUpdate::RenamePropertyAction({Name{ "b" },Name{ "c" }}));
materialTypeCreator.AddVersionUpdate(versionUpdate);
}
{
MaterialVersionUpdate versionUpdate(3);
versionUpdate.AddAction(MaterialVersionUpdate::RenamePropertyAction({Name{ "c" },Name{ "d" }}));
materialTypeCreator.AddVersionUpdate(versionUpdate);
}
Data::Asset<MaterialTypeAsset> materialTypeAsset;
EXPECT_FALSE(materialTypeCreator.End(materialTypeAsset));
errorMessageFinder.CheckExpectedErrorsFound();
EXPECT_EQ(1, materialTypeCreator.GetErrorCount());
}
TEST_F(MaterialTypeAssetTests, Error_InvalidMaterialVersionUpdate_GoesTooFar)
{
MaterialTypeAssetCreator materialTypeCreator;
materialTypeCreator.Begin(Uuid::CreateRandom());
materialTypeCreator.SetVersion(3);
materialTypeCreator.AddShader(m_testShaderAsset);
materialTypeCreator.BeginMaterialProperty(Name{ "d" }, MaterialPropertyDataType::Bool);
materialTypeCreator.EndMaterialProperty();
ErrorMessageFinder errorMessageFinder;
errorMessageFinder.AddExpectedErrorMessage("Version updates go beyond the current material type version. See version update '4'");
{
MaterialVersionUpdate versionUpdate(2);
versionUpdate.AddAction(MaterialVersionUpdate::RenamePropertyAction({Name{ "a" },Name{ "b" }}));
materialTypeCreator.AddVersionUpdate(versionUpdate);
}
{
MaterialVersionUpdate versionUpdate(4);
versionUpdate.AddAction(MaterialVersionUpdate::RenamePropertyAction({Name{ "b" },Name{ "c" }}));
materialTypeCreator.AddVersionUpdate(versionUpdate);
}
Data::Asset<MaterialTypeAsset> materialTypeAsset;
EXPECT_FALSE(materialTypeCreator.End(materialTypeAsset));
errorMessageFinder.CheckExpectedErrorsFound();
EXPECT_EQ(1, materialTypeCreator.GetErrorCount());
}
TEST_F(MaterialTypeAssetTests, MaterialTypeWithNoSRGOrProperties)
{