From 36abde95a9a312ed56625d422138a86150752ff6 Mon Sep 17 00:00:00 2001 From: Chris Santora Date: Fri, 23 Jul 2021 16:09:24 -0700 Subject: [PATCH 1/4] Added a new registry setting that disables automatic conversion of materials from model files like FBX. By default, processing of model files (like FBX) automatically convert the included materials to Atom materials, using StandardPBR. This adds a job dependency on StandardPBR.materialtype, which propagates to any related azsl files as well. Thus any change to azsl code will cause all model files in the project to rebuild. Some game teams have no interest in using the auto-converted materials; they always use a Material Component to apply material overrides for every mesh. This new setting allows teams to disable material auto-conversion for the entire project, thus removing the job dependency on StandardPBR.materialtype. Instead, every mesh will be assigned the same default material. Any change to azsl code will cause that one default material to rebuild, but this will not trigger any models to rebuild. Details: - Added /O3DE/SceneAPI/MaterialConverter registry settings for configuring the scene material converter. It includes an enable flag, and a default material to use when conversion is disabled. - Added SceneBuilderDependencyRequests::AddFingerprintInfo which allows ScenePI components to modify the scene builder analysis fingerprint. We use this to reprocess scene files when the material converter settings change. - Updated SceneAPI's material asset builder to skip the StandardPBR dependency when material conversion is disabled. - Added some code to MaterialComponentController to handle an edge case that may when disabling material conversion on an existing project, and assigned materials disappear. Testing: - Changing the registery setting does trigger a rebuild of the fbx files. - When material conversion is disabled, changing an azsl file does not cause fbx files to rebuild, but the shader still reloads as expected. - Made a test level using multiple models with multiple meshes, made various adjustments to the material slots for each mesh, and tried switcihng the material conversion registry setting from true to false. (Details below) - TODO: Will merge this change to a customer's fork and test on their existing content. Details about my test level: - Made a new test level AtomTest project - Added two entities, both using multi-mat_mesh-groups_1m_cubes.fbx - Added a material component to both entities - Entity 1 material assignments - Blue_Zaxis: left as-is - Green_Yaxis: exported the material - Red_Xaxis: exported the material, and changed the material instance color to pink - StingrayPBS1: exported the material, scaled the UVs in the exported material source, and changed the material instance color to green. - With_Texture: selected an existing brick material, changed the material instance color to red. - Entity 2 material assignments - Default Material: set to an existing brick material - Blue_Zaxis: manually assigned built-in material that was converted from fbx - Green_Yaxis: manually assigned built-in material that was converted from fbx, and changed the material instance color to orange Signed-off-by: santorac <55155825+santorac@users.noreply.github.com> --- .../SceneCore/SceneBuilderDependencyBus.h | 7 +- .../MaterialConverterSystemComponent.cpp | 56 +++++++- .../MaterialConverterSystemComponent.h | 17 +++ .../RPI.Edit/Material/MaterialConverterBus.h | 13 +- .../RPI.Reflect/Model/ModelMaterialSlot.h | 2 + .../Model/MaterialAssetBuilderComponent.cpp | 129 ++++++++++++++++-- .../Model/MaterialAssetBuilderComponent.h | 8 ++ .../Model/ModelExporterComponent.cpp | 10 +- .../Material/MaterialComponentController.cpp | 93 ++++++++++++- .../Material/MaterialComponentController.h | 9 +- .../SceneBuilder/SceneBuilderComponent.cpp | 7 + .../SceneBuilder/SceneBuilderComponent.h | 1 + .../SceneBuilder/SceneBuilderWorker.cpp | 2 + Registry/sceneassetimporter.setreg | 5 + 14 files changed, 335 insertions(+), 24 deletions(-) diff --git a/Code/Tools/SceneAPI/SceneCore/SceneBuilderDependencyBus.h b/Code/Tools/SceneAPI/SceneCore/SceneBuilderDependencyBus.h index 6500b5ca28..752d4a431f 100644 --- a/Code/Tools/SceneAPI/SceneCore/SceneBuilderDependencyBus.h +++ b/Code/Tools/SceneAPI/SceneCore/SceneBuilderDependencyBus.h @@ -24,7 +24,12 @@ namespace AZ : public AZ::EBusTraits { public: - virtual void ReportJobDependencies(JobDependencyList& jobDependencyList, const char* platformIdentifier) = 0; + //! Builders can implement this function to add job dependencies on other assets that may be used in the scene file conversion process. + virtual void ReportJobDependencies(JobDependencyList& jobDependencyList, const char* platformIdentifier) { AZ_UNUSED(jobDependencyList); AZ_UNUSED(platformIdentifier); } + + //! Builders can implement this function to append to the job analysis fingerprint. This can be used to trigger rebuilds when global configuration changes. + //! See also AssetBuilderDesc::m_analysisFingerprint. + virtual void AddFingerprintInfo(AZStd::set& fingerprintInfo) { AZ_UNUSED(fingerprintInfo); } }; using SceneBuilderDependencyBus = EBus; } // namespace SceneAPI diff --git a/Gems/Atom/Feature/Common/Code/Source/Material/MaterialConverterSystemComponent.cpp b/Gems/Atom/Feature/Common/Code/Source/Material/MaterialConverterSystemComponent.cpp index d173018ef7..d5fdcd9e41 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Material/MaterialConverterSystemComponent.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/Material/MaterialConverterSystemComponent.cpp @@ -12,12 +12,26 @@ #include #include #include +#include #include +#include + namespace AZ { namespace Render { + void MaterialConverterSettings::Reflect(AZ::ReflectContext* context) + { + if (auto serializeContext = azrtti_cast(context); serializeContext) + { + serializeContext->Class() + ->Version(1) + ->Field("Enable", &MaterialConverterSettings::m_enable) + ->Field("DefaultMaterial", &MaterialConverterSettings::m_defaultMaterial); + } + } + void MaterialConverterSystemComponent::Reflect(AZ::ReflectContext* context) { if (auto* serialize = azrtti_cast(context)) @@ -26,10 +40,22 @@ namespace AZ ->Version(3) ->Attribute(Edit::Attributes::SystemComponentTags, AZStd::vector({ AssetBuilderSDK::ComponentTags::AssetBuilder })); } + + MaterialConverterSettings::Reflect(context); + } + + void MaterialConverterSystemComponent::GetProvidedServices(AZ::ComponentDescriptor::DependencyArrayType& services) + { + services.emplace_back(AZ_CRC_CE("FingerprintModification")); } void MaterialConverterSystemComponent::Activate() { + if (auto* settingsRegistry = AZ::SettingsRegistry::Get()) + { + settingsRegistry->GetObject(m_settings, "/O3DE/SceneAPI/MaterialConverter"); + } + RPI::MaterialConverterBus::Handler::BusConnect(); } @@ -37,11 +63,21 @@ namespace AZ { RPI::MaterialConverterBus::Handler::BusDisconnect(); } + + bool MaterialConverterSystemComponent::IsEnabled() const + { + return m_settings.m_enable; + } bool MaterialConverterSystemComponent::ConvertMaterial( const AZ::SceneAPI::DataTypes::IMaterialData& materialData, RPI::MaterialSourceData& sourceData) { using namespace AZ::RPI; + + if (!m_settings.m_enable) + { + return false; + } // The source data for generating material asset sourceData.m_materialType = GetMaterialTypePath(); @@ -142,7 +178,25 @@ namespace AZ const char* MaterialConverterSystemComponent::GetMaterialTypePath() const { - return "Materials/Types/StandardPBR.materialtype"; + if (m_settings.m_enable) + { + return "Materials/Types/StandardPBR.materialtype"; + } + else + { + return nullptr; + } + } + + AZStd::string MaterialConverterSystemComponent::GetDefaultMaterialPath() const + { + if (m_settings.m_defaultMaterial.empty()) + { + AZ_Error("MaterialConverterSystemComponent", m_settings.m_enable, + "Material conversion is disabled but a default material not specified in registry /O3DE/SceneAPI/MaterialConverter/DefaultMaterial"); + } + + return m_settings.m_defaultMaterial; } } } diff --git a/Gems/Atom/Feature/Common/Code/Source/Material/MaterialConverterSystemComponent.h b/Gems/Atom/Feature/Common/Code/Source/Material/MaterialConverterSystemComponent.h index 38d4faedc8..e7e5c63732 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Material/MaterialConverterSystemComponent.h +++ b/Gems/Atom/Feature/Common/Code/Source/Material/MaterialConverterSystemComponent.h @@ -18,6 +18,16 @@ namespace AZ { namespace Render { + struct MaterialConverterSettings + { + AZ_TYPE_INFO(MaterialConverterSettings, "{8D91601D-570A-4557-99C8-631DB4928040}"); + + static void Reflect(AZ::ReflectContext* context); + + bool m_enable = true; + AZStd::string m_defaultMaterial; + }; + //! Atom's implementation of converting SceneAPI data into Atom's default material: StandardPBR class MaterialConverterSystemComponent final : public AZ::Component @@ -27,13 +37,20 @@ namespace AZ AZ_COMPONENT(MaterialConverterSystemComponent, "{C2338D45-6456-4521-B469-B000A13F2493}"); static void Reflect(AZ::ReflectContext* context); + + static void GetProvidedServices(AZ::ComponentDescriptor::DependencyArrayType& services); void Activate() override; void Deactivate() override; // MaterialConverterBus overrides ... + bool IsEnabled() const override; bool ConvertMaterial(const AZ::SceneAPI::DataTypes::IMaterialData& materialData, RPI::MaterialSourceData& out) override; const char* GetMaterialTypePath() const override; + AZStd::string GetDefaultMaterialPath() const override; + + private: + MaterialConverterSettings m_settings; }; } } diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialConverterBus.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialConverterBus.h index 120c1ee28b..e5aa32c175 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialConverterBus.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialConverterBus.h @@ -29,10 +29,19 @@ namespace AZ : public AZ::EBusTraits { public: - //! Returns true if the converion was successful + + virtual bool IsEnabled() const = 0; + + //! Converts data from a IMaterialData object to an Atom MaterialSourceData. + //! Only works when IsEnabled() is true. + //! @return true if the MaterialSourceData output was populated with converted material data. virtual bool ConvertMaterial(const AZ::SceneAPI::DataTypes::IMaterialData& materialData, MaterialSourceData& out) = 0; - //! Returns the path to the .materialtype file that the materials are based on, such as StandardPBR.materialtype, etc. + + //! Returns the path to the .materialtype file that the converted materials are based on, such as StandardPBR.materialtype, etc. Or nullptr when conversion is disabled. virtual const char* GetMaterialTypePath() const = 0; + + //! Returns the path to a .material file to use as the default material when conversion is disabled. + virtual AZStd::string GetDefaultMaterialPath() const = 0; }; using MaterialConverterBus = AZ::EBus; diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Model/ModelMaterialSlot.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Model/ModelMaterialSlot.h index dd46aca6cd..30088d2d52 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Model/ModelMaterialSlot.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Model/ModelMaterialSlot.h @@ -25,6 +25,8 @@ namespace AZ static void Reflect(AZ::ReflectContext* context); + // Note that StableId is uint32_t for legacy reasons: we used to use AssetId::m_subId as the material slot ID. But actually the original MaterialUid + // is 64 bit so we might want to switch this to be uint64_t at some point. using StableId = uint32_t; static const StableId InvalidStableId; diff --git a/Gems/Atom/RPI/Code/Source/RPI.Builders/Model/MaterialAssetBuilderComponent.cpp b/Gems/Atom/RPI/Code/Source/RPI.Builders/Model/MaterialAssetBuilderComponent.cpp index 7187cb391a..3eb60956ad 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Builders/Model/MaterialAssetBuilderComponent.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Builders/Model/MaterialAssetBuilderComponent.cpp @@ -28,6 +28,7 @@ #include #include +#include #include #include @@ -70,28 +71,74 @@ namespace AZ void MaterialAssetDependenciesComponent::ReportJobDependencies(SceneAPI::JobDependencyList& jobDependencyList, const char* platformIdentifier) { - AssetBuilderSDK::SourceFileDependency materialTypeSource; + bool conversionEnabled = false; + RPI::MaterialConverterBus::BroadcastResult(conversionEnabled, &RPI::MaterialConverterBus::Events::IsEnabled); + // Right now, scene file importing only supports a single material type, once that changes, this will have to be re-designed, see ATOM-3554 - RPI::MaterialConverterBus::BroadcastResult(materialTypeSource.m_sourceFileDependencyPath, &RPI::MaterialConverterBus::Events::GetMaterialTypePath); + const char* materialTypePath = nullptr; + RPI::MaterialConverterBus::BroadcastResult(materialTypePath, &RPI::MaterialConverterBus::Events::GetMaterialTypePath); - AssetBuilderSDK::JobDependency jobDependency; - jobDependency.m_jobKey = "Atom Material Builder"; - jobDependency.m_sourceFile = materialTypeSource; - jobDependency.m_platformIdentifier = platformIdentifier; - jobDependency.m_type = AssetBuilderSDK::JobDependencyType::Order; - - if (!materialTypeSource.m_sourceFileDependencyPath.empty()) + if (conversionEnabled && materialTypePath) { + AssetBuilderSDK::SourceFileDependency materialTypeSource; + materialTypeSource.m_sourceFileDependencyPath = materialTypePath; + + AssetBuilderSDK::JobDependency jobDependency; + jobDependency.m_jobKey = "Atom Material Builder"; + jobDependency.m_sourceFile = materialTypeSource; + jobDependency.m_platformIdentifier = platformIdentifier; + jobDependency.m_type = AssetBuilderSDK::JobDependencyType::Order; + jobDependencyList.push_back(jobDependency); } } + + void MaterialAssetDependenciesComponent::AddFingerprintInfo(AZStd::set& fingerprintInfo) + { + // This will cause scene files to be reprocessed whenever the global MaterialConverter settings change. + + bool conversionEnabled = false; + RPI::MaterialConverterBus::BroadcastResult(conversionEnabled, &RPI::MaterialConverterBus::Events::IsEnabled); + fingerprintInfo.insert(AZStd::string::format("[MaterialConverter enabled=%d]", conversionEnabled)); + + if (!conversionEnabled) + { + AZStd::string defaultMaterialPath; + RPI::MaterialConverterBus::BroadcastResult(defaultMaterialPath, &RPI::MaterialConverterBus::Events::GetDefaultMaterialPath); + fingerprintInfo.insert(AZStd::string::format("[MaterialConverter defaultMaterial=%s]", defaultMaterialPath.c_str())); + } + } void MaterialAssetBuilderComponent::Reflect(ReflectContext* context) { if (auto* serialize = azrtti_cast(context)) { serialize->Class() - ->Version(16); // Optional material conversion + ->Version(16); // Optional material conversion + } + } + + Data::Asset MaterialAssetBuilderComponent::GetDefaultMaterialAsset() const + { + AZStd::string defaultMaterialPath; + RPI::MaterialConverterBus::BroadcastResult(defaultMaterialPath, &RPI::MaterialConverterBus::Events::GetDefaultMaterialPath); + + if (defaultMaterialPath.empty()) + { + return {}; + } + else + { + auto defaultMaterialAssetId = RPI::AssetUtils::MakeAssetId(defaultMaterialPath, 0); + if (!defaultMaterialAssetId.IsSuccess()) + { + AZ_Error("MaterialAssetBuilderComponent", false, "Could not find asset '%s'", defaultMaterialPath.c_str()); + return {}; + } + else + { + return Data::AssetManager::Instance().CreateAsset(defaultMaterialAssetId.GetValue(), Data::AssetLoadBehaviorNamespace::PreLoad); + } } } @@ -119,8 +166,8 @@ namespace AZ BindToCall(&MaterialAssetBuilderComponent::BuildMaterials); } - - SceneAPI::Events::ProcessingResult MaterialAssetBuilderComponent::BuildMaterials(MaterialAssetBuilderContext& context) const + + SceneAPI::Events::ProcessingResult MaterialAssetBuilderComponent::ConvertMaterials(MaterialAssetBuilderContext& context) const { const auto& scene = context.m_scene; const Uuid sourceSceneUuid = scene.GetSourceGuid(); @@ -193,6 +240,64 @@ namespace AZ return SceneAPI::Events::ProcessingResult::Success; } + + SceneAPI::Events::ProcessingResult MaterialAssetBuilderComponent::AssignDefaultMaterials(MaterialAssetBuilderContext& context) const + { + Data::Asset defaultMaterialAsset = GetDefaultMaterialAsset(); + + if (!defaultMaterialAsset.GetId().IsValid()) + { + AZ_Warning("MaterialAssetBuilderComponent", false, "Material conversion is disabled but no default material was provided. The model will likely be invisible by default."); + // Return success because it's just a warning. + return SceneAPI::Events::ProcessingResult::Success; + } + + const auto& scene = context.m_scene; + const Uuid sourceSceneUuid = scene.GetSourceGuid(); + const auto& sceneGraph = scene.GetGraph(); + + auto names = sceneGraph.GetNameStorage(); + auto content = sceneGraph.GetContentStorage(); + auto pairView = SceneAPI::Containers::Views::MakePairView(names, content); + + auto view = SceneAPI::Containers::Views::MakeSceneGraphDownwardsView< + SceneAPI::Containers::Views::BreadthFirst>( + sceneGraph, sceneGraph.GetRoot(), pairView.cbegin(), true); + + for (const auto& viewIt : view) + { + if (viewIt.second == nullptr) + { + continue; + } + + if (azrtti_istypeof(viewIt.second.get())) + { + auto materialData = AZStd::static_pointer_cast(viewIt.second); + uint64_t materialUid = materialData->GetUniqueId(); + + context.m_outputMaterialsByUid[materialUid] = { defaultMaterialAsset, materialData->GetMaterialName() }; + } + } + + return SceneAPI::Events::ProcessingResult::Success; + } + + SceneAPI::Events::ProcessingResult MaterialAssetBuilderComponent::BuildMaterials(MaterialAssetBuilderContext& context) const + { + bool conversionEnabled = false; + RPI::MaterialConverterBus::BroadcastResult(conversionEnabled, &RPI::MaterialConverterBus::Events::IsEnabled); + + if (conversionEnabled) + { + return ConvertMaterials(context); + } + else + { + return AssignDefaultMaterials(context); + } + + } } // namespace RPI } // namespace AZ diff --git a/Gems/Atom/RPI/Code/Source/RPI.Builders/Model/MaterialAssetBuilderComponent.h b/Gems/Atom/RPI/Code/Source/RPI.Builders/Model/MaterialAssetBuilderComponent.h index f02034c35d..f89ae1cde9 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Builders/Model/MaterialAssetBuilderComponent.h +++ b/Gems/Atom/RPI/Code/Source/RPI.Builders/Model/MaterialAssetBuilderComponent.h @@ -39,6 +39,13 @@ namespace AZ // Required for ExportingComponent static void Reflect(AZ::ReflectContext* context); + + private: + + SceneAPI::Events::ProcessingResult ConvertMaterials(MaterialAssetBuilderContext& context) const; + SceneAPI::Events::ProcessingResult AssignDefaultMaterials(MaterialAssetBuilderContext& context) const; + + Data::Asset GetDefaultMaterialAsset() const; }; /** @@ -65,6 +72,7 @@ namespace AZ // SceneAPI::SceneBuilderDependencyBus::Handler overrides... void ReportJobDependencies(SceneAPI::JobDependencyList& jobDependencyList, const char* platformIdentifier) override; + void AddFingerprintInfo(AZStd::set& fingerprintInfo) override; }; } // namespace RPI } // namespace AZ diff --git a/Gems/Atom/RPI/Code/Source/RPI.Builders/Model/ModelExporterComponent.cpp b/Gems/Atom/RPI/Code/Source/RPI.Builders/Model/ModelExporterComponent.cpp index ea39ea9031..f7e672ab32 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Builders/Model/ModelExporterComponent.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Builders/Model/ModelExporterComponent.cpp @@ -78,9 +78,17 @@ namespace AZ //Export MaterialAssets for (auto& materialPair : materialsByUid) { + const Data::Asset& asset = materialPair.second.m_asset; + + // MaterialAssetBuilderContext could attach an independent material asset rather than + // generate one using the scene data, so we must skip the export step in that case. + if (asset.GetId().m_guid != exportEventContext.GetScene().GetSourceGuid()) + { + continue; + } + uint64_t materialUid = materialPair.first; const AZStd::string& sceneName = exportEventContext.GetScene().GetName(); - const Data::Asset& asset = materialPair.second.m_asset; // escape the material name acceptable for a filename AZStd::string materialName = materialPair.second.m_name; diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.cpp index e39f5cfad5..5d4e7883b0 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.cpp @@ -8,8 +8,10 @@ #include #include +#include #include #include +#include namespace AZ { @@ -85,6 +87,7 @@ namespace AZ void MaterialComponentController::Deactivate() { MaterialComponentRequestBus::Handler::BusDisconnect(); + MeshComponentNotificationBus::Handler::BusDisconnect(); TickBus::Handler::BusDisconnect(); ReleaseMaterials(); @@ -111,6 +114,55 @@ namespace AZ { InitializeMaterialInstance(asset); } + + void MaterialComponentController::OnModelReady(const Data::Asset&, const Data::Instance&) + { + MeshComponentNotificationBus::Handler::BusDisconnect(); + + // If there is a circumstance where the saved material assignments are empty, fill them in with the default material. + // (This could happen as a result of LoadMaterials() clearing the asset reference to deal with an edge case) + + // Now that a model asset is ready, see if there are any empty assignments that need to be filled... + RPI::ModelMaterialSlotMap modelMaterialSlots; + MaterialReceiverRequestBus::EventResult(modelMaterialSlots, m_entityId, &MaterialReceiverRequestBus::Events::GetModelMaterialSlots); + + AZStd::vector> newMaterialAssets; + newMaterialAssets.reserve(m_configuration.m_materials.size()); + + // First we fill the empty slots but don't connect to AssetBus yet. If the same material asset appears multiple times, + // AssetBus will call OnAssetReady only the *first* time we connect for that asset. The full list of m_configuration.m_materials + // needs to be updated before that happens. + for (auto& materialPair : m_configuration.m_materials) + { + auto& materialAsset = materialPair.second.m_materialAsset; + + if (!materialAsset.GetId().IsValid()) + { + auto slotIter = modelMaterialSlots.find(materialPair.first.m_materialSlotStableId); + if (slotIter != modelMaterialSlots.end()) + { + materialAsset = slotIter->second.m_defaultMaterialAsset; + newMaterialAssets.push_back(materialAsset); + } + else + { + AZ_Error("MaterialComponentController", false, "Could not find material slot %d", materialPair.first.m_materialSlotStableId); + } + } + } + + // Now that the configuration is updated with all the default material assets, we can load and connect them. + // If there are duplicates in this list, the redundant calls will be ignored. + for (auto& materialAsset : newMaterialAssets) + { + if (!materialAsset.IsReady()) + { + materialAsset.QueueLoad(); + } + + Data::AssetBus::MultiHandler::BusConnect(materialAsset.GetId()); + } + } void MaterialComponentController::OnTick([[maybe_unused]] float deltaTime, [[maybe_unused]] AZ::ScriptTimePoint time) { @@ -186,11 +238,42 @@ namespace AZ for (auto& materialPair : m_configuration.m_materials) { auto& materialAsset = materialPair.second.m_materialAsset; - if (materialAsset.GetId().IsValid() && !Data::AssetBus::MultiHandler::BusIsConnectedId(materialAsset.GetId())) + + // This is a special case where a material was auto-generated from the model file, connected to a Material Component by the user, + // and then later a setting was changed to NOT auto-generate the model materials anymore. We need to switch to the new default + // material rather than trying to use the old default material which no longer exists. If that's the case, we reset the asset + // and OnModelReady will fill in the appropriate default material asset later. { - anyQueued = true; - materialAsset.QueueLoad(); - Data::AssetBus::MultiHandler::BusConnect(materialAsset.GetId()); + Data::AssetId modelAssetId; + MeshComponentRequestBus::EventResult(modelAssetId, m_entityId, &MeshComponentRequestBus::Events::GetModelAssetId); + bool materialWasGeneratedFromModel = (modelAssetId.m_guid == materialAsset.GetId().m_guid); + + Data::AssetInfo assetInfo; + Data::AssetCatalogRequestBus::BroadcastResult(assetInfo, &Data::AssetCatalogRequestBus::Events::GetAssetInfoById, materialAsset.GetId()); + bool materialAssetExists = assetInfo.m_assetId.IsValid(); + + if (materialWasGeneratedFromModel && !materialAssetExists) + { + AZ_Warning("MaterialComponentController", false, "The default material assignment for this slot has changed and will be replaced (was '%s').", + materialAsset.ToString().c_str()); + materialAsset.Reset(); + } + } + + if (materialAsset.GetId().IsValid()) + { + if (!Data::AssetBus::MultiHandler::BusIsConnectedId(materialAsset.GetId())) + { + anyQueued = true; + materialAsset.QueueLoad(); + Data::AssetBus::MultiHandler::BusConnect(materialAsset.GetId()); + } + } + else + { + // Since a material asset wasn't found, we'll need to supply a default material. But the default materials + // won't be known until after the mesh component has loaded the model data. + MeshComponentNotificationBus::Handler::BusConnect(m_entityId); } } @@ -199,7 +282,7 @@ namespace AZ ReleaseMaterials(); } } - + void MaterialComponentController::InitializeMaterialInstance(const Data::Asset& asset) { bool allReady = true; diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.h b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.h index de7f991d60..b2177d1191 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.h +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.h @@ -13,6 +13,7 @@ #include #include #include +#include namespace AZ { @@ -22,6 +23,7 @@ namespace AZ //! to provide material overrides on a per-entity basis. class MaterialComponentController final : MaterialComponentRequestBus::Handler + , MeshComponentNotificationBus::Handler , Data::AssetBus::MultiHandler , TickBus::Handler { @@ -68,13 +70,16 @@ namespace AZ AZ_DISABLE_COPY(MaterialComponentController); - //! Data::AssetBus interface + //! Data::AssetBus overrides... void OnAssetReady(Data::Asset asset) override; void OnAssetReloaded(Data::Asset asset) override; - //! AZ::TickBus interface implementation + // AZ::TickBus overrides... void OnTick(float deltaTime, AZ::ScriptTimePoint time) override; + // MeshComponentNotificationBus overrides... + void OnModelReady(const Data::Asset& modelAsset, const Data::Instance& model) override; + void LoadMaterials(); void InitializeMaterialInstance(const Data::Asset& asset); void ReleaseMaterials(); diff --git a/Gems/SceneProcessing/Code/Source/SceneBuilder/SceneBuilderComponent.cpp b/Gems/SceneProcessing/Code/Source/SceneBuilder/SceneBuilderComponent.cpp index 524b79b012..430733f3a1 100644 --- a/Gems/SceneProcessing/Code/Source/SceneBuilder/SceneBuilderComponent.cpp +++ b/Gems/SceneProcessing/Code/Source/SceneBuilder/SceneBuilderComponent.cpp @@ -73,6 +73,13 @@ namespace SceneBuilder required.emplace_back(AZ_CRC_CE("AssetImportRequestHandler")); } + void BuilderPluginComponent::GetDependentServices(AZ::ComponentDescriptor::DependencyArrayType& services) + { + // Any components that can modify the analysis fingerprint via SceneBuilderDependencyRequests::AddFingerprintInfo must be activated first, + // so they contribute to the fingerprint calculated in BuilderPluginComponent::Activate(). + services.emplace_back(AZ_CRC_CE("FingerprintModification")); + } + void BuilderPluginComponent::Reflect(AZ::ReflectContext* context) { AZ::SerializeContext* serializeContext = azrtti_cast(context); diff --git a/Gems/SceneProcessing/Code/Source/SceneBuilder/SceneBuilderComponent.h b/Gems/SceneProcessing/Code/Source/SceneBuilder/SceneBuilderComponent.h index b4eee64bc6..0bfe118a4c 100644 --- a/Gems/SceneProcessing/Code/Source/SceneBuilder/SceneBuilderComponent.h +++ b/Gems/SceneProcessing/Code/Source/SceneBuilder/SceneBuilderComponent.h @@ -29,6 +29,7 @@ namespace SceneBuilder void Deactivate() override; static void GetRequiredServices(AZ::ComponentDescriptor::DependencyArrayType& required); + static void GetDependentServices(AZ::ComponentDescriptor::DependencyArrayType& services); private: SceneBuilderWorker m_sceneBuilder; diff --git a/Gems/SceneProcessing/Code/Source/SceneBuilder/SceneBuilderWorker.cpp b/Gems/SceneProcessing/Code/Source/SceneBuilder/SceneBuilderWorker.cpp index 18ee0285b8..2af4613e9e 100644 --- a/Gems/SceneProcessing/Code/Source/SceneBuilder/SceneBuilderWorker.cpp +++ b/Gems/SceneProcessing/Code/Source/SceneBuilder/SceneBuilderWorker.cpp @@ -69,6 +69,8 @@ namespace SceneBuilder context->EnumerateDerived(callback, azrtti_typeid(), azrtti_typeid()); context->EnumerateDerived(callback, azrtti_typeid(), azrtti_typeid()); } + + AZ::SceneAPI::SceneBuilderDependencyBus::Broadcast(&AZ::SceneAPI::SceneBuilderDependencyRequests::AddFingerprintInfo, fragments); for (const AZStd::string& element : fragments) { diff --git a/Registry/sceneassetimporter.setreg b/Registry/sceneassetimporter.setreg index bd7c4d0705..6fef3a40dc 100644 --- a/Registry/sceneassetimporter.setreg +++ b/Registry/sceneassetimporter.setreg @@ -10,6 +10,11 @@ ".fbx", ".stl" ] + }, + "MaterialConverter": + { + "Enable": true, + "DefaultMaterial": "Materials/Presets/PBR/default_grid.material" } } } From 6b23e07af4781de2393367e4c269c53f37ed4889 Mon Sep 17 00:00:00 2001 From: santorac <55155825+santorac@users.noreply.github.com> Date: Fri, 20 Aug 2021 17:22:11 -0700 Subject: [PATCH 2/4] Changed MaterialConverterBus GetMaterialTypePath() to return a string instead of char* Minor code cleanup. Signed-off-by: santorac <55155825+santorac@users.noreply.github.com> --- .../MaterialConverterSystemComponent.cpp | 19 ++++++------------- .../MaterialConverterSystemComponent.h | 2 +- .../RPI.Edit/Material/MaterialConverterBus.h | 4 ++-- .../Model/MaterialAssetBuilderComponent.cpp | 4 ++-- 4 files changed, 11 insertions(+), 18 deletions(-) diff --git a/Gems/Atom/Feature/Common/Code/Source/Material/MaterialConverterSystemComponent.cpp b/Gems/Atom/Feature/Common/Code/Source/Material/MaterialConverterSystemComponent.cpp index d5fdcd9e41..e26a0fb274 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Material/MaterialConverterSystemComponent.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/Material/MaterialConverterSystemComponent.cpp @@ -23,12 +23,12 @@ namespace AZ { void MaterialConverterSettings::Reflect(AZ::ReflectContext* context) { - if (auto serializeContext = azrtti_cast(context); serializeContext) + if (auto serializeContext = azrtti_cast(context)) { serializeContext->Class() - ->Version(1) - ->Field("Enable", &MaterialConverterSettings::m_enable) - ->Field("DefaultMaterial", &MaterialConverterSettings::m_defaultMaterial); + ->Version(1) + ->Field("Enable", &MaterialConverterSettings::m_enable) + ->Field("DefaultMaterial", &MaterialConverterSettings::m_defaultMaterial); } } @@ -176,16 +176,9 @@ namespace AZ return true; } - const char* MaterialConverterSystemComponent::GetMaterialTypePath() const + AZStd::string MaterialConverterSystemComponent::GetMaterialTypePath() const { - if (m_settings.m_enable) - { - return "Materials/Types/StandardPBR.materialtype"; - } - else - { - return nullptr; - } + return "Materials/Types/StandardPBR.materialtype"; } AZStd::string MaterialConverterSystemComponent::GetDefaultMaterialPath() const diff --git a/Gems/Atom/Feature/Common/Code/Source/Material/MaterialConverterSystemComponent.h b/Gems/Atom/Feature/Common/Code/Source/Material/MaterialConverterSystemComponent.h index e7e5c63732..7d95024759 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Material/MaterialConverterSystemComponent.h +++ b/Gems/Atom/Feature/Common/Code/Source/Material/MaterialConverterSystemComponent.h @@ -46,7 +46,7 @@ namespace AZ // MaterialConverterBus overrides ... bool IsEnabled() const override; bool ConvertMaterial(const AZ::SceneAPI::DataTypes::IMaterialData& materialData, RPI::MaterialSourceData& out) override; - const char* GetMaterialTypePath() const override; + AZStd::string GetMaterialTypePath() const override; AZStd::string GetDefaultMaterialPath() const override; private: diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialConverterBus.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialConverterBus.h index e5aa32c175..8052fc4feb 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialConverterBus.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Material/MaterialConverterBus.h @@ -37,8 +37,8 @@ namespace AZ //! @return true if the MaterialSourceData output was populated with converted material data. virtual bool ConvertMaterial(const AZ::SceneAPI::DataTypes::IMaterialData& materialData, MaterialSourceData& out) = 0; - //! Returns the path to the .materialtype file that the converted materials are based on, such as StandardPBR.materialtype, etc. Or nullptr when conversion is disabled. - virtual const char* GetMaterialTypePath() const = 0; + //! Returns the path to the .materialtype file that the converted materials are based on, such as StandardPBR.materialtype, etc. + virtual AZStd::string GetMaterialTypePath() const = 0; //! Returns the path to a .material file to use as the default material when conversion is disabled. virtual AZStd::string GetDefaultMaterialPath() const = 0; diff --git a/Gems/Atom/RPI/Code/Source/RPI.Builders/Model/MaterialAssetBuilderComponent.cpp b/Gems/Atom/RPI/Code/Source/RPI.Builders/Model/MaterialAssetBuilderComponent.cpp index 3eb60956ad..4a4c2156ce 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Builders/Model/MaterialAssetBuilderComponent.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Builders/Model/MaterialAssetBuilderComponent.cpp @@ -75,10 +75,10 @@ namespace AZ RPI::MaterialConverterBus::BroadcastResult(conversionEnabled, &RPI::MaterialConverterBus::Events::IsEnabled); // Right now, scene file importing only supports a single material type, once that changes, this will have to be re-designed, see ATOM-3554 - const char* materialTypePath = nullptr; + AZStd::string materialTypePath; RPI::MaterialConverterBus::BroadcastResult(materialTypePath, &RPI::MaterialConverterBus::Events::GetMaterialTypePath); - if (conversionEnabled && materialTypePath) + if (conversionEnabled && !materialTypePath.empty()) { AssetBuilderSDK::SourceFileDependency materialTypeSource; materialTypeSource.m_sourceFileDependencyPath = materialTypePath; From de932c62b318ab0c02edb379cf5eb2ed617af209 Mon Sep 17 00:00:00 2001 From: santorac <55155825+santorac@users.noreply.github.com> Date: Mon, 23 Aug 2021 12:50:11 -0700 Subject: [PATCH 3/4] Reverted changes in MaterialComponentController for an obsucre edge case. We'll handle the case of missing material assets more generally as a separate task. Signed-off-by: santorac <55155825+santorac@users.noreply.github.com> --- .../Material/MaterialComponentController.cpp | 88 +------------------ .../Material/MaterialComponentController.h | 5 -- 2 files changed, 4 insertions(+), 89 deletions(-) diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.cpp index 5d4e7883b0..8165eefb07 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.cpp @@ -87,7 +87,6 @@ namespace AZ void MaterialComponentController::Deactivate() { MaterialComponentRequestBus::Handler::BusDisconnect(); - MeshComponentNotificationBus::Handler::BusDisconnect(); TickBus::Handler::BusDisconnect(); ReleaseMaterials(); @@ -115,55 +114,6 @@ namespace AZ InitializeMaterialInstance(asset); } - void MaterialComponentController::OnModelReady(const Data::Asset&, const Data::Instance&) - { - MeshComponentNotificationBus::Handler::BusDisconnect(); - - // If there is a circumstance where the saved material assignments are empty, fill them in with the default material. - // (This could happen as a result of LoadMaterials() clearing the asset reference to deal with an edge case) - - // Now that a model asset is ready, see if there are any empty assignments that need to be filled... - RPI::ModelMaterialSlotMap modelMaterialSlots; - MaterialReceiverRequestBus::EventResult(modelMaterialSlots, m_entityId, &MaterialReceiverRequestBus::Events::GetModelMaterialSlots); - - AZStd::vector> newMaterialAssets; - newMaterialAssets.reserve(m_configuration.m_materials.size()); - - // First we fill the empty slots but don't connect to AssetBus yet. If the same material asset appears multiple times, - // AssetBus will call OnAssetReady only the *first* time we connect for that asset. The full list of m_configuration.m_materials - // needs to be updated before that happens. - for (auto& materialPair : m_configuration.m_materials) - { - auto& materialAsset = materialPair.second.m_materialAsset; - - if (!materialAsset.GetId().IsValid()) - { - auto slotIter = modelMaterialSlots.find(materialPair.first.m_materialSlotStableId); - if (slotIter != modelMaterialSlots.end()) - { - materialAsset = slotIter->second.m_defaultMaterialAsset; - newMaterialAssets.push_back(materialAsset); - } - else - { - AZ_Error("MaterialComponentController", false, "Could not find material slot %d", materialPair.first.m_materialSlotStableId); - } - } - } - - // Now that the configuration is updated with all the default material assets, we can load and connect them. - // If there are duplicates in this list, the redundant calls will be ignored. - for (auto& materialAsset : newMaterialAssets) - { - if (!materialAsset.IsReady()) - { - materialAsset.QueueLoad(); - } - - Data::AssetBus::MultiHandler::BusConnect(materialAsset.GetId()); - } - } - void MaterialComponentController::OnTick([[maybe_unused]] float deltaTime, [[maybe_unused]] AZ::ScriptTimePoint time) { AZStd::unordered_set propertyOverrides; @@ -239,41 +189,11 @@ namespace AZ { auto& materialAsset = materialPair.second.m_materialAsset; - // This is a special case where a material was auto-generated from the model file, connected to a Material Component by the user, - // and then later a setting was changed to NOT auto-generate the model materials anymore. We need to switch to the new default - // material rather than trying to use the old default material which no longer exists. If that's the case, we reset the asset - // and OnModelReady will fill in the appropriate default material asset later. + if (materialAsset.GetId().IsValid() && !Data::AssetBus::MultiHandler::BusIsConnectedId(materialAsset.GetId())) { - Data::AssetId modelAssetId; - MeshComponentRequestBus::EventResult(modelAssetId, m_entityId, &MeshComponentRequestBus::Events::GetModelAssetId); - bool materialWasGeneratedFromModel = (modelAssetId.m_guid == materialAsset.GetId().m_guid); - - Data::AssetInfo assetInfo; - Data::AssetCatalogRequestBus::BroadcastResult(assetInfo, &Data::AssetCatalogRequestBus::Events::GetAssetInfoById, materialAsset.GetId()); - bool materialAssetExists = assetInfo.m_assetId.IsValid(); - - if (materialWasGeneratedFromModel && !materialAssetExists) - { - AZ_Warning("MaterialComponentController", false, "The default material assignment for this slot has changed and will be replaced (was '%s').", - materialAsset.ToString().c_str()); - materialAsset.Reset(); - } - } - - if (materialAsset.GetId().IsValid()) - { - if (!Data::AssetBus::MultiHandler::BusIsConnectedId(materialAsset.GetId())) - { - anyQueued = true; - materialAsset.QueueLoad(); - Data::AssetBus::MultiHandler::BusConnect(materialAsset.GetId()); - } - } - else - { - // Since a material asset wasn't found, we'll need to supply a default material. But the default materials - // won't be known until after the mesh component has loaded the model data. - MeshComponentNotificationBus::Handler::BusConnect(m_entityId); + anyQueued = true; + materialAsset.QueueLoad(); + Data::AssetBus::MultiHandler::BusConnect(materialAsset.GetId()); } } diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.h b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.h index b2177d1191..d8a32c7b78 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.h +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.h @@ -13,7 +13,6 @@ #include #include #include -#include namespace AZ { @@ -23,7 +22,6 @@ namespace AZ //! to provide material overrides on a per-entity basis. class MaterialComponentController final : MaterialComponentRequestBus::Handler - , MeshComponentNotificationBus::Handler , Data::AssetBus::MultiHandler , TickBus::Handler { @@ -77,9 +75,6 @@ namespace AZ // AZ::TickBus overrides... void OnTick(float deltaTime, AZ::ScriptTimePoint time) override; - // MeshComponentNotificationBus overrides... - void OnModelReady(const Data::Asset& modelAsset, const Data::Instance& model) override; - void LoadMaterials(); void InitializeMaterialInstance(const Data::Asset& asset); void ReleaseMaterials(); From b6dfb86007481ca705d37cd5b1efc24a1459adfa Mon Sep 17 00:00:00 2001 From: santorac <55155825+santorac@users.noreply.github.com> Date: Mon, 23 Aug 2021 12:53:33 -0700 Subject: [PATCH 4/4] Removed unnecessary #include Signed-off-by: santorac <55155825+santorac@users.noreply.github.com> --- .../Code/Source/Material/MaterialComponentController.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.cpp index 8165eefb07..536b5a3710 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/Material/MaterialComponentController.cpp @@ -11,7 +11,6 @@ #include #include #include -#include namespace AZ {