From b1c746968752a02a621220edf81c902cb3161ebd Mon Sep 17 00:00:00 2001 From: santorac <55155825+santorac@users.noreply.github.com> Date: Fri, 1 Oct 2021 17:11:54 -0700 Subject: [PATCH] Renamed m_groups and m_properties to have "Old" in the name for clarity. Also fixed a potential uninitialized data bug in Conve. Signed-off-by: santorac <55155825+santorac@users.noreply.github.com> --- .../ConvertEmissiveUnitFunctorSourceData.h | 4 ++-- .../RPI.Edit/Material/MaterialTypeSourceData.h | 4 ++-- .../Material/MaterialTypeSourceData.cpp | 18 +++++++++--------- .../Material/MaterialTypeSourceDataTests.cpp | 8 ++++---- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/Gems/Atom/Feature/Common/Code/Source/Material/ConvertEmissiveUnitFunctorSourceData.h b/Gems/Atom/Feature/Common/Code/Source/Material/ConvertEmissiveUnitFunctorSourceData.h index 23219ce940..091351087f 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Material/ConvertEmissiveUnitFunctorSourceData.h +++ b/Gems/Atom/Feature/Common/Code/Source/Material/ConvertEmissiveUnitFunctorSourceData.h @@ -44,8 +44,8 @@ namespace AZ AZStd::string m_shaderInputName; // The indices of photometric units in the dropdown list - uint32_t m_ev100Index; - uint32_t m_nitIndex; + uint32_t m_ev100Index = 0; + uint32_t m_nitIndex = 1; // Minimum and Maximum value for different photometric units AZ::Vector2 m_ev100MinMax; diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialTypeSourceData.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialTypeSourceData.h index 1d918702ff..ec12384286 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialTypeSourceData.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialTypeSourceData.h @@ -170,11 +170,11 @@ namespace AZ //! [Deprecated] Use m_propertySets instead //! List of groups that will contain the available properties - AZStd::vector m_groups; + AZStd::vector m_groupsOld; //! [Deprecated] Use m_propertySets instead //! Collection of all available user-facing properties - AZStd::map> m_properties; + AZStd::map> m_propertiesOld; AZStd::vector> m_propertySets; }; diff --git a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp index 816e9f4f8d..bddbf726e2 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp @@ -83,8 +83,8 @@ namespace AZ serializeContext->Class() ->Version(1) ->Field("version", &PropertyLayout::m_version) - ->Field("groups", &PropertyLayout::m_groups) //< Old, preserved for backward compatibility, replaced by propertySets - ->Field("properties", &PropertyLayout::m_properties) //< Old, preserved for backward compatibility, replaced by propertySets + ->Field("groups", &PropertyLayout::m_groupsOld) //< Deprecated, preserved for backward compatibility, replaced by propertySets + ->Field("properties", &PropertyLayout::m_propertiesOld) //< Deprecated, preserved for backward compatibility, replaced by propertySets ->Field("propertySets", &PropertyLayout::m_propertySets) ; @@ -397,8 +397,8 @@ namespace AZ { for (const auto& group : GetOldFormatGroupDefinitionsInDisplayOrder()) { - auto propertyListItr = m_propertyLayout.m_properties.find(group.m_name); - if (propertyListItr != m_propertyLayout.m_properties.end()) + auto propertyListItr = m_propertyLayout.m_propertiesOld.find(group.m_name); + if (propertyListItr != m_propertyLayout.m_propertiesOld.end()) { const auto& propertyList = propertyListItr->second; for (auto& propertyDefinition : propertyList) @@ -421,8 +421,8 @@ namespace AZ } } - m_propertyLayout.m_groups.clear(); - m_propertyLayout.m_properties.clear(); + m_propertyLayout.m_groupsOld.clear(); + m_propertyLayout.m_propertiesOld.clear(); return true; } @@ -451,11 +451,11 @@ namespace AZ AZStd::vector MaterialTypeSourceData::GetOldFormatGroupDefinitionsInDisplayOrder() const { AZStd::vector groupDefinitions; - groupDefinitions.reserve(m_propertyLayout.m_properties.size()); + groupDefinitions.reserve(m_propertyLayout.m_propertiesOld.size()); // Some groups are defined explicitly in the .materialtype file's "groups" section. This is the primary way groups are sorted in the UI. AZStd::unordered_set foundGroups; - for (const auto& groupDefinition : m_propertyLayout.m_groups) + for (const auto& groupDefinition : m_propertyLayout.m_groupsOld) { if (foundGroups.insert(groupDefinition.m_name).second) { @@ -468,7 +468,7 @@ namespace AZ } // Some groups are defined implicitly, in the "properties" section where a group name is used but not explicitly defined in the "groups" section. - for (const auto& propertyListPair : m_propertyLayout.m_properties) + for (const auto& propertyListPair : m_propertyLayout.m_propertiesOld) { const AZStd::string& groupName = propertyListPair.first; if (foundGroups.insert(groupName).second) diff --git a/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeSourceDataTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeSourceDataTests.cpp index 00523abbd8..e5394106fd 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeSourceDataTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeSourceDataTests.cpp @@ -1781,15 +1781,15 @@ namespace UnitTest JsonTestResult loadResult = LoadTestDataFromJson(material, inputJson); // Before conversion to the new format, the data is in the old place - EXPECT_EQ(material.GetPropertyLayout().m_groups.size(), 2); - EXPECT_EQ(material.GetPropertyLayout().m_properties.size(), 2); + EXPECT_EQ(material.GetPropertyLayout().m_groupsOld.size(), 2); + EXPECT_EQ(material.GetPropertyLayout().m_propertiesOld.size(), 2); EXPECT_EQ(material.GetPropertyLayout().m_propertySets.size(), 0); material.ConvertToNewDataFormat(); // After conversion to the new format, the data is in the new place - EXPECT_EQ(material.GetPropertyLayout().m_groups.size(), 0); - EXPECT_EQ(material.GetPropertyLayout().m_properties.size(), 0); + EXPECT_EQ(material.GetPropertyLayout().m_groupsOld.size(), 0); + EXPECT_EQ(material.GetPropertyLayout().m_propertiesOld.size(), 0); EXPECT_EQ(material.GetPropertyLayout().m_propertySets.size(), 2); EXPECT_EQ(material.m_description, "This is a general description about the material");