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

Moved the Material Component property override renaming to EditorMaterialComponent via ApplyAutomaticPropertyUpdates.

Browse Source 
MaterialAssignment::ApplyProperties() still reports warnings but does not update the m_propertyOverrides.
MaterialAssignment::ApplyProperties() will now skip the old name'd overrides if overrides are present for the new names. I'm not sure if this will ever happen, but it did happen while I had some intermediate changes, so I imagine it could happen again.
I had to update the Material::FindPropertyIndex function to expose information about renames when they occur. This should make it easier for other systems to get (somewhat) automatic benefit from the version update feature.
I also found that there was an issue with material inspector where it wouldn't be initialized the the right override values when renames were present. Now it applies the renames to whatever override data it gets from the Material Component.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
This commit is contained in:
santorac
2021-10-22 16:41:44 -07:00
parent 43b7bf6ba7
commit 3c331e00ff
9 changed files with 154 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Gems/Atom/Feature/Common/Code/Source/Material/MaterialAssignment.cpp
+16 -28
View File
@@ -137,31 +137,28 @@ namespace AZ
if (m_materialInstance->CanCompile())
{
AZStd::vector<AZStd::pair<Name, Name>> renamedProperties;
for (const auto& propertyPair : m_propertyOverrides)
{
if (!propertyPair.second.empty())
{
RPI::MaterialPropertyIndex materialPropertyIndex = m_materialInstance->FindPropertyIndex(propertyPair.first);
bool wasRenamed = false;
Name newName;
RPI::MaterialPropertyIndex materialPropertyIndex = m_materialInstance->FindPropertyIndex(propertyPair.first, &wasRenamed, &newName);
// If we didn't find the name, check to see if there was a rename that should be used.
if (materialPropertyIndex.IsNull())
// FindPropertyIndex will have already reported a message about what the old and new names are. Here we just add some extra info to help the user resolve it.
AZ_Warning("MaterialAssignment", !wasRenamed,
"Consider running \"Apply Automatic Property Updates\" to use the latest property names.",
propertyPair.first.GetCStr(),
newName.GetCStr());
if (wasRenamed && m_propertyOverrides.find(newName) != m_propertyOverrides.end())
{
Name propertyId = propertyPair.first;
if (m_materialInstance->GetAsset()->GetMaterialTypeAsset()->ApplyPropertyRenames(propertyId))
{
renamedProperties.emplace_back(propertyPair.first, propertyId);
// We should be able to find the property now, using the new name
materialPropertyIndex = m_materialInstance->FindPropertyIndex(propertyId);
AZ_Warning("MaterialAssignment", false,
"Material property '%s' has been renamed to '%s', and a material override references the old name. Save the level or prefab to permanently apply this change.",
propertyPair.first.GetCStr(),
propertyId.GetCStr());
}
materialPropertyIndex.Reset();
AZ_Warning("MaterialAssignment", false,
"Material property '%s' has been renamed to '%s', and a property override exists for both. The one with the old name will be ignored.",
propertyPair.first.GetCStr(),
newName.GetCStr());
}
if (!materialPropertyIndex.IsNull())
@@ -172,15 +169,6 @@ namespace AZ
}
}
// Now that we're done looping over all the overrides, it's safe to apply any renames that were scheduled
for (auto& pair : renamedProperties)
{
const Name& oldName = pair.first;
const Name& newName = pair.second;
m_propertyOverrides[newName] = m_propertyOverrides[oldName];
m_propertyOverrides.erase(oldName);
}
return m_materialInstance->Compile();
}
Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Material/Material.h
+3 -1
View File
@@ -71,7 +71,9 @@ namespace AZ
virtual ~Material();
//! Finds the material property index from the material property ID
MaterialPropertyIndex FindPropertyIndex(const Name& propertyId) const;
//! @param wasRenamed optional parameter that is set to true if @propertyId is an old name and an automatic rename was applied to find the index.
//! @param newName optional parameter that is set to the new property name, if the property was renamed.
MaterialPropertyIndex FindPropertyIndex(const Name& propertyId, bool* wasRenamed = nullptr, Name* newName = nullptr) const;
//! Sets the value of a material property. The template data type must match the property's data type.
//! @return true if property value was changed
Gems/Atom/RPI/Code/Source/RPI.Public/Material/Material.cpp
+32 -2
View File
@@ -386,9 +386,39 @@ namespace AZ
return m_currentChangeId;
}
MaterialPropertyIndex Material::FindPropertyIndex(const Name& propertyId) const
MaterialPropertyIndex Material::FindPropertyIndex(const Name& propertyId, bool* wasRenamed, Name* newName) const
{
return m_layout->FindPropertyIndex(propertyId);
if (wasRenamed)
{
*wasRenamed = false;
}
MaterialPropertyIndex index = m_layout->FindPropertyIndex(propertyId);
if (!index.IsValid())
{
Name renamedId = propertyId;
if (m_materialAsset->GetMaterialTypeAsset()->ApplyPropertyRenames(renamedId))
{
index = m_layout->FindPropertyIndex(renamedId);
if (wasRenamed)
{
*wasRenamed = true;
}
if (newName)
{
*newName = renamedId;
}
AZ_Warning("Material", false,
"Material property '%s' has been renamed to '%s'. Consider updating the corresponding source data.",
propertyId.GetCStr(),
renamedId.GetCStr());
}
}
return index;
}
template<typename Type>
Gems/Atom/RPI/Code/Tests/Material/MaterialTests.cpp
+31
View File
@@ -857,4 +857,35 @@ namespace UnitTest
EXPECT_EQ(material->GetPropertyValue<int32_t>(material->FindPropertyIndex(Name{ "MyInt" })), -7);
EXPECT_EQ(srgData->GetConstant<int32_t>(srgData->FindShaderInputConstantIndex(Name{ "m_int" })), -7);
}
TEST_F(MaterialTests, TestFindPropertyIndexUsingOldName)
{
MaterialTypeAssetCreator materialTypeCreator;
materialTypeCreator.Begin(Uuid::CreateRandom());
materialTypeCreator.AddShader(m_testMaterialShaderAsset);
AddCommonTestMaterialProperties(materialTypeCreator);
materialTypeCreator.SetVersion(2);
MaterialVersionUpdate versionUpdate(2);
versionUpdate.AddAction(MaterialVersionUpdate::RenamePropertyAction({Name{ "OldName" },Name{ "MyInt" }}));
materialTypeCreator.AddVersionUpdate(versionUpdate);
materialTypeCreator.End(m_testMaterialTypeAsset);
MaterialAssetCreator materialCreator;
materialCreator.Begin(Uuid::CreateRandom(), *m_testMaterialTypeAsset);
materialCreator.End(m_testMaterialAsset);
Data::Instance<Material> material = Material::FindOrCreate(m_testMaterialAsset);
bool wasRenamed = false;
Name newName;
MaterialPropertyIndex indexFromOldName = material->FindPropertyIndex(Name{"OldName"}, &wasRenamed, &newName);
EXPECT_TRUE(wasRenamed);
EXPECT_EQ(newName, Name{"MyInt"});
MaterialPropertyIndex indexFromNewName = material->FindPropertyIndex(Name{"MyInt"}, &wasRenamed, &newName);
EXPECT_FALSE(wasRenamed);
EXPECT_EQ(indexFromOldName, indexFromNewName);
}
}
Gems/AtomLyIntegration/CommonFeatures/Code/Include/AtomLyIntegration/CommonFeatures/Material/MaterialComponentBus.h
+3
View File
@@ -43,6 +43,9 @@ namespace AZ
virtual void ClearInvalidMaterialOverrides() = 0;
//! Repair materials that reference missing assets by assigning the default asset
virtual void RepairInvalidMaterialOverrides() = 0;
//! Repair material property overrides that reference missing properties by auto-renaming them where possible
//! @return the number of properties that were updated
virtual uint32_t ApplyAutomaticPropertyUpdates() = 0;
//! Set default material override
virtual void SetDefaultMaterialOverride(const AZ::Data::AssetId& materialAssetId) = 0;
//! Get default material override
Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponent.cpp
+13
View File
@@ -240,6 +240,19 @@ namespace AZ
UpdateMaterialSlots();
});
action->setToolTip("Repair materials that reference missing assets by assigning the default asset.");
action = menu->addAction("Apply Automatic Property Updates", [this]() {
AzToolsFramework::ScopedUndoBatch undoBatch("Applying automatic property updates.");
SetDirty();
uint32_t propertiesUpdated = 0;
MaterialComponentRequestBus::EventResult(propertiesUpdated, GetEntityId(), &MaterialComponentRequestBus::Events::ApplyAutomaticPropertyUpdates);
AZ_Printf("EditorMaterialComponent", "Updated %u property(s).", propertiesUpdated);
UpdateMaterialSlots();
});
action->setToolTip("Repair material property overrides that reference missing properties by auto-renaming them where possible.");
}
void EditorMaterialComponent::SetPrimaryAsset(const AZ::Data::AssetId& assetId)
Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentInspector.cpp
+21
View File
@@ -352,6 +352,27 @@ namespace AZ
m_editData.m_materialPropertyOverrideMap, m_entityId, &MaterialComponentRequestBus::Events::GetPropertyOverrides,
m_materialAssignmentId);
// Apply any automatic property renames so that the material inspector will be property initialized with the right values
// for properties that have new names.
{
AZStd::vector<AZStd::pair<Name, Name>> renamedProperties;
for (auto& propertyOverridePair : m_editData.m_materialPropertyOverrideMap)
{
Name name = propertyOverridePair.first;
if (m_materialInstance->GetAsset()->GetMaterialTypeAsset()->ApplyPropertyRenames(name))
{
renamedProperties.emplace_back(propertyOverridePair.first, name);
}
}
for (auto& renamePair : renamedProperties)
{
const Name& oldName = renamePair.first;
const Name& newName = renamePair.second;
m_editData.m_materialPropertyOverrideMap[newName] = m_editData.m_materialPropertyOverrideMap[oldName];
m_editData.m_materialPropertyOverrideMap.erase(oldName);
}
}
for (auto& group : m_groups)
{
for (auto& property : group.second.m_properties)
Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.cpp
+34
View File
@@ -49,6 +49,7 @@ namespace AZ
->Event("ClearIncompatibleMaterialOverrides", &MaterialComponentRequestBus::Events::ClearIncompatibleMaterialOverrides)
->Event("ClearInvalidMaterialOverrides", &MaterialComponentRequestBus::Events::ClearInvalidMaterialOverrides)
->Event("RepairInvalidMaterialOverrides", &MaterialComponentRequestBus::Events::RepairInvalidMaterialOverrides)
->Event("ApplyAutomaticPropertyUpdates", &MaterialComponentRequestBus::Events::ApplyAutomaticPropertyUpdates)
->Event("SetMaterialOverride", &MaterialComponentRequestBus::Events::SetMaterialOverride)
->Event("GetMaterialOverride", &MaterialComponentRequestBus::Events::GetMaterialOverride)
->Event("ClearMaterialOverride", &MaterialComponentRequestBus::Events::ClearMaterialOverride)
@@ -406,6 +407,39 @@ namespace AZ
}
LoadMaterials();
}
uint32_t MaterialComponentController::ApplyAutomaticPropertyUpdates()
{
uint32_t propertiesUpdated = 0;
for (auto& materialAssignmentPair : m_configuration.m_materials)
{
MaterialAssignment& materialAssignment = materialAssignmentPair.second;
AZStd::vector<AZStd::pair<Name, Name>> renamedProperties;
for (const auto& propertyPair : materialAssignment.m_propertyOverrides)
{
Name propertyId = propertyPair.first;
if (materialAssignment.m_materialInstance->GetAsset()->GetMaterialTypeAsset()->ApplyPropertyRenames(propertyId))
{
renamedProperties.emplace_back(propertyPair.first, propertyId);
++propertiesUpdated;
}
}
for (auto& pair : renamedProperties)
{
const Name& oldName = pair.first;
const Name& newName = pair.second;
materialAssignment.m_propertyOverrides[newName] = materialAssignment.m_propertyOverrides[oldName];
materialAssignment.m_propertyOverrides.erase(oldName);
}
}
return propertiesUpdated;
}
void MaterialComponentController::SetDefaultMaterialOverride(const AZ::Data::AssetId& materialAssetId)
{
Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.h
+1
View File
@@ -58,6 +58,7 @@ namespace AZ
void ClearIncompatibleMaterialOverrides() override;
void ClearInvalidMaterialOverrides() override;
void RepairInvalidMaterialOverrides() override;
uint32_t ApplyAutomaticPropertyUpdates() override;
void SetDefaultMaterialOverride(const AZ::Data::AssetId& materialAssetId) override;
const AZ::Data::AssetId GetDefaultMaterialOverride() const override;
void ClearDefaultMaterialOverride() override;