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

Reports warnings when a material version auto update is applied, notifying the user they should update their source data.

Browse Source 
Also improved the MaterialAssetTests UpgradeMaterialAsset() to focus on testing the inputs and outputs of the class rather than the private internal data.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
This commit is contained in:
santorac
2021-10-20 23:36:53 -07:00
parent 836d018de7
commit 1633ced656
7 changed files with 103 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialSourceData.h
+1
View File
@@ -66,6 +66,7 @@ namespace AZ
//! 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.
//! @return true if any changes were applied
bool ApplyVersionUpdates();
//! Creates a MaterialAsset from the MaterialSourceData content.
Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialVersionUpdate.h
+3 -1
View File
@@ -43,7 +43,9 @@ namespace AZ
uint32_t GetVersion() const;
void SetVersion(uint32_t toVersion);
void ApplyVersionUpdates(MaterialAsset& materialAsset) const;
//! Apply version updates to the given material asset.
//! @return true if any changes were made
bool ApplyVersionUpdates(MaterialAsset& materialAsset) const;
using Actions = AZStd::vector<RenamePropertyAction>;
const Actions& GetActions() const;
Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceData.cpp
+18 -2
View File
@@ -80,9 +80,16 @@ namespace AZ
{
return false;
}
MaterialTypeSourceData materialTypeSourceData = materialTypeSourceDataOutcome.TakeValue();
if (m_materialTypeVersion == materialTypeSourceData.m_version)
{
return false;
}
bool changesWereApplied = false;
// Note that the only kind of property update currently supported is rename...
for (auto& groupPair : m_properties)
@@ -97,6 +104,7 @@ namespace AZ
if (materialTypeSourceData.ApplyPropertyRenames(propertyId, m_materialTypeVersion))
{
newPropertyMap[propertyId.GetPropertyName().GetStringView()] = propertyPair.second;
changesWereApplied = true;
}
else
{
@@ -107,8 +115,16 @@ namespace AZ
propertyMap = newPropertyMap;
}
m_materialTypeVersion = materialTypeSourceData.m_version;
if (changesWereApplied)
{
AZ_Warning("MaterialSourceData", false,
"This material is based on version %u of '%s', but the material type is now at version %u. "
"Automatic updates are available. Consider updating the .material source file.",
m_materialTypeVersion, m_materialType.c_str(), materialTypeSourceData.m_version);
}
m_materialTypeVersion = materialTypeSourceData.m_version;
return true;
}
Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAsset.cpp
+26 -1
View File
@@ -105,11 +105,18 @@ namespace AZ
AZStd::array_view<MaterialPropertyValue> MaterialAsset::GetPropertyValues() const
{
// If property names are included, they are used to re-arrange the property value list to align with the
// MaterialPropertiesLayout. This realignment would be necessary if the material type is updated with
// a new property layout, and a corresponding material is not reprocessed by the AP and continues using the
// old property layout.
if (!m_propertyNames.empty())
{
const uint32_t materialTypeVersion = m_materialTypeAsset->GetVersion();
if (m_materialTypeVersion < materialTypeVersion)
{
// It is possible that the material type has had some properties renamed. If that's the case, and this material
// is still referencing the old property layout, we need to apply any auto updates to rename those properties
// before using them to realign the property values.
const_cast<MaterialAsset*>(this)->ApplyVersionUpdates();
}
@@ -196,11 +203,29 @@ namespace AZ
void MaterialAsset::ApplyVersionUpdates()
{
if (m_materialTypeVersion == m_materialTypeAsset->GetVersion())
{
return;
}
bool changesWereApplied = false;
for (int i = 0; i < aznumeric_cast<int>(m_materialTypeAsset->GetVersion() - m_materialTypeVersion); ++i)
{
const auto& versionUpdate = m_materialTypeAsset->GetMaterialVersionUpdate(m_materialTypeVersion + i + 1);
versionUpdate.ApplyVersionUpdates(*this);
if (versionUpdate.ApplyVersionUpdates(*this))
{
changesWereApplied = true;
}
}
if (changesWereApplied)
{
AZ_Warning("MaterialAsset", false,
"This material is based on version %u of '%s', but the material type is now at version %u. "
"Automatic updates are available. Consider updating the .material source file.",
m_materialTypeVersion, m_materialTypeAsset.ToString<AZStd::string>().c_str(), m_materialTypeAsset->GetVersion());
}
m_materialTypeVersion = m_materialTypeAsset->GetVersion();
Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialVersionUpdate.cpp
+6 -1
View File
@@ -51,8 +51,10 @@ namespace AZ
m_toVersion = toVersion;
}
void MaterialVersionUpdate::ApplyVersionUpdates(MaterialAsset& materialAsset) const
bool MaterialVersionUpdate::ApplyVersionUpdates(MaterialAsset& materialAsset) const
{
bool changesWereApplied = false;
for (auto& propertyName : materialAsset.m_propertyNames)
{
for (const auto& action : m_actions)
@@ -60,9 +62,12 @@ namespace AZ
if (propertyName == action.m_fromPropertyId)
{
propertyName = action.m_toPropertyId;
changesWereApplied = true;
}
}
}
return changesWereApplied;
}
const AZ::RPI::MaterialVersionUpdate::Actions& MaterialVersionUpdate::GetActions() const
Gems/Atom/RPI/Code/Tests/Material/MaterialAssetTests.cpp
+41 -17
View File
@@ -10,6 +10,7 @@
#include <Common/RPITestFixture.h>
#include <Common/SerializeTester.h>
#include <Common/ShaderAssetTestUtils.h>
#include <Common/ErrorMessageFinder.h>
#include <Material/MaterialAssetTestUtils.h>
#include <Atom/RPI.Reflect/Material/MaterialAsset.h>
@@ -64,15 +65,9 @@ namespace UnitTest
RPITestFixture::TearDown();
}
void UpgradeAndValidateMaterialAsset(Data::Asset<MaterialAsset> materialAsset, Data::Asset<MaterialTypeAsset> upgradedMaterialTypeAsset)
void ReplaceMaterialType(Data::Asset<MaterialAsset> materialAsset, Data::Asset<MaterialTypeAsset> 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]);
}
};
@@ -215,6 +210,10 @@ namespace UnitTest
TEST_F(MaterialAssetTests, UpgradeMaterialAsset)
{
// Here we test the main way that a material asset upgrade would be applied at runtime: A material type is updated to
// both rename a property *and* change the order in which properties appear in the layout. In this case, the new name
// must be identified and then that new name is used to find the appropriate index in the property layout.
auto materialSrgLayout = CreateCommonTestMaterialSrgLayout();
auto shaderAsset = CreateTestShaderAsset(Uuid::CreateRandom(), materialSrgLayout);
@@ -223,16 +222,20 @@ namespace UnitTest
MaterialTypeAssetCreator materialTypeCreator;
materialTypeCreator.Begin(Uuid::CreateRandom());
materialTypeCreator.AddShader(shaderAsset);
AddMaterialPropertyForSrg(materialTypeCreator, Name{ "MyBool" }, MaterialPropertyDataType::Bool, Name{ "m_bool" });
materialTypeCreator.SetPropertyValue(Name{ "MyBool" }, true);
AddMaterialPropertyForSrg(materialTypeCreator, Name{ "MyInt" }, MaterialPropertyDataType::Int, Name{ "m_int" });
AddMaterialPropertyForSrg(materialTypeCreator, Name{ "MyUInt" }, MaterialPropertyDataType::UInt, Name{ "m_uint" });
AddMaterialPropertyForSrg(materialTypeCreator, Name{ "MyFloat" }, MaterialPropertyDataType::Float, Name{ "m_float" });
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);
const bool includePropertyNames = true;
creator.Begin(assetId, *testMaterialTypeAssetV1, includePropertyNames);
creator.SetPropertyValue(Name{ "MyInt" }, 7);
creator.SetPropertyValue(Name{ "MyUInt" }, 8u);
creator.SetPropertyValue(Name{ "MyFloat" }, 9.0f);
Data::Asset<MaterialAsset> materialAsset;
EXPECT_TRUE(creator.End(materialAsset));
@@ -240,8 +243,8 @@ namespace UnitTest
MaterialVersionUpdate versionUpdate(2);
versionUpdate.AddAction(MaterialVersionUpdate::RenamePropertyAction(
{
Name{ "MyBool" },
Name{ "MyBoolNext" }
Name{ "MyInt" },
Name{ "MyIntRenamed" }
}));
Data::Asset<MaterialTypeAsset> testMaterialTypeAssetV2;
@@ -249,12 +252,33 @@ namespace UnitTest
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);
// 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" });
AddMaterialPropertyForSrg(materialTypeCreator, Name{ "MyFloat" }, MaterialPropertyDataType::Float, Name{ "m_float" });
AddMaterialPropertyForSrg(materialTypeCreator, Name{ "MyIntRenamed" }, MaterialPropertyDataType::Int, Name{ "m_int" });
EXPECT_TRUE(materialTypeCreator.End(testMaterialTypeAssetV2));
// Upgrade the material asset with the materialTypeAsset v2 and verify
UpgradeAndValidateMaterialAsset(materialAsset, testMaterialTypeAssetV2);
// This is our way of faking the idea that an old version of the MaterialAsset could be loaded with a new version of the MaterialTypeAsset.
ReplaceMaterialType(materialAsset, testMaterialTypeAssetV2);
// This can find errors and warnings, we are looking for a warning when the version update is applied
ErrorMessageFinder warningFinder;
warningFinder.AddExpectedErrorMessage("Automatic updates are available. Consider updating the .material source file");
// 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<int32_t>());
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();
myIntIndex = materialAsset->GetMaterialPropertiesLayout()->FindPropertyIndex(Name{"MyIntRenamed"});
EXPECT_EQ(2, myIntIndex.GetIndex());
EXPECT_EQ(7, materialAsset->GetPropertyValues()[myIntIndex.GetIndex()].GetValue<int32_t>());
}
TEST_F(MaterialAssetTests, Error_NoBegin)
Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp
+8
View File
@@ -10,6 +10,7 @@
#include <Common/RPITestFixture.h>
#include <Common/JsonTestUtils.h>
#include <Common/ShaderAssetTestUtils.h>
#include <Common/ErrorMessageFinder.h>
#include <Material/MaterialAssetTestUtils.h>
#include <Atom/RPI.Reflect/Material/MaterialAsset.h>
@@ -725,7 +726,10 @@ namespace UnitTest
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");
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.
@@ -739,6 +743,10 @@ namespace UnitTest
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_MaterialTypeVersionPartialUpdate)