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

Updated MaterialPropertyId class in preparation for nested material property sets.

Browse Source 
Here the class has been generalized for a list of group names and a final property name, rather than assuming a single group containing the property. This included removing the unused GetPropertyName and GetGroupName functions. All that's really need from this class is conversion to a full property ID string.

Testing:
New unit test.
Reprocessed all core material types and StandardPBR test materials used in Atom Sample Viewer's material screenshot test.
Atom Sample Viewer material screenshot test script.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
This commit is contained in:
santorac
2021-09-18 23:57:52 -07:00
parent 5ed7e91a62
commit 02acf2f65e
10 changed files with 210 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialPropertyId.h
+9 -9
View File
@@ -9,6 +9,7 @@
#pragma once
#include <AzCore/Name/Name.h>
#include <AtomCore/std/containers/array_view.h>
namespace AZ
{
@@ -16,27 +17,28 @@ namespace AZ
{
class MaterialAsset;
//! Utility for building material property names consisting of a group name and a property sub-name.
//! Represented as "[groupName].[propertyName]".
//! The group name is optional, in which case the ID will just be "[propertyName]".
//! Utility for building material property IDs.
//! These IDs are represented like "groupA.groupB.[...].propertyName".
//! The groups are optional, in which case the full property ID will just be like "propertyName".
class MaterialPropertyId
{
public:
static bool IsValidName(AZStd::string_view name);
static bool IsValidName(const AZ::Name& name);
//! Creates a MaterialPropertyId from a full name string like "[groupName].[propertyName]" or just "[propertyName]"
//! Creates a MaterialPropertyId from a full name string like "groupA.groupB.[...].propertyName" or just "propertyName".
//! Also checks the name for validity.
static MaterialPropertyId Parse(AZStd::string_view fullPropertyId);
MaterialPropertyId() = default;
explicit MaterialPropertyId(AZStd::string_view propertyName);
MaterialPropertyId(AZStd::string_view groupName, AZStd::string_view propertyName);
MaterialPropertyId(const Name& groupName, const Name& propertyName);
MaterialPropertyId(const AZStd::array_view<AZStd::string> names);
AZ_DEFAULT_COPY_MOVE(MaterialPropertyId);
const Name& GetGroupName() const;
const Name& GetPropertyName() const;
const Name& GetFullName() const;
operator const Name&() const;
//! Returns a pointer to the full name ("[groupName].[propertyName]").
//! This is included for convenience so it can be used for error messages in the same way an AZ::Name is used.
@@ -52,8 +54,6 @@ namespace AZ
private:
Name m_fullName;
Name m_groupName;
Name m_propertyName;
};
} // namespace RPI
Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialPropertyId.cpp
+52 -31
View File
@@ -27,63 +27,84 @@ namespace AZ
bool MaterialPropertyId::IsValid() const
{
const bool groupNameIsValid = m_groupName.IsEmpty() || IsValidName(m_groupName);
const bool propertyNameIsValid = IsValidName(m_propertyName);
return groupNameIsValid && propertyNameIsValid;
return !m_fullName.IsEmpty();
}
MaterialPropertyId MaterialPropertyId::Parse(AZStd::string_view fullPropertyId)
{
AZStd::vector<AZStd::string> tokens;
AzFramework::StringFunc::Tokenize(fullPropertyId.data(), tokens, '.', true, true);
AzFramework::StringFunc::Tokenize(fullPropertyId, tokens, '.', true, true);
if (tokens.size() == 1)
if (tokens.empty())
{
return MaterialPropertyId{"", tokens[0]};
AZ_Error("MaterialPropertyId", false, "Property ID is empty.", fullPropertyId.data());
return MaterialPropertyId{};
}
else if (tokens.size() == 2)
for (const auto& token : tokens)
{
return MaterialPropertyId{tokens[0], tokens[1]};
if (!IsValidName(token))
{
AZ_Error("MaterialPropertyId", false, "Property ID '%.*s' is not a valid identifier.", AZ_STRING_ARG(fullPropertyId));
return MaterialPropertyId{};
}
}
MaterialPropertyId id;
id.m_fullName = fullPropertyId;
return id;
}
MaterialPropertyId::MaterialPropertyId(AZStd::string_view propertyName)
{
if (!IsValidName(propertyName))
{
AZ_Error("MaterialPropertyId", false, "Property name '%.*s' is not a valid identifier.", AZ_STRING_ARG(propertyName));
}
else
{
AZ_Error("MaterialPropertyId", false, "Property ID '%s' is not a valid identifier.", fullPropertyId.data());
return MaterialPropertyId{};
m_fullName = propertyName;
}
}
MaterialPropertyId::MaterialPropertyId(AZStd::string_view groupName, AZStd::string_view propertyName)
: MaterialPropertyId(Name{groupName}, Name{propertyName})
{
}
MaterialPropertyId::MaterialPropertyId(const Name& groupName, const Name& propertyName)
{
AZ_Error("MaterialPropertyId", groupName.IsEmpty() || IsValidName(groupName), "Group name '%s' is not a valid identifier.", groupName.GetCStr());
AZ_Error("MaterialPropertyId", IsValidName(propertyName), "Property name '%s' is not a valid identifier.", propertyName.GetCStr());
m_groupName = groupName;
m_propertyName = propertyName;
if (groupName.IsEmpty())
if (!IsValidName(groupName))
{
m_fullName = m_propertyName.GetStringView();
AZ_Error("MaterialPropertyId", false, "Group name '%.*s' is not a valid identifier.", AZ_STRING_ARG(groupName));
}
else if (!IsValidName(propertyName))
{
AZ_Error("MaterialPropertyId", false, "Property name '%.*s' is not a valid identifier.", AZ_STRING_ARG(propertyName));
}
else
{
m_fullName = AZStd::string::format("%s.%s", m_groupName.GetCStr(), m_propertyName.GetCStr());
m_fullName = AZStd::string::format("%.*s.%.*s", AZ_STRING_ARG(groupName), AZ_STRING_ARG(propertyName));
}
}
const Name& MaterialPropertyId::GetGroupName() const
MaterialPropertyId::MaterialPropertyId(const Name& groupName, const Name& propertyName)
: MaterialPropertyId(groupName.GetStringView(), propertyName.GetStringView())
{
return m_groupName;
}
MaterialPropertyId::MaterialPropertyId(const AZStd::array_view<AZStd::string> names)
{
for (const auto& name : names)
{
if (!IsValidName(name))
{
AZ_Error("MaterialPropertyId", false, "'%s' is not a valid identifier.", name.c_str());
return;
}
}
AZStd::string fullName;
AzFramework::StringFunc::Join(fullName, names.begin(), names.end(), ".");
m_fullName = fullName;
}
const Name& MaterialPropertyId::GetPropertyName() const
{
return m_propertyName;
}
const Name& MaterialPropertyId::GetFullName() const
MaterialPropertyId::operator const Name&() const
{
return m_fullName;
}
Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp
+6 -6
View File
@@ -400,7 +400,7 @@ namespace AZ
continue;
}
materialTypeAssetCreator.BeginMaterialProperty(propertyId.GetFullName(), property.m_dataType);
materialTypeAssetCreator.BeginMaterialProperty(propertyId, property.m_dataType);
if (property.m_dataType == MaterialPropertyDataType::Enum)
{
@@ -454,18 +454,18 @@ namespace AZ
if (result == MaterialUtils::GetImageAssetResult::Missing)
{
materialTypeAssetCreator.ReportError(
"Material property '%s': Could not find the image '%s'", propertyId.GetFullName().GetCStr(),
"Material property '%s': Could not find the image '%s'", propertyId.GetCStr(),
property.m_value.GetValue<AZStd::string>().data());
}
else
{
materialTypeAssetCreator.SetPropertyValue(propertyId.GetFullName(), imageAsset);
materialTypeAssetCreator.SetPropertyValue(propertyId, imageAsset);
}
}
break;
case MaterialPropertyDataType::Enum:
{
MaterialPropertyIndex propertyIndex = materialTypeAssetCreator.GetMaterialPropertiesLayout()->FindPropertyIndex(propertyId.GetFullName());
MaterialPropertyIndex propertyIndex = materialTypeAssetCreator.GetMaterialPropertiesLayout()->FindPropertyIndex(propertyId);
const MaterialPropertyDescriptor* propertyDescriptor = materialTypeAssetCreator.GetMaterialPropertiesLayout()->GetPropertyDescriptor(propertyIndex);
AZ::Name enumName = AZ::Name(property.m_value.GetValue<AZStd::string>());
@@ -476,12 +476,12 @@ namespace AZ
}
else
{
materialTypeAssetCreator.SetPropertyValue(propertyId.GetFullName(), enumValue);
materialTypeAssetCreator.SetPropertyValue(propertyId, enumValue);
}
}
break;
default:
materialTypeAssetCreator.SetPropertyValue(propertyId.GetFullName(), property.m_value);
materialTypeAssetCreator.SetPropertyValue(propertyId, property.m_value);
break;
}
}
Gems/Atom/RPI/Code/Tests/Material/MaterialPropertyIdTests.cpp
+131
View File
@@ -0,0 +1,131 @@
/*
* Copyright (c) Contributors to the Open 3D Engine Project.
* For complete copyright and license terms please see the LICENSE at the root of this distribution.
*
* SPDX-License-Identifier: Apache-2.0 OR MIT
*
*/
#include <AzTest/AzTest.h>
#include <Common/RPITestFixture.h>
#include <Common/ErrorMessageFinder.h>
#include <Atom/RPI.Edit/Material/MaterialPropertyId.h>
namespace UnitTest
{
using namespace AZ;
using namespace RPI;
class MaterialPropertyIdTests
: public RPITestFixture
{
};
TEST_F(MaterialPropertyIdTests, TestConstructWithPropertyName)
{
MaterialPropertyId id{"color"};
EXPECT_TRUE(id.IsValid());
EXPECT_STREQ(id.GetCStr(), "color");
AZ::Name idCastedToName = id;
EXPECT_EQ(idCastedToName, AZ::Name{"color"});
}
TEST_F(MaterialPropertyIdTests, TestConstructWithPropertyName_BadName)
{
ErrorMessageFinder errorMessageFinder;
errorMessageFinder.AddExpectedErrorMessage("not a valid identifier");
MaterialPropertyId id{"color?"};
EXPECT_FALSE(id.IsValid());
errorMessageFinder.CheckExpectedErrorsFound();
}
TEST_F(MaterialPropertyIdTests, TestConstructWithTwoNames)
{
MaterialPropertyId id{"baseColor", "factor"};
EXPECT_TRUE(id.IsValid());
EXPECT_STREQ(id.GetCStr(), "baseColor.factor");
AZ::Name idCastedToName = id;
EXPECT_EQ(idCastedToName, AZ::Name{"baseColor.factor"});
}
TEST_F(MaterialPropertyIdTests, TestConstructWithTwoNames_BadGroupName)
{
ErrorMessageFinder errorMessageFinder;
errorMessageFinder.AddExpectedErrorMessage("not a valid identifier");
MaterialPropertyId id{"layer1.baseColor", "factor"};
EXPECT_FALSE(id.IsValid());
errorMessageFinder.CheckExpectedErrorsFound();
}
TEST_F(MaterialPropertyIdTests, TestConstructWithTwoNames_BadPropertyName)
{
ErrorMessageFinder errorMessageFinder;
errorMessageFinder.AddExpectedErrorMessage("not a valid identifier");
MaterialPropertyId id{"baseColor", ".factor"};
EXPECT_FALSE(id.IsValid());
errorMessageFinder.CheckExpectedErrorsFound();
}
TEST_F(MaterialPropertyIdTests, TestConstructWithMultipleNames)
{
AZStd::vector<AZStd::string> names{"layer1", "clearCoat", "normal", "factor"};
MaterialPropertyId id{names};
EXPECT_TRUE(id.IsValid());
EXPECT_STREQ(id.GetCStr(), "layer1.clearCoat.normal.factor");
AZ::Name idCastedToName = id;
EXPECT_EQ(idCastedToName, AZ::Name{"layer1.clearCoat.normal.factor"});
}
TEST_F(MaterialPropertyIdTests, TestConstructWithMultipleNames_BadName)
{
ErrorMessageFinder errorMessageFinder;
errorMessageFinder.AddExpectedErrorMessage("not a valid identifier");
AZStd::vector<AZStd::string> names{"layer1", "clear-coat", "normal", "factor"};
MaterialPropertyId id{names};
EXPECT_FALSE(id.IsValid());
errorMessageFinder.CheckExpectedErrorsFound();
}
TEST_F(MaterialPropertyIdTests, TestParse)
{
MaterialPropertyId id = MaterialPropertyId::Parse("layer1.clearCoat.normal.factor");
EXPECT_TRUE(id.IsValid());
EXPECT_STREQ(id.GetCStr(), "layer1.clearCoat.normal.factor");
AZ::Name idCastedToName = id;
EXPECT_EQ(idCastedToName, AZ::Name{"layer1.clearCoat.normal.factor"});
}
TEST_F(MaterialPropertyIdTests, TestParse_BadName)
{
ErrorMessageFinder errorMessageFinder;
errorMessageFinder.AddExpectedErrorMessage("not a valid identifier");
MaterialPropertyId id = MaterialPropertyId::Parse("layer1.clearCoat.normal,factor");
EXPECT_FALSE(id.IsValid());
errorMessageFinder.CheckExpectedErrorsFound();
}
TEST_F(MaterialPropertyIdTests, TestNameValidity)
{
EXPECT_TRUE(MaterialPropertyId::IsValidName("a"));
EXPECT_TRUE(MaterialPropertyId::IsValidName("z"));
EXPECT_TRUE(MaterialPropertyId::IsValidName("A"));
EXPECT_TRUE(MaterialPropertyId::IsValidName("Z"));
EXPECT_TRUE(MaterialPropertyId::IsValidName("_"));
EXPECT_TRUE(MaterialPropertyId::IsValidName("m_layer10bazBAZ"));
EXPECT_FALSE(MaterialPropertyId::IsValidName(""));
EXPECT_FALSE(MaterialPropertyId::IsValidName("1layer"));
EXPECT_FALSE(MaterialPropertyId::IsValidName("base-color"));
EXPECT_FALSE(MaterialPropertyId::IsValidName("base.color"));
EXPECT_FALSE(MaterialPropertyId::IsValidName("base/color"));
}
}
Gems/Atom/RPI/Code/atom_rpi_tests_files.cmake
+1
View File
@@ -37,6 +37,7 @@ set(FILES
Tests/Material/MaterialSourceDataTests.cpp
Tests/Material/MaterialFunctorTests.cpp
Tests/Material/MaterialFunctorSourceDataSerializerTests.cpp
Tests/Material/MaterialPropertyIdTests.cpp
Tests/Material/MaterialPropertyValueSourceDataTests.cpp
Tests/Material/MaterialTests.cpp
Tests/Model/ModelTests.cpp
Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp
+4 -4
View File
@@ -589,7 +589,7 @@ namespace MaterialEditor
const MaterialPropertyId propertyId(groupName, propertyName);
const auto it = m_properties.find(propertyId.GetFullName());
const auto it = m_properties.find(propertyId);
if (it != m_properties.end() && propertyFilter(it->second))
{
MaterialPropertyValue propertyValue = AtomToolsFramework::ConvertToRuntimeType(it->second.GetValue());
@@ -597,7 +597,7 @@ namespace MaterialEditor
{
if (!AtomToolsFramework::ConvertToExportFormat(exportPath, propertyId.GetFullName(), propertyDefinition, propertyValue))
{
AZ_Error("MaterialDocument", false, "Material document property could not be converted: '%s' in '%s'.", propertyId.GetFullName().GetCStr(), m_absolutePath.c_str());
AZ_Error("MaterialDocument", false, "Material document property could not be converted: '%s' in '%s'.", propertyId.GetCStr(), m_absolutePath.c_str());
result = false;
return false;
}
@@ -783,7 +783,7 @@ namespace MaterialEditor
AtomToolsFramework::DynamicPropertyConfig propertyConfig;
// Assign id before conversion so it can be used in dynamic description
propertyConfig.m_id = MaterialPropertyId(groupName, propertyName).GetCStr();
propertyConfig.m_id = MaterialPropertyId(groupName, propertyName);
const auto& propertyIndex = m_materialAsset->GetMaterialPropertiesLayout()->FindPropertyIndex(propertyConfig.m_id);
const bool propertyIndexInBounds = propertyIndex.IsValid() && propertyIndex.GetIndex() < m_materialAsset->GetPropertyValues().size();
@@ -854,7 +854,7 @@ namespace MaterialEditor
propertyConfig = {};
propertyConfig.m_dataType = AtomToolsFramework::DynamicPropertyType::String;
propertyConfig.m_id = MaterialPropertyId(UvGroupName, shaderInput).GetCStr();
propertyConfig.m_id = MaterialPropertyId(UvGroupName, shaderInput);
propertyConfig.m_name = shaderInput;
propertyConfig.m_displayName = shaderInput;
propertyConfig.m_groupName = "UV Sets";
Gems/Atom/Tools/MaterialEditor/Code/Source/Window/MaterialInspector/MaterialInspector.cpp
+2 -2
View File
@@ -152,7 +152,7 @@ namespace MaterialEditor
AtomToolsFramework::DynamicProperty property;
AtomToolsFramework::AtomToolsDocumentRequestBus::EventResult(
property, m_documentId, &AtomToolsFramework::AtomToolsDocumentRequestBus::Events::GetProperty,
AZ::RPI::MaterialPropertyId(groupName, uvNamePair.m_shaderInput.ToString()).GetFullName());
AZ::RPI::MaterialPropertyId(groupName, uvNamePair.m_shaderInput.ToString()));
group.m_properties.push_back(property);
property.SetValue(property.GetConfig().m_parentValue);
@@ -189,7 +189,7 @@ namespace MaterialEditor
AtomToolsFramework::DynamicProperty property;
AtomToolsFramework::AtomToolsDocumentRequestBus::EventResult(
property, m_documentId, &AtomToolsFramework::AtomToolsDocumentRequestBus::Events::GetProperty,
AZ::RPI::MaterialPropertyId(groupName, propertyDefinition.m_name).GetFullName());
AZ::RPI::MaterialPropertyId(groupName, propertyDefinition.m_name));
group.m_properties.push_back(property);
}
}
Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentInspector.cpp
+1 -1
View File
@@ -310,7 +310,7 @@ namespace AZ
AtomToolsFramework::DynamicPropertyConfig propertyConfig;
// Assign id before conversion so it can be used in dynamic description
propertyConfig.m_id = AZ::RPI::MaterialPropertyId(groupName, propertyDefinition.m_name).GetFullName();
propertyConfig.m_id = AZ::RPI::MaterialPropertyId(groupName, propertyDefinition.m_name);
AtomToolsFramework::ConvertToPropertyConfig(propertyConfig, propertyDefinition);
Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentUtil.cpp
+2 -2
View File
@@ -116,7 +116,7 @@ namespace AZ
editData.m_materialTypeSourceData.EnumerateProperties([&](const AZStd::string& groupName, const AZStd::string& propertyName, const auto& propertyDefinition){
const AZ::RPI::MaterialPropertyId propertyId(groupName, propertyName);
const AZ::RPI::MaterialPropertyIndex propertyIndex =
editData.m_materialAsset->GetMaterialPropertiesLayout()->FindPropertyIndex(propertyId.GetFullName());
editData.m_materialAsset->GetMaterialPropertiesLayout()->FindPropertyIndex(propertyId);
AZ::RPI::MaterialPropertyValue propertyValue =
editData.m_materialAsset->GetPropertyValues()[propertyIndex.GetIndex()];
@@ -128,7 +128,7 @@ namespace AZ
}
// Check for and apply any property overrides before saving property values
auto propertyOverrideItr = editData.m_materialPropertyOverrideMap.find(propertyId.GetFullName());
auto propertyOverrideItr = editData.m_materialPropertyOverrideMap.find(propertyId);
if (propertyOverrideItr != editData.m_materialPropertyOverrideMap.end())
{
propertyValue = AZ::RPI::MaterialPropertyValue::FromAny(propertyOverrideItr->second);
Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialModelUvNameMapInspector.cpp
+2 -2
View File
@@ -96,7 +96,7 @@ namespace AZ
const AZStd::string materialUvName = m_materialUvNames[i].m_uvName.GetStringView();
propertyConfig.m_dataType = AtomToolsFramework::DynamicPropertyType::Enum;
propertyConfig.m_id = AZ::RPI::MaterialPropertyId(groupName, shaderInput).GetFullName();
propertyConfig.m_id = AZ::RPI::MaterialPropertyId(groupName, shaderInput);
propertyConfig.m_name = shaderInput;
propertyConfig.m_displayName = materialUvName;
propertyConfig.m_description = shaderInput;
@@ -248,7 +248,7 @@ namespace AZ
const AZStd::string materialUvName = m_materialUvNames[i].m_uvName.GetStringView();
propertyConfig.m_dataType = AtomToolsFramework::DynamicPropertyType::Enum;
propertyConfig.m_id = AZ::RPI::MaterialPropertyId(groupName, shaderInput).GetFullName();
propertyConfig.m_id = AZ::RPI::MaterialPropertyId(groupName, shaderInput);
propertyConfig.m_name = shaderInput;
propertyConfig.m_displayName = materialUvName;
propertyConfig.m_description = shaderInput;