From a7bdb05d661e9f3dca0b353926c0737d144980bc Mon Sep 17 00:00:00 2001 From: santorac <55155825+santorac@users.noreply.github.com> Date: Tue, 1 Feb 2022 22:37:47 -0800 Subject: [PATCH] Tied up some loose ends with nested property group support. Fixed the MaterialBuilder to report source dependencies on imported JSON files, so the material type will rebuild when you edit an imported file that contains shared property data. Fixed a spot in LuaMaterialFunctorSourceData where it was only applying the old "prefix" feature and not the new name context. Updated a couple built-in material functors to take advantage of the name context (they were using RuntimeContext::GetShaderResourceGroupLayout instead of the RuntimeContext::FindShaderInputConstantIndex wrapper utility function). Fixed an issue with EnumeratePropertyGroups where it wasn't passing the right name context, so Material Editor wasn't able to load some material types. Reordered the parameters in the MaterialTypeSourceData enumerate callback functions, I felt this order was more natural. Signed-off-by: santorac <55155825+santorac@users.noreply.github.com> --- ...surfaceTransmissionParameterFunctorSourceData.cpp | 6 +++--- .../Source/Material/Transform2DFunctorSourceData.cpp | 4 ++-- .../Atom/RPI.Edit/Material/MaterialTypeSourceData.h | 8 ++++---- .../Include/Atom/RPI.Edit/Material/MaterialUtils.h | 5 ++++- .../Source/RPI.Builders/Material/MaterialBuilder.cpp | 10 +++++++++- .../Material/LuaMaterialFunctorSourceData.cpp | 5 ++++- .../RPI.Edit/Material/MaterialTypeSourceData.cpp | 10 +++++----- .../Code/Source/RPI.Edit/Material/MaterialUtils.cpp | 6 +++++- .../Tests/Material/MaterialTypeSourceDataTests.cpp | 12 ++++++------ .../Code/Source/Document/MaterialDocument.cpp | 12 +++++------- .../Source/Material/EditorMaterialComponentUtil.cpp | 4 ++-- 11 files changed, 49 insertions(+), 33 deletions(-) diff --git a/Gems/Atom/Feature/Common/Code/Source/Material/SubsurfaceTransmissionParameterFunctorSourceData.cpp b/Gems/Atom/Feature/Common/Code/Source/Material/SubsurfaceTransmissionParameterFunctorSourceData.cpp index a8ba40fb7f..ea447a422b 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Material/SubsurfaceTransmissionParameterFunctorSourceData.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/Material/SubsurfaceTransmissionParameterFunctorSourceData.cpp @@ -72,9 +72,9 @@ namespace AZ AddMaterialPropertyDependency(functor, functor->m_scatterDistanceColor); AddMaterialPropertyDependency(functor, functor->m_scatterDistanceIntensity); - functor->m_scatterDistance = context.GetShaderResourceGroupLayout()->FindShaderInputConstantIndex(Name{ m_scatterDistance }); - functor->m_transmissionParams = context.GetShaderResourceGroupLayout()->FindShaderInputConstantIndex(Name{ m_transmissionParams }); - functor->m_transmissionTintThickness = context.GetShaderResourceGroupLayout()->FindShaderInputConstantIndex(Name{ m_transmissionTintThickness }); + functor->m_scatterDistance = context.FindShaderInputConstantIndex(Name{ m_scatterDistance }); + functor->m_transmissionParams = context.FindShaderInputConstantIndex(Name{ m_transmissionParams }); + functor->m_transmissionTintThickness = context.FindShaderInputConstantIndex(Name{ m_transmissionTintThickness }); if (functor->m_scatterDistance.IsNull()) { diff --git a/Gems/Atom/Feature/Common/Code/Source/Material/Transform2DFunctorSourceData.cpp b/Gems/Atom/Feature/Common/Code/Source/Material/Transform2DFunctorSourceData.cpp index 26e7983a4b..dd7b9b891a 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Material/Transform2DFunctorSourceData.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/Material/Transform2DFunctorSourceData.cpp @@ -62,7 +62,7 @@ namespace AZ AddMaterialPropertyDependency(functor, functor->m_translateY); AddMaterialPropertyDependency(functor, functor->m_rotateDegrees); - functor->m_transformMatrix = context.GetShaderResourceGroupLayout()->FindShaderInputConstantIndex(Name{m_transformMatrix}); + functor->m_transformMatrix = context.FindShaderInputConstantIndex(Name{m_transformMatrix}); if (functor->m_transformMatrix.IsNull()) { @@ -74,7 +74,7 @@ namespace AZ // In that case, the.materialtype file will not provide the name of an inverse matrix because it doesn't have one. if (!m_transformMatrixInverse.empty()) { - functor->m_transformMatrixInverse = context.GetShaderResourceGroupLayout()->FindShaderInputConstantIndex(Name{m_transformMatrixInverse}); + functor->m_transformMatrixInverse = context.FindShaderInputConstantIndex(Name{m_transformMatrixInverse}); if (functor->m_transformMatrixInverse.IsNull()) { 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 70d92ffccf..9aa31af97a 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 @@ -284,8 +284,8 @@ namespace AZ //! Call back function type used with the enumeration functions. //! Return false to terminate the traversal. using EnumeratePropertyGroupsCallback = AZStd::function; //! Recursively traverses all of the property groups contained in the material type, executing a callback function for each. @@ -295,8 +295,8 @@ namespace AZ //! Call back function type used with the numeration functions. //! Return false to terminate the traversal. using EnumeratePropertiesCallback = AZStd::function; //! Recursively traverses all of the properties contained in the material type, executing a callback function for each. diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialUtils.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialUtils.h index 6ab7132109..4d1c61146d 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialUtils.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialUtils.h @@ -28,6 +28,8 @@ namespace AZ namespace MaterialUtils { + using ImportedJsonFiles = AZStd::unordered_set; + enum class GetImageAssetResult { Empty, //! No image was actually requested, the path was empty @@ -52,7 +54,8 @@ namespace AZ //! Otherwise, it will use the passed in document first if not null, or load the json document from the path. //! @param filePath path to the JSON file to load, unless the @document is already provided. In either case, this path will be used to resolve any relative file references. //! @param document an optional already loaded json document. - AZ::Outcome LoadMaterialTypeSourceData(const AZStd::string& filePath, rapidjson::Document* document = nullptr); + //! @param importedFiles receives the list of files that were imported by the JSON serializer + AZ::Outcome LoadMaterialTypeSourceData(const AZStd::string& filePath, rapidjson::Document* document = nullptr, ImportedJsonFiles* importedFiles = nullptr); //! Utility function for custom JSON serializers to report results as "Skipped" when encountering keys that aren't recognized //! as part of the custom format. diff --git a/Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.cpp b/Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.cpp index bf65a09cef..5858c3e675 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.cpp @@ -187,13 +187,21 @@ namespace AZ const bool isMaterialTypeFile = AzFramework::StringFunc::Path::IsExtension(request.m_sourceFile.c_str(), MaterialTypeSourceData::Extension); if (isMaterialTypeFile) { - auto materialTypeSourceData = MaterialUtils::LoadMaterialTypeSourceData(fullSourcePath, &document); + MaterialUtils::ImportedJsonFiles importedJsonFiles; + auto materialTypeSourceData = MaterialUtils::LoadMaterialTypeSourceData(fullSourcePath, &document, &importedJsonFiles); if (!materialTypeSourceData.IsSuccess()) { return; } + for (auto& importedJsonFile : importedJsonFiles) + { + AssetBuilderSDK::SourceFileDependency sourceDependency; + sourceDependency.m_sourceFileDependencyPath = importedJsonFile; + response.m_sourceFileDependencyList.push_back(sourceDependency); + } + for (auto& shader : materialTypeSourceData.GetValue().m_shaderCollection) { AddPossibleDependencies(request.m_sourceFile, diff --git a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/LuaMaterialFunctorSourceData.cpp b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/LuaMaterialFunctorSourceData.cpp index c614222ee6..2477360b8c 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/LuaMaterialFunctorSourceData.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/LuaMaterialFunctorSourceData.cpp @@ -198,7 +198,10 @@ namespace AZ for (const Name& materialProperty : materialPropertyDependencies.GetValue()) { - MaterialPropertyIndex index = propertiesLayout->FindPropertyIndex(Name{m_propertyNamePrefix + materialProperty.GetCStr()}); + Name propertyName{materialProperty.GetCStr()}; + functor->m_materialNameContext.ContextualizeProperty(propertyName); + + MaterialPropertyIndex index = propertiesLayout->FindPropertyIndex(propertyName); if (!index.IsValid()) { AZ_Error("LuaMaterialFunctorSourceData", false, "Property '%s' is not found in material type.", materialProperty.GetCStr()); 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 9b251213fe..725041eab4 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp @@ -381,13 +381,13 @@ namespace AZ { for (auto& propertyGroup : inPropertyGroupList) { - if (!callback(materialNameContext, propertyGroup.get())) + MaterialNameContext materialNameContext2 = ExtendNameContext(materialNameContext, *propertyGroup); + + if (!callback(propertyGroup.get(), materialNameContext2)) { return false; // Stop processing } - MaterialNameContext materialNameContext2 = ExtendNameContext(materialNameContext, *propertyGroup); - if (!EnumeratePropertyGroups(callback, materialNameContext2, propertyGroup->m_propertyGroups)) { return false; // Stop processing @@ -415,7 +415,7 @@ namespace AZ for (auto& property : propertyGroup->m_properties) { - if (!callback(materialNameContext2, property.get())) + if (!callback(property.get(), materialNameContext2)) { return false; // Stop processing } @@ -483,7 +483,7 @@ namespace AZ enumValues.push_back(uvNamePair.second); } - EnumerateProperties([&enumValues](const MaterialNameContext&, const MaterialTypeSourceData::PropertyDefinition* property) + EnumerateProperties([&enumValues](const MaterialTypeSourceData::PropertyDefinition* property, const MaterialNameContext&) { if (property->m_dataType == AZ::RPI::MaterialPropertyDataType::Enum && property->m_enumIsUv) { diff --git a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialUtils.cpp b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialUtils.cpp index be5feccb80..926d9abb74 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialUtils.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialUtils.cpp @@ -74,7 +74,7 @@ namespace AZ return true; } - AZ::Outcome LoadMaterialTypeSourceData(const AZStd::string& filePath, rapidjson::Document* document) + AZ::Outcome LoadMaterialTypeSourceData(const AZStd::string& filePath, rapidjson::Document* document, ImportedJsonFiles* importedFiles) { rapidjson::Document localDocument; @@ -96,6 +96,10 @@ namespace AZ importSettings.m_importer = &jsonImporter; importSettings.m_loadedJsonPath = filePath; AZ::JsonSerializationResult::ResultCode result = AZ::JsonSerialization::ResolveImports(document->GetObject(), document->GetAllocator(), importSettings); + if (importedFiles) + { + *importedFiles = importSettings.m_importer->GetImportedFiles(); + } MaterialTypeSourceData materialType; diff --git a/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeSourceDataTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeSourceDataTests.cpp index 98a86fe9ce..04b55b3aaf 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeSourceDataTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/MaterialTypeSourceDataTests.cpp @@ -486,8 +486,8 @@ namespace UnitTest struct EnumeratePropertyGroupsResult { - MaterialNameContext m_materialNameContext; const MaterialTypeSourceData::PropertyGroup* m_propertyGroup; + MaterialNameContext m_materialNameContext; void Check(AZStd::string expectedIdContext, const MaterialTypeSourceData::PropertyGroup* expectedPropertyGroup) { @@ -502,9 +502,9 @@ namespace UnitTest }; AZStd::vector enumeratePropertyGroupsResults; - sourceData.EnumeratePropertyGroups([&enumeratePropertyGroupsResults](const MaterialNameContext& materialNameContext, const MaterialTypeSourceData::PropertyGroup* propertyGroup) + sourceData.EnumeratePropertyGroups([&enumeratePropertyGroupsResults](const MaterialTypeSourceData::PropertyGroup* propertyGroup, const MaterialNameContext& nameContext) { - enumeratePropertyGroupsResults.push_back(EnumeratePropertyGroupsResult{materialNameContext, propertyGroup}); + enumeratePropertyGroupsResults.push_back(EnumeratePropertyGroupsResult{propertyGroup, nameContext}); return true; }); @@ -525,8 +525,8 @@ namespace UnitTest struct EnumeratePropertiesResult { - MaterialNameContext m_materialNameContext; const MaterialTypeSourceData::PropertyDefinition* m_propertyDefinition; + MaterialNameContext m_materialNameContext; void Check(AZStd::string expectedIdContext, const MaterialTypeSourceData::PropertyDefinition* expectedPropertyDefinition) { @@ -541,9 +541,9 @@ namespace UnitTest }; AZStd::vector enumeratePropertiesResults; - sourceData.EnumerateProperties([&enumeratePropertiesResults](const MaterialNameContext& materialNameContext, const MaterialTypeSourceData::PropertyDefinition* propertyDefinition) + sourceData.EnumerateProperties([&enumeratePropertiesResults](const MaterialTypeSourceData::PropertyDefinition* propertyDefinition, const MaterialNameContext& nameContext) { - enumeratePropertiesResults.push_back(EnumeratePropertiesResult{materialNameContext, propertyDefinition}); + enumeratePropertiesResults.push_back(EnumeratePropertiesResult{propertyDefinition, nameContext}); return true; }); diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp index aaad77266c..9349ad4dc9 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp @@ -586,10 +586,10 @@ namespace MaterialEditor bool result = true; // populate sourceData with properties that meet the filter - m_materialTypeSourceData.EnumerateProperties([&](const MaterialNameContext& materialNameContext, const auto& propertyDefinition) { + m_materialTypeSourceData.EnumerateProperties([&](const auto& propertyDefinition, const MaterialNameContext& nameContext) { Name propertyId{propertyDefinition->GetName()}; - materialNameContext.ContextualizeProperty(propertyId); + nameContext.ContextualizeProperty(propertyId); const auto it = m_properties.find(propertyId); if (it != m_properties.end() && propertyFilter(it->second)) @@ -783,17 +783,15 @@ namespace MaterialEditor // Populate the property map from a combination of source data and assets // Assets must still be used for now because they contain the final accumulated value after all other materials // in the hierarchy are applied - m_materialTypeSourceData.EnumeratePropertyGroups([this, &parentPropertyValues](const MaterialNameContext& materialNameContext, const MaterialTypeSourceData::PropertyGroup* propertyGroup) + m_materialTypeSourceData.EnumeratePropertyGroups([this, &parentPropertyValues](const MaterialTypeSourceData::PropertyGroup* propertyGroup, const MaterialNameContext& nameContext) { AtomToolsFramework::DynamicPropertyConfig propertyConfig; for (const auto& propertyDefinition : propertyGroup->GetProperties()) { // Assign id before conversion so it can be used in dynamic description - MaterialNameContext groupNameContext = materialNameContext; - groupNameContext.ExtendPropertyIdContext(propertyGroup->GetName()); propertyConfig.m_id = propertyDefinition->GetName(); - groupNameContext.ContextualizeProperty(propertyConfig.m_id); + nameContext.ContextualizeProperty(propertyConfig.m_id); const auto& propertyIndex = m_materialAsset->GetMaterialPropertiesLayout()->FindPropertyIndex(propertyConfig.m_id); const bool propertyIndexInBounds = propertyIndex.IsValid() && propertyIndex.GetIndex() < m_materialAsset->GetPropertyValues().size(); @@ -907,7 +905,7 @@ namespace MaterialEditor // Add any material functors that are located inside each property group. bool enumerateResult = m_materialTypeSourceData.EnumeratePropertyGroups( - [this](const MaterialNameContext& nameContext, const MaterialTypeSourceData::PropertyGroup* propertyGroup) + [this](const MaterialTypeSourceData::PropertyGroup* propertyGroup, const MaterialNameContext& nameContext) { const MaterialFunctorSourceData::EditorContext editorContext = MaterialFunctorSourceData::EditorContext( m_materialSourceData.m_materialType, m_materialAsset->GetMaterialPropertiesLayout(), &nameContext); diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentUtil.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentUtil.cpp index 320d5e2e08..fd66243cdc 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentUtil.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentUtil.cpp @@ -114,10 +114,10 @@ namespace AZ // Copy all of the properties from the material asset to the source data that will be exported bool result = true; - editData.m_materialTypeSourceData.EnumerateProperties([&](const AZ::RPI::MaterialNameContext& materialNameContext, const AZ::RPI::MaterialTypeSourceData::PropertyDefinition* propertyDefinition) + editData.m_materialTypeSourceData.EnumerateProperties([&](const AZ::RPI::MaterialTypeSourceData::PropertyDefinition* propertyDefinition, const AZ::RPI::MaterialNameContext& nameContext) { AZ::Name propertyId{propertyDefinition->GetName()}; - materialNameContext.ContextualizeProperty(propertyId); + nameContext.ContextualizeProperty(propertyId); const AZ::RPI::MaterialPropertyIndex propertyIndex = editData.m_materialAsset->GetMaterialPropertiesLayout()->FindPropertyIndex(propertyId);