From 3fe5901a77519ceb135243f58d70144096ba0848 Mon Sep 17 00:00:00 2001 From: mnaumov Date: Wed, 11 Aug 2021 23:25:41 -0700 Subject: [PATCH 01/37] Fixing a crash when unparenting prefab instance in a new level Signed-off-by: mnaumov --- .../PrefabEditorEntityOwnershipService.cpp | 2 +- .../Prefab/PrefabPublicHandler.cpp | 36 +++++++++++++------ .../Prefab/PrefabSystemComponent.cpp | 18 ++++++++++ .../Prefab/PrefabSystemComponent.h | 2 ++ .../Prefab/PrefabSystemComponentInterface.h | 2 ++ 5 files changed, 48 insertions(+), 12 deletions(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp index b5cf5fb878..127f04424b 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp @@ -202,7 +202,7 @@ namespace AzToolsFramework m_rootInstance->SetTemplateId(templateId); m_rootInstance->SetTemplateSourcePath(m_loaderInterface->GenerateRelativePath(filename)); m_rootInstance->SetContainerEntityName("Level"); - m_prefabSystemComponent->PropagateTemplateChanges(templateId); + m_prefabSystemComponent->PropagateTemplateChangesDown(templateId); return true; } diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp index 69efe33d72..85fe30c252 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp @@ -482,22 +482,36 @@ namespace AzToolsFramework AZStd::unique_ptr& sourceInstance, TemplateId targetTemplateId, UndoSystem::URSequencePoint* undoBatch) { LinkReference nestedInstanceLink = m_prefabSystemComponentInterface->FindLink(sourceInstance->GetLinkId()); - AZ_Assert( - nestedInstanceLink.has_value(), - "A valid link was not found for one of the instances provided as input for the CreatePrefab operation."); + if (!nestedInstanceLink) + { + AZ_Assert( + false, + "A valid link was not found for one of the instances provided as input for the CreatePrefab operation."); + return; + } PrefabDomReference nestedInstanceLinkDom = nestedInstanceLink->get().GetLinkDom(); - AZ_Assert( - nestedInstanceLinkDom.has_value(), - "A valid DOM was not found for the link corresponding to one of the instances provided as input for the " - "CreatePrefab operation."); + + if (!nestedInstanceLinkDom) + { + AZ_Assert( + false, + "A valid DOM was not found for the link corresponding to one of the instances provided as input for the " + "CreatePrefab operation."); + return; + } PrefabDomValueReference nestedInstanceLinkPatches = PrefabDomUtils::FindPrefabDomValue(nestedInstanceLinkDom->get(), PrefabDomUtils::PatchesName); - AZ_Assert( - nestedInstanceLinkPatches.has_value(), - "A valid DOM for patches was not found for the link corresponding to one of the instances provided as input for the " - "CreatePrefab operation."); + + if (!nestedInstanceLinkPatches) + { + AZ_Assert( + false, + "A valid DOM for patches was not found for the link corresponding to one of the instances provided as input for the " + "CreatePrefab operation."); + return; + } PrefabDom patchesCopyForUndoSupport; patchesCopyForUndoSupport.CopyFrom(nestedInstanceLinkPatches->get(), patchesCopyForUndoSupport.GetAllocator()); diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.cpp index bfdb6b79f2..b75f50198c 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.cpp @@ -153,6 +153,24 @@ namespace AzToolsFramework } } + void PrefabSystemComponent::PropagateTemplateChangesDown(TemplateId templateId, InstanceOptionalReference instanceToExclude) + { + PropagateTemplateChanges(templateId, instanceToExclude); + + auto templateIterator = m_templateIdMap.find(templateId); + if (templateIterator != m_templateIdMap.end()) + { + for (LinkId linkId : templateIterator->second.GetLinks()) + { + auto linkIterator = m_linkIdMap.find(linkId); + if (linkIterator != m_linkIdMap.end()) + { + PropagateTemplateChangesDown(linkIterator->second.GetSourceTemplateId(), instanceToExclude); + } + } + } + } + void PrefabSystemComponent::UpdatePrefabTemplate(TemplateId templateId, const PrefabDom& updatedDom) { auto templateToUpdate = FindTemplate(templateId); diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.h index 04457b5a97..ccc17af133 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.h @@ -213,6 +213,8 @@ namespace AzToolsFramework void PropagateTemplateChanges(TemplateId templateId, InstanceOptionalReference instanceToExclude = AZStd::nullopt) override; + void PropagateTemplateChangesDown(TemplateId templateIdd, InstanceOptionalReference instanceToExclude = AZStd::nullopt) override; + /** * Updates all Instances owned by a Template. * diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponentInterface.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponentInterface.h index 0c758a21af..9792ec0fab 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponentInterface.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponentInterface.h @@ -53,6 +53,8 @@ namespace AzToolsFramework virtual PrefabDom& FindTemplateDom(TemplateId templateId) = 0; virtual void UpdatePrefabTemplate(TemplateId templateId, const PrefabDom& updatedDom) = 0; virtual void PropagateTemplateChanges(TemplateId templateId, InstanceOptionalReference instanceToExclude = AZStd::nullopt) = 0; + //! Propagates template changes recursively down to its dependents + virtual void PropagateTemplateChangesDown(TemplateId templateIdd, InstanceOptionalReference instanceToExclude = AZStd::nullopt) = 0; virtual AZStd::unique_ptr InstantiatePrefab(AZ::IO::PathView filePath) = 0; virtual AZStd::unique_ptr InstantiatePrefab(const TemplateId& templateId) = 0; From 8f21563ba95d042fc8263eacdd9821c4a93ab4f9 Mon Sep 17 00:00:00 2001 From: mnaumov Date: Thu, 12 Aug 2021 14:37:01 -0700 Subject: [PATCH 02/37] Undoing old changes and storing linkId to prefabDom Signed-off-by: mnaumov --- .../PrefabEditorEntityOwnershipService.cpp | 2 +- .../Prefab/Instance/InstanceSerializer.cpp | 7 +++++++ .../Prefab/PrefabSystemComponent.cpp | 18 ------------------ .../Prefab/PrefabSystemComponent.h | 2 -- .../Prefab/PrefabSystemComponentInterface.h | 2 -- 5 files changed, 8 insertions(+), 23 deletions(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp index 127f04424b..b5cf5fb878 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp @@ -202,7 +202,7 @@ namespace AzToolsFramework m_rootInstance->SetTemplateId(templateId); m_rootInstance->SetTemplateSourcePath(m_loaderInterface->GenerateRelativePath(filename)); m_rootInstance->SetContainerEntityName("Level"); - m_prefabSystemComponent->PropagateTemplateChangesDown(templateId); + m_prefabSystemComponent->PropagateTemplateChanges(templateId); return true; } diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceSerializer.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceSerializer.cpp index b5a354ee74..89e0136b12 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceSerializer.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceSerializer.cpp @@ -81,6 +81,13 @@ namespace AzToolsFramework result.Combine(resultInstances); } + { + AZ::ScopedContextPath subPathSource(context, "m_linkId"); + + result = ContinueStoringToJsonObjectField( + outputValue, "LinkId", &(instance->m_linkId), &InvalidLinkId, azrtti_typeid(), context); + } + return context.Report(result, result.GetProcessing() == JSR::Processing::Completed ? "Successfully stored Instance information for Prefab." : "Failed to store Instance information for Prefab."); diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.cpp index b75f50198c..bfdb6b79f2 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.cpp @@ -153,24 +153,6 @@ namespace AzToolsFramework } } - void PrefabSystemComponent::PropagateTemplateChangesDown(TemplateId templateId, InstanceOptionalReference instanceToExclude) - { - PropagateTemplateChanges(templateId, instanceToExclude); - - auto templateIterator = m_templateIdMap.find(templateId); - if (templateIterator != m_templateIdMap.end()) - { - for (LinkId linkId : templateIterator->second.GetLinks()) - { - auto linkIterator = m_linkIdMap.find(linkId); - if (linkIterator != m_linkIdMap.end()) - { - PropagateTemplateChangesDown(linkIterator->second.GetSourceTemplateId(), instanceToExclude); - } - } - } - } - void PrefabSystemComponent::UpdatePrefabTemplate(TemplateId templateId, const PrefabDom& updatedDom) { auto templateToUpdate = FindTemplate(templateId); diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.h index ccc17af133..04457b5a97 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.h @@ -213,8 +213,6 @@ namespace AzToolsFramework void PropagateTemplateChanges(TemplateId templateId, InstanceOptionalReference instanceToExclude = AZStd::nullopt) override; - void PropagateTemplateChangesDown(TemplateId templateIdd, InstanceOptionalReference instanceToExclude = AZStd::nullopt) override; - /** * Updates all Instances owned by a Template. * diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponentInterface.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponentInterface.h index 9792ec0fab..0c758a21af 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponentInterface.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponentInterface.h @@ -53,8 +53,6 @@ namespace AzToolsFramework virtual PrefabDom& FindTemplateDom(TemplateId templateId) = 0; virtual void UpdatePrefabTemplate(TemplateId templateId, const PrefabDom& updatedDom) = 0; virtual void PropagateTemplateChanges(TemplateId templateId, InstanceOptionalReference instanceToExclude = AZStd::nullopt) = 0; - //! Propagates template changes recursively down to its dependents - virtual void PropagateTemplateChangesDown(TemplateId templateIdd, InstanceOptionalReference instanceToExclude = AZStd::nullopt) = 0; virtual AZStd::unique_ptr InstantiatePrefab(AZ::IO::PathView filePath) = 0; virtual AZStd::unique_ptr InstantiatePrefab(const TemplateId& templateId) = 0; From c8cd3b1923971bf0b17b0b0fbcb7ad8d44c699ea Mon Sep 17 00:00:00 2001 From: mnaumov Date: Thu, 12 Aug 2021 14:39:15 -0700 Subject: [PATCH 03/37] reverting another file Signed-off-by: mnaumov --- .../Prefab/PrefabPublicHandler.cpp | 36 ++++++------------- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp index 85fe30c252..69efe33d72 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp @@ -482,36 +482,22 @@ namespace AzToolsFramework AZStd::unique_ptr& sourceInstance, TemplateId targetTemplateId, UndoSystem::URSequencePoint* undoBatch) { LinkReference nestedInstanceLink = m_prefabSystemComponentInterface->FindLink(sourceInstance->GetLinkId()); - if (!nestedInstanceLink) - { - AZ_Assert( - false, - "A valid link was not found for one of the instances provided as input for the CreatePrefab operation."); - return; - } + AZ_Assert( + nestedInstanceLink.has_value(), + "A valid link was not found for one of the instances provided as input for the CreatePrefab operation."); PrefabDomReference nestedInstanceLinkDom = nestedInstanceLink->get().GetLinkDom(); - - if (!nestedInstanceLinkDom) - { - AZ_Assert( - false, - "A valid DOM was not found for the link corresponding to one of the instances provided as input for the " - "CreatePrefab operation."); - return; - } + AZ_Assert( + nestedInstanceLinkDom.has_value(), + "A valid DOM was not found for the link corresponding to one of the instances provided as input for the " + "CreatePrefab operation."); PrefabDomValueReference nestedInstanceLinkPatches = PrefabDomUtils::FindPrefabDomValue(nestedInstanceLinkDom->get(), PrefabDomUtils::PatchesName); - - if (!nestedInstanceLinkPatches) - { - AZ_Assert( - false, - "A valid DOM for patches was not found for the link corresponding to one of the instances provided as input for the " - "CreatePrefab operation."); - return; - } + AZ_Assert( + nestedInstanceLinkPatches.has_value(), + "A valid DOM for patches was not found for the link corresponding to one of the instances provided as input for the " + "CreatePrefab operation."); PrefabDom patchesCopyForUndoSupport; patchesCopyForUndoSupport.CopyFrom(nestedInstanceLinkPatches->get(), patchesCopyForUndoSupport.GetAllocator()); From e633b5bdc70ca5b4ea3bb8cc7cd55e7e0acd3e8d Mon Sep 17 00:00:00 2001 From: mnaumov Date: Fri, 13 Aug 2021 19:43:38 -0700 Subject: [PATCH 04/37] Addressing Ronald's feedback Signed-off-by: mnaumov --- .../Prefab/Instance/InstanceSerializer.cpp | 3 +++ .../AzToolsFramework/Prefab/PrefabDomUtils.cpp | 6 ++++++ .../AzToolsFramework/Prefab/PrefabDomUtils.h | 12 +++++++++++- .../AzToolsFramework/Prefab/PrefabLoader.cpp | 2 +- 4 files changed, 21 insertions(+), 2 deletions(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceSerializer.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceSerializer.cpp index 89e0136b12..af42919637 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceSerializer.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceSerializer.cpp @@ -15,6 +15,7 @@ #include #include #include +#include namespace AzToolsFramework { @@ -81,6 +82,8 @@ namespace AzToolsFramework result.Combine(resultInstances); } + PrefabDomUtils::LinkIdMetadata** linkIdMetadata = context.GetMetadata().Find(); + if (linkIdMetadata && *linkIdMetadata) { AZ::ScopedContextPath subPathSource(context, "m_linkId"); diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.cpp index 86983d7912..c58cb21295 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.cpp @@ -64,6 +64,12 @@ namespace AzToolsFramework settings.m_keepDefaults = true; } + if ((flags & StoreInstanceFlags::StoreLinkIds) != StoreInstanceFlags::None) + { + LinkIdMetadata linkIdMetadata; + settings.m_metadata.Add(&linkIdMetadata); + } + AZ::JsonSerializationResult::ResultCode result = AZ::JsonSerialization::Store(prefabDom, prefabDom.GetAllocator(), instance, settings); diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.h index e773b581dd..fe38eb9e0f 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.h @@ -45,7 +45,11 @@ namespace AzToolsFramework //! By default an instance will be stored with default values. In cases where we want to store less json without defaults //! such as saving to disk, this flag will control that behavior. - StripDefaultValues = 1 << 0 + StripDefaultValues = 1 << 0, + + //! We do not save linkIds to file. However when loading a level we want to temporarily save + //! linkIds to instance dom so any nested prefabs will have linkIds correctly set. + StoreLinkIds = 1 << 1 }; AZ_DEFINE_ENUM_BITWISE_OPERATORS(StoreInstanceFlags); @@ -138,6 +142,12 @@ namespace AzToolsFramework [[maybe_unused]] const AZStd::string_view printMessage, [[maybe_unused]] const AzToolsFramework::Prefab::PrefabDomValue& prefabDomValue); + //! An empty struct for passing to JsonSerializerSettings.m_metadata that is consumed by InstanceSerializer::Store. + //! If present in metadata, linkIds will be stored to instance dom. + struct LinkIdMetadata + { + AZ_RTTI(LinkIdMetadata, "{8FF7D299-14E3-41D4-90C5-393A240FAE7C}"); + }; } // namespace PrefabDomUtils } // namespace Prefab } // namespace AzToolsFramework diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabLoader.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabLoader.cpp index ea54c9d10c..2b16134744 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabLoader.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabLoader.cpp @@ -300,7 +300,7 @@ namespace AzToolsFramework } PrefabDom storedPrefabDom(&loadedTemplateDom->get().GetAllocator()); - if (!PrefabDomUtils::StoreInstanceInPrefabDom(loadedPrefabInstance, storedPrefabDom)) + if (!PrefabDomUtils::StoreInstanceInPrefabDom(loadedPrefabInstance, storedPrefabDom, PrefabDomUtils::StoreInstanceFlags::StoreLinkIds)) { return false; } From 9aa391bf7413d15f3814cb6115eac95163733fa1 Mon Sep 17 00:00:00 2001 From: mnaumov Date: Fri, 13 Aug 2021 16:54:57 -0700 Subject: [PATCH 05/37] Fixing Level Save As Signed-off-by: mnaumov --- .../PrefabEditorEntityOwnershipService.cpp | 36 ++----------------- 1 file changed, 2 insertions(+), 34 deletions(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp index b5cf5fb878..7df1e1b5c1 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp @@ -219,42 +219,10 @@ namespace AzToolsFramework bool PrefabEditorEntityOwnershipService::SaveToStream(AZ::IO::GenericStream& stream, AZStd::string_view filename) { AZ::IO::Path relativePath = m_loaderInterface->GenerateRelativePath(filename); - AzToolsFramework::Prefab::TemplateId templateId = m_prefabSystemComponent->GetTemplateIdFromFilePath(relativePath); m_rootInstance->SetTemplateSourcePath(relativePath); - - if (templateId == AzToolsFramework::Prefab::InvalidTemplateId) - { - m_rootInstance->m_containerEntity->AddComponent(aznew Prefab::EditorPrefabComponent()); - HandleEntitiesAdded({ m_rootInstance->m_containerEntity.get() }); - - AzToolsFramework::Prefab::PrefabDom dom; - bool success = AzToolsFramework::Prefab::PrefabDomUtils::StoreInstanceInPrefabDom(*m_rootInstance, dom); - if (!success) - { - AZ_Error("Prefab", false, "Failed to convert current root instance into a DOM when saving file '%.*s'", AZ_STRING_ARG(filename)); - return false; - } - templateId = m_prefabSystemComponent->AddTemplate(relativePath, AZStd::move(dom)); - - if (templateId == AzToolsFramework::Prefab::InvalidTemplateId) - { - AZ_Error("Prefab", false, "Couldn't add new template id '%i' when saving file '%.*s'", templateId, AZ_STRING_ARG(filename)); - return false; - } - } - - Prefab::TemplateId prevTemplateId = m_rootInstance->GetTemplateId(); - m_rootInstance->SetTemplateId(templateId); - - if (prevTemplateId != Prefab::InvalidTemplateId && templateId != prevTemplateId) - { - // Make sure we only have one level template loaded at a time - m_prefabSystemComponent->RemoveTemplate(prevTemplateId); - } - + AZStd::string out; - if (!m_loaderInterface->SaveTemplateToString(m_rootInstance->GetTemplateId(), out)) { return false; @@ -266,7 +234,7 @@ namespace AzToolsFramework { return false; } - m_prefabSystemComponent->SetTemplateDirtyFlag(templateId, false); + m_prefabSystemComponent->SetTemplateDirtyFlag(m_rootInstance->GetTemplateId(), false); return true; } From 9ecb746563909b6092156ffade30d25077508e44 Mon Sep 17 00:00:00 2001 From: lsemp3d <58790905+lsemp3d@users.noreply.github.com> Date: Sat, 14 Aug 2021 16:50:57 -0700 Subject: [PATCH 06/37] Completed namespace support for the ScriptCanvasGrammar template Signed-off-by: lsemp3d <58790905+lsemp3d@users.noreply.github.com> --- .../AutoGen/ScriptCanvasGrammar_Header.jinja | 2 +- .../AutoGen/ScriptCanvasGrammar_Source.jinja | 21 ++++++++++++++++++- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/AutoGen/ScriptCanvasGrammar_Header.jinja b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/AutoGen/ScriptCanvasGrammar_Header.jinja index 00c23b9f35..d743a95ee1 100644 --- a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/AutoGen/ScriptCanvasGrammar_Header.jinja +++ b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/AutoGen/ScriptCanvasGrammar_Header.jinja @@ -77,7 +77,7 @@ public: \ {% if Class.attrib['GraphEntryPoint'] is defined %} bool IsEntryPoint() const override { return {%if Class.attrib['GraphEntryPoint'] == "True" %}true{%else%}false{%endif%}; } \ {% endif %} public: \ - friend struct ::{{ className | replace(' ','') }}Property; + friend struct {% if attribute_Namespace is not defined %}::{% endif %}{{ className | replace(' ','') }}Property; // Helpers for easily accessing properties and slots struct {{ className | replace(' ','') }}Property diff --git a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/AutoGen/ScriptCanvasGrammar_Source.jinja b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/AutoGen/ScriptCanvasGrammar_Source.jinja index 205e6ebd8c..6a381398c1 100644 --- a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/AutoGen/ScriptCanvasGrammar_Source.jinja +++ b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/AutoGen/ScriptCanvasGrammar_Source.jinja @@ -26,6 +26,20 @@ SPDX-License-Identifier: Apache-2.0 OR MIT #include "{{ xml.attrib['Include'] }}" {% for Class in xml.iter('Class') %} + +{% set attribute_Namespace = undefined %} +{%- if Class.attrib['Namespace'] is defined %} +{% if Class.attrib['Namespace'] != "None" %} +{% set attribute_Namespace = Class.attrib['Namespace'] %} +{% endif %} +{% endif %} + +{% if attribute_Namespace is defined %} +namespace {{attribute_Namespace}} +{ +{% endif %} + + void {{ Class.attrib['QualifiedName'] }}::ConfigureSlots() { {% if Class.attrib['Base'] is defined %} @@ -269,7 +283,12 @@ void {{ Class.attrib['QualifiedName'] }}::Reflect(AZ::ReflectContext* context) return datumValue ? *datumValue : {{ Property.attrib['Type'] }}(); } - {% endfor %} + +{% if attribute_Namespace is defined %} +} +{% endif %} + + {% endfor %} {% endfor %} From 0884f96997ab18916b4b83ce01f167eb75d1eea6 Mon Sep 17 00:00:00 2001 From: mnaumov Date: Mon, 16 Aug 2021 16:00:18 -0700 Subject: [PATCH 07/37] PR feedback Signed-off-by: mnaumov --- .../AzToolsFramework/Prefab/Instance/InstanceSerializer.cpp | 6 +++--- .../AzToolsFramework/Prefab/PrefabDomUtils.cpp | 3 +-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceSerializer.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceSerializer.cpp index af42919637..92b37b10db 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceSerializer.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceSerializer.cpp @@ -82,13 +82,13 @@ namespace AzToolsFramework result.Combine(resultInstances); } - PrefabDomUtils::LinkIdMetadata** linkIdMetadata = context.GetMetadata().Find(); - if (linkIdMetadata && *linkIdMetadata) + PrefabDomUtils::LinkIdMetadata* subPathLinkId = context.GetMetadata().Find(); + if (subPathLinkId) { AZ::ScopedContextPath subPathSource(context, "m_linkId"); result = ContinueStoringToJsonObjectField( - outputValue, "LinkId", &(instance->m_linkId), &InvalidLinkId, azrtti_typeid(), context); + outputValue, "LinkId", &(instance->m_linkId), &InvalidLinkId, azrtti_typeidm_linkId)>(), context); } return context.Report(result, diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.cpp index 2462bc0348..dcc14d3b91 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.cpp @@ -90,8 +90,7 @@ namespace AzToolsFramework if ((flags & StoreFlags::StoreLinkIds) != StoreFlags::None) { - LinkIdMetadata linkIdMetadata; - settings.m_metadata.Add(&linkIdMetadata); + settings.m_metadata.Create(); } AZStd::string scratchBuffer; From a20428d3933beb532430e09950ef26b104bc982f Mon Sep 17 00:00:00 2001 From: mnaumov Date: Mon, 16 Aug 2021 23:16:04 -0700 Subject: [PATCH 08/37] Fixing non unity build Signed-off-by: Mikhail Naumov --- .../AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.h index 77b4b752a6..7cab24ad9f 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.h @@ -159,6 +159,8 @@ namespace AzToolsFramework struct LinkIdMetadata { AZ_RTTI(LinkIdMetadata, "{8FF7D299-14E3-41D4-90C5-393A240FAE7C}"); + + virtual ~LinkIdMetadata() {} }; } // namespace PrefabDomUtils } // namespace Prefab From 8f4a35b146af277ad903ab5522c33a2362ce2cb7 Mon Sep 17 00:00:00 2001 From: Guthrie Adams Date: Tue, 17 Aug 2021 13:21:13 -0500 Subject: [PATCH 09/37] AtomTools: fix unused variable errors Signed-off-by: Guthrie Adams --- .../Code/Source/Document/AtomToolsDocument.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocument.cpp b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocument.cpp index 48212fe154..e8eb9401d4 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocument.cpp +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocument.cpp @@ -40,29 +40,35 @@ namespace AtomToolsFramework const AZStd::any& AtomToolsDocument::GetPropertyValue(const AZ::Name& propertyFullName) const { + AZ_UNUSED(propertyFullName); AZ_Error("AtomToolsDocument", false, "%s not implemented.", __FUNCTION__); return m_invalidValue; } const AtomToolsFramework::DynamicProperty& AtomToolsDocument::GetProperty(const AZ::Name& propertyFullName) const { + AZ_UNUSED(propertyFullName); AZ_Error("AtomToolsDocument", false, "%s not implemented.", __FUNCTION__); return m_invalidProperty; } bool AtomToolsDocument::IsPropertyGroupVisible(const AZ::Name& propertyGroupFullName) const { + AZ_UNUSED(propertyGroupFullName); AZ_Error("AtomToolsDocument", false, "%s not implemented.", __FUNCTION__); return false; } void AtomToolsDocument::SetPropertyValue(const AZ::Name& propertyFullName, const AZStd::any& value) { + AZ_UNUSED(propertyFullName); + AZ_UNUSED(value); AZ_Error("AtomToolsDocument", false, "%s not implemented.", __FUNCTION__); } bool AtomToolsDocument::Open(AZStd::string_view loadPath) { + AZ_UNUSED(loadPath); AZ_Error("AtomToolsDocument", false, "%s not implemented.", __FUNCTION__); return false; } @@ -81,6 +87,7 @@ namespace AtomToolsFramework bool AtomToolsDocument::SaveAsCopy(AZStd::string_view savePath) { + AZ_UNUSED(savePath); AZ_Error("AtomToolsDocument", false, "%s not implemented.", __FUNCTION__); return false; } @@ -88,6 +95,7 @@ namespace AtomToolsFramework bool AtomToolsDocument::SaveAsChild(AZStd::string_view savePath) { + AZ_UNUSED(savePath); AZ_Error("AtomToolsDocument", false, "%s not implemented.", __FUNCTION__); return false; } From e865ad5d2368096caf00896dd0fdb0eee846c139 Mon Sep 17 00:00:00 2001 From: Jacob Hilliard <64656371+jcbhl@users.noreply.github.com> Date: Tue, 17 Aug 2021 11:33:13 -0700 Subject: [PATCH 10/37] Profiler: implement loading from saved captures (#3026) * Profiler: implement loading from saved capture Adds functionality for finding a saved capture on disk and then deserializing it using rapidjson's built-in buffered stream reader. This does require use of raw file pointers since saved captures can be hundreds of megabytes. Actually showing the data in the visualizer is TODO. * Profiler: use heap buffer over stack buffer * Profiler: move deserialization logic to ImGuiCpuProfiler Signed-off-by: Jacob Hilliard --- .../ProfilingCaptureSystemComponent.cpp | 98 +----------- .../Code/Include/Atom/RHI/CpuProfilerImpl.h | 31 +++- .../RHI/Code/Source/RHI/CpuProfilerImpl.cpp | 63 +++++++- .../Include/Atom/RPI.Edit/Common/JsonUtils.h | 3 +- .../Include/Atom/Utils/ImGuiCpuProfiler.h | 15 ++ .../Include/Atom/Utils/ImGuiCpuProfiler.inl | 151 +++++++++++++++++- 6 files changed, 260 insertions(+), 101 deletions(-) diff --git a/Gems/Atom/Feature/Common/Code/Source/ProfilingCaptureSystemComponent.cpp b/Gems/Atom/Feature/Common/Code/Source/ProfilingCaptureSystemComponent.cpp index add6d0e098..7c6dfcf744 100644 --- a/Gems/Atom/Feature/Common/Code/Source/ProfilingCaptureSystemComponent.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/ProfilingCaptureSystemComponent.cpp @@ -9,6 +9,7 @@ #include "ProfilingCaptureSystemComponent.h" #include +#include #include #include #include @@ -141,36 +142,6 @@ namespace AZ AZStd::vector m_pipelineStatisticsEntries; }; - // Intermediate class to serialize Cpu TimedRegion data. - class CpuProfilingStatisticsSerializer - { - public: - class CpuProfilingStatisticsSerializerEntry - { - public: - AZ_TYPE_INFO(CpuProfilingStatisticsSerializer::CpuProfilingStatisticsSerializerEntry, "{26B78F65-EB96-46E2-BE7E-A1233880B225}"); - static void Reflect(AZ::ReflectContext* context); - - CpuProfilingStatisticsSerializerEntry() = default; - CpuProfilingStatisticsSerializerEntry(const RHI::CachedTimeRegion& cachedTimeRegion); - - private: - Name m_groupName; - Name m_regionName; - uint16_t m_stackDepth; - AZStd::sys_time_t m_startTick; - AZStd::sys_time_t m_endTick; - }; - - AZ_TYPE_INFO(CpuProfilingStatisticsSerializer, "{D5B02946-0D27-474F-9A44-364C2706DD41}"); - static void Reflect(AZ::ReflectContext* context); - - CpuProfilingStatisticsSerializer() = default; - CpuProfilingStatisticsSerializer(const AZStd::ring_buffer& continuousData); - - AZStd::vector m_cpuProfilingStatisticsSerializerEntries; - }; - // Intermediate class to serialize benchmark metadata. class BenchmarkMetadataSerializer { @@ -327,65 +298,6 @@ namespace AZ } } - // --- CpuProfilingStatisticsSerializer --- - - CpuProfilingStatisticsSerializer::CpuProfilingStatisticsSerializer(const AZStd::ring_buffer& continuousData) - { - // Create serializable entries - for (const auto& timeRegionMap : continuousData) - { - for (const auto& threadEntry : timeRegionMap) - { - for (const auto& cachedRegionEntry : threadEntry.second) - { - m_cpuProfilingStatisticsSerializerEntries.insert( - m_cpuProfilingStatisticsSerializerEntries.end(), - cachedRegionEntry.second.begin(), - cachedRegionEntry.second.end()); - } - } - } - } - - void CpuProfilingStatisticsSerializer::Reflect(AZ::ReflectContext* context) - { - if (auto* serializeContext = azrtti_cast(context)) - { - serializeContext->Class() - ->Version(1) - ->Field("cpuProfilingStatisticsSerializerEntry", &CpuProfilingStatisticsSerializer::m_cpuProfilingStatisticsSerializerEntries) - ; - } - - CpuProfilingStatisticsSerializerEntry::Reflect(context); - } - - // --- CpuProfilingStatisticsSerializerEntry --- - - CpuProfilingStatisticsSerializer::CpuProfilingStatisticsSerializerEntry::CpuProfilingStatisticsSerializerEntry(const RHI::CachedTimeRegion& cachedTimeRegion) - { - m_groupName = cachedTimeRegion.m_groupRegionName->m_groupName; - m_regionName = cachedTimeRegion.m_groupRegionName->m_regionName; - m_stackDepth = cachedTimeRegion.m_stackDepth; - m_startTick = cachedTimeRegion.m_startTick; - m_endTick = cachedTimeRegion.m_endTick; - } - - void CpuProfilingStatisticsSerializer::CpuProfilingStatisticsSerializerEntry::Reflect(AZ::ReflectContext* context) - { - if (auto* serializeContext = azrtti_cast(context)) - { - serializeContext->Class() - ->Version(1) - ->Field("groupName", &CpuProfilingStatisticsSerializerEntry::m_groupName) - ->Field("regionName", &CpuProfilingStatisticsSerializerEntry::m_regionName) - ->Field("stackDepth", &CpuProfilingStatisticsSerializerEntry::m_stackDepth) - ->Field("startTick", &CpuProfilingStatisticsSerializerEntry::m_startTick) - ->Field("endTick", &CpuProfilingStatisticsSerializerEntry::m_endTick) - ; - } - } - // --- BenchmarkMetadataSerializer --- BenchmarkMetadataSerializer::BenchmarkMetadataSerializer(const AZStd::string& benchmarkName, const RHI::PhysicalDeviceDescriptor& gpuDescriptor) @@ -458,7 +370,7 @@ namespace AZ TimestampSerializer::Reflect(context); CpuFrameTimeSerializer::Reflect(context); PipelineStatisticsSerializer::Reflect(context); - CpuProfilingStatisticsSerializer::Reflect(context); + RHI::CpuProfilingStatisticsSerializer::Reflect(context); BenchmarkMetadataSerializer::Reflect(context); } @@ -651,10 +563,10 @@ namespace AZ JsonSerializerSettings serializationSettings; serializationSettings.m_keepDefaults = true; - CpuProfilingStatisticsSerializer serializer(data); + RHI::CpuProfilingStatisticsSerializer serializer(data); const auto saveResult = JsonSerializationUtils::SaveObjectToFile(&serializer, - outputFilePath, (CpuProfilingStatisticsSerializer*)nullptr, &serializationSettings); + outputFilePath, (RHI::CpuProfilingStatisticsSerializer*)nullptr, &serializationSettings); AZStd::string captureInfo = outputFilePath; if (!saveResult.IsSuccess()) @@ -694,7 +606,7 @@ namespace AZ const bool captureStarted = m_cpuProfilingStatisticsCapture.StartCapture([this, outputFilePath, wasEnabled]() { // Blocking call for a single frame of data, avoid thread overhead - AZStd::ring_buffer singleFrameData; + AZStd::ring_buffer singleFrameData(1); singleFrameData.push_back(RHI::CpuProfiler::Get()->GetTimeRegionMap()); SerializeCpuProfilingData(singleFrameData, outputFilePath, wasEnabled); }); diff --git a/Gems/Atom/RHI/Code/Include/Atom/RHI/CpuProfilerImpl.h b/Gems/Atom/RHI/Code/Include/Atom/RHI/CpuProfilerImpl.h index 2e4ca67db8..9372977d8e 100644 --- a/Gems/Atom/RHI/Code/Include/Atom/RHI/CpuProfilerImpl.h +++ b/Gems/Atom/RHI/Code/Include/Atom/RHI/CpuProfilerImpl.h @@ -12,6 +12,7 @@ #include #include +#include #include #include #include @@ -161,5 +162,33 @@ namespace AZ AZStd::ring_buffer m_continuousCaptureData; }; - }; // namespace RPI + // Intermediate class to serialize Cpu TimedRegion data. + class CpuProfilingStatisticsSerializer + { + public: + class CpuProfilingStatisticsSerializerEntry + { + public: + AZ_TYPE_INFO(CpuProfilingStatisticsSerializer::CpuProfilingStatisticsSerializerEntry, "{26B78F65-EB96-46E2-BE7E-A1233880B225}"); + static void Reflect(AZ::ReflectContext* context); + + CpuProfilingStatisticsSerializerEntry() = default; + CpuProfilingStatisticsSerializerEntry(const RHI::CachedTimeRegion& cachedTimeRegion); + + Name m_groupName; + Name m_regionName; + uint16_t m_stackDepth; + AZStd::sys_time_t m_startTick; + AZStd::sys_time_t m_endTick; + }; + + AZ_TYPE_INFO(CpuProfilingStatisticsSerializer, "{D5B02946-0D27-474F-9A44-364C2706DD41}"); + static void Reflect(AZ::ReflectContext* context); + + CpuProfilingStatisticsSerializer() = default; + CpuProfilingStatisticsSerializer(const AZStd::ring_buffer& continuousData); + + AZStd::vector m_cpuProfilingStatisticsSerializerEntries; + }; + }; // namespace RHI }; // namespace AZ diff --git a/Gems/Atom/RHI/Code/Source/RHI/CpuProfilerImpl.cpp b/Gems/Atom/RHI/Code/Source/RHI/CpuProfilerImpl.cpp index d41b5d656e..5585cc7032 100644 --- a/Gems/Atom/RHI/Code/Source/RHI/CpuProfilerImpl.cpp +++ b/Gems/Atom/RHI/Code/Source/RHI/CpuProfilerImpl.cpp @@ -409,5 +409,64 @@ namespace AZ m_cachedTimeRegionMutex.unlock(); } } - } -} + + // --- CpuProfilingStatisticsSerializer --- + + CpuProfilingStatisticsSerializer::CpuProfilingStatisticsSerializer(const AZStd::ring_buffer& continuousData) + { + // Create serializable entries + for (const auto& timeRegionMap : continuousData) + { + for (const auto& threadEntry : timeRegionMap) + { + for (const auto& cachedRegionEntry : threadEntry.second) + { + m_cpuProfilingStatisticsSerializerEntries.insert( + m_cpuProfilingStatisticsSerializerEntries.end(), + cachedRegionEntry.second.begin(), + cachedRegionEntry.second.end()); + } + } + } + } + + void CpuProfilingStatisticsSerializer::Reflect(AZ::ReflectContext* context) + { + if (auto* serializeContext = azrtti_cast(context)) + { + serializeContext->Class() + ->Version(1) + ->Field("cpuProfilingStatisticsSerializerEntries", &CpuProfilingStatisticsSerializer::m_cpuProfilingStatisticsSerializerEntries) + ; + } + + CpuProfilingStatisticsSerializerEntry::Reflect(context); + } + + // --- CpuProfilingStatisticsSerializerEntry --- + + CpuProfilingStatisticsSerializer::CpuProfilingStatisticsSerializerEntry::CpuProfilingStatisticsSerializerEntry(const RHI::CachedTimeRegion& cachedTimeRegion) + { + m_groupName = cachedTimeRegion.m_groupRegionName->m_groupName; + m_regionName = cachedTimeRegion.m_groupRegionName->m_regionName; + m_stackDepth = cachedTimeRegion.m_stackDepth; + m_startTick = cachedTimeRegion.m_startTick; + m_endTick = cachedTimeRegion.m_endTick; + } + + void CpuProfilingStatisticsSerializer::CpuProfilingStatisticsSerializerEntry::Reflect(AZ::ReflectContext* context) + { + if (auto* serializeContext = azrtti_cast(context)) + { + serializeContext->Class() + ->Version(1) + ->Field("groupName", &CpuProfilingStatisticsSerializerEntry::m_groupName) + ->Field("regionName", &CpuProfilingStatisticsSerializerEntry::m_regionName) + ->Field("stackDepth", &CpuProfilingStatisticsSerializerEntry::m_stackDepth) + ->Field("startTick", &CpuProfilingStatisticsSerializerEntry::m_startTick) + ->Field("endTick", &CpuProfilingStatisticsSerializerEntry::m_endTick) + ; + } + } + } // namespace RHI +} // namespace AZ diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Common/JsonUtils.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Common/JsonUtils.h index 3e3fbec8fe..549f787ac9 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Common/JsonUtils.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Common/JsonUtils.h @@ -10,7 +10,9 @@ #include #include + #include + #include #include @@ -118,7 +120,6 @@ namespace AZ AZ_Error("AZ::RPI::JsonUtils", false, "Failed to load object from json string: %s", loadResult.GetError().c_str()); return false; } - } // namespace JsonUtils } // namespace RPI } // namespace AZ diff --git a/Gems/Atom/Utils/Code/Include/Atom/Utils/ImGuiCpuProfiler.h b/Gems/Atom/Utils/Code/Include/Atom/Utils/ImGuiCpuProfiler.h index 75b7ec9fdd..bdaf38a64a 100644 --- a/Gems/Atom/Utils/Code/Include/Atom/Utils/ImGuiCpuProfiler.h +++ b/Gems/Atom/Utils/Code/Include/Atom/Utils/ImGuiCpuProfiler.h @@ -9,6 +9,7 @@ #pragma once #include +#include #include #include @@ -102,6 +103,12 @@ namespace AZ //! Draws the statistical view of the CPU profiling data. void DrawStatisticsView(); + //! Callback invoked when the "Load File" button is pressed in the file picker. + void LoadFile(); + + //! Draws the file picker window. + void DrawFilePicker(); + //! Draws the CPU profiling visualizer. void DrawVisualizer(); @@ -198,6 +205,14 @@ namespace AZ AZ::RHI::CpuTimingStatistics m_cpuTimingStatisticsWhenPause; AZStd::string m_lastCapturedFilePath; + + bool m_showFilePicker = false; + + // Cached file paths to previous traces on disk, sorted with the most recent trace at the front. + AZStd::vector m_cachedCapturePaths; + + // Index into the file picker, used to determine which file to load when "Load File" is pressed. + int m_currentFileIndex = 0; }; } // namespace Render } // namespace AZ diff --git a/Gems/Atom/Utils/Code/Include/Atom/Utils/ImGuiCpuProfiler.inl b/Gems/Atom/Utils/Code/Include/Atom/Utils/ImGuiCpuProfiler.inl index a24fdbf1d8..9b4eebf043 100644 --- a/Gems/Atom/Utils/Code/Include/Atom/Utils/ImGuiCpuProfiler.inl +++ b/Gems/Atom/Utils/Code/Include/Atom/Utils/ImGuiCpuProfiler.inl @@ -9,9 +9,14 @@ #include #include #include +#include +#include #include -#include +#include +#include +#include +#include #include #include #include @@ -45,6 +50,74 @@ namespace AZ AZ_Assert(ticksPerSecond >= 1000, "Error in converting ticks to ms, expected ticksPerSecond >= 1000"); return static_cast((ticks * 1000) / (ticksPerSecond / 1000)) / 1000.0f; } + + using DeserializedCpuData = AZStd::vector; + inline Outcome LoadSavedCpuProfilingStatistics(const AZStd::string& capturePath) + { + auto* base = IO::FileIOBase::GetInstance(); + + char resolvedPath[IO::MaxPathLength]; + if (!base->ResolvePath(capturePath.c_str(), resolvedPath, IO::MaxPathLength)) + { + return Failure(AZStd::string::format("Could not resolve the path to file %s, is the path correct?", resolvedPath)); + } + + u64 captureSizeBytes; + const IO::Result fileSizeResult = base->Size(resolvedPath, captureSizeBytes); + if (!fileSizeResult) + { + return Failure(AZStd::string::format("Could not read the size of file %s, is the path correct?", resolvedPath)); + } + + // NOTE: this uses raw file pointers over the abstractions and utility functions provided by AZ::JsonSerializationUtils because + // saved profiling captures can be upwards of 400 MB. This necessitates a buffered approach to avoid allocating huge chunks of memory. + FILE* fp = nullptr; + azfopen(&fp, resolvedPath, "rb"); + if (!fp) + { + return Failure(AZStd::string::format("Could not fopen file %s, is the path correct?\n", resolvedPath)); + } + + constexpr AZStd::size_t MaxBufSize = 65536; + const AZStd::size_t bufSize = AZStd::min(MaxBufSize, aznumeric_cast(captureSizeBytes)); + char* buf = reinterpret_cast(azmalloc(bufSize)); + + rapidjson::Document document; + rapidjson::FileReadStream inputStream(fp, buf, bufSize); + document.ParseStream(inputStream); + + azfree(buf); + fclose(fp); + + if (document.HasParseError()) + { + const auto pe = document.GetParseError(); + return Failure(AZStd::string::format( + "Rapidjson could not parse the document with ParseErrorCode %u. See 3rdParty/rapidjson/error.h for definitions.\n", pe)); + } + + if (!document.IsObject() || !document.HasMember("ClassData")) + { + return Failure(AZStd::string::format( + "Error in loading saved capture: top-level object does not have a ClassData field. Did the serialization format change recently?\n")); + } + + AZ_TracePrintf("JsonUtils", "Successfully loaded JSON into memory.\n"); + + const auto& root = document["ClassData"]; + RHI::CpuProfilingStatisticsSerializer serializer; + const JsonSerializationResult::ResultCode deserializationResult = JsonSerialization::Load(serializer, root); + if (deserializationResult.GetProcessing() == JsonSerializationResult::Processing::Halted + || serializer.m_cpuProfilingStatisticsSerializerEntries.empty()) + { + return Failure(AZStd::string::format("Error in deserializing document: %s\n", deserializationResult.ToString(capturePath.c_str()).c_str())); + } + + AZ_TracePrintf("JsonUtils", "Successfully loaded CPU profiling data with %zu profiling entries.\n", + serializer.m_cpuProfilingStatisticsSerializerEntries.size()); + + return Success(AZStd::move(serializer.m_cpuProfilingStatisticsSerializerEntries)); + } } // namespace CpuProfilerImGuiHelper inline void ImGuiCpuProfiler::Draw(bool& keepDrawing, const AZ::RHI::CpuTimingStatistics& currentCpuTimingStatistics) @@ -80,6 +153,11 @@ namespace AZ { DrawStatisticsView(); } + + if (m_showFilePicker) + { + DrawFilePicker(); + } } ImGui::End(); @@ -110,6 +188,11 @@ namespace AZ inline void ImGuiCpuProfiler::DrawCommonHeader() { + if (!m_lastCapturedFilePath.empty()) + { + ImGui::Text("Saved: %s", m_lastCapturedFilePath.c_str()); + } + if (ImGui::Button(m_enableVisualizer ? "Swap to statistics" : "Swap to visualizer")) { m_enableVisualizer = !m_enableVisualizer; @@ -157,10 +240,31 @@ namespace AZ } } - if (!m_lastCapturedFilePath.empty()) + ImGui::SameLine(); + if (ImGui::Button("Load file")) { - ImGui::SameLine(); - ImGui::Text("Saved: %s", m_lastCapturedFilePath.c_str()); + m_showFilePicker = true; + + // Only update the cached file list when opened so that we aren't making IO calls on every frame. + auto* base = AZ::IO::FileIOBase::GetInstance(); + const AZStd::string defaultSavedCapturePath = "@user@/CpuProfiler"; + + m_cachedCapturePaths.clear(); + base->FindFiles( + defaultSavedCapturePath.c_str(), "*.json", + [&paths = m_cachedCapturePaths](const char* path) -> bool + { + auto foundPath = IO::Path(path); + paths.push_back(foundPath); + return true; + }); + + // Sort by decreasing modification time (most recent at the top) + AZStd::sort(m_cachedCapturePaths.begin(), m_cachedCapturePaths.end(), + [&base](const IO::Path& lhs, const IO::Path& rhs) + { + return base->ModificationTime(lhs.c_str()) > base->ModificationTime(rhs.c_str()); + }); } } @@ -313,6 +417,45 @@ namespace AZ } } + inline void ImGuiCpuProfiler::DrawFilePicker() + { + ImGui::SetNextWindowSize({ 500, 200 }, ImGuiCond_Once); + if (ImGui::Begin("File Picker", &m_showFilePicker)) + { + if (ImGui::Button("Load selected")) + { + LoadFile(); + } + + auto getter = [](void* vectorPointer, int idx, const char** out_text) -> bool + { + const auto& pathVec = *static_cast*>(vectorPointer); + if (idx < 0 || idx >= pathVec.size()) + { + return false; + } + *out_text = pathVec[idx].c_str(); + return true; + }; + + ImGui::SetNextItemWidth(ImGui::GetWindowContentRegionWidth()); + ImGui::ListBox("", &m_currentFileIndex, getter, &m_cachedCapturePaths, aznumeric_cast(m_cachedCapturePaths.size())); + } + ImGui::End(); + } + + inline void ImGuiCpuProfiler::LoadFile() + { + const IO::Path& pathToLoad = m_cachedCapturePaths[m_currentFileIndex]; + auto res = CpuProfilerImGuiHelper::LoadSavedCpuProfilingStatistics(pathToLoad.String()); + if (!res.IsSuccess()) + { + AZ_TracePrintf("ImGuiCpuProfiler", "%s", res.GetError().c_str()); + return; + } + // TODO ATOM-16022 Parse this data and display it in the visualizer widget. + } + // -- CPU Visualizer -- inline void ImGuiCpuProfiler::DrawVisualizer() { From fb05beffe3c7059f14e0a13f269b8e150ad25456 Mon Sep 17 00:00:00 2001 From: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> Date: Tue, 17 Aug 2021 12:05:51 -0700 Subject: [PATCH 11/37] Change LY_UNITY_BUILD default to "ON" (#3244) Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> --- cmake/LYWrappers.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/LYWrappers.cmake b/cmake/LYWrappers.cmake index e4398099d1..1a8e805f91 100644 --- a/cmake/LYWrappers.cmake +++ b/cmake/LYWrappers.cmake @@ -6,7 +6,7 @@ # # -set(LY_UNITY_BUILD OFF CACHE BOOL "UNITY builds") +set(LY_UNITY_BUILD ON CACHE BOOL "UNITY builds") include(CMakeFindDependencyMacro) include(cmake/LyAutoGen.cmake) From 401d0c1ad5e17a03409eb5267271ff6504df4f86 Mon Sep 17 00:00:00 2001 From: Junbo Liang <68558268+junbo75@users.noreply.github.com> Date: Tue, 17 Aug 2021 15:17:48 -0700 Subject: [PATCH 12/37] Update the AWSNativeSDK version and hash for Mac (#3259) --- cmake/3rdParty/Platform/Mac/BuiltInPackages_mac.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/3rdParty/Platform/Mac/BuiltInPackages_mac.cmake b/cmake/3rdParty/Platform/Mac/BuiltInPackages_mac.cmake index e66dfdaca1..a0001d3e4e 100644 --- a/cmake/3rdParty/Platform/Mac/BuiltInPackages_mac.cmake +++ b/cmake/3rdParty/Platform/Mac/BuiltInPackages_mac.cmake @@ -32,7 +32,7 @@ ly_associate_package(PACKAGE_NAME DirectXShaderCompilerDxc-1.6.2104-o3de-rev2-ma ly_associate_package(PACKAGE_NAME SPIRVCross-2021.04.29-rev1-mac TARGETS SPIRVCross PACKAGE_HASH 78c6376ed2fd195b9b1f5fb2b56e5267a32c3aa21fb399e905308de470eb4515) ly_associate_package(PACKAGE_NAME freetype-2.10.4.14-mac-ios TARGETS freetype PACKAGE_HASH 67b4f57aed92082d3fd7c16aa244a7d908d90122c296b0a63f73e0a0b8761977) ly_associate_package(PACKAGE_NAME tiff-4.2.0.15-mac-ios TARGETS tiff PACKAGE_HASH a23ae1f8991a29f8e5df09d6d5b00d7768a740f90752cef465558c1768343709) -ly_associate_package(PACKAGE_NAME AWSNativeSDK-1.7.167-rev3-mac TARGETS AWSNativeSDK PACKAGE_HASH 21920372e90355407578b45ac19580df1463a39a25a867bcd0ffd8b385c8254a) +ly_associate_package(PACKAGE_NAME AWSNativeSDK-1.7.167-rev4-mac TARGETS AWSNativeSDK PACKAGE_HASH 89e1651cde6b4e6bd80cdb96ed6b624accad9f9688ff38bfca226777f4fcb678) ly_associate_package(PACKAGE_NAME Lua-5.3.5-rev6-mac TARGETS Lua PACKAGE_HASH b9079fd35634774c9269028447562c6b712dbc83b9c64975c095fd423ff04c08) ly_associate_package(PACKAGE_NAME PhysX-4.1.2.29882248-rev3-mac TARGETS PhysX PACKAGE_HASH 5e092a11d5c0a50c4dd99bb681a04b566a4f6f29aa08443d9bffc8dc12c27c8e) ly_associate_package(PACKAGE_NAME etc2comp-9cd0f9cae0-rev1-mac TARGETS etc2comp PACKAGE_HASH 1966ab101c89db7ecf30984917e0a48c0d02ee0e4d65b798743842b9469c0818) From 3e51240a05117c9573f91392e8ee4acecd3e9293 Mon Sep 17 00:00:00 2001 From: Shirang Jia Date: Tue, 17 Aug 2021 15:25:32 -0700 Subject: [PATCH 13/37] Allow customized Jenkins parameters for different pipelines and add aws integration deployment pipeline (#3248) Allow customized Jenkins parameters for different pipelines so that we can define different Jenkins parameters for a new pipeline and doesn't affect AR build parameters. Add aws integration deployment pipeline. --- scripts/build/Jenkins/Jenkinsfile | 34 +++++++++++++++++++ .../build/Platform/Windows/build_config.json | 5 +++ scripts/build/Platform/Windows/pipeline.json | 32 +++++++++++++++++ 3 files changed, 71 insertions(+) diff --git a/scripts/build/Jenkins/Jenkinsfile b/scripts/build/Jenkins/Jenkinsfile index 30efdc00ad..1c0409ceec 100644 --- a/scripts/build/Jenkins/Jenkinsfile +++ b/scripts/build/Jenkins/Jenkinsfile @@ -634,6 +634,40 @@ try { pipelineParameters.add(booleanParam(defaultValue: true, description: '', name: platform.key)) } } + // Add additional Jenkins parameters + pipelineConfig.platforms.each { platform -> + platformEnv = platform.value.PIPELINE_ENV + pipelineJenkinsParameters = platformEnv['PIPELINE_JENKINS_PARAMETERS'] ?: [:] + jenkinsParametersToAdd = pipelineJenkinsParameters[pipelineName] ?: [:] + jenkinsParametersToAdd.each{ jenkinsParameter -> + defaultValue = jenkinsParameter['default_value'] + // Use last run's value as default value so we can save values in different Jenkins environment + if (jenkinsParameter['use_last_run_value']?.toBoolean()) { + defaultValue = params."$jenkinsParameter['parameter_name']" ?: jenkinsParameter['default_value'] + } + switch (jenkinsParameter['parameter_type']) { + case 'string': + pipelineParameters.add(stringParam(defaultValue: defaultValue, + description: jenkinsParameter['description'], + name: jenkinsParameter['parameter_name'] + )) + break + case 'boolean': + pipelineParameters.add(booleanParam(defaultValue: defaultValue, + description: jenkinsParameter['description'], + name: jenkinsParameter['parameter_name'] + )) + break + case 'password': + pipelineParameters.add(password(defaultValue: defaultValue, + description: jenkinsParameter['description'], + name: jenkinsParameter['parameter_name'] + )) + break + } + } + } + pipelineProperties.add(parameters(pipelineParameters)) properties(pipelineProperties) diff --git a/scripts/build/Platform/Windows/build_config.json b/scripts/build/Platform/Windows/build_config.json index 0268412aea..8928794a43 100644 --- a/scripts/build/Platform/Windows/build_config.json +++ b/scripts/build/Platform/Windows/build_config.json @@ -376,5 +376,10 @@ "install_profile_vs2019", "project_engineinstall_profile_vs2019" ] + }, + "awsi_deployment": { + "TAGS": ["awsi-deployment"], + "COMMAND": "deploy_cdk_applications.cmd", + "PARAMETERS": {} } } diff --git a/scripts/build/Platform/Windows/pipeline.json b/scripts/build/Platform/Windows/pipeline.json index 622fa9d5ae..4cfc6f696a 100644 --- a/scripts/build/Platform/Windows/pipeline.json +++ b/scripts/build/Platform/Windows/pipeline.json @@ -16,5 +16,37 @@ "nightly-clean": { "CLEAN_WORKSPACE": true } + }, + "PIPELINE_JENKINS_PARAMETERS": { + "awsi-deployment": [ + { + "parameter_name": "O3DE_AWS_PROJECT_NAME", + "parameter_type": "string", + "default_value": "", + "use_last_run_value": true, + "description": "" + }, + { + "parameter_name": "O3DE_AWS_DEPLOY_REGION", + "parameter_type": "string", + "default_value": "", + "use_last_run_value": true, + "description": "" + }, + { + "parameter_name": "ASSUME_ROLE_ARN", + "parameter_type": "string", + "default_value": "", + "use_last_run_value": true, + "description": "" + }, + { + "parameter_name": "COMMIT_ID", + "parameter_type": "string", + "default_value": "", + "use_last_run_value": true, + "description": "" + } + ] } } \ No newline at end of file From 078e0a86938699d3fce2c44027540cc5637f5735 Mon Sep 17 00:00:00 2001 From: amzn-phist <52085794+amzn-phist@users.noreply.github.com> Date: Tue, 17 Aug 2021 17:59:09 -0500 Subject: [PATCH 14/37] Audio legacy cleanup - Move global functions to be handled by AudioSystemComponent (#3283) * Removes legacy audio listener updates from ViewSys These functions were empty and logging a warning, removed. Signed-off-by: amzn-phist <52085794+amzn-phist@users.noreply.github.com> * Move more audio functions to AudioSystemComponent For global actions like Mute/Unmute, Reload, StopAll, Load/Unload Level, we move those functions to be handled by the AudioSystemComponent in LmbrCentral. This lets us remove some includes of IAudioSystem.h from Editor and Legacy/CrySystem. There were several locations where audio banks were being loaded and unloaded for a level. Now they all call into the AudioSystemComponent and we don't have multiple copies of the same code. Signed-off-by: amzn-phist <52085794+amzn-phist@users.noreply.github.com> --- Code/Editor/CryEditDoc.cpp | 50 +------ Code/Editor/MainWindow.cpp | 20 ++- Code/Editor/ViewportTitleDlg.cpp | 41 ++++-- Code/Editor/ViewportTitleDlg.h | 8 - Code/Legacy/CryCommon/IViewSystem.h | 3 - .../CrySystem/LevelSystem/LevelSystem.cpp | 52 ------- .../LevelSystem/SpawnableLevelSystem.cpp | 58 -------- Code/Legacy/CrySystem/System.cpp | 18 --- .../CrySystem/ViewSystem/DebugCamera.cpp | 6 - Code/Legacy/CrySystem/ViewSystem/View.cpp | 5 - Code/Legacy/CrySystem/ViewSystem/View.h | 1 - .../CrySystem/ViewSystem/ViewSystem.cpp | 11 -- Code/Legacy/CrySystem/ViewSystem/ViewSystem.h | 2 - .../Source/Audio/AudioSystemComponent.cpp | 138 ++++++++++++++++-- .../Code/Source/Audio/AudioSystemComponent.h | 13 ++ .../Audio/AudioSystemComponentBus.h | 11 ++ .../Code/Source/Animation/AzEntityNode.cpp | 1 - 17 files changed, 192 insertions(+), 246 deletions(-) diff --git a/Code/Editor/CryEditDoc.cpp b/Code/Editor/CryEditDoc.cpp index c720299ce8..90b020f452 100644 --- a/Code/Editor/CryEditDoc.cpp +++ b/Code/Editor/CryEditDoc.cpp @@ -32,9 +32,6 @@ #include #include -// CryCommon -#include - // Editor #include "Settings.h" @@ -60,6 +57,7 @@ #include // LmbrCentral +#include #include // for LmbrCentral::EditorLightComponentRequestBus //#define PROFILE_LOADING_WITH_VTUNE @@ -269,20 +267,7 @@ void CCryEditDoc::DeleteContents() CErrorReportDialog::Clear(); // Unload level specific audio binary data. - Audio::SAudioManagerRequestData oAMData(Audio::eADS_LEVEL_SPECIFIC); - Audio::SAudioRequest oAudioRequestData; - oAudioRequestData.nFlags = (Audio::eARF_PRIORITY_HIGH | Audio::eARF_EXECUTE_BLOCKING); - oAudioRequestData.pData = &oAMData; - Audio::AudioSystemRequestBus::Broadcast(&Audio::AudioSystemRequestBus::Events::PushRequestBlocking, oAudioRequestData); - - // Now unload level specific audio config data. - Audio::SAudioManagerRequestData oAMData2(Audio::eADS_LEVEL_SPECIFIC); - oAudioRequestData.pData = &oAMData2; - Audio::AudioSystemRequestBus::Broadcast(&Audio::AudioSystemRequestBus::Events::PushRequestBlocking, oAudioRequestData); - - Audio::SAudioManagerRequestData oAMData3(Audio::eADS_LEVEL_SPECIFIC); - oAudioRequestData.pData = &oAMData3; - Audio::AudioSystemRequestBus::Broadcast(&Audio::AudioSystemRequestBus::Events::PushRequestBlocking, oAudioRequestData); + LmbrCentral::AudioSystemComponentRequestBus::Broadcast(&LmbrCentral::AudioSystemComponentRequestBus::Events::LevelUnloadAudio); GetIEditor()->Notify(eNotify_OnSceneClosed); CrySystemEventBus::Broadcast(&CrySystemEventBus::Events::OnCryEditorSceneClosed); @@ -413,32 +398,11 @@ void CCryEditDoc::Load(TDocMultiArchive& arrXmlAr, const QString& szFilename) #ifdef PROFILE_LOADING_WITH_VTUNE VTResume(); #endif - // Parse level specific config data. - const char* controlsPath = nullptr; - Audio::AudioSystemRequestBus::BroadcastResult(controlsPath, &Audio::AudioSystemRequestBus::Events::GetControlsPath); - QString sAudioLevelPath(controlsPath); - sAudioLevelPath += "levels/"; - AZStd::string const sLevelNameOnly = PathUtil::GetFileName(fileName.toUtf8().data()); - sAudioLevelPath += sLevelNameOnly.c_str(); - QByteArray path = sAudioLevelPath.toUtf8(); - Audio::SAudioManagerRequestData oAMData(path, Audio::eADS_LEVEL_SPECIFIC); - Audio::SAudioRequest oAudioRequestData; - oAudioRequestData.nFlags = (Audio::eARF_PRIORITY_HIGH | Audio::eARF_EXECUTE_BLOCKING); // Needs to be blocking so data is available for next preloading request! - oAudioRequestData.pData = &oAMData; - Audio::AudioSystemRequestBus::Broadcast(&Audio::AudioSystemRequestBus::Events::PushRequestBlocking, oAudioRequestData); - - Audio::SAudioManagerRequestData oAMData2(path, Audio::eADS_LEVEL_SPECIFIC); - oAudioRequestData.pData = &oAMData2; - Audio::AudioSystemRequestBus::Broadcast(&Audio::AudioSystemRequestBus::Events::PushRequestBlocking, oAudioRequestData); - - Audio::TAudioPreloadRequestID nPreloadRequestID = INVALID_AUDIO_PRELOAD_REQUEST_ID; - Audio::AudioSystemRequestBus::BroadcastResult(nPreloadRequestID, &Audio::AudioSystemRequestBus::Events::GetAudioPreloadRequestID, sLevelNameOnly.c_str()); - if (nPreloadRequestID != INVALID_AUDIO_PRELOAD_REQUEST_ID) - { - Audio::SAudioManagerRequestData oAMData3(nPreloadRequestID); - oAudioRequestData.pData = &oAMData3; - Audio::AudioSystemRequestBus::Broadcast(&Audio::AudioSystemRequestBus::Events::PushRequestBlocking, oAudioRequestData); - } + // Load level-specific audio data. + AZStd::string levelFileName{ fileName.toUtf8().constData() }; + AZStd::to_lower(levelFileName.begin(), levelFileName.end()); + LmbrCentral::AudioSystemComponentRequestBus::Broadcast( + &LmbrCentral::AudioSystemComponentRequestBus::Events::LevelLoadAudio, AZStd::string_view{ levelFileName }); { CAutoLogTime logtime("Game Engine level load"); diff --git a/Code/Editor/MainWindow.cpp b/Code/Editor/MainWindow.cpp index bb246c2337..9560fd0fe7 100644 --- a/Code/Editor/MainWindow.cpp +++ b/Code/Editor/MainWindow.cpp @@ -97,6 +97,7 @@ AZ_POP_DISABLE_WARNING #include "ActionManager.h" #include +#include using namespace AZ; using namespace AzQtComponents; @@ -1474,25 +1475,22 @@ int MainWindow::ViewPaneVersion() const void MainWindow::OnStopAllSounds() { - Audio::SAudioRequest oStopAllSoundsRequest; - Audio::SAudioManagerRequestData oStopAllSoundsRequestData; - oStopAllSoundsRequest.pData = &oStopAllSoundsRequestData; - - CryLogAlways("