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

Code cleanup.

Browse Source 
Made PropertyDefinition::m_name private.
Moved code around for a cleaner diff in MaterialTypeSourceData.h.
Added API comments.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
This commit is contained in:
santorac
2022-01-26 18:10:02 -08:00
parent ed21e97dcd
commit e0cee13747
4 changed files with 118 additions and 89 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialTypeSourceData.h
+94 -62
View File
@@ -23,8 +23,10 @@ namespace AZ
{
class MaterialTypeAsset;
class MaterialFunctorSourceDataHolder;
class JsonMaterialPropertySerializer;
//! This is a simple data structure for serializing in/out material type source files.
//! Note that there may be a mixture of public and private members, as we are gradually introducing a proper API.
class MaterialTypeSourceData final
{
public:
@@ -69,15 +71,22 @@ namespace AZ
struct PropertyDefinition
{
friend class JsonMaterialPropertySerializer;
AZ_CLASS_ALLOCATOR(PropertyDefinition, SystemAllocator, 0);
AZ_TYPE_INFO(AZ::RPI::MaterialTypeSourceData::PropertyDefinition, "{E0DB3C0D-75DB-4ADB-9E79-30DA63FA18B7}");
static const float DefaultMin;
static const float DefaultMax;
static const float DefaultStep;
PropertyDefinition() = default;
// TODO: Consider making this private and readonly because it is used as the key for lookups and collision validation.
AZStd::string m_name; //!< The name of the property within the property group. The full property ID will be groupName.propertyName.
explicit PropertyDefinition(AZStd::string_view name) : m_name(name)
{
}
const AZStd::string& GetName() const { return m_name; }
MaterialPropertyVisibility m_visibility = MaterialPropertyVisibility::Default;
@@ -99,6 +108,51 @@ namespace AZ
MaterialPropertyValue m_softMin;
MaterialPropertyValue m_softMax;
MaterialPropertyValue m_step;
private:
// We are gradually moving toward having a more proper API for MaterialTypeSourceData code, but we still some public members
// like above. However, it's important for m_name to be private because it is used as the key for lookups, collision validation, etc.
AZStd::string m_name; //!< The name of the property within the property group. The full property ID will be groupName.propertyName.
};
using PropertyList = AZStd::vector<AZStd::unique_ptr<PropertyDefinition>>;
struct PropertySet
{
friend class MaterialTypeSourceData;
AZ_CLASS_ALLOCATOR(PropertySet, SystemAllocator, 0);
AZ_TYPE_INFO(AZ::RPI::MaterialTypeSourceData::PropertySet, "{BA3AA0E4-C74D-4FD0-ADB2-00B060F06314}");
public:
PropertySet() = default;
AZ_DISABLE_COPY(PropertySet)
const AZStd::string& GetName() const { return m_name; }
const AZStd::string& GetDisplayName() const { return m_displayName; }
const AZStd::string& GetDescription() const { return m_description; }
const PropertyList& GetProperties() const { return m_properties; }
const AZStd::vector<AZStd::unique_ptr<PropertySet>>& GetPropertySets() const { return m_propertySets; }
const AZStd::vector<Ptr<MaterialFunctorSourceDataHolder>>& GetFunctors() const { return m_materialFunctorSourceData; }
void SetDisplayName(AZStd::string_view displayName) { m_displayName = displayName; }
void SetDescription(AZStd::string_view description) { m_description = description; }
PropertyDefinition* AddProperty(AZStd::string_view name);
PropertySet* AddPropertySet(AZStd::string_view name);
private:
static PropertySet* AddPropertySet(AZStd::string_view name, AZStd::vector<AZStd::unique_ptr<PropertySet>>& toPropertySetList);
AZStd::string m_name;
AZStd::string m_displayName;
AZStd::string m_description;
PropertyList m_properties;
AZStd::vector<AZStd::unique_ptr<PropertySet>> m_propertySets;
AZStd::vector<Ptr<MaterialFunctorSourceDataHolder>> m_materialFunctorSourceData;
};
struct ShaderVariantReferenceData
@@ -144,47 +198,6 @@ namespace AZ
using VersionUpdates = AZStd::vector<VersionUpdateDefinition>;
using PropertyList = AZStd::vector<AZStd::unique_ptr<PropertyDefinition>>;
struct PropertySet
{
friend class MaterialTypeSourceData;
AZ_CLASS_ALLOCATOR(PropertySet, SystemAllocator, 0);
AZ_TYPE_INFO(AZ::RPI::MaterialTypeSourceData::PropertySet, "{BA3AA0E4-C74D-4FD0-ADB2-00B060F06314}");
public:
PropertySet() = default;
AZ_DISABLE_COPY(PropertySet)
const AZStd::string& GetName() const { return m_name; }
const AZStd::string& GetDisplayName() const { return m_displayName; }
const AZStd::string& GetDescription() const { return m_description; }
const PropertyList& GetProperties() const { return m_properties; }
const AZStd::vector<AZStd::unique_ptr<PropertySet>>& GetPropertySets() const { return m_propertySets; }
const AZStd::vector<Ptr<MaterialFunctorSourceDataHolder>>& GetFunctors() const { return m_materialFunctorSourceData; }
void SetDisplayName(AZStd::string_view displayName) { m_displayName = displayName; }
void SetDescription(AZStd::string_view description) { m_description = description; }
PropertyDefinition* AddProperty(AZStd::string_view name);
PropertySet* AddPropertySet(AZStd::string_view name);
private:
static PropertySet* AddPropertySet(AZStd::string_view name, AZStd::vector<AZStd::unique_ptr<PropertySet>>& toPropertySetList);
AZStd::string m_name;
AZStd::string m_displayName;
AZStd::string m_description;
PropertyList m_properties;
AZStd::vector<AZStd::unique_ptr<PropertySet>> m_propertySets;
AZStd::vector<Ptr<MaterialFunctorSourceDataHolder>> m_materialFunctorSourceData;
};
using VersionUpdates = AZStd::vector<VersionUpdateDefinition>;
struct PropertyLayout
{
AZ_TYPE_INFO(AZ::RPI::MaterialTypeSourceData::PropertyLayout, "{AE53CF3F-5C3B-44F5-B2FB-306F0EB06393}");
@@ -206,52 +219,69 @@ namespace AZ
AZStd::vector<AZStd::unique_ptr<PropertySet>> m_propertySets;
};
PropertySet* AddPropertySet(AZStd::string_view propertySetId);
PropertyDefinition* AddProperty(AZStd::string_view propertyId);
const PropertyLayout& GetPropertyLayout() const { return m_propertyLayout; }
AZStd::string m_description; //< TODO: Make this private
AZStd::string m_description;
//! Version 1 is the default and should not contain any version update.
uint32_t m_version = 1; //< TODO: Make this private
VersionUpdates m_versionUpdates; //< TODO: Make this private
uint32_t m_version = 1;
VersionUpdates m_versionUpdates;
//! A list of shader variants that are always used at runtime; they cannot be turned off
AZStd::vector<ShaderVariantReferenceData> m_shaderCollection; //< TODO: Make this private
AZStd::vector<ShaderVariantReferenceData> m_shaderCollection;
//! Material functors provide custom logic and calculations to configure shaders, render states, and more. See MaterialFunctor.h for details.
AZStd::vector<Ptr<MaterialFunctorSourceDataHolder>> m_materialFunctorSourceData; //< TODO: Make this private
AZStd::vector<Ptr<MaterialFunctorSourceDataHolder>> m_materialFunctorSourceData;
//! Override names for UV input in the shaders of this material type.
//! Using ordered map to sort names on loading.
using UvNameMap = AZStd::map<AZStd::string, AZStd::string>;
UvNameMap m_uvNameMap; //< TODO: Make this private
UvNameMap m_uvNameMap;
//! Copy over UV custom names to the properties enum values.
void ResolveUvEnums();
const PropertySet* FindPropertySet(AZStd::string_view propertySetId) const;
//! Add a new PropertySet for containing properties or other PropertySets.
//! @param propertySetId The ID of the new property set. To add as a nested PropertySet, use a full path ID like "levelA.levelB.levelC"; in this case a property set "levelA.levelB" must already exist.
//! @return a pointer to the new PropertySet or null if there was a problem (an AZ_Error will be reported).
PropertySet* AddPropertySet(AZStd::string_view propertySetId);
//! Add a new property to a PropertySet.
//! @param propertyId The ID of the new property, like "layerBlend.factor" or "layer2.roughness.texture". The indicated property set must already exist.
//! @return a pointer to the new PropertyDefinition or null if there was a problem (an AZ_Error will be reported).
PropertyDefinition* AddProperty(AZStd::string_view propertyId);
//! Return the PropertyLayout containing the tree of property sets and property definitions.
const PropertyLayout& GetPropertyLayout() const { return m_propertyLayout; }
//! Find the PropertySet with the given ID.
//! @param propertySetId The full ID of a property set to find, like "levelA.levelB.levelC".
//! @return the found PropertySet or null if it doesn't exist.
const PropertySet* FindPropertySet(AZStd::string_view propertySetId) const;
//! Find the definition for a property with the given ID.
//! @param propertyId The full ID of a property to find, like "baseColor.texture".
//! @return the found PropertyDefinition or null if it doesn't exist.
const PropertyDefinition* FindProperty(AZStd::string_view propertyId) const;
//! Tokenizes an ID string like "itemA.itemB.itemC" into a vector like ["itemA", "itemB", "itemC"]
//! Tokenizes an ID string like "itemA.itemB.itemC" into a vector like ["itemA", "itemB", "itemC"].
static AZStd::vector<AZStd::string_view> TokenizeId(AZStd::string_view id);
//! Splits an ID string like "itemA.itemB.itemC" into a vector like ["itemA.itemB", "itemC"]
//! Splits an ID string like "itemA.itemB.itemC" into a vector like ["itemA.itemB", "itemC"].
static AZStd::vector<AZStd::string_view> SplitId(AZStd::string_view id);
//! Call back function type used with the enumeration functions
//! Call back function type used with the enumeration functions.
//! Return false to terminate the traversal.
using EnumeratePropertySetsCallback = AZStd::function<bool(
const AZStd::string&, // The property ID context (i.e. "levelA.levelB."
const AZStd::string&, // The property ID context (i.e. "levelA.levelB.")
const PropertySet* // the next property set in the tree
)>;
//! Recursively traverses all of the property sets contained in the material type, executing a callback function for each.
//! @return false if the enumeration was terminated early by the callback returning false.
bool EnumeratePropertySets(const EnumeratePropertySetsCallback& callback) const;
//! Call back function type used with the numeration functions
//! Call back function type used with the numeration functions.
//! Return false to terminate the traversal.
using EnumeratePropertiesCallback = AZStd::function<bool(
const AZStd::string&, // The property ID context (i.e. "levelA.levelB."
const PropertyDefinition* // the property definition object
@@ -263,6 +293,8 @@ namespace AZ
Outcome<Data::Asset<MaterialTypeAsset>> CreateMaterialTypeAsset(Data::AssetId assetId, AZStd::string_view materialTypeSourceFilePath = "", bool elevateWarnings = true) const;
//! If the data was loaded from an old format file (i.e. where "groups" and "properties" were separate sections),
//! this converts to the new format where properties are listed inside property sets.
bool ConvertToNewDataFormat();
private:
Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp
+7 -8
View File
@@ -160,7 +160,7 @@ namespace AZ
{
auto propertyIter = AZStd::find_if(m_properties.begin(), m_properties.end(), [name](const AZStd::unique_ptr<PropertyDefinition>& existingProperty)
{
return existingProperty->m_name == name;
return existingProperty->GetName() == name;
});
if (propertyIter != m_properties.end())
@@ -186,8 +186,7 @@ namespace AZ
return nullptr;
}
m_properties.emplace_back(AZStd::make_unique<PropertyDefinition>());
m_properties.back()->m_name = name;
m_properties.emplace_back(AZStd::make_unique<PropertyDefinition>(name));
return m_properties.back().get();
}
@@ -195,7 +194,7 @@ namespace AZ
{
auto iter = AZStd::find_if(m_properties.begin(), m_properties.end(), [name](const AZStd::unique_ptr<PropertyDefinition>& existingProperty)
{
return existingProperty->m_name == name;
return existingProperty->GetName() == name;
});
if (iter != m_properties.end())
@@ -298,7 +297,7 @@ namespace AZ
{
for (AZStd::unique_ptr<PropertyDefinition>& property : propertySet->m_properties)
{
if (property->m_name == subPath[0])
if (property->GetName() == subPath[0])
{
return property.get();
}
@@ -440,7 +439,7 @@ namespace AZ
propertySet = m_propertyLayout.m_propertySets.back().get();
}
PropertyDefinition* newProperty = propertySet->AddProperty(propertyDefinition.m_name);
PropertyDefinition* newProperty = propertySet->AddProperty(propertyDefinition.GetName());
*newProperty = propertyDefinition;
}
@@ -518,7 +517,7 @@ namespace AZ
{
// Register the property...
MaterialPropertyId propertyId{propertyNameContext, property->m_name};
MaterialPropertyId propertyId{propertyNameContext, property->GetName()};
if (!propertyId.IsValid())
{
@@ -529,7 +528,7 @@ namespace AZ
auto propertySetIter = AZStd::find_if(propertySet->GetPropertySets().begin(), propertySet->GetPropertySets().end(),
[&property](const AZStd::unique_ptr<PropertySet>& existingPropertySet)
{
return existingPropertySet->GetName() == property->m_name;
return existingPropertySet->GetName() == property->GetName();
});
if (propertySetIter != propertySet->GetPropertySets().end())
Gems/Atom/RPI/Code/Tests/Material/MaterialPropertySerializerTests.cpp
+7 -9
View File
@@ -45,8 +45,7 @@ namespace JsonSerializationTests
AZStd::shared_ptr<AZ::RPI::MaterialTypeSourceData::PropertyDefinition> CreatePartialDefaultInstance() override
{
auto result = AZStd::make_shared<AZ::RPI::MaterialTypeSourceData::PropertyDefinition>();
result->m_name = "testProperty";
auto result = AZStd::make_shared<AZ::RPI::MaterialTypeSourceData::PropertyDefinition>("testProperty");
result->m_dataType = AZ::RPI::MaterialPropertyDataType::Float;
result->m_step = 1.0f;
result->m_value = 0.0f;
@@ -65,8 +64,7 @@ namespace JsonSerializationTests
AZStd::shared_ptr<AZ::RPI::MaterialTypeSourceData::PropertyDefinition> CreateFullySetInstance() override
{
auto result = AZStd::make_shared<AZ::RPI::MaterialTypeSourceData::PropertyDefinition>();
result->m_name = "testProperty";
auto result = AZStd::make_shared<AZ::RPI::MaterialTypeSourceData::PropertyDefinition>("testProperty");
result->m_description = "description";
result->m_displayName = "display_name";
result->m_dataType = AZ::RPI::MaterialPropertyDataType::Float;
@@ -135,7 +133,7 @@ namespace JsonSerializationTests
const AZ::RPI::MaterialTypeSourceData::PropertyDefinition& lhs,
const AZ::RPI::MaterialTypeSourceData::PropertyDefinition& rhs) override
{
if (lhs.m_name != rhs.m_name) { return false; }
if (lhs.GetName() != rhs.GetName()) { return false; }
if (lhs.m_description != rhs.m_description) { return false; }
if (lhs.m_displayName != rhs.m_displayName) { return false; }
if (lhs.m_dataType != rhs.m_dataType) { return false; }
@@ -216,7 +214,7 @@ namespace UnitTest
EXPECT_EQ(AZ::JsonSerializationResult::Processing::Completed, loadResult.m_jsonResultCode.GetProcessing());
EXPECT_EQ(AZ::JsonSerializationResult::Outcomes::PartialDefaults, loadResult.m_jsonResultCode.GetOutcome());
EXPECT_EQ("testProperty", propertyData.m_name);
EXPECT_EQ("testProperty", propertyData.GetName());
EXPECT_EQ("Test Property", propertyData.m_displayName);
EXPECT_EQ("This is a property description", propertyData.m_description);
EXPECT_EQ(MaterialPropertyDataType::Float, propertyData.m_dataType);
@@ -851,7 +849,7 @@ namespace UnitTest
EXPECT_EQ(AZ::JsonSerializationResult::Tasks::ReadField, loadResult.m_jsonResultCode.GetTask());
EXPECT_EQ(AZ::JsonSerializationResult::Processing::Completed, loadResult.m_jsonResultCode.GetProcessing());
EXPECT_EQ("testProperty", propertyData.m_name);
EXPECT_EQ("testProperty", propertyData.GetName());
EXPECT_EQ(1, propertyData.m_outputConnections.size());
EXPECT_EQ(MaterialPropertyOutputType::ShaderOption, propertyData.m_outputConnections[0].m_type);
@@ -934,7 +932,7 @@ namespace UnitTest
EXPECT_EQ(AZ::JsonSerializationResult::Tasks::ReadField, loadResult.m_jsonResultCode.GetTask());
EXPECT_EQ(AZ::JsonSerializationResult::Processing::Completed, loadResult.m_jsonResultCode.GetProcessing());
EXPECT_EQ(propertyData.m_name, "testProperty");
EXPECT_EQ(propertyData.GetName(), "testProperty");
EXPECT_EQ(propertyData.m_dataType, MaterialPropertyDataType::Float);
EXPECT_EQ(propertyData.m_outputConnections.size(), 0);
@@ -964,7 +962,7 @@ namespace UnitTest
EXPECT_EQ(AZ::JsonSerializationResult::Tasks::ReadField, loadResult.m_jsonResultCode.GetTask());
EXPECT_EQ(AZ::JsonSerializationResult::Processing::Completed, loadResult.m_jsonResultCode.GetProcessing());
EXPECT_EQ(propertyData.m_name, "testProperty");
EXPECT_EQ(propertyData.GetName(), "testProperty");
EXPECT_EQ(propertyData.m_dataType, MaterialPropertyDataType::Float);
EXPECT_EQ(propertyData.m_outputConnections.size(), 1);
EXPECT_EQ(propertyData.m_outputConnections[0].m_fieldName, "o_foo");
Gems/Atom/RPI/Code/Tests/Material/MaterialTypeSourceDataTests.cpp
+10 -10
View File
@@ -1645,12 +1645,12 @@ namespace UnitTest
EXPECT_NE(material.FindProperty("groupC.groupD.foo"), nullptr);
EXPECT_NE(material.FindProperty("groupC.groupE.bar"), nullptr);
EXPECT_EQ(material.FindProperty("groupA.foo")->m_name, "foo");
EXPECT_EQ(material.FindProperty("groupA.bar")->m_name, "bar");
EXPECT_EQ(material.FindProperty("groupB.foo")->m_name, "foo");
EXPECT_EQ(material.FindProperty("groupB.bar")->m_name, "bar");
EXPECT_EQ(material.FindProperty("groupC.groupD.foo")->m_name, "foo");
EXPECT_EQ(material.FindProperty("groupC.groupE.bar")->m_name, "bar");
EXPECT_EQ(material.FindProperty("groupA.foo")->GetName(), "foo");
EXPECT_EQ(material.FindProperty("groupA.bar")->GetName(), "bar");
EXPECT_EQ(material.FindProperty("groupB.foo")->GetName(), "foo");
EXPECT_EQ(material.FindProperty("groupB.bar")->GetName(), "bar");
EXPECT_EQ(material.FindProperty("groupC.groupD.foo")->GetName(), "foo");
EXPECT_EQ(material.FindProperty("groupC.groupE.bar")->GetName(), "bar");
EXPECT_EQ(material.FindProperty("groupA.foo")->m_dataType, MaterialPropertyDataType::Bool);
EXPECT_EQ(material.FindProperty("groupA.bar")->m_dataType, MaterialPropertyDataType::Image);
EXPECT_EQ(material.FindProperty("groupB.foo")->m_dataType, MaterialPropertyDataType::Float);
@@ -1823,10 +1823,10 @@ namespace UnitTest
EXPECT_TRUE(material.FindProperty("groupB.foo") != nullptr);
EXPECT_TRUE(material.FindProperty("groupB.bar") != nullptr);
EXPECT_EQ(material.FindProperty("groupA.foo")->m_name, "foo");
EXPECT_EQ(material.FindProperty("groupA.bar")->m_name, "bar");
EXPECT_EQ(material.FindProperty("groupB.foo")->m_name, "foo");
EXPECT_EQ(material.FindProperty("groupB.bar")->m_name, "bar");
EXPECT_EQ(material.FindProperty("groupA.foo")->GetName(), "foo");
EXPECT_EQ(material.FindProperty("groupA.bar")->GetName(), "bar");
EXPECT_EQ(material.FindProperty("groupB.foo")->GetName(), "foo");
EXPECT_EQ(material.FindProperty("groupB.bar")->GetName(), "bar");
EXPECT_EQ(material.FindProperty("groupA.foo")->m_dataType, MaterialPropertyDataType::Bool);
EXPECT_EQ(material.FindProperty("groupA.bar")->m_dataType, MaterialPropertyDataType::Image);
EXPECT_EQ(material.FindProperty("groupB.foo")->m_dataType, MaterialPropertyDataType::Float);