Got the RPI unit tests building and working again after merge. There were some incorrectly resolved conflicts that I had to re-resolve, especially in MaterialTypeSourceData::CreateMaterialTypeAsset.

I removed support for property rename version updates in MaterialTypeSourceData (i.e. ApplyPropertyRenames) because MaterialSourceData serialization no longer loads material property definitions from the .materialtype file, per a recent change on the development branch. The unit tests were broken and it wasn't worth updating them since we don't need this functionality anymore. Material property renames and other version updates are now exclusively applied by the MaterialAsset class.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
This commit is contained in:
santorac
2022-01-26 12:19:49 -08:00
parent 76add4d0d9
commit 4312c636af
3 changed files with 38 additions and 486 deletions
@@ -132,32 +132,6 @@ namespace AZ
const float MaterialTypeSourceData::PropertyDefinition::DefaultMax = std::numeric_limits<float>::max();
const float MaterialTypeSourceData::PropertyDefinition::DefaultStep = 0.1f;
bool MaterialTypeSourceData::ApplyPropertyRenames(MaterialPropertyId& propertyId) const
{
bool renamed = false;
for (const VersionUpdateDefinition& versionUpdate : m_versionUpdates)
{
for (const VersionUpdatesRenameOperationDefinition& action : versionUpdate.m_actions)
{
if (action.m_operation == "rename")
{
if (action.m_renameFrom == propertyId.GetStringView())
{
propertyId = MaterialPropertyId::Parse(action.m_renameTo);
renamed = true;
}
}
else
{
AZ_Warning("Material source data", false, "Unsupported material version update operation '%s'", action.m_operation.c_str());
}
}
}
return renamed;
}
/*static*/ MaterialTypeSourceData::PropertySet* MaterialTypeSourceData::PropertySet::AddPropertySet(AZStd::string_view name, AZStd::vector<AZStd::unique_ptr<PropertySet>>& toPropertySetList)
{
auto iter = AZStd::find_if(toPropertySetList.begin(), toPropertySetList.end(), [name](const AZStd::unique_ptr<PropertySet>& existingPropertySet)
@@ -528,49 +502,6 @@ namespace AZ
return groupDefinitions;
}
// TODO: It looks like this function doesn't operate on MaterialTypeSourceData data, it belongs in MaterialUtils
bool MaterialTypeSourceData::ConvertPropertyValueToSourceDataFormat(const PropertyDefinition& propertyDefinition, MaterialPropertyValue& propertyValue) const
{
if (propertyDefinition.m_dataType == AZ::RPI::MaterialPropertyDataType::Enum && propertyValue.Is<uint32_t>())
{
const uint32_t index = propertyValue.GetValue<uint32_t>();
if (index >= propertyDefinition.m_enumValues.size())
{
AZ_Error("Material source data", false, "Invalid value for material enum property: '%s'.", propertyDefinition.m_name.c_str());
return false;
}
propertyValue = propertyDefinition.m_enumValues[index];
return true;
}
// Image asset references must be converted from asset IDs to a relative source file path
if (propertyDefinition.m_dataType == AZ::RPI::MaterialPropertyDataType::Image && propertyValue.Is<Data::Asset<ImageAsset>>())
{
const Data::Asset<ImageAsset>& imageAsset = propertyValue.GetValue<Data::Asset<ImageAsset>>();
Data::AssetInfo imageAssetInfo;
if (imageAsset.GetId().IsValid())
{
bool result = false;
AZStd::string rootFilePath;
const AZStd::string platformName = ""; // Empty for default
AzToolsFramework::AssetSystemRequestBus::BroadcastResult(result, &AzToolsFramework::AssetSystem::AssetSystemRequest::GetAssetInfoById,
imageAsset.GetId(), imageAsset.GetType(), platformName, imageAssetInfo, rootFilePath);
if (!result)
{
AZ_Error("Material source data", false, "Image asset could not be found for property: '%s'.", propertyDefinition.m_name.c_str());
return false;
}
}
propertyValue = imageAssetInfo.m_relativePath;
return true;
}
return true;
}
bool MaterialTypeSourceData::BuildPropertyList(
const AZStd::string& materialTypeSourceFilePath,
MaterialTypeAssetCreator& materialTypeAssetCreator,
@@ -651,15 +582,20 @@ namespace AZ
{
case MaterialPropertyDataType::Image:
{
Outcome<Data::Asset<ImageAsset>> imageAssetResult = MaterialUtils::GetImageAssetReference(materialTypeSourceFilePath, property->m_value.GetValue<AZStd::string>());
Data::Asset<ImageAsset> imageAsset;
if (imageAssetResult.IsSuccess())
MaterialUtils::GetImageAssetResult result = MaterialUtils::GetImageAssetReference(
imageAsset, materialTypeSourceFilePath, property->m_value.GetValue<AZStd::string>());
if (result == MaterialUtils::GetImageAssetResult::Missing)
{
materialTypeAssetCreator.SetPropertyValue(propertyId, imageAssetResult.GetValue());
materialTypeAssetCreator.ReportError(
"Material property '%s': Could not find the image '%s'", propertyId.GetCStr(),
property->m_value.GetValue<AZStd::string>().data());
}
else
{
materialTypeAssetCreator.ReportError("Material property '%s': Could not find the image '%s'", propertyId.GetCStr(), property->m_value.GetValue<AZStd::string>().data());
materialTypeAssetCreator.SetPropertyValue(propertyId, imageAsset);
}
}
break;
@@ -743,145 +679,6 @@ namespace AZ
}
Outcome<Data::Asset<MaterialTypeAsset>> MaterialTypeSourceData::CreateMaterialTypeAsset(Data::AssetId assetId, AZStd::string_view materialTypeSourceFilePath, bool elevateWarnings) const
{
MaterialTypeAssetCreator materialTypeAssetCreator;
materialTypeAssetCreator.SetElevateWarnings(elevateWarnings);
materialTypeAssetCreator.Begin(assetId);
// Used to gather all the UV streams used in this material type from its shaders in alphabetical order.
auto semanticComp = [](const RHI::ShaderSemantic& lhs, const RHI::ShaderSemantic& rhs) -> bool
{
return lhs.ToString() < rhs.ToString();
};
AZStd::set<RHI::ShaderSemantic, decltype(semanticComp)> uvsInThisMaterialType(semanticComp);
for (const ShaderVariantReferenceData& shaderRef : m_shaderCollection)
{
const auto& shaderFile = shaderRef.m_shaderFilePath;
auto shaderAssetResult = AssetUtils::LoadAsset<ShaderAsset>(materialTypeSourceFilePath, shaderFile, 0);
if (shaderAssetResult)
{
auto shaderAsset = shaderAssetResult.GetValue();
auto optionsLayout = shaderAsset->GetShaderOptionGroupLayout();
ShaderOptionGroup options{ optionsLayout };
for (auto& iter : shaderRef.m_shaderOptionValues)
{
if (!options.SetValue(iter.first, iter.second))
{
return Failure();
}
}
materialTypeAssetCreator.AddShader(
shaderAsset, options.GetShaderVariantId(),
shaderRef.m_shaderTag.IsEmpty() ? Uuid::CreateRandom().ToString<AZ::Name>() : shaderRef.m_shaderTag);
// Gather UV names
const ShaderInputContract& shaderInputContract = shaderAsset->GetInputContract();
for (const ShaderInputContract::StreamChannelInfo& channel : shaderInputContract.m_streamChannels)
{
const RHI::ShaderSemantic& semantic = channel.m_semantic;
if (semantic.m_name.GetStringView().starts_with(RHI::ShaderSemantic::UvStreamSemantic))
{
uvsInThisMaterialType.insert(semantic);
}
}
}
else
{
materialTypeAssetCreator.ReportError("Shader '%s' not found", shaderFile.data());
return Failure();
}
}
for (const AZStd::unique_ptr<PropertySet>& propertySet : m_propertyLayout.m_propertySets)
{
AZStd::vector<AZStd::string> propertyNameContext;
propertyNameContext.push_back(propertySet->m_name);
materialTypeAssetCreator.BeginMaterialProperty(propertyId.GetFullName(), property.m_dataType);
if (!success)
{
return Failure();
Data::Asset<ImageAsset> imageAsset;
MaterialUtils::GetImageAssetResult result = MaterialUtils::GetImageAssetReference(
Outcome<Data::Asset<ImageAsset>> imageAssetResult = MaterialUtils::GetImageAssetReference(materialTypeSourceFilePath, property.m_value.GetValue<AZStd::string>());
if (imageAssetResult.IsSuccess())
materialTypeAssetCreator.SetPropertyValue(propertyId.GetFullName(), imageAssetResult.GetValue());
"Material property '%s': Could not find the image '%s'", propertyId.GetCStr(),
property.m_value.GetValue<AZStd::string>().data());
materialTypeAssetCreator.ReportError("Material property '%s': Could not find the image '%s'", propertyId.GetFullName().GetCStr(), property.m_value.GetValue<AZStd::string>().data());
MaterialPropertyIndex propertyIndex = materialTypeAssetCreator.GetMaterialPropertiesLayout()->FindPropertyIndex(propertyId.GetFullName());
materialTypeAssetCreator.SetPropertyValue(propertyId.GetFullName(), enumValue);
materialTypeAssetCreator.SetPropertyValue(propertyId.GetFullName(), property.m_value);
}
}
// We cannot create the MaterialFunctor until after all the properties are added because
// CreateFunctor() may need to look up properties in the MaterialPropertiesLayout
for (auto& functorData : m_materialFunctorSourceData)
{
MaterialFunctorSourceData::FunctorResult result = functorData->CreateFunctor(
MaterialFunctorSourceData::RuntimeContext(
materialTypeSourceFilePath,
materialTypeAssetCreator.GetMaterialPropertiesLayout(),
materialTypeAssetCreator.GetMaterialShaderResourceGroupLayout(),
materialTypeAssetCreator.GetShaderCollection()
)
);
if (result.IsSuccess())
{
Ptr<MaterialFunctor>& functor = result.GetValue();
if (functor != nullptr)
{
materialTypeAssetCreator.AddMaterialFunctor(functor);
for (const AZ::Name& optionName : functorData->GetActualSourceData()->GetShaderOptionDependencies())
{
materialTypeAssetCreator.ClaimShaderOptionOwnership(optionName);
}
}
}
else
{
materialTypeAssetCreator.ReportError("Failed to create MaterialFunctor");
return Failure();
}
}
// Only add the UV mapping related to this material type.
for (const auto& uvInput : uvsInThisMaterialType)
{
// We may have cases where the uv map is empty or inconsistent (exported from other projects),
// So we use semantic if mapping is not found.
auto iter = m_uvNameMap.find(uvInput.ToString());
if (iter != m_uvNameMap.end())
{
materialTypeAssetCreator.AddUvName(uvInput, Name(iter->second));
}
else
{
materialTypeAssetCreator.AddUvName(uvInput, Name(uvInput.ToString()));
}
}
Data::Asset<MaterialTypeAsset> materialTypeAsset;
if (materialTypeAssetCreator.End(materialTypeAsset))
{
return Success(AZStd::move(materialTypeAsset));
}
else
{
return Failure();
}
}
Outcome<Data::Asset<MaterialTypeAsset>> MaterialTypeSourceData::CreateMaterialTypeAsset(Data::AssetId assetId, AZStd::string_view materialTypeSourceFilePath, bool elevateWarnings) const
{
MaterialTypeAssetCreator materialTypeAssetCreator;
@@ -970,108 +767,16 @@ namespace AZ
return Failure();
}
}
for (auto& groupIter : m_propertyLayout.m_properties)
for (const AZStd::unique_ptr<PropertySet>& propertySet : m_propertyLayout.m_propertySets)
{
const AZStd::string& groupName = groupIter.first;
AZStd::vector<AZStd::string> propertyNameContext;
propertyNameContext.push_back(propertySet->m_name);
bool success = BuildPropertyList(materialTypeSourceFilePath, materialTypeAssetCreator, propertyNameContext, propertySet.get());
for (const PropertyDefinition& property : groupIter.second)
if (!success)
{
// Register the property...
MaterialPropertyId propertyId{ groupName, property.m_name };
if (!propertyId.IsValid())
{
materialTypeAssetCreator.ReportWarning("Cannot create material property with invalid ID '%s'.", propertyId.GetCStr());
continue;
}
materialTypeAssetCreator.BeginMaterialProperty(propertyId, property.m_dataType);
if (property.m_dataType == MaterialPropertyDataType::Enum)
{
materialTypeAssetCreator.SetMaterialPropertyEnumNames(property.m_enumValues);
}
for (auto& output : property.m_outputConnections)
{
switch (output.m_type)
{
case MaterialPropertyOutputType::ShaderInput:
materialTypeAssetCreator.ConnectMaterialPropertyToShaderInput(Name{ output.m_fieldName.data() });
break;
case MaterialPropertyOutputType::ShaderOption:
if (output.m_shaderIndex >= 0)
{
materialTypeAssetCreator.ConnectMaterialPropertyToShaderOption(Name{ output.m_fieldName.data() }, output.m_shaderIndex);
}
else
{
materialTypeAssetCreator.ConnectMaterialPropertyToShaderOptions(Name{ output.m_fieldName.data() });
}
break;
case MaterialPropertyOutputType::Invalid:
// Don't add any output mappings, this is the case when material functors are expected to process the property
break;
default:
AZ_Assert(false, "Unsupported MaterialPropertyOutputType");
return Failure();
}
}
materialTypeAssetCreator.EndMaterialProperty();
// Parse and set the property's value...
if (!property.m_value.IsValid())
{
AZ_Warning("Material source data", false, "Source data for material property value is invalid.");
}
else
{
switch (property.m_dataType)
{
case MaterialPropertyDataType::Image:
{
Data::Asset<ImageAsset> imageAsset;
MaterialUtils::GetImageAssetResult result = MaterialUtils::GetImageAssetReference(
imageAsset, materialTypeSourceFilePath, property.m_value.GetValue<AZStd::string>());
if (result == MaterialUtils::GetImageAssetResult::Missing)
{
materialTypeAssetCreator.ReportError(
"Material property '%s': Could not find the image '%s'", propertyId.GetCStr(),
property.m_value.GetValue<AZStd::string>().data());
}
else
{
materialTypeAssetCreator.SetPropertyValue(propertyId, imageAsset);
}
}
break;
case MaterialPropertyDataType::Enum:
{
MaterialPropertyIndex propertyIndex = materialTypeAssetCreator.GetMaterialPropertiesLayout()->FindPropertyIndex(propertyId);
const MaterialPropertyDescriptor* propertyDescriptor = materialTypeAssetCreator.GetMaterialPropertiesLayout()->GetPropertyDescriptor(propertyIndex);
AZ::Name enumName = AZ::Name(property.m_value.GetValue<AZStd::string>());
uint32_t enumValue = propertyDescriptor ? propertyDescriptor->GetEnumValue(enumName) : MaterialPropertyDescriptor::InvalidEnumValue;
if (enumValue == MaterialPropertyDescriptor::InvalidEnumValue)
{
materialTypeAssetCreator.ReportError("Enum value '%s' couldn't be found in the 'enumValues' list", enumName.GetCStr());
}
else
{
materialTypeAssetCreator.SetPropertyValue(propertyId, enumValue);
}
}
break;
default:
materialTypeAssetCreator.SetPropertyValue(propertyId, property.m_value);
break;
}
}
return Failure();
}
}
@@ -93,20 +93,23 @@ namespace UnitTest
{
"version": 10,
"propertyLayout": {
"properties": {
"general": [
{"name": "MyBool", "type": "bool"},
{"name": "MyInt", "type": "Int"},
{"name": "MyUInt", "type": "UInt"},
{"name": "MyFloat", "type": "Float"},
{"name": "MyFloat2", "type": "Vector2"},
{"name": "MyFloat3", "type": "Vector3"},
{"name": "MyFloat4", "type": "Vector4"},
{"name": "MyColor", "type": "Color"},
{"name": "MyImage", "type": "Image"},
{"name": "MyEnum", "type": "Enum", "enumValues": ["Enum0", "Enum1", "Enum2"], "defaultValue": "Enum0"}
]
}
"propertySets": [
{
"name": "general",
"properties": [
{"name": "MyBool", "type": "bool"},
{"name": "MyInt", "type": "Int"},
{"name": "MyUInt", "type": "UInt"},
{"name": "MyFloat", "type": "Float"},
{"name": "MyFloat2", "type": "Vector2"},
{"name": "MyFloat3", "type": "Vector3"},
{"name": "MyFloat4", "type": "Vector4"},
{"name": "MyColor", "type": "Color"},
{"name": "MyImage", "type": "Image"},
{"name": "MyEnum", "type": "Enum", "enumValues": ["Enum0", "Enum1", "Enum2"], "defaultValue": "Enum0"}
]
}
]
},
"shaders": [
{
@@ -580,18 +583,12 @@ namespace UnitTest
errorMessageFinder.Reset();
errorMessageFinder.AddExpectedErrorMessage("Could not find asset [DoesNotExist.materialtype]");
"properties": {
[
{
"general": [
"properties": [
]
result = material.CreateMaterialAsset(AZ::Uuid::CreateRandom(), "test.material", AZ::RPI::MaterialAssetProcessingMode::PreBake, elevateWarnings);
EXPECT_FALSE(result.IsSuccess());
errorMessageFinder.CheckExpectedErrorsFound();
errorMessageFinder.Reset();
EXPECT_TRUE(loadResult.ContainsMessage("[simpleMaterialType.materialtype]/propertyLayout/properties", "Successfully read"));
errorMessageFinder.AddExpectedErrorMessage("Could not find asset [DoesNotExist.materialtype]");
errorMessageFinder.AddIgnoredErrorMessage("Failed to create material type asset ID", true);
result = material.CreateMaterialAssetFromSourceData(AZ::Uuid::CreateRandom(), "test.material", elevateWarnings);
EXPECT_FALSE(result.IsSuccess());
@@ -600,12 +597,6 @@ namespace UnitTest
TEST_F(MaterialSourceDataTests, CreateMaterialAsset_MaterialPropertyNotFound)
{
"properties": {
[
{
"general": [
"properties": [
]
MaterialSourceData material;
material.m_materialType = "@exefolder@/Temp/test.materialtype";
AddPropertyGroup(material, "general");
@@ -1911,157 +1911,13 @@ namespace UnitTest
}
TEST_F(MaterialTypeSourceDataTests, FindPropertyUsingOldName)
{
const AZStd::string inputJson = R"(
{
"version": 10,
"versionUpdates": [
{
"toVersion": 2,
"actions": [
{ "op": "rename", "from": "general.fooA", "to": "general.fooB" }
]
},
{
"toVersion": 4,
"actions": [
{ "op": "rename", "from": "general.barA", "to": "general.barB" }
]
},
{
"toVersion": 6,
"actions": [
{ "op": "rename", "from": "general.fooB", "to": "general.fooC" },
{ "op": "rename", "from": "general.barB", "to": "general.barC" }
]
},
{
"toVersion": 7,
"actions": [
{ "op": "rename", "from": "general.bazA", "to": "otherGroup.bazB" },
{ "op": "rename", "from": "onlyOneProperty.bopA", "to": "otherGroup.bopB" } // This tests a group 'onlyOneProperty' that no longer exists in the material type
]
}
],
"propertyLayout": {
"properties": {
"general": [
{
"name": "fooC",
"type": "Bool"
},
{
"name": "barC",
"type": "Float"
}
],
"otherGroup": [
{
"name": "dontMindMe",
"type": "Bool"
},
{
"name": "bazB",
"type": "Float"
},
{
"name": "bopB",
"type": "Float"
}
]
}
}
}
)";
MaterialTypeSourceData materialType;
JsonTestResult loadResult = LoadTestDataFromJson(materialType, inputJson);
EXPECT_EQ(materialType.m_version, 10);
// First find the properties using their correct current names
const MaterialTypeSourceData::PropertyDefinition* foo = materialType.FindProperty("general", "fooC");
const MaterialTypeSourceData::PropertyDefinition* bar = materialType.FindProperty("general", "barC");
const MaterialTypeSourceData::PropertyDefinition* baz = materialType.FindProperty("otherGroup", "bazB");
const MaterialTypeSourceData::PropertyDefinition* bop = materialType.FindProperty("otherGroup", "bopB");
EXPECT_TRUE(foo);
EXPECT_TRUE(bar);
EXPECT_TRUE(baz);
EXPECT_TRUE(bop);
EXPECT_EQ(foo->m_name, "fooC");
EXPECT_EQ(bar->m_name, "barC");
EXPECT_EQ(baz->m_name, "bazB");
EXPECT_EQ(bop->m_name, "bopB");
// Now try doing the property lookup using old versions of the name and make sure the same property can be found
EXPECT_EQ(foo, materialType.FindProperty("general", "fooA"));
EXPECT_EQ(foo, materialType.FindProperty("general", "fooB"));
EXPECT_EQ(bar, materialType.FindProperty("general", "barA"));
EXPECT_EQ(bar, materialType.FindProperty("general", "barB"));
EXPECT_EQ(baz, materialType.FindProperty("general", "bazA"));
EXPECT_EQ(bop, materialType.FindProperty("onlyOneProperty", "bopA"));
EXPECT_EQ(nullptr, materialType.FindProperty("general", "fooX"));
EXPECT_EQ(nullptr, materialType.FindProperty("general", "barX"));
EXPECT_EQ(nullptr, materialType.FindProperty("general", "bazX"));
EXPECT_EQ(nullptr, materialType.FindProperty("general", "bazB"));
EXPECT_EQ(nullptr, materialType.FindProperty("otherGroup", "bazA"));
EXPECT_EQ(nullptr, materialType.FindProperty("onlyOneProperty", "bopB"));
EXPECT_EQ(nullptr, materialType.FindProperty("otherGroup", "bopA"));
}
TEST_F(MaterialTypeSourceDataTests, FindPropertyUsingOldName_Error_UnsupportedVersionUpdate)
{
const AZStd::string inputJson = R"(
{
"version": 10,
"versionUpdates": [
{
"toVersion": 2,
"actions": [
{ "op": "notRename", "from": "general.fooA", "to": "general.fooB" }
]
}
],
"propertyLayout": {
"properties": {
"general": [
{
"name": "fooB",
"type": "Bool"
}
]
}
}
}
)";
MaterialTypeSourceData materialType;
JsonTestResult loadResult = LoadTestDataFromJson(materialType, inputJson);
ErrorMessageFinder errorMessageFinder;
errorMessageFinder.AddExpectedErrorMessage("Unsupported material version update operation 'notRename'");
const MaterialTypeSourceData::PropertyDefinition* foo = materialType.FindProperty("general", "fooA");
EXPECT_EQ(nullptr, foo);
errorMessageFinder.CheckExpectedErrorsFound();
}
TEST_F(MaterialTypeSourceDataTests, CreateMaterialTypeAsset_Error_UnsupportedVersionUpdate)
{
MaterialTypeSourceData sourceData;
MaterialTypeSourceData::PropertyDefinition propertySource;
propertySource.m_name = "a";
propertySource.m_dataType = MaterialPropertyDataType::Int;
propertySource.m_value = 0;
sourceData.m_propertyLayout.m_properties["general"].push_back(propertySource);
MaterialTypeSourceData::PropertyDefinition* propertySource = sourceData.AddPropertySet("general")->AddProperty("a");
propertySource->m_dataType = MaterialPropertyDataType::Int;
propertySource->m_value = 0;
sourceData.m_version = 2;