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

Temporary fix for material component losing image overrides with prefabs

Browse Source 
The bug reported that overridden texture properties would be lost whenever an entity was created, destroyed, or a prefab was created. Initially, it seemed like there was a problem with the custom JSON serializer for material properties. Debugging proved this to be incorrect because all of the data was converted to JSON values in the serializer on multiple passes. At some point during prefab patching, the data for the asset properties is lost while other values like colors and floats serialize correctly. Converting the asset data values into asset IDs resolves the immediate problem for the material component but the underlying issue is still under investigation by the prefab team. This change is being posted for review in case the underlying issue cannot be resolved in time for the next release.

Signed-off-by: Guthrie Adams <guthadam@amazon.com>

Fixing unittests and moving texture conversion into material component controller

Signed-off-by: Guthrie Adams <guthadam@amazon.com>
This commit is contained in:
Guthrie Adams
2021-12-11 15:50:37 -06:00
parent 40e1116e26
commit 3bca63bb71
5 changed files with 76 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Gems/Atom/Feature/Common/Code/Source/Material/MaterialAssignmentSerializer.cpp
+17 -7
View File
@@ -16,7 +16,9 @@ namespace AZ
AZ_CLASS_ALLOCATOR_IMPL(JsonMaterialAssignmentSerializer, AZ::SystemAllocator, 0);
JsonSerializationResult::Result JsonMaterialAssignmentSerializer::Load(
void* outputValue, [[maybe_unused]] const Uuid& outputValueTypeId, const rapidjson::Value& inputValue,
void* outputValue,
[[maybe_unused]] const Uuid& outputValueTypeId,
const rapidjson::Value& inputValue,
JsonDeserializerContext& context)
{
namespace JSR = JsonSerializationResult;
@@ -62,6 +64,7 @@ namespace AZ
LoadAny<AZ::Color>(propertyValue, inputPropertyPair.value, context, result) ||
LoadAny<AZStd::string>(propertyValue, inputPropertyPair.value, context, result) ||
LoadAny<AZ::Data::AssetId>(propertyValue, inputPropertyPair.value, context, result) ||
LoadAny<AZ::Data::Asset<AZ::Data::AssetData>>(propertyValue, inputPropertyPair.value, context, result) ||
LoadAny<AZ::Data::Asset<AZ::RPI::ImageAsset>>(propertyValue, inputPropertyPair.value, context, result) ||
LoadAny<AZ::Data::Asset<AZ::RPI::StreamingImageAsset>>(propertyValue, inputPropertyPair.value, context, result))
{
@@ -78,7 +81,10 @@ namespace AZ
}
JsonSerializationResult::Result JsonMaterialAssignmentSerializer::Store(
rapidjson::Value& outputValue, const void* inputValue, const void* defaultValue, [[maybe_unused]] const Uuid& valueTypeId,
rapidjson::Value& outputValue,
const void* inputValue,
const void* defaultValue,
[[maybe_unused]] const Uuid& valueTypeId,
JsonSerializerContext& context)
{
namespace JSR = AZ::JsonSerializationResult;
@@ -138,9 +144,9 @@ namespace AZ
StoreAny<AZ::Color>(propertyValue, outputPropertyValue, context, result) ||
StoreAny<AZStd::string>(propertyValue, outputPropertyValue, context, result) ||
StoreAny<AZ::Data::AssetId>(propertyValue, outputPropertyValue, context, result) ||
StoreAny<AZ::Data::Asset<AZ::Data::AssetData>>(propertyValue, outputPropertyValue, context, result) ||
StoreAny<AZ::Data::Asset<AZ::RPI::ImageAsset>>(propertyValue, outputPropertyValue, context, result) ||
StoreAny<AZ::Data::Asset<AZ::RPI::StreamingImageAsset>>(
propertyValue, outputPropertyValue, context, result))
StoreAny<AZ::Data::Asset<AZ::RPI::StreamingImageAsset>>(propertyValue, outputPropertyValue, context, result))
{
outputPropertyValueContainer.AddMember(
rapidjson::Value::StringRefType(propertyName.GetCStr()), outputPropertyValue,
@@ -164,7 +170,9 @@ namespace AZ
template<typename T>
bool JsonMaterialAssignmentSerializer::LoadAny(
AZStd::any& propertyValue, const rapidjson::Value& inputPropertyValue, AZ::JsonDeserializerContext& context,
AZStd::any& propertyValue,
const rapidjson::Value& inputPropertyValue,
AZ::JsonDeserializerContext& context,
AZ::JsonSerializationResult::ResultCode& result)
{
if (inputPropertyValue.IsObject() && inputPropertyValue.HasMember("Value") && inputPropertyValue.HasMember("$type"))
@@ -187,7 +195,9 @@ namespace AZ
template<typename T>
bool JsonMaterialAssignmentSerializer::StoreAny(
const AZStd::any& propertyValue, rapidjson::Value& outputPropertyValue, AZ::JsonSerializerContext& context,
const AZStd::any& propertyValue,
rapidjson::Value& outputPropertyValue,
AZ::JsonSerializerContext& context,
AZ::JsonSerializationResult::ResultCode& result)
{
if (propertyValue.is<T>())
@@ -199,7 +209,7 @@ namespace AZ
result.Combine(StoreTypeId(typeValue, azrtti_typeid<T>(), context));
outputPropertyValue.AddMember("$type", typeValue, context.GetJsonAllocator());
T value = AZStd::any_cast<T>(propertyValue);
const T& value = AZStd::any_cast<T>(propertyValue);
result.Combine(
ContinueStoringToJsonObjectField(outputPropertyValue, "Value", &value, nullptr, azrtti_typeid<T>(), context));
return true;
Gems/Atom/Feature/Common/Code/Source/Material/MaterialAssignmentSerializer.h
+14 -4
View File
@@ -25,21 +25,31 @@ namespace AZ
AZ_CLASS_ALLOCATOR_DECL;
JsonSerializationResult::Result Load(
void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue,
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,
rapidjson::Value& outputValue,
const void* inputValue,
const void* defaultValue,
const Uuid& valueTypeId,
JsonSerializerContext& context) override;
private:
template<typename T>
bool LoadAny(
AZStd::any& propertyValue, const rapidjson::Value& inputPropertyValue, AZ::JsonDeserializerContext& context,
AZStd::any& propertyValue,
const rapidjson::Value& inputPropertyValue,
AZ::JsonDeserializerContext& context,
AZ::JsonSerializationResult::ResultCode& result);
template<typename T>
bool StoreAny(
const AZStd::any& propertyValue, rapidjson::Value& outputPropertyValue, AZ::JsonSerializerContext& context,
const AZStd::any& propertyValue,
rapidjson::Value& outputPropertyValue,
AZ::JsonSerializerContext& context,
AZ::JsonSerializationResult::ResultCode& result);
};
} // namespace Render
Gems/Atom/Tools/AtomToolsFramework/Code/Source/Util/MaterialPropertyUtil.cpp
+7 -5
View File
@@ -33,11 +33,13 @@ namespace AtomToolsFramework
{
if (value.Is<AZ::Data::Asset<AZ::RPI::ImageAsset>>())
{
const AZ::Data::Asset<AZ::RPI::ImageAsset>& imageAsset = value.GetValue<AZ::Data::Asset<AZ::RPI::ImageAsset>>();
return AZStd::any(AZ::Data::Asset<AZ::RPI::StreamingImageAsset>(
imageAsset.GetId(),
azrtti_typeid<AZ::RPI::StreamingImageAsset>(),
imageAsset.GetHint()));
const auto& imageAsset = value.GetValue<AZ::Data::Asset<AZ::RPI::ImageAsset>>();
return AZStd::any(AZ::Data::Asset<AZ::RPI::StreamingImageAsset>(imageAsset.GetId(), azrtti_typeid<AZ::RPI::StreamingImageAsset>(), imageAsset.GetHint()));
}
else if (value.Is<AZ::Data::Instance<AZ::RPI::Image>>())
{
const auto& image = value.GetValue<AZ::Data::Instance<AZ::RPI::Image>>();
return AZStd::any(AZ::Data::Asset<AZ::RPI::StreamingImageAsset>(image->GetAssetId(), azrtti_typeid<AZ::RPI::StreamingImageAsset>()));
}
return AZ::RPI::MaterialPropertyValue::ToAny(value);
Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.cpp
+33
View File
@@ -104,6 +104,7 @@ namespace AZ
MaterialComponentController::MaterialComponentController(const MaterialComponentConfig& config)
: m_configuration(config)
{
ConvertAssetsForSerialization();
}
void MaterialComponentController::Activate(EntityId entityId)
@@ -135,6 +136,7 @@ namespace AZ
void MaterialComponentController::SetConfiguration(const MaterialComponentConfig& config)
{
m_configuration = config;
ConvertAssetsForSerialization();
}
const MaterialComponentConfig& MaterialComponentController::GetConfiguration() const
@@ -338,6 +340,7 @@ namespace AZ
// before LoadMaterials() is called [LYN-2249]
auto temp = m_configuration.m_materials;
m_configuration.m_materials = materials;
ConvertAssetsForSerialization();
LoadMaterials();
}
@@ -489,6 +492,7 @@ namespace AZ
auto& materialAssignment = m_configuration.m_materials[materialAssignmentId];
const bool wasEmpty = materialAssignment.m_propertyOverrides.empty();
materialAssignment.m_propertyOverrides[AZ::Name(propertyName)] = value;
ConvertAssetsForSerialization();
if (materialAssignment.RequiresLoading())
{
@@ -586,6 +590,7 @@ namespace AZ
auto& materialAssignment = m_configuration.m_materials[materialAssignmentId];
const bool wasEmpty = materialAssignment.m_propertyOverrides.empty();
materialAssignment.m_propertyOverrides = propertyOverrides;
ConvertAssetsForSerialization();
if (materialAssignment.RequiresLoading())
{
@@ -667,5 +672,33 @@ namespace AZ
TickBus::Handler::BusConnect();
}
}
void MaterialComponentController::ConvertAssetsForSerialization()
{
for (auto& materialAssignmentPair : m_configuration.m_materials)
{
MaterialAssignment& materialAssignment = materialAssignmentPair.second;
for (auto& propertyPair : materialAssignment.m_propertyOverrides)
{
auto& value = propertyPair.second;
if (value.is<AZ::Data::Asset<AZ::Data::AssetData>>())
{
value = AZStd::any_cast<AZ::Data::Asset<AZ::Data::AssetData>>(value).GetId();
}
else if (value.is<AZ::Data::Asset<AZ::RPI::StreamingImageAsset>>())
{
value = AZStd::any_cast<AZ::Data::Asset<AZ::RPI::StreamingImageAsset>>(value).GetId();
}
else if (value.is<AZ::Data::Asset<AZ::RPI::ImageAsset>>())
{
value = AZStd::any_cast<AZ::Data::Asset<AZ::RPI::ImageAsset>>(value).GetId();
}
else if (value.is<AZ::Data::Instance<AZ::RPI::Image>>())
{
value = AZStd::any_cast<AZ::Data::Instance<AZ::RPI::Image>>(value)->GetAssetId();
}
}
}
}
} // namespace Render
} // namespace AZ
Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.h
+5
View File
@@ -99,6 +99,11 @@ namespace AZ
//! Queue material instance recreation notifiucations until tick
void QueueMaterialUpdateNotification();
//! Converts property overrides storing image asset references into asset IDs. This addresses a problem where image property
//! overrides are lost during prefab serialization and patching. This suboptimal function will be removed once the underlying
//! problem is resolved.
void ConvertAssetsForSerialization();
EntityId m_entityId;
MaterialComponentConfig m_configuration;
AZStd::unordered_set<MaterialAssignmentId> m_materialsWithDirtyProperties;