From edbe05bd81eaff3b5bad649043af5cd6f692efed Mon Sep 17 00:00:00 2001 From: Chris Galvan Date: Thu, 22 Jul 2021 11:57:40 -0500 Subject: [PATCH 01/24] Fixed Project Manager to enable save/restore of the window geometry so its window position/sizing will be restored after it is closed/re-launched. Also improved WindowDecorationWrapper logic to center by default when using the showFromSettings API for first time launch. Signed-off-by: Chris Galvan --- .../AzQtComponents/Components/WindowDecorationWrapper.cpp | 4 ++++ Code/Tools/ProjectManager/Source/Application.cpp | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/Code/Framework/AzQtComponents/AzQtComponents/Components/WindowDecorationWrapper.cpp b/Code/Framework/AzQtComponents/AzQtComponents/Components/WindowDecorationWrapper.cpp index 25aa575727..36f00f41f6 100644 --- a/Code/Framework/AzQtComponents/AzQtComponents/Components/WindowDecorationWrapper.cpp +++ b/Code/Framework/AzQtComponents/AzQtComponents/Components/WindowDecorationWrapper.cpp @@ -661,6 +661,10 @@ namespace AzQtComponents if (!restoreGeometryFromSettings()) { show(); + + // If we failed to restore from settings (the first time this window is loaded), + // then center it on the screen by default + centerOnScreen(this); } } diff --git a/Code/Tools/ProjectManager/Source/Application.cpp b/Code/Tools/ProjectManager/Source/Application.cpp index 8888febe3b..bdcb59897b 100644 --- a/Code/Tools/ProjectManager/Source/Application.cpp +++ b/Code/Tools/ProjectManager/Source/Application.cpp @@ -170,7 +170,8 @@ namespace O3DE::ProjectManager // the decoration wrapper is intended to remember window positioning and sizing auto wrapper = new AzQtComponents::WindowDecorationWrapper(); wrapper->setGuest(m_mainWindow.data()); - wrapper->show(); + wrapper->enableSaveRestoreGeometry("O3DE", "ProjectManager", "mainWindowGeometry"); + wrapper->showFromSettings(); m_mainWindow->show(); qApp->setQuitOnLastWindowClosed(true); From ce3b9e2917775d148dca3517ceb0ef8fa9aa5b51 Mon Sep 17 00:00:00 2001 From: Chris Galvan Date: Thu, 22 Jul 2021 12:34:41 -0500 Subject: [PATCH 02/24] Improved auto-resizing when centering dialogs when they are first displayed. Signed-off-by: Chris Galvan --- .../Components/WindowDecorationWrapper.cpp | 40 ++++++++++++++++++- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/Code/Framework/AzQtComponents/AzQtComponents/Components/WindowDecorationWrapper.cpp b/Code/Framework/AzQtComponents/AzQtComponents/Components/WindowDecorationWrapper.cpp index 25aa575727..96c158f032 100644 --- a/Code/Framework/AzQtComponents/AzQtComponents/Components/WindowDecorationWrapper.cpp +++ b/Code/Framework/AzQtComponents/AzQtComponents/Components/WindowDecorationWrapper.cpp @@ -240,15 +240,51 @@ namespace AzQtComponents // Center within the parent QRect geo = geometry(); + + // If the base size of the guest widget is larger than the screen, + // then we need to resize it so that it will fit by either using + // the minimum size (if one is set), or fallback to the screen size. + if (m_guestWidget) + { + if (auto screen = m_guestWidget->screen()) + { + const QRect screenGeometry = screen->availableGeometry(); + if (geo.width() > screenGeometry.width()) + { + auto guestMinimumWidth = m_guestWidget->minimumWidth(); + if (guestMinimumWidth && guestMinimumWidth <= screenGeometry.width()) + { + geo.setWidth(guestMinimumWidth); + } + else + { + geo.setWidth(screenGeometry.width()); + } + } + else if (geo.height() > screenGeometry.height()) + { + auto guestMinimumHeight = m_guestWidget->minimumHeight(); + if (guestMinimumHeight && guestMinimumHeight <= screenGeometry.height()) + { + geo.setHeight(guestMinimumHeight); + } + else + { + geo.setHeight(screenGeometry.height()); + } + } + } + } + geo.moveCenter(parentWindowCenter); - QWindow *w = topLevelWidget->windowHandle(); + QWindow* w = topLevelWidget->windowHandle(); if (!w) { return; } - QScreen *screen = w->screen(); + QScreen* screen = w->screen(); if (!screen) { // defensive, shouldn't happen From 8cf7f8ed7bc8826b2a3ef98f23ea08e8aa835088 Mon Sep 17 00:00:00 2001 From: Chris Galvan Date: Thu, 22 Jul 2021 12:49:51 -0500 Subject: [PATCH 03/24] Update to check both width and height at the same time. Signed-off-by: Chris Galvan --- .../AzQtComponents/Components/WindowDecorationWrapper.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Code/Framework/AzQtComponents/AzQtComponents/Components/WindowDecorationWrapper.cpp b/Code/Framework/AzQtComponents/AzQtComponents/Components/WindowDecorationWrapper.cpp index 96c158f032..505858218f 100644 --- a/Code/Framework/AzQtComponents/AzQtComponents/Components/WindowDecorationWrapper.cpp +++ b/Code/Framework/AzQtComponents/AzQtComponents/Components/WindowDecorationWrapper.cpp @@ -261,7 +261,7 @@ namespace AzQtComponents geo.setWidth(screenGeometry.width()); } } - else if (geo.height() > screenGeometry.height()) + if (geo.height() > screenGeometry.height()) { auto guestMinimumHeight = m_guestWidget->minimumHeight(); if (guestMinimumHeight && guestMinimumHeight <= screenGeometry.height()) From 5ad0cac085d04ded92bfbb205b4edec372f57cd2 Mon Sep 17 00:00:00 2001 From: moudgils <47460854+moudgils@users.noreply.github.com> Date: Thu, 22 Jul 2021 10:57:30 -0700 Subject: [PATCH 04/24] Deleting a missed file from a previous PR (#2350) Signed-off-by: moudgils --- .../Assets/Passes/Reflections_nomsaa.pass | 298 ------------------ 1 file changed, 298 deletions(-) delete mode 100644 Gems/Atom/Feature/Common/Assets/Passes/Reflections_nomsaa.pass diff --git a/Gems/Atom/Feature/Common/Assets/Passes/Reflections_nomsaa.pass b/Gems/Atom/Feature/Common/Assets/Passes/Reflections_nomsaa.pass deleted file mode 100644 index 64292da63c..0000000000 --- a/Gems/Atom/Feature/Common/Assets/Passes/Reflections_nomsaa.pass +++ /dev/null @@ -1,298 +0,0 @@ -{ - "Type": "JsonSerialization", - "Version": 1, - "ClassName": "PassAsset", - "ClassData": { - "PassTemplate": { - "Name": "ReflectionsParentPass_nomsaaTemplate", - "PassClass": "ParentPass", - "Slots": [ - { - "Name": "NormalInput", - "SlotType": "Input", - "ScopeAttachmentUsage": "Shader" - }, - { - "Name": "SpecularF0Input", - "SlotType": "Input", - "ScopeAttachmentUsage": "Shader" - }, - { - "Name": "DepthStencilInputOutput", - "SlotType": "InputOutput", - "ScopeAttachmentUsage": "DepthStencil" - }, - { - "Name": "SpecularInputOutput", - "SlotType": "InputOutput", - "ScopeAttachmentUsage": "RenderTarget" - }, - { - "Name": "ReflectionOutput", - "SlotType": "Output", - "ScopeAttachmentUsage": "RenderTarget" - } - ], - "Connections": [ - { - "LocalSlot": "ReflectionOutput", - "AttachmentRef": { - "Pass": "ReflectionProbeRenderInnerPass", - "Attachment": "ReflectionInputOutput" - } - } - ], - "PassRequests": [ - { - "Name": "ReflectionProbeStencilPass", - "TemplateName": "ReflectionProbeStencilPassTemplate", - "Connections": [ - { - "LocalSlot": "DepthStencilInputOutput", - "AttachmentRef": { - "Pass": "Parent", - "Attachment": "DepthStencilInputOutput" - } - } - ], - "PassData": { - "$type": "RasterPassData", - "DrawListTag": "reflectionprobestencil", - "PipelineViewTag": "MainCamera" - } - }, - { - "Name": "ReflectionProbeBlendWeightPass", - "TemplateName": "ReflectionProbeBlendWeightPassTemplate", - "Connections": [ - { - "LocalSlot": "DepthStencilTextureInput", - "AttachmentRef": { - "Pass": "ReflectionProbeStencilPass", - "Attachment": "DepthStencilInputOutput" - } - }, - { - "LocalSlot": "DepthStencilInput", - "AttachmentRef": { - "Pass": "ReflectionProbeStencilPass", - "Attachment": "DepthStencilInputOutput" - } - } - ], - "PassData": { - "$type": "RasterPassData", - "DrawListTag": "reflectionprobeblendweight", - "PipelineViewTag": "MainCamera", - "PassSrgShaderAsset": { - "FilePath": "Shaders/Reflections/ReflectionProbeBlendWeight.shader" - } - } - }, - { - "Name": "ReflectionGlobalFullscreenPass", - "TemplateName": "ReflectionGlobalFullscreenPass_nomsaaTemplate", - "Connections": [ - { - "LocalSlot": "DepthStencilTextureInput", - "AttachmentRef": { - "Pass": "ReflectionProbeStencilPass", - "Attachment": "DepthStencilInputOutput" - } - }, - { - "LocalSlot": "NormalInput", - "AttachmentRef": { - "Pass": "Parent", - "Attachment": "NormalInput" - } - }, - { - "LocalSlot": "DepthStencilInput", - "AttachmentRef": { - "Pass": "ReflectionProbeStencilPass", - "Attachment": "DepthStencilInputOutput" - - } - }, - { - "LocalSlot": "SpecularF0Input", - "AttachmentRef": { - "Pass": "Parent", - "Attachment": "SpecularF0Input" - } - }, - { - "LocalSlot": "ReflectionBlendWeightInput", - "AttachmentRef": { - "Pass": "ReflectionProbeBlendWeightPass", - "Attachment": "Output" - } - } - ], - "PassData": { - "$type": "FullscreenTrianglePassData", - "ShaderAsset": { - "FilePath": "Shaders/Reflections/ReflectionGlobalFullscreen_nomsaa.shader" - }, - "StencilRef": 3, // See RenderCommon.h and ReflectionGlobalFullscreen_nomsaa.shader - "PipelineViewTag": "MainCamera" - } - }, - { - "Name": "ReflectionProbeRenderOuterPass", - "TemplateName": "ReflectionProbeRenderOuterPassTemplate", - "Connections": [ - { - "LocalSlot": "ReflectionInputOutput", - "AttachmentRef": { - "Pass": "ReflectionGlobalFullscreenPass", - "Attachment": "ReflectionOutput" - } - }, - { - "LocalSlot": "DepthStencilTextureInput", - "AttachmentRef": { - "Pass": "ReflectionProbeStencilPass", - "Attachment": "DepthStencilInputOutput" - } - }, - { - "LocalSlot": "NormalInput", - "AttachmentRef": { - "Pass": "Parent", - "Attachment": "NormalInput" - } - }, - { - "LocalSlot": "DepthStencilInput", - "AttachmentRef": { - "Pass": "ReflectionProbeStencilPass", - "Attachment": "DepthStencilInputOutput" - - } - }, - { - "LocalSlot": "SpecularF0Input", - "AttachmentRef": { - "Pass": "Parent", - "Attachment": "SpecularF0Input" - } - }, - { - "LocalSlot": "ReflectionBlendWeightInput", - "AttachmentRef": { - "Pass": "ReflectionProbeBlendWeightPass", - "Attachment": "Output" - } - } - ], - "PassData": { - "$type": "RasterPassData", - "DrawListTag": "reflectionproberenderouter", - "PipelineViewTag": "MainCamera", - "PassSrgShaderAsset": { - "FilePath": "Shaders/Reflections/ReflectionProbeRenderOuter.shader" - } - } - }, - { - "Name": "ReflectionProbeRenderInnerPass", - "TemplateName": "ReflectionProbeRenderInnerPassTemplate", - "Connections": [ - { - "LocalSlot": "ReflectionInputOutput", - "AttachmentRef": { - "Pass": "ReflectionProbeRenderOuterPass", - "Attachment": "ReflectionInputOutput" - } - }, - { - "LocalSlot": "DepthStencilTextureInput", - "AttachmentRef": { - "Pass": "ReflectionProbeStencilPass", - "Attachment": "DepthStencilInputOutput" - } - }, - { - "LocalSlot": "NormalInput", - "AttachmentRef": { - "Pass": "Parent", - "Attachment": "NormalInput" - } - }, - { - "LocalSlot": "DepthStencilInput", - "AttachmentRef": { - "Pass": "ReflectionProbeStencilPass", - "Attachment": "DepthStencilInputOutput" - - } - }, - { - "LocalSlot": "SpecularF0Input", - "AttachmentRef": { - "Pass": "Parent", - "Attachment": "SpecularF0Input" - } - } - ], - "PassData": { - "$type": "RasterPassData", - "DrawListTag": "reflectionproberenderinner", - "PipelineViewTag": "MainCamera", - "PassSrgShaderAsset": { - "FilePath": "Shaders/Reflections/ReflectionProbeRenderInner.shader" - } - } - }, - { - // Using cut-down version of screen space reflection for handling the attachment format compatibility issues. - // This is used for mobile devices with limited capabilities. - "Name": "ReflectionScreenSpaceMobilePass", - "TemplateName": "ReflectionScreenSpaceMobilePassTemplate", - "Enabled": false, - "Connections": [ - { - "LocalSlot": "ReflectionInputOutput", - "AttachmentRef": { - "Pass": "ReflectionProbeRenderInnerPass", - "Attachment": "ReflectionInputOutput" - } - }, - { - "LocalSlot": "NormalInput", - "AttachmentRef": { - "Pass": "Parent", - "Attachment": "NormalInput" - } - }, - { - "LocalSlot": "DepthStencilInput", - "AttachmentRef": { - "Pass": "ReflectionProbeStencilPass", - "Attachment": "DepthStencilInputOutput" - - } - }, - { - "LocalSlot": "SpecularF0Input", - "AttachmentRef": { - "Pass": "Parent", - "Attachment": "SpecularF0Input" - } - }, - { - "LocalSlot": "SpecularInput", - "AttachmentRef": { - "Pass": "Parent", - "Attachment": "SpecularInputOutput" - } - } - ] - } - ] - } - } -} - From a007f1a4622b825fd9b54d66fdf615b51673e381 Mon Sep 17 00:00:00 2001 From: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> Date: Thu, 22 Jul 2021 11:20:58 -0700 Subject: [PATCH 05/24] Issues/2126 PhysX Gem can't be used as build dependency in engine SDK Part (#2337) (#2340) * Applying GENEX_EVAL to 2 cases where the genex can produce another genex Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> * Missing case when the folder is an external one to the engine/project (e.g. external gems) Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> --- Code/LauncherUnified/launcher_generator.cmake | 4 ++-- cmake/LYWrappers.cmake | 15 ++++++++++----- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/Code/LauncherUnified/launcher_generator.cmake b/Code/LauncherUnified/launcher_generator.cmake index 5a0c6f2801..9c8a6b8f16 100644 --- a/Code/LauncherUnified/launcher_generator.cmake +++ b/Code/LauncherUnified/launcher_generator.cmake @@ -41,9 +41,9 @@ foreach(project_name project_path IN ZIP_LISTS LY_PROJECTS_TARGET_NAME LY_PROJEC add_custom_target(${project_name}.Assets COMMENT "Processing ${project_name} assets..." COMMAND "${CMAKE_COMMAND}" - -DLY_LOCK_FILE=$/project_assets.lock + -DLY_LOCK_FILE=$>/project_assets.lock -P ${LY_ROOT_FOLDER}/cmake/CommandExecution.cmake - EXEC_COMMAND $ + EXEC_COMMAND $> --zeroAnalysisMode --project-path=${project_real_path} --platforms=${LY_ASSET_DEPLOY_ASSET_TYPE} diff --git a/cmake/LYWrappers.cmake b/cmake/LYWrappers.cmake index 7c554d2b0c..e4398099d1 100644 --- a/cmake/LYWrappers.cmake +++ b/cmake/LYWrappers.cmake @@ -392,10 +392,10 @@ function(ly_delayed_target_link_libraries) endif() if(item_type STREQUAL MODULE_LIBRARY) - target_include_directories(${target} ${visibility} $) - target_link_libraries(${target} ${visibility} $) - target_compile_definitions(${target} ${visibility} $) - target_compile_options(${target} ${visibility} $) + target_include_directories(${target} ${visibility} $>) + target_link_libraries(${target} ${visibility} $>) + target_compile_definitions(${target} ${visibility} $>) + target_compile_options(${target} ${visibility} $>) else() ly_parse_third_party_dependencies(${item}) target_link_libraries(${target} ${visibility} ${item}) @@ -660,7 +660,12 @@ function(ly_get_vs_folder_directory absolute_target_source_dir output_source_dir if(is_target_prefix_of_engine_root) cmake_path(RELATIVE_PATH absolute_target_source_dir BASE_DIRECTORY ${LY_ROOT_FOLDER} OUTPUT_VARIABLE relative_target_source_dir) else() - cmake_path(GET absolute_target_source_dir RELATIVE_PART relative_target_source_dir) + cmake_path(IS_PREFIX CMAKE_SOURCE_DIR ${absolute_target_source_dir} is_target_prefix_of_source_dir) + if(is_target_prefix_of_source_dir) + cmake_path(RELATIVE_PATH absolute_target_source_dir BASE_DIRECTORY ${CMAKE_SOURCE_DIR} OUTPUT_VARIABLE relative_target_source_dir) + else() + cmake_path(GET absolute_target_source_dir RELATIVE_PART relative_target_source_dir) + endif() endif() set(${output_source_dir} ${relative_target_source_dir} PARENT_SCOPE) From 90ef2ba13f2f0704fbf6575c488de08892829ca6 Mon Sep 17 00:00:00 2001 From: Chris Galvan Date: Thu, 22 Jul 2021 15:21:08 -0500 Subject: [PATCH 06/24] Removed slice based context menu options from the RPE when prefabs are enabled. Signed-off-by: Chris Galvan --- .../UI/PropertyEditor/EntityPropertyEditor.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/EntityPropertyEditor.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/EntityPropertyEditor.cpp index 319307009c..17155076ec 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/EntityPropertyEditor.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/EntityPropertyEditor.cpp @@ -1951,12 +1951,13 @@ namespace AzToolsFramework return; } + // If prefabs are enabled, there will be no root slice so bail out here since we don't need + // to show any slice options in the menu AZ::SliceComponent* rootSlice = nullptr; AzFramework::SliceEntityOwnershipServiceRequestBus::EventResult(rootSlice, contextId, &AzFramework::SliceEntityOwnershipServiceRequestBus::Events::GetRootSlice); if (!rootSlice) { - AZ_Error("PropertyEditor", false, "Entity context has no root slice"); return; } @@ -2105,10 +2106,6 @@ namespace AzToolsFramework { QMenu* revertMenu = nullptr; - revertMenu = menu.addMenu(tr("Revert overrides")); - revertMenu->setToolTipsVisible(true); - revertMenu->setEnabled(false); - //check for changes on selected property if (componentClassData) { @@ -2128,6 +2125,11 @@ namespace AzToolsFramework return; } + // Only add the "Revert overrides" menu option if it belongs to a slice + revertMenu = menu.addMenu(tr("Revert overrides")); + revertMenu->setToolTipsVisible(true); + revertMenu->setEnabled(false); + if (fieldNode) { bool hasChanges = fieldNode->HasChangesVersusComparison(false); From 4fa4ad27db076c07b19faaf8d71909fc36fa9268 Mon Sep 17 00:00:00 2001 From: Ken Pruiksma Date: Thu, 22 Jul 2021 16:01:50 -0500 Subject: [PATCH 07/24] Fixing missing definition of GpuCrashTracker for Aftermath. (#2354) Recently some changes were made to pch, and this was probably missed. This will just include the file directly when Aftermath is enabled. Signed-off-by: Ken Pruiksma --- .../DX12/Code/Source/Platform/Windows/RHI/Device_Windows.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Gems/Atom/RHI/DX12/Code/Source/Platform/Windows/RHI/Device_Windows.h b/Gems/Atom/RHI/DX12/Code/Source/Platform/Windows/RHI/Device_Windows.h index 4be50ca536..01608d2b61 100644 --- a/Gems/Atom/RHI/DX12/Code/Source/Platform/Windows/RHI/Device_Windows.h +++ b/Gems/Atom/RHI/DX12/Code/Source/Platform/Windows/RHI/Device_Windows.h @@ -9,6 +9,10 @@ #include +#if defined(USE_NSIGHT_AFTERMATH) + #include +#endif + namespace AZ { namespace DX12 From 70c9ed93f8ab72b2d856ff7c6eace36a52746067 Mon Sep 17 00:00:00 2001 From: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> Date: Thu, 22 Jul 2021 16:42:51 -0700 Subject: [PATCH 08/24] Make locations for LY_PACKAGE_DOWNLOAD_CACHE_LOCATION and LY_PACKAGE_UNPACK_LOCATION relative to LY_3RDPARTY_PATH across configurations (#2360) Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> --- cmake/3rdPartyPackages.cmake | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/cmake/3rdPartyPackages.cmake b/cmake/3rdPartyPackages.cmake index 2228e36653..efe67b4d24 100644 --- a/cmake/3rdPartyPackages.cmake +++ b/cmake/3rdPartyPackages.cmake @@ -40,22 +40,18 @@ endif() # If you keep packages after downloading, then they can be moved to a network share # or checked into source control so that others on the same project can avoid re-downloading set(LY_PACKAGE_KEEP_AFTER_DOWNLOADING TRUE CACHE BOOL "If enabled, packages will be kept after downloading them for later re-use") -set(LY_PACKAGE_DOWNLOAD_CACHE_LOCATION ${LY_3RDPARTY_PATH}/downloaded_packages CACHE PATH "You can make it store the packages in a folder of your choosing") +set(LY_PACKAGE_DOWNLOAD_CACHE_LOCATION @LY_3RDPARTY_PATH@/downloaded_packages CACHE PATH "Download location for packages (Defaults to @LY_3RDPARTY_PATH@/downloaded_packages)") if (DEFINED ENV{LY_PACKAGE_DOWNLOAD_CACHE_LOCATION}) set(LY_PACKAGE_DOWNLOAD_CACHE_LOCATION $ENV{LY_PACKAGE_DOWNLOAD_CACHE_LOCATION}) endif() +string(CONFIGURE ${LY_PACKAGE_DOWNLOAD_CACHE_LOCATION} LY_PACKAGE_DOWNLOAD_CACHE_LOCATION @ONLY) # LY_PACKAGE_UNPACK_LOCATION - you can change this to any path reachable. -set(LY_PACKAGE_UNPACK_LOCATION ${LY_3RDPARTY_PATH}/packages CACHE PATH "Location to unpack downloaded packages to") +set(LY_PACKAGE_UNPACK_LOCATION @LY_3RDPARTY_PATH@/packages CACHE PATH "Unpack location of downloaded packages (Defaults to @LY_3RDPARTY_PATH@/packages)") if (DEFINED ENV{LY_PACKAGE_UNPACK_LOCATION}) set(LY_PACKAGE_UNPACK_LOCATION $ENV{LY_PACKAGE_UNPACK_LOCATION}) endif() - -# note that sometimes the user configures first without populating LY_3RDPARTY_PATH -# in that case, we'll try overwriting the cache value, only if it is blank: -if (NOT LY_PACKAGE_UNPACK_LOCATION) - set(LY_PACKAGE_UNPACK_LOCATION ${LY_3RDPARTY_PATH}/packages CACHE PATH "Location to unpack downloaded packages to" FORCE ) -endif() +string(CONFIGURE ${LY_PACKAGE_UNPACK_LOCATION} LY_PACKAGE_UNPACK_LOCATION @ONLY) # while developing you can set one or both to true to force auto downloads from your local cache set(LY_PACKAGE_VALIDATE_CONTENTS FALSE CACHE BOOL "If enabled, will fully validate every file in every package based on the SHA256SUMS file from the package") From 62356b811f5534648e0f1d006fbe3a510d0938ce Mon Sep 17 00:00:00 2001 From: Steve Pham <82231385+spham-amzn@users.noreply.github.com> Date: Thu, 22 Jul 2021 17:01:01 -0700 Subject: [PATCH 09/24] Non unity compile fixes (#2365) * Add missing RTTI include to fix non-unity build error * Add missing includes that were automatically added as part of unity builds Signed-off-by: Steve Pham --- Code/Framework/AzCore/Tests/Asset/AssetManagerLoadingTests.cpp | 3 ++- .../Source/Editor/Attribution/AWSCoreAttributionMetric.cpp | 1 + .../Code/Source/AssetMemoryAnalyzerSystemComponent.cpp | 2 ++ Gems/Atom/Utils/Code/Include/Atom/Utils/StableDynamicArray.inl | 1 + .../Code/Source/InAppPurchasesSystemComponent.cpp | 2 ++ Gems/WhiteBox/Code/Include/WhiteBox/WhiteBoxToolApi.h | 2 ++ 6 files changed, 10 insertions(+), 1 deletion(-) diff --git a/Code/Framework/AzCore/Tests/Asset/AssetManagerLoadingTests.cpp b/Code/Framework/AzCore/Tests/Asset/AssetManagerLoadingTests.cpp index fd3e4c8f14..156719b8a7 100644 --- a/Code/Framework/AzCore/Tests/Asset/AssetManagerLoadingTests.cpp +++ b/Code/Framework/AzCore/Tests/Asset/AssetManagerLoadingTests.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -698,7 +699,7 @@ namespace UnitTest auto& assetManager = AssetManager::Instance(); AssetBusCallbacks callbacks{}; - callbacks.SetOnAssetReadyCallback([&](const Asset&, AssetBusCallbacks&) + callbacks.SetOnAssetReadyCallback([&, AssetNoRefB](const Asset&, AssetBusCallbacks&) { // This callback should run inside the "main thread" dispatch events loop auto loadAsset = assetManager.GetAsset(AZ::Uuid(AssetNoRefB), AssetLoadBehavior::Default); diff --git a/Gems/AWSCore/Code/Source/Editor/Attribution/AWSCoreAttributionMetric.cpp b/Gems/AWSCore/Code/Source/Editor/Attribution/AWSCoreAttributionMetric.cpp index d9e7dea8c7..f54d7f71af 100644 --- a/Gems/AWSCore/Code/Source/Editor/Attribution/AWSCoreAttributionMetric.cpp +++ b/Gems/AWSCore/Code/Source/Editor/Attribution/AWSCoreAttributionMetric.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #pragma warning(disable : 4996) diff --git a/Gems/AssetMemoryAnalyzer/Code/Source/AssetMemoryAnalyzerSystemComponent.cpp b/Gems/AssetMemoryAnalyzer/Code/Source/AssetMemoryAnalyzerSystemComponent.cpp index 987444dd3c..bc9763496c 100644 --- a/Gems/AssetMemoryAnalyzer/Code/Source/AssetMemoryAnalyzerSystemComponent.cpp +++ b/Gems/AssetMemoryAnalyzer/Code/Source/AssetMemoryAnalyzerSystemComponent.cpp @@ -6,6 +6,8 @@ * */ +#include + #include #include // For AZ_MAX_PATH_LEN #include diff --git a/Gems/Atom/Utils/Code/Include/Atom/Utils/StableDynamicArray.inl b/Gems/Atom/Utils/Code/Include/Atom/Utils/StableDynamicArray.inl index 5a66dec7d8..25f3f528ab 100644 --- a/Gems/Atom/Utils/Code/Include/Atom/Utils/StableDynamicArray.inl +++ b/Gems/Atom/Utils/Code/Include/Atom/Utils/StableDynamicArray.inl @@ -7,6 +7,7 @@ */ #pragma once +#include #include namespace AZ diff --git a/Gems/InAppPurchases/Code/Source/InAppPurchasesSystemComponent.cpp b/Gems/InAppPurchases/Code/Source/InAppPurchasesSystemComponent.cpp index 8b91b72a60..fd1f4146dc 100644 --- a/Gems/InAppPurchases/Code/Source/InAppPurchasesSystemComponent.cpp +++ b/Gems/InAppPurchases/Code/Source/InAppPurchasesSystemComponent.cpp @@ -6,6 +6,8 @@ * */ +#include + #include #include #include diff --git a/Gems/WhiteBox/Code/Include/WhiteBox/WhiteBoxToolApi.h b/Gems/WhiteBox/Code/Include/WhiteBox/WhiteBoxToolApi.h index 02b563c970..941368e5ac 100644 --- a/Gems/WhiteBox/Code/Include/WhiteBox/WhiteBoxToolApi.h +++ b/Gems/WhiteBox/Code/Include/WhiteBox/WhiteBoxToolApi.h @@ -12,6 +12,8 @@ #include #include #include +#include +#include namespace AZ::IO { From 9a4a7bc1564887b9e9772d365056db4f91501b52 Mon Sep 17 00:00:00 2001 From: lsemp3d <58790905+lsemp3d@users.noreply.github.com> Date: Thu, 22 Jul 2021 17:19:29 -0700 Subject: [PATCH 10/24] Script Canvas: Fixed Scope and Initial Value selection boxes Signed-off-by: lsemp3d <58790905+lsemp3d@users.noreply.github.com> --- .../ScriptCanvas/Variable/GraphVariable.cpp | 18 ++---------------- .../ScriptCanvas/Variable/GraphVariable.h | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Variable/GraphVariable.cpp b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Variable/GraphVariable.cpp index 31bd5cbe41..cc9e60e765 100644 --- a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Variable/GraphVariable.cpp +++ b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Variable/GraphVariable.cpp @@ -186,20 +186,6 @@ namespace ScriptCanvas if (auto editContext = serializeContext->GetEditContext()) { - auto propertyChoices = [] { - AZStd::vector< AZStd::pair> choices; - choices.emplace_back(AZStd::make_pair(VariableFlags::InitialValueSource::Graph, s_InitialValueSourceNames[0])); - choices.emplace_back(AZStd::make_pair(VariableFlags::InitialValueSource::Component, s_InitialValueSourceNames[1])); - return choices; - }; - - auto scopeChoices = [] { - AZStd::vector< AZStd::pair> choices; - choices.emplace_back(AZStd::make_pair(VariableFlags::Scope::Graph, s_ScopeNames[0])); - choices.emplace_back(AZStd::make_pair(VariableFlags::Scope::Function, s_ScopeNames[1])); - return choices; - }; - editContext->Class("Variable", "Represents a Variable field within a Script Canvas Graph") ->ClassElement(AZ::Edit::ClassElements::EditorData, "") ->Attribute(AZ::Edit::Attributes::Visibility, &GraphVariable::GetVisibility) @@ -208,7 +194,7 @@ namespace ScriptCanvas ->Attribute(AZ::Edit::Attributes::DescriptionTextOverride, &GraphVariable::GetDescriptionOverride) ->DataElement(AZ::Edit::UIHandlers::ComboBox, &GraphVariable::m_InitialValueSource, "Initial Value Source", "Variables can get their values from within the graph or through component properties.") - ->Attribute(AZ::Edit::Attributes::GenericValueList, propertyChoices) + ->Attribute(AZ::Edit::Attributes::GenericValueList, &GraphVariable::GetPropertyChoices) ->Attribute(AZ::Edit::Attributes::ChangeNotify, &GraphVariable::OnInitialValueSourceChanged) ->Attribute(AZ::Edit::Attributes::ChangeNotify, AZ::Edit::PropertyRefreshLevels::AttributesAndValues) ->Attribute(AZ::Edit::Attributes::Visibility, &GraphVariable::GetInputControlVisibility) @@ -219,7 +205,7 @@ namespace ScriptCanvas ->DataElement(AZ::Edit::UIHandlers::ComboBox, &GraphVariable::m_scope, "Scope", "Controls the scope of this variable. i.e. If this is exposed as input to this script, or output from this script, or if the variable is just locally scoped.") ->Attribute(AZ::Edit::Attributes::Visibility, &GraphVariable::GetScopeControlVisibility) - ->Attribute(AZ::Edit::Attributes::GenericValueList, scopeChoices) + ->Attribute(AZ::Edit::Attributes::GenericValueList, &GraphVariable::GetScopeChoices) ->Attribute(AZ::Edit::Attributes::ChangeNotify, &GraphVariable::OnScopeTypedChanged) ->DataElement(AZ::Edit::UIHandlers::Default, &GraphVariable::m_networkProperties, "Network Properties", "Enables whether or not this value should be network synchronized") diff --git a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Variable/GraphVariable.h b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Variable/GraphVariable.h index 9ccc00ce14..fd15ac95ee 100644 --- a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Variable/GraphVariable.h +++ b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Variable/GraphVariable.h @@ -182,6 +182,22 @@ namespace ScriptCanvas private: + AZStd::vector> GetPropertyChoices() const + { + AZStd::vector< AZStd::pair> choices; + choices.emplace_back(AZStd::make_pair(static_cast(VariableFlags::InitialValueSource::Graph), s_InitialValueSourceNames[0])); + choices.emplace_back(AZStd::make_pair(static_cast(VariableFlags::InitialValueSource::Component), s_InitialValueSourceNames[1])); + return choices; + } + + AZStd::vector> GetScopeChoices() const + { + AZStd::vector< AZStd::pair> choices; + choices.emplace_back(AZStd::make_pair(static_cast(VariableFlags::Scope::Graph), s_ScopeNames[0])); + choices.emplace_back(AZStd::make_pair(static_cast(VariableFlags::Scope::Function), s_ScopeNames[1])); + return choices; + } + bool IsInFunction() const; void OnScopeTypedChanged(); From d7627c05f64bddca17a8fa49d17dc4c0d733da5e Mon Sep 17 00:00:00 2001 From: onecent1101 Date: Wed, 7 Jul 2021 15:33:37 -0700 Subject: [PATCH 11/24] Fix autogen template RewindableObject construction for linux platform Signed-off-by: onecent1101 --- Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Header.jinja | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Header.jinja b/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Header.jinja index 2eb57498aa..c18faec74b 100644 --- a/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Header.jinja +++ b/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Header.jinja @@ -176,7 +176,7 @@ RewindableFixedVector<{{ Property.attrib['Type'] }}, {{ Property.attrib['Count'] AZStd::fixed_vector<{{ Property.attrib['Type'] }}, {{ Property.attrib['Count'] }}> m_{{ LowerFirst(Property.attrib['Name']) }}; {% endif %} {% elif Property.attrib['IsRewindable']|booleanTrue %} -Multiplayer::RewindableObject<{{ Property.attrib['Type'] }}, Multiplayer::RewindHistorySize> m_{{ LowerFirst(Property.attrib['Name']) }} = {{ Property.attrib['Init'] }}; +Multiplayer::RewindableObject<{{ Property.attrib['Type'] }}, Multiplayer::RewindHistorySize> m_{{ LowerFirst(Property.attrib['Name']) }} = { {{ Property.attrib['Init'] }} }; {% else %} {{ Property.attrib['Type'] }} m_{{ LowerFirst(Property.attrib['Name']) }} = {{ Property.attrib['Init'] }}; {% endif %} From 85c403ea06d870365429f2e1571d0bbea5f663d4 Mon Sep 17 00:00:00 2001 From: onecent1101 Date: Wed, 7 Jul 2021 15:57:10 -0700 Subject: [PATCH 12/24] Remove = Signed-off-by: onecent1101 --- Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Header.jinja | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Header.jinja b/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Header.jinja index c18faec74b..84fdc9ae54 100644 --- a/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Header.jinja +++ b/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Header.jinja @@ -176,7 +176,7 @@ RewindableFixedVector<{{ Property.attrib['Type'] }}, {{ Property.attrib['Count'] AZStd::fixed_vector<{{ Property.attrib['Type'] }}, {{ Property.attrib['Count'] }}> m_{{ LowerFirst(Property.attrib['Name']) }}; {% endif %} {% elif Property.attrib['IsRewindable']|booleanTrue %} -Multiplayer::RewindableObject<{{ Property.attrib['Type'] }}, Multiplayer::RewindHistorySize> m_{{ LowerFirst(Property.attrib['Name']) }} = { {{ Property.attrib['Init'] }} }; +Multiplayer::RewindableObject<{{ Property.attrib['Type'] }}, Multiplayer::RewindHistorySize> m_{{ LowerFirst(Property.attrib['Name']) }} { {{ Property.attrib['Init'] }} }; {% else %} {{ Property.attrib['Type'] }} m_{{ LowerFirst(Property.attrib['Name']) }} = {{ Property.attrib['Init'] }}; {% endif %} From 4268376587af6dd9d23d2746f701bb2525dfa974 Mon Sep 17 00:00:00 2001 From: hultonha <82228511+hultonha@users.noreply.github.com> Date: Fri, 23 Jul 2021 07:52:44 +0100 Subject: [PATCH 13/24] Ensure camera system correctly consumes events when it is active (#2346) * add typename for struct initializer Signed-off-by: hultonha * fix for ensuring while the camera is 'active', events are consumed and not propagated Signed-off-by: hultonha * refactor how we decide which priority to repsond to events to Signed-off-by: hultonha * also -> only Signed-off-by: hultonha --- .../AzFramework/Viewport/CameraInput.cpp | 4 +++- .../AzFramework/Viewport/CameraInput.h | 2 ++ .../ModularViewportCameraController.cpp | 21 ++++++++++++------- .../Source/Viewport/RenderViewportWidget.cpp | 2 +- 4 files changed, 19 insertions(+), 10 deletions(-) diff --git a/Code/Framework/AzFramework/AzFramework/Viewport/CameraInput.cpp b/Code/Framework/AzFramework/AzFramework/Viewport/CameraInput.cpp index 1e324a4919..c5b6a2ff96 100644 --- a/Code/Framework/AzFramework/AzFramework/Viewport/CameraInput.cpp +++ b/Code/Framework/AzFramework/AzFramework/Viewport/CameraInput.cpp @@ -147,7 +147,9 @@ namespace AzFramework m_scrollDelta = scroll->m_delta; } - return m_cameras.HandleEvents(event, m_motionDelta, m_scrollDelta); + m_handlingEvents = m_cameras.HandleEvents(event, m_motionDelta, m_scrollDelta); + + return m_handlingEvents; } Camera CameraSystem::StepCamera(const Camera& targetCamera, const float deltaTime) diff --git a/Code/Framework/AzFramework/AzFramework/Viewport/CameraInput.h b/Code/Framework/AzFramework/AzFramework/Viewport/CameraInput.h index 7c97bc7f89..bb0df4853a 100644 --- a/Code/Framework/AzFramework/AzFramework/Viewport/CameraInput.h +++ b/Code/Framework/AzFramework/AzFramework/Viewport/CameraInput.h @@ -262,12 +262,14 @@ namespace AzFramework public: bool HandleEvents(const InputEvent& event); Camera StepCamera(const Camera& targetCamera, float deltaTime); + bool HandlingEvents() const { return m_handlingEvents; } Cameras m_cameras; //!< Represents a collection of camera inputs that together provide a camera controller. private: ScreenVector m_motionDelta; //!< The delta used for look/orbit/pan (rotation + translation) - two dimensional. float m_scrollDelta = 0.0f; //!< The delta used for dolly/movement (translation) - one dimensional. + bool m_handlingEvents = false; //!< Is the camera system currently handling events (events are consumed and not propagated). }; //! A camera input to handle motion deltas that can rotate or orbit the camera. diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Viewport/ModularViewportCameraController.cpp b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Viewport/ModularViewportCameraController.cpp index 9e2945fe31..10cfa059aa 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Viewport/ModularViewportCameraController.cpp +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Viewport/ModularViewportCameraController.cpp @@ -117,19 +117,24 @@ namespace AtomToolsFramework AzFramework::ViewportDebugDisplayEventBus::Handler::BusDisconnect(); } - // should the camera system respond to this particular event - static bool ShouldHandle(const AzFramework::ViewportControllerPriority priority, const bool exclusive) + // what priority should the camera system respond to + static AzFramework::ViewportControllerPriority GetPriority(const AzFramework::CameraSystem& cameraSystem) { - // ModernViewportCameraControllerInstance receives events at all priorities, it should only respond - // to normal priority events if it is not in 'exclusive' mode and when in 'exclusive' mode it should - // only respond to the highest priority events - return !exclusive && priority == AzFramework::ViewportControllerPriority::Normal || - exclusive && priority == AzFramework::ViewportControllerPriority::Highest; + // ModernViewportCameraControllerInstance receives events at all priorities, when it is in 'exclusive' mode + // or it is actively handling events (essentially when the camera system is 'active' and responding to inputs) + // it should only respond to the highest priority + if (cameraSystem.m_cameras.Exclusive() || cameraSystem.HandlingEvents()) + { + return AzFramework::ViewportControllerPriority::Highest; + } + + // otherwise it should only respond to normal priority events + return AzFramework::ViewportControllerPriority::Normal; } bool ModernViewportCameraControllerInstance::HandleInputChannelEvent(const AzFramework::ViewportControllerInputEvent& event) { - if (ShouldHandle(event.m_priority, m_cameraSystem.m_cameras.Exclusive())) + if (event.m_priority == GetPriority(m_cameraSystem)) { return m_cameraSystem.HandleEvents(AzFramework::BuildInputEvent(event.m_inputChannel)); } diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Viewport/RenderViewportWidget.cpp b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Viewport/RenderViewportWidget.cpp index 0ea59fcf9e..5167e3d3f6 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Viewport/RenderViewportWidget.cpp +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Viewport/RenderViewportWidget.cpp @@ -88,7 +88,7 @@ namespace AtomToolsFramework [this](const AzFramework::InputChannel* inputChannel, QEvent* event) { AzFramework::NativeWindowHandle windowId = reinterpret_cast(winId()); - if (m_controllerList->HandleInputChannelEvent({GetId(), windowId, *inputChannel})) + if (m_controllerList->HandleInputChannelEvent(AzFramework::ViewportControllerInputEvent{GetId(), windowId, *inputChannel})) { // If the controller handled the input event, mark the event as accepted so it doesn't continue to propagate. if (event) From 1a407d9a0842128dd18d83542bd5d7d8982a9c96 Mon Sep 17 00:00:00 2001 From: moraaar Date: Fri, 23 Jul 2021 08:40:03 +0100 Subject: [PATCH 14/24] Debug Trace Messages for Automation queues bus calls. (#2328) Since TraceMessageBus will be called from multiple threads and python interpreter is single threaded, all the bus calls are queued into a list and called at the end of the frame in the main thread. Signed-off-by: moraaar --- .../AzCore/AzCore/Debug/TraceReflection.cpp | 227 +++++++++++++++--- .../Code/Tests/PythonThreadingTests.cpp | 75 ++++++ 2 files changed, 275 insertions(+), 27 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/Debug/TraceReflection.cpp b/Code/Framework/AzCore/AzCore/Debug/TraceReflection.cpp index 4e9d7a698b..debbea5235 100644 --- a/Code/Framework/AzCore/AzCore/Debug/TraceReflection.cpp +++ b/Code/Framework/AzCore/AzCore/Debug/TraceReflection.cpp @@ -10,28 +10,74 @@ #include #include #include +#include namespace AZ { namespace Debug { + //! Trace Message Event Handler for Automation. + //! Since TraceMessageBus will be called from multiple threads and + //! python interpreter is single threaded, all the bus calls are + //! queued into a list and called at the end of the frame in the main thread. + //! @note this class is not using the usual AZ_EBUS_BEHAVIOR_BINDER + //! macro as the signature needs to be changed to connect to Tick bus. class TraceMessageBusHandler : public AZ::Debug::TraceMessageBus::Handler , public AZ::BehaviorEBusHandler + , public AZ::TickBus::Handler { public: + AZ_CLASS_ALLOCATOR(TraceMessageBusHandler, AZ::SystemAllocator, 0); + AZ_RTTI(TraceMessageBusHandler, "{5CDBAF09-5EB0-48AC-B327-2AF8601BB550}", AZ::BehaviorEBusHandler); - AZ_EBUS_BEHAVIOR_BINDER(TraceMessageBusHandler, "{5CDBAF09-5EB0-48AC-B327-2AF8601BB550}", AZ::SystemAllocator - , OnPreAssert - , OnPreError - , OnPreWarning - , OnAssert - , OnError - , OnWarning - , OnException - , OnPrintf - , OnOutput - ); + TraceMessageBusHandler(); + + using EventFunctionsParameterPack = AZStd::Internal::pack_traits_arg_sequence< + decltype(&TraceMessageBusHandler::OnPreAssert), + decltype(&TraceMessageBusHandler::OnPreError), + decltype(&TraceMessageBusHandler::OnPreWarning), + decltype(&TraceMessageBusHandler::OnAssert), + decltype(&TraceMessageBusHandler::OnError), + decltype(&TraceMessageBusHandler::OnWarning), + decltype(&TraceMessageBusHandler::OnException), + decltype(&TraceMessageBusHandler::OnPrintf), + decltype(&TraceMessageBusHandler::OnOutput) + >; + + enum + { + FN_OnPreAssert = 0, + FN_OnPreError, + FN_OnPreWarning, + FN_OnAssert, + FN_OnError, + FN_OnWarning, + FN_OnException, + FN_OnPrintf, + FN_OnOutput, + FN_MAX + }; + + static inline constexpr const char* m_functionNames[FN_MAX] = + { + "OnPreAssert", + "OnPreError", + "OnPreWarning", + "OnAssert", + "OnError", + "OnWarning", + "OnException", + "OnPrintf", + "OnOutput" + }; + + // AZ::BehaviorEBusHandler overrides... + int GetFunctionIndex(const char* functionName) const override; + void Disconnect() override; + bool Connect(AZ::BehaviorValueParameter* id = nullptr) override; + bool IsConnected() override; + bool IsConnectedId(AZ::BehaviorValueParameter* id) override; // TraceMessageBus /* @@ -48,63 +94,190 @@ namespace AZ bool OnPrintf(const char* window, const char* message) override; bool OnOutput(const char* window, const char* message) override; + // AZ::TickBus::Handler overrides ... + void OnTick(float deltaTime, AZ::ScriptTimePoint time) override; + int GetTickOrder() override; + private: - template - R CallResultReturn(const R& defaultReturnValue, int index, Args&&... args) const - { - R returnVal = defaultReturnValue; - CallResult(returnVal, index, AZStd::forward(args)...); - return returnVal; - } + void QueueMessageCall(AZStd::function messageCall); + void FlushMessageCalls(); + + AZStd::list> m_messageCalls; + AZStd::mutex m_messageCallsLock; }; + TraceMessageBusHandler::TraceMessageBusHandler() + { + m_events.resize(FN_MAX); + + SetEvent(&TraceMessageBusHandler::OnPreAssert, m_functionNames[FN_OnPreAssert]); + SetEvent(&TraceMessageBusHandler::OnPreError, m_functionNames[FN_OnPreError]); + SetEvent(&TraceMessageBusHandler::OnPreWarning, m_functionNames[FN_OnPreWarning]); + SetEvent(&TraceMessageBusHandler::OnAssert, m_functionNames[FN_OnAssert]); + SetEvent(&TraceMessageBusHandler::OnError, m_functionNames[FN_OnError]); + SetEvent(&TraceMessageBusHandler::OnWarning, m_functionNames[FN_OnWarning]); + SetEvent(&TraceMessageBusHandler::OnException, m_functionNames[FN_OnException]); + SetEvent(&TraceMessageBusHandler::OnPrintf, m_functionNames[FN_OnPrintf]); + SetEvent(&TraceMessageBusHandler::OnOutput, m_functionNames[FN_OnOutput]); + } + + int TraceMessageBusHandler::GetFunctionIndex(const char* functionName) const + { + for (int i = 0; i < FN_MAX; ++i) + { + if (azstricmp(functionName, m_functionNames[i]) == 0) + { + return i; + } + } + return -1; + } + + void TraceMessageBusHandler::Disconnect() + { + AZ::Debug::TraceMessageBus::Handler::BusDisconnect(); + AZ::TickBus::Handler::BusDisconnect(); + } + + bool TraceMessageBusHandler::Connect(AZ::BehaviorValueParameter* id) + { + AZ::TickBus::Handler::BusConnect(); + return AZ::Internal::EBusConnector::Connect(this, id); + } + + bool TraceMessageBusHandler::IsConnected() + { + return AZ::Internal::EBusConnector::IsConnected(this); + } + + bool TraceMessageBusHandler::IsConnectedId(AZ::BehaviorValueParameter* id) + { + return AZ::Internal::EBusConnector::IsConnectedId(this, id); + } + ////////////////////////////////////////////////////////////////////////// // TraceMessageBusHandler Implementation inline bool TraceMessageBusHandler::OnPreAssert(const char* fileName, int line, const char* func, const char* message) { - return CallResultReturn(false, FN_OnPreAssert, fileName, line, func, message); + QueueMessageCall( + [this, fileNameString = AZStd::string(fileName), line, funcString = AZStd::string(func), messageString = AZStd::string(message)]() + { + Call(FN_OnPreAssert, fileNameString.c_str(), line, funcString.c_str(), messageString.c_str()); + }); + return false; } inline bool TraceMessageBusHandler::OnPreError(const char* window, const char* fileName, int line, const char* func, const char* message) { - return CallResultReturn(false, FN_OnPreError, window, fileName, line, func, message); + QueueMessageCall( + [this, windowString = AZStd::string(window), fileNameString = AZStd::string(fileName), line, funcString = AZStd::string(func), messageString = AZStd::string(message)]() + { + Call(FN_OnPreError, windowString.c_str(), fileNameString.c_str(), line, funcString.c_str(), messageString.c_str()); + }); + return false; } inline bool TraceMessageBusHandler::OnPreWarning(const char* window, const char* fileName, int line, const char* func, const char* message) { - return CallResultReturn(false, FN_OnPreWarning, window, fileName, line, func, message); + QueueMessageCall( + [this, windowString = AZStd::string(window), fileNameString = AZStd::string(fileName), line, funcString = AZStd::string(func), messageString = AZStd::string(message)]() + { + return Call(FN_OnPreWarning, windowString.c_str(), fileNameString.c_str(), line, funcString.c_str(), messageString.c_str()); + }); + return false; } inline bool TraceMessageBusHandler::OnAssert(const char* message) { - return CallResultReturn(false, FN_OnAssert, message); + QueueMessageCall( + [this, messageString = AZStd::string(message)]() + { + return Call(FN_OnAssert, messageString.c_str()); + }); + return false; } inline bool TraceMessageBusHandler::OnError(const char* window, const char* message) { - return CallResultReturn(false, FN_OnError, window, message); + QueueMessageCall( + [this, windowString = AZStd::string(window), messageString = AZStd::string(message)]() + { + return Call(FN_OnError, windowString.c_str(), messageString.c_str()); + }); + return false; } inline bool TraceMessageBusHandler::OnWarning(const char* window, const char* message) { - return CallResultReturn(false, FN_OnWarning, window, message); + QueueMessageCall( + [this, windowString = AZStd::string(window), messageString = AZStd::string(message)]() + { + return Call(FN_OnWarning, windowString.c_str(), messageString.c_str()); + }); + return false; } inline bool TraceMessageBusHandler::OnException(const char* message) { - return CallResultReturn(false, FN_OnException, message); + QueueMessageCall( + [this, messageString = AZStd::string(message)]() + { + return Call(FN_OnException, messageString.c_str()); + }); + return false; } inline bool TraceMessageBusHandler::OnPrintf(const char* window, const char* message) { - return CallResultReturn(false, FN_OnPrintf, window, message); + QueueMessageCall( + [this, windowString = AZStd::string(window), messageString = AZStd::string(message)]() + { + return Call(FN_OnPrintf, windowString.c_str(), messageString.c_str()); + }); + return false; } inline bool TraceMessageBusHandler::OnOutput(const char* window, const char* message) { - return CallResultReturn(false, FN_OnOutput, window, message); + QueueMessageCall( + [this, windowString = AZStd::string(window), messageString = AZStd::string(message)]() + { + return Call(FN_OnOutput, windowString.c_str(), messageString.c_str()); + }); + return false; } + void TraceMessageBusHandler::OnTick( + [[maybe_unused]] float deltaTime, + [[maybe_unused]] AZ::ScriptTimePoint time) + { + FlushMessageCalls(); + } + + int TraceMessageBusHandler::GetTickOrder() + { + return AZ::TICK_LAST; + } + + void TraceMessageBusHandler::QueueMessageCall(AZStd::function messageCall) + { + AZStd::lock_guard lock(m_messageCallsLock); + m_messageCalls.push_back(messageCall); + } + + void TraceMessageBusHandler::FlushMessageCalls() + { + AZStd::list> messageCalls; + { + AZStd::lock_guard lock(m_messageCallsLock); + m_messageCalls.swap(messageCalls); // Move calls to a new list to release the lock as soon as possible + } + + for (auto& messageCall : messageCalls) + { + messageCall(); + } + } void TraceReflect(ReflectContext* context) { diff --git a/Gems/EditorPythonBindings/Code/Tests/PythonThreadingTests.cpp b/Gems/EditorPythonBindings/Code/Tests/PythonThreadingTests.cpp index b1cb2ef162..b2e18265cb 100644 --- a/Gems/EditorPythonBindings/Code/Tests/PythonThreadingTests.cpp +++ b/Gems/EditorPythonBindings/Code/Tests/PythonThreadingTests.cpp @@ -217,4 +217,79 @@ namespace UnitTest e.Deactivate(); } + TEST_F(PythonThreadingTest, PythonInterface_DebugTrace_CallsOnTick) + { + enum class LogTypes + { + Skip = 0, + OnPrewarning + }; + + m_testSink.m_evaluateMessage = [](const char* window, const char* message) -> int + { + if (AzFramework::StringFunc::Equal(window, "python")) + { + if (AzFramework::StringFunc::StartsWith(message, "OnPrewarning")) + { + return aznumeric_cast(LogTypes::OnPrewarning); + } + } + return aznumeric_cast(LogTypes::Skip); + }; + + AZ::Entity e; + Activate(e); + SimulateEditorBecomingInitialized(); + + try + { + // prepare handler on this thread + pybind11::exec(R"( + import azlmbr.debug + + def on_prewarning(args): + print ('OnPrewarning: ' + args[0]) + + handler = azlmbr.debug.TraceMessageBusHandler() + handler.connect() + handler.add_callback('OnPreWarning', on_prewarning) + )"); + + const size_t numWarnings = 64; + auto doWarning = []() + { + AZ_Warning("PythonThreadingTest", false, "This is a warning message"); + }; + + // start threads. In thread issue a warning. + AZStd::vector threads; + threads.reserve(numWarnings); + for (size_t i = 0; i < numWarnings; ++i) + { + threads.emplace_back(doWarning); + } + for (AZStd::thread& thread : threads) + { + thread.join(); + } + + // No prewarning calls should have happened because all of them were queued + EXPECT_EQ(0, m_testSink.m_evaluationMap[aznumeric_cast(LogTypes::OnPrewarning)]); + + // Do one tick + const float timeOneFrameSeconds = 0.016f; //approx 60 fps + AZ::TickBus::Broadcast(&AZ::TickEvents::OnTick, + timeOneFrameSeconds, + AZ::ScriptTimePoint(AZStd::chrono::system_clock::now())); + + // After one tick all the queued calls should have been processed + EXPECT_EQ(numWarnings, m_testSink.m_evaluationMap[aznumeric_cast(LogTypes::OnPrewarning)]); + } + catch ([[maybe_unused]] const std::exception& e) + { + AZ_Error("UnitTest", false, "Failed during thread test with %s", e.what()); + } + + e.Deactivate(); + } } From a33bc107be33190a4ba7ed9da1034f51df1361dd Mon Sep 17 00:00:00 2001 From: John Jones-Steele Date: Fri, 23 Jul 2021 10:58:36 +0100 Subject: [PATCH 15/24] Fixed number truncation in deisplayed values Signed-off-by: John Jones-Steele --- .../AzQtComponents/Utilities/Conversions.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Code/Framework/AzQtComponents/AzQtComponents/Utilities/Conversions.cpp b/Code/Framework/AzQtComponents/AzQtComponents/Utilities/Conversions.cpp index 4764b9f911..591b4a3701 100644 --- a/Code/Framework/AzQtComponents/AzQtComponents/Utilities/Conversions.cpp +++ b/Code/Framework/AzQtComponents/AzQtComponents/Utilities/Conversions.cpp @@ -43,19 +43,20 @@ namespace AzQtComponents { const QChar decimalPoint = locale.decimalPoint(); const QChar zeroDigit = locale.zeroDigit(); + const int numToStringDecimals = numDecimals < 8 ? 8 : numDecimals; - // We want to truncate, not round. toString will round, so we add an extra decimal place to the formatting - // so we can remove the last value - QString retValue = locale.toString(value, 'f', (numDecimals > 0) ? numDecimals + 1 : 0); + // We want to truncate, not round. toString will round, so we add extra decimal places to the formatting + // so we can remove the last values + QString retValue = locale.toString(value, 'f', (numDecimals > 0) ? numToStringDecimals : 0); // Handle special cases when we have decimals in our value if (numDecimals > 0) { // Truncate the extra digit now, if it's still there int decimalPointIndex = retValue.lastIndexOf(decimalPoint); - if ((decimalPointIndex > 0) && (retValue.size() - (decimalPointIndex + 1)) == (numDecimals + 1)) + if ((decimalPointIndex > 0) && (retValue.size() - (decimalPointIndex + 1)) == numToStringDecimals) { - retValue.resize(retValue.size() - 1); + retValue.resize(retValue.size() - (numToStringDecimals - numDecimals)); } // Remove trailing zeros, since the locale conversion won't do From b1922fe95e7096c5e35cba6f94c3dccdab5af49d Mon Sep 17 00:00:00 2001 From: John Jones-Steele Date: Fri, 23 Jul 2021 15:26:38 +0100 Subject: [PATCH 16/24] Changed comment to reflect PR Signed-off-by: John Jones-Steele --- .../AzQtComponents/AzQtComponents/Utilities/Conversions.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Code/Framework/AzQtComponents/AzQtComponents/Utilities/Conversions.cpp b/Code/Framework/AzQtComponents/AzQtComponents/Utilities/Conversions.cpp index 591b4a3701..c9327d199e 100644 --- a/Code/Framework/AzQtComponents/AzQtComponents/Utilities/Conversions.cpp +++ b/Code/Framework/AzQtComponents/AzQtComponents/Utilities/Conversions.cpp @@ -52,7 +52,7 @@ namespace AzQtComponents // Handle special cases when we have decimals in our value if (numDecimals > 0) { - // Truncate the extra digit now, if it's still there + // Truncate the extra digits now, if they're still there int decimalPointIndex = retValue.lastIndexOf(decimalPoint); if ((decimalPointIndex > 0) && (retValue.size() - (decimalPointIndex + 1)) == numToStringDecimals) { From 1e10ee61bac2ac56797ebbdd4d1b950689a5e5fc Mon Sep 17 00:00:00 2001 From: Chris Burel Date: Fri, 23 Jul 2021 10:13:53 -0700 Subject: [PATCH 17/24] [MeshOptimizer] Return the correct size from the position map when there are blendshapes (#2391) The previous version returned the incorrect size from the position map when blendshapes are present in the model. When there are blendshapes, the vertex welding is disabled, and nothing is inserted into the position map. However, the position map's size was being used to dictate how many elements to create in the skinning info. The skinning info tries to compensate for an incorrect max vertex index by resizing its underlying vector when adding an influence, but that was using the index as the new size of the vector, so it was off by one. This fixes both errors. Signed-off-by: Chris Burel --- .../MeshOptimizer/MeshBuilderSkinningInfo.h | 2 +- .../MeshOptimizer/MeshOptimizerComponent.cpp | 32 +++++++++++++++---- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/Gems/SceneProcessing/Code/Source/Generation/Components/MeshOptimizer/MeshBuilderSkinningInfo.h b/Gems/SceneProcessing/Code/Source/Generation/Components/MeshOptimizer/MeshBuilderSkinningInfo.h index f5b716876e..dd3b715a21 100644 --- a/Gems/SceneProcessing/Code/Source/Generation/Components/MeshOptimizer/MeshBuilderSkinningInfo.h +++ b/Gems/SceneProcessing/Code/Source/Generation/Components/MeshOptimizer/MeshBuilderSkinningInfo.h @@ -35,7 +35,7 @@ namespace AZ::MeshBuilder MeshBuilderSkinningInfo(size_t numOrgVertices); - void AddInfluence(size_t orgVtxNr, const Influence& influence) { mInfluences.resize(AZStd::max(mInfluences.size(), orgVtxNr)); mInfluences.at(orgVtxNr).emplace_back(influence); } + void AddInfluence(size_t orgVtxNr, const Influence& influence) { mInfluences.resize(AZStd::max(mInfluences.size(), orgVtxNr + 1)); mInfluences.at(orgVtxNr).emplace_back(influence); } void RemoveInfluence(size_t orgVtxNr, size_t influenceNr) { mInfluences.at(orgVtxNr).erase(mInfluences.at(orgVtxNr).begin() + influenceNr); } const Influence& GetInfluence(size_t orgVtxNr, size_t influenceNr) const { return mInfluences.at(orgVtxNr).at(influenceNr); } size_t GetNumInfluences(size_t orgVtxNr) const { return mInfluences.at(orgVtxNr).size(); } diff --git a/Gems/SceneProcessing/Code/Source/Generation/Components/MeshOptimizer/MeshOptimizerComponent.cpp b/Gems/SceneProcessing/Code/Source/Generation/Components/MeshOptimizer/MeshOptimizerComponent.cpp index 35d6db6d03..6bc47476f8 100644 --- a/Gems/SceneProcessing/Code/Source/Generation/Components/MeshOptimizer/MeshOptimizerComponent.cpp +++ b/Gems/SceneProcessing/Code/Source/Generation/Components/MeshOptimizer/MeshOptimizerComponent.cpp @@ -105,7 +105,6 @@ namespace AZ::SceneGenerationComponents // Vector3 as a key into a unordered_map. template class Vector3Map - : private AZStd::unordered_map { public: Vector3Map(const MeshDataType* meshData, bool hasBlendShapes, float positionTolerance) @@ -116,9 +115,6 @@ namespace AZ::SceneGenerationComponents { } - using AZStd::unordered_map::reserve; - using AZStd::unordered_map::size; - AZ::u32 operator[](const AZ::u32 vertexIndex) { if (m_hasBlendShapes) @@ -130,7 +126,7 @@ namespace AZ::SceneGenerationComponents return m_meshData->GetUsedPointIndexForControlPoint(m_meshData->GetControlPointIndex(vertexIndex)); } - const auto& [iter, didInsert] = try_emplace(GetPositionForIndex(vertexIndex), m_currentOriginalVertexIndex); + const auto& [iter, didInsert] = m_map.try_emplace(GetPositionForIndex(vertexIndex), m_currentOriginalVertexIndex); if (didInsert) { ++m_currentOriginalVertexIndex; @@ -149,11 +145,32 @@ namespace AZ::SceneGenerationComponents return m_meshData->GetUsedPointIndexForControlPoint(m_meshData->GetControlPointIndex(vertexIndex)); } - auto iter = find(GetPositionForIndex(vertexIndex)); - AZSTD_CONTAINER_ASSERT(iter != end(), "Element with key is not present"); + auto iter = m_map.find(GetPositionForIndex(vertexIndex)); + AZSTD_CONTAINER_ASSERT(iter != m_map.end(), "Element with key is not present"); return iter->second; } + [[nodiscard]] size_t size() const + { + if (m_hasBlendShapes) + { + // Since blend shapes are present, the vertex welding is disabled, and the map will always be empty. + // Use the underlying mesh's vertex count instead. + return m_meshData->GetUsedControlPointCount(); + } + return m_map.size(); + } + + void reserve(size_t count) + { + if (m_hasBlendShapes) + { + // Since blend shapes are present, the vertex welding is disabled, and the map will always be empty. + return; + } + m_map.reserve(count); + } + private: AZ::Vector3 GetPositionForIndex(const AZ::u32 vertexIndex) const @@ -167,6 +184,7 @@ namespace AZ::SceneGenerationComponents ) * m_positionTolerance; } + AZStd::unordered_map m_map; const MeshDataType* m_meshData; bool m_hasBlendShapes; float m_positionTolerance; From 5b8b51924a49e125f379bd7b84938407c6e8dbbd Mon Sep 17 00:00:00 2001 From: Junbo Liang <68558268+junbo75@users.noreply.github.com> Date: Fri, 23 Jul 2021 10:29:03 -0700 Subject: [PATCH 18/24] [AWS][AppSec][Medium] Bucket logging not enabled in sample template (#2243) Add S3 bucket logging not enabled in sample template --- Gems/AWSCore/cdk/app.py | 13 ++- Gems/AWSCore/cdk/core/aws_core.py | 4 + Gems/AWSCore/cdk/core/core_stack.py | 26 ++++-- Gems/AWSCore/cdk/core_stack_properties.py | 25 ------ .../cdk/example/example_resources_stack.py | 90 ++++++++++++++++--- .../cdk/aws_metrics/aws_metrics_construct.py | 4 +- .../cdk/aws_metrics/aws_metrics_stack.py | 4 +- .../cdk/aws_metrics/data_lake_integration.py | 16 +++- 8 files changed, 126 insertions(+), 56 deletions(-) delete mode 100755 Gems/AWSCore/cdk/core_stack_properties.py diff --git a/Gems/AWSCore/cdk/app.py b/Gems/AWSCore/cdk/app.py index 17b7241227..ea038f8a51 100755 --- a/Gems/AWSCore/cdk/app.py +++ b/Gems/AWSCore/cdk/app.py @@ -37,7 +37,7 @@ env = core.Environment(account=ACCOUNT, region=REGION) app = core.App() -core = AWSCore( +core_construct = AWSCore( app, id_=f'{PROJECT_FEATURE_NAME}-Construct', project_name=PROJECT_NAME, @@ -46,20 +46,19 @@ core = AWSCore( ) # Below is the Core example stack which is provided for working with AWSCore ScriptCanvas examples. -# It also provided as an example how to reference properties across stacks in the same CDK applications -# Note: This will make the consuming stack a dependent stack on core -# CDK will deploy the dependent stack first and then the core stack +# It also provided as an example how to reference resources across stacks via stack outputs. # See https://docs.aws.amazon.com/cdk/latest/guide/resources.html#resource_stack -core_properties = core.properties -example = ExampleResources( +example_stack = ExampleResources( app, id_=f'{PROJECT_FEATURE_NAME}-Example-{env.region}', - props_=core_properties, project_name=f'{PROJECT_NAME}', feature_name=FEATURE_NAME, tags={Constants.O3DE_PROJECT_TAG_NAME: PROJECT_NAME, Constants.O3DE_FEATURE_TAG_NAME: FEATURE_NAME}, env=env ) +# +# Add the common stack as a dependency of the feature stack +example_stack.add_dependency(core_construct.common_stack) app.synth() diff --git a/Gems/AWSCore/cdk/core/aws_core.py b/Gems/AWSCore/cdk/core/aws_core.py index 49b5449443..53f1d0693b 100755 --- a/Gems/AWSCore/cdk/core/aws_core.py +++ b/Gems/AWSCore/cdk/core/aws_core.py @@ -42,3 +42,7 @@ class AWSCore(core.Construct): @property def properties(self): return self._feature_stack.properties + + @property + def common_stack(self): + return self._feature_stack diff --git a/Gems/AWSCore/cdk/core/core_stack.py b/Gems/AWSCore/cdk/core/core_stack.py index 24efca774f..fc1b4cf8d8 100755 --- a/Gems/AWSCore/cdk/core/core_stack.py +++ b/Gems/AWSCore/cdk/core/core_stack.py @@ -8,11 +8,11 @@ SPDX-License-Identifier: Apache-2.0 OR MIT from aws_cdk import ( core, aws_iam as iam, + aws_s3 as s3, aws_resourcegroups as resource_groups, ) from constants import Constants -from core_stack_properties import CoreStackProperties class CoreStack(core.Stack): @@ -60,6 +60,17 @@ class CoreStack(core.Stack): type='TAG_FILTERS_1_0') ) + # Create an S3 bucket for Amazon S3 server access logging + # See https://docs.aws.amazon.com/AmazonS3/latest/dev/security-best-practices.html + self._server_access_logs_bucket = s3.Bucket( + self, + f'{self._project_name}-{self._feature_name}-Access-Log-Bucket', + block_public_access=s3.BlockPublicAccess.BLOCK_ALL, + encryption=s3.BucketEncryption.S3_MANAGED, + access_control=s3.BucketAccessControl.LOG_DELIVERY_WRITE + ) + self._server_access_logs_bucket.grant_read(self._admin_group) + # Define exports # Export resource group self._resource_group_output = core.CfnOutput( @@ -83,9 +94,10 @@ class CoreStack(core.Stack): export_name=f"{self._project_name}:AdminGroup", value=self._admin_group.group_arn) - @property - def properties(self) -> CoreStackProperties: - _props = CoreStackProperties() - _props.user_group = self._user_group - _props.admin_group = self._admin_group - return _props + # Export access log bucket name + self._server_access_logs_bucket_output = core.CfnOutput( + self, + id=f'ServerAccessLogsBucketOutput', + description='Name of the S3 bucket for storing server access logs generated by the sample CDK application(s)', + export_name=f"{self._project_name}:ServerAccessLogsBucket", + value=self._server_access_logs_bucket.bucket_name) diff --git a/Gems/AWSCore/cdk/core_stack_properties.py b/Gems/AWSCore/cdk/core_stack_properties.py deleted file mode 100755 index 60ec697d53..0000000000 --- a/Gems/AWSCore/cdk/core_stack_properties.py +++ /dev/null @@ -1,25 +0,0 @@ -""" -Copyright (c) Contributors to the Open 3D Engine Project. -For complete copyright and license terms please see the LICENSE at the root of this distribution. - -SPDX-License-Identifier: Apache-2.0 OR MIT -""" - -from aws_cdk import ( - core, - aws_iam as iam -) - - -class CoreStackProperties(core.StackProps): - """ - Support for cross stack references in the application. - - Define any properties from the CoreStack other stacks in this application - may need to consume. - """ - # Common IAM group for users - user_group: iam.Group - - # Common IAM group for Admin users - admin_group: iam.Group diff --git a/Gems/AWSCore/cdk/example/example_resources_stack.py b/Gems/AWSCore/cdk/example/example_resources_stack.py index 7a63e1a727..23bc78d8fc 100755 --- a/Gems/AWSCore/cdk/example/example_resources_stack.py +++ b/Gems/AWSCore/cdk/example/example_resources_stack.py @@ -8,13 +8,13 @@ import os from aws_cdk import ( aws_lambda as lambda_, + aws_iam as iam, aws_s3 as s3, aws_s3_deployment as s3_deployment, aws_dynamodb as dynamo, core ) -from core_stack_properties import CoreStackProperties from .auth import AuthPolicy @@ -25,8 +25,7 @@ class ExampleResources(core.Stack): * A python 'echo' lambda * A small dynamodb table with the a primary 'id': str key """ - def __init__(self, scope: core.Construct, id_: str, project_name: str, feature_name: str, - props_: CoreStackProperties, **kwargs) -> None: + def __init__(self, scope: core.Construct, id_: str, project_name: str, feature_name: str, **kwargs) -> None: super().__init__(scope, id_, **kwargs, description=f'Contains resources for the AWSCore examples as part of the ' f'{project_name} project') @@ -42,17 +41,74 @@ class ExampleResources(core.Stack): self.__create_outputs() # Finally grant cross stack references - self.__grant_access(props=props_) + self.__grant_access() - def __grant_access(self, props: CoreStackProperties): - self._s3_bucket.grant_read(props.user_group) - self._s3_bucket.grant_read(props.admin_group) + def __grant_access(self): + user_group = iam.Group.from_group_arn( + self, + f'{self._project_name}-{self._feature_name}-ImportedUserGroup', + core.Fn.import_value(f'{self._project_name}:UserGroup') + ) + admin_group = iam.Group.from_group_arn( + self, + f'{self._project_name}-{self._feature_name}-ImportedAdminGroup', + core.Fn.import_value(f'{self._project_name}:AdminGroup') + ) - self._lambda.grant_invoke(props.user_group) - self._lambda.grant_invoke(props.admin_group) + # Provide the admin and user groups permissions to read the example S3 bucket. + # Cannot use the grant_read method defined by the Bucket structure since the method tries to add to + # the resource-based policy but the imported IAM groups (which are tokens from Fn.ImportValue) are + # not valid principals in S3 bucket policies. + # Check https://aws.amazon.com/premiumsupport/knowledge-center/s3-invalid-principal-in-policy-error/ + user_group.add_to_principal_policy( + iam.PolicyStatement( + actions=[ + "s3:GetBucket*", + "s3:GetObject*", + "s3:List*" + ], + effect=iam.Effect.ALLOW, + resources=[self._s3_bucket.bucket_arn, f'{self._s3_bucket.bucket_arn}/*'] + ) + ) + admin_group.add_to_principal_policy( + iam.PolicyStatement( + actions=[ + "s3:GetBucket*", + "s3:GetObject*", + "s3:List*" + ], + effect=iam.Effect.ALLOW, + resources=[self._s3_bucket.bucket_arn, f'{self._s3_bucket.bucket_arn}/*'] + ) + ) - self._table.grant_read_data(props.user_group) - self._table.grant_read_data(props.admin_group) + # Provide the admin and user groups permissions to invoke the example Lambda function. + # Cannot use the grant_invoke method defined by the Function structure since the method tries to add to + # the resource-based policy but the imported IAM groups (which are tokens from Fn.ImportValue) are + # not valid principals in Lambda function policies. + user_group.add_to_principal_policy( + iam.PolicyStatement( + actions=[ + "lambda:InvokeFunction" + ], + effect=iam.Effect.ALLOW, + resources=[self._lambda.function_arn] + ) + ) + admin_group.add_to_principal_policy( + iam.PolicyStatement( + actions=[ + "lambda:InvokeFunction" + ], + effect=iam.Effect.ALLOW, + resources=[self._lambda.function_arn] + ) + ) + + # Provide the admin and user groups permissions to read from the DynamoDB table. + self._table.grant_read_data(user_group) + self._table.grant_read_data(admin_group) def __create_s3_bucket(self) -> s3.Bucket: # Create a sample S3 bucket following S3 best practices @@ -60,11 +116,21 @@ class ExampleResources(core.Stack): # 1. Block all public access to the bucket # 2. Use SSE-S3 encryption. Explore encryption at rest options via # https://docs.aws.amazon.com/AmazonS3/latest/userguide/serv-side-encryption.html + # 3. Enable Amazon S3 server access logging + # https://docs.aws.amazon.com/AmazonS3/latest/userguide/ServerLogs.html + server_access_logs_bucket = s3.Bucket.from_bucket_name( + self, + f'{self._project_name}-{self._feature_name}-ImportedAccessLogsBucket', + core.Fn.import_value(f"{self._project_name}:ServerAccessLogsBucket") + ) + example_bucket = s3.Bucket( self, f'{self._project_name}-{self._feature_name}-Example-S3bucket', block_public_access=s3.BlockPublicAccess.BLOCK_ALL, - encryption=s3.BucketEncryption.S3_MANAGED + encryption=s3.BucketEncryption.S3_MANAGED, + server_access_logs_bucket=server_access_logs_bucket, + server_access_logs_prefix=f'{self._project_name}-{self._feature_name}-{self.region}-AccessLogs' ) s3_deployment.BucketDeployment( diff --git a/Gems/AWSMetrics/cdk/aws_metrics/aws_metrics_construct.py b/Gems/AWSMetrics/cdk/aws_metrics/aws_metrics_construct.py index c78fe06a82..d107c20b22 100755 --- a/Gems/AWSMetrics/cdk/aws_metrics/aws_metrics_construct.py +++ b/Gems/AWSMetrics/cdk/aws_metrics/aws_metrics_construct.py @@ -28,14 +28,14 @@ class AWSMetrics(core.Construct): # Check context variables to get enabled optional features optional_features = { - 'batch_processing': self.node.try_get_context("batch_processing") == 'true' + 'batch_processing': self.node.try_get_context("batch_processing") == 'true', + 'server_access_logs_bucket': self.node.try_get_context("server_access_logs_bucket") } # Deploy AWS Metrics Stack self._feature_stack = AWSMetricsStack( scope, stack_name, - stack_name=stack_name, application_name=application_name, description=f'Contains resources for the AWS Metrics Gem Feature stack as part of the {project_name} project', optional_features=optional_features, diff --git a/Gems/AWSMetrics/cdk/aws_metrics/aws_metrics_stack.py b/Gems/AWSMetrics/cdk/aws_metrics/aws_metrics_stack.py index aaea9b27a6..337a14a301 100755 --- a/Gems/AWSMetrics/cdk/aws_metrics/aws_metrics_stack.py +++ b/Gems/AWSMetrics/cdk/aws_metrics/aws_metrics_stack.py @@ -43,9 +43,11 @@ class AWSMetricsStack(core.Stack): ) batch_processing_enabled = optional_features.get('batch_processing', False) + server_access_logs_bucket = optional_features.get('server_access_logs_bucket') self._data_lake_integration = DataLakeIntegration( self, - application_name=application_name + application_name=application_name, + server_access_logs_bucket=server_access_logs_bucket ) if batch_processing_enabled else None self._batch_processing = BatchProcessing( diff --git a/Gems/AWSMetrics/cdk/aws_metrics/data_lake_integration.py b/Gems/AWSMetrics/cdk/aws_metrics/data_lake_integration.py index 3e0527b8e3..dff165ce56 100755 --- a/Gems/AWSMetrics/cdk/aws_metrics/data_lake_integration.py +++ b/Gems/AWSMetrics/cdk/aws_metrics/data_lake_integration.py @@ -19,9 +19,11 @@ class DataLakeIntegration: """ Create the AWS resources including the S3 bucket, Glue database, table and crawler for data lake integration """ - def __init__(self, stack: core.Construct, application_name: str) -> None: + def __init__(self, stack: core.Construct, application_name: str, + server_access_logs_bucket: str = None) -> None: self._stack = stack self._application_name = application_name + self._server_access_logs_bucket = server_access_logs_bucket self._create_analytics_bucket() self._create_events_database() @@ -34,6 +36,14 @@ class DataLakeIntegration: The bucket uses server-side encryption with a CMK managed by S3: https://docs.aws.amazon.com/AmazonS3/latest/userguide/UsingKMSEncryption.html """ + # Enable server access logging if the server access logs bucket is provided following S3 best practices. + # See https://docs.aws.amazon.com/AmazonS3/latest/dev/security-best-practices.html + server_access_logs_bucket = s3.Bucket.from_bucket_name( + self._stack, + f'{self._stack.stack_name}-ImportedAccessLogsBucket', + self._server_access_logs_bucket, + ) if self._server_access_logs_bucket else None + # Bucket name cannot contain uppercase characters # Do not specify the bucket name here since bucket name is required to be unique globally. If we set # a specific name here, only one customer can deploy the bucket successfully. @@ -46,7 +56,9 @@ class DataLakeIntegration: block_public_policy=True, ignore_public_acls=True, restrict_public_buckets=True - ) + ), + server_access_logs_bucket=server_access_logs_bucket, + server_access_logs_prefix=f'{self._stack.stack_name}-AccessLogs' if server_access_logs_bucket else None ) # For Amazon S3 buckets, you must delete all objects in the bucket for deletion to succeed. From c8b55af401182af6a47e0a10d4676b28934f205e Mon Sep 17 00:00:00 2001 From: Junbo Liang <68558268+junbo75@users.noreply.github.com> Date: Fri, 23 Jul 2021 11:33:11 -0700 Subject: [PATCH 19/24] [AWS][Metrics] Resource names are too long leading to CDK deployment failures (#2291) Sanitize resource names to prevent AWS resource failures. --- Gems/AWSMetrics/cdk/aws_metrics/auth.py | 6 ++- .../cdk/aws_metrics/aws_metrics_construct.py | 4 +- .../cdk/aws_metrics/aws_utils/__init__.py | 6 +++ .../aws_utils/resource_name_sanitizer.py | 45 +++++++++++++++++++ .../cdk/aws_metrics/batch_analytics.py | 13 ++++-- .../cdk/aws_metrics/batch_processing.py | 17 ++++--- Gems/AWSMetrics/cdk/aws_metrics/dashboard.py | 4 +- .../cdk/aws_metrics/data_ingestion.py | 8 ++-- .../cdk/aws_metrics/data_lake_integration.py | 8 +++- .../aws_metrics/real_time_data_processing.py | 13 ++++-- 10 files changed, 102 insertions(+), 22 deletions(-) create mode 100644 Gems/AWSMetrics/cdk/aws_metrics/aws_utils/__init__.py create mode 100644 Gems/AWSMetrics/cdk/aws_metrics/aws_utils/resource_name_sanitizer.py diff --git a/Gems/AWSMetrics/cdk/aws_metrics/auth.py b/Gems/AWSMetrics/cdk/aws_metrics/auth.py index 4b445c0c6d..95fbecd915 100755 --- a/Gems/AWSMetrics/cdk/aws_metrics/auth.py +++ b/Gems/AWSMetrics/cdk/aws_metrics/auth.py @@ -13,6 +13,7 @@ from aws_cdk import ( from .aws_metrics_stack import AWSMetricsStack from aws_metrics.policy_statements_builder.user_policy_statements_builder import UserPolicyStatementsBuilder from aws_metrics.policy_statements_builder.admin_policy_statements_builder import AdminPolicyStatementsBuilder +from .aws_utils import resource_name_sanitizer class AuthPolicy: @@ -58,12 +59,13 @@ class AuthPolicy: policy = iam.ManagedPolicy( self._stack, policy_id, - managed_policy_name=f'{self._stack.stack_name}-{role_name}Policy', + managed_policy_name=resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-{role_name}Policy', 'iam_managed_policy'), statements=policy_statements) policy_output = core.CfnOutput( self._stack, id=f'{policy_id}Output', description=f'{role_name} policy arn to call service', - export_name=f"{self._application_name}:{policy_id}", + export_name=f'{self._application_name}:{policy_id}', value=policy.managed_policy_arn) diff --git a/Gems/AWSMetrics/cdk/aws_metrics/aws_metrics_construct.py b/Gems/AWSMetrics/cdk/aws_metrics/aws_metrics_construct.py index d107c20b22..70d85d1586 100755 --- a/Gems/AWSMetrics/cdk/aws_metrics/aws_metrics_construct.py +++ b/Gems/AWSMetrics/cdk/aws_metrics/aws_metrics_construct.py @@ -8,6 +8,7 @@ SPDX-License-Identifier: Apache-2.0 OR MIT from aws_cdk import core from .aws_metrics_stack import AWSMetricsStack from .auth import AuthPolicy +from .aws_utils import resource_name_sanitizer class AWSMetrics(core.Construct): @@ -23,7 +24,8 @@ class AWSMetrics(core.Construct): env: core.Environment) -> None: super().__init__(scope, id_) # Set-up any stack name(s) to be unique in account - stack_name = f'{project_name}-{feature_name}-{env.region}' + stack_name = resource_name_sanitizer.sanitize_resource_name( + f'{project_name}-{feature_name}-{env.region}', 'cloudformation_stack') application_name = f'{project_name}-{feature_name}' # Check context variables to get enabled optional features diff --git a/Gems/AWSMetrics/cdk/aws_metrics/aws_utils/__init__.py b/Gems/AWSMetrics/cdk/aws_metrics/aws_utils/__init__.py new file mode 100644 index 0000000000..50cbb262dd --- /dev/null +++ b/Gems/AWSMetrics/cdk/aws_metrics/aws_utils/__init__.py @@ -0,0 +1,6 @@ +""" +Copyright (c) Contributors to the Open 3D Engine Project. +For complete copyright and license terms please see the LICENSE at the root of this distribution. + +SPDX-License-Identifier: Apache-2.0 OR MIT +""" diff --git a/Gems/AWSMetrics/cdk/aws_metrics/aws_utils/resource_name_sanitizer.py b/Gems/AWSMetrics/cdk/aws_metrics/aws_utils/resource_name_sanitizer.py new file mode 100644 index 0000000000..45d7bb34cd --- /dev/null +++ b/Gems/AWSMetrics/cdk/aws_metrics/aws_utils/resource_name_sanitizer.py @@ -0,0 +1,45 @@ +""" +Copyright (c) Contributors to the Open 3D Engine Project. +For complete copyright and license terms please see the LICENSE at the root of this distribution. + +SPDX-License-Identifier: Apache-2.0 OR MIT +""" + +import hashlib + +MAX_RESOURCE_NAME_LENGTH_MAPPING = { + 'athena_work_group': 128, + 'athena_named_query': 128, + 'cloudformation_stack': 128, + 'cloudwatch_dashboard': 255, + 'cloudwatch_log_group': 512, + 'firehose_delivery_stream': 64, + 'iam_managed_policy': 144, + 'iam_role': 64, + 'kinesis_application': 128, + 'kinesis_stream': 128, + 'lambda_function': 64, + 's3_bucket': 63 +} + + +def sanitize_resource_name(resource_name: str, resource_type: str) -> str: + """ + Truncate the resource name if its length exceeds the limit. + This is the best effort for sanitizing resource names based on the AWS documents since each AWS service + has its unique restrictions. Customers can extend this function for validation or sanitization. + + :param resource_name: Original name of the resource. + :param resource_type: Type of the resource. + :return Sanitized resource name that can be deployed with AWS. + """ + result = resource_name + if not MAX_RESOURCE_NAME_LENGTH_MAPPING.get(resource_type): + return result + + if len(resource_name) > MAX_RESOURCE_NAME_LENGTH_MAPPING[resource_type]: + # PYTHONHASHSEED is set to "random" by default in Python 3.3 and up. Cannot use + # the built-in hash function here since it will give a different return value in each session + digest = "-%x" % (int(hashlib.md5(resource_name.encode('ascii', 'ignore')).hexdigest(), 16) & 0xffffffff) + result = resource_name[:MAX_RESOURCE_NAME_LENGTH_MAPPING[resource_type] - len(digest)] + digest + return result diff --git a/Gems/AWSMetrics/cdk/aws_metrics/batch_analytics.py b/Gems/AWSMetrics/cdk/aws_metrics/batch_analytics.py index 1b6bda7345..5be6562982 100755 --- a/Gems/AWSMetrics/cdk/aws_metrics/batch_analytics.py +++ b/Gems/AWSMetrics/cdk/aws_metrics/batch_analytics.py @@ -11,6 +11,7 @@ from aws_cdk import ( ) from . import aws_metrics_constants +from .aws_utils import resource_name_sanitizer class BatchAnalytics: @@ -37,7 +38,8 @@ class BatchAnalytics: self._athena_work_group = athena.CfnWorkGroup( self._stack, id='AthenaWorkGroup', - name=f'{self._stack.stack_name}-AthenaWorkGroup', + name=resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-AthenaWorkGroup', 'athena_work_group'), recursive_delete_option=True, state='ENABLED', work_group_configuration=athena.CfnWorkGroup.WorkGroupConfigurationProperty( @@ -65,7 +67,8 @@ class BatchAnalytics: athena.CfnNamedQuery( self._stack, id='NamedQuery-CreatePartitionedEventsJson', - name=f'{self._stack.stack_name}-NamedQuery-CreatePartitionedEventsJson', + name=resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-NamedQuery-CreatePartitionedEventsJson', 'athena_named_query'), database=self._events_database_name, query_string="CREATE TABLE events_json " "WITH (format='JSON',partitioned_by=ARRAY['application_id']) " @@ -78,7 +81,8 @@ class BatchAnalytics: athena.CfnNamedQuery( self._stack, id='NamedQuery-TotalEventsLastMonth', - name=f'{self._stack.stack_name}-NamedQuery-TotalEventsLastMonth', + name=resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-NamedQuery-TotalEventsLastMonth', 'athena_named_query'), database=self._events_database_name, query_string="WITH detail AS " "(SELECT date_trunc('month', date(date_parse(CONCAT(year, '-', month, '-', day), '%Y-%m-%d'))) as event_month, * " @@ -93,7 +97,8 @@ class BatchAnalytics: athena.CfnNamedQuery( self._stack, id='NamedQuery-NewUsersLastMonth', - name=f'{self._stack.stack_name}-NamedQuery-NewUsersLastMonth', + name=resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-NamedQuery-NewUsersLastMonth', 'athena_named_query'), database=self._events_database_name, query_string="WITH detail AS (" "SELECT date_trunc('month', date(date_parse(CONCAT(year, '-', month, '-', day), '%Y-%m-%d'))) as event_month, * " diff --git a/Gems/AWSMetrics/cdk/aws_metrics/batch_processing.py b/Gems/AWSMetrics/cdk/aws_metrics/batch_processing.py index 4b40d4393c..4dbb3b2120 100755 --- a/Gems/AWSMetrics/cdk/aws_metrics/batch_processing.py +++ b/Gems/AWSMetrics/cdk/aws_metrics/batch_processing.py @@ -16,6 +16,7 @@ from aws_cdk import ( import os from . import aws_metrics_constants +from .aws_utils import resource_name_sanitizer class BatchProcessing: @@ -42,7 +43,8 @@ class BatchProcessing: """ Generate the events processing lambda to filter the invalid metrics events. """ - events_processing_lambda_name = f'{self._stack.stack_name}-EventsProcessingLambda' + events_processing_lambda_name = resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-EventsProcessingLambda', 'lambda_function') self._create_events_processing_lambda_role(events_processing_lambda_name) self._events_processing_lambda = lambda_.Function( @@ -89,7 +91,8 @@ class BatchProcessing: self._events_processing_lambda_role = iam.Role( self._stack, id='EventsProcessingLambdaRole', - role_name=f'{self._stack.stack_name}-EventsProcessingLambdaRole', + role_name=resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-EventsProcessingLambdaRole', 'iam_role'), assumed_by=iam.ServicePrincipal( service='lambda.amazonaws.com' ), @@ -107,8 +110,10 @@ class BatchProcessing: self._events_firehose_delivery_stream = kinesisfirehose.CfnDeliveryStream( self._stack, - id=f'{self._stack.stack_name}-EventsFirehoseDeliveryStream', + id=f'EventsFirehoseDeliveryStream', delivery_stream_type='KinesisStreamAsSource', + delivery_stream_name=resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-EventsFirehoseDeliveryStream', 'firehose_delivery_stream'), kinesis_stream_source_configuration=kinesisfirehose.CfnDeliveryStream.KinesisStreamSourceConfigurationProperty( kinesis_stream_arn=self._input_stream_arn, role_arn=self._firehose_delivery_stream_role.role_arn @@ -192,7 +197,8 @@ class BatchProcessing: self._firehose_delivery_stream_log_group = logs.LogGroup( self._stack, id='FirehoseLogGroup', - log_group_name=f'{self._stack.stack_name}-FirehoseLogGroup', + log_group_name=resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-FirehoseLogGroup', 'cloudwatch_log_group'), removal_policy=core.RemovalPolicy.DESTROY, retention=logs.RetentionDays.ONE_MONTH ) @@ -299,7 +305,8 @@ class BatchProcessing: self._firehose_delivery_stream_role = iam.Role( self._stack, id='GameEventsFirehoseRole', - role_name=f'{self._stack.stack_name}-GameEventsFirehoseRole', + role_name=resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-GameEventsFirehoseRole', 'iam_role'), assumed_by=iam.ServicePrincipal( service='firehose.amazonaws.com' ), diff --git a/Gems/AWSMetrics/cdk/aws_metrics/dashboard.py b/Gems/AWSMetrics/cdk/aws_metrics/dashboard.py index f86b374abf..32ff0d9c84 100755 --- a/Gems/AWSMetrics/cdk/aws_metrics/dashboard.py +++ b/Gems/AWSMetrics/cdk/aws_metrics/dashboard.py @@ -12,6 +12,7 @@ from aws_cdk import ( from . import aws_metrics_constants from .layout_widget_construct import LayoutWidget +from .aws_utils import resource_name_sanitizer class Dashboard: @@ -28,7 +29,8 @@ class Dashboard: events_processing_lambda_name: str = '', ) -> None: - self._dashboard_name = f"{stack.stack_name}-Dashboard" + self._dashboard_name = resource_name_sanitizer.sanitize_resource_name( + f'{stack.stack_name}-Dashboard', 'cloudwatch_dashboard') self._dashboard = cloudwatch.Dashboard( stack, id="DashBoard", diff --git a/Gems/AWSMetrics/cdk/aws_metrics/data_ingestion.py b/Gems/AWSMetrics/cdk/aws_metrics/data_ingestion.py index 6f31818bf4..a21e629c38 100755 --- a/Gems/AWSMetrics/cdk/aws_metrics/data_ingestion.py +++ b/Gems/AWSMetrics/cdk/aws_metrics/data_ingestion.py @@ -12,10 +12,11 @@ from aws_cdk import ( aws_kinesis as kinesis ) -from . import aws_metrics_constants - import json +from . import aws_metrics_constants +from .aws_utils import resource_name_sanitizer + class DataIngestion: """ @@ -29,7 +30,8 @@ class DataIngestion: self._input_stream = kinesis.Stream( self._stack, id='InputStream', - stream_name=f'{self._stack.stack_name}-InputStream', + stream_name=resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-InputStream', 'kinesis_stream'), shard_count=1 ) diff --git a/Gems/AWSMetrics/cdk/aws_metrics/data_lake_integration.py b/Gems/AWSMetrics/cdk/aws_metrics/data_lake_integration.py index dff165ce56..aaaf03b1fa 100755 --- a/Gems/AWSMetrics/cdk/aws_metrics/data_lake_integration.py +++ b/Gems/AWSMetrics/cdk/aws_metrics/data_lake_integration.py @@ -13,6 +13,7 @@ from aws_cdk import ( ) from . import aws_metrics_constants +from .aws_utils import resource_name_sanitizer class DataLakeIntegration: @@ -49,7 +50,9 @@ class DataLakeIntegration: # a specific name here, only one customer can deploy the bucket successfully. self._analytics_bucket = s3.Bucket( self._stack, - id=f'{self._stack.stack_name}-AnalyticsBucket'.lower(), + id=f'AnalyticsBucket'.lower(), + bucket_name=resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-AnalyticsBucket'.lower(), 's3_bucket'), encryption=s3.BucketEncryption.S3_MANAGED, block_public_access=s3.BlockPublicAccess( block_public_acls=True, @@ -297,7 +300,8 @@ class DataLakeIntegration: self._events_crawler_role = iam.Role( self._stack, id='EventsCrawlerRole', - role_name=f'{self._stack.stack_name}-EventsCrawlerRole', + role_name=resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-EventsCrawlerRole', 'iam_role'), assumed_by=iam.ServicePrincipal( service='glue.amazonaws.com' ), diff --git a/Gems/AWSMetrics/cdk/aws_metrics/real_time_data_processing.py b/Gems/AWSMetrics/cdk/aws_metrics/real_time_data_processing.py index 9809bf0c9c..52fdcdc122 100755 --- a/Gems/AWSMetrics/cdk/aws_metrics/real_time_data_processing.py +++ b/Gems/AWSMetrics/cdk/aws_metrics/real_time_data_processing.py @@ -16,6 +16,7 @@ from aws_cdk import ( import os from . import aws_metrics_constants +from .aws_utils import resource_name_sanitizer class RealTimeDataProcessing: @@ -44,7 +45,8 @@ class RealTimeDataProcessing: self._analytics_application = analytics.CfnApplication( self._stack, 'AnalyticsApplication', - application_name=f'{self._stack.stack_name}-AnalyticsApplication', + application_name=resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-AnalyticsApplication', 'kinesis_application'), inputs=[ analytics.CfnApplication.InputProperty( input_schema=analytics.CfnApplication.InputSchemaProperty( @@ -162,7 +164,8 @@ class RealTimeDataProcessing: kinesis_analytics_role = iam.Role( self._stack, id='AnalyticsApplicationRole', - role_name=f'{self._stack.stack_name}-AnalyticsApplicationRole', + role_name=resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-AnalyticsApplicationRole', 'iam_role'), assumed_by=iam.ServicePrincipal( service='kinesisanalytics.amazonaws.com' ), @@ -178,7 +181,8 @@ class RealTimeDataProcessing: """ Generate the analytics processing lambda to send processed data to CloudWatch for visualization. """ - analytics_processing_function_name = f'{self._stack.stack_name}-AnalyticsProcessingLambdaName' + analytics_processing_function_name = resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-AnalyticsProcessingLambdaName', 'lambda_function') self._analytics_processing_lambda_role = self._create_analytics_processing_lambda_role( analytics_processing_function_name ) @@ -246,7 +250,8 @@ class RealTimeDataProcessing: analytics_processing_lambda_role = iam.Role( self._stack, id='AnalyticsLambdaRole', - role_name=f'{self._stack.stack_name}-AnalyticsLambdaRole', + role_name=resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-AnalyticsLambdaRole', 'iam_role'), assumed_by=iam.ServicePrincipal( service='lambda.amazonaws.com' ), From f3a5e9d9489f7c666c9163847184841a9768de07 Mon Sep 17 00:00:00 2001 From: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> Date: Fri, 23 Jul 2021 15:50:27 -0700 Subject: [PATCH 20/24] changing type that was coming from a cry header (#2397) Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> --- .../Code/Source/Platform/iOS/MicrophoneSystemComponent_iOS.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gems/Microphone/Code/Source/Platform/iOS/MicrophoneSystemComponent_iOS.mm b/Gems/Microphone/Code/Source/Platform/iOS/MicrophoneSystemComponent_iOS.mm index a603aa0ab4..fe8580723a 100644 --- a/Gems/Microphone/Code/Source/Platform/iOS/MicrophoneSystemComponent_iOS.mm +++ b/Gems/Microphone/Code/Source/Platform/iOS/MicrophoneSystemComponent_iOS.mm @@ -238,7 +238,7 @@ public: void ProcessAudio(AudioBufferList* bufferList) { AudioBuffer sourceBuffer = bufferList->mBuffers[0]; - m_captureData->AddData((int16*)sourceBuffer.mData, sourceBuffer.mDataByteSize / 2, m_config.m_numChannels); + m_captureData->AddData((AZ::s16*)sourceBuffer.mData, sourceBuffer.mDataByteSize / 2, m_config.m_numChannels); } From 8f35def25bd3cf977e6eb06761df9ba6b67a7152 Mon Sep 17 00:00:00 2001 From: Benjamin Jillich <43751992+amzn-jillich@users.noreply.github.com> Date: Sun, 25 Jul 2021 23:39:02 -0700 Subject: [PATCH 21/24] AssImp skeleton import improvements (#2348) * Disabled to skip exporting a node in case there are child bones underneath, this broke motion extraction as we skipped the motion extraction node. * Added several helper methods for getting the local space bind pose transform, finding all bones, getting the first bone for a given node name and a recursive has child bones. * Unified the cloned get all bone methods. Signed-off-by: Benjamin Jillich --- .../Importers/AssImpBoneImporter.cpp | 87 ++++---------- .../Importers/AssImpImporterUtilities.cpp | 112 +++++++++++++++++- .../Importers/AssImpImporterUtilities.h | 15 ++- .../Importers/AssImpTransformImporter.cpp | 88 +------------- 4 files changed, 147 insertions(+), 155 deletions(-) diff --git a/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpBoneImporter.cpp b/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpBoneImporter.cpp index 92f84ad8fc..eeddf4a0b3 100644 --- a/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpBoneImporter.cpp +++ b/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpBoneImporter.cpp @@ -41,45 +41,6 @@ namespace AZ } } - void MakeBoneMap(const aiScene* scene, AZStd::unordered_map& boneLookup) - { - AZStd::queue queue; - AZStd::unordered_set nodesWithNoMesh; - - queue.push(scene->mRootNode); - - while (!queue.empty()) - { - const aiNode* currentNode = queue.front(); - queue.pop(); - - if (currentNode->mNumMeshes == 0) - { - nodesWithNoMesh.emplace(currentNode->mName.C_Str()); - } - - for (int childIndex = 0; childIndex < currentNode->mNumChildren; ++childIndex) - { - queue.push(currentNode->mChildren[childIndex]); - } - } - - for (unsigned int meshIndex = 0; meshIndex < scene->mNumMeshes; ++meshIndex) - { - const aiMesh* mesh = scene->mMeshes[meshIndex]; - - for (unsigned int boneIndex = 0; boneIndex < mesh->mNumBones; ++boneIndex) - { - const aiBone* bone = mesh->mBones[boneIndex]; - - if (nodesWithNoMesh.contains(bone->mName.C_Str())) - { - boneLookup.emplace(bone->mName.C_Str(), bone); - } - } - } - } - aiMatrix4x4 CalculateWorldTransform(const aiNode* currentNode) { aiMatrix4x4 transform = {}; @@ -106,37 +67,39 @@ namespace AZ return Events::ProcessingResult::Ignored; } - bool isBone = false; - + AZStd::unordered_multimap boneByNameMap; + FindAllBones(scene, boneByNameMap); + + bool isBone = FindFirstBoneByNodeName(currentNode, boneByNameMap); + if (!isBone) { - AZStd::unordered_map boneLookup; - MakeBoneMap(scene, boneLookup); - - isBone = boneLookup.contains(currentNode->mName.C_Str()); - - // If we have an animation, the bones will be listed in there - if (!isBone) + for(unsigned animIndex = 0; animIndex < scene->mNumAnimations; ++animIndex) { - for(unsigned animIndex = 0; animIndex < scene->mNumAnimations; ++animIndex) + aiAnimation* animation = scene->mAnimations[animIndex]; + + for (unsigned channelIndex = 0; channelIndex < animation->mNumChannels; ++channelIndex) { - aiAnimation* animation = scene->mAnimations[animIndex]; + aiNodeAnim* nodeAnim = animation->mChannels[channelIndex]; - for (unsigned channelIndex = 0; channelIndex < animation->mNumChannels; ++channelIndex) - { - aiNodeAnim* nodeAnim = animation->mChannels[channelIndex]; - - if (nodeAnim->mNodeName == currentNode->mName) - { - isBone = true; - break; - } - } - - if (isBone) + if (nodeAnim->mNodeName == currentNode->mName) { + isBone = true; break; } } + + if (isBone) + { + break; + } + } + + // In case any of the children, or children of children is a bone, make sure to not skip this node. + // Don't do this for the scene root itself, else wise all mesh nodes will be exported as bones and pollute the skeleton. + if (currentNode != scene->mRootNode && + RecursiveHasChildBone(currentNode, boneByNameMap)) + { + isBone = true; } } diff --git a/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpImporterUtilities.cpp b/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpImporterUtilities.cpp index 15bb65399c..81feff7d69 100644 --- a/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpImporterUtilities.cpp +++ b/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpImporterUtilities.cpp @@ -6,12 +6,13 @@ * */ -#include - -#include -#include - #include +#include +#include +#include +#include +#include +#include namespace AZ { @@ -85,6 +86,107 @@ namespace AZ return combinedTransform; } + + void FindAllBones(const aiScene* scene, AZStd::unordered_multimap& outBoneByNameMap) + { + outBoneByNameMap.clear(); + AZStd::queue queue; + AZStd::unordered_set nodesWithNoMesh; + + queue.push(scene->mRootNode); + + while (!queue.empty()) + { + const aiNode* currentNode = queue.front(); + queue.pop(); + + if (currentNode->mNumMeshes == 0) + { + nodesWithNoMesh.emplace(currentNode->mName.C_Str()); + } + + for (int childIndex = 0; childIndex < currentNode->mNumChildren; ++childIndex) + { + queue.push(currentNode->mChildren[childIndex]); + } + } + + for (unsigned meshIndex = 0; meshIndex < scene->mNumMeshes; ++meshIndex) + { + const aiMesh* mesh = scene->mMeshes[meshIndex]; + + for (unsigned boneIndex = 0; boneIndex < mesh->mNumBones; ++boneIndex) + { + const aiBone* bone = mesh->mBones[boneIndex]; + + if (nodesWithNoMesh.contains(bone->mName.C_Str())) + { + outBoneByNameMap.emplace(bone->mName.C_Str(), bone); + } + } + } + } + + DataTypes::MatrixType GetLocalSpaceBindPoseTransform(const aiScene* scene, const aiNode* node) + { + AZStd::unordered_multimap boneByNameMap; + FindAllBones(scene, boneByNameMap); + + const aiBone* bone = FindFirstBoneByNodeName(node, boneByNameMap); + if (bone) + { + const DataTypes::MatrixType inverseOffsetMatrix = AssImpSDKWrapper::AssImpTypeConverter::ToTransform(bone->mOffsetMatrix).GetInverseFull(); + + const aiBone* parentBone = FindFirstBoneByNodeName(node->mParent, boneByNameMap); + if (parentBone) + { + const DataTypes::MatrixType parentBoneOffsetMatrix = AssImpSDKWrapper::AssImpTypeConverter::ToTransform(parentBone->mOffsetMatrix); + return parentBoneOffsetMatrix * inverseOffsetMatrix; + } + else + { + return inverseOffsetMatrix; + } + } + + return AssImpSDKWrapper::AssImpTypeConverter::ToTransform(GetConcatenatedLocalTransform(node)); + } + + const aiBone* FindFirstBoneByNodeName(const aiNode* node, AZStd::unordered_multimap& boneByNameMap) + { + if (!node) + { + return nullptr; + } + + auto boneIterator = boneByNameMap.find(node->mName.C_Str()); + if (boneIterator != boneByNameMap.end()) + { + return boneIterator->second; + } + + return nullptr; + } + + bool RecursiveHasChildBone(const aiNode* node, const AZStd::unordered_multimap& boneByNameMap) + { + const bool isBone = boneByNameMap.contains(node->mName.C_Str()); + if (isBone) + { + return true; + } + + for (int childIndex = 0; childIndex < node->mNumChildren; ++childIndex) + { + const aiNode* childNode = node->mChildren[childIndex]; + if (RecursiveHasChildBone(childNode, boneByNameMap)) + { + return true; + } + } + + return false; + } } // namespace SceneBuilder } // namespace SceneAPI } // namespace AZ diff --git a/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpImporterUtilities.h b/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpImporterUtilities.h index 5a943f339c..a629fe52d8 100644 --- a/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpImporterUtilities.h +++ b/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpImporterUtilities.h @@ -9,13 +9,15 @@ #pragma once #include +#include #include +#include +struct aiBone; struct aiNode; struct aiScene; struct aiString; - namespace AZ::SceneAPI::SceneBuilder { inline constexpr char PivotNodeMarker[] = "_$AssimpFbx$_"; @@ -30,5 +32,16 @@ namespace AZ::SceneAPI::SceneBuilder // Gets the entire, combined local transform for a node taking pivot nodes into account. When pivot nodes are not used, this just returns the node's transform aiMatrix4x4 GetConcatenatedLocalTransform(const aiNode* currentNode); + + DataTypes::MatrixType GetLocalSpaceBindPoseTransform(const aiScene* scene, const aiNode* node); + + // Gather all bones from the scene. (Bone in AssImp corresponds to nodes that influence any of the vertices). + void FindAllBones(const aiScene* scene, AZStd::unordered_multimap& outBoneByNameMap); + + // Find the first bone with the name of the given node. + const aiBone* FindFirstBoneByNodeName(const aiNode* node, AZStd::unordered_multimap& boneByNameMap); + + // Check if the given node or any of its children, or children of children, is a bone by checking if the node name is part of the given map. + bool RecursiveHasChildBone(const aiNode* node, const AZStd::unordered_multimap& boneByNameMap); } // namespace AZ diff --git a/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpTransformImporter.cpp b/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpTransformImporter.cpp index eba1063a1e..134c408bae 100644 --- a/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpTransformImporter.cpp +++ b/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpTransformImporter.cpp @@ -42,45 +42,6 @@ namespace AZ serializeContext->Class()->Version(1); } } - - void GetAllBones(const aiScene* scene, AZStd::unordered_multimap& boneLookup) - { - AZStd::queue queue; - AZStd::unordered_set nodesWithNoMesh; - - queue.push(scene->mRootNode); - - while (!queue.empty()) - { - const aiNode* currentNode = queue.front(); - queue.pop(); - - if (currentNode->mNumMeshes == 0) - { - nodesWithNoMesh.emplace(currentNode->mName.C_Str()); - } - - for (int childIndex = 0; childIndex < currentNode->mNumChildren; ++childIndex) - { - queue.push(currentNode->mChildren[childIndex]); - } - } - - for (unsigned meshIndex = 0; meshIndex < scene->mNumMeshes; ++meshIndex) - { - const aiMesh* mesh = scene->mMeshes[meshIndex]; - - for (unsigned boneIndex = 0; boneIndex < mesh->mNumBones; ++boneIndex) - { - const aiBone* bone = mesh->mBones[boneIndex]; - - if (nodesWithNoMesh.contains(bone->mName.C_Str())) - { - boneLookup.emplace(bone->mName.C_Str(), bone); - } - } - } - } Events::ProcessingResult AssImpTransformImporter::ImportTransform(AssImpSceneNodeAppendedContext& context) { @@ -93,54 +54,7 @@ namespace AZ return Events::ProcessingResult::Ignored; } - AZStd::unordered_multimap boneLookup; - GetAllBones(scene, boneLookup); - - auto boneIterator = boneLookup.find(currentNode->mName.C_Str()); - const bool isBone = boneIterator != boneLookup.end(); - - DataTypes::MatrixType localTransform; - - if (isBone) - { - AZStd::vector offsets, inverseOffsets; - auto iteratingNode = currentNode; - - while (iteratingNode && boneLookup.count(iteratingNode->mName.C_Str())) - { - AZStd::string name = iteratingNode->mName.C_Str(); - - auto range = boneLookup.equal_range(name); - - if (range.first != range.second) - { - // There can be multiple offsetMatrices for a given bone, we're only interested in grabbing the first one - auto boneFirstOffsetMatrix = range.first->second->mOffsetMatrix; - auto azMat = AssImpSDKWrapper::AssImpTypeConverter::ToTransform(boneFirstOffsetMatrix); - offsets.push_back(azMat); - inverseOffsets.push_back(azMat.GetInverseFull()); - } - - iteratingNode = iteratingNode->mParent; - } - - if (inverseOffsets.size() == 1) - { - // If this is the root bone, just use the inverseOffset, otherwise the equation below just results in the identity matrix - localTransform = inverseOffsets[0]; - } - else - { - localTransform = offsets.at(1) // parent bone offset - * inverseOffsets.at(inverseOffsets.size() - 1) // Inverse of root bone offset - * offsets.at(offsets.size() - 1) // Root bone offset - * inverseOffsets.at(0); // Inverse of current node offset - } - } - else - { - localTransform = AssImpSDKWrapper::AssImpTypeConverter::ToTransform(GetConcatenatedLocalTransform(currentNode)); - } + DataTypes::MatrixType localTransform = GetLocalSpaceBindPoseTransform(scene, currentNode); // Don't bother adding a node with the identity matrix if (localTransform == DataTypes::MatrixType::Identity()) From 3ade0987421c85bc74ec46c53acd1fbe5796f523 Mon Sep 17 00:00:00 2001 From: amzn-sean <75276488+amzn-sean@users.noreply.github.com> Date: Mon, 26 Jul 2021 11:16:13 +0100 Subject: [PATCH 22/24] support node switching in a pipe (#2345) Signed-off-by: amzn-sean <75276488+amzn-sean@users.noreply.github.com> --- scripts/build/Jenkins/Jenkinsfile | 232 +++++++++++++++++++++--------- 1 file changed, 161 insertions(+), 71 deletions(-) diff --git a/scripts/build/Jenkins/Jenkinsfile b/scripts/build/Jenkins/Jenkinsfile index ae6ac59c3c..9aa09e136e 100644 --- a/scripts/build/Jenkins/Jenkinsfile +++ b/scripts/build/Jenkins/Jenkinsfile @@ -266,32 +266,42 @@ def CheckoutRepo(boolean disableSubmodules = false) { palRm('commitdate') } +def HandleDriveMount(String snapshot, String repositoryName, String projectName, String pipeline, String branchName, String platform, String buildType, String workspace, boolean recreateVolume = false) { + unstash name: 'incremental_build_script' + + def pythonCmd = '' + if(env.IS_UNIX) pythonCmd = 'sudo -E python3 -u ' + else pythonCmd = 'python3 -u ' + + if(recreateVolume) { + palSh("${pythonCmd} ${INCREMENTAL_BUILD_SCRIPT_PATH} --action delete --repository_name ${repositoryName} --project ${projectName} --pipeline ${pipeline} --branch ${branchName} --platform ${platform} --build_type ${buildType}", 'Deleting volume', winSlashReplacement=false) + } + timeout(5) { + palSh("${pythonCmd} ${INCREMENTAL_BUILD_SCRIPT_PATH} --action mount --snapshot ${snapshot} --repository_name ${repositoryName} --project ${projectName} --pipeline ${pipeline} --branch ${branchName} --platform ${platform} --build_type ${buildType}", 'Mounting volume', winSlashReplacement=false) + } + + if(env.IS_UNIX) { + sh label: 'Setting volume\'s ownership', + script: """ + if sudo test ! -d "${workspace}"; then + sudo mkdir -p ${workspace} + cd ${workspace}/.. + sudo chown -R lybuilder:root . + fi + """ + } +} + def PreBuildCommonSteps(Map pipelineConfig, String snapshot, String repositoryName, String projectName, String pipeline, String branchName, String platform, String buildType, String workspace, boolean mount = true, boolean disableSubmodules = false) { echo 'Starting pre-build common steps...' if (mount) { - unstash name: 'incremental_build_script' - - def pythonCmd = '' - if(env.IS_UNIX) pythonCmd = 'sudo -E python3 -u ' - else pythonCmd = 'python3 -u ' - - if(env.RECREATE_VOLUME?.toBoolean()) { - palSh("${pythonCmd} ${INCREMENTAL_BUILD_SCRIPT_PATH} --action delete --repository_name ${repositoryName} --project ${projectName} --pipeline ${pipeline} --branch ${branchName} --platform ${platform} --build_type ${buildType}", 'Deleting volume', winSlashReplacement=false) - } - timeout(5) { - palSh("${pythonCmd} ${INCREMENTAL_BUILD_SCRIPT_PATH} --action mount --snapshot ${snapshot} --repository_name ${repositoryName} --project ${projectName} --pipeline ${pipeline} --branch ${branchName} --platform ${platform} --build_type ${buildType}", 'Mounting volume', winSlashReplacement=false) - } - - if(env.IS_UNIX) { - sh label: 'Setting volume\'s ownership', - script: """ - if sudo test ! -d "${workspace}"; then - sudo mkdir -p ${workspace} - cd ${workspace}/.. - sudo chown -R lybuilder:root . - fi - """ + if(env.RECREATE_VOLUME?.toBoolean()){ + echo 'Starting to recreating drive...' + HandleDriveMount(snapshot, repositoryName, projectName, pipeline, branchName, platform, buildType, workspace, true) + } else { + echo 'Starting to mounting drive...' + HandleDriveMount(snapshot, repositoryName, projectName, pipeline, branchName, platform, buildType, workspace, false) } } @@ -399,10 +409,14 @@ def PostBuildCommonSteps(String workspace, boolean mount = true) { } } -def CreateSetupStage(Map pipelineConfig, String snapshot, String repositoryName, String projectName, String pipelineName, String branchName, String platformName, String jobName, Map environmentVars) { +def CreateSetupStage(Map pipelineConfig, String snapshot, String repositoryName, String projectName, String pipelineName, String branchName, String platformName, String jobName, Map environmentVars, boolean onlyMountEBSVolume = false) { return { stage('Setup') { - PreBuildCommonSteps(pipelineConfig, snapshot, repositoryName, projectName, pipelineName, branchName, platformName, jobName, environmentVars['WORKSPACE'], environmentVars['MOUNT_VOLUME']) + if(onlyMountEBSVolume) { + HandleDriveMount(snapshot, repositoryName, projectName, pipelineName, branchName, platformName, jobName, environmentVars['WORKSPACE'], false) + } else { + PreBuildCommonSteps(pipelineConfig, snapshot, repositoryName, projectName, pipelineName, branchName, platformName, jobName, environmentVars['WORKSPACE'], environmentVars['MOUNT_VOLUME']) + } } } } @@ -439,6 +453,128 @@ def CreateTeardownStage(Map environmentVars) { } } +def CreateSingleNode(Map pipelineConfig, def platform, def build_job, Map envVars, String branchName, String pipelineName, String repositoryName, String projectName, boolean onlyMountEBSVolume = false) { + def nodeLabel = envVars['NODE_LABEL'] + return { + node("${nodeLabel}") { + if(isUnix()) { // Has to happen inside a node + envVars['IS_UNIX'] = 1 + } + withEnv(GetEnvStringList(envVars)) { + def build_job_name = build_job.key + try { + CreateSetupStage(pipelineConfig, snapshot, repositoryName, projectName, pipelineName, branchName, platform.key, build_job.key, envVars, onlyMountEBSVolume).call() + + if(build_job.value.steps) { //this is a pipe with many steps so create all the build stages + build_job.value.steps.each { build_step -> + build_job_name = build_step + CreateBuildStage(pipelineConfig, platform.key, build_step, envVars).call() + } + } else { + CreateBuildStage(pipelineConfig, platform.key, build_job.key, envVars).call() + } + } + catch(Exception e) { + // https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/model/Result.java + // {SUCCESS,UNSTABLE,FAILURE,NOT_BUILT,ABORTED} + def currentResult = envVars['ON_FAILURE_MARK'] ?: 'FAILURE' + if (currentResult == 'FAILURE') { + currentBuild.result = 'FAILURE' + error "FAILURE: ${e}" + } else if (currentResult == 'UNSTABLE') { + currentBuild.result = 'UNSTABLE' + unstable(message: "UNSTABLE: ${e}") + } + } + finally { + def params = platform.value.build_types[build_job_name].PARAMETERS + if (env.MARS_REPO && params && params.containsKey('TEST_METRICS') && params.TEST_METRICS == 'True') { + def output_directory = params.OUTPUT_DIRECTORY + def configuration = params.CONFIGURATION + CreateTestMetricsStage(pipelineConfig, branchName, envVars, build_job_name, output_directory, configuration).call() + } + if (params && params.containsKey('TEST_RESULTS') && params.TEST_RESULTS == 'True') { + CreateExportTestResultsStage(pipelineConfig, platform.key, build_job_name, envVars, params).call() + } + CreateTeardownStage(envVars).call() + } + } + } + } +} + +// Used in CreateBuildJobs() to preprocess the build_job steps to programically create +// Node sections with a set of steps that can run on that node. +class PipeStepJobData { + String m_nodeLabel = "" + def m_steps = [] + + PipeStepJobData(String label) { + this.m_nodeLabel = label + } + + def addStep(def step) { + this.m_steps.add(step) + } +} + +def CreateBuildJobs(Map pipelineConfig, def platform, def build_job, Map envVars, String branchName, String pipelineName, String repositoryName, String projectName) { + + // if this is a pipeline, split jobs based on the NODE_LABEL + if(build_job.value.steps) { + def defaultLabel = envVars['NODE_LABEL'] + def lastNodeLable = "" + def jobList = [] + def currentIdx = -1; + + // iterate the steps to build the order of node label + steps sets. + // Order matters, as it is executed from first to last. + // example layout. + // node A + // step 1 + // step 2 + // node B + // step 3 + // node C + // step 4 + build_job.value.steps.each { build_step -> + //if node label defined + if(platform.value.build_types[build_step] && platform.value.build_types[build_step].PIPELINE_ENV && + platform.value.build_types[build_step].PIPELINE_ENV['NODE_LABEL']) { + + //if the last node label doen't match the new one, append it. + if(platform.value.build_types[build_step].PIPELINE_ENV['NODE_LABEL'] != lastNodeLable) { + lastNodeLable = platform.value.build_types[build_step].PIPELINE_ENV['NODE_LABEL'] + jobList.add(new PipeStepJobData(lastNodeLable)) + currentIdx++ + } + } + //no label define, so it needs to run on the default node label + else if(lastNodeLable != defaultLabel) { //if the last node is not the default, append default + lastNodeLable = defaultLabel + jobList.add(new PipeStepJobData(lastNodeLable)) + currentIdx++ + } + //add the build_step to the current node + jobList[currentIdx].addStep(build_step) + } + + return { + jobList.eachWithIndex{ element, idx -> + //update the node label + steps to the discovered data + envVars['NODE_LABEL'] = element.m_nodeLabel + build_job.value.steps = element.m_steps + //no any additional nodes just mount the drive, do not handle clean parameters as that will be done by the first node. + boolean onlyMountEBSVolume = idx != 0; + //add this node + CreateSingleNode(pipelineConfig, platform, build_job, envVars, branchName, pipelineName, repositoryName, projectName, onlyMountEBSVolume).call() + } + } + } else { + return CreateSingleNode(pipelineConfig, platform, build_job, envVars, branchName, pipelineName, repositoryName, projectName) + } +} + def projectName = '' def pipelineName = '' def branchName = '' @@ -527,55 +663,9 @@ try { if (IsJobEnabled(branchName, build_job, pipelineName, platform.key)) { // User can filter jobs, jobs are tagged by pipeline def envVars = GetBuildEnvVars(platform.value.PIPELINE_ENV ?: EMPTY_JSON, build_job.value.PIPELINE_ENV ?: EMPTY_JSON, pipelineName) envVars['JOB_NAME'] = "${branchName}_${platform.key}_${build_job.key}" // backwards compatibility, some scripts rely on this - def nodeLabel = envVars['NODE_LABEL'] someBuildHappened = true - buildConfigs["${platform.key} [${build_job.key}]"] = { - node("${nodeLabel}") { - if(isUnix()) { // Has to happen inside a node - envVars['IS_UNIX'] = 1 - } - withEnv(GetEnvStringList(envVars)) { - def build_job_name = build_job.key - try { - CreateSetupStage(pipelineConfig, snapshot, repositoryName, projectName, pipelineName, branchName, platform.key, build_job.key, envVars).call() - - if(build_job.value.steps) { //this is a pipe with many steps so create all the build stages - build_job.value.steps.each { build_step -> - build_job_name = build_step - CreateBuildStage(pipelineConfig, platform.key, build_step, envVars).call() - } - } else { - CreateBuildStage(pipelineConfig, platform.key, build_job.key, envVars).call() - } - } - catch(Exception e) { - // https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/model/Result.java - // {SUCCESS,UNSTABLE,FAILURE,NOT_BUILT,ABORTED} - def currentResult = envVars['ON_FAILURE_MARK'] ?: 'FAILURE' - if (currentResult == 'FAILURE') { - currentBuild.result = 'FAILURE' - error "FAILURE: ${e}" - } else if (currentResult == 'UNSTABLE') { - currentBuild.result = 'UNSTABLE' - unstable(message: "UNSTABLE: ${e}") - } - } - finally { - def params = platform.value.build_types[build_job_name].PARAMETERS - if (env.MARS_REPO && params && params.containsKey('TEST_METRICS') && params.TEST_METRICS == 'True') { - def output_directory = params.OUTPUT_DIRECTORY - def configuration = params.CONFIGURATION - CreateTestMetricsStage(pipelineConfig, branchName, envVars, build_job_name, output_directory, configuration).call() - } - if (params && params.containsKey('TEST_RESULTS') && params.TEST_RESULTS == 'True') { - CreateExportTestResultsStage(pipelineConfig, platform.key, build_job_name, envVars, params).call() - } - CreateTeardownStage(envVars).call() - } - } - } - } + buildConfigs["${platform.key} [${build_job.key}]"] = CreateBuildJobs(pipelineConfig, platform, build_job, envVars, branchName, pipelineName, repositoryName, projectName) } } } From 73706fa04e6d17a26ab4a7b24f8984d5228bb438 Mon Sep 17 00:00:00 2001 From: John Jones-Steele Date: Mon, 26 Jul 2021 16:10:21 +0100 Subject: [PATCH 23/24] Added Tests to check code is correct Signed-off-by: John Jones-Steele --- .../AzQtComponents/Utilities/Conversions.cpp | 2 +- .../AzToolsFramework/Tests/SpinBoxTests.cpp | 43 +++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/Code/Framework/AzQtComponents/AzQtComponents/Utilities/Conversions.cpp b/Code/Framework/AzQtComponents/AzQtComponents/Utilities/Conversions.cpp index c9327d199e..bef41bb487 100644 --- a/Code/Framework/AzQtComponents/AzQtComponents/Utilities/Conversions.cpp +++ b/Code/Framework/AzQtComponents/AzQtComponents/Utilities/Conversions.cpp @@ -43,7 +43,7 @@ namespace AzQtComponents { const QChar decimalPoint = locale.decimalPoint(); const QChar zeroDigit = locale.zeroDigit(); - const int numToStringDecimals = numDecimals < 8 ? 8 : numDecimals; + const int numToStringDecimals = AZStd::max(numDecimals, 20); // We want to truncate, not round. toString will round, so we add extra decimal places to the formatting // so we can remove the last values diff --git a/Code/Framework/AzToolsFramework/Tests/SpinBoxTests.cpp b/Code/Framework/AzToolsFramework/Tests/SpinBoxTests.cpp index d1c3ee5b3c..eb09c68cee 100644 --- a/Code/Framework/AzToolsFramework/Tests/SpinBoxTests.cpp +++ b/Code/Framework/AzToolsFramework/Tests/SpinBoxTests.cpp @@ -77,6 +77,19 @@ namespace UnitTest m_intSpinBox.reset(); } + QString setupTruncationTest(QString textValue) + { + QString retval; + m_doubleSpinBoxWithLineEdit->setDecimals(7); + m_doubleSpinBoxWithLineEdit->setDisplayDecimals(3); + m_doubleSpinBoxWithLineEdit->setFocus(); + m_doubleSpinBoxWithLineEdit->GetLineEdit()->setText(textValue); + m_doubleSpinBoxWithLineEdit->clearFocus(); + + return m_doubleSpinBoxWithLineEdit->textFromValue(m_doubleSpinBoxWithLineEdit->value()); + } + + AZStd::unique_ptr m_dummyWidget; AZStd::unique_ptr m_intSpinBox; AZStd::unique_ptr m_doubleSpinBox; @@ -277,4 +290,34 @@ namespace UnitTest // test would result in a crash EXPECT_TRUE(m_intSpinBox.get() == nullptr); } + + TEST_F(SpinBoxFixture, SpinBoxCheckHighValueTruncatesCorrectly) + { + QString value = setupTruncationTest("0.9999999"); + + EXPECT_TRUE(value == "0.999"); + } + + TEST_F(SpinBoxFixture, SpinBoxCheckLowValueTruncatesCorrectly) + { + QString value = setupTruncationTest("0.0000001"); + + EXPECT_TRUE(value == "0.0"); + } + + TEST_F(SpinBoxFixture, SpinBoxCheckBugValuesTruncatesCorrectly) + { + QString value = setupTruncationTest("0.12395"); + + EXPECT_TRUE(value == "0.123"); + + value = setupTruncationTest("0.94496"); + + EXPECT_TRUE(value == "0.944"); + + value = setupTruncationTest("0.0009999"); + + EXPECT_TRUE(value == "0.0"); + } + } // namespace UnitTest From 24db0c82eb5ec5d54c45381043613f9c0ec47e89 Mon Sep 17 00:00:00 2001 From: jiaweig <51759646+jiaweig-amzn@users.noreply.github.com> Date: Mon, 26 Jul 2021 10:17:35 -0700 Subject: [PATCH 24/24] fix the assert from incorrect size calculation (#2404) Signed-off-by: jiaweig --- Gems/Atom/RHI/Vulkan/Code/Source/RHI/PhysicalDevice.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gems/Atom/RHI/Vulkan/Code/Source/RHI/PhysicalDevice.cpp b/Gems/Atom/RHI/Vulkan/Code/Source/RHI/PhysicalDevice.cpp index 76a5486953..c00c1804b9 100644 --- a/Gems/Atom/RHI/Vulkan/Code/Source/RHI/PhysicalDevice.cpp +++ b/Gems/Atom/RHI/Vulkan/Code/Source/RHI/PhysicalDevice.cpp @@ -261,7 +261,7 @@ namespace AZ VK_KHR_SHADER_FLOAT_CONTROLS_EXTENSION_NAME } }; - [[maybe_unused]] uint32_t optionalExtensionCount = sizeof(optionalExtensions) / sizeof(VK_EXT_SAMPLE_LOCATIONS_EXTENSION_NAME); + [[maybe_unused]] uint32_t optionalExtensionCount = aznumeric_cast(optionalExtensions.size()); AZ_Assert(optionalExtensionCount == static_cast(OptionalDeviceExtension::Count), "The order and size must match the enum OptionalDeviceExtensions.");