From a896ff11bc3aa8f13696e078e66fee1dbcf269ae Mon Sep 17 00:00:00 2001 From: santorac <55155825+santorac@users.noreply.github.com> Date: Fri, 14 Jan 2022 12:57:52 -0800 Subject: [PATCH] Changed .material serialization to avoid loading the .materialtype file, since the .material builder doesn't declare a source dependency on the .materialtype. Otherwise there can be ambiguous edge cases where changes to the .materialtype might or might not impact the baked MaterialAsset. Note that another option would have been to add a the appropriate source dependency, but that would hurt iteration time as any change to the .materialtype file would cause every .material file and .fbx to rebuild. These changes have the added benefit of simplifying some of the serialization code. MaterialSourceDataSerializer is no longer needed, as its main purpose was to pass the MaterialTypeSourceData down to the MaterialPropertyValueSerializer. Before, the JSON serialization system gave a lot of data flexibility because it did best-effort conversions, like allowing a float to be loaded as an int for example. But now the material serialization code doesn't know target data type, so it has to assume the data type based on what's in the .material file, and then the MaterialAsset will convert the data to the appropriate type later when Finalize() is called. Signed-off-by: santorac <55155825+santorac@users.noreply.github.com> --- .../MaterialPropertyValueSerializer.h | 7 - .../MaterialPropertyValueSourceData.h | 2 +- .../Material/MaterialSourceDataSerializer.h | 40 -- .../Material/MaterialTypeSourceData.h | 6 +- .../MaterialPropertyValueSerializer.cpp | 102 +++-- .../RPI.Edit/Material/MaterialSourceData.cpp | 21 +- .../Material/MaterialSourceDataSerializer.cpp | 162 ------- .../Material/MaterialTypeSourceData.cpp | 11 +- .../RPI.Reflect/Material/MaterialAsset.cpp | 132 +++++- .../Tests/Material/MaterialAssetTests.cpp | 32 +- .../Material/MaterialSourceDataTests.cpp | 406 ++++++++++-------- Gems/Atom/RPI/Code/atom_rpi_edit_files.cmake | 2 - 12 files changed, 446 insertions(+), 477 deletions(-) delete mode 100644 Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialSourceDataSerializer.h delete mode 100644 Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceDataSerializer.cpp diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialPropertyValueSerializer.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialPropertyValueSerializer.h index 29371618cf..befbb7c990 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialPropertyValueSerializer.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialPropertyValueSerializer.h @@ -24,13 +24,6 @@ namespace AZ AZ_RTTI(AZ::RPI::JsonMaterialPropertyValueSerializer, "{A52B1ED8-C849-4269-9AA7-9D0814D2EC59}", BaseJsonSerializer); AZ_CLASS_ALLOCATOR_DECL; - //! A LoadContext object must be passed down to the serializer via JsonDeserializerContext::GetMetadata().Add(...) - struct LoadContext - { - AZ_TYPE_INFO(JsonMaterialPropertyValueSerializer::LoadContext, "{5E0A891A-27F6-4AD7-88A5-B9EA50F88B45}"); - uint32_t m_materialTypeVersion; //!< The version number from the .materialtype file - }; - JsonSerializationResult::Result Load(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, JsonDeserializerContext& context) override; diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialPropertyValueSourceData.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialPropertyValueSourceData.h index 178cacba15..a0640a1522 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialPropertyValueSourceData.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialPropertyValueSourceData.h @@ -54,7 +54,7 @@ namespace AZ //! The resolved value with a valid type of a property. It needs to be mutable to allow post-resolving when parent objects are declared as const. mutable MaterialPropertyValue m_resolvedValue; //! Candidate values from serialization. - AZStd::map m_possibleValues; + AZStd::map m_possibleValues; }; } // namespace RPI } // namespace AZ diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialSourceDataSerializer.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialSourceDataSerializer.h deleted file mode 100644 index 301b80ed82..0000000000 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialSourceDataSerializer.h +++ /dev/null @@ -1,40 +0,0 @@ -/* - * Copyright (c) Contributors to the Open 3D Engine Project. - * For complete copyright and license terms please see the LICENSE at the root of this distribution. - * - * SPDX-License-Identifier: Apache-2.0 OR MIT - * - */ - -#pragma once - -#include -#include - -namespace AZ -{ - class ReflectContext; - - namespace RPI - { - //! This custom serializer is needed to load the material type file and saves its data in the - //! JsonDeserializerSettings for JsonMaterialPropertyValueSerializer to use. - //! (Note we could have made a custom serializer specifically for the 'materialType' field but that - //! would require 'materialType' to appear before 'properties'. By having a custom serializer for the common - //! parent of 'materialType' and 'properties', we can avoid an order dependency within the JSON file). - class JsonMaterialSourceDataSerializer - : public BaseJsonSerializer - { - public: - AZ_RTTI(AZ::RPI::JsonMaterialSourceDataSerializer, "{008A7423-8DF6-4BA3-BF5E-B0C189CCBE58}", BaseJsonSerializer); - AZ_CLASS_ALLOCATOR_DECL; - - JsonSerializationResult::Result Load(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, - JsonDeserializerContext& context) override; - - JsonSerializationResult::Result Store(rapidjson::Value& outputValue, const void* inputValue, - const void* defaultValue, const Uuid& valueTypeId, JsonSerializerContext& context) override; - }; - - } // 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 f5336807c7..9333880594 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 @@ -186,9 +186,8 @@ namespace AZ //! Searches for a specific property. //! Note this function can find properties using old versions of the property name; in that case, //! the name in the returned PropertyDefinition* will not match the @propertyName that was searched for. - //! @param materialTypeVersion indicates the version number of the property name being passed in. Only renames above this version number will be applied. //! @return the requested property, or null if it could not be found - const PropertyDefinition* FindProperty(AZStd::string_view groupName, AZStd::string_view propertyName, uint32_t materialTypeVersion = 0) const; + const PropertyDefinition* FindProperty(AZStd::string_view groupName, AZStd::string_view propertyName) const; //! Construct a complete list of group definitions, including implicit groups, arranged in the same order as the source data //! Groups with the same name will be consolidated into a single entry @@ -212,9 +211,8 @@ namespace AZ Outcome> CreateMaterialTypeAsset(Data::AssetId assetId, AZStd::string_view materialTypeSourceFilePath = "", bool elevateWarnings = true) const; //! Possibly renames @propertyId based on the material version update steps. - //! @param materialTypeVersion indicates the version number of the property name being passed in. Only renames above this version number will be applied. //! @return true if the property was renamed - bool ApplyPropertyRenames(MaterialPropertyId& propertyId, uint32_t materialTypeVersion = 0) const; + bool ApplyPropertyRenames(MaterialPropertyId& propertyId) const; }; //! The wrapper class for derived material functors. diff --git a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialPropertyValueSerializer.cpp b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialPropertyValueSerializer.cpp index 5e04365ffb..10b45ca8df 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialPropertyValueSerializer.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialPropertyValueSerializer.cpp @@ -8,7 +8,6 @@ #include #include -#include #include #include @@ -55,15 +54,6 @@ namespace AZ MaterialSourceData::Property* property = reinterpret_cast(outputValue); AZ_Assert(property, "Output value for JsonMaterialPropertyValueSerializer can't be null."); - const MaterialTypeSourceData* materialType = context.GetMetadata().Find(); - if (!materialType) - { - AZ_Assert(false, "Material type reference not found"); - return context.Report(JsonSerializationResult::Tasks::ReadField, JsonSerializationResult::Outcomes::Catastrophic, "Material type reference not found."); - } - - const JsonMaterialPropertyValueSerializer::LoadContext* loadContext = context.GetMetadata().Find(); - // Construct the full property name (groupName.propertyName) by parsing it from the JSON path string. size_t startPropertyName = context.GetPath().Get().rfind('/'); size_t startGroupName = context.GetPath().Get().rfind('/', startPropertyName-1); @@ -72,48 +62,70 @@ namespace AZ JSR::ResultCode result(JSR::Tasks::ReadField); - auto propertyDefinition = materialType->FindProperty(groupName, propertyName, loadContext->m_materialTypeVersion); - if (!propertyDefinition) + if (inputValue.IsBool()) { - AZStd::string message = AZStd::string::format("Property '%.*s.%.*s' not found in material type.", AZ_STRING_ARG(groupName), AZ_STRING_ARG(propertyName)); - return context.Report(JsonSerializationResult::Tasks::ReadField, JsonSerializationResult::Outcomes::Unsupported, message); + result.Combine(LoadVariant(property->m_value, false, inputValue, context)); } - else + else if (inputValue.IsInt() || inputValue.IsInt64()) + { + result.Combine(LoadVariant(property->m_value, 0, inputValue, context)); + } + else if (inputValue.IsUint() || inputValue.IsUint64()) + { + result.Combine(LoadVariant(property->m_value, 0u, inputValue, context)); + } + else if (inputValue.IsFloat() || inputValue.IsDouble()) + { + result.Combine(LoadVariant(property->m_value, 0.0f, inputValue, context)); + } + else if (inputValue.IsArray() && inputValue.Size() == 4) + { + result.Combine(LoadVariant(property->m_value, Vector4{0.0f, 0.0f, 0.0f, 0.0f}, inputValue, context)); + } + else if (inputValue.IsArray() && inputValue.Size() == 3) + { + result.Combine(LoadVariant(property->m_value, Vector3{0.0f, 0.0f, 0.0f}, inputValue, context)); + } + else if (inputValue.IsArray() && inputValue.Size() == 2) { - switch (propertyDefinition->m_dataType) + result.Combine(LoadVariant(property->m_value, Vector2{0.0f, 0.0f}, inputValue, context)); + } + else if (inputValue.IsObject()) + { + JsonSerializationResult::ResultCode resultCode = LoadVariant(property->m_value, Color::CreateZero(), inputValue, context); + + if(resultCode.GetProcessing() != JsonSerializationResult::Processing::Completed) + { + resultCode = LoadVariant(property->m_value, Vector4::CreateZero(), inputValue, context); + } + + if(resultCode.GetProcessing() != JsonSerializationResult::Processing::Completed) + { + resultCode = LoadVariant(property->m_value, Vector3::CreateZero(), inputValue, context); + } + + if(resultCode.GetProcessing() != JsonSerializationResult::Processing::Completed) + { + resultCode = LoadVariant(property->m_value, Vector2::CreateZero(), inputValue, context); + } + + if(resultCode.GetProcessing() == JsonSerializationResult::Processing::Completed) + { + result.Combine(resultCode); + } + else { - case MaterialPropertyDataType::Bool: - result.Combine(LoadVariant(property->m_value, false, inputValue, context)); - break; - case MaterialPropertyDataType::Int: - result.Combine(LoadVariant(property->m_value, 0, inputValue, context)); - break; - case MaterialPropertyDataType::UInt: - result.Combine(LoadVariant(property->m_value, 0u, inputValue, context)); - break; - case MaterialPropertyDataType::Float: - result.Combine(LoadVariant(property->m_value, 0.0f, inputValue, context)); - break; - case MaterialPropertyDataType::Vector2: - result.Combine(LoadVariant(property->m_value, Vector2{0.0f, 0.0f}, inputValue, context)); - break; - case MaterialPropertyDataType::Vector3: - result.Combine(LoadVariant(property->m_value, Vector3{0.0f, 0.0f, 0.0f}, inputValue, context)); - break; - case MaterialPropertyDataType::Vector4: - result.Combine(LoadVariant(property->m_value, Vector4{0.0f, 0.0f, 0.0f, 0.0f}, inputValue, context)); - break; - case MaterialPropertyDataType::Color: - result.Combine(LoadVariant(property->m_value, AZ::Colors::White, inputValue, context)); - break; - case MaterialPropertyDataType::Image: - case MaterialPropertyDataType::Enum: - result.Combine(LoadVariant(property->m_value, AZStd::string{}, inputValue, context)); - break; - default: return context.Report(JsonSerializationResult::Tasks::ReadField, JsonSerializationResult::Outcomes::Unsupported, "Unknown data type"); } } + else if (inputValue.IsString()) + { + result.Combine(LoadVariant(property->m_value, AZStd::string{}, inputValue, context)); + } + else + { + return context.Report(JsonSerializationResult::Tasks::ReadField, JsonSerializationResult::Outcomes::Unsupported, "Unknown data type"); + } if (result.GetProcessing() == JsonSerializationResult::Processing::Completed) { 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 bc764ec7fb..b921b186c0 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceData.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceData.cpp @@ -8,7 +8,6 @@ #include #include -#include #include #include #include @@ -45,13 +44,17 @@ namespace AZ { if (JsonRegistrationContext* jsonContext = azrtti_cast(context)) { - jsonContext->Serializer()->HandlesType(); jsonContext->Serializer()->HandlesType(); } else if (auto* serializeContext = azrtti_cast(context)) { serializeContext->Class() - ->Version(1) + ->Version(2) + ->Field("description", &MaterialSourceData::m_description) + ->Field("materialType", &MaterialSourceData::m_materialType) + ->Field("materialTypeVersion", &MaterialSourceData::m_materialTypeVersion) + ->Field("parentMaterial", &MaterialSourceData::m_parentMaterial) + ->Field("properties", &MaterialSourceData::m_properties) ; serializeContext->RegisterGenericType(); @@ -80,6 +83,12 @@ namespace AZ MaterialAssetCreator materialAssetCreator; materialAssetCreator.SetElevateWarnings(elevateWarnings); + if (m_materialType.empty()) + { + AZ_Error("MaterialSourceData", false, "materialType was not specified"); + return Failure(); + } + Outcome materialTypeAssetId = AssetUtils::MakeAssetId(materialSourceFilePath, m_materialType, 0); if (!materialTypeAssetId) { @@ -194,6 +203,12 @@ namespace AZ bool elevateWarnings, AZStd::unordered_set* sourceDependencies) const { + if (m_materialType.empty()) + { + AZ_Error("MaterialSourceData", false, "materialType was not specified"); + return Failure(); + } + const auto materialTypeSourcePath = AssetUtils::ResolvePathReference(materialSourceFilePath, m_materialType); const auto materialTypeAssetId = AssetUtils::MakeAssetId(materialTypeSourcePath, 0); if (!materialTypeAssetId.IsSuccess()) diff --git a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceDataSerializer.cpp b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceDataSerializer.cpp deleted file mode 100644 index 2a504fc345..0000000000 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceDataSerializer.cpp +++ /dev/null @@ -1,162 +0,0 @@ -/* - * Copyright (c) Contributors to the Open 3D Engine Project. - * For complete copyright and license terms please see the LICENSE at the root of this distribution. - * - * SPDX-License-Identifier: Apache-2.0 OR MIT - * - */ - -#include -#include -#include -#include -#include -#include -#include - -#include -#include -#include -#include - -namespace AZ -{ - namespace RPI - { - AZ_CLASS_ALLOCATOR_IMPL(JsonMaterialSourceDataSerializer, SystemAllocator, 0); - - JsonSerializationResult::Result JsonMaterialSourceDataSerializer::Load(void* outputValue, const Uuid& outputValueTypeId, - const rapidjson::Value& inputValue, JsonDeserializerContext& context) - { - namespace JSR = JsonSerializationResult; - - AZ_Assert(azrtti_typeid() == outputValueTypeId, - "Unable to deserialize material to json because the provided type is %s", - outputValueTypeId.ToString().c_str()); - AZ_UNUSED(outputValueTypeId); - - MaterialSourceData* materialSourceData = reinterpret_cast(outputValue); - AZ_Assert(materialSourceData, "Output value for JsonMaterialSourceDataSerializer can't be null."); - - JSR::ResultCode result(JSR::Tasks::ReadField); - - if (!inputValue.IsObject()) - { - return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::Unsupported, "Material data must be a JSON object"); - } - - result.Combine(ContinueLoadingFromJsonObjectField(&materialSourceData->m_description, azrtti_typeid(), inputValue, "description", context)); - result.Combine(ContinueLoadingFromJsonObjectField(&materialSourceData->m_parentMaterial, azrtti_typeid(), inputValue, "parentMaterial", context)); - result.Combine(ContinueLoadingFromJsonObjectField(&materialSourceData->m_materialType, azrtti_typeid(), inputValue, "materialType", context)); - result.Combine(ContinueLoadingFromJsonObjectField(&materialSourceData->m_materialTypeVersion, azrtti_typeid(), inputValue, "materialTypeVersion", context)); - - if (materialSourceData->m_materialType.empty()) - { - return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::Catastrophic, "Required field 'materialType' is missing or invalid"); - } - - JsonFileLoadContext* jsonFileLoadContext = context.GetMetadata().Find(); - - if (!jsonFileLoadContext) - { - // Go ahead and create a JsonFileLoadContext because we'll need to use it below when loading the material type - context.GetMetadata().Add(JsonFileLoadContext{}); - jsonFileLoadContext = context.GetMetadata().Find(); - } - - // Load the material type file because we need the property type information in order to know how to read the property values - MaterialTypeSourceData materialTypeData; - { - AZStd::string materialTypePath = AssetUtils::ResolvePathReference(jsonFileLoadContext->GetFilePath(), materialSourceData->m_materialType); - - auto materialTypeJson = JsonSerializationUtils::ReadJsonFile(materialTypePath, AZ::RPI::JsonUtils::DefaultMaxFileSize); - if (!materialTypeJson.IsSuccess()) - { - AZStd::string failureMessage; - failureMessage = AZStd::string::format("Failed to load material-type file '%s': %s", materialTypePath.c_str(), materialTypeJson.GetError().c_str()); - ScopedContextPath subPath{context, "materialType"}; - return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::Catastrophic, failureMessage); - } - else - { - // Since we're about to load a different file the JsonFileLoadContext needs to be changed to reflect the file that's being loaded. - jsonFileLoadContext->PushFilePath(materialTypePath); - - // We also need a special reporting function for the material type, to note the fact that the issue is in the material type not this file. - auto reportingPrev = context.GetReporter(); - context.PushReporter([materialTypePath, reportingPrev](AZStd::string_view message, JSR::ResultCode result, AZStd::string_view path) -> JSR::ResultCode - { - AZStd::string materialTypeFilename; - if (!AzFramework::StringFunc::Path::GetFullFileName(materialTypePath.c_str(), materialTypeFilename)) - { - materialTypeFilename = materialTypePath; - } - - AZStd::string newPath = AZStd::string::format("[%.*s]%.*s", AZ_STRING_ARG(materialTypeFilename), AZ_STRING_ARG(path)); - return reportingPrev(message, result, newPath); - }); - - JsonDeserializerSettings settings; - settings.m_metadata = context.GetMetadata(); - settings.m_reporting = context.GetReporter(); - settings.m_registrationContext = context.GetRegistrationContext(); - settings.m_serializeContext = context.GetSerializeContext(); - settings.m_clearContainers = context.ShouldClearContainers(); - - JsonSerializationResult::ResultCode materialTypeLoadResult = JsonSerialization::Load(materialTypeData, materialTypeJson.GetValue(), settings); - materialTypeData.ResolveUvEnums(); - - // Restore prior configuration - context.PopReporter(); - jsonFileLoadContext->PopFilePath(); - - // Even though results from the material type file is a separate JSON serialization, we combine the results to make sure - // any issues are bubbled up. I'm not sure if this is the most desirable approach, but better to over-report issues than - // under-report them. - result.Combine(materialTypeLoadResult); - } - } - - context.GetMetadata().Add(AZStd::move(materialTypeData)); - - JsonMaterialPropertyValueSerializer::LoadContext materialPropertyValueLoadContext; - materialPropertyValueLoadContext.m_materialTypeVersion = materialSourceData->m_materialTypeVersion; - context.GetMetadata().Add(materialPropertyValueLoadContext); - - result.Combine(ContinueLoadingFromJsonObjectField(&materialSourceData->m_properties, azrtti_typeid(), inputValue, "properties", context)); - - if (result.GetProcessing() == JsonSerializationResult::Processing::Completed) - { - return context.Report(result, "Successfully loaded material."); - } - else - { - return context.Report(result, "Partially loaded material."); - } - } - - - JsonSerializationResult::Result JsonMaterialSourceDataSerializer::Store(rapidjson::Value& outputValue, const void* inputValue, - [[maybe_unused]] const void* defaultValue, const Uuid& valueTypeId, JsonSerializerContext& context) - { - namespace JSR = JsonSerializationResult; - - AZ_Assert(azrtti_typeid() == valueTypeId, - "Unable to serialize material to json because the provided type is %s", - valueTypeId.ToString().c_str()); - AZ_UNUSED(valueTypeId); - - const MaterialSourceData* materialSourceData = reinterpret_cast(inputValue); - AZ_Assert(materialSourceData, "Input value for JsonMaterialSourceDataSerializer can't be null."); - - JSR::ResultCode resultCode(JSR::Tasks::ReadField); - resultCode.Combine(ContinueStoringToJsonObjectField(outputValue, "description", &materialSourceData->m_description, nullptr, azrtti_typeid(), context)); - resultCode.Combine(ContinueStoringToJsonObjectField(outputValue, "parentMaterial", &materialSourceData->m_parentMaterial, nullptr, azrtti_typeid(), context)); - resultCode.Combine(ContinueStoringToJsonObjectField(outputValue, "materialType", &materialSourceData->m_materialType, nullptr, azrtti_typeid(), context)); - resultCode.Combine(ContinueStoringToJsonObjectField(outputValue, "materialTypeVersion", &materialSourceData->m_materialTypeVersion, nullptr, azrtti_typeid(), context)); - resultCode.Combine(ContinueStoringToJsonObjectField(outputValue, "properties", &materialSourceData->m_properties, nullptr, azrtti_typeid(), context)); - - return context.Report(resultCode, "Processed material."); - } - } // namespace RPI -} // namespace AZ 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 d8e6c156be..87c064f571 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialTypeSourceData.cpp @@ -130,17 +130,12 @@ namespace AZ return nullptr; } - bool MaterialTypeSourceData::ApplyPropertyRenames(MaterialPropertyId& propertyId, uint32_t materialTypeVersion) const + bool MaterialTypeSourceData::ApplyPropertyRenames(MaterialPropertyId& propertyId) const { bool renamed = false; for (const VersionUpdateDefinition& versionUpdate : m_versionUpdates) { - if (materialTypeVersion >= versionUpdate.m_toVersion) - { - continue; - } - for (const VersionUpdatesRenameOperationDefinition& action : versionUpdate.m_actions) { if (action.m_operation == "rename") @@ -161,7 +156,7 @@ namespace AZ return renamed; } - const MaterialTypeSourceData::PropertyDefinition* MaterialTypeSourceData::FindProperty(AZStd::string_view groupName, AZStd::string_view propertyName, uint32_t materialTypeVersion) const + const MaterialTypeSourceData::PropertyDefinition* MaterialTypeSourceData::FindProperty(AZStd::string_view groupName, AZStd::string_view propertyName) const { auto groupIter = m_propertyLayout.m_properties.find(groupName); if (groupIter != m_propertyLayout.m_properties.end()) @@ -178,7 +173,7 @@ namespace AZ // Property has not been found, try looking for renames in the version history MaterialPropertyId propertyId = MaterialPropertyId{groupName, propertyName}; - ApplyPropertyRenames(propertyId, materialTypeVersion); + ApplyPropertyRenames(propertyId); // Do the search again with the new names 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 309daf8b62..8bf975fd82 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAsset.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Material/MaterialAsset.cpp @@ -109,6 +109,77 @@ namespace AZ return m_wasPreFinalized; } + template + MaterialPropertyValue CastNumericMaterialPropertyValue(const MaterialPropertyValue& value) + { + TypeId typeId = value.GetTypeId(); + + if (typeId == azrtti_typeid()) + { + return aznumeric_cast(value.GetValue()); + } + else if (typeId == azrtti_typeid()) + { + return aznumeric_cast(value.GetValue()); + } + else if (typeId == azrtti_typeid()) + { + return aznumeric_cast(value.GetValue()); + } + else if (typeId == azrtti_typeid()) + { + return aznumeric_cast(value.GetValue()); + } + else + { + return value; + } + } + + + + template + MaterialPropertyValue CastVectorMaterialPropertyValue(const MaterialPropertyValue& value) + { + float values[4] = {}; + + TypeId typeId = value.GetTypeId(); + if (typeId == azrtti_typeid()) + { + value.GetValue().StoreToFloat2(values); + } + else if (typeId == azrtti_typeid()) + { + value.GetValue().StoreToFloat3(values); + } + else if (typeId == azrtti_typeid()) + { + value.GetValue().StoreToFloat4(values); + } + else + { + return value; + } + + typeId = azrtti_typeid(); + if (typeId == azrtti_typeid()) + { + return Vector2::CreateFromFloat2(values); + } + else if (typeId == azrtti_typeid()) + { + return Vector3::CreateFromFloat3(values); + } + else if (typeId == azrtti_typeid()) + { + return Vector4::CreateFromFloat4(values); + } + else + { + return value; + } + } + void MaterialAsset::Finalize(AZStd::function reportWarning, AZStd::function reportError) { if (m_wasPreFinalized) @@ -180,9 +251,66 @@ namespace AZ } else { - if (ValidateMaterialPropertyDataType(value.GetTypeId(), name, propertyDescriptor, reportError)) + // The material asset could be finalized sometime after the original JSON is loaded, and the material type might not have been available + // at that time, so the data type would not be known for each property. So each raw property's type could be based on what appeared in the JSON + // and this is the first opportunity we have to resolve that value with the actual type. For example, a float property could have been specified in + // the JSON as 7 instead of 7.0, which is valid. Similarly, a Color and a Vector3 can both be specified as "[0.0,0.0,0.0]" in the JSON file. + + MaterialPropertyValue finalValue = value; + + switch (propertyDescriptor->GetDataType()) + { + case MaterialPropertyDataType::Bool: + finalValue = CastNumericMaterialPropertyValue(value); + break; + case MaterialPropertyDataType::Int: + finalValue = CastNumericMaterialPropertyValue(value); + break; + case MaterialPropertyDataType::UInt: + finalValue = CastNumericMaterialPropertyValue(value); + break; + case MaterialPropertyDataType::Float: + finalValue = CastNumericMaterialPropertyValue(value); + break; + case MaterialPropertyDataType::Color: + if (value.GetTypeId() == azrtti_typeid()) + { + finalValue = Color::CreateFromVector3(value.GetValue()); + } + else if (value.GetTypeId() == azrtti_typeid()) + { + Vector4 vector4 = value.GetValue(); + finalValue = Color::CreateFromVector3AndFloat(vector4.GetAsVector3(), vector4.GetW()); + } + break; + case MaterialPropertyDataType::Vector2: + finalValue = CastVectorMaterialPropertyValue(value); + break; + case MaterialPropertyDataType::Vector3: + if (value.GetTypeId() == azrtti_typeid()) + { + finalValue = value.GetValue().GetAsVector3(); + } + else + { + finalValue = CastVectorMaterialPropertyValue(value); + } + break; + case MaterialPropertyDataType::Vector4: + if (value.GetTypeId() == azrtti_typeid()) + { + finalValue = value.GetValue().GetAsVector4(); + } + else + { + finalValue = CastVectorMaterialPropertyValue(value); + } + break; + } + + if (ValidateMaterialPropertyDataType(finalValue.GetTypeId(), name, propertyDescriptor, reportError)) { - finalizedPropertyValues[propertyIndex.GetIndex()] = value; + finalizedPropertyValues[propertyIndex.GetIndex()] = finalValue; } } } diff --git a/Gems/Atom/RPI/Code/Tests/Material/MaterialAssetTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/MaterialAssetTests.cpp index 61e1283f52..ea637fb17d 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/MaterialAssetTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/MaterialAssetTests.cpp @@ -449,37 +449,7 @@ namespace UnitTest expectCreatorError("Type mismatch", [](MaterialAssetCreator& creator) { - creator.SetPropertyValue(Name{ "MyInt" }, 0.0f); - }); - - expectCreatorError("Type mismatch", - [](MaterialAssetCreator& creator) - { - creator.SetPropertyValue(Name{ "MyUInt" }, -1); - }); - - expectCreatorError("Type mismatch", - [](MaterialAssetCreator& creator) - { - creator.SetPropertyValue(Name{ "MyFloat" }, 10u); - }); - - expectCreatorError("Type mismatch", - [](MaterialAssetCreator& creator) - { - creator.SetPropertyValue(Name{ "MyFloat2" }, 1.0f); - }); - - expectCreatorError("Type mismatch", - [](MaterialAssetCreator& creator) - { - creator.SetPropertyValue(Name{ "MyFloat3" }, AZ::Vector4{}); - }); - - expectCreatorError("Type mismatch", - [](MaterialAssetCreator& creator) - { - creator.SetPropertyValue(Name{ "MyFloat4" }, AZ::Vector3{}); + creator.SetPropertyValue(Name{ "MyFloat" }, AZ::Vector4{}); }); expectCreatorError("Type mismatch", diff --git a/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp index ef89e0138c..5e09fe6612 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/MaterialSourceDataTests.cpp @@ -297,6 +297,63 @@ namespace UnitTest checkRawPropertyValues(); } + // Can return a Vector4 or a Color as a Vector4 + Vector4 GetAsVector4(const MaterialPropertyValue& value) + { + if (value.GetTypeId() == azrtti_typeid()) + { + return value.GetValue(); + } + else if (value.GetTypeId() == azrtti_typeid()) + { + return value.GetValue().GetAsVector4(); + } + else + { + return Vector4::CreateZero(); + } + } + + // Can return a Int or a UInt as a Int + int32_t GetAsInt(const MaterialPropertyValue& value) + { + if (value.GetTypeId() == azrtti_typeid()) + { + return value.GetValue(); + } + else if (value.GetTypeId() == azrtti_typeid()) + { + return aznumeric_cast(value.GetValue()); + } + else + { + return 0; + } + } + + template + bool AreTypesCompatible(const MaterialPropertyValue& a, const MaterialPropertyValue& b) + { + auto fixupType = [](TypeId t) + { + if (t == azrtti_typeid()) + { + return azrtti_typeid(); + } + + if (t == azrtti_typeid()) + { + return azrtti_typeid(); + } + + return t; + }; + + TypeId targetTypeId = azrtti_typeid(); + + return fixupType(a.GetTypeId()) == fixupType(targetTypeId) && fixupType(b.GetTypeId()) == fixupType(targetTypeId); + } + void CheckEqual(MaterialSourceData& a, MaterialSourceData& b) { EXPECT_STREQ(a.m_materialType.data(), b.m_materialType.data()); @@ -334,27 +391,41 @@ namespace UnitTest auto& propertyA = propertyIterA.second; auto& propertyB = propertyIterB->second; - bool typesMatch = propertyA.m_value.GetTypeId() == propertyB.m_value.GetTypeId(); - EXPECT_TRUE(typesMatch); - if (typesMatch) + AZStd::string propertyReference = AZStd::string::format(" for property '%s.%s'", groupName.c_str(), propertyName.c_str()); + + // 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.m_value, propertyB.m_value)) { - AZStd::string propertyReference = AZStd::string::format(" for property '%s.%s'", groupName.c_str(), propertyName.c_str()); - - auto typeId = propertyA.m_value.GetTypeId(); - - if (typeId == azrtti_typeid()) { EXPECT_EQ(propertyA.m_value.GetValue(), propertyB.m_value.GetValue()) << propertyReference.c_str(); } - else if (typeId == azrtti_typeid()) { EXPECT_EQ(propertyA.m_value.GetValue(), propertyB.m_value.GetValue()) << propertyReference.c_str(); } - else if (typeId == azrtti_typeid()) { EXPECT_EQ(propertyA.m_value.GetValue(), propertyB.m_value.GetValue()) << propertyReference.c_str(); } - else if (typeId == azrtti_typeid()) { EXPECT_NEAR(propertyA.m_value.GetValue(), propertyB.m_value.GetValue(), 0.01) << propertyReference.c_str(); } - else if (typeId == azrtti_typeid()) { EXPECT_TRUE(propertyA.m_value.GetValue().IsClose(propertyB.m_value.GetValue())) << propertyReference.c_str(); } - else if (typeId == azrtti_typeid()) { EXPECT_TRUE(propertyA.m_value.GetValue().IsClose(propertyB.m_value.GetValue())) << propertyReference.c_str(); } - else if (typeId == azrtti_typeid()) { EXPECT_TRUE(propertyA.m_value.GetValue().IsClose(propertyB.m_value.GetValue())) << propertyReference.c_str(); } - else if (typeId == azrtti_typeid()) { EXPECT_TRUE(propertyA.m_value.GetValue().IsClose(propertyB.m_value.GetValue())) << propertyReference.c_str(); } - else if (typeId == azrtti_typeid()) { EXPECT_STREQ(propertyA.m_value.GetValue().c_str(), propertyB.m_value.GetValue().c_str()) << propertyReference.c_str(); } - else - { - ADD_FAILURE(); - } + EXPECT_EQ(propertyA.m_value.GetValue(), propertyB.m_value.GetValue()) << propertyReference.c_str(); + } + else if (AreTypesCompatible(propertyA.m_value, propertyB.m_value)) + { + EXPECT_EQ(GetAsInt(propertyA.m_value), GetAsInt(propertyB.m_value)) << propertyReference.c_str(); + } + else if (AreTypesCompatible(propertyA.m_value, propertyB.m_value)) + { + EXPECT_NEAR(propertyA.m_value.GetValue(), propertyB.m_value.GetValue(), 0.01) << propertyReference.c_str(); + } + else if (AreTypesCompatible(propertyA.m_value, propertyB.m_value)) + { + EXPECT_TRUE(propertyA.m_value.GetValue().IsClose(propertyB.m_value.GetValue())) << propertyReference.c_str(); + } + else if (AreTypesCompatible(propertyA.m_value, propertyB.m_value)) + { + EXPECT_TRUE(propertyA.m_value.GetValue().IsClose(propertyB.m_value.GetValue())) << propertyReference.c_str(); + } + else if (AreTypesCompatible(propertyA.m_value, propertyB.m_value)) + { + EXPECT_TRUE(GetAsVector4(propertyA.m_value).IsClose(GetAsVector4(propertyB.m_value))) << propertyReference.c_str(); + } + else if (AreTypesCompatible(propertyA.m_value, propertyB.m_value)) + { + EXPECT_STREQ(propertyA.m_value.GetValue().c_str(), propertyB.m_value.GetValue().c_str()) << propertyReference.c_str(); + } + else + { + ADD_FAILURE(); } } } @@ -363,42 +434,8 @@ namespace UnitTest TEST_F(MaterialSourceDataTests, TestJsonRoundTrip) { - const char* materialTypeJson = - "{ \n" - " \"propertyLayout\": { \n" - " \"version\": 1, \n" - " \"groups\": [ \n" - " { \"name\": \"groupA\" }, \n" - " { \"name\": \"groupB\" }, \n" - " { \"name\": \"groupC\" } \n" - " ], \n" - " \"properties\": { \n" - " \"groupA\": [ \n" - " {\"name\": \"MyBool\", \"type\": \"bool\"}, \n" - " {\"name\": \"MyInt\", \"type\": \"int\"}, \n" - " {\"name\": \"MyUInt\", \"type\": \"uint\"} \n" - " ], \n" - " \"groupB\": [ \n" - " {\"name\": \"MyFloat\", \"type\": \"float\"}, \n" - " {\"name\": \"MyFloat2\", \"type\": \"vector2\"}, \n" - " {\"name\": \"MyFloat3\", \"type\": \"vector3\"} \n" - " ], \n" - " \"groupC\": [ \n" - " {\"name\": \"MyFloat4\", \"type\": \"vector4\"}, \n" - " {\"name\": \"MyColor\", \"type\": \"color\"}, \n" - " {\"name\": \"MyImage\", \"type\": \"image\"} \n" - " ] \n" - " } \n" - " } \n" - "} \n"; - const char* materialTypeFilePath = "@exefolder@/Temp/roundTripTest.materialtype"; - AZ::IO::FileIOStream file; - EXPECT_TRUE(file.Open(materialTypeFilePath, AZ::IO::OpenMode::ModeWrite | AZ::IO::OpenMode::ModeCreatePath)); - file.Write(strlen(materialTypeJson), materialTypeJson); - file.Close(); - MaterialSourceData sourceDataOriginal; sourceDataOriginal.m_materialType = materialTypeFilePath; sourceDataOriginal.m_parentMaterial = materialTypeFilePath; @@ -434,8 +471,8 @@ namespace UnitTest "properties": { "general": [ { - "name": "testColor", - "type": "color" + "name": "testValue", + "type": "Float" } ] } @@ -456,7 +493,7 @@ namespace UnitTest { "properties": { "general": { - "testColor": [0.1,0.2,0.3] + "testValue": 1.2 } }, "materialType": "@exefolder@/Temp/simpleMaterialType.materialtype" @@ -469,27 +506,11 @@ namespace UnitTest EXPECT_EQ(AZ::JsonSerializationResult::Tasks::ReadField, loadResult.m_jsonResultCode.GetTask()); EXPECT_EQ(AZ::JsonSerializationResult::Processing::Completed, loadResult.m_jsonResultCode.GetProcessing()); - AZ::Color testColor = material.m_properties["general"]["testColor"].m_value.GetValue(); - EXPECT_TRUE(AZ::Color(0.1f, 0.2f, 0.3f, 1.0f).IsClose(testColor, 0.01)); - } - - TEST_F(MaterialSourceDataTests, Load_Error_NotAnObject) - { - const AZStd::string inputJson = R"( - [] - )"; - - MaterialSourceData material; - JsonTestResult loadResult = LoadTestDataFromJson(material, inputJson); - - EXPECT_EQ(AZ::JsonSerializationResult::Tasks::ReadField, loadResult.m_jsonResultCode.GetTask()); - EXPECT_EQ(AZ::JsonSerializationResult::Processing::Altered, loadResult.m_jsonResultCode.GetProcessing()); - EXPECT_EQ(AZ::JsonSerializationResult::Outcomes::Unsupported, loadResult.m_jsonResultCode.GetOutcome()); - - EXPECT_TRUE(loadResult.ContainsMessage("", "Material data must be a JSON object")); + float testValue = material.m_properties["general"]["testValue"].m_value.GetValue(); + EXPECT_FLOAT_EQ(1.2f, testValue); } - - TEST_F(MaterialSourceDataTests, Load_Error_NoMaterialType) + + TEST_F(MaterialSourceDataTests, CreateMaterialAsset_NoMaterialType) { const AZStd::string inputJson = R"( { @@ -505,14 +526,29 @@ namespace UnitTest MaterialSourceData material; JsonTestResult loadResult = LoadTestDataFromJson(material, inputJson); - EXPECT_EQ(AZ::JsonSerializationResult::Tasks::ReadField, loadResult.m_jsonResultCode.GetTask()); - EXPECT_EQ(AZ::JsonSerializationResult::Processing::Halted, loadResult.m_jsonResultCode.GetProcessing()); - EXPECT_EQ(AZ::JsonSerializationResult::Outcomes::Catastrophic, loadResult.m_jsonResultCode.GetOutcome()); + const bool elevateWarnings = false; - EXPECT_TRUE(loadResult.ContainsMessage("", "Required field 'materialType' is missing")); - } + ErrorMessageFinder errorMessageFinder; - TEST_F(MaterialSourceDataTests, Load_Error_MaterialTypeDoesNotExist) + errorMessageFinder.AddExpectedErrorMessage("materialType was not specified"); + auto result = material.CreateMaterialAsset(AZ::Uuid::CreateRandom(), "test.material", AZ::RPI::MaterialAssetProcessingMode::DeferredBake, elevateWarnings); + EXPECT_FALSE(result.IsSuccess()); + errorMessageFinder.CheckExpectedErrorsFound(); + + errorMessageFinder.Reset(); + errorMessageFinder.AddExpectedErrorMessage("materialType was not specified"); + result = material.CreateMaterialAsset(AZ::Uuid::CreateRandom(), "test.material", AZ::RPI::MaterialAssetProcessingMode::PreBake, elevateWarnings); + EXPECT_FALSE(result.IsSuccess()); + errorMessageFinder.CheckExpectedErrorsFound(); + + errorMessageFinder.Reset(); + errorMessageFinder.AddExpectedErrorMessage("materialType was not specified"); + result = material.CreateMaterialAssetFromSourceData(AZ::Uuid::CreateRandom(), "test.material", elevateWarnings); + EXPECT_FALSE(result.IsSuccess()); + errorMessageFinder.CheckExpectedErrorsFound(); + } + + TEST_F(MaterialSourceDataTests, CreateMaterialAsset_MaterialTypeDoesNotExist) { const AZStd::string inputJson = R"( { @@ -529,102 +565,43 @@ namespace UnitTest MaterialSourceData material; JsonTestResult loadResult = LoadTestDataFromJson(material, inputJson); - EXPECT_EQ(AZ::JsonSerializationResult::Tasks::ReadField, loadResult.m_jsonResultCode.GetTask()); - EXPECT_EQ(AZ::JsonSerializationResult::Processing::Halted, loadResult.m_jsonResultCode.GetProcessing()); - EXPECT_EQ(AZ::JsonSerializationResult::Outcomes::Catastrophic, loadResult.m_jsonResultCode.GetOutcome()); + const bool elevateWarnings = false; - EXPECT_TRUE(loadResult.ContainsMessage("/materialType", "Failed to load material-type file")); - } + ErrorMessageFinder errorMessageFinder; - TEST_F(MaterialSourceDataTests, Load_MaterialTypeMessagesAreReported) - { - const AZStd::string simpleMaterialTypeJson = R"( - { - "propertyLayout": { - "properties": { - "general": [ - { - "name": "testColor", - "type": "color" - } - ] - } - } - } - )"; - - 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(); - - const AZStd::string inputJson = R"( - { - "materialType": "@exefolder@/Temp/simpleMaterialType.materialtype", - "materialTypeVersion": 1, - "properties": { - "general": { - "testColor": [1.0,1.0,1.0] - } - } - } - )"; + errorMessageFinder.AddExpectedErrorMessage("Could not find asset [DoesNotExist.materialtype]"); + auto result = material.CreateMaterialAsset(AZ::Uuid::CreateRandom(), "test.material", AZ::RPI::MaterialAssetProcessingMode::DeferredBake, elevateWarnings); + EXPECT_FALSE(result.IsSuccess()); + errorMessageFinder.CheckExpectedErrorsFound(); - MaterialSourceData material; - JsonTestResult loadResult = LoadTestDataFromJson(material, inputJson); - - EXPECT_EQ(AZ::JsonSerializationResult::Tasks::ReadField, loadResult.m_jsonResultCode.GetTask()); - EXPECT_EQ(AZ::JsonSerializationResult::Processing::Completed, loadResult.m_jsonResultCode.GetProcessing()); - - // propertyLayout is a field in the material type, not the material - EXPECT_TRUE(loadResult.ContainsMessage("[simpleMaterialType.materialtype]/propertyLayout/properties", "Successfully read")); + errorMessageFinder.Reset(); + errorMessageFinder.AddExpectedErrorMessage("Could not find asset [DoesNotExist.materialtype]"); + result = material.CreateMaterialAsset(AZ::Uuid::CreateRandom(), "test.material", AZ::RPI::MaterialAssetProcessingMode::PreBake, elevateWarnings); + EXPECT_FALSE(result.IsSuccess()); + errorMessageFinder.CheckExpectedErrorsFound(); + + errorMessageFinder.Reset(); + errorMessageFinder.AddExpectedErrorMessage("Could not find asset [DoesNotExist.materialtype]"); + errorMessageFinder.AddIgnoredErrorMessage("Failed to create material type asset ID", true); + result = material.CreateMaterialAssetFromSourceData(AZ::Uuid::CreateRandom(), "test.material", elevateWarnings); + EXPECT_FALSE(result.IsSuccess()); + errorMessageFinder.CheckExpectedErrorsFound(); } - - TEST_F(MaterialSourceDataTests, Load_Error_PropertyNotFound) + + TEST_F(MaterialSourceDataTests, CreateMaterialAsset_MaterialPropertyNotFound) { - const AZStd::string simpleMaterialTypeJson = R"( - { - "propertyLayout": { - "properties": { - "general": [ - { - "name": "testColor", - "type": "color" - } - ] - } - } - } - )"; - - 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(); - - const AZStd::string inputJson = R"( - { - "materialType": "@exefolder@/Temp/simpleMaterialType.materialtype", - "materialTypeVersion": 1, - "properties": { - "general": { - "doesNotExist": [1.0,1.0,1.0] - } - } - } - )"; - MaterialSourceData material; - JsonTestResult loadResult = LoadTestDataFromJson(material, inputJson); - - EXPECT_EQ(AZ::JsonSerializationResult::Tasks::ReadField, loadResult.m_jsonResultCode.GetTask()); - EXPECT_EQ(AZ::JsonSerializationResult::Processing::PartialAlter, loadResult.m_jsonResultCode.GetProcessing()); + material.m_materialType = "@exefolder@/Temp/test.materialtype"; + AddPropertyGroup(material, "general"); + AddProperty(material, "general", "FieldDoesNotExist", 1.5f); + + const bool elevateWarnings = true; - EXPECT_TRUE(loadResult.ContainsMessage("/properties/general/doesNotExist", "Property 'general.doesNotExist' not found in material type.")); + ErrorMessageFinder errorMessageFinder("\"general.FieldDoesNotExist\" is not found"); + errorMessageFinder.AddIgnoredErrorMessage("Failed to build MaterialAsset", true); + auto result = material.CreateMaterialAsset(AZ::Uuid::CreateRandom(), "test.material", AZ::RPI::MaterialAssetProcessingMode::PreBake, elevateWarnings); + EXPECT_FALSE(result.IsSuccess()); + errorMessageFinder.CheckExpectedErrorsFound(); } TEST_F(MaterialSourceDataTests, CreateMaterialAsset_MultiLevelDataInheritance) @@ -896,7 +873,92 @@ namespace UnitTest AddProperty(materialSourceData, "general", "MyImage", AZStd::string("doesNotExist.streamingimage")); }, true); // In this case, the warning does happen even when the asset is not finalized, because the image path is checked earlier than that } + + template + void CheckSimilar(PropertyTypeT a, PropertyTypeT b); + + template<> void CheckSimilar(float a, float b) { EXPECT_FLOAT_EQ(a, b); } + template<> void CheckSimilar(Vector2 a, Vector2 b) { EXPECT_TRUE(a.IsClose(b)); } + template<> void CheckSimilar(Vector3 a, Vector3 b) { EXPECT_TRUE(a.IsClose(b)); } + template<> void CheckSimilar(Vector4 a, Vector4 b) { EXPECT_TRUE(a.IsClose(b)); } + template<> void CheckSimilar(Color a, Color b) { EXPECT_TRUE(a.IsClose(b)); } + + template void CheckSimilar(PropertyTypeT a, PropertyTypeT b) { EXPECT_EQ(a, b); } + + template + void CheckEndToEndDataTypeResolution(const char* propertyName, const char* jsonValue, PropertyTypeT expectedFinalValue) + { + const char* groupName = "general"; + + const AZStd::string inputJson = AZStd::string::format(R"( + { + "materialType": "@exefolder@/Temp/test.materialtype", + "properties": { + "%s": { + "%s": %s + } + } + } + )", groupName, propertyName, jsonValue); + + MaterialSourceData material; + JsonTestResult loadResult = LoadTestDataFromJson(material, inputJson); + auto materialAssetResult = material.CreateMaterialAsset(Uuid::CreateRandom(), "test.material", AZ::RPI::MaterialAssetProcessingMode::PreBake); + EXPECT_TRUE(materialAssetResult); + MaterialPropertyIndex propertyIndex = materialAssetResult.GetValue()->GetMaterialPropertiesLayout()->FindPropertyIndex(MaterialPropertyId{groupName, propertyName}.GetFullName()); + CheckSimilar(expectedFinalValue, materialAssetResult.GetValue()->GetPropertyValues()[propertyIndex.GetIndex()].GetValue()); + } + TEST_F(MaterialSourceDataTests, TestEndToEndDataTypeResolution) + { + // Data types in .material files don't have to exactly match the types in .materialtype files as specified in the properties layout. + // The exact location of the data type resolution has moved around over the life of the project, but the important thing is that + // the data type in the source .material file gets applied correctly by the time a finalized MaterialAsset comes out the other side. + + CheckEndToEndDataTypeResolution("MyBool", "true", true); + CheckEndToEndDataTypeResolution("MyBool", "false", false); + CheckEndToEndDataTypeResolution("MyBool", "1", true); + CheckEndToEndDataTypeResolution("MyBool", "0", false); + CheckEndToEndDataTypeResolution("MyBool", "1.0", true); + CheckEndToEndDataTypeResolution("MyBool", "0.0", false); + + CheckEndToEndDataTypeResolution("MyInt", "5", 5); + CheckEndToEndDataTypeResolution("MyInt", "-6", -6); + CheckEndToEndDataTypeResolution("MyInt", "-7.0", -7); + CheckEndToEndDataTypeResolution("MyInt", "false", 0); + CheckEndToEndDataTypeResolution("MyInt", "true", 1); + + CheckEndToEndDataTypeResolution("MyUInt", "8", 8u); + CheckEndToEndDataTypeResolution("MyUInt", "9.0", 9u); + CheckEndToEndDataTypeResolution("MyUInt", "false", 0u); + CheckEndToEndDataTypeResolution("MyUInt", "true", 1u); + + CheckEndToEndDataTypeResolution("MyFloat", "2", 2.0f); + CheckEndToEndDataTypeResolution("MyFloat", "-2", -2.0f); + CheckEndToEndDataTypeResolution("MyFloat", "2.1", 2.1f); + CheckEndToEndDataTypeResolution("MyFloat", "false", 0.0f); + CheckEndToEndDataTypeResolution("MyFloat", "true", 1.0f); + + CheckEndToEndDataTypeResolution("MyColor", "[0.1,0.2,0.3]", Color{0.1f, 0.2f, 0.3f, 1.0}); + CheckEndToEndDataTypeResolution("MyColor", "[0.1, 0.2, 0.3, 0.5]", Color{0.1f, 0.2f, 0.3f, 0.5f}); + CheckEndToEndDataTypeResolution("MyColor", "{\"RGB8\": [255, 0, 255, 0]}", Color{1.0f, 0.0f, 1.0f, 0.0f}); + + CheckEndToEndDataTypeResolution("MyFloat2", "[0.1,0.2]", Vector2{0.1f, 0.2f}); + CheckEndToEndDataTypeResolution("MyFloat2", "{\"y\":0.2, \"x\":0.1}", Vector2{0.1f, 0.2f}); + CheckEndToEndDataTypeResolution("MyFloat2", "{\"y\":0.2, \"x\":0.1, \"Z\":0.3}", Vector2{0.1f, 0.2f}); + CheckEndToEndDataTypeResolution("MyFloat2", "{\"y\":0.2, \"W\":0.4, \"x\":0.1, \"Z\":0.3}", Vector2{0.1f, 0.2f}); + + CheckEndToEndDataTypeResolution("MyFloat3", "[0.1,0.2,0.3]", Vector3{0.1f, 0.2f, 0.3f}); + CheckEndToEndDataTypeResolution("MyFloat3", "{\"y\":0.2, \"x\":0.1}", Vector3{0.1f, 0.2f, 0.0f}); + CheckEndToEndDataTypeResolution("MyFloat3", "{\"y\":0.2, \"x\":0.1, \"Z\":0.3}", Vector3{0.1f, 0.2f, 0.3f}); + CheckEndToEndDataTypeResolution("MyFloat3", "{\"y\":0.2, \"W\":0.4, \"x\":0.1, \"Z\":0.3}", Vector3{0.1f, 0.2f, 0.3f}); + + CheckEndToEndDataTypeResolution("MyFloat4", "[0.1,0.2,0.3,0.4]", Vector4{0.1f, 0.2f, 0.3f, 0.4f}); + CheckEndToEndDataTypeResolution("MyFloat4", "{\"y\":0.2, \"x\":0.1}", Vector4{0.1f, 0.2f, 0.0f, 0.0f}); + 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}); + } + } diff --git a/Gems/Atom/RPI/Code/atom_rpi_edit_files.cmake b/Gems/Atom/RPI/Code/atom_rpi_edit_files.cmake index e32d19ffd7..3c345cc00b 100644 --- a/Gems/Atom/RPI/Code/atom_rpi_edit_files.cmake +++ b/Gems/Atom/RPI/Code/atom_rpi_edit_files.cmake @@ -25,7 +25,6 @@ set(FILES Include/Atom/RPI.Edit/Material/MaterialPropertyValueSourceData.h Include/Atom/RPI.Edit/Material/MaterialPropertyValueSourceDataSerializer.h Include/Atom/RPI.Edit/Material/MaterialSourceData.h - Include/Atom/RPI.Edit/Material/MaterialSourceDataSerializer.h Include/Atom/RPI.Edit/Material/MaterialFunctorSourceData.h Include/Atom/RPI.Edit/Material/MaterialFunctorSourceDataSerializer.h Include/Atom/RPI.Edit/Material/MaterialFunctorSourceDataRegistration.h @@ -45,7 +44,6 @@ set(FILES Source/RPI.Edit/Material/MaterialPropertyValueSourceData.cpp Source/RPI.Edit/Material/MaterialPropertyValueSourceDataSerializer.cpp Source/RPI.Edit/Material/MaterialSourceData.cpp - Source/RPI.Edit/Material/MaterialSourceDataSerializer.cpp Source/RPI.Edit/Material/MaterialFunctorSourceData.cpp Source/RPI.Edit/Material/MaterialFunctorSourceDataSerializer.cpp Source/RPI.Edit/Material/MaterialFunctorSourceDataRegistration.cpp