diff --git a/Gems/Atom/Feature/Common/Code/Source/Material/MaterialConverterSystemComponent.cpp b/Gems/Atom/Feature/Common/Code/Source/Material/MaterialConverterSystemComponent.cpp index fd1c836b96..d5096d7a0b 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Material/MaterialConverterSystemComponent.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/Material/MaterialConverterSystemComponent.cpp @@ -28,8 +28,7 @@ namespace AZ serializeContext->Class() ->Version(2) ->Field("Enable", &MaterialConverterSettings::m_enable) - ->Field("DefaultMaterial", &MaterialConverterSettings::m_defaultMaterial) - ->Field("IncludeMaterialPropertyNames", &MaterialConverterSettings::m_includeMaterialPropertyNames); + ->Field("DefaultMaterial", &MaterialConverterSettings::m_defaultMaterial); } } @@ -70,11 +69,6 @@ namespace AZ return m_settings.m_enable; } - bool MaterialConverterSystemComponent::ShouldIncludeMaterialPropertyNames() const - { - return m_settings.m_includeMaterialPropertyNames; - } - bool MaterialConverterSystemComponent::ConvertMaterial( const AZ::SceneAPI::DataTypes::IMaterialData& materialData, RPI::MaterialSourceData& sourceData) { diff --git a/Gems/Atom/Feature/Common/Code/Source/Material/MaterialConverterSystemComponent.h b/Gems/Atom/Feature/Common/Code/Source/Material/MaterialConverterSystemComponent.h index 150529b474..7d95024759 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Material/MaterialConverterSystemComponent.h +++ b/Gems/Atom/Feature/Common/Code/Source/Material/MaterialConverterSystemComponent.h @@ -26,11 +26,6 @@ namespace AZ bool m_enable = true; AZStd::string m_defaultMaterial; - //! Sets whether to include material property names when generating material assets. If this - //! setting is true, material property name resolution and validation is deferred into load - //! time rather than at build time, allowing to break some dependencies (e.g. fbx files will no - //! longer need to be dependent on materialtype files). - bool m_includeMaterialPropertyNames = true; }; //! Atom's implementation of converting SceneAPI data into Atom's default material: StandardPBR @@ -50,7 +45,6 @@ namespace AZ // MaterialConverterBus overrides ... bool IsEnabled() const override; - bool ShouldIncludeMaterialPropertyNames() const override; bool ConvertMaterial(const AZ::SceneAPI::DataTypes::IMaterialData& materialData, RPI::MaterialSourceData& out) override; AZStd::string GetMaterialTypePath() const override; AZStd::string GetDefaultMaterialPath() const override; diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialConverterBus.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialConverterBus.h index 1807fca15e..8052fc4feb 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialConverterBus.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialConverterBus.h @@ -32,10 +32,6 @@ namespace AZ virtual bool IsEnabled() const = 0; - //! Returns true if material property names should be included in azmaterials. This allows unlinking of dependencies for some - //! file types to materialtype files (e.g. fbx). - virtual bool ShouldIncludeMaterialPropertyNames() const = 0; - //! Converts data from a IMaterialData object to an Atom MaterialSourceData. //! Only works when IsEnabled() is true. //! @return true if the MaterialSourceData output was populated with converted material data. diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialSourceData.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialSourceData.h index 53d3072370..a5fb0214e6 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialSourceData.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialSourceData.h @@ -33,6 +33,12 @@ namespace AZ class MaterialAsset; class MaterialAssetCreator; + enum MaterialAssetProcessingMode + { + PreBake, //!< all material asset processing is done in the Asset Processor, producing a finalized material asset + DeferredBake //!< some material asset processing is deferred, and the material asset is finalized at runtime after loading + }; + //! This is a simple data structure for serializing in/out material source files. class MaterialSourceData final { @@ -72,35 +78,28 @@ namespace AZ UpdatesApplied }; - //! Checks the material type version and potentially applies a series of property changes (most common are simple property renames) - //! based on the MaterialTypeAsset's version update procedure. - //! @param materialSourceFilePath Indicates the path of the .material file that the MaterialSourceData represents. Used for resolving file-relative paths. - ApplyVersionUpdatesResult ApplyVersionUpdates(AZStd::string_view materialSourceFilePath = ""); - //! Creates a MaterialAsset from the MaterialSourceData content. //! @param assetId ID for the MaterialAsset //! @param materialSourceFilePath Indicates the path of the .material file that the MaterialSourceData represents. Used for //! resolving file-relative paths. + //! @param processingMode Indicates whether to finalize the material asset using data from the MaterialTypeAsset. //! @param elevateWarnings Indicates whether to treat warnings as errors - //! @param includeMaterialPropertyNames Indicates whether to save material property names into the material asset file Outcome> CreateMaterialAsset( Data::AssetId assetId, - AZStd::string_view materialSourceFilePath = "", - bool elevateWarnings = true, - bool includeMaterialPropertyNames = true) const; + AZStd::string_view materialSourceFilePath, + MaterialAssetProcessingMode processingMode, + bool elevateWarnings = true) const; //! Creates a MaterialAsset from the MaterialSourceData content. //! @param assetId ID for the MaterialAsset //! @param materialSourceFilePath Indicates the path of the .material file that the MaterialSourceData represents. Used for //! resolving file-relative paths. //! @param elevateWarnings Indicates whether to treat warnings as errors - //! @param includeMaterialPropertyNames Indicates whether to save material property names into the material asset file //! @param sourceDependencies if not null, will be populated with a set of all of the loaded material and material type paths Outcome> CreateMaterialAssetFromSourceData( Data::AssetId assetId, AZStd::string_view materialSourceFilePath = "", bool elevateWarnings = true, - bool includeMaterialPropertyNames = true, AZStd::unordered_set* sourceDependencies = nullptr) const; private: 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 c1183c7aa1..2fb55e5f40 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 @@ -64,6 +64,11 @@ namespace AZ void CheckForUnrecognizedJsonFields( const AZStd::string_view* acceptedFieldNames, uint32_t acceptedFieldNameCount, const rapidjson::Value& object, JsonDeserializerContext& context, JsonSerializationResult::ResultCode& result); + + //! Materials assets can either be finalized during asset-processing time or when materials are loaded at runtime. + //! Finalizing during asset processing reduces load times and obfuscates the material data. + //! Waiting to finalize at load time reduces dependencies on the material type data, resulting in fewer asset rebuilds and less time spent processing assets. + bool BuildersShouldFinalizeMaterialAssets(); } } } diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialAsset.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialAsset.h index 2a1de6debd..34e51b40af 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialAsset.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialAsset.h @@ -105,6 +105,16 @@ namespace AZ //! Returns a layout that includes a list of MaterialPropertyDescriptors for each material property. const MaterialPropertiesLayout* GetMaterialPropertiesLayout() const; + //! Returns whether the material's properties are fully processed or not. + //! If true, property values can be accessed through GetPropertyValues(). + //! If false, property values can be accessed through GetRawPropertyValues(). + bool IsFinalized() const; + + //! If the material asset is not finalized yet, this does the final processing of m_rawPropertyValues to + //! get the material asset ready to be used. + //! Note m_materialTypeAsset must be valid before this is called. + void Finalize(); + //! Returns the list of values for all properties in this material. //! The entries in this list align with the entries in the MaterialPropertiesLayout. Each AZStd::any is guaranteed //! to have a value of type that matches the corresponding MaterialPropertyDescriptor. @@ -112,16 +122,13 @@ namespace AZ //! //! Note that even though material source data files contain only override values and inherit the rest from //! their parent material, they all get flattened at build time so every MaterialAsset has the full set of values. - AZStd::array_view GetPropertyValues() const; + const AZStd::vector& GetPropertyValues() const; + + const AZStd::vector>& GetRawPropertyValues() const; private: bool PostLoadInit() override; - //! Realigns property value and name indices with MaterialProperiesLayout by using m_propertyNames. Property names not found in the - //! MaterialPropertiesLayout are discarded, while property names not included in m_propertyNames will use the default value - //! from m_materialTypeAsset. - void RealignPropertyValuesAndNames(); - //! Checks the material type version and potentially applies a series of property changes (most common are simple property renames) //! based on the MaterialTypeAsset's version update procedure. void ApplyVersionUpdates(); @@ -146,17 +153,33 @@ namespace AZ //! Holds values for each material property, used to initialize Material instances. //! This is indexed by MaterialPropertyIndex and aligns with entries in m_materialPropertiesLayout. AZStd::vector m_propertyValues; - //! This is used to realign m_propertyValues as well as itself with MaterialPropertiesLayout when not empty. - //! If empty, this implies that m_propertyValues is aligned with the entries in m_materialPropertiesLayout. - AZStd::vector m_propertyNames; - //! The materialTypeVersion this materialAsset was based of. If the versions do not match at runtime when a - //! materialTypeAsset is loaded, an update will be performed on m_propertyNames if populated. + //! The MaterialAsset can be created in a "half-baked" state where minimal processing has been done because it does + //! not yet have access to the MaterialTypeAsset. In that case, this list will be populated with values copied from + //! the source .material file with little or no validation or other processing, and the m_propertyValues list will be empty. + //! Once a MaterialTypeAsset is available, Finalize() must be called to finish processing these values into the + //! final m_propertyValues list. + //! Note that the content of this list will remain after finalizing in order to support hot-reload of the MaterialTypeAsset. + //! The reason we use a vector instead of a map is to ensure inherited property values are applied in the right order; + //! if the material has a parent, and that parent uses an older material type version with renamed properties, then + //! m_rawPropertyValues could be holding two values for the same property under different names. The auto-rename process + //! can't be applied until the MaterialTypeAsset is available, so we have to keep the properties in the same order they + //! were originally encountered. + AZStd::vector> m_rawPropertyValues; + + //! Tracks whether Finalize() has been called, meaning m_propertyValues is populated with data matching the material type's property layout. + bool m_isFinalized = false; + + //! Tracks whether the MaterialAsset was already in a finalized state when it was loaded. + //! (This value is intentionally not serialized) + bool m_wasPreFinalized = false; + + //! The materialTypeVersion this materialAsset was based off. If the versions do not match at runtime when a + //! materialTypeAsset is loaded, automatic updates will be attempted at runtime. Note this is not needed to + //! determine which updates to apply, but simply as an optimization to ignore the update procedure when the + //! version numbers match. (We determine which updates to apply by simply checking the property name, and not + //! allowing the same name to ever be used for two different properties, see MaterialTypeAssetCreator::ValidateMaterialVersion) uint32_t m_materialTypeVersion = 1; - - //! A flag to determine if m_propertyValues needs to be aligned with MaterialPropertiesLayout. Set to true whenever - //! m_materialTypeAsset is reinitializing. - bool m_isDirty = true; }; diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialAssetCreator.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialAssetCreator.h index 862d98ce0c..b7b1f9705f 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialAssetCreator.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialAssetCreator.h @@ -9,7 +9,6 @@ #include #include -#include namespace AZ { @@ -19,21 +18,21 @@ namespace AZ //! The MaterialAsset will be based on a MaterialTypeAsset or another MaterialAsset. //! Either way, the base provides the necessary data to define the layout //! and behavior of the material. The MaterialAsset only provides property value overrides. - class MaterialAssetCreator + //! Note however that the MaterialTypeAsset does not have to be loaded and available yet; + //! only the AssetId is required. The resulting MaterialAsset will be in a non-finalized state; + //! it must be finalized afterwards when the MaterialTypeAsset is available before it can be used. + class MaterialAssetCreator : public AssetCreator - , public MaterialAssetCreatorCommon { public: friend class MaterialSourceData; - - void Begin(const Data::AssetId& assetId, MaterialAsset& parentMaterial, bool includeMaterialPropertyNames = true); - void Begin(const Data::AssetId& assetId, MaterialTypeAsset& materialType, bool includeMaterialPropertyNames = true); + + void Begin(const Data::AssetId& assetId, const Data::Asset& materialType); bool End(Data::Asset& result); - private: - void PopulatePropertyNameList(); - - const MaterialPropertiesLayout* m_materialPropertiesLayout = nullptr; + void SetMaterialTypeVersion(uint32_t version); + + void SetPropertyValue(const Name& name, const MaterialPropertyValue& value); }; } // namespace RPI } // namespace AZ diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialPropertyValue.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialPropertyValue.h index 718615eb9f..79dda1d80a 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialPropertyValue.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialPropertyValue.h @@ -30,6 +30,10 @@ namespace AZ { namespace RPI { + //! This is a variant data type that represents the value of a material property. + //! For convenience, it supports all the types necessary for *both* the runtime data (MaterialAsset) as well as .material file data (MaterialSourceData). + //! For example, Instance is exclusive to the runtime data and AZStd::string is primarily for image file paths in .material files. Most other + //! data types are relevant in both contexts. class MaterialPropertyValue final { public: 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 f11b2f94ac..1b5635a1c1 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.cpp @@ -43,17 +43,24 @@ namespace AZ const char* MaterialBuilder::JobKey = "Atom Material Builder"; + AZStd::string GetBuilderSettingsFingerprint() + { + return AZStd::string::format("[BuildersShouldFinalizeMaterialAssets=%d]", MaterialUtils::BuildersShouldFinalizeMaterialAssets()); + } + void MaterialBuilder::RegisterBuilder() { AssetBuilderSDK::AssetBuilderDesc materialBuilderDescriptor; materialBuilderDescriptor.m_name = JobKey; - materialBuilderDescriptor.m_version = 110; // Material version auto update feature + materialBuilderDescriptor.m_version = 111; // material dependency improvements materialBuilderDescriptor.m_patterns.push_back(AssetBuilderSDK::AssetBuilderPattern("*.material", AssetBuilderSDK::AssetBuilderPattern::PatternType::Wildcard)); materialBuilderDescriptor.m_patterns.push_back(AssetBuilderSDK::AssetBuilderPattern("*.materialtype", AssetBuilderSDK::AssetBuilderPattern::PatternType::Wildcard)); materialBuilderDescriptor.m_busId = azrtti_typeid(); materialBuilderDescriptor.m_createJobFunction = AZStd::bind(&MaterialBuilder::CreateJobs, this, AZStd::placeholders::_1, AZStd::placeholders::_2); materialBuilderDescriptor.m_processJobFunction = AZStd::bind(&MaterialBuilder::ProcessJob, this, AZStd::placeholders::_1, AZStd::placeholders::_2); + materialBuilderDescriptor.m_analysisFingerprint = GetBuilderSettingsFingerprint(); + BusConnect(materialBuilderDescriptor.m_busId); AssetBuilderSDK::AssetBuilderBus::Broadcast(&AssetBuilderSDK::AssetBuilderBus::Handler::RegisterBuilderInformation, materialBuilderDescriptor); @@ -63,7 +70,7 @@ namespace AZ { BusDisconnect(); } - + bool MaterialBuilder::ReportMaterialAssetWarningsAsErrors() const { bool warningsAsErrors = false; @@ -77,15 +84,18 @@ namespace AZ //! Adds all relevant dependencies for a referenced source file, considering that the path might be relative to the original file location or a full asset path. //! This will usually include multiple source dependencies and a single job dependency, but will include only source dependencies if the file is not found. //! Note the AssetBuilderSDK::JobDependency::m_platformIdentifier will not be set by this function. The calling code must set this value before passing back - //! to the AssetBuilderSDK::CreateJobsResponse. If isOrderedOnceForMaterialTypes is true and the dependency is a .materialtype file, the job dependency type - //! will be set to JobDependencyType::OrderOnce. - void AddPossibleDependencies(AZStd::string_view currentFilePath, - AZStd::string_view referencedParentPath, + //! to the AssetBuilderSDK::CreateJobsResponse. + void AddPossibleDependencies( + const AZStd::string& currentFilePath, + const AZStd::string& referencedParentPath, const char* jobKey, - AZStd::vector& jobDependencies, - bool isOrderedOnceForMaterialTypes = false) + AZStd::vector& jobDependencies) { bool dependencyFileFound = false; + + const bool currentFileIsMaterial = AzFramework::StringFunc::Path::IsExtension(currentFilePath.c_str(), MaterialSourceData::Extension); + const bool referencedFileIsMaterialType = AzFramework::StringFunc::Path::IsExtension(referencedParentPath.c_str(), MaterialTypeSourceData::Extension); + const bool ShouldFinalizeMaterialAssets = MaterialUtils::BuildersShouldFinalizeMaterialAssets(); AZStd::vector possibleDependencies = RPI::AssetUtils::GetPossibleDepenencyPaths(currentFilePath, referencedParentPath); for (auto& file : possibleDependencies) @@ -103,9 +113,15 @@ namespace AZ AssetBuilderSDK::JobDependency jobDependency; jobDependency.m_jobKey = jobKey; jobDependency.m_sourceFile.m_sourceFileDependencyPath = file; - - const bool isMaterialTypeFile = AzFramework::StringFunc::Path::IsExtension(file.c_str(), MaterialTypeSourceData::Extension); - jobDependency.m_type = (isMaterialTypeFile && isOrderedOnceForMaterialTypes) ? AssetBuilderSDK::JobDependencyType::OrderOnce : AssetBuilderSDK::JobDependencyType::Order; + jobDependency.m_type = AssetBuilderSDK::JobDependencyType::Order; + + // If we aren't finalizing material assets, then a normal job dependency isn't needed because the MaterialTypeAsset data won't be used. + // However, we do still need at least an OrderOnce dependency to ensure the Asset Processor knows about the material type asset so the builder can get it's AssetId. + // This can significantly reduce AP processing time when a material type or its shaders are edited. + if (currentFileIsMaterial && referencedFileIsMaterialType && !ShouldFinalizeMaterialAssets) + { + jobDependency.m_type = AssetBuilderSDK::JobDependencyType::OrderOnce; + } jobDependencies.push_back(jobDependency); } @@ -156,7 +172,8 @@ namespace AZ // We'll build up this one JobDescriptor and reuse it to register each of the platforms AssetBuilderSDK::JobDescriptor outputJobDescriptor; outputJobDescriptor.m_jobKey = JobKey; - + outputJobDescriptor.m_additionalFingerprintInfo = GetBuilderSettingsFingerprint(); + // Load the file so we can detect and report dependencies. // If the file is a .materialtype, report dependencies on the .shader files. // If the file is a .material, report a dependency on the .materialtype and parent .material file @@ -233,24 +250,12 @@ namespace AZ parentMaterialPath = materialTypePath; } - // If includeMaterialPropertyNames is false, then a job dependency is needed so the material builder can validate MaterialAsset properties - // against the MaterialTypeAsset at asset build time. - // If includeMaterialPropertyNames is true, the material properties will be validated at runtime when the material is loaded, so the job dependency - // is needed only for first-time processing to set up the initial MaterialAsset. This speeds up AP processing time when a materialtype file - // is edited (e.g. 10s when editing StandardPBR.materialtype on AtomTest project from 45s). - bool includeMaterialPropertyNames = true; - if (auto settingsRegistry = AZ::SettingsRegistry::Get(); settingsRegistry != nullptr) - { - settingsRegistry->Get(includeMaterialPropertyNames, "/O3DE/Atom/RPI/MaterialBuilder/IncludeMaterialPropertyNames"); - } - // Register dependency on the parent material source file so we can load it and use it's data to build this variant material. // Note, we don't need a direct dependency on the material type because the parent material will depend on it. AddPossibleDependencies(request.m_sourceFile, parentMaterialPath, JobKey, - outputJobDescriptor.m_jobDependencyList, - includeMaterialPropertyNames); + outputJobDescriptor.m_jobDependencyList); } } @@ -297,12 +302,9 @@ namespace AZ return {}; } - if (MaterialSourceData::ApplyVersionUpdatesResult::Failed == material.GetValue().ApplyVersionUpdates(materialSourceFilePath)) - { - return {}; - } + MaterialAssetProcessingMode processingMode = MaterialUtils::BuildersShouldFinalizeMaterialAssets() ? MaterialAssetProcessingMode::PreBake : MaterialAssetProcessingMode::DeferredBake; - auto materialAssetOutcome = material.GetValue().CreateMaterialAsset(Uuid::CreateRandom(), materialSourceFilePath, ReportMaterialAssetWarningsAsErrors()); + auto materialAssetOutcome = material.GetValue().CreateMaterialAsset(Uuid::CreateRandom(), materialSourceFilePath, processingMode, ReportMaterialAssetWarningsAsErrors()); if (!materialAssetOutcome.IsSuccess()) { return {}; diff --git a/Gems/Atom/RPI/Code/Source/RPI.Builders/Model/MaterialAssetBuilderComponent.cpp b/Gems/Atom/RPI/Code/Source/RPI.Builders/Model/MaterialAssetBuilderComponent.cpp index c83761cf8e..ef251b6848 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Builders/Model/MaterialAssetBuilderComponent.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Builders/Model/MaterialAssetBuilderComponent.cpp @@ -28,6 +28,7 @@ #include #include +#include #include #include @@ -44,7 +45,7 @@ namespace AZ if (auto* serialize = azrtti_cast(context)) { serialize->Class() - ->Version(5) // Set materialtype dependency to OrderOnce + ->Version(5) // <<<<< This probably is NOT the version number you want to bump. What you're looking for is MaterialAssetBuilderComponent::Reflect below ->Attribute(Edit::Attributes::SystemComponentTags, AZStd::vector({ AssetBuilderSDK::ComponentTags::AssetBuilder })); } } @@ -93,9 +94,11 @@ namespace AZ // material properties will be validated at runtime when the material is loaded, so the job dependency is needed only for // first-time processing to set up the initial MaterialAsset. This speeds up AP processing time when a materialtype file is // edited (e.g. 10s when editing StandardPBR.materialtype on AtomTest project from 45s). - bool includeMaterialPropertyNames = true; - RPI::MaterialConverterBus::BroadcastResult(includeMaterialPropertyNames, &RPI::MaterialConverterBus::Events::ShouldIncludeMaterialPropertyNames); - jobDependency.m_type = includeMaterialPropertyNames ? AssetBuilderSDK::JobDependencyType::OrderOnce : AssetBuilderSDK::JobDependencyType::Order; + + // If we aren't finalizing material assets, then a normal job dependency isn't needed because the MaterialTypeAsset data won't be used. + // However, we do still need at least an OrderOnce dependency to ensure the Asset Processor knows about the material type asset so the builder can get it's AssetId. + // This can significantly reduce AP processing time when a material type or its shaders are edited. + jobDependency.m_type = MaterialUtils::BuildersShouldFinalizeMaterialAssets() ? AssetBuilderSDK::JobDependencyType::OrderOnce : AssetBuilderSDK::JobDependencyType::Order; jobDependencyList.push_back(jobDependency); } @@ -108,10 +111,8 @@ namespace AZ bool conversionEnabled = false; RPI::MaterialConverterBus::BroadcastResult(conversionEnabled, &RPI::MaterialConverterBus::Events::IsEnabled); fingerprintInfo.insert(AZStd::string::format("[MaterialConverter enabled=%d]", conversionEnabled)); - - bool includeMaterialPropertyNames = true; - RPI::MaterialConverterBus::BroadcastResult(includeMaterialPropertyNames, &RPI::MaterialConverterBus::Events::ShouldIncludeMaterialPropertyNames); - fingerprintInfo.insert(AZStd::string::format("[MaterialConverter includeMaterialPropertyNames=%d]", includeMaterialPropertyNames)); + + fingerprintInfo.insert(AZStd::string::format("[BuildersShouldFinalizeMaterialAssets=%d]", MaterialUtils::BuildersShouldFinalizeMaterialAssets())); if (!conversionEnabled) { @@ -126,7 +127,7 @@ namespace AZ if (auto* serialize = azrtti_cast(context)) { serialize->Class() - ->Version(16); // Optional material conversion + ->Version(17); // Optional material conversion } } @@ -230,9 +231,9 @@ namespace AZ } } } + + MaterialAssetProcessingMode processingMode = MaterialUtils::BuildersShouldFinalizeMaterialAssets() ? MaterialAssetProcessingMode::PreBake : MaterialAssetProcessingMode::DeferredBake; - bool includeMaterialPropertyNames = true; - RPI::MaterialConverterBus::BroadcastResult(includeMaterialPropertyNames, &RPI::MaterialConverterBus::Events::ShouldIncludeMaterialPropertyNames); // Build material assets. for (auto& itr : materialSourceDataByUid) { @@ -240,7 +241,7 @@ namespace AZ Data::AssetId assetId(sourceSceneUuid, GetMaterialAssetSubId(materialUid)); auto materialSourceData = itr.second; - Outcome> result = materialSourceData.m_data.CreateMaterialAsset(assetId, "", false, includeMaterialPropertyNames); + Outcome> result = materialSourceData.m_data.CreateMaterialAsset(assetId, "", processingMode, false); if (result.IsSuccess()) { context.m_outputMaterialsByUid[materialUid] = { result.GetValue(), materialSourceData.m_name }; diff --git a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceData.cpp b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceData.cpp index 4351213c22..cd01544bfa 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceData.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceData.cpp @@ -74,77 +74,49 @@ namespace AZ } } - MaterialSourceData::ApplyVersionUpdatesResult MaterialSourceData::ApplyVersionUpdates(AZStd::string_view materialSourceFilePath) - { - AZStd::string materialTypeFullPath = AssetUtils::ResolvePathReference(materialSourceFilePath, m_materialType); - auto materialTypeSourceDataOutcome = MaterialUtils::LoadMaterialTypeSourceData(materialTypeFullPath); - if (!materialTypeSourceDataOutcome.IsSuccess()) - { - return ApplyVersionUpdatesResult::Failed; - } - - MaterialTypeSourceData materialTypeSourceData = materialTypeSourceDataOutcome.TakeValue(); - - if (m_materialTypeVersion == materialTypeSourceData.m_version) - { - return ApplyVersionUpdatesResult::NoUpdates; - } - - bool changesWereApplied = false; - - // Note that the only kind of property update currently supported is rename... - - PropertyGroupMap newPropertyGroups; - for (auto& groupPair : m_properties) - { - PropertyMap& propertyMap = groupPair.second; - - for (auto& propertyPair : propertyMap) - { - MaterialPropertyId propertyId{groupPair.first, propertyPair.first}; - - if (materialTypeSourceData.ApplyPropertyRenames(propertyId, m_materialTypeVersion)) - { - changesWereApplied = true; - } - - newPropertyGroups[propertyId.GetGroupName().GetStringView()][propertyId.GetPropertyName().GetStringView()] = propertyPair.second; - } - } - - if (changesWereApplied) - { - m_properties = AZStd::move(newPropertyGroups); - - AZ_Warning( - "MaterialSourceData", false, - "This material is based on version '%u' of '%s', but the material type is now at version '%u'. " - "Automatic updates are available. Consider updating the .material source file: '%s'.", - m_materialTypeVersion, materialTypeFullPath.c_str(), materialTypeSourceData.m_version, materialSourceFilePath.data()); - } - - m_materialTypeVersion = materialTypeSourceData.m_version; - - return changesWereApplied ? ApplyVersionUpdatesResult::UpdatesApplied : ApplyVersionUpdatesResult::NoUpdates; - } - Outcome> MaterialSourceData::CreateMaterialAsset( - Data::AssetId assetId, AZStd::string_view materialSourceFilePath, bool elevateWarnings, bool includeMaterialPropertyNames) const + Data::AssetId assetId, AZStd::string_view materialSourceFilePath, MaterialAssetProcessingMode processingMode, bool elevateWarnings) const { MaterialAssetCreator materialAssetCreator; materialAssetCreator.SetElevateWarnings(elevateWarnings); - if (m_parentMaterial.empty()) + Outcome materialTypeAssetId = AssetUtils::MakeAssetId(materialSourceFilePath, m_materialType, 0); + if (!materialTypeAssetId) { - auto materialTypeAsset = AssetUtils::LoadAsset(materialSourceFilePath, m_materialType); - if (!materialTypeAsset.IsSuccess()) + return Failure(); + } + + Data::Asset materialTypeAsset; + + switch (processingMode) + { + case MaterialAssetProcessingMode::DeferredBake: { + // Don't load the material type data, just create a reference to it + materialTypeAsset = Data::Asset{ materialTypeAssetId.GetValue(), azrtti_typeid(), m_materialType }; + break; + } + case MaterialAssetProcessingMode::PreBake: + { + // In this case we need to load the material type data in preparation for the material->Finalize() step below. + auto materialTypeAssetOutcome = AssetUtils::LoadAsset(materialTypeAssetId.GetValue()); + if (!materialTypeAssetOutcome) + { + return Failure(); + } + materialTypeAsset = materialTypeAssetOutcome.GetValue(); + break; + } + default: + { + AZ_Assert(false, "Unhandled MaterialAssetProcessingMode"); return Failure(); } - - materialAssetCreator.Begin(assetId, *materialTypeAsset.GetValue().Get(), includeMaterialPropertyNames); } - else + + materialAssetCreator.Begin(assetId, materialTypeAsset); + + if (!m_parentMaterial.empty()) { auto parentMaterialAsset = AssetUtils::LoadAsset(materialSourceFilePath, m_parentMaterial); if (!parentMaterialAsset.IsSuccess()) @@ -154,25 +126,53 @@ namespace AZ // Make sure the parent material has the same material type { - auto materialTypeIdOutcome = AssetUtils::MakeAssetId(materialSourceFilePath, m_materialType, 0); - if (!materialTypeIdOutcome.IsSuccess()) - { - return Failure(); - } - - Data::AssetId expectedMaterialTypeId = materialTypeIdOutcome.GetValue(); - Data::AssetId parentMaterialId = parentMaterialAsset.GetValue().GetId(); - // This will only be valid if the parent material is not a material type Data::AssetId parentsMaterialTypeId = parentMaterialAsset.GetValue()->GetMaterialTypeAsset().GetId(); - if (expectedMaterialTypeId != parentMaterialId && expectedMaterialTypeId != parentsMaterialTypeId) + if (materialTypeAssetId.GetValue() != parentsMaterialTypeId) { AZ_Error("MaterialSourceData", false, "This material and its parent material do not share the same material type."); return Failure(); } } - materialAssetCreator.Begin(assetId, *parentMaterialAsset.GetValue().Get(), includeMaterialPropertyNames); + // Inherit the parent's property values... + switch (processingMode) + { + case MaterialAssetProcessingMode::DeferredBake: + { + for (auto& property : parentMaterialAsset.GetValue()->GetRawPropertyValues()) + { + materialAssetCreator.SetPropertyValue(property.first, property.second); + } + + break; + } + case MaterialAssetProcessingMode::PreBake: + { + const MaterialPropertiesLayout* propertiesLayout = parentMaterialAsset.GetValue()->GetMaterialPropertiesLayout(); + + if (parentMaterialAsset.GetValue()->GetPropertyValues().size() != propertiesLayout->GetPropertyCount()) + { + AZ_Assert(false, "The parent material should have been finalized with %zu properties but it has %zu. Something is out of sync.", + propertiesLayout->GetPropertyCount(), parentMaterialAsset.GetValue()->GetPropertyValues().size()); + return Failure(); + } + + for (size_t propertyIndex = 0; propertyIndex < propertiesLayout->GetPropertyCount(); ++propertyIndex) + { + materialAssetCreator.SetPropertyValue( + propertiesLayout->GetPropertyDescriptor(MaterialPropertyIndex{propertyIndex})->GetName(), + parentMaterialAsset.GetValue()->GetPropertyValues()[propertyIndex]); + } + + break; + } + default: + { + AZ_Assert(false, "Unhandled MaterialAssetProcessingMode"); + return Failure(); + } + } } ApplyPropertiesToAssetCreator(materialAssetCreator, materialSourceFilePath); @@ -180,6 +180,11 @@ namespace AZ Data::Asset material; if (materialAssetCreator.End(material)) { + if (processingMode == MaterialAssetProcessingMode::PreBake) + { + material->Finalize(); + } + return Success(material); } else @@ -192,7 +197,6 @@ namespace AZ Data::AssetId assetId, AZStd::string_view materialSourceFilePath, bool elevateWarnings, - bool includeMaterialPropertyNames, AZStd::unordered_set* sourceDependencies) const { const auto materialTypeSourcePath = AssetUtils::ResolvePathReference(materialSourceFilePath, m_materialType); @@ -270,7 +274,7 @@ namespace AZ // Create the material asset from all the previously loaded source data MaterialAssetCreator materialAssetCreator; materialAssetCreator.SetElevateWarnings(elevateWarnings); - materialAssetCreator.Begin(assetId, *materialTypeAsset.GetValue().Get(), includeMaterialPropertyNames); + materialAssetCreator.Begin(assetId, materialTypeAsset.GetValue()); while (!parentSourceDataStack.empty()) { @@ -283,6 +287,13 @@ namespace AZ Data::Asset material; if (materialAssetCreator.End(material)) { + // Unlike CreateMaterialAsset(), we can always finalize the material here because we loaded created the MaterialTypeAsset from + // the source .materialtype file, so the necessary data is always available. + // (In case you are wondering why we don't use CreateMaterialAssetFromSourceData in MaterialBuilder: that would require a + // source dependency between the .materialtype and .material file, which would cause all .material files to rebuild when you + // edit the .materialtype; it's faster to not read the material type data at all ... until it's needed at runtime) + material->Finalize(); + if (sourceDependencies) { sourceDependencies->insert(dependencies.begin(), dependencies.end()); @@ -306,59 +317,26 @@ namespace AZ { materialAssetCreator.ReportWarning("Source data for material property value is invalid."); } - else + else if (property.second.m_value.Is() && AzFramework::StringFunc::Contains(property.second.m_value.GetValue(), ".")) { - MaterialPropertyIndex propertyIndex = - materialAssetCreator.m_materialPropertiesLayout->FindPropertyIndex(propertyId.GetFullName()); - if (propertyIndex.IsValid()) - { - const MaterialPropertyDescriptor* propertyDescriptor = - materialAssetCreator.m_materialPropertiesLayout->GetPropertyDescriptor(propertyIndex); - switch (propertyDescriptor->GetDataType()) - { - case MaterialPropertyDataType::Image: - { - Data::Asset imageAsset; + Data::Asset imageAsset; - MaterialUtils::GetImageAssetResult result = MaterialUtils::GetImageAssetReference( - imageAsset, materialSourceFilePath, property.second.m_value.GetValue()); + MaterialUtils::GetImageAssetResult result = MaterialUtils::GetImageAssetReference( + imageAsset, materialSourceFilePath, property.second.m_value.GetValue()); - if (result == MaterialUtils::GetImageAssetResult::Missing) - { - materialAssetCreator.ReportWarning( - "Material property '%s': Could not find the image '%s'", propertyId.GetFullName().GetCStr(), - property.second.m_value.GetValue().data()); - } - - imageAsset.SetAutoLoadBehavior(Data::AssetLoadBehavior::PreLoad); - materialAssetCreator.SetPropertyValue(propertyId.GetFullName(), imageAsset); - } - break; - case MaterialPropertyDataType::Enum: - { - AZ::Name enumName = AZ::Name(property.second.m_value.GetValue()); - uint32_t enumValue = propertyDescriptor->GetEnumValue(enumName); - if (enumValue == MaterialPropertyDescriptor::InvalidEnumValue) - { - materialAssetCreator.ReportError( - "Enum value '%s' couldn't be found in the 'enumValues' list", enumName.GetCStr()); - } - else - { - materialAssetCreator.SetPropertyValue(propertyId.GetFullName(), enumValue); - } - } - break; - default: - materialAssetCreator.SetPropertyValue(propertyId.GetFullName(), property.second.m_value); - break; - } - } - else + if (result == MaterialUtils::GetImageAssetResult::Missing) { materialAssetCreator.ReportWarning( - "Can not find property id '%s' in MaterialPropertyLayout", propertyId.GetFullName().GetStringView().data()); + "Material property '%s': Could not find the image '%s'", propertyId.GetFullName().GetCStr(), + property.second.m_value.GetValue().data()); } + + imageAsset.SetAutoLoadBehavior(Data::AssetLoadBehavior::PreLoad); + materialAssetCreator.SetPropertyValue(propertyId.GetFullName(), imageAsset); + } + else + { + materialAssetCreator.SetPropertyValue(propertyId.GetFullName(), property.second.m_value); } } } 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 7fff8d81bc..475c6ca219 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialUtils.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialUtils.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include @@ -135,6 +136,20 @@ namespace AZ } } } + + bool BuildersShouldFinalizeMaterialAssets() + { + // We default to the faster workflow for developers. Enable this registry setting when releasing the + // game for faster load times and obfuscation of material assets. + bool shouldFinalize = false; + + if (auto settingsRegistry = AZ::SettingsRegistry::Get(); settingsRegistry != nullptr) + { + settingsRegistry->Get(shouldFinalize, "/O3DE/Atom/RPI/MaterialBuilder/FinalizeMaterialAssets"); + } + + return shouldFinalize; + } } } } diff --git a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAsset.cpp b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAsset.cpp index 3c6947b83d..7962844736 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAsset.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAsset.cpp @@ -33,11 +33,12 @@ namespace AZ if (auto* serializeContext = azrtti_cast(context)) { serializeContext->Class() - ->Version(11) // Material version update + ->Version(13) // added m_rawPropertyValues ->Field("materialTypeAsset", &MaterialAsset::m_materialTypeAsset) ->Field("materialTypeVersion", &MaterialAsset::m_materialTypeVersion) ->Field("propertyValues", &MaterialAsset::m_propertyValues) - ->Field("propertyNames", &MaterialAsset::m_propertyNames) + ->Field("rawPropertyValues", &MaterialAsset::m_rawPropertyValues) + ->Field("isFinalized", &MaterialAsset::m_isFinalized) ; } } @@ -102,32 +103,95 @@ namespace AZ { return m_materialTypeAsset->GetMaterialPropertiesLayout(); } - - AZStd::array_view MaterialAsset::GetPropertyValues() const + + bool MaterialAsset::IsFinalized() const { - // If property names are included, they are used to re-arrange the property value list to align with the - // MaterialPropertiesLayout. This realignment would be necessary if the material type is updated with - // a new property layout, and a corresponding material is not reprocessed by the AP and continues using the - // old property layout. - if (!m_propertyNames.empty()) + if (m_isFinalized) { - const uint32_t materialTypeVersion = m_materialTypeAsset->GetVersion(); - if (m_materialTypeVersion < materialTypeVersion) - { - // It is possible that the material type has had some properties renamed. If that's the case, and this material - // is still referencing the old property layout, we need to apply any auto updates to rename those properties - // before using them to realign the property values. - const_cast(this)->ApplyVersionUpdates(); - } + AZ_Assert(GetMaterialPropertiesLayout() && m_propertyValues.size() == GetMaterialPropertiesLayout()->GetPropertyCount(), "MaterialAsset is marked as Finalized but does not have the right number of property values."); + } - if (m_isDirty) + return m_isFinalized; + } + + void MaterialAsset::Finalize() + { + if (IsFinalized()) + { + return; + } + + const uint32_t materialTypeVersion = m_materialTypeAsset->GetVersion(); + if (m_materialTypeVersion < materialTypeVersion) + { + // It is possible that the material type has had some properties renamed or otherwise updated. If that's the case, + // and this material is still referencing the old property layout, we need to apply any auto updates to rename those + // properties before using them to realign the property values. + ApplyVersionUpdates(); + } + + const MaterialPropertiesLayout* propertyLayout = GetMaterialPropertiesLayout(); + + AZStd::vector finalizedPropertyValues(m_materialTypeAsset->GetDefaultPropertyValues().begin(), m_materialTypeAsset->GetDefaultPropertyValues().end()); + + for (const auto& [name, value] : m_rawPropertyValues) + { + const MaterialPropertyIndex propertyIndex = propertyLayout->FindPropertyIndex(name); + if (propertyIndex.IsValid()) { - const_cast(this)->RealignPropertyValuesAndNames(); + const MaterialPropertyDescriptor* propertyDescriptor = propertyLayout->GetPropertyDescriptor(propertyIndex); + + if (value.Is() && propertyDescriptor->GetDataType() == MaterialPropertyDataType::Enum) + { + AZ::Name enumName = AZ::Name(value.GetValue()); + uint32_t enumValue = propertyDescriptor->GetEnumValue(enumName); + if (enumValue == MaterialPropertyDescriptor::InvalidEnumValue) + { + AZ_Error(s_debugTraceName, false, "Material property name \"%s\" has invalid enum value \"%s\".", name.GetCStr(), enumName.GetCStr()); + } + else + { + finalizedPropertyValues[propertyIndex.GetIndex()] = enumValue; + } + } + else if (value.Is() && propertyDescriptor->GetDataType() == MaterialPropertyDataType::Image) + { + // Here we assume that the material asset builder resolved any image source file paths to an ImageAsset reference. + // So the only way a string could be present is if it's an empty image path reference, meaning no image should be bound. + AZ_Assert(value.GetValue().empty(), "Material property '%s' references in image '%s'. Image file paths must be resolved by the material asset builder."); + + finalizedPropertyValues[propertyIndex.GetIndex()] = Data::Asset{}; + } + else + { + finalizedPropertyValues[propertyIndex.GetIndex()] = value; + } + } + else + { + AZ_Warning(s_debugTraceName, false, "Material property name \"%s\" is not found in the material properties layout and will not be used.", name.GetCStr()); } } + m_propertyValues.swap(finalizedPropertyValues); + + m_isFinalized = true; + } + + const AZStd::vector& MaterialAsset::GetPropertyValues() const + { + // This can't be done in MaterialAssetHandler::LoadAssetData because the MaterialTypeAsset isn't necessarily loaded at that point. + // And it can't be done in PostLoadInit() because that happens on the next frame which might be too late. So we finalize just-in-time + // when properties are accessed. + const_cast(this)->Finalize(); + return m_propertyValues; } + + const AZStd::vector>& MaterialAsset::GetRawPropertyValues() const + { + return m_rawPropertyValues; + } void MaterialAsset::SetReady() { @@ -173,34 +237,6 @@ namespace AZ } } - void MaterialAsset::RealignPropertyValuesAndNames() - { - const MaterialPropertiesLayout* propertyLayout = GetMaterialPropertiesLayout(); - AZStd::vector alignedPropertyValues(m_materialTypeAsset->GetDefaultPropertyValues().begin(), m_materialTypeAsset->GetDefaultPropertyValues().end()); - for (size_t i = 0; i < m_propertyNames.size(); ++i) - { - const MaterialPropertyIndex propertyIndex = propertyLayout->FindPropertyIndex(m_propertyNames[i]); - if (propertyIndex.IsValid()) - { - alignedPropertyValues[propertyIndex.GetIndex()] = m_propertyValues[i]; - } - else - { - AZ_Warning(s_debugTraceName, false, "Material property name \"%s\" is not found in the material properties layout and will not be used.", m_propertyNames[i].GetCStr()); - } - } - m_propertyValues.swap(alignedPropertyValues); - - const size_t propertyCount = propertyLayout->GetPropertyCount(); - m_propertyNames.resize(propertyCount); - for (size_t i = 0; i < propertyCount; ++i) - { - m_propertyNames[i] = propertyLayout->GetPropertyDescriptor(MaterialPropertyIndex{ i })->GetName(); - } - - m_isDirty = false; - } - void MaterialAsset::ApplyVersionUpdates() { if (m_materialTypeVersion == m_materialTypeAsset->GetVersion()) @@ -248,7 +284,13 @@ namespace AZ // This also covers the case where just the MaterialTypeAsset is reloaded and not the MaterialAsset. m_materialTypeAsset = newMaterialTypeAsset; - m_isDirty = true; + // If the material asset was not finalized on disk, then we clear the previously finalized property values to force re-finalize. + // This + if (!m_wasPreFinalized) + { + m_isFinalized = false; + m_propertyValues.clear(); + } // Notify interested parties that this MaterialAsset is changed and may require other data to reinitialize as well MaterialReloadNotificationBus::Event(GetId(), &MaterialReloadNotifications::OnMaterialAssetReinitialized, Data::Asset{this, AZ::Data::AssetLoadBehavior::PreLoad}); @@ -276,6 +318,7 @@ namespace AZ if (Base::LoadAssetData(asset, stream, assetLoadFilterCB) == Data::AssetHandler::LoadResult::LoadComplete) { asset.GetAs()->AssetInitBus::Handler::BusConnect(); + asset.GetAs()->m_wasPreFinalized = asset.GetAs()->m_isFinalized; return Data::AssetHandler::LoadResult::LoadComplete; } diff --git a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAssetCreator.cpp b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAssetCreator.cpp index b62a91a98d..4bee663791 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAssetCreator.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAssetCreator.cpp @@ -16,89 +16,19 @@ namespace AZ { namespace RPI { - void MaterialAssetCreator::Begin(const Data::AssetId& assetId, MaterialAsset& parentMaterial, bool includeMaterialPropertyNames) - { - BeginCommon(assetId); - - if (ValidateIsReady()) - { - m_asset->m_materialTypeAsset = parentMaterial.m_materialTypeAsset; - m_asset->m_materialTypeVersion = m_asset->m_materialTypeAsset->GetVersion(); - - if (!m_asset->m_materialTypeAsset) - { - ReportError("MaterialTypeAsset is null"); - return; - } - - m_materialPropertiesLayout = m_asset->GetMaterialPropertiesLayout(); - if (!m_materialPropertiesLayout) - { - ReportError("MaterialPropertiesLayout is null"); - return; - } - if (includeMaterialPropertyNames) - { - PopulatePropertyNameList(); - } - - // Note we don't have to check the validity of these property values because the parent material's AssetCreator already did that. - m_asset->m_propertyValues.assign(parentMaterial.GetPropertyValues().begin(), parentMaterial.GetPropertyValues().end()); - - auto warningFunc = [this](const char* message) - { - ReportWarning("%s", message); - }; - auto errorFunc = [this](const char* message) - { - ReportError("%s", message); - }; - MaterialAssetCreatorCommon::OnBegin(m_materialPropertiesLayout, &(m_asset->m_propertyValues), warningFunc, errorFunc); - } - } - - void MaterialAssetCreator::Begin(const Data::AssetId& assetId, MaterialTypeAsset& materialType, bool includeMaterialPropertyNames) + void MaterialAssetCreator::Begin(const Data::AssetId& assetId, const Data::Asset& materialType) { BeginCommon(assetId); if (ValidateIsReady()) { - m_asset->m_materialTypeAsset = { &materialType, AZ::Data::AssetLoadBehavior::PreLoad }; + m_asset->m_materialTypeAsset = materialType; - if (!m_asset->m_materialTypeAsset) - { - ReportError("MaterialTypeAsset is null"); - return; - } - m_asset->m_materialTypeVersion = m_asset->m_materialTypeAsset->GetVersion(); + m_asset->m_materialTypeAsset.SetAutoLoadBehavior(AZ::Data::AssetLoadBehavior::PreLoad); - m_materialPropertiesLayout = m_asset->GetMaterialPropertiesLayout(); - if (includeMaterialPropertyNames) - { - PopulatePropertyNameList(); - } - - if (!m_materialPropertiesLayout) - { - ReportError("MaterialPropertiesLayout is null"); - return; - } - - // Note we don't have to check the validity of these property values because the parent material's AssetCreator already did that. - m_asset->m_propertyValues.assign(materialType.GetDefaultPropertyValues().begin(), materialType.GetDefaultPropertyValues().end()); - - auto warningFunc = [this](const char* message) - { - ReportWarning("%s", message); - }; - auto errorFunc = [this](const char* message) - { - ReportError("%s", message); - }; - MaterialAssetCreatorCommon::OnBegin(m_materialPropertiesLayout, &(m_asset->m_propertyValues), warningFunc, errorFunc); } } - + bool MaterialAssetCreator::End(Data::Asset& result) { if (!ValidateIsReady()) @@ -106,20 +36,39 @@ namespace AZ return false; } - m_materialPropertiesLayout = nullptr; - MaterialAssetCreatorCommon::OnEnd(); - m_asset->SetReady(); return EndCommon(result); } - - void MaterialAssetCreator::PopulatePropertyNameList() + + void MaterialAssetCreator::SetMaterialTypeVersion(uint32_t version) { - for (int i = 0; i < m_materialPropertiesLayout->GetPropertyCount(); ++i) + if (ValidateIsReady()) { - MaterialPropertyIndex propertyIndex{ i }; - auto& propertyName = m_materialPropertiesLayout->GetPropertyDescriptor(propertyIndex)->GetName(); - m_asset->m_propertyNames.emplace_back(propertyName); + m_asset->m_materialTypeVersion = version; + } + } + + void MaterialAssetCreator::SetPropertyValue(const Name& name, const MaterialPropertyValue& value) + { + if (ValidateIsReady()) + { + // Here we are careful to keep the properties in the same order they were encountered. When the MaterialAsset + // is later finalized with a MaterialTypeAsset, there could be a version update procedure that includes renamed + // properties. So it's possible that the same property could be encountered twice but with two different names. + // Preserving the original order will ensure that the later properties still overwrite the earlier ones even after + // renames have been applied. + + auto iter = AZStd::find_if(m_asset->m_rawPropertyValues.begin(), m_asset->m_rawPropertyValues.end(), [&name](const AZStd::pair& pair) + { + return pair.first == name; + }); + + if (iter != m_asset->m_rawPropertyValues.end()) + { + m_asset->m_rawPropertyValues.erase(iter); + } + + m_asset->m_rawPropertyValues.emplace_back(name, value); } } diff --git a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialTypeAssetCreator.cpp b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialTypeAssetCreator.cpp index 46086dfecc..dd023c56b8 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialTypeAssetCreator.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialTypeAssetCreator.cpp @@ -122,6 +122,21 @@ namespace AZ return false; } + // We don't allow previously renamed property names to be reused for new properties. This would just complicate too many things, + // as every use of every property name (like in Material Component, or in scripts, for example) would have to have a version number + // associated with it, in order to know whether or which rename to apply. + for (size_t propertyIndex = 0; propertyIndex < m_asset->m_materialPropertiesLayout->GetPropertyCount(); ++propertyIndex) + { + Name originalPropertyName = m_asset->m_materialPropertiesLayout->GetPropertyDescriptor(MaterialPropertyIndex{propertyIndex})->GetName(); + Name newPropertyName = originalPropertyName; + if (versionUpdate.ApplyPropertyRenames(newPropertyName)) + { + ReportError("There was a material property named '%s' at material type version %d. This name cannot be reused for another property.", + originalPropertyName.GetCStr(), versionUpdate.GetVersion()); + return false; + } + } + prevVersion = versionUpdate.GetVersion(); } diff --git a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialVersionUpdate.cpp b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialVersionUpdate.cpp index f5dfe9e80c..f12c865eee 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialVersionUpdate.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialVersionUpdate.cpp @@ -77,18 +77,14 @@ namespace AZ { bool changesWereApplied = false; - for (auto& propertyName : materialAsset.m_propertyNames) + for (auto& [name, value] : materialAsset.m_rawPropertyValues) { - for (const auto& action : m_actions) + if (ApplyPropertyRenames(name)) { - if (propertyName == action.m_fromPropertyId) - { - propertyName = action.m_toPropertyId; - changesWereApplied = true; - } + changesWereApplied = true; } } - + return changesWereApplied; } diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp index 97d8d5e354..93da118b5b 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp @@ -680,12 +680,6 @@ namespace MaterialEditor return false; } m_materialTypeSourceData = materialTypeOutcome.GetValue(); - - if (MaterialSourceData::ApplyVersionUpdatesResult::Failed == m_materialSourceData.ApplyVersionUpdates(m_absolutePath)) - { - AZ_Error("MaterialDocument", false, "Material source data could not be auto updated to the latest version of the material type: '%s'.", m_materialSourceData.m_materialType.c_str()); - return false; - } } else if (AzFramework::StringFunc::Path::IsExtension(m_absolutePath.c_str(), MaterialTypeSourceData::Extension)) {