diff --git a/Gems/Atom/Feature/Common/Code/Source/Material/MaterialConverterSystemComponent.cpp b/Gems/Atom/Feature/Common/Code/Source/Material/MaterialConverterSystemComponent.cpp index 835aa90ceb..680ad5f690 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Material/MaterialConverterSystemComponent.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/Material/MaterialConverterSystemComponent.cpp @@ -82,9 +82,8 @@ namespace AZ // The source data for generating material asset sourceData.m_materialType = GetMaterialTypePath(); - auto handleTexture = [&materialData, &sourceData]( - const char* propertyTextureGroup, SceneAPI::DataTypes::IMaterialData::TextureMapType textureType) { - MaterialSourceData::PropertyMap& properties = sourceData.m_properties[propertyTextureGroup]; + auto handleTexture = [&materialData, &sourceData](const char* propertyTextureGroup, SceneAPI::DataTypes::IMaterialData::TextureMapType textureType) + { const AZStd::string& texturePath = materialData.GetTexture(textureType); // Check to see if the image asset exists. If not, skip this texture map and just disable it. @@ -101,7 +100,7 @@ namespace AZ if (assetFound) { - properties["textureMap"] = texturePath; + sourceData.SetPropertyValue(MaterialPropertyId{propertyTextureGroup, "textureMap"}, texturePath); } else if (!texturePath.empty()) { @@ -120,7 +119,7 @@ namespace AZ { anyPBRInUse = true; handleTexture("baseColor", SceneAPI::DataTypes::IMaterialData::TextureMapType::BaseColor); - sourceData.m_properties["baseColor"]["textureBlendMode"] = AZStd::string("Lerp"); + sourceData.SetPropertyValue(Name{"baseColor.textureBlendMode"}, AZStd::string("Lerp")); } else { @@ -135,10 +134,10 @@ namespace AZ if (baseColor.has_value()) { anyPBRInUse = true; - sourceData.m_properties["baseColor"]["color"] = toColor(baseColor.value()); + sourceData.SetPropertyValue(Name{"baseColor.color"}, toColor(baseColor.value())); } - sourceData.m_properties["opacity"]["factor"] = materialData.GetOpacity(); + sourceData.SetPropertyValue(Name{"opacity.factor"}, materialData.GetOpacity()); auto applyOptionalPropertiesFunc = [&sourceData, &anyPBRInUse](const auto& propertyGroup, const auto& propertyName, const auto& propertyOptional) { @@ -147,7 +146,7 @@ namespace AZ if (propertyOptional.has_value()) { anyPBRInUse = true; - sourceData.m_properties[propertyGroup][propertyName] = propertyOptional.value(); + sourceData.SetPropertyValue(MaterialPropertyId{propertyGroup, propertyName}, propertyOptional.value()); } }; @@ -160,7 +159,7 @@ namespace AZ applyOptionalPropertiesFunc("roughness", "useTexture", materialData.GetUseRoughnessMap()); handleTexture("emissive", SceneAPI::DataTypes::IMaterialData::TextureMapType::Emissive); - sourceData.m_properties["emissive"]["color"] = toColor(materialData.GetEmissiveColor()); + sourceData.SetPropertyValue(Name{"emissive.color"}, toColor(materialData.GetEmissiveColor())); applyOptionalPropertiesFunc("emissive", "intensity", materialData.GetEmissiveIntensity()); applyOptionalPropertiesFunc("emissive", "useTexture", materialData.GetUseEmissiveMap()); @@ -171,7 +170,7 @@ namespace AZ { // If it doesn't have the useColorMap property, then it's a non-PBR material and the baseColor // texture needs to be set to the diffuse color. - sourceData.m_properties["baseColor"]["color"] = toColor(materialData.GetDiffuseColor()); + sourceData.SetPropertyValue(Name{"baseColor.color"}, toColor(materialData.GetDiffuseColor())); } return true; } 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 8b782fe66c..b8c613fd12 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 @@ -59,17 +59,25 @@ namespace AZ uint32_t m_materialTypeVersion = 0; //!< The version of the material type that was used to configure this material - using PropertyMap = AZStd::map; - using PropertyGroupMap = AZStd::map; - - PropertyGroupMap m_properties; - enum class ApplyVersionUpdatesResult { Failed, NoUpdates, UpdatesApplied }; + + //! If the data was loaded from an old format file (i.e. where "properties" was a tree with property values nested under groups), + //! this converts to the new format where properties are stored in a flat list. + void ConvertToNewDataFormat(); + + // Note that even though we use an unordered map, the JSON serialization system is nice enough to sort the data when saving to JSON. + using PropertyValueMap = AZStd::unordered_map; + + void SetPropertyValue(const Name& propertyId, const MaterialPropertyValue& value); + const MaterialPropertyValue& GetPropertyValue(const Name& propertyId) const; + PropertyValueMap GetPropertyValues() const; + bool HasPropertyValue(const Name& propertyId) const; + void RemovePropertyValue(const Name& propertyId); //! Creates a MaterialAsset from the MaterialSourceData content. //! @param assetId ID for the MaterialAsset @@ -96,8 +104,16 @@ namespace AZ AZStd::unordered_set* sourceDependencies = nullptr) const; private: + void ApplyPropertiesToAssetCreator( AZ::RPI::MaterialAssetCreator& materialAssetCreator, const AZStd::string_view& materialSourceFilePath) const; + + // @deprecated: Don't use "properties" in JSON, use "propertyValues" instead. + using PropertyGroupMap = AZStd::unordered_map; + PropertyGroupMap m_propertiesOld; + + PropertyValueMap m_propertyValues; + MaterialPropertyValue m_invalidValue; }; } // namespace RPI } // namespace AZ 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 79c73495e0..23219cfa89 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 @@ -216,11 +216,11 @@ namespace AZ //! This field is unused, and has been replaced by MaterialTypeSourceData::m_version below. It is kept for legacy file compatibility to suppress warnings and errors. uint32_t m_versionOld = 0; - //! [Deprecated] Use m_propertyGroups instead + //! @deprecated: Use m_propertyGroups instead //! List of groups that will contain the available properties AZStd::vector m_groupsOld; - //! [Deprecated] Use m_propertyGroups instead + //! @deprecated: Use m_propertyGroups instead AZStd::map> m_propertiesOld; //! Collection of all available user-facing properties 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 2fb55e5f40..f6c10e003e 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 @@ -24,6 +24,7 @@ namespace AZ namespace RPI { + class MaterialSourceData; class MaterialTypeSourceData; namespace MaterialUtils @@ -48,11 +49,14 @@ namespace AZ //! @return if resolving is successful. An error will be reported if it fails. bool ResolveMaterialPropertyEnumValue(const MaterialPropertyDescriptor* propertyDescriptor, const AZ::Name& enumName, MaterialPropertyValue& outResolvedValue); - //! Load material type from a json file. If the file path is relative, the loaded json document must be provided. + //! Load a material type from a json file. If the file path is relative, the loaded json document must be provided. //! Otherwise, it will use the passed in document first if not null, or load the json document from the path. //! @param filePath a relative path if document is provided, an absolute path if document is not provided. //! @param document the loaded json document. AZ::Outcome LoadMaterialTypeSourceData(const AZStd::string& filePath, const rapidjson::Value* document = nullptr); + + //! Load a material from a json file. + AZ::Outcome LoadMaterialSourceData(const AZStd::string& filePath, const rapidjson::Value* document = nullptr, bool warningsAsErrors = false); //! 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 d55f202c03..bb0330e7b6 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.cpp @@ -52,7 +52,7 @@ namespace AZ { AssetBuilderSDK::AssetBuilderDesc materialBuilderDescriptor; materialBuilderDescriptor.m_name = JobKey; - materialBuilderDescriptor.m_version = 117; // new material type file format + materialBuilderDescriptor.m_version = 119; // new material file format 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(); @@ -129,38 +129,6 @@ namespace AZ } } - template - AZ::Outcome LoadSourceData(const rapidjson::Value& value, const AZStd::string& filePath) - { - MaterialSourceDataT material; - - JsonDeserializerSettings settings; - - JsonReportingHelper reportingHelper; - reportingHelper.Attach(settings); - - // This is required by some custom material serializers to support relative path references. - JsonFileLoadContext fileLoadContext; - fileLoadContext.PushFilePath(filePath); - settings.m_metadata.Add(fileLoadContext); - - JsonSerialization::Load(material, value, settings); - - if (reportingHelper.ErrorsReported()) - { - return AZ::Failure(); - } - else if (reportingHelper.WarningsReported()) - { - AZ_Error(MaterialBuilderName, false, "Warnings reported while loading '%s'", filePath.c_str()); - return AZ::Failure(); - } - else - { - return AZ::Success(AZStd::move(material)); - } - } - void MaterialBuilder::CreateJobs(const AssetBuilderSDK::CreateJobsRequest& request, AssetBuilderSDK::CreateJobsResponse& response) const { if (m_isShuttingDown) @@ -295,7 +263,7 @@ namespace AZ AZ::Data::Asset MaterialBuilder::CreateMaterialAsset(AZStd::string_view materialSourceFilePath, const rapidjson::Value& json) const { - auto material = LoadSourceData(json, materialSourceFilePath); + auto material = MaterialUtils::LoadMaterialSourceData(materialSourceFilePath, &json, true); if (!material.IsSuccess()) { 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 c2b80e726b..e5092b5480 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceData.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceData.cpp @@ -54,10 +54,11 @@ namespace AZ ->Field("materialType", &MaterialSourceData::m_materialType) ->Field("materialTypeVersion", &MaterialSourceData::m_materialTypeVersion) ->Field("parentMaterial", &MaterialSourceData::m_parentMaterial) - ->Field("properties", &MaterialSourceData::m_properties) + ->Field("properties", &MaterialSourceData::m_propertiesOld) + ->Field("propertyValues", &MaterialSourceData::m_propertyValues) ; - serializeContext->RegisterGenericType(); + serializeContext->RegisterGenericType(); serializeContext->RegisterGenericType(); } } @@ -73,6 +74,55 @@ namespace AZ } } + void MaterialSourceData::SetPropertyValue(const Name& propertyId, const MaterialPropertyValue& value) + { + if (!propertyId.IsEmpty()) + { + m_propertyValues[propertyId] = value; + } + } + + const MaterialPropertyValue& MaterialSourceData::GetPropertyValue(const Name& propertyId) const + { + auto iter = m_propertyValues.find(propertyId); + if (iter == m_propertyValues.end()) + { + return m_invalidValue; + } + else + { + return iter->second; + } + } + + void MaterialSourceData::RemovePropertyValue(const Name& propertyId) + { + m_propertyValues.erase(propertyId); + } + + MaterialSourceData::PropertyValueMap MaterialSourceData::GetPropertyValues() const + { + return m_propertyValues; + } + + bool MaterialSourceData::HasPropertyValue(const Name& propertyId) const + { + return m_propertyValues.find(propertyId) != m_propertyValues.end(); + } + + void MaterialSourceData::ConvertToNewDataFormat() + { + for (auto& [groupName, propertyList] : m_propertiesOld) + { + for (auto& [propertyName, propertyValue] : propertyList) + { + SetPropertyValue(MaterialPropertyId{groupName, propertyName}, propertyValue); + } + } + + m_propertiesOld.clear(); + } + Outcome> MaterialSourceData::CreateMaterialAsset( Data::AssetId assetId, AZStd::string_view materialSourceFilePath, MaterialAssetProcessingMode processingMode, bool elevateWarnings) const { @@ -250,12 +300,14 @@ namespace AZ return Failure(); } - MaterialSourceData parentSourceData; - if (!AZ::RPI::JsonUtils::LoadObjectFromFile(parentSourceAbsPath, parentSourceData)) + auto loadParentResult = MaterialUtils::LoadMaterialSourceData(parentSourceAbsPath); + if (!loadParentResult) { AZ_Error("MaterialSourceData", false, "Failed to load MaterialSourceData for parent material: '%s'.", parentSourceAbsPath.c_str()); return Failure(); } + + MaterialSourceData parentSourceData = loadParentResult.TakeValue(); // Make sure that all materials in the hierarchy share the same material type const auto parentTypeAssetId = AssetUtils::MakeAssetId(parentSourceAbsPath, parentSourceData.m_materialType, 0); @@ -314,30 +366,29 @@ namespace AZ void MaterialSourceData::ApplyPropertiesToAssetCreator( AZ::RPI::MaterialAssetCreator& materialAssetCreator, const AZStd::string_view& materialSourceFilePath) const { - for (auto& group : m_properties) + for (auto& [propertyId, propertyValue] : m_propertyValues) { - for (auto& property : group.second) + if (!propertyValue.IsValid()) + { + materialAssetCreator.ReportWarning("Source data for material property value is invalid."); + } + else { - MaterialPropertyId propertyId{ group.first, property.first }; - if (!property.second.IsValid()) - { - materialAssetCreator.ReportWarning("Source data for material property value is invalid."); - } // If the source value type is a string, there are two possible property types: Image and Enum. If there is a "." in // the string (for the extension) we assume it's an Image and look up the referenced Asset. Otherwise, we can assume // it's an Enum value and just preserve the original string. - else if (property.second.Is() && AzFramework::StringFunc::Contains(property.second.GetValue(), ".")) + if (propertyValue.Is() && AzFramework::StringFunc::Contains(propertyValue.GetValue(), ".")) { Data::Asset imageAsset; MaterialUtils::GetImageAssetResult result = MaterialUtils::GetImageAssetReference( - imageAsset, materialSourceFilePath, property.second.GetValue()); + imageAsset, materialSourceFilePath, propertyValue.GetValue()); if (result == MaterialUtils::GetImageAssetResult::Missing) { materialAssetCreator.ReportWarning( "Material property '%s': Could not find the image '%s'", propertyId.GetCStr(), - property.second.GetValue().data()); + propertyValue.GetValue().data()); } imageAsset.SetAutoLoadBehavior(Data::AssetLoadBehavior::PreLoad); @@ -345,7 +396,7 @@ namespace AZ } else { - materialAssetCreator.SetPropertyValue(propertyId, property.second); + materialAssetCreator.SetPropertyValue(propertyId, propertyValue); } } } 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 b9a5754732..23976cf357 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp @@ -100,9 +100,9 @@ namespace AZ serializeContext->Class() ->Version(3) // Added propertyGroups - ->Field("version", &PropertyLayout::m_versionOld) //< Deprecated, preserved for backward compatibility, replaced by MaterialTypeSourceData::version - ->Field("groups", &PropertyLayout::m_groupsOld) //< Deprecated, preserved for backward compatibility, replaced by propertyGroups - ->Field("properties", &PropertyLayout::m_propertiesOld) //< Deprecated, preserved for backward compatibility, replaced by propertyGroups + ->Field("version", &PropertyLayout::m_versionOld) //< @deprecated: preserved for backward compatibility, replaced by MaterialTypeSourceData::version + ->Field("groups", &PropertyLayout::m_groupsOld) //< @deprecated: preserved for backward compatibility, replaced by propertyGroups + ->Field("properties", &PropertyLayout::m_propertiesOld) //< @deprecated: preserved for backward compatibility, replaced by propertyGroups ->Field("propertyGroups", &PropertyLayout::m_propertyGroups) ; 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 b03771173f..3855a0a222 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialUtils.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialUtils.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -82,7 +83,7 @@ namespace AZ loadOutcome = AZ::JsonSerializationUtils::ReadJsonFile(filePath, AZ::RPI::JsonUtils::DefaultMaxFileSize); if (!loadOutcome.IsSuccess()) { - AZ_Error("AZ::RPI::JsonUtils", false, "%s", loadOutcome.GetError().c_str()); + AZ_Error("MaterialUtils", false, "%s", loadOutcome.GetError().c_str()); return AZ::Failure(); } @@ -114,6 +115,46 @@ namespace AZ return AZ::Success(AZStd::move(materialType)); } } + + AZ::Outcome LoadMaterialSourceData(const AZStd::string& filePath, const rapidjson::Value* document, bool warningsAsErrors) + { + AZ::Outcome loadOutcome; + if (document == nullptr) + { + loadOutcome = AZ::JsonSerializationUtils::ReadJsonFile(filePath, AZ::RPI::JsonUtils::DefaultMaxFileSize); + if (!loadOutcome.IsSuccess()) + { + AZ_Error("MaterialUtils", false, "%s", loadOutcome.GetError().c_str()); + return AZ::Failure(); + } + + document = &loadOutcome.GetValue(); + } + + MaterialSourceData material; + + JsonDeserializerSettings settings; + + JsonReportingHelper reportingHelper; + reportingHelper.Attach(settings); + + JsonSerialization::Load(material, *document, settings); + material.ConvertToNewDataFormat(); + + if (reportingHelper.ErrorsReported()) + { + return AZ::Failure(); + } + else if (warningsAsErrors && reportingHelper.WarningsReported()) + { + AZ_Error("MaterialUtils", false, "Warnings reported while loading '%s'", filePath.c_str()); + return AZ::Failure(); + } + else + { + return AZ::Success(AZStd::move(material)); + } + } void CheckForUnrecognizedJsonFields(const AZStd::string_view* acceptedFieldNames, uint32_t acceptedFieldNameCount, const rapidjson::Value& object, JsonDeserializerContext& context, JsonSerializationResult::ResultCode &result) { diff --git a/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp index 2ac363058c..362d7d8b6d 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp @@ -86,7 +86,7 @@ namespace UnitTest RPITestFixture::TearDown(); } - + Data::Asset CreateTestMaterialTypeAsset(Data::AssetId assetId) { const char* materialTypeJson = R"( @@ -146,21 +146,21 @@ namespace UnitTest } )"; - MaterialTypeSourceData materialTypeSourceData; LoadTestDataFromJson(materialTypeSourceData, materialTypeJson); return materialTypeSourceData.CreateMaterialTypeAsset(assetId).TakeValue(); } }; - void AddPropertyGroup(MaterialSourceData& material, AZStd::string_view groupName) + void AddPropertyGroup(MaterialSourceData&, AZStd::string_view) { - material.m_properties.insert(groupName); + // Old function, left blank intentionally } - void AddProperty(MaterialSourceData& material, AZStd::string_view groupName, AZStd::string_view propertyName, const MaterialPropertyValue& anyValue) + void AddProperty(MaterialSourceData& material, AZStd::string_view groupName, AZStd::string_view propertyName, const MaterialPropertyValue& value) { - material.m_properties[groupName][propertyName] = anyValue; + MaterialPropertyId id{groupName, propertyName}; + material.SetPropertyValue(id, value); } TEST_F(MaterialSourceDataTests, CreateMaterialAsset_BasicProperties) @@ -359,80 +359,61 @@ namespace UnitTest void CheckEqual(MaterialSourceData& a, MaterialSourceData& b) { - EXPECT_STREQ(a.m_materialType.data(), b.m_materialType.data()); - EXPECT_STREQ(a.m_description.data(), b.m_description.data()); - EXPECT_STREQ(a.m_parentMaterial.data(), b.m_parentMaterial.data()); + EXPECT_STREQ(a.m_materialType.c_str(), b.m_materialType.c_str()); + EXPECT_STREQ(a.m_description.c_str(), b.m_description.c_str()); + EXPECT_STREQ(a.m_parentMaterial.c_str(), b.m_parentMaterial.c_str()); EXPECT_EQ(a.m_materialTypeVersion, b.m_materialTypeVersion); - EXPECT_EQ(a.m_properties.size(), b.m_properties.size()); - for (auto& groupA : a.m_properties) - { - AZStd::string groupName = groupA.first; + EXPECT_EQ(a.GetPropertyValues().size(), b.GetPropertyValues().size()); - auto groupIterB = b.m_properties.find(groupName); - if (groupIterB == b.m_properties.end()) + for (auto& [propertyId, propertyValue] : a.GetPropertyValues()) + { + if (!b.HasPropertyValue(propertyId)) { - EXPECT_TRUE(false) << "groupB[" << groupName.c_str() << "] not found"; + EXPECT_TRUE(false) << "Property '" << propertyId.GetCStr() << "' not found in material B"; continue; } - auto& groupB = *groupIterB; - - EXPECT_EQ(groupA.second.size(), groupB.second.size()) << " for group[" << groupName.c_str() << "]"; - - for (auto& propertyIterA : groupA.second) - { - AZStd::string propertyName = propertyIterA.first; + auto& propertyA = propertyValue; + auto& propertyB = b.GetPropertyValue(propertyId); - auto propertyIterB = groupB.second.find(propertyName); - if (propertyIterB == groupB.second.end()) - { - EXPECT_TRUE(false) << "groupB[" << groupName.c_str() << "][" << propertyName.c_str() << "] not found"; - continue; - } - - auto& propertyA = propertyIterA.second; - auto& propertyB = propertyIterB->second; - - AZStd::string propertyReference = AZStd::string::format(" for property '%s.%s'", groupName.c_str(), propertyName.c_str()); + AZStd::string propertyReference = AZStd::string::format(" for property '%s'", propertyId.GetCStr()); - // We allow some types like Vector4 and Color or Int and UInt to be interchangeable since they serialize the same and can be converted when the MaterialAsset is finalized. + // We allow some types like Vector4 and Color or Int and UInt to be interchangeable since they serialize the same and can be converted when the MaterialAsset is finalized. - if (AreTypesCompatible(propertyA, propertyB)) - { - EXPECT_EQ(propertyA.GetValue(), propertyB.GetValue()) << propertyReference.c_str(); - } - else if (AreTypesCompatible(propertyA, propertyB)) - { - EXPECT_EQ(GetAsInt(propertyA), GetAsInt(propertyB)) << propertyReference.c_str(); - } - else if (AreTypesCompatible(propertyA, propertyB)) - { - EXPECT_NEAR(propertyA.GetValue(), propertyB.GetValue(), 0.01) << propertyReference.c_str(); - } - else if (AreTypesCompatible(propertyA, propertyB)) - { - EXPECT_TRUE(propertyA.GetValue().IsClose(propertyB.GetValue())) << propertyReference.c_str(); - } - else if (AreTypesCompatible(propertyA, propertyB)) - { - EXPECT_TRUE(propertyA.GetValue().IsClose(propertyB.GetValue())) << propertyReference.c_str(); - } - else if (AreTypesCompatible(propertyA, propertyB)) - { - EXPECT_TRUE(GetAsVector4(propertyA).IsClose(GetAsVector4(propertyB))) << propertyReference.c_str(); - } - else if (AreTypesCompatible(propertyA, propertyB)) - { - EXPECT_STREQ(propertyA.GetValue().c_str(), propertyB.GetValue().c_str()) << propertyReference.c_str(); - } - else - { - ADD_FAILURE(); - } + if (AreTypesCompatible(propertyA, propertyB)) + { + EXPECT_EQ(propertyA.GetValue(), propertyB.GetValue()) << propertyReference.c_str(); + } + else if (AreTypesCompatible(propertyA, propertyB)) + { + EXPECT_EQ(GetAsInt(propertyA), GetAsInt(propertyB)) << propertyReference.c_str(); + } + else if (AreTypesCompatible(propertyA, propertyB)) + { + EXPECT_NEAR(propertyA.GetValue(), propertyB.GetValue(), 0.01) << propertyReference.c_str(); + } + else if (AreTypesCompatible(propertyA, propertyB)) + { + EXPECT_TRUE(propertyA.GetValue().IsClose(propertyB.GetValue())) << propertyReference.c_str(); + } + else if (AreTypesCompatible(propertyA, propertyB)) + { + EXPECT_TRUE(propertyA.GetValue().IsClose(propertyB.GetValue())) << propertyReference.c_str(); + } + else if (AreTypesCompatible(propertyA, propertyB)) + { + EXPECT_TRUE(GetAsVector4(propertyA).IsClose(GetAsVector4(propertyB))) << propertyReference.c_str(); + } + else if (AreTypesCompatible(propertyA, propertyB)) + { + EXPECT_STREQ(propertyA.GetValue().c_str(), propertyB.GetValue().c_str()) << propertyReference.c_str(); + } + else + { + ADD_FAILURE(); } } - } TEST_F(MaterialSourceDataTests, TestJsonRoundTrip) @@ -465,6 +446,88 @@ namespace UnitTest CheckEqual(sourceDataOriginal, sourceDataCopy); } + + TEST_F(MaterialSourceDataTests, TestLoadLegacyFormat) + { + const AZStd::string inputJson = R"( + { + "materialType": "test.materialtype", // Doesn't matter, this isn't loaded + "properties": { + "groupA": { + "myBool": true, + "myInt": 5, + "myFloat": 0.5 + }, + "groupB": { + "myFloat2": [0.1, 0.2], + "myFloat3": [0.3, 0.4, 0.5], + "myFloat4": [0.6, 0.7, 0.8, 0.9], + "myString": "Hello" + } + } + } + )"; + + MaterialSourceData material; + LoadTestDataFromJson(material, inputJson); + material.ConvertToNewDataFormat(); + + MaterialSourceData expectedMaterial; + expectedMaterial.m_materialType = "test.materialtype"; + expectedMaterial.SetPropertyValue(Name{"groupA.myBool"}, true); + expectedMaterial.SetPropertyValue(Name{"groupA.myInt"}, 5); + expectedMaterial.SetPropertyValue(Name{"groupA.myFloat"}, 0.5f); + expectedMaterial.SetPropertyValue(Name{"groupB.myFloat2"}, Vector2(0.1f, 0.2f)); + expectedMaterial.SetPropertyValue(Name{"groupB.myFloat3"}, Vector3(0.3f, 0.4f, 0.5f)); + expectedMaterial.SetPropertyValue(Name{"groupB.myFloat4"}, Vector4(0.6f, 0.7f, 0.8f, 0.9f)); + expectedMaterial.SetPropertyValue(Name{"groupB.myString"}, AZStd::string{"Hello"}); + + CheckEqual(expectedMaterial, material); + } + + TEST_F(MaterialSourceDataTests, TestPropertyValues) + { + MaterialSourceData material; + + Name foo{"foo"}; + Name bar{"bar"}; + Name baz{"baz"}; + + EXPECT_EQ(0, material.GetPropertyValues().size()); + EXPECT_FALSE(material.HasPropertyValue(foo)); + EXPECT_FALSE(material.HasPropertyValue(bar)); + EXPECT_FALSE(material.HasPropertyValue(baz)); + EXPECT_FALSE(material.GetPropertyValue(foo).IsValid()); + EXPECT_FALSE(material.GetPropertyValue(bar).IsValid()); + EXPECT_FALSE(material.GetPropertyValue(baz).IsValid()); + + material.SetPropertyValue(Name{"foo"}, 2); + material.SetPropertyValue(Name{"bar"}, true); + material.SetPropertyValue(Name{"baz"}, 0.5f); + + EXPECT_EQ(3, material.GetPropertyValues().size()); + EXPECT_TRUE(material.HasPropertyValue(foo)); + EXPECT_TRUE(material.HasPropertyValue(bar)); + EXPECT_TRUE(material.HasPropertyValue(baz)); + EXPECT_TRUE(material.GetPropertyValue(foo).IsValid()); + EXPECT_TRUE(material.GetPropertyValue(bar).IsValid()); + EXPECT_TRUE(material.GetPropertyValue(baz).IsValid()); + EXPECT_EQ(material.GetPropertyValue(foo).GetValue(), 2); + EXPECT_EQ(material.GetPropertyValue(bar).GetValue(), true); + EXPECT_EQ(material.GetPropertyValue(baz).GetValue(), 0.5f); + + material.RemovePropertyValue(bar); + + EXPECT_EQ(2, material.GetPropertyValues().size()); + EXPECT_TRUE(material.HasPropertyValue(foo)); + EXPECT_FALSE(material.HasPropertyValue(bar)); + EXPECT_TRUE(material.HasPropertyValue(baz)); + EXPECT_TRUE(material.GetPropertyValue(foo).IsValid()); + EXPECT_FALSE(material.GetPropertyValue(bar).IsValid()); + EXPECT_TRUE(material.GetPropertyValue(baz).IsValid()); + EXPECT_EQ(material.GetPropertyValue(foo).GetValue(), 2); + EXPECT_EQ(material.GetPropertyValue(baz).GetValue(), 0.5f); + } TEST_F(MaterialSourceDataTests, Load_MaterialTypeAfterPropertyList) { @@ -487,21 +550,14 @@ namespace UnitTest } )"; - const char* materialTypeFilePath = "@exefolder@/Temp/simpleMaterialType.materialtype"; - - AZ::IO::FileIOStream file; - EXPECT_TRUE(file.Open(materialTypeFilePath, AZ::IO::OpenMode::ModeWrite | AZ::IO::OpenMode::ModeCreatePath)); - file.Write(simpleMaterialTypeJson.size(), simpleMaterialTypeJson.data()); - file.Close(); + AZ::Utils::WriteFile(simpleMaterialTypeJson, "@exefolder@/Temp/simpleMaterialType.materialtype"); // It shouldn't matter whether the materialType field appears before the property value list. This allows for the possibility // that customer scripts generate material data and happen to use an unexpected order. const AZStd::string inputJson = R"( { - "properties": { - "general": { - "testValue": 1.2 - } + "propertyValues": { + "general.testValue": 1.2 }, "materialType": "@exefolder@/Temp/simpleMaterialType.materialtype" } @@ -513,7 +569,7 @@ namespace UnitTest EXPECT_EQ(AZ::JsonSerializationResult::Tasks::ReadField, loadResult.m_jsonResultCode.GetTask()); EXPECT_EQ(AZ::JsonSerializationResult::Processing::Completed, loadResult.m_jsonResultCode.GetProcessing()); - float testValue = material.m_properties["general"]["testValue"].GetValue(); + float testValue = material.GetPropertyValue(Name{"general.testValue"}).GetValue(); EXPECT_FLOAT_EQ(1.2f, testValue); } @@ -522,10 +578,8 @@ namespace UnitTest const AZStd::string inputJson = R"( { "materialTypeVersion": 1, - "properties": { - "baseColor": { - "color": [1.0,1.0,1.0] - } + "propertyValues": { + "baseColor.color": [1.0,1.0,1.0] } } )"; @@ -561,10 +615,8 @@ namespace UnitTest { "materialType": "DoesNotExist.materialtype", "materialTypeVersion": 1, - "properties": { - "baseColor": { - "color": [1.0,1.0,1.0] - } + "propertyValues": { + "baseColor.color": [1.0,1.0,1.0] } } )"; @@ -900,10 +952,8 @@ namespace UnitTest const AZStd::string inputJson = AZStd::string::format(R"( { "materialType": "@exefolder@/Temp/test.materialtype", - "properties": { - "%s": { - "%s": %s - } + "propertyValues": { + "%s.%s": %s } } )", groupName, propertyName, jsonValue); @@ -965,7 +1015,239 @@ namespace UnitTest CheckEndToEndDataTypeResolution("MyFloat4", "{\"y\":0.2, \"x\":0.1, \"Z\":0.3}", Vector4{0.1f, 0.2f, 0.3f, 0.0f}); CheckEndToEndDataTypeResolution("MyFloat4", "{\"y\":0.2, \"W\":0.4, \"x\":0.1, \"Z\":0.3}", Vector4{0.1f, 0.2f, 0.3f, 0.4f}); } + + TEST_F(MaterialSourceDataTests, CreateMaterialAssetFromSourceData_MultiLevelDataInheritance) + { + // Note the data being tested here is based on CreateMaterialAsset_MultiLevelDataInheritance() + + const AZStd::string simpleMaterialTypeJson = R"( + { + "propertyLayout": { + "propertyGroups": + [ + { + "name": "general", + "properties": [ + { + "name": "MyFloat", + "type": "Float" + }, + { + "name": "MyFloat2", + "type": "Vector2" + }, + { + "name": "MyColor", + "type": "Color" + } + ] + } + ] + } + } + )"; + + AZ::Utils::WriteFile(simpleMaterialTypeJson, "@exefolder@/Temp/test.materialtype"); + + const AZStd::string material1Json = R"( + { + "materialType": "@exefolder@/Temp/test.materialtype", + "propertyValues": { + "general.MyFloat": 1.5, + "general.MyColor": [0.1, 0.2, 0.3, 0.4] + } + } + )"; + + AZ::Utils::WriteFile(material1Json, "@exefolder@/Temp/m1.material"); + + const AZStd::string material2Json = R"( + { + "materialType": "@exefolder@/Temp/test.materialtype", + "parentMaterial": "@exefolder@/Temp/m1.material", + "propertyValues": { + "general.MyFloat2": [4.1, 4.2], + "general.MyColor": [0.15, 0.25, 0.35, 0.45] + } + } + )"; + + AZ::Utils::WriteFile(material2Json, "@exefolder@/Temp/m2.material"); + + const AZStd::string material3Json = R"( + { + "materialType": "@exefolder@/Temp/test.materialtype", + "parentMaterial": "@exefolder@/Temp/m2.material", + "propertyValues": { + "general.MyFloat": 3.5 + } + } + )"; + + AZ::Utils::WriteFile(material3Json, "@exefolder@/Temp/m3.material"); + + MaterialSourceData sourceDataLevel1 = MaterialUtils::LoadMaterialSourceData("@exefolder@/Temp/m1.material").TakeValue(); + MaterialSourceData sourceDataLevel2 = MaterialUtils::LoadMaterialSourceData("@exefolder@/Temp/m2.material").TakeValue(); + MaterialSourceData sourceDataLevel3 = MaterialUtils::LoadMaterialSourceData("@exefolder@/Temp/m3.material").TakeValue(); + + auto materialAssetLevel1 = sourceDataLevel1.CreateMaterialAssetFromSourceData(Uuid::CreateRandom()); + EXPECT_TRUE(materialAssetLevel1.IsSuccess()); + EXPECT_TRUE(materialAssetLevel1.GetValue()->WasPreFinalized()); + + auto materialAssetLevel2 = sourceDataLevel2.CreateMaterialAssetFromSourceData(Uuid::CreateRandom()); + EXPECT_TRUE(materialAssetLevel2.IsSuccess()); + EXPECT_TRUE(materialAssetLevel2.GetValue()->WasPreFinalized()); + + auto materialAssetLevel3 = sourceDataLevel3.CreateMaterialAssetFromSourceData(Uuid::CreateRandom()); + EXPECT_TRUE(materialAssetLevel3.IsSuccess()); + EXPECT_TRUE(materialAssetLevel3.GetValue()->WasPreFinalized()); + + auto layout = materialAssetLevel1.GetValue()->GetMaterialPropertiesLayout(); + MaterialPropertyIndex myFloat = layout->FindPropertyIndex(Name("general.MyFloat")); + MaterialPropertyIndex myFloat2 = layout->FindPropertyIndex(Name("general.MyFloat2")); + MaterialPropertyIndex myColor = layout->FindPropertyIndex(Name("general.MyColor")); + + AZStd::span properties; + + // Check level 1 properties + properties = materialAssetLevel1.GetValue()->GetPropertyValues(); + EXPECT_EQ(properties[myFloat.GetIndex()].GetValue(), 1.5f); + EXPECT_EQ(properties[myFloat2.GetIndex()].GetValue(), Vector2(0.0f, 0.0f)); + EXPECT_EQ(properties[myColor.GetIndex()].GetValue(), Color(0.1f, 0.2f, 0.3f, 0.4f)); + + // Check level 2 properties + properties = materialAssetLevel2.GetValue()->GetPropertyValues(); + EXPECT_EQ(properties[myFloat.GetIndex()].GetValue(), 1.5f); + EXPECT_EQ(properties[myFloat2.GetIndex()].GetValue(), Vector2(4.1f, 4.2f)); + EXPECT_EQ(properties[myColor.GetIndex()].GetValue(), Color(0.15f, 0.25f, 0.35f, 0.45f)); + + // Check level 3 properties + properties = materialAssetLevel3.GetValue()->GetPropertyValues(); + EXPECT_EQ(properties[myFloat.GetIndex()].GetValue(), 3.5f); + EXPECT_EQ(properties[myFloat2.GetIndex()].GetValue(), Vector2(4.1f, 4.2f)); + EXPECT_EQ(properties[myColor.GetIndex()].GetValue(), Color(0.15f, 0.25f, 0.35f, 0.45f)); + } + TEST_F(MaterialSourceDataTests, CreateMaterialAssetFromSourceData_MultiLevelDataInheritance_OldFormat) + { + // This test is the same as CreateMaterialAssetFromSourceData_MultiLevelDataInheritance except it uses the old format + // where material property values in the .material file were nested, with properties listed under a group object, + // rather than using a flat list of property values. + // Basically, we are making sure that MaterialSourceData::ConvertToNewDataFormat() is getting called. + + const AZStd::string simpleMaterialTypeJson = R"( + { + "propertyLayout": { + "propertyGroups": + [ + { + "name": "general", + "properties": [ + { + "name": "MyFloat", + "type": "Float" + }, + { + "name": "MyFloat2", + "type": "Vector2" + }, + { + "name": "MyColor", + "type": "Color" + } + ] + } + ] + } + } + )"; + + AZ::Utils::WriteFile(simpleMaterialTypeJson, "@exefolder@/Temp/test.materialtype"); + + const AZStd::string material1Json = R"( + { + "materialType": "@exefolder@/Temp/test.materialtype", + "properties": { + "general": { + "MyFloat": 1.5, + "MyColor": [0.1, 0.2, 0.3, 0.4] + } + } + } + )"; + + AZ::Utils::WriteFile(material1Json, "@exefolder@/Temp/m1.material"); + + const AZStd::string material2Json = R"( + { + "materialType": "@exefolder@/Temp/test.materialtype", + "parentMaterial": "@exefolder@/Temp/m1.material", + "properties": { + "general": { + "MyFloat2": [4.1, 4.2], + "MyColor": [0.15, 0.25, 0.35, 0.45] + } + } + } + )"; + + AZ::Utils::WriteFile(material2Json, "@exefolder@/Temp/m2.material"); + + const AZStd::string material3Json = R"( + { + "materialType": "@exefolder@/Temp/test.materialtype", + "parentMaterial": "@exefolder@/Temp/m2.material", + "properties": { + "general": { + "MyFloat": 3.5 + } + } + } + )"; + + AZ::Utils::WriteFile(material3Json, "@exefolder@/Temp/m3.material"); + + MaterialSourceData sourceDataLevel1 = MaterialUtils::LoadMaterialSourceData("@exefolder@/Temp/m1.material").TakeValue(); + MaterialSourceData sourceDataLevel2 = MaterialUtils::LoadMaterialSourceData("@exefolder@/Temp/m2.material").TakeValue(); + MaterialSourceData sourceDataLevel3 = MaterialUtils::LoadMaterialSourceData("@exefolder@/Temp/m3.material").TakeValue(); + + auto materialAssetLevel1 = sourceDataLevel1.CreateMaterialAssetFromSourceData(Uuid::CreateRandom()); + EXPECT_TRUE(materialAssetLevel1.IsSuccess()); + EXPECT_TRUE(materialAssetLevel1.GetValue()->WasPreFinalized()); + + auto materialAssetLevel2 = sourceDataLevel2.CreateMaterialAssetFromSourceData(Uuid::CreateRandom()); + EXPECT_TRUE(materialAssetLevel2.IsSuccess()); + EXPECT_TRUE(materialAssetLevel2.GetValue()->WasPreFinalized()); + + auto materialAssetLevel3 = sourceDataLevel3.CreateMaterialAssetFromSourceData(Uuid::CreateRandom()); + EXPECT_TRUE(materialAssetLevel3.IsSuccess()); + EXPECT_TRUE(materialAssetLevel3.GetValue()->WasPreFinalized()); + + auto layout = materialAssetLevel1.GetValue()->GetMaterialPropertiesLayout(); + MaterialPropertyIndex myFloat = layout->FindPropertyIndex(Name("general.MyFloat")); + MaterialPropertyIndex myFloat2 = layout->FindPropertyIndex(Name("general.MyFloat2")); + MaterialPropertyIndex myColor = layout->FindPropertyIndex(Name("general.MyColor")); + + AZStd::span properties; + + // Check level 1 properties + properties = materialAssetLevel1.GetValue()->GetPropertyValues(); + EXPECT_EQ(properties[myFloat.GetIndex()].GetValue(), 1.5f); + EXPECT_EQ(properties[myFloat2.GetIndex()].GetValue(), Vector2(0.0f, 0.0f)); + EXPECT_EQ(properties[myColor.GetIndex()].GetValue(), Color(0.1f, 0.2f, 0.3f, 0.4f)); + + // Check level 2 properties + properties = materialAssetLevel2.GetValue()->GetPropertyValues(); + EXPECT_EQ(properties[myFloat.GetIndex()].GetValue(), 1.5f); + EXPECT_EQ(properties[myFloat2.GetIndex()].GetValue(), Vector2(4.1f, 4.2f)); + EXPECT_EQ(properties[myColor.GetIndex()].GetValue(), Color(0.15f, 0.25f, 0.35f, 0.45f)); + + // Check level 3 properties + properties = materialAssetLevel3.GetValue()->GetPropertyValues(); + EXPECT_EQ(properties[myFloat.GetIndex()].GetValue(), 3.5f); + EXPECT_EQ(properties[myFloat2.GetIndex()].GetValue(), Vector2(4.1f, 4.2f)); + EXPECT_EQ(properties[myColor.GetIndex()].GetValue(), Color(0.15f, 0.25f, 0.35f, 0.45f)); + } } diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp index 28de56c75e..7490fa5848 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp @@ -362,8 +362,8 @@ namespace MaterialEditor } // TODO: Support populating the Material Editor with nested property groups, not just the top level. - const AZStd::string groupName = propertyId.GetStringView().substr(0, propertyId.GetStringView().size() - propertyDefinition->GetName().size() - 1); - sourceData.m_properties[groupName][propertyDefinition->GetName()] = propertyValue; + AZStd::string_view groupName = propertyId.GetStringView().substr(0, propertyId.GetStringView().size() - propertyDefinition->GetName().size() - 1); + sourceData.SetPropertyValue(AZ::RPI::MaterialPropertyId{groupName, propertyDefinition->GetName()}, propertyValue); } } return true; @@ -395,12 +395,15 @@ namespace MaterialEditor if (AzFramework::StringFunc::Path::IsExtension(m_absolutePath.c_str(), AZ::RPI::MaterialSourceData::Extension)) { // Load the material source data so that we can check properties and create a material asset from it - if (!AZ::RPI::JsonUtils::LoadObjectFromFile(m_absolutePath, m_materialSourceData)) + auto loadResult = AZ::RPI::MaterialUtils::LoadMaterialSourceData(m_absolutePath); + if (!loadResult) { AZ_Error("MaterialDocument", false, "Material source data could not be loaded: '%s'.", m_absolutePath.c_str()); return OpenFailed(); } + m_materialSourceData = loadResult.TakeValue(); + // We always need the absolute path for the material type and parent material to load source data and resolving // relative paths when saving. This will convert and store them as absolute paths for use within the document. if (!m_materialSourceData.m_parentMaterial.empty()) @@ -482,12 +485,15 @@ namespace MaterialEditor if (!m_materialSourceData.m_parentMaterial.empty()) { AZ::RPI::MaterialSourceData parentMaterialSourceData; - if (!AZ::RPI::JsonUtils::LoadObjectFromFile(m_materialSourceData.m_parentMaterial, parentMaterialSourceData)) + auto loadResult = AZ::RPI::MaterialUtils::LoadMaterialSourceData(m_materialSourceData.m_parentMaterial); + if (!loadResult) { AZ_Error("MaterialDocument", false, "Material parent source data could not be loaded for: '%s'.", m_materialSourceData.m_parentMaterial.c_str()); return OpenFailed(); } + parentMaterialSourceData = loadResult.TakeValue(); + const auto parentMaterialAssetIdResult = AZ::RPI::AssetUtils::MakeAssetId(m_materialSourceData.m_parentMaterial, 0); if (!parentMaterialAssetIdResult) { diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentUtil.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentUtil.cpp index 1e5e25fa30..52e22645ca 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentUtil.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/EditorMaterialComponentUtil.cpp @@ -149,8 +149,8 @@ namespace AZ } // TODO: Support populating the Material Editor with nested property groups, not just the top level. - const AZStd::string groupName = propertyId.GetStringView().substr(0, propertyId.GetStringView().size() - propertyDefinition->GetName().size() - 1); - exportData.m_properties[groupName][propertyDefinition->GetName()] = propertyValue; + AZStd::string_view groupName = propertyId.GetStringView().substr(0, propertyId.GetStringView().size() - propertyDefinition->GetName().size() - 1); + exportData.SetPropertyValue(RPI::MaterialPropertyId{groupName, propertyDefinition->GetName()}, propertyValue); return true; });