From 0fcd6e84ece985151153176482f2ad054da2d1e6 Mon Sep 17 00:00:00 2001 From: Terry Michaels Date: Tue, 8 Jun 2021 12:02:02 -0500 Subject: [PATCH 01/16] Added mechanism for viewpanes to request buttons on the main toolbar (#1189) --- .../AzToolsFramework/API/ViewPaneOptions.h | 3 +++ .../Sandbox/Editor/Core/LevelEditorMenuHandler.cpp | 11 +++++++++++ Code/Sandbox/Editor/MainWindow.cpp | 4 +++- Code/Sandbox/Editor/ToolbarManager.cpp | 14 ++++++++++++++ Code/Sandbox/Editor/ToolbarManager.h | 2 ++ 5 files changed, 33 insertions(+), 1 deletion(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/API/ViewPaneOptions.h b/Code/Framework/AzToolsFramework/AzToolsFramework/API/ViewPaneOptions.h index fb86968ebb..4891d2ef85 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/API/ViewPaneOptions.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/API/ViewPaneOptions.h @@ -42,6 +42,9 @@ namespace AzToolsFramework bool detachedWindow = false; ///< set to true if the view pane should use a detached, non-dockable widget. This is to workaround a problem with QOpenGLWidget on macOS. Currently this has no effect on other platforms. bool isDisabledInSimMode = false; ///< set to true if the view pane should not be openable from level editor menu when editor is in simulation mode. + + bool showOnToolsToolbar = false; ///< set to true if the view pane should create a button on the tools toolbar to open/close the pane + QString toolbarIcon; ///< path to the icon to use for the toolbar button - only used if showOnToolsToolbar is set to true }; } // namespace AzToolsFramework diff --git a/Code/Sandbox/Editor/Core/LevelEditorMenuHandler.cpp b/Code/Sandbox/Editor/Core/LevelEditorMenuHandler.cpp index 8f6e927a84..6292a99ec5 100644 --- a/Code/Sandbox/Editor/Core/LevelEditorMenuHandler.cpp +++ b/Code/Sandbox/Editor/Core/LevelEditorMenuHandler.cpp @@ -912,6 +912,12 @@ QAction* LevelEditorMenuHandler::CreateViewPaneAction(const QtViewPane* view) action = new QAction(menuText, this); action->setObjectName(view->m_name); action->setCheckable(true); + + if (view->m_options.showOnToolsToolbar) + { + action->setIcon(QIcon(view->m_options.toolbarIcon)); + } + m_actionManager->AddAction(view->m_id, action); if (!view->m_options.shortcut.isEmpty()) @@ -941,6 +947,11 @@ QAction* LevelEditorMenuHandler::CreateViewPaneMenuItem( menu->addAction(action); + if (view->m_options.showOnToolsToolbar) + { + m_mainWindow->GetToolbarManager()->AddButtonToEditToolbar(action); + } + return action; } diff --git a/Code/Sandbox/Editor/MainWindow.cpp b/Code/Sandbox/Editor/MainWindow.cpp index 31eac05824..3753c29064 100644 --- a/Code/Sandbox/Editor/MainWindow.cpp +++ b/Code/Sandbox/Editor/MainWindow.cpp @@ -470,9 +470,11 @@ void MainWindow::Initialize() InitToolActionHandlers(); + // Initialize toolbars before we setup the menu so that any tools can be added to the toolbar as needed + InitToolBars(); + m_levelEditorMenuHandler->Initialize(); - InitToolBars(); InitStatusBar(); AzToolsFramework::SourceControlNotificationBus::Handler::BusConnect(); diff --git a/Code/Sandbox/Editor/ToolbarManager.cpp b/Code/Sandbox/Editor/ToolbarManager.cpp index 241b969da7..137057a4fa 100644 --- a/Code/Sandbox/Editor/ToolbarManager.cpp +++ b/Code/Sandbox/Editor/ToolbarManager.cpp @@ -623,6 +623,20 @@ AmazonToolbar ToolbarManager::GetMiscToolbar() const return t; } +void ToolbarManager::AddButtonToEditToolbar(QAction* action) +{ + QString toolbarName = "EditMode"; + const AmazonToolbar* toolbar = FindToolbar(toolbarName); + + if (toolbar) + { + if (toolbar->Toolbar()) + { + toolbar->Toolbar()->addAction(action); + } + } +} + const AmazonToolbar* ToolbarManager::FindDefaultToolbar(const QString& toolbarName) const { for (const AmazonToolbar& toolbar : m_standardToolbars) diff --git a/Code/Sandbox/Editor/ToolbarManager.h b/Code/Sandbox/Editor/ToolbarManager.h index 70228636f5..1867316d01 100644 --- a/Code/Sandbox/Editor/ToolbarManager.h +++ b/Code/Sandbox/Editor/ToolbarManager.h @@ -169,6 +169,8 @@ public: AmazonToolbar GetMiscToolbar() const; AmazonToolbar GetPlayConsoleToolbar() const; + void AddButtonToEditToolbar(QAction* action); + private: Q_DISABLE_COPY(ToolbarManager); void SaveToolbars(); From 2d1e47793de79a98a7e7f9f0b84403ed424c55ef Mon Sep 17 00:00:00 2001 From: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com> Date: Tue, 8 Jun 2021 10:06:25 -0700 Subject: [PATCH 02/16] Move Duplicate menu items and shortcuts out of the Prefab Wip flag Make duplicate prefab workflows available by default in Prefab mode. --- .../EditorTransformComponentSelection.cpp | 51 ++++++++----------- .../Editor/Core/LevelEditorMenuHandler.cpp | 14 +---- .../SandboxIntegration.cpp | 15 ++---- 3 files changed, 26 insertions(+), 54 deletions(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorTransformComponentSelection.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorTransformComponentSelection.cpp index fee0267766..eadb870563 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorTransformComponentSelection.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorTransformComponentSelection.cpp @@ -2240,42 +2240,31 @@ namespace AzToolsFramework RegenerateManipulators(); }); - bool isPrefabSystemEnabled = false; - AzFramework::ApplicationRequests::Bus::BroadcastResult( - isPrefabSystemEnabled, &AzFramework::ApplicationRequests::IsPrefabSystemEnabled); - - bool prefabWipFeaturesEnabled = false; - AzFramework::ApplicationRequests::Bus::BroadcastResult( - prefabWipFeaturesEnabled, &AzFramework::ApplicationRequests::ArePrefabWipFeaturesEnabled); + // duplicate selection + AddAction( + m_actions, { QKeySequence(Qt::CTRL + Qt::Key_D) }, + /*ID_EDIT_CLONE =*/33525, s_duplicateTitle, s_duplicateDesc, + []() + { + AZ_PROFILE_FUNCTION(AZ::Debug::ProfileCategory::AzToolsFramework); - if (!isPrefabSystemEnabled || (isPrefabSystemEnabled && prefabWipFeaturesEnabled)) - { - // duplicate selection - AddAction( - m_actions, { QKeySequence(Qt::CTRL + Qt::Key_D) }, - /*ID_EDIT_CLONE =*/33525, s_duplicateTitle, s_duplicateDesc, - []() + // Clear Widget selection - Prevents issues caused by cloning entities while a property in the Reflected Property Editor + // is being edited. + if (QApplication::focusWidget()) { - AZ_PROFILE_FUNCTION(AZ::Debug::ProfileCategory::AzToolsFramework); - - // Clear Widget selection - Prevents issues caused by cloning entities while a property in the Reflected Property Editor - // is being edited. - if (QApplication::focusWidget()) - { - QApplication::focusWidget()->clearFocus(); - } + QApplication::focusWidget()->clearFocus(); + } - ScopedUndoBatch undoBatch(s_duplicateUndoRedoDesc); - auto selectionCommand = AZStd::make_unique(EntityIdList(), s_duplicateUndoRedoDesc); - selectionCommand->SetParent(undoBatch.GetUndoBatch()); - selectionCommand.release(); + ScopedUndoBatch undoBatch(s_duplicateUndoRedoDesc); + auto selectionCommand = AZStd::make_unique(EntityIdList(), s_duplicateUndoRedoDesc); + selectionCommand->SetParent(undoBatch.GetUndoBatch()); + selectionCommand.release(); - bool handled = false; - EditorRequestBus::Broadcast(&EditorRequests::CloneSelection, handled); + bool handled = false; + EditorRequestBus::Broadcast(&EditorRequests::CloneSelection, handled); - // selection update handled in AfterEntitySelectionChanged - }); - } + // selection update handled in AfterEntitySelectionChanged + }); // delete selection AddAction( diff --git a/Code/Sandbox/Editor/Core/LevelEditorMenuHandler.cpp b/Code/Sandbox/Editor/Core/LevelEditorMenuHandler.cpp index 6292a99ec5..39c7ae43fd 100644 --- a/Code/Sandbox/Editor/Core/LevelEditorMenuHandler.cpp +++ b/Code/Sandbox/Editor/Core/LevelEditorMenuHandler.cpp @@ -473,18 +473,8 @@ void LevelEditorMenuHandler::PopulateEditMenu(ActionManager::MenuWrapper& editMe // editMenu->addAction(ID_EDIT_PASTE); // editMenu.AddSeparator(); - bool isPrefabSystemEnabled = false; - AzFramework::ApplicationRequests::Bus::BroadcastResult(isPrefabSystemEnabled, &AzFramework::ApplicationRequests::IsPrefabSystemEnabled); - - bool prefabWipFeaturesEnabled = false; - AzFramework::ApplicationRequests::Bus::BroadcastResult( - prefabWipFeaturesEnabled, &AzFramework::ApplicationRequests::ArePrefabWipFeaturesEnabled); - - if (!isPrefabSystemEnabled || (isPrefabSystemEnabled && prefabWipFeaturesEnabled)) - { - // Duplicate - editMenu.AddAction(ID_EDIT_CLONE); - } + // Duplicate + editMenu.AddAction(ID_EDIT_CLONE); // Delete editMenu.AddAction(ID_EDIT_DELETE); diff --git a/Code/Sandbox/Plugins/ComponentEntityEditorPlugin/SandboxIntegration.cpp b/Code/Sandbox/Plugins/ComponentEntityEditorPlugin/SandboxIntegration.cpp index 8161d07547..694714cc6e 100644 --- a/Code/Sandbox/Plugins/ComponentEntityEditorPlugin/SandboxIntegration.cpp +++ b/Code/Sandbox/Plugins/ComponentEntityEditorPlugin/SandboxIntegration.cpp @@ -670,18 +670,11 @@ void SandboxIntegrationManager::PopulateEditorGlobalContextMenu(QMenu* menu, con AzToolsFramework::EditorContextMenuBus::Broadcast(&AzToolsFramework::EditorContextMenuEvents::PopulateEditorGlobalContextMenu, menu); } - bool prefabWipFeaturesEnabled = false; - AzFramework::ApplicationRequests::Bus::BroadcastResult( - prefabWipFeaturesEnabled, &AzFramework::ApplicationRequests::ArePrefabWipFeaturesEnabled); - - if (!prefabSystemEnabled || (prefabSystemEnabled && prefabWipFeaturesEnabled)) + action = menu->addAction(QObject::tr("Duplicate")); + QObject::connect(action, &QAction::triggered, action, [this] { ContextMenu_Duplicate(); }); + if (selected.size() == 0) { - action = menu->addAction(QObject::tr("Duplicate")); - QObject::connect(action, &QAction::triggered, action, [this] { ContextMenu_Duplicate(); }); - if (selected.size() == 0) - { - action->setDisabled(true); - } + action->setDisabled(true); } if (!prefabSystemEnabled) From 47e5c72f2e0a5e036ce367053e691fe4d2ebc00c Mon Sep 17 00:00:00 2001 From: amzn-sean <75276488+amzn-sean@users.noreply.github.com> Date: Tue, 8 Jun 2021 18:20:34 +0100 Subject: [PATCH 03/16] fixed missing methods in SC from Trigger and Collision events (#1185) --- .../Physics/Collision/CollisionEvents.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/Code/Framework/AzFramework/AzFramework/Physics/Collision/CollisionEvents.cpp b/Code/Framework/AzFramework/AzFramework/Physics/Collision/CollisionEvents.cpp index 4c9124f594..9f9e5c51ad 100644 --- a/Code/Framework/AzFramework/AzFramework/Physics/Collision/CollisionEvents.cpp +++ b/Code/Framework/AzFramework/AzFramework/Physics/Collision/CollisionEvents.cpp @@ -37,9 +37,10 @@ namespace AzPhysics if (auto* behaviorContext = azdynamic_cast(context)) { behaviorContext->Class() - ->Attribute(AZ::Script::Attributes::ExcludeFrom, AZ::Script::Attributes::ExcludeFlags::All) - ->Method("GetTriggerEntityId", &TriggerEvent::GetTriggerEntityId) - ->Method("GetOtherEntityId", &TriggerEvent::GetOtherEntityId) + ->Attribute(AZ::Script::Attributes::Module, "physics") + ->Attribute(AZ::Script::Attributes::Category, "Physics") + ->Method("Get Trigger EntityId", &TriggerEvent::GetTriggerEntityId) + ->Method("Get Other EntityId", &TriggerEvent::GetOtherEntityId) ; } } @@ -104,10 +105,11 @@ namespace AzPhysics if (auto* behaviorContext = azdynamic_cast(context)) { behaviorContext->Class() - ->Attribute(AZ::Script::Attributes::ExcludeFrom, AZ::Script::Attributes::ExcludeFlags::All) - ->Property("Contacts", BehaviorValueProperty(&CollisionEvent::m_contacts)) - ->Method("GetBody1EntityId", &CollisionEvent::GetBody1EntityId) - ->Method("GetBody2EntityId", &CollisionEvent::GetBody2EntityId) + ->Attribute(AZ::Script::Attributes::Module, "physics") + ->Attribute(AZ::Script::Attributes::Category, "Physics") + ->Property("Contacts", BehaviorValueGetter(&CollisionEvent::m_contacts), nullptr) + ->Method("Get Body 1 EntityId", &CollisionEvent::GetBody1EntityId) + ->Method("Get Body 2 EntityId", &CollisionEvent::GetBody2EntityId) ; } } From 80f62d0523d61a401e37c1e66c09f57c680f9cd5 Mon Sep 17 00:00:00 2001 From: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com> Date: Tue, 8 Jun 2021 10:44:20 -0700 Subject: [PATCH 04/16] LYN-3708 | Optimize Prefab instance propagation to stabilize UX (#700) * Add instanceToIgnore to calls leading to instances being added to the queue for propagation. * Change PrefabUndoEntityUpdate to make it so that the instance triggering the prefab template change is not reloaded on propagation, since it will already be up to date due to the way we generated the patch to begin with. * Add FindPrefabDomValue utility function for paths * Expose the level root prefab template id in the Prefab EOS Interface * Fix Instance Alias Path generation to work with the new FindValueInPrefabDom function * Stop reloading ancestors on propagation, and fix instance reloading so that the level dom is used (and overrides are preserved) * Remove commented out code, refactor FindPrefabDomValue for paths (was handling an edge case incorrectly, and it's not even triggered) * Fix issue with PathView reference - with PathView already being a reference, this resulted in a copy and triggered a warning during automated review builds. * Additional fix to the build warning, remove redundant error message * Revert changes to Instance::GetAbsoluteInstanceAliasPath(), as they were impacting serialization. * Remove the dependency to the level root prefab template in the propagation code, climb up the hierarchy instead. This allows tests to work despite not using the EOS properly. Also use PrefabDomPaths to retrieve the instance dom from the root dom instead of iterating. * Remove now unused PrefabDomUtils function, extend optimization to link updates. * Trigger a full instance propagation to correctly refresh alias references. This is an issue in the test because some operations are called from the backend API and will not trigger propagation properly. Tests will soon be rewritten to more properly represent frontend workflows. * Fixes lingering issues with propagation: - Restores code that fixes the selection if entityIds have changed; - Fixes Do() function on link update. Prefab containers will propagate correctly while still being stable during editing. * Remove GetRootPrefabInstanceTemplateId (no longer necessary after the code has been rewritten) * Fix optimization code to account for instances being removed and propagation being run out of order in Create Prefab undo. * Renamed variable, added comments for clarity. * Restore asserts on instance not being found; Rename Do to Redo for clarity; Add comments. * Fixed incomplete comment. --- .../PrefabEditorEntityOwnershipService.h | 2 +- .../Instance/InstanceToTemplateInterface.h | 9 +- .../Instance/InstanceToTemplatePropagator.cpp | 4 +- .../Instance/InstanceToTemplatePropagator.h | 2 +- .../Instance/InstanceUpdateExecutor.cpp | 89 ++++++++++++++----- .../Prefab/Instance/InstanceUpdateExecutor.h | 2 +- .../InstanceUpdateExecutorInterface.h | 2 +- .../Prefab/PrefabPublicHandler.cpp | 24 ++--- .../Prefab/PrefabPublicHandler.h | 6 +- .../Prefab/PrefabSystemComponent.cpp | 14 ++- .../Prefab/PrefabSystemComponent.h | 4 +- .../Prefab/PrefabSystemComponentInterface.h | 2 +- .../AzToolsFramework/Prefab/PrefabUndo.cpp | 26 ++++-- .../AzToolsFramework/Prefab/PrefabUndo.h | 9 +- .../Tests/Prefab/PrefabEntityAliasTests.cpp | 1 + 15 files changed, 135 insertions(+), 61 deletions(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.h index d8eb81dd40..d8bc63cfc6 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.h @@ -197,7 +197,7 @@ namespace AzToolsFramework AZ::IO::PathView filePath, Prefab::InstanceOptionalReference instanceToParentUnder) override; Prefab::InstanceOptionalReference GetRootPrefabInstance() override; - + const AZStd::vector>& GetPlayInEditorAssetData() override; ////////////////////////////////////////////////////////////////////////// diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplateInterface.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplateInterface.h index c2ddbcf24f..c9b4b5acc3 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplateInterface.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplateInterface.h @@ -47,8 +47,13 @@ namespace AzToolsFramework virtual void AppendEntityAliasToPatchPaths(PrefabDom& providedPatch, const AZ::EntityId& entityId) = 0; - //! Updates the template links (updating instances) for the given templateId using the providedPatch - virtual bool PatchTemplate(PrefabDomValue& providedPatch, TemplateId templateId) = 0; + //! Updates the template links (updating instances) for the given template and triggers propagation on its instances. + //! @param providedPatch The patch to apply to the template. + //! @param templateId The id of the template to update. + //! @param instanceToExclude An optional reference to an instance of the template being updated that should not be refreshes as part of propagation. + //! Defaults to nullopt, which means that all instances will be refreshed. + //! @return True if the template was patched correctly, false if the operation failed. + virtual bool PatchTemplate(PrefabDomValue& providedPatch, TemplateId templateId, InstanceOptionalReference instanceToExclude = AZStd::nullopt) = 0; virtual void ApplyPatchesToInstance(const AZ::EntityId& entityId, PrefabDom& patches, const Instance& instanceToAddPatches) = 0; diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplatePropagator.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplatePropagator.cpp index 6d3ddedd51..9fb6293b74 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplatePropagator.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplatePropagator.cpp @@ -172,7 +172,7 @@ namespace AzToolsFramework } } - bool InstanceToTemplatePropagator::PatchTemplate(PrefabDomValue& providedPatch, TemplateId templateId) + bool InstanceToTemplatePropagator::PatchTemplate(PrefabDomValue& providedPatch, TemplateId templateId, InstanceOptionalReference instanceToExclude) { PrefabDom& templateDomReference = m_prefabSystemComponentInterface->FindTemplateDom(templateId); @@ -184,7 +184,7 @@ namespace AzToolsFramework if (result.GetOutcome() == AZ::JsonSerializationResult::Outcomes::Success) { m_prefabSystemComponentInterface->SetTemplateDirtyFlag(templateId, true); - m_prefabSystemComponentInterface->PropagateTemplateChanges(templateId); + m_prefabSystemComponentInterface->PropagateTemplateChanges(templateId, instanceToExclude); return true; } else diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplatePropagator.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplatePropagator.h index 9a6aad8ac1..358494091d 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplatePropagator.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplatePropagator.h @@ -37,7 +37,7 @@ namespace AzToolsFramework InstanceOptionalReference GetTopMostInstanceInHierarchy(AZ::EntityId entityId); - bool PatchTemplate(PrefabDomValue& providedPatch, TemplateId templateId) override; + bool PatchTemplate(PrefabDomValue& providedPatch, TemplateId templateId, InstanceOptionalReference instanceToExclude = AZStd::nullopt) override; void ApplyPatchesToInstance(const AZ::EntityId& entityId, PrefabDom& patches, const Instance& instanceToAddPatches) override; diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceUpdateExecutor.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceUpdateExecutor.cpp index 6194adf784..b7a81a7f0c 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceUpdateExecutor.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceUpdateExecutor.cpp @@ -56,7 +56,7 @@ namespace AzToolsFramework AZ::Interface::Unregister(this); } - void InstanceUpdateExecutor::AddTemplateInstancesToQueue(TemplateId instanceTemplateId) + void InstanceUpdateExecutor::AddTemplateInstancesToQueue(TemplateId instanceTemplateId, InstanceOptionalReference instanceToExclude) { auto findInstancesResult = m_templateInstanceMapperInterface->FindInstancesOwnedByTemplate(instanceTemplateId); @@ -70,9 +70,18 @@ namespace AzToolsFramework return; } + Instance* instanceToExcludePtr = nullptr; + if (instanceToExclude.has_value()) + { + instanceToExcludePtr = &(instanceToExclude->get()); + } + for (auto instance : findInstancesResult->get()) { - m_instancesUpdateQueue.emplace_back(instance); + if (instance != instanceToExcludePtr) + { + m_instancesUpdateQueue.emplace_back(instance); + } } } @@ -103,7 +112,7 @@ namespace AzToolsFramework EntityIdList selectedEntityIds; ToolsApplicationRequestBus::BroadcastResult(selectedEntityIds, &ToolsApplicationRequests::GetSelectedEntities); - ToolsApplicationRequestBus::Broadcast(&ToolsApplicationRequests::SetSelectedEntities, EntityIdList()); + PrefabDom instanceDomFromRootDocument; // Process all instances in the queue, capped to the batch size. // Even though we potentially initialized the batch size to the queue, it's possible for the queue size to shrink @@ -148,13 +157,62 @@ namespace AzToolsFramework continue; } - Template& currentTemplate = currentTemplateReference->get(); Instance::EntityList newEntities; - if (PrefabDomUtils::LoadInstanceFromPrefabDom(*instanceToUpdate, newEntities, currentTemplate.GetPrefabDom())) + + // Climb up to the root of the instance hierarchy from this instance + InstanceOptionalConstReference rootInstance = *instanceToUpdate; + AZStd::vector pathOfInstances; + + while (rootInstance->get().GetParentInstance() != AZStd::nullopt) + { + pathOfInstances.emplace_back(rootInstance); + rootInstance = rootInstance->get().GetParentInstance(); + } + + AZStd::string aliasPathResult = ""; + for (auto instanceIter = pathOfInstances.rbegin(); instanceIter != pathOfInstances.rend(); ++instanceIter) + { + aliasPathResult.append("/Instances/"); + aliasPathResult.append((*instanceIter)->get().GetInstanceAlias()); + } + + PrefabDomPath rootPrefabDomPath(aliasPathResult.c_str()); + + PrefabDom& rootPrefabTemplateDom = + m_prefabSystemComponentInterface->FindTemplateDom(rootInstance->get().GetTemplateId()); + + auto instanceDomFromRootValue = rootPrefabDomPath.Get(rootPrefabTemplateDom); + if (!instanceDomFromRootValue) + { + AZ_Assert( + false, + "InstanceUpdateExecutor::UpdateTemplateInstancesInQueue - " + "Could not load Instance DOM from the top level ancestor's DOM."); + + isUpdateSuccessful = false; + continue; + } + + PrefabDomValueReference instanceDomFromRoot = *instanceDomFromRootValue; + if (!instanceDomFromRoot.has_value()) + { + AZ_Assert( + false, + "InstanceUpdateExecutor::UpdateTemplateInstancesInQueue - " + "Could not load Instance DOM from the top level ancestor's DOM."); + + isUpdateSuccessful = false; + continue; + } + + // If a link was created for a nested instance before the changes were propagated, + // then we associate it correctly here + instanceDomFromRootDocument.CopyFrom(instanceDomFromRoot->get(), instanceDomFromRootDocument.GetAllocator()); + if (PrefabDomUtils::LoadInstanceFromPrefabDom(*instanceToUpdate, newEntities, instanceDomFromRootDocument)) { - // If a link was created for a nested instance before the changes were propagated, - // then we associate it correctly here - instanceToUpdate->GetNestedInstances([&](AZStd::unique_ptr& nestedInstance) { + Template& currentTemplate = currentTemplateReference->get(); + instanceToUpdate->GetNestedInstances([&](AZStd::unique_ptr& nestedInstance) + { if (nestedInstance->GetLinkId() != InvalidLinkId) { return; @@ -179,22 +237,11 @@ namespace AzToolsFramework AzToolsFramework::EditorEntityContextRequestBus::Broadcast( &AzToolsFramework::EditorEntityContextRequests::HandleEntitiesAdded, newEntities); } - else - { - AZ_Error( - "Prefab", false, - "InstanceUpdateExecutor::UpdateTemplateInstancesInQueue - " - "Could not load Instance from Prefab DOM of Template with Id '%llu' on file path '%s'.", - currentTemplateId, currentTemplate.GetFilePath().c_str()); - - isUpdateSuccessful = false; - } } - for (auto entityIdIterator = selectedEntityIds.begin(); entityIdIterator != selectedEntityIds.end(); entityIdIterator++) { - // Since entities get recreated during propagation, we need to check whether the entities correspoding to the list - // of selected entity ids are present or not. + // Since entities get recreated during propagation, we need to check whether the entities + // corresponding to the list of selected entity ids are present or not. AZ::Entity* entity = GetEntityById(*entityIdIterator); if (entity == nullptr) { diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceUpdateExecutor.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceUpdateExecutor.h index fa13c34b98..a3fdd019c2 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceUpdateExecutor.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceUpdateExecutor.h @@ -35,7 +35,7 @@ namespace AzToolsFramework explicit InstanceUpdateExecutor(int instanceCountToUpdateInBatch = 0); - void AddTemplateInstancesToQueue(TemplateId instanceTemplateId) override; + void AddTemplateInstancesToQueue(TemplateId instanceTemplateId, InstanceOptionalReference instanceToExclude = AZStd::nullopt) override; bool UpdateTemplateInstancesInQueue() override; virtual void RemoveTemplateInstanceFromQueue(const Instance* instance) override; diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceUpdateExecutorInterface.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceUpdateExecutorInterface.h index d794c4929d..2454a995cf 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceUpdateExecutorInterface.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceUpdateExecutorInterface.h @@ -27,7 +27,7 @@ namespace AzToolsFramework virtual ~InstanceUpdateExecutorInterface() = default; // Add all Instances of Template with given Id into a queue for updating them later. - virtual void AddTemplateInstancesToQueue(TemplateId instanceTemplateId) = 0; + virtual void AddTemplateInstancesToQueue(TemplateId instanceTemplateId, InstanceOptionalReference instanceToExclude = AZStd::nullopt) = 0; // Update Instances in the waiting queue. virtual bool UpdateTemplateInstancesInQueue() = 0; diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp index 4656dcf48f..fcdbc5ce07 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp @@ -242,11 +242,13 @@ namespace AzToolsFramework m_instanceToTemplateInterface->GenerateDomForEntity(containerAfterReset, *containerEntity); // Update the state of the entity - PrefabUndoEntityUpdate* state = aznew PrefabUndoEntityUpdate(AZStd::to_string(static_cast(containerEntityId))); - state->SetParent(undoBatch.GetUndoBatch()); - state->Capture(containerBeforeReset, containerAfterReset, containerEntityId); + auto templateId = instanceToCreate->get().GetTemplateId(); - state->Redo(); + PrefabDom transformPatch; + m_instanceToTemplateInterface->GeneratePatch(transformPatch, containerBeforeReset, containerAfterReset); + m_instanceToTemplateInterface->AppendEntityAliasToPatchPaths(transformPatch, containerEntityId); + + m_instanceToTemplateInterface->PatchTemplate(transformPatch, templateId); } // This clears any entities marked as dirty due to reparenting of entities during the process of creating a prefab. @@ -661,12 +663,12 @@ namespace AzToolsFramework else { Internal_HandleContainerOverride( - parentUndoBatch, entityId, patch, owningInstance->get().GetLinkId()); + parentUndoBatch, entityId, patch, owningInstance->get().GetLinkId(), owningInstance->get().GetParentInstance()); } } else { - Internal_HandleEntityChange(parentUndoBatch, entityId, beforeState, afterState); + Internal_HandleEntityChange(parentUndoBatch, entityId, beforeState, afterState, owningInstance); if (isNewParentOwnedByDifferentInstance) { @@ -679,25 +681,27 @@ namespace AzToolsFramework } void PrefabPublicHandler::Internal_HandleContainerOverride( - UndoSystem::URSequencePoint* undoBatch, AZ::EntityId entityId, const PrefabDom& patch, const LinkId linkId) + UndoSystem::URSequencePoint* undoBatch, AZ::EntityId entityId, const PrefabDom& patch, + const LinkId linkId, InstanceOptionalReference parentInstance) { // Save these changes as patches to the link PrefabUndoLinkUpdate* linkUpdate = aznew PrefabUndoLinkUpdate(AZStd::to_string(static_cast(entityId))); linkUpdate->SetParent(undoBatch); linkUpdate->Capture(patch, linkId); - linkUpdate->Redo(); + linkUpdate->Redo(parentInstance); } void PrefabPublicHandler::Internal_HandleEntityChange( - UndoSystem::URSequencePoint* undoBatch, AZ::EntityId entityId, PrefabDom& beforeState, PrefabDom& afterState) + UndoSystem::URSequencePoint* undoBatch, AZ::EntityId entityId, PrefabDom& beforeState, + PrefabDom& afterState, InstanceOptionalReference instance) { // Update the state of the entity PrefabUndoEntityUpdate* state = aznew PrefabUndoEntityUpdate(AZStd::to_string(static_cast(entityId))); state->SetParent(undoBatch); state->Capture(beforeState, afterState, entityId); - state->Redo(); + state->Redo(instance); } void PrefabPublicHandler::Internal_HandleInstanceChange( diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h index 167791d1c1..f3d778d8de 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h @@ -162,9 +162,11 @@ namespace AzToolsFramework InstanceOptionalConstReference instance, const AZStd::unordered_set& templateSourcePaths); static void Internal_HandleContainerOverride( - UndoSystem::URSequencePoint* undoBatch, AZ::EntityId entityId, const PrefabDom& patch, const LinkId linkId); + UndoSystem::URSequencePoint* undoBatch, AZ::EntityId entityId, const PrefabDom& patch, + const LinkId linkId, InstanceOptionalReference parentInstance = AZStd::nullopt); static void Internal_HandleEntityChange( - UndoSystem::URSequencePoint* undoBatch, AZ::EntityId entityId, PrefabDom& beforeState, PrefabDom& afterState); + UndoSystem::URSequencePoint* undoBatch, AZ::EntityId entityId, PrefabDom& beforeState, + PrefabDom& afterState, InstanceOptionalReference instance = AZStd::nullopt); void Internal_HandleInstanceChange(UndoSystem::URSequencePoint* undoBatch, AZ::Entity* entity, AZ::EntityId beforeParentId, AZ::EntityId afterParentId); void UpdateLinkPatchesWithNewEntityAliases( diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.cpp index 5f5564b4e1..ab77d53283 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.cpp @@ -141,8 +141,10 @@ namespace AzToolsFramework return newInstance; } - void PrefabSystemComponent::PropagateTemplateChanges(TemplateId templateId) + void PrefabSystemComponent::PropagateTemplateChanges(TemplateId templateId, InstanceOptionalReference instanceToExclude) { + UpdatePrefabInstances(templateId, instanceToExclude); + auto templateIdToLinkIdsIterator = m_templateToLinkIdsMap.find(templateId); if (templateIdToLinkIdsIterator != m_templateToLinkIdsMap.end()) { @@ -153,10 +155,6 @@ namespace AzToolsFramework templateIdToLinkIdsIterator->second.end())); UpdateLinkedInstances(linkIdsToUpdateQueue); } - else - { - UpdatePrefabInstances(templateId); - } } void PrefabSystemComponent::UpdatePrefabTemplate(TemplateId templateId, const PrefabDom& updatedDom) @@ -174,9 +172,9 @@ namespace AzToolsFramework } } - void PrefabSystemComponent::UpdatePrefabInstances(const TemplateId& templateId) + void PrefabSystemComponent::UpdatePrefabInstances(const TemplateId& templateId, InstanceOptionalReference instanceToExclude) { - m_instanceUpdateExecutor.AddTemplateInstancesToQueue(templateId); + m_instanceUpdateExecutor.AddTemplateInstancesToQueue(templateId, instanceToExclude); } void PrefabSystemComponent::UpdateLinkedInstances(AZStd::queue& linkIdsQueue) @@ -250,8 +248,6 @@ namespace AzToolsFramework if (targetTemplateIdToLinkIdMap[targetTemplateId].first.empty() && targetTemplateIdToLinkIdMap[targetTemplateId].second) { - UpdatePrefabInstances(targetTemplateId); - auto templateToLinkIter = m_templateToLinkIdsMap.find(targetTemplateId); if (templateToLinkIter != m_templateToLinkIdsMap.end()) { diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.h index 0a9a450f64..a5170b8eef 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.h @@ -215,14 +215,14 @@ namespace AzToolsFramework */ void UpdatePrefabTemplate(TemplateId templateId, const PrefabDom& updatedDom) override; - void PropagateTemplateChanges(TemplateId templateId) override; + void PropagateTemplateChanges(TemplateId templateId, InstanceOptionalReference instanceToExclude = AZStd::nullopt) override; /** * Updates all Instances owned by a Template. * * @param templateId The id of the Template owning Instances to update. */ - void UpdatePrefabInstances(const TemplateId& templateId); + void UpdatePrefabInstances(const TemplateId& templateId, InstanceOptionalReference instanceToExclude = AZStd::nullopt); private: AZ_DISABLE_COPY_MOVE(PrefabSystemComponent); diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponentInterface.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponentInterface.h index f47941254a..8daf4e731b 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponentInterface.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponentInterface.h @@ -56,7 +56,7 @@ namespace AzToolsFramework virtual PrefabDom& FindTemplateDom(TemplateId templateId) = 0; virtual void UpdatePrefabTemplate(TemplateId templateId, const PrefabDom& updatedDom) = 0; - virtual void PropagateTemplateChanges(TemplateId templateId) = 0; + virtual void PropagateTemplateChanges(TemplateId templateId, InstanceOptionalReference instanceToExclude = AZStd::nullopt) = 0; virtual AZStd::unique_ptr InstantiatePrefab(AZ::IO::PathView filePath) = 0; virtual AZStd::unique_ptr InstantiatePrefab(const TemplateId& templateId) = 0; diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndo.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndo.cpp index aadcdcdea0..d0b3426495 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndo.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndo.cpp @@ -70,10 +70,10 @@ namespace AzToolsFramework const AZ::EntityId& entityId) { //get the entity alias for future undo/redo - InstanceOptionalReference instanceOptionalReference = m_instanceEntityMapperInterface->FindOwningInstance(entityId); - AZ_Error("Prefab", instanceOptionalReference, + auto instanceReference = m_instanceEntityMapperInterface->FindOwningInstance(entityId); + AZ_Error("Prefab", instanceReference, "Failed to find an owning instance for the entity with id %llu.", static_cast(entityId)); - Instance& instance = instanceOptionalReference->get(); + Instance& instance = instanceReference->get(); m_templateId = instance.GetTemplateId(); m_entityAlias = (instance.GetEntityAlias(entityId)).value(); @@ -106,6 +106,17 @@ namespace AzToolsFramework m_templateId); } + void PrefabUndoEntityUpdate::Redo(InstanceOptionalReference instanceToExclude) + { + [[maybe_unused]] bool isPatchApplicationSuccessful = + m_instanceToTemplateInterface->PatchTemplate(m_redoPatch, m_templateId, instanceToExclude); + + AZ_Error( + "Prefab", isPatchApplicationSuccessful, + "Applying the patch on the entity with alias '%s' in template with id '%llu' was unsuccessful", m_entityAlias.c_str(), + m_templateId); + } + //PrefabInstanceLinkUndo PrefabUndoInstanceLink::PrefabUndoInstanceLink(const AZStd::string& undoOperationName) : PrefabUndoBase(undoOperationName) @@ -290,7 +301,12 @@ namespace AzToolsFramework UpdateLink(m_linkDomNext); } - void PrefabUndoLinkUpdate::UpdateLink(PrefabDom& linkDom) + void PrefabUndoLinkUpdate::Redo(InstanceOptionalReference instanceToExclude) + { + UpdateLink(m_linkDomNext, instanceToExclude); + } + + void PrefabUndoLinkUpdate::UpdateLink(PrefabDom& linkDom, InstanceOptionalReference instanceToExclude) { LinkReference link = m_prefabSystemComponentInterface->FindLink(m_linkId); @@ -304,7 +320,7 @@ namespace AzToolsFramework //propagate the link changes link->get().UpdateTarget(); - m_prefabSystemComponentInterface->PropagateTemplateChanges(link->get().GetTargetTemplateId()); + m_prefabSystemComponentInterface->PropagateTemplateChanges(link->get().GetTargetTemplateId(), instanceToExclude); //mark as dirty m_prefabSystemComponentInterface->SetTemplateDirtyFlag(link->get().GetTargetTemplateId(), true); diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndo.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndo.h index 33d9e5ad33..7ae677571a 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndo.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndo.h @@ -71,11 +71,12 @@ namespace AzToolsFramework void Capture( PrefabDom& initialState, - PrefabDom& endState, - const AZ::EntityId& entity); + PrefabDom& endState, const AZ::EntityId& entity); void Undo() override; void Redo() override; + //! Overload to allow to apply the change, but prevent instanceToExclude from being refreshed. + void Redo(InstanceOptionalReference instanceToExclude); private: InstanceEntityMapperInterface* m_instanceEntityMapperInterface = nullptr; @@ -139,9 +140,11 @@ namespace AzToolsFramework void Undo() override; void Redo() override; + //! Overload to allow to apply the change, but prevent instanceToExclude from being refreshed. + void Redo(InstanceOptionalReference instanceToExclude); private: - void UpdateLink(PrefabDom& linkDom); + void UpdateLink(PrefabDom& linkDom, InstanceOptionalReference instanceToExclude = AZStd::nullopt); LinkId m_linkId; PrefabDom m_linkDomNext; //data for delete/update diff --git a/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabEntityAliasTests.cpp b/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabEntityAliasTests.cpp index 0cf39a3572..458625fa3f 100644 --- a/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabEntityAliasTests.cpp +++ b/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabEntityAliasTests.cpp @@ -242,6 +242,7 @@ namespace UnitTest // Patch the nested prefab to reference an entity in its parent ASSERT_TRUE(m_instanceToTemplateInterface->PatchEntityInTemplate(patch, newEntity->GetId())); + m_instanceUpdateExecutorInterface->AddTemplateInstancesToQueue(rootInstance->GetTemplateId()); m_instanceUpdateExecutorInterface->UpdateTemplateInstancesInQueue(); // Using the aliases we saved grab the updated entities so we can verify the entity reference is still preserved From dd95d2b02e4a65c460de95f004950d426d742330 Mon Sep 17 00:00:00 2001 From: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com> Date: Tue, 8 Jun 2021 19:44:33 +0100 Subject: [PATCH 05/16] ensure brute force ray intersection works (#1170) * ensure brute force ray intersection works in the same space as kd-tree intersection * add additional tests for ray casts against meshes using brute force approach * update api and add some additional test cases * comment tidy-up and other small updates/fixes for ray intersection code * fix issue with values at the end of a ray --- Gems/Atom/RPI/Code/CMakeLists.txt | 1 + .../Include/Atom/RPI.Public/Model/Model.h | 33 +- .../Atom/RPI.Reflect/Model/ModelAsset.h | 26 +- .../Code/Source/RPI.Public/Model/Model.cpp | 21 +- .../Source/RPI.Reflect/Model/ModelAsset.cpp | 51 ++-- .../Source/RPI.Reflect/Model/ModelKdTree.cpp | 6 +- Gems/Atom/RPI/Code/Tests/Model/ModelTests.cpp | 286 ++++++++++++------ 7 files changed, 280 insertions(+), 144 deletions(-) diff --git a/Gems/Atom/RPI/Code/CMakeLists.txt b/Gems/Atom/RPI/Code/CMakeLists.txt index 2898967add..8a2684347e 100644 --- a/Gems/Atom/RPI/Code/CMakeLists.txt +++ b/Gems/Atom/RPI/Code/CMakeLists.txt @@ -150,6 +150,7 @@ if(PAL_TRAIT_BUILD_TESTS_SUPPORTED AND PAL_TRAIT_BUILD_HOST_TOOLS) PRIVATE AZ::AtomCore AZ::AzTest + AZ::AzTestShared AZ::AzFramework AZ::AzToolsFramework Legacy::CryCommon diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Model/Model.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Model/Model.h index 35af200759..514e3e37a5 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Model/Model.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Model/Model.h @@ -61,12 +61,13 @@ namespace AZ //! Important: only to be used in the Editor, it may kick off a job to calculate spatial information. //! [GFX TODO][ATOM-4343 Bake mesh spatial during AP processing] //! - //! @param rayStart position where the ray starts - //! @param dir direction where the ray ends (does not have to be unit length) - //! @param distanceFactor if an intersection is detected, this will be set such that distanceFactor * dir.length == distance to intersection - //! @param normal if an intersection is detected, this will be set to the normal at the point of intersection - //! @return true if the ray intersects the mesh - bool LocalRayIntersection(const AZ::Vector3& rayStart, const AZ::Vector3& dir, float& distanceFactor, AZ::Vector3& normal) const; + //! @param rayStart The starting point of the ray. + //! @param rayDir The direction and length of the ray (magnitude is encoded in the direction). + //! @param[out] distanceNormalized If an intersection is found, will be set to the normalized distance of the intersection + //! (in the range 0.0-1.0) - to calculate the actual distance, multiply distanceNormalized by the magnitude of rayDir. + //! @param[out] normal If an intersection is found, will be set to the normal at the point of collision. + //! @return True if the ray intersects the mesh. + bool LocalRayIntersection(const AZ::Vector3& rayStart, const AZ::Vector3& rayDir, float& distanceNormalized, AZ::Vector3& normal) const; //! Checks a ray for intersection against this model, where the ray is in a different coordinate space. //! Important: only to be used in the Editor, it may kick off a job to calculate spatial information. @@ -74,13 +75,19 @@ namespace AZ //! //! @param modelTransform a transform that puts the model into the ray's coordinate space //! @param nonUniformScale Non-uniform scale applied in the model's local frame. - //! @param rayStart position where the ray starts - //! @param dir direction where the ray ends (does not have to be unit length) - //! @param distanceFactor if an intersection is detected, this will be set such that distanceFactor * dir.length == distance to intersection - //! @param normal if an intersection is detected, this will be set to the normal at the point of intersection - //! @return true if the ray intersects the mesh - bool RayIntersection(const AZ::Transform& modelTransform, const AZ::Vector3& nonUniformScale, const AZ::Vector3& rayStart, - const AZ::Vector3& dir, float& distanceFactor, AZ::Vector3& normal) const; + //! @param rayStart The starting point of the ray. + //! @param rayDir The direction and length of the ray (magnitude is encoded in the direction). + //! @param[out] distanceNormalized If an intersection is found, will be set to the normalized distance of the intersection + //! (in the range 0.0-1.0) - to calculate the actual distance, multiply distanceNormalized by the magnitude of rayDir. + //! @param[out] normal If an intersection is found, will be set to the normal at the point of collision. + //! @return True if the ray intersects the mesh. + bool RayIntersection( + const AZ::Transform& modelTransform, + const AZ::Vector3& nonUniformScale, + const AZ::Vector3& rayStart, + const AZ::Vector3& rayDir, + float& distanceNormalized, + AZ::Vector3& normal) const; //! Get available UV names from the model and its lods. const AZStd::unordered_set& GetUvNames() const; diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Model/ModelAsset.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Model/ModelAsset.h index 8a773dc29e..f3da349195 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Model/ModelAsset.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Model/ModelAsset.h @@ -63,12 +63,14 @@ namespace AZ //! Important: only to be used in the Editor, it may kick off a job to calculate spatial information. //! [GFX TODO][ATOM-4343 Bake mesh spatial information during AP processing] //! - //! @param rayStart position where the ray starts - //! @param dir direction where the ray ends (does not have to be unit length) - //! @param distance if an intersection is detected, this will be set such that distanceFactor * dir.length == distance to intersection - //! @param normal if an intersection is detected, this will be set to the normal at the point of collision - //! @return true if the ray intersects the mesh - virtual bool LocalRayIntersectionAgainstModel(const AZ::Vector3& rayStart, const AZ::Vector3& dir, float& distance, AZ::Vector3& normal) const; + //! @param rayStart The starting point of the ray. + //! @param rayDir The direction and length of the ray (magnitude is encoded in the direction). + //! @param[out] distanceNormalized If an intersection is found, will be set to the normalized distance of the intersection + //! (in the range 0.0-1.0) - to calculate the actual distance, multiply distanceNormalized by the magnitude of rayDir. + //! @param[out] normal If an intersection is found, will be set to the normal at the point of collision. + //! @return True if the ray intersects the mesh. + virtual bool LocalRayIntersectionAgainstModel( + const AZ::Vector3& rayStart, const AZ::Vector3& rayDir, float& distanceNormalized, AZ::Vector3& normal) const; private: void SetReady(); @@ -79,9 +81,15 @@ namespace AZ // mutable method void BuildKdTree() const; - bool BruteForceRayIntersect(const AZ::Vector3& rayStart, const AZ::Vector3& dir, float& distance, AZ::Vector3& normal) const; - - bool LocalRayIntersectionAgainstMesh(const ModelLodAsset::Mesh& mesh, const AZ::Vector3& rayStart, const AZ::Vector3& dir, float& distance, AZ::Vector3& normal) const; + bool BruteForceRayIntersect( + const AZ::Vector3& rayStart, const AZ::Vector3& rayDir, float& distanceNormalized, AZ::Vector3& normal) const; + + bool LocalRayIntersectionAgainstMesh( + const ModelLodAsset::Mesh& mesh, + const AZ::Vector3& rayStart, + const AZ::Vector3& rayDir, + float& distanceNormalized, + AZ::Vector3& normal) const; // Various model information used in raycasting AZ::Name m_positionName{ "POSITION" }; diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Model/Model.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Model/Model.cpp index 86477bf785..17ff2c64c8 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Model/Model.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Model/Model.cpp @@ -137,12 +137,12 @@ namespace AZ return m_modelAsset; } - bool Model::LocalRayIntersection(const AZ::Vector3& rayStart, const AZ::Vector3& dir, float& distance, AZ::Vector3& normal) const + bool Model::LocalRayIntersection(const AZ::Vector3& rayStart, const AZ::Vector3& rayDir, float& distanceNormalized, AZ::Vector3& normal) const { AZ_PROFILE_FUNCTION(Debug::ProfileCategory::AzRender); float start; float end; - const int result = Intersect::IntersectRayAABB2(rayStart, dir.GetReciprocal(), m_aabb, start, end); + const int result = Intersect::IntersectRayAABB2(rayStart, rayDir.GetReciprocal(), m_aabb, start, end); if (Intersect::ISECT_RAY_AABB_NONE != result) { if (ModelAsset* modelAssetPtr = m_modelAsset.Get()) @@ -151,7 +151,7 @@ namespace AZ AZ::Debug::Timer timer; timer.Stamp(); #endif - const bool hit = modelAssetPtr->LocalRayIntersectionAgainstModel(rayStart, dir, distance, normal); + const bool hit = modelAssetPtr->LocalRayIntersectionAgainstModel(rayStart, rayDir, distanceNormalized, normal); #if defined(AZ_RPI_PROFILE_RAYCASTING_AGAINST_MODELS) if (hit) { @@ -166,8 +166,12 @@ namespace AZ } bool Model::RayIntersection( - const AZ::Transform& modelTransform, const AZ::Vector3& nonUniformScale, const AZ::Vector3& rayStart, const AZ::Vector3& dir, - float& distanceFactor, AZ::Vector3& normal) const + const AZ::Transform& modelTransform, + const AZ::Vector3& nonUniformScale, + const AZ::Vector3& rayStart, + const AZ::Vector3& rayDir, + float& distanceNormalized, + AZ::Vector3& normal) const { AZ_PROFILE_FUNCTION(Debug::ProfileCategory::AzRender); const AZ::Vector3 clampedScale = nonUniformScale.GetMax(AZ::Vector3(AZ::MinTransformScale)); @@ -175,12 +179,13 @@ namespace AZ const AZ::Transform inverseTM = modelTransform.GetInverse(); const AZ::Vector3 raySrcLocal = inverseTM.TransformPoint(rayStart) / clampedScale; - // Instead of just rotating 'dir' we need it to be scaled too, so that 'distanceFactor' will be in the target units rather than object local units. - const AZ::Vector3 rayDest = rayStart + dir; + // Instead of just rotating 'rayDir' we need it to be scaled too, so that 'distanceNormalized' will be in the target units rather + // than object local units. + const AZ::Vector3 rayDest = rayStart + rayDir; const AZ::Vector3 rayDestLocal = inverseTM.TransformPoint(rayDest) / clampedScale; const AZ::Vector3 rayDirLocal = rayDestLocal - raySrcLocal; - bool result = LocalRayIntersection(raySrcLocal, rayDirLocal, distanceFactor, normal); + const bool result = LocalRayIntersection(raySrcLocal, rayDirLocal, distanceNormalized, normal); normal = (normal * clampedScale).GetNormalized(); return result; } diff --git a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Model/ModelAsset.cpp b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Model/ModelAsset.cpp index 988d07e66d..52fda0f56b 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Model/ModelAsset.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Model/ModelAsset.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -75,7 +76,8 @@ namespace AZ m_status = Data::AssetData::AssetStatus::Ready; } - bool ModelAsset::LocalRayIntersectionAgainstModel(const AZ::Vector3& rayStart, const AZ::Vector3& dir, float& distance, AZ::Vector3& normal) const + bool ModelAsset::LocalRayIntersectionAgainstModel( + const AZ::Vector3& rayStart, const AZ::Vector3& rayDir, float& distanceNormalized, AZ::Vector3& normal) const { AZ_PROFILE_FUNCTION(AZ::Debug::ProfileCategory::AzRender); @@ -85,7 +87,7 @@ namespace AZ m_modelTriangleCount = CalculateTriangleCount(); } - // check the total vertex count for this model and skip kdtree if the model is simple enough + // check the total vertex count for this model and skip kd-tree if the model is simple enough if (*m_modelTriangleCount > s_minimumModelTriangleCountToOptimize) { if (!m_kdTree) @@ -97,11 +99,11 @@ namespace AZ } else { - return m_kdTree->RayIntersection(rayStart, dir, distance, normal); + return m_kdTree->RayIntersection(rayStart, rayDir, distanceNormalized, normal); } } - return BruteForceRayIntersect(rayStart, dir, distance, normal); + return BruteForceRayIntersect(rayStart, rayDir, distanceNormalized, normal); } void ModelAsset::BuildKdTree() const @@ -136,7 +138,8 @@ namespace AZ } } - bool ModelAsset::BruteForceRayIntersect(const AZ::Vector3& rayStart, const AZ::Vector3& dir, float& distance, AZ::Vector3& normal) const + bool ModelAsset::BruteForceRayIntersect( + const AZ::Vector3& rayStart, const AZ::Vector3& rayDir, float& distanceNormalized, AZ::Vector3& normal) const { // brute force - check every triangle if (GetLodAssets().empty() == false) @@ -144,27 +147,27 @@ namespace AZ // intersect against the highest level of detail if (ModelLodAsset* loadAssetPtr = GetLodAssets()[0].Get()) { - float shortestDistance = std::numeric_limits::max(); bool anyHit = false; - AZ::Vector3 intersectionNormal; - + float shortestDistanceNormalized = AZStd::numeric_limits::max(); for (const ModelLodAsset::Mesh& mesh : loadAssetPtr->GetMeshes()) { - if (LocalRayIntersectionAgainstMesh(mesh, rayStart, dir, distance, intersectionNormal)) + float currentDistanceNormalized; + if (LocalRayIntersectionAgainstMesh(mesh, rayStart, rayDir, currentDistanceNormalized, intersectionNormal)) { anyHit = true; - if (distance < shortestDistance) + + if (currentDistanceNormalized < shortestDistanceNormalized) { normal = intersectionNormal; - shortestDistance = distance; + shortestDistanceNormalized = currentDistanceNormalized; } } } if (anyHit) { - distance = shortestDistance; + distanceNormalized = shortestDistanceNormalized; } return anyHit; @@ -174,7 +177,12 @@ namespace AZ return false; } - bool ModelAsset::LocalRayIntersectionAgainstMesh(const ModelLodAsset::Mesh& mesh, const AZ::Vector3& rayStart, const AZ::Vector3& dir, float& distance, AZ::Vector3& normal) const + bool ModelAsset::LocalRayIntersectionAgainstMesh( + const ModelLodAsset::Mesh& mesh, + const AZ::Vector3& rayStart, + const AZ::Vector3& rayDir, + float& distanceNormalized, + AZ::Vector3& normal) const { const BufferAssetView& indexBufferView = mesh.GetIndexBufferAssetView(); const AZStd::array_view& streamBufferList = mesh.GetStreamBufferInfoList(); @@ -217,14 +225,13 @@ namespace AZ AZStd::array_view indexRawBuffer = indexAssetViewPtr->GetBuffer(); RHI::BufferViewDescriptor indexRawDesc = indexAssetViewPtr->GetBufferViewDescriptor(); - float closestNormalizedDistance = 1.f; bool anyHit = false; - const AZ::Vector3 rayEnd = rayStart + dir * distance; + const AZ::Vector3 rayEnd = rayStart + rayDir; AZ::Vector3 a, b, c; AZ::Vector3 intersectionNormal; - float normalizedDistance = 1.f; + float shortestDistanceNormalized = AZStd::numeric_limits::max(); const AZ::u32* indexPtr = reinterpret_cast(indexRawBuffer.data()); for (uint32_t indexIter = 0; indexIter <= indexRawDesc.m_elementCount - 3; indexIter += 3, indexPtr += 3) { @@ -247,20 +254,22 @@ namespace AZ p = reinterpret_cast(&positionRawBuffer[index2 * positionElementSize]); c.Set(const_cast(p)); - if (AZ::Intersect::IntersectSegmentTriangleCCW(rayStart, rayEnd, a, b, c, intersectionNormal, normalizedDistance)) + float currentDistanceNormalized; + if (AZ::Intersect::IntersectSegmentTriangleCCW(rayStart, rayEnd, a, b, c, intersectionNormal, currentDistanceNormalized)) { - if (normalizedDistance < closestNormalizedDistance) + anyHit = true; + + if (currentDistanceNormalized < shortestDistanceNormalized) { normal = intersectionNormal; - closestNormalizedDistance = normalizedDistance; + shortestDistanceNormalized = currentDistanceNormalized; } - anyHit = true; } } if (anyHit) { - distance = closestNormalizedDistance * distance; + distanceNormalized = shortestDistanceNormalized; } return anyHit; diff --git a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Model/ModelKdTree.cpp b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Model/ModelKdTree.cpp index bee489c2fd..2ee6d93df3 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Reflect/Model/ModelKdTree.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Reflect/Model/ModelKdTree.cpp @@ -208,10 +208,10 @@ namespace AZ bool ModelKdTree::RayIntersection( const AZ::Vector3& raySrc, const AZ::Vector3& rayDir, float& distanceNormalized, AZ::Vector3& normal) const { - float closestDistanceNormalized = AZStd::numeric_limits::max(); - if (RayIntersectionRecursively(m_pRootNode.get(), raySrc, rayDir, closestDistanceNormalized, normal)) + float shortestDistanceNormalized = AZStd::numeric_limits::max(); + if (RayIntersectionRecursively(m_pRootNode.get(), raySrc, rayDir, shortestDistanceNormalized, normal)) { - distanceNormalized = closestDistanceNormalized; + distanceNormalized = shortestDistanceNormalized; return true; } diff --git a/Gems/Atom/RPI/Code/Tests/Model/ModelTests.cpp b/Gems/Atom/RPI/Code/Tests/Model/ModelTests.cpp index 3ce17bee8b..f039240ee0 100644 --- a/Gems/Atom/RPI/Code/Tests/Model/ModelTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Model/ModelTests.cpp @@ -22,6 +22,7 @@ #include #include +#include #include #include @@ -568,7 +569,7 @@ namespace UnitTest ValidateModelAsset(serializedModelAsset.Get(), expectedModel); } - // Tests that if we try to set the name on a Model + // Tests that if we try to set the name on a Model // before calling Begin that it will fail. TEST_F(ModelTests, SetNameNoBegin) { @@ -581,7 +582,7 @@ namespace UnitTest creator.SetName("TestName"); } - // Tests that if we try to add a ModelLod to a Model + // Tests that if we try to add a ModelLod to a Model // before calling Begin that it will fail. TEST_F(ModelTests, AddLodNoBegin) { @@ -598,7 +599,7 @@ namespace UnitTest creator.AddLodAsset(AZStd::move(lod)); } - // Tests that if we create a ModelAsset without adding + // Tests that if we create a ModelAsset without adding // any ModelLodAssets that the creator will properly fail to produce an asset. TEST_F(ModelTests, CreateModelNoLods) { @@ -618,8 +619,8 @@ namespace UnitTest ASSERT_EQ(asset.Get(), nullptr); } - // Tests that if we call SetLodIndexBuffer without calling - // Begin first on the ModelLodAssetCreator that it + // Tests that if we call SetLodIndexBuffer without calling + // Begin first on the ModelLodAssetCreator that it // fails as expected. TEST_F(ModelTests, SetLodIndexBufferNoBegin) { @@ -633,8 +634,8 @@ namespace UnitTest creator.SetLodIndexBuffer(validIndexBuffer); } - // Tests that if we call AddLodStreamBuffer without calling - // Begin first on the ModelLodAssetCreator that it + // Tests that if we call AddLodStreamBuffer without calling + // Begin first on the ModelLodAssetCreator that it // fails as expected. TEST_F(ModelTests, AddLodStreamBufferNoBegin) { @@ -648,8 +649,8 @@ namespace UnitTest creator.AddLodStreamBuffer(validStreamBuffer); } - // Tests that if we call BeginMesh without calling - // Begin first on the ModelLodAssetCreator that it + // Tests that if we call BeginMesh without calling + // Begin first on the ModelLodAssetCreator that it // fails as expected. TEST_F(ModelTests, BeginMeshNoBegin) { @@ -662,13 +663,13 @@ namespace UnitTest } // Tests that if we try to set an AABB on a mesh - // without calling Begin or BeginMesh that it fails + // without calling Begin or BeginMesh that it fails // as expected. Also tests the case that Begin *is* // called but BeginMesh is not. TEST_F(ModelTests, SetAabbNoBeginNoBeginMesh) { using namespace AZ; - + RPI::ModelLodAssetCreator creator; AZ::Aabb aabb = AZ::Aabb::CreateCenterRadius(AZ::Vector3::CreateZero(), 1.0f); @@ -691,13 +692,13 @@ namespace UnitTest } // Tests that if we try to set the material id on a mesh - // without calling Begin or BeginMesh that it fails + // without calling Begin or BeginMesh that it fails // as expected. Also tests the case that Begin *is* // called but BeginMesh is not. TEST_F(ModelTests, SetMaterialIdNoBeginNoBeginMesh) { using namespace AZ; - + RPI::ModelLodAssetCreator creator; { @@ -715,7 +716,7 @@ namespace UnitTest } // Tests that if we try to set the index buffer on a mesh - // without calling Begin or BeginMesh that it fails + // without calling Begin or BeginMesh that it fails // as expected. Also tests the case that Begin *is* // called but BeginMesh is not. TEST_F(ModelTests, SetIndexBufferNoBeginNoBeginMesh) @@ -751,7 +752,7 @@ namespace UnitTest } // Tests that if we try to add a stream buffer on a mesh - // without calling Begin or BeginMesh that it fails + // without calling Begin or BeginMesh that it fails // as expected. Also tests the case that Begin *is* // called but BeginMesh is not. TEST_F(ModelTests, AddStreamBufferNoBeginNoBeginMesh) @@ -785,7 +786,7 @@ namespace UnitTest } } - // Tests that if we try to end the creation of a + // Tests that if we try to end the creation of a // ModelLodAsset that has no meshes that it fails // as expected. TEST_F(ModelTests, CreateLodNoMeshes) @@ -804,7 +805,7 @@ namespace UnitTest ASSERT_EQ(asset.Get(), nullptr); } - // Tests that validation still fails when expected + // Tests that validation still fails when expected // even after producing a valid mesh due to a missing // BeginMesh call TEST_F(ModelTests, SecondMeshFailureNoBeginMesh) @@ -862,8 +863,8 @@ namespace UnitTest ASSERT_EQ(asset->GetMeshes().size(), 1); } - // Tests that validation still fails when expected - // even after producing a valid mesh due to SetMeshX + // Tests that validation still fails when expected + // even after producing a valid mesh due to SetMeshX // calls coming after End TEST_F(ModelTests, SecondMeshAfterEnd) { @@ -907,7 +908,7 @@ namespace UnitTest AZ::Aabb aabb = AZ::Aabb::CreateCenterRadius(Vector3::CreateZero(), 1.0f); ErrorMessageFinder messageFinder("Begin() was not called", 6); - + creator.BeginMesh(); creator.SetMeshAabb(AZStd::move(aabb)); creator.SetMeshMaterialAsset(m_materialAsset); @@ -955,6 +956,20 @@ namespace UnitTest EXPECT_EQ(uvStreamTangentBitmask.GetFullTangentBitmask(), 0x70000F51); } + // + // +----+ + // / /| + // +----+ | + // | | + + // | |/ + // +----+ + // + static constexpr AZStd::array CubePositions = { -1.0f, 1.0f, 1.0f, 1.0f, 1.0f, 1.0f, -1.0f, -1.0f, 1.0f, 1.0f, -1.0f, 1.0f, + -1.0f, 1.0f, -1.0f, 1.0f, 1.0f, -1.0f, -1.0f, -1.0f, -1.0f, 1.0f, -1.0f, -1.0f }; + static constexpr AZStd::array CubeIndices = { + uint32_t{ 0 }, 2, 1, 1, 2, 3, 4, 5, 6, 5, 7, 6, 0, 4, 2, 4, 6, 2, 1, 3, 5, 5, 3, 7, 0, 1, 4, 4, 1, 5, 2, 6, 3, 6, 7, 3, + }; + // This class creates a Model with one LOD, whose mesh contains 2 planes. Plane 1 is in the XY plane at Z=-0.5, and // plane 2 is in the XY plane at Z=0.5. The two planes each have 9 quads which have been triangulated. It only has // a position and index buffer. @@ -972,52 +987,75 @@ namespace UnitTest // *---*---*---* // \ / \ / \ / \ // *---*---*---* + static constexpr AZStd::array TwoSeparatedPlanesPositions{ + -1.0f, -0.333f, -0.5f, -0.333f, -1.0f, -0.5f, -0.333f, -0.333f, -0.5f, 0.333f, -0.333f, -0.5f, 1.0f, -1.0f, -0.5f, + 1.0f, -0.333f, -0.5f, 0.333f, -1.0f, -0.5f, 0.333f, 1.0f, -0.5f, 1.0f, 0.333f, -0.5f, 1.0f, 1.0f, -0.5f, + 0.333f, 0.333f, -0.5f, -0.333f, 1.0f, -0.5f, -0.333f, 0.333f, -0.5f, -1.0f, 1.0f, -0.5f, -1.0f, 0.333f, -0.5f, + -1.0f, -0.333f, 0.5f, -0.333f, -1.0f, 0.5f, -0.333f, -0.333f, 0.5f, 0.333f, -0.333f, 0.5f, 1.0f, -1.0f, 0.5f, + 1.0f, -0.333f, 0.5f, -0.333f, -0.333f, 0.5f, 0.333f, -1.0f, 0.5f, 0.333f, -0.333f, 0.5f, 0.333f, 1.0f, 0.5f, + 1.0f, 0.333f, 0.5f, 1.0f, 1.0f, 0.5f, 0.333f, 0.333f, 0.5f, 1.0f, -0.333f, 0.5f, -0.333f, 1.0f, 0.5f, + -0.333f, 0.333f, 0.5f, 0.333f, -0.333f, 0.5f, 0.333f, 0.333f, 0.5f, -1.0f, 1.0f, 0.5f, -0.333f, 0.333f, 0.5f, + -1.0f, 0.333f, 0.5f, -1.0f, -1.0f, -0.5f, -1.0f, -1.0f, 0.5f, 0.333f, -0.333f, 0.5f, 0.333f, -1.0f, 0.5f, + 1.0f, -1.0f, 0.5f, 0.333f, -1.0f, 0.5f, 0.333f, 0.333f, 0.5f, 0.333f, -0.333f, 0.5f, 1.0f, -0.333f, 0.5f, + -0.333f, 0.333f, 0.5f, -0.333f, -0.333f, 0.5f, 0.333f, -0.333f, 0.5f, + }; + // clang-format off + static constexpr AZStd::array TwoSeparatedPlanesIndices{ + uint32_t{ 0 }, 1, 2, 3, 4, 5, 2, 6, 3, 7, 8, 9, 10, 5, 8, 11, 10, 7, 12, 3, 10, 13, 12, 11, 14, 2, 12, + 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 25, 29, 27, 24, 30, 31, 32, 33, 34, 29, 35, 17, 34, + 0, 36, 1, 3, 6, 4, 2, 1, 6, 7, 10, 8, 10, 3, 5, 11, 12, 10, 12, 2, 3, 13, 14, 12, 14, 0, 2, + 15, 37, 16, 38, 39, 40, 17, 16, 41, 24, 27, 25, 42, 43, 44, 29, 34, 27, 45, 46, 47, 33, 35, 34, 35, 15, 17, + }; + // clang-format on + + // Ensure that the index buffer references all the positions in the position buffer + static constexpr inline auto minmaxElement = AZStd::minmax_element(begin(TwoSeparatedPlanesIndices), end(TwoSeparatedPlanesIndices)); + static_assert(*minmaxElement.second == (TwoSeparatedPlanesPositions.size() / 3) - 1); + template class TD; - class TwoSeparatedPlanesMesh + class TestMesh { public: - TwoSeparatedPlanesMesh() + TestMesh(const float* positions, size_t positionCount, const uint32_t* indices, size_t indicesCount) { - using namespace AZ; - - RPI::ModelLodAssetCreator lodCreator; - lodCreator.Begin(Data::AssetId(AZ::Uuid::CreateRandom())); + AZ::RPI::ModelLodAssetCreator lodCreator; + lodCreator.Begin(AZ::Data::AssetId(AZ::Uuid::CreateRandom())); lodCreator.BeginMesh(); - lodCreator.SetMeshAabb(Aabb::CreateFromMinMax({-1.0f, -1.0f, -0.5f}, {1.0f, 1.0f, 0.5f})); + lodCreator.SetMeshAabb(AZ::Aabb::CreateFromMinMax({-1.0f, -1.0f, -0.5f}, {1.0f, 1.0f, 0.5f})); lodCreator.SetMeshMaterialAsset( AZ::Data::Asset(AZ::Data::AssetId(AZ::Uuid::CreateRandom(), 0), AZ::AzTypeInfo::Uuid(), "") ); { - AZ::Data::Asset indexBuffer = BuildTestBuffer(s_indexes.size(), sizeof(uint32_t)); - AZStd::copy(s_indexes.begin(), s_indexes.end(), reinterpret_cast(const_cast(indexBuffer->GetBuffer().data()))); + AZ::Data::Asset indexBuffer = BuildTestBuffer(indicesCount, sizeof(uint32_t)); + AZStd::copy(indices, indices + indicesCount, reinterpret_cast(const_cast(indexBuffer->GetBuffer().data()))); lodCreator.SetMeshIndexBuffer({ indexBuffer, - RHI::BufferViewDescriptor::CreateStructured(0, s_indexes.size(), sizeof(uint32_t)) + AZ::RHI::BufferViewDescriptor::CreateStructured(0, indicesCount, sizeof(uint32_t)) }); } { - AZ::Data::Asset positionBuffer = BuildTestBuffer(s_positions.size() / 3, sizeof(float) * 3); - AZStd::copy(s_positions.begin(), s_positions.end(), reinterpret_cast(const_cast(positionBuffer->GetBuffer().data()))); + AZ::Data::Asset positionBuffer = BuildTestBuffer(positionCount / 3, sizeof(float) * 3); + AZStd::copy(positions, positions + positionCount, reinterpret_cast(const_cast(positionBuffer->GetBuffer().data()))); lodCreator.AddMeshStreamBuffer( AZ::RHI::ShaderSemantic(AZ::Name("POSITION")), AZ::Name(), { positionBuffer, - RHI::BufferViewDescriptor::CreateStructured(0, s_positions.size() / 3, sizeof(float) * 3) + AZ::RHI::BufferViewDescriptor::CreateStructured(0, positionCount / 3, sizeof(float) * 3) } ); } lodCreator.EndMesh(); - Data::Asset lodAsset; + AZ::Data::Asset lodAsset; lodCreator.End(lodAsset); - RPI::ModelAssetCreator modelCreator; - modelCreator.Begin(Data::AssetId(AZ::Uuid::CreateRandom())); + AZ::RPI::ModelAssetCreator modelCreator; + modelCreator.Begin(AZ::Data::AssetId(AZ::Uuid::CreateRandom())); modelCreator.SetName("TestModel"); modelCreator.AddLodAsset(AZStd::move(lodAsset)); modelCreator.End(m_modelAsset); @@ -1030,40 +1068,20 @@ namespace UnitTest private: AZ::Data::Asset m_modelAsset; - - static constexpr AZStd::array s_positions{ - -1.0f, -0.333f, -0.5f, -0.333f, -1.0f, -0.5f, -0.333f, -0.333f, -0.5f, 0.333f, -0.333f, -0.5f, 1.0f, -1.0f, -0.5f, - 1.0f, -0.333f, -0.5f, 0.333f, -1.0f, -0.5f, 0.333f, 1.0f, -0.5f, 1.0f, 0.333f, -0.5f, 1.0f, 1.0f, -0.5f, - 0.333f, 0.333f, -0.5f, -0.333f, 1.0f, -0.5f, -0.333f, 0.333f, -0.5f, -1.0f, 1.0f, -0.5f, -1.0f, 0.333f, -0.5f, - -1.0f, -0.333f, 0.5f, -0.333f, -1.0f, 0.5f, -0.333f, -0.333f, 0.5f, 0.333f, -0.333f, 0.5f, 1.0f, -1.0f, 0.5f, - 1.0f, -0.333f, 0.5f, -0.333f, -0.333f, 0.5f, 0.333f, -1.0f, 0.5f, 0.333f, -0.333f, 0.5f, 0.333f, 1.0f, 0.5f, - 1.0f, 0.333f, 0.5f, 1.0f, 1.0f, 0.5f, 0.333f, 0.333f, 0.5f, 1.0f, -0.333f, 0.5f, -0.333f, 1.0f, 0.5f, - -0.333f, 0.333f, 0.5f, 0.333f, -0.333f, 0.5f, 0.333f, 0.333f, 0.5f, -1.0f, 1.0f, 0.5f, -0.333f, 0.333f, 0.5f, - -1.0f, 0.333f, 0.5f, -1.0f, -1.0f, -0.5f, -1.0f, -1.0f, 0.5f, 0.333f, -0.333f, 0.5f, 0.333f, -1.0f, 0.5f, - 1.0f, -1.0f, 0.5f, 0.333f, -1.0f, 0.5f, 0.333f, 0.333f, 0.5f, 0.333f, -0.333f, 0.5f, 1.0f, -0.333f, 0.5f, - -0.333f, 0.333f, 0.5f, -0.333f, -0.333f, 0.5f, 0.333f, -0.333f, 0.5f, - }; - static constexpr AZStd::array s_indexes{ - uint32_t{0}, 1, 2, 3, 4, 5, 2, 6, 3, 7, 8, 9, 10, 5, 8, 11, 10, 7, 12, 3, 10, 13, 12, 11, 14, 2, 12, - 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 25, 29, 27, 24, 30, 31, 32, 33, 34, 29, 35, 17, 34, - 0, 36, 1, 3, 6, 4, 2, 1, 6, 7, 10, 8, 10, 3, 5, 11, 12, 10, 12, 2, 3, 13, 14, 12, 14, 0, 2, - 15, 37, 16, 38, 39, 40, 17, 16, 41, 24, 27, 25, 42, 43, 44, 29, 34, 27, 45, 46, 47, 33, 35, 34, 35, 15, 17, - }; - - // Ensure that the index buffer references all the positions in the position buffer - static constexpr inline auto minmaxElement = AZStd::minmax_element(begin(s_indexes), end(s_indexes)); - static_assert(*minmaxElement.second == (s_positions.size() / 3) - 1); }; - struct KdTreeIntersectParams + struct IntersectParams { float xpos; float ypos; float zpos; + float xdir; + float ydir; + float zdir; float expectedDistance; bool expectedShouldIntersect; - friend std::ostream& operator<<(std::ostream& os, const KdTreeIntersectParams& param) + friend std::ostream& operator<<(std::ostream& os, const IntersectParams& param) { return os << "xpos:" << param.xpos @@ -1076,13 +1094,15 @@ namespace UnitTest class KdTreeIntersectsParameterizedFixture : public ModelTests - , public ::testing::WithParamInterface + , public ::testing::WithParamInterface { }; TEST_P(KdTreeIntersectsParameterizedFixture, KdTreeIntersects) { - TwoSeparatedPlanesMesh mesh; + TestMesh mesh( + TwoSeparatedPlanesPositions.data(), TwoSeparatedPlanesPositions.size(), TwoSeparatedPlanesIndices.data(), + TwoSeparatedPlanesIndices.size()); AZ::RPI::ModelKdTree kdTree; ASSERT_TRUE(kdTree.Build(mesh.GetModel().Get())); @@ -1092,38 +1112,40 @@ namespace UnitTest EXPECT_THAT( kdTree.RayIntersection( - AZ::Vector3(GetParam().xpos, GetParam().ypos, GetParam().zpos), AZ::Vector3::CreateAxisZ(-1.0f), distance, normal), + AZ::Vector3(GetParam().xpos, GetParam().ypos, GetParam().zpos), + AZ::Vector3(GetParam().xdir, GetParam().ydir, GetParam().zdir), distance, normal), testing::Eq(GetParam().expectedShouldIntersect)); EXPECT_THAT(distance, testing::FloatEq(GetParam().expectedDistance)); } - static constexpr inline AZStd::array intersectTestData{ - KdTreeIntersectParams{ -0.1f, 0.0f, 1.0f, 0.5f, true }, - KdTreeIntersectParams{ 0.0f, 0.0f, 1.0f, 0.5f, true }, - KdTreeIntersectParams{ 0.1f, 0.0f, 1.0f, 0.5f, true }, + static constexpr AZStd::array KdTreeIntersectTestData{ + IntersectParams{ -0.1f, 0.0f, 1.0f, 0.0f, 0.0f, -1.0f, 0.5f, true }, + IntersectParams{ 0.0f, 0.0f, 1.0f, 0.0f, 0.0f, -1.0f, 0.5f, true }, + IntersectParams{ 0.1f, 0.0f, 1.0f, 0.0f, 0.0f, -1.0f, 0.5f, true }, // Test the center of each triangle - KdTreeIntersectParams{-0.111f, -0.111f, 1.0f, 0.5f, true}, - KdTreeIntersectParams{-0.111f, -0.778f, 1.0f, 0.5f, true}, - KdTreeIntersectParams{-0.111f, 0.555f, 1.0f, 0.5f, true}, // Should intersect triangle with indices {29, 34, 27} and {11, 12, 10} - KdTreeIntersectParams{-0.555f, -0.555f, 1.0f, 0.5f, true}, - KdTreeIntersectParams{-0.555f, 0.111f, 1.0f, 0.5f, true}, - KdTreeIntersectParams{-0.555f, 0.778f, 1.0f, 0.5f, true}, - KdTreeIntersectParams{-0.778f, -0.111f, 1.0f, 0.5f, true}, - KdTreeIntersectParams{-0.778f, -0.778f, 1.0f, 0.5f, true}, - KdTreeIntersectParams{-0.778f, 0.555f, 1.0f, 0.5f, true}, - KdTreeIntersectParams{0.111f, -0.555f, 1.0f, 0.5f, true}, - KdTreeIntersectParams{0.111f, 0.111f, 1.0f, 0.5f, true}, - KdTreeIntersectParams{0.111f, 0.778f, 1.0f, 0.5f, true}, - KdTreeIntersectParams{0.555f, -0.111f, 1.0f, 0.5f, true}, - KdTreeIntersectParams{0.555f, -0.778f, 1.0f, 0.5f, true}, - KdTreeIntersectParams{0.555f, 0.555f, 1.0f, 0.5f, true}, - KdTreeIntersectParams{0.778f, -0.555f, 1.0f, 0.5f, true}, - KdTreeIntersectParams{0.778f, 0.111f, 1.0f, 0.5f, true}, - KdTreeIntersectParams{0.778f, 0.778f, 1.0f, 0.5f, true}, + IntersectParams{ -0.111f, -0.111f, 1.0f, 0.0f, 0.0f, -1.0f, 0.5f, true }, + IntersectParams{ -0.111f, -0.778f, 1.0f, 0.0f, 0.0f, -1.0f, 0.5f, true }, + IntersectParams{ -0.111f, 0.555f, 1.0f, 0.0f, 0.0f, -1.0f, 0.5f, + true }, // Should intersect triangle with indices {29, 34, 27} and {11, 12, 10} + IntersectParams{ -0.555f, -0.555f, 1.0f, 0.0f, 0.0f, -1.0f, 0.5f, true }, + IntersectParams{ -0.555f, 0.111f, 1.0f, 0.0f, 0.0f, -1.0f, 0.5f, true }, + IntersectParams{ -0.555f, 0.778f, 1.0f, 0.0f, 0.0f, -1.0f, 0.5f, true }, + IntersectParams{ -0.778f, -0.111f, 1.0f, 0.0f, 0.0f, -1.0f, 0.5f, true }, + IntersectParams{ -0.778f, -0.778f, 1.0f, 0.0f, 0.0f, -1.0f, 0.5f, true }, + IntersectParams{ -0.778f, 0.555f, 1.0f, 0.0f, 0.0f, -1.0f, 0.5f, true }, + IntersectParams{ 0.111f, -0.555f, 1.0f, 0.0f, 0.0f, -1.0f, 0.5f, true }, + IntersectParams{ 0.111f, 0.111f, 1.0f, 0.0f, 0.0f, -1.0f, 0.5f, true }, + IntersectParams{ 0.111f, 0.778f, 1.0f, 0.0f, 0.0f, -1.0f, 0.5f, true }, + IntersectParams{ 0.555f, -0.111f, 1.0f, 0.0f, 0.0f, -1.0f, 0.5f, true }, + IntersectParams{ 0.555f, -0.778f, 1.0f, 0.0f, 0.0f, -1.0f, 0.5f, true }, + IntersectParams{ 0.555f, 0.555f, 1.0f, 0.0f, 0.0f, -1.0f, 0.5f, true }, + IntersectParams{ 0.778f, -0.555f, 1.0f, 0.0f, 0.0f, -1.0f, 0.5f, true }, + IntersectParams{ 0.778f, 0.111f, 1.0f, 0.0f, 0.0f, -1.0f, 0.5f, true }, + IntersectParams{ 0.778f, 0.778f, 1.0f, 0.0f, 0.0f, -1.0f, 0.5f, true }, }; - INSTANTIATE_TEST_CASE_P(KdTreeIntersectsPlane, KdTreeIntersectsParameterizedFixture, ::testing::ValuesIn(intersectTestData)); + INSTANTIATE_TEST_CASE_P(KdTreeIntersectsPlane, KdTreeIntersectsParameterizedFixture, ::testing::ValuesIn(KdTreeIntersectTestData)); class KdTreeIntersectsFixture : public ModelTests @@ -1133,7 +1155,10 @@ namespace UnitTest { ModelTests::SetUp(); - m_mesh = AZStd::make_unique(); + m_mesh = AZStd::make_unique( + TwoSeparatedPlanesPositions.data(), TwoSeparatedPlanesPositions.size(), TwoSeparatedPlanesIndices.data(), + TwoSeparatedPlanesIndices.size()); + m_kdTree = AZStd::make_unique(); ASSERT_TRUE(m_kdTree->Build(m_mesh->GetModel().Get())); } @@ -1146,7 +1171,7 @@ namespace UnitTest ModelTests::TearDown(); } - AZStd::unique_ptr m_mesh; + AZStd::unique_ptr m_mesh; AZStd::unique_ptr m_kdTree; }; @@ -1154,7 +1179,7 @@ namespace UnitTest { float t = AZStd::numeric_limits::max(); AZ::Vector3 normal; - + constexpr float rayLength = 100.0f; EXPECT_THAT( m_kdTree->RayIntersection( @@ -1181,4 +1206,85 @@ namespace UnitTest EXPECT_THAT( m_kdTree->RayIntersection(AZ::Vector3::CreateAxisZ(5.0f), -AZ::Vector3::CreateAxisZ(), t, normal), testing::Eq(false)); } + + class BruteForceIntersectsParameterizedFixture + : public ModelTests + , public ::testing::WithParamInterface + { + }; + + TEST_P(BruteForceIntersectsParameterizedFixture, BruteForceIntersectsCube) + { + TestMesh mesh(CubePositions.data(), CubePositions.size(), CubeIndices.data(), CubeIndices.size()); + + float distance = AZStd::numeric_limits::max(); + AZ::Vector3 normal; + + EXPECT_THAT( + mesh.GetModel()->LocalRayIntersectionAgainstModel( + AZ::Vector3(GetParam().xpos, GetParam().ypos, GetParam().zpos), + AZ::Vector3(GetParam().xdir, GetParam().ydir, GetParam().zdir), distance, normal), + testing::Eq(GetParam().expectedShouldIntersect)); + EXPECT_THAT(distance, testing::FloatEq(GetParam().expectedDistance)); + } + + static constexpr AZStd::array BruteForceIntersectTestData{ + IntersectParams{ 5.0f, 0.0f, 5.0f, 0.0f, 0.0f, -1.0f, AZStd::numeric_limits::max(), false }, + IntersectParams{ 0.0f, 0.0f, 1.5f, 0.0f, 0.0f, -1.0f, 0.5f, true }, + IntersectParams{ 5.0f, 0.0f, 0.0f, -10.0f, 0.0f, 0.0f, 0.4f, true }, + IntersectParams{ -5.0f, 0.0f, 0.0f, 20.0f, 0.0f, 0.0f, 0.2f, true }, + IntersectParams{ 0.0f, -10.0f, 0.0f, 0.0f, 20.0f, 0.0f, 0.45f, true }, + IntersectParams{ 0.0f, 20.0f, 0.0f, 0.0f, -40.0f, 0.0f, 0.475f, true }, + IntersectParams{ 0.0f, 20.0f, 0.0f, 0.0f, -19.0f, 0.0f, 1.0f, true }, + }; + + INSTANTIATE_TEST_CASE_P( + BruteForceIntersects, BruteForceIntersectsParameterizedFixture, ::testing::ValuesIn(BruteForceIntersectTestData)); + + class BruteForceModelIntersectsFixture + : public ModelTests + { + public: + void SetUp() override + { + ModelTests::SetUp(); + m_mesh = AZStd::make_unique(CubePositions.data(), CubePositions.size(), CubeIndices.data(), CubeIndices.size()); + } + + void TearDown() override + { + m_mesh.reset(); + ModelTests::TearDown(); + } + + AZStd::unique_ptr m_mesh; + }; + + TEST_F(BruteForceModelIntersectsFixture, BruteForceIntersectionDetectedWithCube) + { + float t = 0.0f; + AZ::Vector3 normal; + + // firing down the negative z axis, positioned 5 units from cube (cube is 2x2x2 so intersection + // happens at 1 in z) + EXPECT_THAT( + m_mesh->GetModel()->LocalRayIntersectionAgainstModel( + AZ::Vector3::CreateAxisZ(5.0f), -AZ::Vector3::CreateAxisZ(10.0f), t, normal), + testing::Eq(true)); + EXPECT_THAT(t, testing::FloatEq(0.4f)); + } + + TEST_F(BruteForceModelIntersectsFixture, BruteForceIntersectionDetectedAndNormalSetAtEndOfRay) + { + float t = 0.0f; + AZ::Vector3 normal = AZ::Vector3::CreateOne(); // invalid starting normal + + // ensure the intersection happens right at the end of the ray + EXPECT_THAT( + m_mesh->GetModel()->LocalRayIntersectionAgainstModel( + AZ::Vector3::CreateAxisY(10.0f), -AZ::Vector3::CreateAxisY(9.0f), t, normal), + testing::Eq(true)); + EXPECT_THAT(t, testing::FloatEq(1.0f)); + EXPECT_THAT(normal, IsClose(AZ::Vector3::CreateAxisY())); + } } // namespace UnitTest From a766e3af5c08a63e2f6edcddae26376ef8b57dd4 Mon Sep 17 00:00:00 2001 From: chcurran Date: Tue, 8 Jun 2021 13:54:25 -0700 Subject: [PATCH 06/16] Fixes for internal if-branch node parser bug (LYN-4347) and exposing properties for AZStd::tuple (LYN-3910) --- .../AzCore/RTTI/AzStdOnDemandPrettyName.inl | 51 +- .../AzCore/RTTI/AzStdOnDemandReflection.inl | 24 +- .../Grammar/AbstractCodeModel.cpp | 32 +- ...ionIfBranchWithConnectedInput.scriptcanvas | 2499 +++++++++++++++++ .../Tests/ScriptCanvas_RuntimeInterpreted.cpp | 5 + 5 files changed, 2588 insertions(+), 23 deletions(-) create mode 100644 Gems/ScriptCanvasTesting/Assets/ScriptCanvas/UnitTests/LY_SC_UnitTest_ParseFunctionIfBranchWithConnectedInput.scriptcanvas diff --git a/Code/Framework/AzCore/AzCore/RTTI/AzStdOnDemandPrettyName.inl b/Code/Framework/AzCore/AzCore/RTTI/AzStdOnDemandPrettyName.inl index 26e269573d..4f4a21301c 100644 --- a/Code/Framework/AzCore/AzCore/RTTI/AzStdOnDemandPrettyName.inl +++ b/Code/Framework/AzCore/AzCore/RTTI/AzStdOnDemandPrettyName.inl @@ -161,7 +161,56 @@ namespace AZ return "A pair is an fixed size collection of two elements."; } }; - + + template + void GetTypeNamesFold(AZStd::vector& result, AZ::BehaviorContext& context) + { + result.push_back(OnDemandPrettyName::Get(context)); + }; + + template + void GetTypeNames(AZStd::vector& result, AZ::BehaviorContext& context) + { + (GetTypeNamesFold(result, context), ...); + }; + + template + void GetTypeNamesFold(AZStd::string& result, AZ::BehaviorContext& context) + { + if (!result.empty()) + { + result += ", "; + } + + result += OnDemandPrettyName::Get(context); + }; + + template + void GetTypeNames(AZStd::string& result, AZ::BehaviorContext& context) + { + (GetTypeNamesFold(result, context), ...); + }; + + template + struct OnDemandPrettyName> + { + static AZStd::string Get(AZ::BehaviorContext& context) + { + AZStd::string typeNames; + GetTypeNames(typeNames, context); + return AZStd::string::format("Tuple<%s>", typeNames.c_str()); + } + }; + + template + struct OnDemandToolTip> + { + static AZStd::string Get(AZ::BehaviorContext&) + { + return "A tuple is an fixed size collection of any number of any type of element."; + } + }; + template struct OnDemandPrettyName< AZStd::unordered_map > { diff --git a/Code/Framework/AzCore/AzCore/RTTI/AzStdOnDemandReflection.inl b/Code/Framework/AzCore/AzCore/RTTI/AzStdOnDemandReflection.inl index 537d3e5c83..2132dff3c4 100644 --- a/Code/Framework/AzCore/AzCore/RTTI/AzStdOnDemandReflection.inl +++ b/Code/Framework/AzCore/AzCore/RTTI/AzStdOnDemandReflection.inl @@ -813,20 +813,27 @@ namespace AZ { using ContainerType = AZStd::tuple; - template - static void ReflectUnpackMethodFold(BehaviorContext::ClassBuilder& builder) + template + static void ReflectUnpackMethodFold(BehaviorContext::ClassBuilder& builder, const AZStd::vector& typeNames, [[maybe_unused]] size_t inputIndex) { const AZStd::string methodName = AZStd::string::format("Get%zu", Index); - builder->Method(methodName.data(), [](ContainerType& value) { return AZStd::get(value); }) - ->Attribute(AZ::Script::Attributes::ExcludeFrom, AZ::Script::Attributes::ExcludeFlags::All) + builder->Method(methodName.data(), [](ContainerType& thisPointer) { return AZStd::get(thisPointer); }) + ->Attribute(AZ::Script::Attributes::ExcludeFrom, AZ::Script::Attributes::ExcludeFlags::List) ->Attribute(AZ::ScriptCanvasAttributes::TupleGetFunctionIndex, Index) ; + + builder->Property + ( AZStd::string::format("element_%zu_%s", Index, typeNames[Index].c_str()).c_str() + , [](ContainerType& thisPointer) { return AZStd::get(thisPointer); } + , [](ContainerType& thisPointer, const t_Arg& element) { AZStd::get(thisPointer) = element; }); } - template + template static void ReflectUnpackMethods(BehaviorContext::ClassBuilder& builder, AZStd::index_sequence) { - (ReflectUnpackMethodFold(builder), ...); + AZStd::vector typeNames; + ScriptCanvasOnDemandReflection::GetTypeNames(typeNames, *builder.m_context); + (ReflectUnpackMethodFold(builder, typeNames, Indices), ...); } static void Reflect(ReflectContext* context) @@ -851,9 +858,10 @@ namespace AZ ->Attribute(AZ::ScriptCanvasAttributes::TupleConstructorFunction, constructorHolder) ; - ReflectUnpackMethods(builder, AZStd::make_index_sequence{}); + ReflectUnpackMethods(builder, AZStd::make_index_sequence{}); + builder->Method("GetSize", []() { return AZStd::tuple_size::value; }) - ->Attribute(AZ::Script::Attributes::ExcludeFrom, AZ::Script::Attributes::ExcludeFlags::All) + ->Attribute(AZ::Script::Attributes::ExcludeFrom, AZ::Script::Attributes::ExcludeFlags::List) ; } } diff --git a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Grammar/AbstractCodeModel.cpp b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Grammar/AbstractCodeModel.cpp index 2a485c9501..bc07d16a7e 100644 --- a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Grammar/AbstractCodeModel.cpp +++ b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Grammar/AbstractCodeModel.cpp @@ -3356,22 +3356,26 @@ namespace ScriptCanvas if (executionIf->GetId().m_node->IsIfBranchPrefacedWithBooleanExpression()) { - auto removeChildOutcome = RemoveChild(executionIf->ModParent(), executionIf); - if (!removeChildOutcome.IsSuccess()) - { - AddError(executionIf->GetNodeId(), executionIf, ScriptCanvas::ParseErrors::FailedToRemoveChild); - } + ExecutionTreePtr booleanExpression; - if (!IsErrorFree()) { - return; - } + auto removeChildOutcome = RemoveChild(executionIf->ModParent(), executionIf); + if (!removeChildOutcome.IsSuccess()) + { + AddError(executionIf->GetNodeId(), executionIf, ScriptCanvas::ParseErrors::FailedToRemoveChild); + } + + if (!IsErrorFree()) + { + return; + } - const auto indexAndChild = removeChildOutcome.TakeValue(); + const auto indexAndChild = removeChildOutcome.TakeValue(); - ExecutionTreePtr booleanExpression = CreateChild(executionIf->ModParent(), executionIf->GetId().m_node, executionIf->GetId().m_slot); - executionIf->ModParent()->InsertChild(indexAndChild.first, { indexAndChild.second.m_slot, indexAndChild.second.m_output, booleanExpression }); - executionIf->SetParent(booleanExpression); + booleanExpression = CreateChild(executionIf->ModParent(), executionIf->GetId().m_node, executionIf->GetId().m_slot); + executionIf->ModParent()->InsertChild(indexAndChild.first, { indexAndChild.second.m_slot, indexAndChild.second.m_output, booleanExpression }); + executionIf->SetParent(booleanExpression); + } // make a condition here auto symbol = CheckLogicalExpressionSymbol(booleanExpression); @@ -3402,7 +3406,7 @@ namespace ScriptCanvas return; } - const auto indexAndChild2 = removeChildOutcome.TakeValue(); + const auto indexAndChild2 = removeChildOutcome2.TakeValue(); // parse if statement internal function ExecutionTreePtr internalFunction = CreateChild(booleanExpression->ModParent(), booleanExpression->GetId().m_node, booleanExpression->GetId().m_slot); @@ -4782,7 +4786,7 @@ namespace ScriptCanvas { PropertyExtractionPtr extraction = AZStd::make_shared(); extraction->m_slot = slot; - extraction->m_name = propertyField.first; + extraction->m_name = AZ::ReplaceCppArtifacts(propertyField.first); execution->AddPropertyExtractionSource(slot, extraction); } else diff --git a/Gems/ScriptCanvasTesting/Assets/ScriptCanvas/UnitTests/LY_SC_UnitTest_ParseFunctionIfBranchWithConnectedInput.scriptcanvas b/Gems/ScriptCanvasTesting/Assets/ScriptCanvas/UnitTests/LY_SC_UnitTest_ParseFunctionIfBranchWithConnectedInput.scriptcanvas new file mode 100644 index 0000000000..38df5159c2 --- /dev/null +++ b/Gems/ScriptCanvasTesting/Assets/ScriptCanvas/UnitTests/LY_SC_UnitTest_ParseFunctionIfBranchWithConnectedInput.scriptcanvas @@ -0,0 +1,2499 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/Gems/ScriptCanvasTesting/Code/Tests/ScriptCanvas_RuntimeInterpreted.cpp b/Gems/ScriptCanvasTesting/Code/Tests/ScriptCanvas_RuntimeInterpreted.cpp index 1633b0fc51..5bda0f3941 100644 --- a/Gems/ScriptCanvasTesting/Code/Tests/ScriptCanvas_RuntimeInterpreted.cpp +++ b/Gems/ScriptCanvasTesting/Code/Tests/ScriptCanvas_RuntimeInterpreted.cpp @@ -90,6 +90,11 @@ public: } }; +TEST_F(ScriptCanvasTestFixture, ParseFunctionIfBranchWithConnectedInput) +{ + RunUnitTestGraph("LY_SC_UnitTest_ParseFunctionIfBranchWithConnectedInput"); +} + TEST_F(ScriptCanvasTestFixture, UseRawBehaviorProperties) { RunUnitTestGraph("LY_SC_UnitTest_UseRawBehaviorProperties"); From 4b3d0d1054d88833de47d07840b356fa436562b2 Mon Sep 17 00:00:00 2001 From: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> Date: Tue, 8 Jun 2021 14:01:50 -0700 Subject: [PATCH 07/16] LYN-4327 [SDK] External Gem's aren't added to the project solution when using SDK (#1191) * should pickup the external directories registered by the project * Add support for AzTest and AzTestRunner in the SDK * missing IMPORT_LIB * Moved where .Assets targets get generated so they are visible in the SDK * generate the Directory.Build.props in the right path * excluding target on platforms that dont support it --- CMakeLists.txt | 36 +++++++-------- Code/Framework/AzTest/CMakeLists.txt | 46 +++++++++---------- Code/LauncherUnified/launcher_generator.cmake | 23 ++++++++++ Code/Tools/AzTestRunner/CMakeLists.txt | 36 ++++++++------- .../Android/platform_traits_android.cmake | 2 +- .../Linux/platform_traits_linux.cmake | 2 +- .../Platform/Mac/platform_traits_mac.cmake | 2 +- .../Windows/platform_traits_windows.cmake | 2 +- .../Platform/iOS/platform_traits_ios.cmake | 2 +- cmake/Platform/Common/Install_common.cmake | 13 +++++- .../Platform/Common/VisualStudio_common.cmake | 2 +- scripts/ctest/CMakeLists.txt | 29 ------------ 12 files changed, 100 insertions(+), 95 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 387f536966..6ca9aeacb8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -88,28 +88,28 @@ if(NOT INSTALLED_ENGINE) # Add external subdirectories listed in the engine.json. LY_EXTERNAL_SUBDIRS is a cache variable so the user can add extra # external subdirectories add_engine_json_external_subdirectories() - get_property(external_subdirs GLOBAL PROPERTY LY_EXTERNAL_SUBDIRS) - list(APPEND LY_EXTERNAL_SUBDIRS ${external_subdirs}) - - # Loop over the additional external subdirectories and invoke add_subdirectory on them - foreach(external_directory ${LY_EXTERNAL_SUBDIRS}) - # Hash the extenal_directory name and append it to the Binary Directory section of add_subdirectory - # This is to deal with potential situations where multiple external directories has the same last directory name - # For example if D:/Company1/RayTracingGem and F:/Company2/Path/RayTracingGem were both added as a subdirectory - file(REAL_PATH ${external_directory} full_directory_path) - string(SHA256 full_directory_hash ${full_directory_path}) - # Truncate the full_directory_hash down to 8 characters to avoid hitting the Windows 260 character path limit - # when the external subdirectory contains relative paths of significant length - string(SUBSTRING ${full_directory_hash} 0 8 full_directory_hash) - # Use the last directory as the suffix path to use for the Binary Directory - get_filename_component(directory_name ${external_directory} NAME) - add_subdirectory(${external_directory} ${CMAKE_BINARY_DIR}/External/${directory_name}-${full_directory_hash}) - endforeach() - else() ly_find_o3de_packages() endif() +get_property(external_subdirs GLOBAL PROPERTY LY_EXTERNAL_SUBDIRS) +list(APPEND LY_EXTERNAL_SUBDIRS ${external_subdirs}) + +# Loop over the additional external subdirectories and invoke add_subdirectory on them +foreach(external_directory ${LY_EXTERNAL_SUBDIRS}) + # Hash the extenal_directory name and append it to the Binary Directory section of add_subdirectory + # This is to deal with potential situations where multiple external directories has the same last directory name + # For example if D:/Company1/RayTracingGem and F:/Company2/Path/RayTracingGem were both added as a subdirectory + file(REAL_PATH ${external_directory} full_directory_path) + string(SHA256 full_directory_hash ${full_directory_path}) + # Truncate the full_directory_hash down to 8 characters to avoid hitting the Windows 260 character path limit + # when the external subdirectory contains relative paths of significant length + string(SUBSTRING ${full_directory_hash} 0 8 full_directory_hash) + # Use the last directory as the suffix path to use for the Binary Directory + get_filename_component(directory_name ${external_directory} NAME) + add_subdirectory(${external_directory} ${CMAKE_BINARY_DIR}/External/${directory_name}-${full_directory_hash}) +endforeach() + ################################################################################ # Post-processing ################################################################################ diff --git a/Code/Framework/AzTest/CMakeLists.txt b/Code/Framework/AzTest/CMakeLists.txt index ff31b32a24..fe5ec2d0ff 100644 --- a/Code/Framework/AzTest/CMakeLists.txt +++ b/Code/Framework/AzTest/CMakeLists.txt @@ -8,29 +8,25 @@ # remove or modify any license notices. This file is distributed on an "AS IS" BASIS, # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # + +ly_get_list_relative_pal_filename(pal_dir ${CMAKE_CURRENT_LIST_DIR}/AzTest/Platform/${PAL_PLATFORM_NAME}) -if(PAL_TRAIT_BUILD_TESTS_SUPPORTED) - - ly_get_list_relative_pal_filename(pal_dir ${CMAKE_CURRENT_LIST_DIR}/AzTest/Platform/${PAL_PLATFORM_NAME}) - - ly_add_target( - NAME AzTest STATIC - NAMESPACE AZ - FILES_CMAKE - AzTest/aztest_files.cmake - ${pal_dir}/platform_${PAL_PLATFORM_NAME_LOWERCASE}_files.cmake - INCLUDE_DIRECTORIES - PUBLIC - . - ${pal_dir} - BUILD_DEPENDENCIES - PUBLIC - 3rdParty::googletest::GMock - 3rdParty::googletest::GTest - 3rdParty::GoogleBenchmark - AZ::AzCore - PLATFORM_INCLUDE_FILES - ${pal_dir}/platform_${PAL_PLATFORM_NAME_LOWERCASE}.cmake - ) - -endif() +ly_add_target( + NAME AzTest STATIC + NAMESPACE AZ + FILES_CMAKE + AzTest/aztest_files.cmake + ${pal_dir}/platform_${PAL_PLATFORM_NAME_LOWERCASE}_files.cmake + INCLUDE_DIRECTORIES + PUBLIC + . + ${pal_dir} + BUILD_DEPENDENCIES + PUBLIC + 3rdParty::googletest::GMock + 3rdParty::googletest::GTest + 3rdParty::GoogleBenchmark + AZ::AzCore + PLATFORM_INCLUDE_FILES + ${pal_dir}/platform_${PAL_PLATFORM_NAME_LOWERCASE}.cmake +) diff --git a/Code/LauncherUnified/launcher_generator.cmake b/Code/LauncherUnified/launcher_generator.cmake index c5d60eb29e..2f4c8a8b74 100644 --- a/Code/LauncherUnified/launcher_generator.cmake +++ b/Code/LauncherUnified/launcher_generator.cmake @@ -16,6 +16,7 @@ set_property(GLOBAL PROPERTY LAUNCHER_UNIFIED_BINARY_DIR ${CMAKE_CURRENT_BINARY_ # When using an installed engine, this file will be included by the FindLauncherGenerator.cmake script get_property(LY_PROJECTS_TARGET_NAME GLOBAL PROPERTY LY_PROJECTS_TARGET_NAME) foreach(project_name project_path IN ZIP_LISTS LY_PROJECTS_TARGET_NAME LY_PROJECTS) + # Computes the realpath to the project # If the project_path is relative, it is evaluated relative to the ${LY_ROOT_FOLDER} # Otherwise the the absolute project_path is returned with symlinks resolved @@ -35,6 +36,28 @@ foreach(project_name project_path IN ZIP_LISTS LY_PROJECTS_TARGET_NAME LY_PROJEC "to the LY_PROJECTS_TARGET_NAME global property. Other configuration errors might occur") endif() endif() + + ################################################################################ + # Assets + ################################################################################ + if(PAL_TRAIT_BUILD_HOST_TOOLS) + add_custom_target(${project_name}.Assets + COMMENT "Processing ${project_name} assets..." + COMMAND "${CMAKE_COMMAND}" + -DLY_LOCK_FILE=$/project_assets.lock + -P ${LY_ROOT_FOLDER}/cmake/CommandExecution.cmake + EXEC_COMMAND $ + --zeroAnalysisMode + --project-path=${project_real_path} + --platforms=${LY_ASSET_DEPLOY_ASSET_TYPE} + ) + set_target_properties(${project_name}.Assets + PROPERTIES + EXCLUDE_FROM_ALL TRUE + FOLDER ${project_name} + ) + endif() + ################################################################################ # Monolithic game ################################################################################ diff --git a/Code/Tools/AzTestRunner/CMakeLists.txt b/Code/Tools/AzTestRunner/CMakeLists.txt index d3598c397e..e6dd09e15b 100644 --- a/Code/Tools/AzTestRunner/CMakeLists.txt +++ b/Code/Tools/AzTestRunner/CMakeLists.txt @@ -9,12 +9,12 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # -if(PAL_TRAIT_BUILD_TESTS_SUPPORTED) +ly_get_list_relative_pal_filename(pal_dir ${CMAKE_CURRENT_LIST_DIR}/Platform/${PAL_PLATFORM_NAME}) - ly_get_list_relative_pal_filename(pal_dir ${CMAKE_CURRENT_LIST_DIR}/Platform/${PAL_PLATFORM_NAME}) - - include(${pal_dir}/platform_traits_${PAL_PLATFORM_NAME_LOWERCASE}.cmake) +include(${pal_dir}/platform_traits_${PAL_PLATFORM_NAME_LOWERCASE}.cmake) +if(PAL_TRAIT_AZTESTRUNNER_SUPPORTED) + ly_add_target( NAME AzTestRunner ${PAL_TRAIT_AZTESTRUNNER_LAUNCHER_TYPE} NAMESPACE AZ @@ -32,19 +32,23 @@ if(PAL_TRAIT_BUILD_TESTS_SUPPORTED) AZ::AzTest AZ::AzFramework ) + + if(PAL_TRAIT_BUILD_TESTS_SUPPORTED) + + ly_add_target( + NAME AzTestRunner.Tests ${PAL_TRAIT_TEST_TARGET_TYPE} + NAMESPACE AZ + FILES_CMAKE + aztestrunner_test_files.cmake + BUILD_DEPENDENCIES + PRIVATE + AZ::AzTest + ) - ly_add_target( - NAME AzTestRunner.Tests ${PAL_TRAIT_TEST_TARGET_TYPE} - NAMESPACE AZ - FILES_CMAKE - aztestrunner_test_files.cmake - BUILD_DEPENDENCIES - PRIVATE - AZ::AzTest - ) + ly_add_googletest( + NAME AZ::AzTestRunner.Tests + ) - ly_add_googletest( - NAME AZ::AzTestRunner.Tests - ) + endif() endif() diff --git a/Code/Tools/AzTestRunner/Platform/Android/platform_traits_android.cmake b/Code/Tools/AzTestRunner/Platform/Android/platform_traits_android.cmake index 40de4bd3eb..77d7b868d4 100644 --- a/Code/Tools/AzTestRunner/Platform/Android/platform_traits_android.cmake +++ b/Code/Tools/AzTestRunner/Platform/Android/platform_traits_android.cmake @@ -9,5 +9,5 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # +set(PAL_TRAIT_AZTESTRUNNER_SUPPORTED TRUE) set(PAL_TRAIT_AZTESTRUNNER_LAUNCHER_TYPE MODULE) - diff --git a/Code/Tools/AzTestRunner/Platform/Linux/platform_traits_linux.cmake b/Code/Tools/AzTestRunner/Platform/Linux/platform_traits_linux.cmake index 59922965bb..4c623c1f7b 100644 --- a/Code/Tools/AzTestRunner/Platform/Linux/platform_traits_linux.cmake +++ b/Code/Tools/AzTestRunner/Platform/Linux/platform_traits_linux.cmake @@ -9,5 +9,5 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # +set(PAL_TRAIT_AZTESTRUNNER_SUPPORTED TRUE) set(PAL_TRAIT_AZTESTRUNNER_LAUNCHER_TYPE EXECUTABLE) - diff --git a/Code/Tools/AzTestRunner/Platform/Mac/platform_traits_mac.cmake b/Code/Tools/AzTestRunner/Platform/Mac/platform_traits_mac.cmake index 59922965bb..4c623c1f7b 100644 --- a/Code/Tools/AzTestRunner/Platform/Mac/platform_traits_mac.cmake +++ b/Code/Tools/AzTestRunner/Platform/Mac/platform_traits_mac.cmake @@ -9,5 +9,5 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # +set(PAL_TRAIT_AZTESTRUNNER_SUPPORTED TRUE) set(PAL_TRAIT_AZTESTRUNNER_LAUNCHER_TYPE EXECUTABLE) - diff --git a/Code/Tools/AzTestRunner/Platform/Windows/platform_traits_windows.cmake b/Code/Tools/AzTestRunner/Platform/Windows/platform_traits_windows.cmake index 59922965bb..4c623c1f7b 100644 --- a/Code/Tools/AzTestRunner/Platform/Windows/platform_traits_windows.cmake +++ b/Code/Tools/AzTestRunner/Platform/Windows/platform_traits_windows.cmake @@ -9,5 +9,5 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # +set(PAL_TRAIT_AZTESTRUNNER_SUPPORTED TRUE) set(PAL_TRAIT_AZTESTRUNNER_LAUNCHER_TYPE EXECUTABLE) - diff --git a/Code/Tools/AzTestRunner/Platform/iOS/platform_traits_ios.cmake b/Code/Tools/AzTestRunner/Platform/iOS/platform_traits_ios.cmake index 59922965bb..4c623c1f7b 100644 --- a/Code/Tools/AzTestRunner/Platform/iOS/platform_traits_ios.cmake +++ b/Code/Tools/AzTestRunner/Platform/iOS/platform_traits_ios.cmake @@ -9,5 +9,5 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # +set(PAL_TRAIT_AZTESTRUNNER_SUPPORTED TRUE) set(PAL_TRAIT_AZTESTRUNNER_LAUNCHER_TYPE EXECUTABLE) - diff --git a/cmake/Platform/Common/Install_common.cmake b/cmake/Platform/Common/Install_common.cmake index 4aeaf21e95..a579e3128d 100644 --- a/cmake/Platform/Common/Install_common.cmake +++ b/cmake/Platform/Common/Install_common.cmake @@ -162,7 +162,18 @@ function(ly_setup_target OUTPUT_CONFIGURED_TARGET ALIAS_TARGET_NAME) elseif(target_type STREQUAL MODULE_LIBRARY) set(target_location "\${LY_ROOT_FOLDER}/${library_output_directory}/${PAL_PLATFORM_NAME}/$/${target_library_output_subdirectory}/$") elseif(target_type STREQUAL SHARED_LIBRARY) - string(APPEND target_file_contents "set_property(TARGET ${TARGET_NAME} PROPERTY IMPORTED_IMPLIB_$ \"\${LY_ROOT_FOLDER}/${archive_output_directory}/${PAL_PLATFORM_NAME}/$/$\")\n") + string(APPEND target_file_contents +"set_property(TARGET ${TARGET_NAME} + APPEND_STRING PROPERTY IMPORTED_IMPLIB + $<$$:\"\${LY_ROOT_FOLDER}/${archive_output_directory}/${PAL_PLATFORM_NAME}/$/$\"$ +) +") + string(APPEND target_file_contents +"set_property(TARGET ${TARGET_NAME} + PROPERTY IMPORTED_IMPLIB_$> + \"\${LY_ROOT_FOLDER}/${archive_output_directory}/${PAL_PLATFORM_NAME}/$/$\" +) +") set(target_location "\${LY_ROOT_FOLDER}/${library_output_directory}/${PAL_PLATFORM_NAME}/$/${target_library_output_subdirectory}/$") else() # STATIC_LIBRARY, OBJECT_LIBRARY, INTERFACE_LIBRARY set(target_location "\${LY_ROOT_FOLDER}/${archive_output_directory}/${PAL_PLATFORM_NAME}/$/$") diff --git a/cmake/Platform/Common/VisualStudio_common.cmake b/cmake/Platform/Common/VisualStudio_common.cmake index fd38a8c7ce..9a23a26d34 100644 --- a/cmake/Platform/Common/VisualStudio_common.cmake +++ b/cmake/Platform/Common/VisualStudio_common.cmake @@ -10,5 +10,5 @@ # if(CMAKE_GENERATOR MATCHES "Visual Studio 16") - configure_file("${CMAKE_CURRENT_LIST_DIR}/Directory.Build.props" "${CMAKE_CURRENT_BINARY_DIR}/Directory.Build.props" COPYONLY) + configure_file("${CMAKE_CURRENT_LIST_DIR}/Directory.Build.props" "${CMAKE_BINARY_DIR}/Directory.Build.props" COPYONLY) endif() \ No newline at end of file diff --git a/scripts/ctest/CMakeLists.txt b/scripts/ctest/CMakeLists.txt index f9824889e3..575be53cc7 100644 --- a/scripts/ctest/CMakeLists.txt +++ b/scripts/ctest/CMakeLists.txt @@ -16,35 +16,6 @@ if(NOT PAL_TRAIT_BUILD_TESTS_SUPPORTED) return() endif() -################################################################################ -# Asset Processing Target -# i.e. Tests depend on AutomatedTesting.Assets -################################################################################ - -if(PAL_TRAIT_BUILD_TESTS_SUPPORTED AND PAL_TRAIT_BUILD_HOST_TOOLS) - get_property(LY_PROJECTS_TARGET_NAME GLOBAL PROPERTY LY_PROJECTS_TARGET_NAME) - foreach(project_target_name project_path IN ZIP_LISTS LY_PROJECTS_TARGET_NAME LY_PROJECTS) - file(REAL_PATH ${project_path} project_real_path BASE_DIRECTORY ${LY_ROOT_FOLDER}) - # With the lock file, asset processing jobs are serialized to avoid race conditions - # on files that are created temporarily in source folders during shader processing. - add_custom_target(${project_target_name}.Assets - COMMENT "Processing ${project_target_name} assets..." - COMMAND "${CMAKE_COMMAND}" - -DLY_LOCK_FILE=$/project_assets.lock - -P ${LY_ROOT_FOLDER}/cmake/CommandExecution.cmake - EXEC_COMMAND $ - --zeroAnalysisMode - --project-path=${project_real_path} - --platforms=${LY_ASSET_DEPLOY_ASSET_TYPE} - ) - set_target_properties(${project_target_name}.Assets - PROPERTIES - EXCLUDE_FROM_ALL TRUE - FOLDER ${project_target_name} - ) - endforeach() -endif() - ################################################################################ # Tests ################################################################################ From 5f82e5a11134a4571a6b69d351a178cd6faf5372 Mon Sep 17 00:00:00 2001 From: bosnichd Date: Tue, 8 Jun 2021 15:15:20 -0600 Subject: [PATCH 08/16] Fix typo. (#1192) --- Gems/ImGui/Code/Source/LYCommonMenu/ImGuiLYCommonMenu.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gems/ImGui/Code/Source/LYCommonMenu/ImGuiLYCommonMenu.cpp b/Gems/ImGui/Code/Source/LYCommonMenu/ImGuiLYCommonMenu.cpp index 5107a02835..c750aea38d 100644 --- a/Gems/ImGui/Code/Source/LYCommonMenu/ImGuiLYCommonMenu.cpp +++ b/Gems/ImGui/Code/Source/LYCommonMenu/ImGuiLYCommonMenu.cpp @@ -693,7 +693,7 @@ namespace ImGui ImGui::TextColored(ImGui::Colors::s_PlainLabelColor, "Left Stick"); ImGui::NextColumn(); ImGui::Bullet(); - ImGui::TextColored(ImGui::Colors::s_PlainLabelColor, "Mova Mouse Pointer"); + ImGui::TextColored(ImGui::Colors::s_PlainLabelColor, "Move Mouse Pointer"); ImGui::Separator(); ImGui::NextColumn(); From 8cb6ef0721bb295b74bbf5c72c3bd1bc1d83ff86 Mon Sep 17 00:00:00 2001 From: chcurran Date: Tue, 8 Jun 2021 16:39:36 -0700 Subject: [PATCH 09/16] Fix tmeplate arg names --- .../AzCore/AzCore/RTTI/AzStdOnDemandReflection.inl | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/RTTI/AzStdOnDemandReflection.inl b/Code/Framework/AzCore/AzCore/RTTI/AzStdOnDemandReflection.inl index 2132dff3c4..9bda5cdcce 100644 --- a/Code/Framework/AzCore/AzCore/RTTI/AzStdOnDemandReflection.inl +++ b/Code/Framework/AzCore/AzCore/RTTI/AzStdOnDemandReflection.inl @@ -813,8 +813,8 @@ namespace AZ { using ContainerType = AZStd::tuple; - template - static void ReflectUnpackMethodFold(BehaviorContext::ClassBuilder& builder, const AZStd::vector& typeNames, [[maybe_unused]] size_t inputIndex) + template + static void ReflectUnpackMethodFold(BehaviorContext::ClassBuilder& builder, const AZStd::vector& typeNames) { const AZStd::string methodName = AZStd::string::format("Get%zu", Index); builder->Method(methodName.data(), [](ContainerType& thisPointer) { return AZStd::get(thisPointer); }) @@ -825,15 +825,15 @@ namespace AZ builder->Property ( AZStd::string::format("element_%zu_%s", Index, typeNames[Index].c_str()).c_str() , [](ContainerType& thisPointer) { return AZStd::get(thisPointer); } - , [](ContainerType& thisPointer, const t_Arg& element) { AZStd::get(thisPointer) = element; }); + , [](ContainerType& thisPointer, const Targ& element) { AZStd::get(thisPointer) = element; }); } - template + template static void ReflectUnpackMethods(BehaviorContext::ClassBuilder& builder, AZStd::index_sequence) { AZStd::vector typeNames; ScriptCanvasOnDemandReflection::GetTypeNames(typeNames, *builder.m_context); - (ReflectUnpackMethodFold(builder, typeNames, Indices), ...); + (ReflectUnpackMethodFold(builder, typeNames), ...); } static void Reflect(ReflectContext* context) From 9dec723e33e1bd2ab498b5516868ab076697dcb4 Mon Sep 17 00:00:00 2001 From: srikappa Date: Tue, 8 Jun 2021 17:28:23 -0700 Subject: [PATCH 10/16] Remove file size limits when loading prefabs and prefab-based-levels --- Code/Framework/AzCore/AzCore/Utils/Utils.cpp | 31 +++++++++++++++++++ Code/Framework/AzCore/AzCore/Utils/Utils.h | 7 ++++- .../PrefabEditorEntityOwnershipService.cpp | 8 +---- .../AzToolsFramework/Prefab/PrefabLoader.cpp | 2 +- .../Prefab/PrefabLoaderInterface.h | 2 -- 5 files changed, 39 insertions(+), 11 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/Utils/Utils.cpp b/Code/Framework/AzCore/AzCore/Utils/Utils.cpp index 7025b3977e..5521ff6131 100644 --- a/Code/Framework/AzCore/AzCore/Utils/Utils.cpp +++ b/Code/Framework/AzCore/AzCore/Utils/Utils.cpp @@ -169,6 +169,37 @@ namespace AZ::Utils template AZ::Outcome, AZStd::string> ReadFile(AZStd::string_view filePath, size_t maxFileSize); template AZ::Outcome, AZStd::string> ReadFile(AZStd::string_view filePath, size_t maxFileSize); + template + AZ::Outcome ReadFileWithNoSizeLimit(AZStd::string_view filePath) + { + IO::FileIOStream file; + if (!file.Open(filePath.data(), IO::OpenMode::ModeRead)) + { + return AZ::Failure(AZStd::string::format("Failed to open '%.*s'.", AZ_STRING_ARG(filePath))); + } + + AZ::IO::SizeType length = file.GetLength(); + + if (length == 0) + { + return AZ::Failure(AZStd::string::format("Failed to load '%.*s'. File is empty.", AZ_STRING_ARG(filePath))); + } + + Container fileContent; + fileContent.resize(length); + AZ::IO::SizeType bytesRead = file.Read(length, fileContent.data()); + file.Close(); + + // Resize again just in case bytesRead is less than length for some reason + fileContent.resize(bytesRead); + + return AZ::Success(AZStd::move(fileContent)); + } + + template AZ::Outcome ReadFileWithNoSizeLimit(AZStd::string_view filePath); + template AZ::Outcome, AZStd::string> ReadFileWithNoSizeLimit(AZStd::string_view filePath); + template AZ::Outcome, AZStd::string> ReadFileWithNoSizeLimit(AZStd::string_view filePath); + AZ::IO::FixedMaxPathString GetO3deManifestDirectory() { AZ::IO::FixedMaxPath path = GetHomeDirectory(); diff --git a/Code/Framework/AzCore/AzCore/Utils/Utils.h b/Code/Framework/AzCore/AzCore/Utils/Utils.h index 4e64ce5a39..6aaa1b868f 100644 --- a/Code/Framework/AzCore/AzCore/Utils/Utils.h +++ b/Code/Framework/AzCore/AzCore/Utils/Utils.h @@ -113,8 +113,13 @@ namespace AZ //! Save a string to a file. Otherwise returns a failure with error message. AZ::Outcome WriteFile(AZStd::string_view content, AZStd::string_view filePath); - //! Read a file into a string. Returns a failure with error message if the content could not be loaded. + //! Read a file into a string. Returns a failure with error message if the content could not be loaded or if + //! the file size is larger than the max file size provided. template AZ::Outcome ReadFile(AZStd::string_view filePath, size_t maxFileSize = DefaultMaxFileSize); + + //! Read a file into a string. Returns a failure with error message if the content could not be loaded. + template + AZ::Outcome ReadFileWithNoSizeLimit(AZStd::string_view filePath); } } diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp index 7fd11ff9bf..fa7d5f6e9b 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp @@ -185,13 +185,7 @@ namespace AzToolsFramework bool PrefabEditorEntityOwnershipService::LoadFromStream(AZ::IO::GenericStream& stream, AZStd::string_view filename) { Reset(); - // Make loading from stream to behave the same in terms of filesize as regular loading of prefabs - // This may need to be revisited in the future for supporting higher sizes along with prefab loading - if (stream.GetLength() > Prefab::MaxPrefabFileSize) - { - AZ_Error("Prefab", false, "'%.*s' prefab content is bigger than the max supported size (%f MB)", AZ_STRING_ARG(filename), Prefab::MaxPrefabFileSize / (1024.f * 1024.f)); - return false; - } + const size_t bufSize = stream.GetLength(); AZStd::unique_ptr buf(new char[bufSize]); AZ::IO::SizeType bytes = stream.Read(bufSize, buf.get()); diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabLoader.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabLoader.cpp index d7de634c11..25a16f18a8 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabLoader.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabLoader.cpp @@ -74,7 +74,7 @@ namespace AzToolsFramework return InvalidTemplateId; } - auto readResult = AZ::Utils::ReadFile(GetFullPath(filePath).Native(), MaxPrefabFileSize); + auto readResult = AZ::Utils::ReadFileWithNoSizeLimit(GetFullPath(filePath).Native()); if (!readResult.IsSuccess()) { AZ_Error( diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabLoaderInterface.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabLoaderInterface.h index 0e551cee6b..a9a1ee0607 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabLoaderInterface.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabLoaderInterface.h @@ -21,8 +21,6 @@ namespace AzToolsFramework { namespace Prefab { - constexpr size_t MaxPrefabFileSize = 1024 * 1024; - /*! * PrefabLoaderInterface * Interface for saving/loading Prefab files. From fd81000d53b2beea63a45d0353ed9f62e8fad280 Mon Sep 17 00:00:00 2001 From: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com> Date: Tue, 8 Jun 2021 17:44:30 -0700 Subject: [PATCH 11/16] Fix the relative path in the Source param for the PrefabLevel_OpensLevelWithEntities prefab (#1196) * Fixed the relative path in the Source param for the PrefabLevel_OpensLevelWithEntities prefab. Plus bonus capitalization fix. * Add empty Cached Transform Parent parameter to prevent patch application issues. This will be fixed once prefab sanitation on load is added. --- .../PythonTests/prefab/PrefabLevel_OpensLevelWithEntities.py | 2 +- .../PrefabLevel_OpensLevelWithEntities.prefab | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/AutomatedTesting/Gem/PythonTests/prefab/PrefabLevel_OpensLevelWithEntities.py b/AutomatedTesting/Gem/PythonTests/prefab/PrefabLevel_OpensLevelWithEntities.py index 3401c6a0a9..5860294f83 100644 --- a/AutomatedTesting/Gem/PythonTests/prefab/PrefabLevel_OpensLevelWithEntities.py +++ b/AutomatedTesting/Gem/PythonTests/prefab/PrefabLevel_OpensLevelWithEntities.py @@ -41,7 +41,7 @@ def PrefabLevel_OpensLevelWithEntities(): EXPECTED_EMPTY_ENTITY_POS = Vector3(10.00, 20.0, 30.0) helper.init_idle() - helper.open_level("prefab", "PrefabLevel_OpensLevelWithEntities") + helper.open_level("Prefab", "PrefabLevel_OpensLevelWithEntities") def find_entity(entity_name): searchFilter = entity.SearchFilter() diff --git a/AutomatedTesting/Levels/Prefab/PrefabLevel_OpensLevelWithEntities/PrefabLevel_OpensLevelWithEntities.prefab b/AutomatedTesting/Levels/Prefab/PrefabLevel_OpensLevelWithEntities/PrefabLevel_OpensLevelWithEntities.prefab index 0776f25935..2a26699fd5 100644 --- a/AutomatedTesting/Levels/Prefab/PrefabLevel_OpensLevelWithEntities/PrefabLevel_OpensLevelWithEntities.prefab +++ b/AutomatedTesting/Levels/Prefab/PrefabLevel_OpensLevelWithEntities/PrefabLevel_OpensLevelWithEntities.prefab @@ -1,5 +1,5 @@ { - "Source": "Levels/PrefabLevel_OpensLevelWithEntities/PrefabLevel_OpensLevelWithEntities.prefab", + "Source": "Levels/Prefab/PrefabLevel_OpensLevelWithEntities/PrefabLevel_OpensLevelWithEntities.prefab", "ContainerEntity": { "Id": "Entity_[403811863694]", "Name": "Level", @@ -15,7 +15,8 @@ "Component_[13764860261821571747]": { "$type": "{27F1E1A1-8D9D-4C3B-BD3A-AFB9762449C0} TransformComponent", "Id": 13764860261821571747, - "Parent Entity": "" + "Parent Entity": "", + "Cached World Transform Parent": "" }, "Component_[15844324401733835865]": { "$type": "EditorEntitySortComponent", From b23c95cab3872bb619768274ec29949f71de27c4 Mon Sep 17 00:00:00 2001 From: srikappa Date: Tue, 8 Jun 2021 17:58:59 -0700 Subject: [PATCH 12/16] Removed the new added flavor of ReadFile and reused existing one --- Code/Framework/AzCore/AzCore/Utils/Utils.cpp | 31 ------------------- Code/Framework/AzCore/AzCore/Utils/Utils.h | 4 --- .../AzToolsFramework/Prefab/PrefabLoader.cpp | 2 +- 3 files changed, 1 insertion(+), 36 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/Utils/Utils.cpp b/Code/Framework/AzCore/AzCore/Utils/Utils.cpp index 5521ff6131..7025b3977e 100644 --- a/Code/Framework/AzCore/AzCore/Utils/Utils.cpp +++ b/Code/Framework/AzCore/AzCore/Utils/Utils.cpp @@ -169,37 +169,6 @@ namespace AZ::Utils template AZ::Outcome, AZStd::string> ReadFile(AZStd::string_view filePath, size_t maxFileSize); template AZ::Outcome, AZStd::string> ReadFile(AZStd::string_view filePath, size_t maxFileSize); - template - AZ::Outcome ReadFileWithNoSizeLimit(AZStd::string_view filePath) - { - IO::FileIOStream file; - if (!file.Open(filePath.data(), IO::OpenMode::ModeRead)) - { - return AZ::Failure(AZStd::string::format("Failed to open '%.*s'.", AZ_STRING_ARG(filePath))); - } - - AZ::IO::SizeType length = file.GetLength(); - - if (length == 0) - { - return AZ::Failure(AZStd::string::format("Failed to load '%.*s'. File is empty.", AZ_STRING_ARG(filePath))); - } - - Container fileContent; - fileContent.resize(length); - AZ::IO::SizeType bytesRead = file.Read(length, fileContent.data()); - file.Close(); - - // Resize again just in case bytesRead is less than length for some reason - fileContent.resize(bytesRead); - - return AZ::Success(AZStd::move(fileContent)); - } - - template AZ::Outcome ReadFileWithNoSizeLimit(AZStd::string_view filePath); - template AZ::Outcome, AZStd::string> ReadFileWithNoSizeLimit(AZStd::string_view filePath); - template AZ::Outcome, AZStd::string> ReadFileWithNoSizeLimit(AZStd::string_view filePath); - AZ::IO::FixedMaxPathString GetO3deManifestDirectory() { AZ::IO::FixedMaxPath path = GetHomeDirectory(); diff --git a/Code/Framework/AzCore/AzCore/Utils/Utils.h b/Code/Framework/AzCore/AzCore/Utils/Utils.h index 6aaa1b868f..d082a3ebe0 100644 --- a/Code/Framework/AzCore/AzCore/Utils/Utils.h +++ b/Code/Framework/AzCore/AzCore/Utils/Utils.h @@ -117,9 +117,5 @@ namespace AZ //! the file size is larger than the max file size provided. template AZ::Outcome ReadFile(AZStd::string_view filePath, size_t maxFileSize = DefaultMaxFileSize); - - //! Read a file into a string. Returns a failure with error message if the content could not be loaded. - template - AZ::Outcome ReadFileWithNoSizeLimit(AZStd::string_view filePath); } } diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabLoader.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabLoader.cpp index 25a16f18a8..44dff93cb5 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabLoader.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabLoader.cpp @@ -74,7 +74,7 @@ namespace AzToolsFramework return InvalidTemplateId; } - auto readResult = AZ::Utils::ReadFileWithNoSizeLimit(GetFullPath(filePath).Native()); + auto readResult = AZ::Utils::ReadFile(GetFullPath(filePath).Native(), AZStd::numeric_limits::max()); if (!readResult.IsSuccess()) { AZ_Error( From 432b330d2293a30ef64a4929f132378226219581 Mon Sep 17 00:00:00 2001 From: Doug McDiarmid Date: Tue, 8 Jun 2021 21:24:01 -0700 Subject: [PATCH 13/16] Skipped empty unbounded arrays when updating the Vulkan DescriptorSet --- .../Code/Source/RHI/ShaderResourceGroupPool.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Gems/Atom/RHI/Vulkan/Code/Source/RHI/ShaderResourceGroupPool.cpp b/Gems/Atom/RHI/Vulkan/Code/Source/RHI/ShaderResourceGroupPool.cpp index 8771301bb1..289975dc56 100644 --- a/Gems/Atom/RHI/Vulkan/Code/Source/RHI/ShaderResourceGroupPool.cpp +++ b/Gems/Atom/RHI/Vulkan/Code/Source/RHI/ShaderResourceGroupPool.cpp @@ -135,6 +135,12 @@ namespace AZ { const RHI::ShaderInputBufferUnboundedArrayIndex index(groupIndex); auto bufViews = groupData.GetBufferViewUnboundedArray(index); + if (bufViews.empty()) + { + // skip empty unbounded arrays + continue; + } + uint32_t layoutIndex = m_descriptorSetLayout->GetLayoutIndexFromGroupIndex(groupIndex, DescriptorSetLayout::ResourceType::BufferViewUnboundedArray); descriptorSet.UpdateBufferViews(layoutIndex, bufViews); } @@ -144,6 +150,12 @@ namespace AZ { const RHI::ShaderInputImageUnboundedArrayIndex index(groupIndex); auto imgViews = groupData.GetImageViewUnboundedArray(index); + if (imgViews.empty()) + { + // skip empty unbounded arrays + continue; + } + uint32_t layoutIndex = m_descriptorSetLayout->GetLayoutIndexFromGroupIndex(groupIndex, DescriptorSetLayout::ResourceType::ImageViewUnboundedArray); descriptorSet.UpdateImageViews(layoutIndex, imgViews, shaderImageUnboundeArrayList[groupIndex].m_type); } From 3fd2f1305f2f8c6de2982570763efd37783264f2 Mon Sep 17 00:00:00 2001 From: rhongAMZ <69218254+rhongAMZ@users.noreply.github.com> Date: Wed, 9 Jun 2021 09:15:09 -0700 Subject: [PATCH 14/16] Selecting and deleting the level prefab root entity crashes editor (#1179) Early remove the level instance from the entity id list in the delete function and duplicate function. --- .../Prefab/PrefabPublicHandler.cpp | 40 +++++++++++++++---- .../Prefab/PrefabPublicHandler.h | 1 + 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp index fcdbc5ce07..690d408007 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp @@ -901,7 +901,13 @@ namespace AzToolsFramework return AZ::Failure(AZStd::string("No entities to duplicate.")); } - if (!EntitiesBelongToSameInstance(entityIds)) + const EntityIdList entityIdsNoLevelInstance = GenerateEntityIdListWithoutLevelInstance(entityIds); + if (entityIdsNoLevelInstance.empty()) + { + return AZ::Failure(AZStd::string("No entities to duplicate because only instance selected is the level instance.")); + } + + if (!EntitiesBelongToSameInstance(entityIdsNoLevelInstance)) { return AZ::Failure(AZStd::string("Cannot duplicate multiple entities belonging to different instances with one operation." "Change your selection to contain entities in the same instance.")); @@ -909,7 +915,7 @@ namespace AzToolsFramework // We've already verified the entities are all owned by the same instance, // so we can just retrieve our instance from the first entity in the list. - AZ::EntityId firstEntityIdToDuplicate = entityIds[0]; + AZ::EntityId firstEntityIdToDuplicate = entityIdsNoLevelInstance[0]; InstanceOptionalReference commonOwningInstance = GetOwnerInstanceByEntityId(firstEntityIdToDuplicate); if (!commonOwningInstance.has_value()) { @@ -929,7 +935,7 @@ namespace AzToolsFramework // This will cull out any entities that have ancestors in the list, since we will end up duplicating // the full nested hierarchy with what is returned from RetrieveAndSortPrefabEntitiesAndInstances - AzToolsFramework::EntityIdSet duplicationSet = AzToolsFramework::GetCulledEntityHierarchy(entityIds); + AzToolsFramework::EntityIdSet duplicationSet = AzToolsFramework::GetCulledEntityHierarchy(entityIdsNoLevelInstance); AZ_PROFILE_FUNCTION(AZ::Debug::ProfileCategory::AzToolsFramework); @@ -1004,17 +1010,19 @@ namespace AzToolsFramework PrefabOperationResult PrefabPublicHandler::DeleteFromInstance(const EntityIdList& entityIds, bool deleteDescendants) { - if (entityIds.empty()) + const EntityIdList entityIdsNoLevelInstance = GenerateEntityIdListWithoutLevelInstance(entityIds); + + if (entityIdsNoLevelInstance.empty()) { return AZ::Success(); } - if (!EntitiesBelongToSameInstance(entityIds)) + if (!EntitiesBelongToSameInstance(entityIdsNoLevelInstance)) { return AZ::Failure(AZStd::string("Cannot delete multiple entities belonging to different instances with one operation.")); } - AZ::EntityId firstEntityIdToDelete = entityIds[0]; + AZ::EntityId firstEntityIdToDelete = entityIdsNoLevelInstance[0]; InstanceOptionalReference commonOwningInstance = GetOwnerInstanceByEntityId(firstEntityIdToDelete); // If the first entity id is a container entity id, then we need to mark its parent as the common owning instance because you @@ -1025,7 +1033,7 @@ namespace AzToolsFramework } // Retrieve entityList from entityIds - EntityList inputEntityList = EntityIdListToEntityList(entityIds); + EntityList inputEntityList = EntityIdListToEntityList(entityIdsNoLevelInstance); AZ_PROFILE_FUNCTION(AZ::Debug::ProfileCategory::AzToolsFramework); @@ -1081,7 +1089,7 @@ namespace AzToolsFramework } else { - for (AZ::EntityId entityId : entityIds) + for (AZ::EntityId entityId : entityIdsNoLevelInstance) { InstanceOptionalReference owningInstance = m_instanceEntityMapperInterface->FindOwningInstance(entityId); // If this is the container entity, it actually represents the instance so get its owner @@ -1437,6 +1445,22 @@ namespace AzToolsFramework return (outEntities.size() + outInstances.size()) > 0; } + EntityIdList PrefabPublicHandler::GenerateEntityIdListWithoutLevelInstance( + const EntityIdList& entityIds) const + { + EntityIdList outEntityIds; + outEntityIds.reserve(entityIds.size()); // Actual size could be smaller. + + for (const AZ::EntityId& entityId : entityIds) + { + if (!IsLevelInstanceContainerEntity(entityId)) + { + outEntityIds.emplace_back(entityId); + } + } + return outEntityIds; + } + bool PrefabPublicHandler::EntitiesBelongToSameInstance(const EntityIdList& entityIds) const { if (entityIds.size() <= 1) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h index f3d778d8de..65e1391722 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h @@ -70,6 +70,7 @@ namespace AzToolsFramework PrefabOperationResult DeleteFromInstance(const EntityIdList& entityIds, bool deleteDescendants); bool RetrieveAndSortPrefabEntitiesAndInstances(const EntityList& inputEntities, Instance& commonRootEntityOwningInstance, EntityList& outEntities, AZStd::vector& outInstances) const; + EntityIdList GenerateEntityIdListWithoutLevelInstance(const EntityIdList& entityIds) const; InstanceOptionalReference GetOwnerInstanceByEntityId(AZ::EntityId entityId) const; bool EntitiesBelongToSameInstance(const EntityIdList& entityIds) const; From 4241eb9c2cec0d525bf0b3ff5ea2c616268e65cf Mon Sep 17 00:00:00 2001 From: amzn-sean <75276488+amzn-sean@users.noreply.github.com> Date: Wed, 9 Jun 2021 17:42:16 +0100 Subject: [PATCH 15/16] fix crash in ragdoll (#1210) --- .../Source/PhysXCharacters/API/Ragdoll.cpp | 22 ++++++++++++++++--- .../Code/Source/PhysXCharacters/API/Ragdoll.h | 2 ++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/Gems/PhysX/Code/Source/PhysXCharacters/API/Ragdoll.cpp b/Gems/PhysX/Code/Source/PhysXCharacters/API/Ragdoll.cpp index 02ebe7281c..4725212a9d 100644 --- a/Gems/PhysX/Code/Source/PhysXCharacters/API/Ragdoll.cpp +++ b/Gems/PhysX/Code/Source/PhysXCharacters/API/Ragdoll.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -40,6 +41,8 @@ namespace PhysX } // namespace Internal // PhysX::Ragdoll + /*static*/ AZStd::mutex Ragdoll::m_sceneEventMutex; + void Ragdoll::Reflect(AZ::ReflectContext* context) { AZ::SerializeContext* serializeContext = azrtti_cast(context); @@ -114,7 +117,10 @@ namespace PhysX Ragdoll::~Ragdoll() { - m_sceneStartSimHandler.Disconnect(); + { + AZStd::scoped_lock lock(m_sceneEventMutex); + m_sceneStartSimHandler.Disconnect(); + } m_nodes.clear(); //the nodes destructor will remove the simulated body from the scene. } @@ -212,7 +218,13 @@ namespace PhysX } } - sceneInterface->RegisterSceneSimulationStartHandler(m_sceneOwner, m_sceneStartSimHandler); + // the handler is also connected in EnableSimulationQueued(), + // which will call this function, so if called from that path dont connect here. + if (!m_sceneStartSimHandler.IsConnected()) + { + AZStd::scoped_lock lock(m_sceneEventMutex); + sceneInterface->RegisterSceneSimulationStartHandler(m_sceneOwner, m_sceneStartSimHandler); + } sceneInterface->EnableSimulationOfBody(m_sceneOwner, m_bodyHandle); } @@ -225,6 +237,7 @@ namespace PhysX if (auto* sceneInterface = AZ::Interface::Get()) { + AZStd::scoped_lock lock(m_sceneEventMutex); sceneInterface->RegisterSceneSimulationStartHandler(m_sceneOwner, m_sceneStartSimHandler); } @@ -244,7 +257,10 @@ namespace PhysX return; } - m_sceneStartSimHandler.Disconnect(); + { + AZStd::scoped_lock lock(m_sceneEventMutex); + m_sceneStartSimHandler.Disconnect(); + } physx::PxScene* pxScene = Internal::GetPxScene(m_sceneOwner); const size_t numNodes = m_nodes.size(); diff --git a/Gems/PhysX/Code/Source/PhysXCharacters/API/Ragdoll.h b/Gems/PhysX/Code/Source/PhysXCharacters/API/Ragdoll.h index 7182a3a44c..006ce5878b 100644 --- a/Gems/PhysX/Code/Source/PhysXCharacters/API/Ragdoll.h +++ b/Gems/PhysX/Code/Source/PhysXCharacters/API/Ragdoll.h @@ -12,6 +12,7 @@ #pragma once +#include #include #include #include @@ -84,5 +85,6 @@ namespace PhysX bool m_queuedDisableSimulation = false; AzPhysics::SceneEvents::OnSceneSimulationStartHandler m_sceneStartSimHandler; + static AZStd::mutex m_sceneEventMutex; }; } // namespace PhysX From b26b472bba54f266ce4c5d131574a680704b6322 Mon Sep 17 00:00:00 2001 From: AMZN-nggieber <52797929+AMZN-nggieber@users.noreply.github.com> Date: Wed, 9 Jun 2021 11:59:15 -0700 Subject: [PATCH 16/16] Fix Editor being opened twice by project (#1213) --- Code/Tools/ProjectManager/Source/ProjectButtonWidget.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/Code/Tools/ProjectManager/Source/ProjectButtonWidget.cpp b/Code/Tools/ProjectManager/Source/ProjectButtonWidget.cpp index db1b1a4850..761572ffa5 100644 --- a/Code/Tools/ProjectManager/Source/ProjectButtonWidget.cpp +++ b/Code/Tools/ProjectManager/Source/ProjectButtonWidget.cpp @@ -151,7 +151,6 @@ namespace O3DE::ProjectManager void ProjectButton::ReadySetup() { - connect(m_projectImageLabel, &LabelButton::triggered, [this]() { emit OpenProject(m_projectInfo.m_path); }); connect(m_projectImageLabel->GetBuildButton(), &QPushButton::clicked, [this](){ emit BuildProject(m_projectInfo); }); QMenu* menu = new QMenu(this);