From 1dabb39b9834666a8a1b99608a0a4c7124cd34c3 Mon Sep 17 00:00:00 2001 From: nvsickle Date: Fri, 9 Apr 2021 12:40:43 -0700 Subject: [PATCH 1/8] Migrate fixes from CodeCommit branch --- Code/Sandbox/Editor/EditorViewportWidget.cpp | 166 ++++++------------ Code/Sandbox/Editor/EditorViewportWidget.h | 26 +-- Code/Sandbox/Editor/RenderViewport.cpp | 10 -- Code/Sandbox/Editor/Viewport.cpp | 3 +- Code/Sandbox/Editor/Viewport.h | 2 +- .../Editor/ViewportManipulatorController.cpp | 18 +- .../Source/RPI.Public/ViewportContext.cpp | 3 + .../RPI.Public/ViewportContextManager.cpp | 6 +- .../Source/Viewport/RenderViewportWidget.cpp | 1 + .../AtomBridge/Code/CMakeLists.txt | 4 +- .../AtomShim_RendPipeline.cpp | 2 +- .../CryRenderAtomShim/AtomShim_Renderer.cpp | 7 +- 12 files changed, 94 insertions(+), 154 deletions(-) diff --git a/Code/Sandbox/Editor/EditorViewportWidget.cpp b/Code/Sandbox/Editor/EditorViewportWidget.cpp index c09a428b3f..f26d4ff1dc 100644 --- a/Code/Sandbox/Editor/EditorViewportWidget.cpp +++ b/Code/Sandbox/Editor/EditorViewportWidget.cpp @@ -30,6 +30,7 @@ #include #include #include +#include // AzFramework #include @@ -96,6 +97,10 @@ #include +AZ_CVAR( + bool, ed_visibility_logTiming, false, nullptr, AZ::ConsoleFunctorFlags::Null, + "Output the timing of the new IVisibilitySystem query"); + EditorViewportWidget* EditorViewportWidget::m_pPrimaryViewport = nullptr; #if AZ_TRAIT_OS_PLATFORM_APPLE @@ -139,35 +144,6 @@ namespace AZ::ViewportHelpers }; } // namespace AZ::ViewportHelpers -struct EditorViewportWidget::SScopedCurrentContext -{ - const EditorViewportWidget* m_viewport; - EditorViewportWidget::SPreviousContext m_previousContext; - - explicit SScopedCurrentContext(const EditorViewportWidget* viewport) - : m_viewport(viewport) - { - m_previousContext = viewport->SetCurrentContext(); - - // During normal updates of RenderViewport the value of m_cameraSetForWidgetRenderingCount is expected to be 0. - // This is to guarantee no loss in performance by tracking unnecessary calls to SetCurrentContext/RestorePreviousContext. - // If some code makes additional calls to Pre/PostWidgetRendering then the assert will be triggered because - // m_cameraSetForWidgetRenderingCount will be greater than 0. - // There is a legitimate case where the counter can be greater than 0. This is when QtViewport is processing mouse callbacks. - // QtViewport::MouseCallback() is surrounded by Pre/PostWidgetRendering and the m_processingMouseCallbacksCounter - // tracks this specific case. If an update of a RenderViewport happens while processing the mouse callback, - // for example when showing a QMessageBox, then both counters must match. - AZ_Assert(viewport->m_cameraSetForWidgetRenderingCount == viewport->m_processingMouseCallbacksCounter, - "SScopedCurrentContext constructor was called while viewport widget context is active " - "- this is unnecessary"); - } - - ~SScopedCurrentContext() - { - m_viewport->RestorePreviousContext(m_previousContext); - } -}; - ////////////////////////////////////////////////////////////////////////// // EditorViewportWidget ////////////////////////////////////////////////////////////////////////// @@ -224,7 +200,7 @@ EditorViewportWidget::EditorViewportWidget(const QString& name, QWidget* parent) m_manipulatorManager = GetIEditor()->GetViewManager()->GetManipulatorManager(); if (!m_pPrimaryViewport) { - m_pPrimaryViewport = this; + SetAsActiveViewport(); } } @@ -489,7 +465,7 @@ void EditorViewportWidget::Update() { if (CheckRespondToInput()) // If this is the focused window, set primary viewport. { - m_pPrimaryViewport = this; + SetAsActiveViewport(); } else if (!m_bUpdateViewport) // Skip this viewport. { @@ -546,8 +522,6 @@ void EditorViewportWidget::Update() // Render { - SScopedCurrentContext context(this); - // TODO: Move out this logic to a controller and refactor to work with Atom // m_renderer->SetClearColor(Vec3(0.4f, 0.4f, 0.4f)); // 3D engine stats @@ -581,6 +555,19 @@ void EditorViewportWidget::Update() gEnv->pSystem->SetViewCamera(CurCamera); } + { + auto start = std::chrono::steady_clock::now(); + + m_entityVisibilityQuery.UpdateVisibility(GetCameraState()); + + if (ed_visibility_logTiming) + { + auto stop = std::chrono::steady_clock::now(); + std::chrono::duration diff = stop - start; + AZ_Printf("Visibility", "FindVisibleEntities (new) - Duration: %f", diff); + } + } + QtViewport::Update(); PopDisableRendering(); @@ -680,17 +667,13 @@ void EditorViewportWidget::OnEditorNotifyEvent(EEditorNotifyEvent event) if (deviceInfo) { + // Note: This may also need to adjust the viewport size outputToHMD->Set(1); - m_previousContext = SetCurrentContext(deviceInfo->renderWidth, deviceInfo->renderHeight); SetActiveWindow(); SetFocus(); SetSelected(true); } } - else - { - m_previousContext = SetCurrentContext(); - } SetCurrentCursor(STD_CURSOR_GAME); AzFramework::InputSystemCursorConstraintRequestBus::Handler::BusConnect(); } @@ -706,7 +689,6 @@ void EditorViewportWidget::OnEditorNotifyEvent(EEditorNotifyEvent event) { outputToHMD->Set(0); } - RestorePreviousContext(m_previousContext); m_bInRotateMode = false; m_bInMoveMode = false; m_bInOrbitMode = false; @@ -1180,15 +1162,7 @@ void EditorViewportWidget::FindVisibleEntities(AZStd::vector& visi { FUNCTION_PROFILER(GetIEditor()->GetSystem(), PROFILE_EDITOR); - if (m_displayContext.GetView() == nullptr) - { - return; - } - - const AZStd::vector& entityIdCache = - m_displayContext.GetView()->GetVisibleObjectsCache()->GetEntityIdCache(); - - visibleEntitiesOut.assign(entityIdCache.begin(), entityIdCache.end()); + visibleEntitiesOut.assign(m_entityVisibilityQuery.Begin(), m_entityVisibilityQuery.End()); } QPoint EditorViewportWidget::ViewportWorldToScreen(const AZ::Vector3& worldPosition) @@ -1233,6 +1207,7 @@ void EditorViewportWidget::SetViewportId(int id) // Now that we have an ID, we can initialize our viewport. m_renderViewport = new AtomToolsFramework::RenderViewportWidget(id, this); + m_defaultViewportContextName = m_renderViewport->GetViewportContext()->GetName(); QBoxLayout* layout = new QBoxLayout(QBoxLayout::Direction::TopToBottom, this); layout->setContentsMargins(QMargins()); layout->addWidget(m_renderViewport); @@ -1244,6 +1219,11 @@ void EditorViewportWidget::SetViewportId(int id) m_renderViewport->GetControllerList()->Add(AZStd::make_shared()); m_renderViewport->GetControllerList()->Add(AZStd::make_shared()); UpdateScene(); + + if (m_pPrimaryViewport == this) + { + SetAsActiveViewport(); + } } void EditorViewportWidget::ConnectViewportInteractionRequestBus() @@ -1986,9 +1966,6 @@ void EditorViewportWidget::RenderSelectedRegion() Vec3 EditorViewportWidget::WorldToView3D(const Vec3& wp, [[maybe_unused]] int nFlags) const { - AZ_Assert(m_cameraSetForWidgetRenderingCount > 0, - "WorldToView3D was called but viewport widget rendering was not set. PreWidgetRendering must be called before."); - Vec3 out(0, 0, 0); float x, y, z; @@ -2007,10 +1984,6 @@ Vec3 EditorViewportWidget::WorldToView3D(const Vec3& wp, [[maybe_unused]] int nF ////////////////////////////////////////////////////////////////////////// QPoint EditorViewportWidget::WorldToView(const Vec3& wp) const { - AZ_Assert(m_cameraSetForWidgetRenderingCount > 0, - "WorldToView was called but viewport widget rendering was not set. PreWidgetRendering must be called before."); - - return m_renderViewport->ViewportWorldToScreen(LYVec3ToAZVec3(wp)); } ////////////////////////////////////////////////////////////////////////// @@ -2178,9 +2151,6 @@ void EditorViewportWidget::ProjectToScreen(float ptx, float pty, float ptz, floa ////////////////////////////////////////////////////////////////////////// void EditorViewportWidget::ViewToWorldRay(const QPoint& vp, Vec3& raySrc, Vec3& rayDir) const { - AZ_Assert(m_cameraSetForWidgetRenderingCount > 0, - "ViewToWorldRay was called but SScopedCurrentContext was not set at a higher scope! This means the camera for this call is incorrect."); - QRect rc = m_rcClient; Vec3 pos0, pos1; @@ -2245,7 +2215,7 @@ bool EditorViewportWidget::CheckRespondToInput() const return false; } - if (!hasFocus()) + if (!hasFocus() && !m_renderViewport->hasFocus()) { return false; } @@ -2650,55 +2620,6 @@ void EditorViewportWidget::OnStopPlayInEditor() } } -////////////////////////////////////////////////////////////////////////// -EditorViewportWidget::SPreviousContext EditorViewportWidget::SetCurrentContext(int /*newWidth*/, int /*newHeight*/) const -{ - SPreviousContext x; - - return x; -} - -////////////////////////////////////////////////////////////////////////// -EditorViewportWidget::SPreviousContext EditorViewportWidget::SetCurrentContext() const -{ - const auto r = rect(); - return SetCurrentContext(r.width(), r.height()); -} - -////////////////////////////////////////////////////////////////////////// -void EditorViewportWidget::RestorePreviousContext(const SPreviousContext& /*x*/) const -{ -} - -void EditorViewportWidget::PreWidgetRendering() -{ - // if we have not already set the render context for the viewport, do it now - // based on the current state of the renderer/viewport, record the previous - // context to restore afterwards - if (m_cameraSetForWidgetRenderingCount == 0) - { - m_preWidgetContext = SetCurrentContext(); - } - - // keep track of how many times we've attempted to update the context - m_cameraSetForWidgetRenderingCount++; -} - -void EditorViewportWidget::PostWidgetRendering() -{ - if (m_cameraSetForWidgetRenderingCount > 0) - { - m_cameraSetForWidgetRenderingCount--; - - // unwinding - when the viewport context is no longer required, - // restore the previous context when widget rendering first began - if (m_cameraSetForWidgetRenderingCount == 0) - { - RestorePreviousContext(m_preWidgetContext); - } - } -} - ////////////////////////////////////////////////////////////////////////// void EditorViewportWidget::OnCameraFOVVariableChanged([[maybe_unused]] IVariable* var) { @@ -2888,4 +2809,33 @@ void EditorViewportWidget::UpdateCameraFromViewportContext() m_Camera.SetZRange(cameraState.m_nearClip, cameraState.m_farClip); } +void EditorViewportWidget::SetAsActiveViewport() +{ + auto viewportContextManager = AZ::Interface::Get(); + + const AZ::Name defaultContextName = viewportContextManager->GetDefaultViewportContextName(); + + // If another viewport was active before, restore its name to its per-ID one. + if (m_pPrimaryViewport && m_pPrimaryViewport != this && m_pPrimaryViewport->m_renderViewport) + { + auto viewportContext = m_pPrimaryViewport->m_renderViewport->GetViewportContext(); + if (viewportContext) + { + viewportContextManager->PopView(defaultContextName, viewportContext->GetDefaultView()); + viewportContextManager->RenameViewportContext(viewportContext, m_pPrimaryViewport->m_defaultViewportContextName); + } + } + + m_pPrimaryViewport = this; + if (m_renderViewport) + { + auto viewportContext = m_renderViewport->GetViewportContext(); + if (viewportContext) + { + viewportContextManager->PushView(defaultContextName, viewportContext->GetDefaultView()); + viewportContextManager->RenameViewportContext(viewportContext, defaultContextName); + } + } +} + #include diff --git a/Code/Sandbox/Editor/EditorViewportWidget.h b/Code/Sandbox/Editor/EditorViewportWidget.h index cf5ce4c5b5..652c1dbe8e 100644 --- a/Code/Sandbox/Editor/EditorViewportWidget.h +++ b/Code/Sandbox/Editor/EditorViewportWidget.h @@ -354,26 +354,6 @@ protected: void RenderAll(); - struct SPreviousContext - { - CCamera rendererCamera; - HWND window; - int width; - int height; - bool mainViewport; - }; - - SPreviousContext m_preWidgetContext; - - // Create an auto-sized render context that is sized based on the Editor's current - // viewport. - SPreviousContext SetCurrentContext() const; - - SPreviousContext SetCurrentContext(int newWidth, int newHeight) const; - void RestorePreviousContext(const SPreviousContext& x) const; - - void PreWidgetRendering() override; - void PostWidgetRendering() override; void OnBeginPrepareRender() override; // Update the safe frame, safe action, safe title, and borders rectangles based on @@ -579,6 +559,7 @@ protected: void BuildDragDropContext(AzQtComponents::ViewportDragContext& context, const QPoint& pt) override; private: + void SetAsActiveViewport(); void PushDisableRendering(); void PopDisableRendering(); bool IsRenderingDisabled() const; @@ -606,13 +587,10 @@ private: AzFramework::EntityVisibilityQuery m_entityVisibilityQuery; - SPreviousContext m_previousContext; QSet m_keyDown; bool m_freezeViewportInput = false; - size_t m_cameraSetForWidgetRenderingCount = 0; ///< How many calls to PreWidgetRendering happened before - ///< subsequent calls to PostWidetRendering. AZStd::shared_ptr m_manipulatorManager; // Used to prevent circular set camera events @@ -627,5 +605,7 @@ private: AZ::RPI::ViewportContext::MatrixChangedEvent::Handler m_cameraProjectionMatrixChangeHandler; AzFramework::DebugDisplayRequests* m_debugDisplay = nullptr; + AZ::Name m_defaultViewportContextName; + AZ_POP_DISABLE_DLL_EXPORT_MEMBER_WARNING }; diff --git a/Code/Sandbox/Editor/RenderViewport.cpp b/Code/Sandbox/Editor/RenderViewport.cpp index 34d8b73a0b..125aeaf23c 100644 --- a/Code/Sandbox/Editor/RenderViewport.cpp +++ b/Code/Sandbox/Editor/RenderViewport.cpp @@ -94,9 +94,6 @@ AZ_CVAR( bool, ed_visibility_use, true, nullptr, AZ::ConsoleFunctorFlags::Null, "Enable/disable using the new IVisibilitySystem for Entity visibility determination"); -AZ_CVAR( - bool, ed_visibility_logTiming, false, nullptr, AZ::ConsoleFunctorFlags::Null, - "Output the timing of the new IVisibilitySystem query"); CRenderViewport* CRenderViewport::m_pPrimaryViewport = nullptr; @@ -1394,13 +1391,6 @@ void CRenderViewport::Update() auto start = std::chrono::steady_clock::now(); m_entityVisibilityQuery.UpdateVisibility(GetCameraState()); - - if (ed_visibility_logTiming) - { - auto stop = std::chrono::steady_clock::now(); - std::chrono::duration diff = stop - start; - AZ_Printf("Visibility", "FindVisibleEntities (new) - Duration: %f", diff); - } } { diff --git a/Code/Sandbox/Editor/Viewport.cpp b/Code/Sandbox/Editor/Viewport.cpp index 175fe0c6a0..af8eafe7ae 100644 --- a/Code/Sandbox/Editor/Viewport.cpp +++ b/Code/Sandbox/Editor/Viewport.cpp @@ -218,10 +218,11 @@ QtViewport::QtViewport(QWidget* parent) // Create drop target to handle Qt drop events. setAcceptDrops(true); - m_renderOverlay.setVisible(false); + m_renderOverlay.setVisible(true); m_renderOverlay.setUpdatesEnabled(false); m_renderOverlay.setMouseTracking(true); m_renderOverlay.setObjectName("renderOverlay"); + m_renderOverlay.winId(); // Force the render overlay to create a backing native window m_viewportUi.InitializeViewportUi(this, &m_renderOverlay); diff --git a/Code/Sandbox/Editor/Viewport.h b/Code/Sandbox/Editor/Viewport.h index 6e483e7150..acc5a3acfb 100644 --- a/Code/Sandbox/Editor/Viewport.h +++ b/Code/Sandbox/Editor/Viewport.h @@ -280,7 +280,7 @@ public: virtual CViewport *asCViewport() { return this; } protected: - CLayoutViewPane* m_viewPane; + CLayoutViewPane* m_viewPane = nullptr; CViewManager* m_viewManager; AZ_PUSH_DISABLE_DLL_EXPORT_MEMBER_WARNING // Viewport matrix. diff --git a/Code/Sandbox/Editor/ViewportManipulatorController.cpp b/Code/Sandbox/Editor/ViewportManipulatorController.cpp index 748fb81e6b..bb7726f76b 100644 --- a/Code/Sandbox/Editor/ViewportManipulatorController.cpp +++ b/Code/Sandbox/Editor/ViewportManipulatorController.cpp @@ -20,8 +20,8 @@ #include -static const auto ManipulatorPriority = AzFramework::ViewportControllerPriority::High; -static const auto InteractionPriority = AzFramework::ViewportControllerPriority::Low; +static const auto ManipulatorPriority = AzFramework::ViewportControllerPriority::Highest; +static const auto InteractionPriority = AzFramework::ViewportControllerPriority::High; namespace SandboxEditor { @@ -127,12 +127,20 @@ bool ViewportManipulatorControllerInstance::HandleInputChannelEvent(const AzFram m_state.m_mouseButtons.m_mouseButtons |= static_cast(mouseButton); if (IsDoubleClick(mouseButton)) { - m_pendingDoubleClicks.erase(mouseButton); + // Only remove the double click flag once we're done processing both Manipulator and Interaction events + if (event.m_priority == InteractionPriority) + { + m_pendingDoubleClicks.erase(mouseButton); + } eventType = MouseEvent::DoubleClick; } else { - m_pendingDoubleClicks[mouseButton] = m_curTime; + // Only insert the double click timing once we're done processing both Manipulator and Interaction events, to avoid a false IsDoubleClick positive + if (event.m_priority == InteractionPriority) + { + m_pendingDoubleClicks[mouseButton] = m_curTime; + } eventType = MouseEvent::Down; } } @@ -161,7 +169,7 @@ bool ViewportManipulatorControllerInstance::HandleInputChannelEvent(const AzFram { mouseInteraction.m_mouseButtons.m_mouseButtons = static_cast(overrideButton.value()); } - MouseInteractionEvent mouseEvent = MouseInteractionEvent(mouseInteraction, eventType.value()); + mouseInteraction.m_interactionId.m_viewportId = GetViewportId(); // Depending on priority, we dispatch to either the manipulator or viewport interaction event const auto& targetInteractionEvent = diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/ViewportContext.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/ViewportContext.cpp index 88c1215a08..12268fca9f 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/ViewportContext.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/ViewportContext.cpp @@ -192,6 +192,9 @@ namespace AZ { m_defaultView = view; UpdatePipelineView(); + + m_viewMatrixChangedEvent.Signal(view->GetWorldToViewMatrix()); + m_projectionMatrixChangedEvent.Signal(view->GetViewToClipMatrix()); } } diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/ViewportContextManager.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/ViewportContextManager.cpp index 8ab6339015..dd27f4774b 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/ViewportContextManager.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/ViewportContextManager.cpp @@ -142,7 +142,7 @@ namespace AZ params.renderScene ); viewportContext->GetWindowContext()->RegisterAssociatedViewportContext(viewportContext); - RegisterViewportContext(contextName, viewportContext); + RegisterViewportContext(nameToUse, viewportContext); return viewportContext; } @@ -170,7 +170,9 @@ namespace AZ AZ_Assert(false, "Attempted to rename ViewportContext \"%s\" to \"%s\", but \"%s\" is already assigned to another ViewportContext", viewportContext->m_name.GetCStr(), newContextName.GetCStr(), newContextName.GetCStr()); return; } - RegisterViewportContext(newContextName, viewportContext); + GetOrCreateViewStackForContext(newContextName); + viewportContext->m_name = newContextName; + UpdateViewForContext(newContextName); } void ViewportContextManager::EnumerateViewportContexts(AZStd::function visitorFunction) diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Viewport/RenderViewportWidget.cpp b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Viewport/RenderViewportWidget.cpp index 132ef3a0a4..ee2dbb87cf 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Viewport/RenderViewportWidget.cpp +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Viewport/RenderViewportWidget.cpp @@ -42,6 +42,7 @@ namespace AtomToolsFramework params.device = AZ::RHI::RHISystemInterface::Get()->GetDevice(); params.windowHandle = reinterpret_cast(winId()); params.id = id; + AzFramework::WindowRequestBus::Handler::BusConnect(params.windowHandle); m_viewportContext = viewportContextManager->CreateViewportContext(AZ::Name(), params); SetControllerList(AZStd::make_shared()); diff --git a/Gems/AtomLyIntegration/AtomBridge/Code/CMakeLists.txt b/Gems/AtomLyIntegration/AtomBridge/Code/CMakeLists.txt index 0eb6898b07..b684e445b3 100644 --- a/Gems/AtomLyIntegration/AtomBridge/Code/CMakeLists.txt +++ b/Gems/AtomLyIntegration/AtomBridge/Code/CMakeLists.txt @@ -21,7 +21,7 @@ ly_add_target( Include COMPILE_DEFINITIONS PRIVATE - ENABLE_ATOM_DEBUG_DISPLAY=0 + ENABLE_ATOM_DEBUG_DISPLAY=1 BUILD_DEPENDENCIES PUBLIC AZ::AtomCore @@ -43,7 +43,7 @@ ly_add_target( Include COMPILE_DEFINITIONS PRIVATE - ENABLE_ATOM_DEBUG_DISPLAY=0 + ENABLE_ATOM_DEBUG_DISPLAY=1 BUILD_DEPENDENCIES PRIVATE Gem::Atom_AtomBridge.Static diff --git a/Gems/AtomLyIntegration/CryRenderAtomShim/AtomShim_RendPipeline.cpp b/Gems/AtomLyIntegration/CryRenderAtomShim/AtomShim_RendPipeline.cpp index ba45d49889..4ce4377b30 100644 --- a/Gems/AtomLyIntegration/CryRenderAtomShim/AtomShim_RendPipeline.cpp +++ b/Gems/AtomLyIntegration/CryRenderAtomShim/AtomShim_RendPipeline.cpp @@ -165,7 +165,7 @@ void CAtomShimRenderer::EF_EndEf3D([[maybe_unused]] const int nFlags, [[maybe_un // Only render the UI Canvas and the Console on the main window // If we're not in the editor, don't bother to check viewport. - if (!gEnv->IsEditor() || m_currContext->m_isMainViewport) + if (!gEnv->IsEditor() || m_currContext == nullptr || m_currContext->m_isMainViewport) { EBUS_EVENT(AZ::RenderNotificationsBus, OnScene3DEnd); } diff --git a/Gems/AtomLyIntegration/CryRenderAtomShim/AtomShim_Renderer.cpp b/Gems/AtomLyIntegration/CryRenderAtomShim/AtomShim_Renderer.cpp index de3670dd6b..b9c9e3eb58 100644 --- a/Gems/AtomLyIntegration/CryRenderAtomShim/AtomShim_Renderer.cpp +++ b/Gems/AtomLyIntegration/CryRenderAtomShim/AtomShim_Renderer.cpp @@ -295,7 +295,12 @@ void CAtomShimRenderer::EndFrame() if (!m_viewportContext) { auto viewContextManager = AZ::Interface::Get(); - m_viewportContext = viewContextManager->GetViewportContextByName(viewContextManager->GetDefaultViewportContextName()); + auto viewportContext = viewContextManager->GetDefaultViewportContext(); + // If the viewportContext exists and is created with the default ID, we can safely assume control + if (viewportContext && viewportContext->GetId() == -10) + { + m_viewportContext = viewportContext; + } } if (m_viewportContext) From d09dc500d35febc4961f71f86f7c0c5c64da7c65 Mon Sep 17 00:00:00 2001 From: jackalbe Date: Fri, 9 Apr 2021 14:55:39 -0500 Subject: [PATCH 2/8] fake commit --- Gems/PythonAssetBuilder/3rdParty/readme.md | 1 - 1 file changed, 1 deletion(-) diff --git a/Gems/PythonAssetBuilder/3rdParty/readme.md b/Gems/PythonAssetBuilder/3rdParty/readme.md index c462b15cdb..7a986b06dc 100644 --- a/Gems/PythonAssetBuilder/3rdParty/readme.md +++ b/Gems/PythonAssetBuilder/3rdParty/readme.md @@ -1,4 +1,3 @@ # Python Asset Builder This gem is meant to run Python scripts that want to run as asset builders in the Lumberyard asset processing system. - From 2190787c40a120d51504780ad2b98a0be6c1b5f4 Mon Sep 17 00:00:00 2001 From: nvsickle Date: Fri, 9 Apr 2021 14:19:05 -0700 Subject: [PATCH 3/8] Fix build --- Gems/AtomLyIntegration/CryRenderAtomShim/AtomShim_Renderer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gems/AtomLyIntegration/CryRenderAtomShim/AtomShim_Renderer.cpp b/Gems/AtomLyIntegration/CryRenderAtomShim/AtomShim_Renderer.cpp index b9c9e3eb58..f40d2f35ee 100644 --- a/Gems/AtomLyIntegration/CryRenderAtomShim/AtomShim_Renderer.cpp +++ b/Gems/AtomLyIntegration/CryRenderAtomShim/AtomShim_Renderer.cpp @@ -295,7 +295,7 @@ void CAtomShimRenderer::EndFrame() if (!m_viewportContext) { auto viewContextManager = AZ::Interface::Get(); - auto viewportContext = viewContextManager->GetDefaultViewportContext(); + auto viewportContext = viewContextManager->GetViewportContextByName(viewContextManager->GetDefaultViewportContextName()); // If the viewportContext exists and is created with the default ID, we can safely assume control if (viewportContext && viewportContext->GetId() == -10) { From 1c47a264943634d2be7c432795dd5979bdeccc84 Mon Sep 17 00:00:00 2001 From: nvsickle Date: Fri, 9 Apr 2021 14:19:41 -0700 Subject: [PATCH 4/8] Clarify the reason for the default view juggling in SetAsActiveViewport --- Code/Sandbox/Editor/EditorViewportWidget.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Code/Sandbox/Editor/EditorViewportWidget.cpp b/Code/Sandbox/Editor/EditorViewportWidget.cpp index f26d4ff1dc..31314abc89 100644 --- a/Code/Sandbox/Editor/EditorViewportWidget.cpp +++ b/Code/Sandbox/Editor/EditorViewportWidget.cpp @@ -2821,6 +2821,7 @@ void EditorViewportWidget::SetAsActiveViewport() auto viewportContext = m_pPrimaryViewport->m_renderViewport->GetViewportContext(); if (viewportContext) { + // Remove the old viewport's camera from the stack, as it's no longer the owning viewport viewportContextManager->PopView(defaultContextName, viewportContext->GetDefaultView()); viewportContextManager->RenameViewportContext(viewportContext, m_pPrimaryViewport->m_defaultViewportContextName); } @@ -2832,6 +2833,8 @@ void EditorViewportWidget::SetAsActiveViewport() auto viewportContext = m_renderViewport->GetViewportContext(); if (viewportContext) { + // Push our camera onto the default viewport's view stack to preserve camera state continuity + // Other views can still be pushed on top of our view for e.g. game mode viewportContextManager->PushView(defaultContextName, viewportContext->GetDefaultView()); viewportContextManager->RenameViewportContext(viewportContext, defaultContextName); } From 04d171ecefea57d845f56404990d0a9b9dfa2a87 Mon Sep 17 00:00:00 2001 From: shiranj Date: Fri, 9 Apr 2021 17:09:14 -0700 Subject: [PATCH 5/8] Pipeline fails when using a new empty ebs volume --- AutomatedReview/Jenkinsfile | 150 +++++++++++++++++------------------- 1 file changed, 71 insertions(+), 79 deletions(-) diff --git a/AutomatedReview/Jenkinsfile b/AutomatedReview/Jenkinsfile index 54ec974917..da42da5a1f 100644 --- a/AutomatedReview/Jenkinsfile +++ b/AutomatedReview/Jenkinsfile @@ -16,7 +16,7 @@ INCREMENTAL_BUILD_SCRIPT_PATH = 'scripts/build/bootstrap/incremental_build_util. EMPTY_JSON = readJSON text: '{}' -ENGINE_REPOSITORY_NAME = 'o3de' +ENGINE_REPOSITORY_NAME = env.DEFAULT_REPOSITORY_NAME def pipelineProperties = [] @@ -96,7 +96,7 @@ def IsJobEnabled(buildTypeMap, pipelineName, platformName) { if (params[platformName]) { if(buildTypeMap.value.TAGS) { return buildTypeMap.value.TAGS.contains(pipelineName) - } + } } } return false @@ -194,9 +194,9 @@ def GetBuildEnvVars(Map platformEnv, Map buildTypeEnv, String pipelineName) { } buildTypeEnv.each { var -> // This may override the above one if there is an entry defined by the job - envVarMap[var.key] = var.value + envVarMap[var.key] = var.value } - + // Environment that only applies to to Jenkins tweaks. // For 3rdParty downloads, we store them in the EBS volume so we can reuse them across node // instances. This allow us to scale up and down without having to re-download 3rdParty @@ -223,7 +223,7 @@ def PullFilesFromGit(String filenamePath, String branchName, boolean failIfNotFo folderPathParts.remove(folderPathParts.size()-1) // remove the filename def folderPath = folderPathParts.join('/') if (folderPath.contains('*')) { - + def currentPath = '' for (int i = 0; i < folderPathParts.size(); i++) { if (folderPathParts[i] == '*') { @@ -259,7 +259,7 @@ def PullFilesFromGit(String filenamePath, String branchName, boolean failIfNotFo } else { - def errorFile = "${folderPath}/error.txt" + def errorFile = "${folderPath}/error.txt" palMkdir(folderPath) retry(3) { try { @@ -273,7 +273,7 @@ def PullFilesFromGit(String filenamePath, String branchName, boolean failIfNotFo win_filenamePath = filenamePath.replace('/', '\\') bat label: "Downloading ${win_filenamePath}", script: "aws codecommit get-file --repository-name ${repositoryName} --commit-specifier ${branchName} --file-path ${filenamePath} --query fileContent --output text 2>${errorFile} > ${win_filenamePath}_encoded" - bat label: 'Decoding', + bat label: 'Decoding', script: "certutil -decode ${win_filenamePath}_encoded ${win_filenamePath}" } palRm("${filenamePath}_encoded") @@ -296,7 +296,7 @@ def SetLfsCredentials(cmd, lbl = '') { if (env.IS_UNIX) { sh label: lbl, script: cmd - } else { + } else { bat label: lbl, script: cmd } @@ -325,19 +325,17 @@ def CheckoutBootstrapScripts(String branchName) { } def CheckoutRepo(boolean disableSubmodules = false) { - dir(ENGINE_REPOSITORY_NAME) { - palSh('git lfs uninstall', 'Git LFS Uninstall') // Prevent git from pulling lfs objects during checkout + palSh('git lfs uninstall', 'Git LFS Uninstall') // Prevent git from pulling lfs objects during checkout - if(fileExists('.git')) { - // If the repository after checkout is locked, likely we took a snapshot while git was running, - // to leave the repo in a usable state, garbagecollect. This also helps in situations where - def indexLockFile = '.git/index.lock' - if(fileExists(indexLockFile)) { - palSh('git gc', 'Git GarbageCollect') - } - if(fileExists(indexLockFile)) { // if it is still there, remove it - palRm(indexLockFile) - } + if(fileExists('.git')) { + // If the repository after checkout is locked, likely we took a snapshot while git was running, + // to leave the repo in a usable state, garbagecollect. This also helps in situations where + def indexLockFile = '.git/index.lock' + if(fileExists(indexLockFile)) { + palSh('git gc', 'Git GarbageCollect') + } + if(fileExists(indexLockFile)) { // if it is still there, remove it + palRm(indexLockFile) } } @@ -350,52 +348,41 @@ def CheckoutRepo(boolean disableSubmodules = false) { retryAttempt = retryAttempt + 1 if(params.PULL_REQUEST_ID) { // This is a pull request build. Perform merge with destination branch before building. - dir(ENGINE_REPOSITORY_NAME) { - checkout scm: [ - $class: 'GitSCM', - branches: scm.branches, - extensions: [ - [$class: 'PreBuildMerge', options: [mergeRemote: 'origin', mergeTarget: params.DESTINATION_BRANCH]], - [$class: 'SubmoduleOption', disableSubmodules: disableSubmodules, recursiveSubmodules: true], - [$class: 'CheckoutOption', timeout: 60] - ], - userRemoteConfigs: scm.userRemoteConfigs - ] - } + checkout scm: [ + $class: 'GitSCM', + branches: scm.branches, + extensions: [ + [$class: 'PreBuildMerge', options: [mergeRemote: 'origin', mergeTarget: params.DESTINATION_BRANCH]], + [$class: 'SubmoduleOption', disableSubmodules: disableSubmodules, recursiveSubmodules: true], + [$class: 'CheckoutOption', timeout: 60] + ], + userRemoteConfigs: scm.userRemoteConfigs + ] } else { - dir(ENGINE_REPOSITORY_NAME) { - checkout scm: [ - $class: 'GitSCM', - branches: scm.branches, - extensions: [ - [$class: 'SubmoduleOption', disableSubmodules: disableSubmodules, recursiveSubmodules: true], - [$class: 'CheckoutOption', timeout: 60] - ], - userRemoteConfigs: scm.userRemoteConfigs - ] - } + checkout scm: [ + $class: 'GitSCM', + branches: scm.branches, + extensions: [ + [$class: 'SubmoduleOption', disableSubmodules: disableSubmodules, recursiveSubmodules: true], + [$class: 'CheckoutOption', timeout: 60] + ], + userRemoteConfigs: scm.userRemoteConfigs + ] } } - // Add folder where we will store the 3rdParty downloads and packages - if(!fileExists('3rdParty')) { - palMkdir('3rdParty') - } - - dir(ENGINE_REPOSITORY_NAME) { - // Run lfs in a separate step. Jenkins is unable to load the credentials for the custom LFS endpoint - withCredentials([usernamePassword(credentialsId: "${env.GITHUB_USER}", passwordVariable: 'accesstoken', usernameVariable: 'username')]) { - SetLfsCredentials("git config -f .lfsconfig lfs.url https://${username}:${accesstoken}@${env.LFS_URL}", 'Set credentials') - } - palSh('git lfs install', 'Git LFS Install') - palSh('git lfs pull', 'Git LFS Pull') - - // CHANGE_ID is used by some scripts to identify uniquely the current change (usually metric jobs) - palSh('git rev-parse HEAD > commitid', 'Getting commit id') - env.CHANGE_ID = readFile file: 'commitid' - env.CHANGE_ID = env.CHANGE_ID.trim() - palRm('commitid') + // Run lfs in a separate step. Jenkins is unable to load the credentials for the custom LFS endpoint + withCredentials([usernamePassword(credentialsId: "${env.GITHUB_USER}", passwordVariable: 'accesstoken', usernameVariable: 'username')]) { + SetLfsCredentials("git config -f .lfsconfig lfs.url https://${username}:${accesstoken}@${env.LFS_URL}", 'Set credentials') } + palSh('git lfs install', 'Git LFS Install') + palSh('git lfs pull', 'Git LFS Pull') + + // CHANGE_ID is used by some scripts to identify uniquely the current change (usually metric jobs) + palSh('git rev-parse HEAD > commitid', 'Getting commit id') + env.CHANGE_ID = readFile file: 'commitid' + env.CHANGE_ID = env.CHANGE_ID.trim() + palRm('commitid') } def PreBuildCommonSteps(Map pipelineConfig, String projectName, String pipeline, String branchName, String platform, String buildType, String workspace, boolean mount = true, boolean disableSubmodules = false) { @@ -419,7 +406,7 @@ def PreBuildCommonSteps(Map pipelineConfig, String projectName, String pipeline, sh label: 'Setting volume\'s ownership', script: """ if sudo test ! -d "${workspace}"; then - sudo mkdir -p ${workspace} + sudo mkdir -p ${workspace} cd ${workspace}/.. sudo chown -R lybuilder:root . fi @@ -436,28 +423,33 @@ def PreBuildCommonSteps(Map pipelineConfig, String projectName, String pipeline, } dir(workspace) { + // Add folder where we will store the 3rdParty downloads and packages + if(!fileExists('3rdParty')) { + palMkdir('3rdParty') + } + } + + dir("${workspace}/${ENGINE_REPOSITORY_NAME}") { CheckoutRepo(disableSubmodules) // Get python - dir(ENGINE_REPOSITORY_NAME) { - if(env.IS_UNIX) { - sh label: 'Getting python', - script: 'python/get_python.sh' - } else { - bat label: 'Getting python', - script: 'python/get_python.bat' - } + if(env.IS_UNIX) { + sh label: 'Getting python', + script: 'python/get_python.sh' + } else { + bat label: 'Getting python', + script: 'python/get_python.bat' + } - if(env.CLEAN_OUTPUT_DIRECTORY.toBoolean() || env.CLEAN_ASSETS.toBoolean()) { - def command = "${pipelineConfig.BUILD_ENTRY_POINT} --platform ${platform} --type clean" - if (env.IS_UNIX) { - sh label: "Running ${platform} clean", - script: "${pipelineConfig.PYTHON_DIR}/python.sh -u ${command}" - } else { - bat label: "Running ${platform} clean", - script: "${pipelineConfig.PYTHON_DIR}/python.cmd -u ${command}".replace('/','\\') - } + if(env.CLEAN_OUTPUT_DIRECTORY.toBoolean() || env.CLEAN_ASSETS.toBoolean()) { + def command = "${pipelineConfig.BUILD_ENTRY_POINT} --platform ${platform} --type clean" + if (env.IS_UNIX) { + sh label: "Running ${platform} clean", + script: "${pipelineConfig.PYTHON_DIR}/python.sh -u ${command}" + } else { + bat label: "Running ${platform} clean", + script: "${pipelineConfig.PYTHON_DIR}/python.cmd -u ${command}".replace('/','\\') } } } From 53d3ae436d9375d4dc04a2361f934a78e43b1c74 Mon Sep 17 00:00:00 2001 From: shiranj Date: Fri, 9 Apr 2021 17:35:47 -0700 Subject: [PATCH 6/8] Pipeline fails when using a new empty ebs volume --- scripts/build/Jenkins/Jenkinsfile | 293 ++++++++---------------------- 1 file changed, 71 insertions(+), 222 deletions(-) diff --git a/scripts/build/Jenkins/Jenkinsfile b/scripts/build/Jenkins/Jenkinsfile index 08bf5e90f1..015dc7cd14 100644 --- a/scripts/build/Jenkins/Jenkinsfile +++ b/scripts/build/Jenkins/Jenkinsfile @@ -27,15 +27,7 @@ def pipelineParameters = [ booleanParam(defaultValue: false, description: 'Deletes the contents of the output directories of the AssetProcessor before building.', name: 'CLEAN_ASSETS'), booleanParam(defaultValue: false, description: 'Deletes the contents of the workspace and forces a complete pull.', name: 'CLEAN_WORKSPACE'), booleanParam(defaultValue: false, description: 'Recreates the volume used for the workspace. The volume will be created out of a snapshot taken from main.', name: 'RECREATE_VOLUME'), - string(defaultValue: '', description: 'Filters and overrides the list of jobs to run for each of the below platforms (comma-separated). Can\'t be used during a pull request.', name: 'JOB_LIST_OVERRIDE'), - - // Pull Request Parameters - string(defaultValue: '', description: '', name: 'DESTINATION_BRANCH'), - string(defaultValue: '', description: '', name: 'DESTINATION_COMMIT'), - string(defaultValue: '', description: '', name: 'PULL_REQUEST_ID'), - string(defaultValue: '', description: '', name: 'REPOSITORY_NAME'), - string(defaultValue: '', description: '', name: 'SOURCE_BRANCH'), - string(defaultValue: '', description: '', name: 'SOURCE_COMMIT') + string(defaultValue: '', description: 'Filters and overrides the list of jobs to run for each of the below platforms (comma-separated). Can\'t be used during a pull request.', name: 'JOB_LIST_OVERRIDE') ] def palSh(cmd, lbl = '', winSlashReplacement = true) { @@ -86,17 +78,13 @@ def palRmDir(path) { def IsJobEnabled(buildTypeMap, pipelineName, platformName) { def job_list_override = params.JOB_LIST_OVERRIDE.tokenize(',') - if(params.PULL_REQUEST_ID) { // dont allow pull requests to filter platforms/jobs - if(buildTypeMap.value.TAGS) { - return buildTypeMap.value.TAGS.contains(pipelineName) - } - } else if (!job_list_override.isEmpty()) { + if (!job_list_override.isEmpty()) { return params[platformName] && job_list_override.contains(buildTypeMap.key); } else { if (params[platformName]) { if(buildTypeMap.value.TAGS) { return buildTypeMap.value.TAGS.contains(pipelineName) - } + } } } return false @@ -117,11 +105,8 @@ def RegexMatcher(str, regex) { return matcher ? matcher.group(1) : null } -def LoadPipelineConfig(String pipelineName, String branchName, String scmType) { +def LoadPipelineConfig(String pipelineName, String branchName) { echo 'Loading pipeline config' - if (scmType == 'codecommit') { - PullFilesFromGit(PIPELINE_CONFIG_FILE, branchName, true, ENGINE_REPOSITORY_NAME) - } def pipelineConfig = {} pipelineConfig = readJSON file: PIPELINE_CONFIG_FILE palRm(PIPELINE_CONFIG_FILE) @@ -133,10 +118,6 @@ def LoadPipelineConfig(String pipelineName, String branchName, String scmType) { if (!env.IS_UNIX) { platform_regex = platform_regex.replace('/','\\\\') } - echo "Downloading platform pipeline configs ${pipeline_config}" - if (scmType == 'codecommit') { - PullFilesFromGit(pipeline_config, branchName, false, ENGINE_REPOSITORY_NAME) - } echo "Searching platform pipeline configs in ${pipeline_config} using ${platform_regex}" for (pipeline_config_path in findFiles(glob: pipeline_config)) { echo "\tFound platform pipeline config ${pipeline_config_path}" @@ -155,10 +136,6 @@ def LoadPipelineConfig(String pipelineName, String branchName, String scmType) { if (!env.IS_UNIX) { platform_regex = platform_regex.replace('/','\\\\') } - echo "Downloading configs ${build_config}" - if (scmType == 'codecommit') { - PullFilesFromGit(build_config, branchName, false, ENGINE_REPOSITORY_NAME) - } echo "Searching configs in ${build_config} using ${platform_regex}" for (build_config_path in findFiles(glob: build_config)) { echo "\tFound config ${build_config_path}" @@ -171,16 +148,6 @@ def LoadPipelineConfig(String pipelineName, String branchName, String scmType) { return pipelineConfig } -def GetSCMType() { - def gitUrl = scm.getUserRemoteConfigs()[0].getUrl() - if (gitUrl ==~ /https:\/\/git-codecommit.*/) { - return 'codecommit' - } else if (gitUrl ==~ /https:\/\/github.com.*/) { - return 'github' - } - return 'unknown' -} - def GetBuildEnvVars(Map platformEnv, Map buildTypeEnv, String pipelineName) { def envVarMap = [:] platformPipelineEnv = platformEnv['ENV'] ?: [:] @@ -194,9 +161,9 @@ def GetBuildEnvVars(Map platformEnv, Map buildTypeEnv, String pipelineName) { } buildTypeEnv.each { var -> // This may override the above one if there is an entry defined by the job - envVarMap[var.key] = var.value + envVarMap[var.key] = var.value } - + // Environment that only applies to to Jenkins tweaks. // For 3rdParty downloads, we store them in the EBS volume so we can reuse them across node // instances. This allow us to scale up and down without having to re-download 3rdParty @@ -214,89 +181,11 @@ def GetEnvStringList(Map envVarMap) { return strList } -// Pulls/downloads files from the repo through codecommit. Despite Glob matching is NOT supported, '*' is supported -// as a folder or filename (not a portion, it has to be the whole folder or filename) -def PullFilesFromGit(String filenamePath, String branchName, boolean failIfNotFound = true, String repositoryName = env.DEFAULT_REPOSITORY_NAME) { - echo "PullFilesFromGit filenamePath=${filenamePath} branchName=${branchName} repositoryName=${repositoryName}" - def folderPathParts = filenamePath.tokenize('/') - def filename = folderPathParts[folderPathParts.size()-1] - folderPathParts.remove(folderPathParts.size()-1) // remove the filename - def folderPath = folderPathParts.join('/') - if (folderPath.contains('*')) { - - def currentPath = '' - for (int i = 0; i < folderPathParts.size(); i++) { - if (folderPathParts[i] == '*') { - palMkdir(currentPath) - retry(3) { palSh("aws codecommit get-folder --repository-name ${repositoryName} --commit-specifier ${branchName} --folder-path ${currentPath} > ${currentPath}/.codecommit", "GetFolder ${currentPath}") } - def folderInfo = readJSON file: "${currentPath}/.codecommit" - folderInfo.subFolders.each { folder -> - def newSubPath = currentPath + '/' + folder.relativePath - for (int j = i+1; j < folderPathParts.size(); j++) { - newSubPath = newSubPath + '/' + folderPathParts[j] - } - newSubPath = newSubPath + '/' + filename - PullFilesFromGit(newSubPath, branchName, false, repositoryName) - } - palRm("${currentPath}/.codecommit") - } - if (i == 0) { - currentPath = folderPathParts[i] - } else { - currentPath = currentPath + '/' + folderPathParts[i] - } - } - - } else if (filename.contains('*')) { - - palMkdir(folderPath) - retry(3) { palSh("aws codecommit get-folder --repository-name ${repositoryName} --commit-specifier ${branchName} --folder-path ${folderPath} > ${folderPath}/.codecommit", "GetFolder ${folderPath}") } - def folderInfo = readJSON file: "${folderPath}/.codecommit" - folderInfo.files.each { file -> - PullFilesFromGit("${folderPath}/${filename}", branchName, false, repositoryName) - } - palRm("${folderPath}/.codecommit") - - } else { - - def errorFile = "${folderPath}/error.txt" - palMkdir(folderPath) - retry(3) { - try { - if(env.IS_UNIX) { - sh label: "Downloading ${filenamePath}", - script: "aws codecommit get-file --repository-name ${repositoryName} --commit-specifier ${branchName} --file-path ${filenamePath} --query fileContent --output text 2>${errorFile} > ${filenamePath}_encoded" - sh label: 'Decoding', - script: "base64 --decode ${filenamePath}_encoded > ${filenamePath}" - } else { - errorFile = errorFile.replace('/','\\') - win_filenamePath = filenamePath.replace('/', '\\') - bat label: "Downloading ${win_filenamePath}", - script: "aws codecommit get-file --repository-name ${repositoryName} --commit-specifier ${branchName} --file-path ${filenamePath} --query fileContent --output text 2>${errorFile} > ${win_filenamePath}_encoded" - bat label: 'Decoding', - script: "certutil -decode ${win_filenamePath}_encoded ${win_filenamePath}" - } - palRm("${filenamePath}_encoded") - } catch (Exception ex) { - def error = '' - if(fileExists(errorFile)) { - error = readFile errorFile - } - if (!error || !(!failIfNotFound && error.contains('FileDoesNotExistException'))) { - palRm("${errorFile} ${filenamePath}.encoded ${filenamePath}") - throw new Exception("Could not get file: ${filenamePath}, ex: ${ex}, stderr: ${error}") - } - } - palRm(errorFile) - } - } -} - def SetLfsCredentials(cmd, lbl = '') { if (env.IS_UNIX) { sh label: lbl, script: cmd - } else { + } else { bat label: lbl, script: cmd } @@ -325,19 +214,17 @@ def CheckoutBootstrapScripts(String branchName) { } def CheckoutRepo(boolean disableSubmodules = false) { - dir(ENGINE_REPOSITORY_NAME) { - palSh('git lfs uninstall', 'Git LFS Uninstall') // Prevent git from pulling lfs objects during checkout + palSh('git lfs uninstall', 'Git LFS Uninstall') // Prevent git from pulling lfs objects during checkout - if(fileExists('.git')) { - // If the repository after checkout is locked, likely we took a snapshot while git was running, - // to leave the repo in a usable state, garbagecollect. This also helps in situations where - def indexLockFile = '.git/index.lock' - if(fileExists(indexLockFile)) { - palSh('git gc', 'Git GarbageCollect') - } - if(fileExists(indexLockFile)) { // if it is still there, remove it - palRm(indexLockFile) - } + if(fileExists('.git')) { + // If the repository after checkout is locked, likely we took a snapshot while git was running, + // to leave the repo in a usable state, garbagecollect. This also helps in situations where + def indexLockFile = '.git/index.lock' + if(fileExists(indexLockFile)) { + palSh('git gc', 'Git GarbageCollect') + } + if(fileExists(indexLockFile)) { // if it is still there, remove it + palRm(indexLockFile) } } @@ -348,54 +235,29 @@ def CheckoutRepo(boolean disableSubmodules = false) { sleep random.nextInt(60 * retryAttempt) // Stagger checkouts to prevent HTTP 429 (Too Many Requests) response from CodeCommit } retryAttempt = retryAttempt + 1 - if(params.PULL_REQUEST_ID) { - // This is a pull request build. Perform merge with destination branch before building. - dir(ENGINE_REPOSITORY_NAME) { - checkout scm: [ - $class: 'GitSCM', - branches: scm.branches, - extensions: [ - [$class: 'PreBuildMerge', options: [mergeRemote: 'origin', mergeTarget: params.DESTINATION_BRANCH]], - [$class: 'SubmoduleOption', disableSubmodules: disableSubmodules, recursiveSubmodules: true], - [$class: 'CheckoutOption', timeout: 60] - ], - userRemoteConfigs: scm.userRemoteConfigs - ] - } - } else { - dir(ENGINE_REPOSITORY_NAME) { - checkout scm: [ - $class: 'GitSCM', - branches: scm.branches, - extensions: [ - [$class: 'SubmoduleOption', disableSubmodules: disableSubmodules, recursiveSubmodules: true], - [$class: 'CheckoutOption', timeout: 60] - ], - userRemoteConfigs: scm.userRemoteConfigs - ] - } - } + checkout scm: [ + $class: 'GitSCM', + branches: scm.branches, + extensions: [ + [$class: 'SubmoduleOption', disableSubmodules: disableSubmodules, recursiveSubmodules: true], + [$class: 'CheckoutOption', timeout: 60] + ], + userRemoteConfigs: scm.userRemoteConfigs + ] } - // Add folder where we will store the 3rdParty downloads and packages - if(!fileExists('3rdParty')) { - palMkdir('3rdParty') - } - - dir(ENGINE_REPOSITORY_NAME) { - // Run lfs in a separate step. Jenkins is unable to load the credentials for the custom LFS endpoint - withCredentials([usernamePassword(credentialsId: "${env.GITHUB_USER}", passwordVariable: 'accesstoken', usernameVariable: 'username')]) { - SetLfsCredentials("git config -f .lfsconfig lfs.url https://${username}:${accesstoken}@${env.LFS_URL}", 'Set credentials') - } - palSh('git lfs install', 'Git LFS Install') - palSh('git lfs pull', 'Git LFS Pull') - - // CHANGE_ID is used by some scripts to identify uniquely the current change (usually metric jobs) - palSh('git rev-parse HEAD > commitid', 'Getting commit id') - env.CHANGE_ID = readFile file: 'commitid' - env.CHANGE_ID = env.CHANGE_ID.trim() - palRm('commitid') + // Run lfs in a separate step. Jenkins is unable to load the credentials for the custom LFS endpoint + withCredentials([usernamePassword(credentialsId: "${env.GITHUB_USER}", passwordVariable: 'accesstoken', usernameVariable: 'username')]) { + SetLfsCredentials("git config -f .lfsconfig lfs.url https://${username}:${accesstoken}@${env.LFS_URL}", 'Set credentials') } + palSh('git lfs install', 'Git LFS Install') + palSh('git lfs pull', 'Git LFS Pull') + + // CHANGE_ID is used by some scripts to identify uniquely the current change (usually metric jobs) + palSh('git rev-parse HEAD > commitid', 'Getting commit id') + env.CHANGE_ID = readFile file: 'commitid' + env.CHANGE_ID = env.CHANGE_ID.trim() + palRm('commitid') } def PreBuildCommonSteps(Map pipelineConfig, String projectName, String pipeline, String branchName, String platform, String buildType, String workspace, boolean mount = true, boolean disableSubmodules = false) { @@ -419,7 +281,7 @@ def PreBuildCommonSteps(Map pipelineConfig, String projectName, String pipeline, sh label: 'Setting volume\'s ownership', script: """ if sudo test ! -d "${workspace}"; then - sudo mkdir -p ${workspace} + sudo mkdir -p ${workspace} cd ${workspace}/.. sudo chown -R lybuilder:root . fi @@ -436,28 +298,31 @@ def PreBuildCommonSteps(Map pipelineConfig, String projectName, String pipeline, } dir(workspace) { - + // Add folder where we will store the 3rdParty downloads and packages + if(!fileExists('3rdParty')) { + palMkdir('3rdParty') + } + } + dir("${workspace}/${ENGINE_REPOSITORY_NAME}") { CheckoutRepo(disableSubmodules) // Get python - dir(ENGINE_REPOSITORY_NAME) { - if(env.IS_UNIX) { - sh label: 'Getting python', - script: 'python/get_python.sh' - } else { - bat label: 'Getting python', - script: 'python/get_python.bat' - } + if(env.IS_UNIX) { + sh label: 'Getting python', + script: 'python/get_python.sh' + } else { + bat label: 'Getting python', + script: 'python/get_python.bat' + } - if(env.CLEAN_OUTPUT_DIRECTORY.toBoolean() || env.CLEAN_ASSETS.toBoolean()) { - def command = "${pipelineConfig.BUILD_ENTRY_POINT} --platform ${platform} --type clean" - if (env.IS_UNIX) { - sh label: "Running ${platform} clean", - script: "${pipelineConfig.PYTHON_DIR}/python.sh -u ${command}" - } else { - bat label: "Running ${platform} clean", - script: "${pipelineConfig.PYTHON_DIR}/python.cmd -u ${command}".replace('/','\\') - } + if(env.CLEAN_OUTPUT_DIRECTORY.toBoolean() || env.CLEAN_ASSETS.toBoolean()) { + def command = "${pipelineConfig.BUILD_ENTRY_POINT} --platform ${platform} --type clean" + if (env.IS_UNIX) { + sh label: "Running ${platform} clean", + script: "${pipelineConfig.PYTHON_DIR}/python.sh -u ${command}" + } else { + bat label: "Running ${platform} clean", + script: "${pipelineConfig.PYTHON_DIR}/python.cmd -u ${command}".replace('/','\\') } } } @@ -488,8 +353,6 @@ def TestMetrics(Map options, String workspace, String branchName, String repoNam ] withCredentials([usernamePassword(credentialsId: "${env.SERVICE_USER}", passwordVariable: 'apitoken', usernameVariable: 'username')]) { def command = "${options.PYTHON_DIR}/python.cmd -u mars/scripts/python/ctest_test_metric_scraper.py -e jenkins.creds.user ${username} -e jenkins.creds.pass ${apitoken} ${cmakeBuildDir} ${branchName} %BUILD_NUMBER% AR ${configuration} ${repoName} " - if (params.DESTINATION_BRANCH) - command += "--destination-branch ${params.DESTINATION_BRANCH} " bat label: "Publishing ${buildJobName} Test Metrics", script: command } @@ -500,14 +363,6 @@ def TestMetrics(Map options, String workspace, String branchName, String repoNam def PostBuildCommonSteps(String workspace, boolean mount = true) { echo 'Starting post-build common steps...' - if(params.PULL_REQUEST_ID) { - dir("${workspace}/${ENGINE_REPOSITORY_NAME}") { - if(fileExists('.git')) { - palSh('git reset --hard HEAD', 'Discard PR merge, git reset') - } - } - } - if (mount) { def pythonCmd = '' if(env.IS_UNIX) pythonCmd = 'sudo -E python -u ' @@ -571,7 +426,6 @@ try { withEnv(envVarList) { timestamps { (projectName, pipelineName) = GetRunningPipelineName(env.JOB_NAME) // env.JOB_NAME is the name of the job given by Jenkins - scmType = GetSCMType() if(env.BRANCH_NAME) { branchName = env.BRANCH_NAME @@ -583,13 +437,11 @@ try { echo "Running \"${pipelineName}\" for \"${branchName}\"..." - if (scmType == 'github') { - CheckoutBootstrapScripts(branchName) - } + CheckoutBootstrapScripts(branchName) // Load configs - pipelineConfig = LoadPipelineConfig(pipelineName, branchName, scmType) - + pipelineConfig = LoadPipelineConfig(pipelineName, branchName) + // Add each platform as a parameter that the user can disable if needed pipelineConfig.platforms.each { platform -> pipelineParameters.add(booleanParam(defaultValue: true, description: '', name: platform.key)) @@ -598,18 +450,15 @@ try { properties(pipelineProperties) // Stash the INCREMENTAL_BUILD_SCRIPT_PATH since all nodes will use it - if (scmType == 'codecommit') { - PullFilesFromGit(INCREMENTAL_BUILD_SCRIPT_PATH, branchName, true, ENGINE_REPOSITORY_NAME) - } stash name: 'incremental_build_script', includes: INCREMENTAL_BUILD_SCRIPT_PATH - } + } } } } if(env.BUILD_NUMBER == '1') { - // Exit pipeline early on the intial build. This allows Jenkins to load the pipeline for the branch and enables users + // Exit pipeline early on the intial build. This allows Jenkins to load the pipeline for the branch and enables users // to select build parameters on their first actual build. See https://issues.jenkins.io/browse/JENKINS-41929 currentBuild.result = 'SUCCESS' return @@ -626,16 +475,16 @@ try { envVars['JOB_NAME'] = "${branchName}_${platform.key}_${build_job.key}" // backwards compatibility, some scripts rely on this def nodeLabel = envVars['NODE_LABEL'] - buildConfigs["${platform.key} [${build_job.key}]"] = { + buildConfigs["${platform.key} [${build_job.key}]"] = { node("${nodeLabel}") { if(isUnix()) { // Has to happen inside a node envVars['IS_UNIX'] = 1 - } + } withEnv(GetEnvStringList(envVars)) { timeout(time: envVars['TIMEOUT'], unit: 'MINUTES', activity: true) { try { def build_job_name = build_job.key - + CreateSetupStage(pipelineConfig, 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 @@ -646,7 +495,7 @@ try { } else { CreateBuildStage(pipelineConfig, platform.key, build_job.key, envVars).call() } - + if (env.MARS_REPO && platform.key == 'Windows' && build_job_name.startsWith('test')) { def output_directory = platform.value.build_types[build_job_name].PARAMETERS.OUTPUT_DIRECTORY def configuration = platform.value.build_types[build_job_name].PARAMETERS.CONFIGURATION @@ -692,9 +541,9 @@ finally { try { if(env.SNS_TOPIC) { snsPublish( - topicArn: env.SNS_TOPIC, - subject:'Build Result', - message:"${currentBuild.currentResult}:${params.REPOSITORY_NAME}:${params.SOURCE_BRANCH}:${params.SOURCE_COMMIT}:${params.DESTINATION_COMMIT}:${params.PULL_REQUEST_ID}:${BUILD_URL}:${env.RECREATE_VOLUME}:${env.CLEAN_OUTPUT_DIRECTORY}:${env.CLEAN_ASSETS}" + topicArn: env.SNS_TOPIC, + subject:'Build Result', + message:"${currentBuild.currentResult}:${BUILD_URL}:${env.RECREATE_VOLUME}:${env.CLEAN_OUTPUT_DIRECTORY}:${env.CLEAN_ASSETS}" ) } step([ From 84c42018c711beaa172d43fed6861f256e5d988c Mon Sep 17 00:00:00 2001 From: shiranj Date: Fri, 9 Apr 2021 17:37:25 -0700 Subject: [PATCH 7/8] Revert AutomatedReview/Jenkinsfile --- AutomatedReview/Jenkinsfile | 132 +++++++++++++++++++----------------- 1 file changed, 70 insertions(+), 62 deletions(-) diff --git a/AutomatedReview/Jenkinsfile b/AutomatedReview/Jenkinsfile index 348152d2e7..8cf7a62726 100644 --- a/AutomatedReview/Jenkinsfile +++ b/AutomatedReview/Jenkinsfile @@ -16,7 +16,7 @@ INCREMENTAL_BUILD_SCRIPT_PATH = 'scripts/build/bootstrap/incremental_build_util. EMPTY_JSON = readJSON text: '{}' -ENGINE_REPOSITORY_NAME = env.DEFAULT_REPOSITORY_NAME +ENGINE_REPOSITORY_NAME = 'o3de' def pipelineProperties = [] @@ -325,17 +325,19 @@ def CheckoutBootstrapScripts(String branchName) { } def CheckoutRepo(boolean disableSubmodules = false) { - palSh('git lfs uninstall', 'Git LFS Uninstall') // Prevent git from pulling lfs objects during checkout + dir(ENGINE_REPOSITORY_NAME) { + palSh('git lfs uninstall', 'Git LFS Uninstall') // Prevent git from pulling lfs objects during checkout - if(fileExists('.git')) { - // If the repository after checkout is locked, likely we took a snapshot while git was running, - // to leave the repo in a usable state, garbagecollect. This also helps in situations where - def indexLockFile = '.git/index.lock' - if(fileExists(indexLockFile)) { - palSh('git gc', 'Git GarbageCollect') - } - if(fileExists(indexLockFile)) { // if it is still there, remove it - palRm(indexLockFile) + if(fileExists('.git')) { + // If the repository after checkout is locked, likely we took a snapshot while git was running, + // to leave the repo in a usable state, garbagecollect. This also helps in situations where + def indexLockFile = '.git/index.lock' + if(fileExists(indexLockFile)) { + palSh('git gc', 'Git GarbageCollect') + } + if(fileExists(indexLockFile)) { // if it is still there, remove it + palRm(indexLockFile) + } } } @@ -348,41 +350,52 @@ def CheckoutRepo(boolean disableSubmodules = false) { retryAttempt = retryAttempt + 1 if(params.PULL_REQUEST_ID) { // This is a pull request build. Perform merge with destination branch before building. - checkout scm: [ - $class: 'GitSCM', - branches: scm.branches, - extensions: [ - [$class: 'PreBuildMerge', options: [mergeRemote: 'origin', mergeTarget: params.DESTINATION_BRANCH]], - [$class: 'SubmoduleOption', disableSubmodules: disableSubmodules, recursiveSubmodules: true], - [$class: 'CheckoutOption', timeout: 60] - ], - userRemoteConfigs: scm.userRemoteConfigs - ] + dir(ENGINE_REPOSITORY_NAME) { + checkout scm: [ + $class: 'GitSCM', + branches: scm.branches, + extensions: [ + [$class: 'PreBuildMerge', options: [mergeRemote: 'origin', mergeTarget: params.DESTINATION_BRANCH]], + [$class: 'SubmoduleOption', disableSubmodules: disableSubmodules, recursiveSubmodules: true], + [$class: 'CheckoutOption', timeout: 60] + ], + userRemoteConfigs: scm.userRemoteConfigs + ] + } } else { - checkout scm: [ - $class: 'GitSCM', - branches: scm.branches, - extensions: [ - [$class: 'SubmoduleOption', disableSubmodules: disableSubmodules, recursiveSubmodules: true], - [$class: 'CheckoutOption', timeout: 60] - ], - userRemoteConfigs: scm.userRemoteConfigs - ] + dir(ENGINE_REPOSITORY_NAME) { + checkout scm: [ + $class: 'GitSCM', + branches: scm.branches, + extensions: [ + [$class: 'SubmoduleOption', disableSubmodules: disableSubmodules, recursiveSubmodules: true], + [$class: 'CheckoutOption', timeout: 60] + ], + userRemoteConfigs: scm.userRemoteConfigs + ] + } } } - // Run lfs in a separate step. Jenkins is unable to load the credentials for the custom LFS endpoint - withCredentials([usernamePassword(credentialsId: "${env.GITHUB_USER}", passwordVariable: 'accesstoken', usernameVariable: 'username')]) { - SetLfsCredentials("git config -f .lfsconfig lfs.url https://${username}:${accesstoken}@${env.LFS_URL}", 'Set credentials') + // Add folder where we will store the 3rdParty downloads and packages + if(!fileExists('3rdParty')) { + palMkdir('3rdParty') } - palSh('git lfs install', 'Git LFS Install') - palSh('git lfs pull', 'Git LFS Pull') - // CHANGE_ID is used by some scripts to identify uniquely the current change (usually metric jobs) - palSh('git rev-parse HEAD > commitid', 'Getting commit id') - env.CHANGE_ID = readFile file: 'commitid' - env.CHANGE_ID = env.CHANGE_ID.trim() - palRm('commitid') + dir(ENGINE_REPOSITORY_NAME) { + // Run lfs in a separate step. Jenkins is unable to load the credentials for the custom LFS endpoint + withCredentials([usernamePassword(credentialsId: "${env.GITHUB_USER}", passwordVariable: 'accesstoken', usernameVariable: 'username')]) { + SetLfsCredentials("git config -f .lfsconfig lfs.url https://${username}:${accesstoken}@${env.LFS_URL}", 'Set credentials') + } + palSh('git lfs install', 'Git LFS Install') + palSh('git lfs pull', 'Git LFS Pull') + + // CHANGE_ID is used by some scripts to identify uniquely the current change (usually metric jobs) + palSh('git rev-parse HEAD > commitid', 'Getting commit id') + env.CHANGE_ID = readFile file: 'commitid' + env.CHANGE_ID = env.CHANGE_ID.trim() + palRm('commitid') + } } def PreBuildCommonSteps(Map pipelineConfig, String projectName, String pipeline, String branchName, String platform, String buildType, String workspace, boolean mount = true, boolean disableSubmodules = false) { @@ -423,33 +436,28 @@ def PreBuildCommonSteps(Map pipelineConfig, String projectName, String pipeline, } dir(workspace) { - // Add folder where we will store the 3rdParty downloads and packages - if(!fileExists('3rdParty')) { - palMkdir('3rdParty') - } - } - - dir("${workspace}/${ENGINE_REPOSITORY_NAME}") { CheckoutRepo(disableSubmodules) // Get python - if(env.IS_UNIX) { - sh label: 'Getting python', - script: 'python/get_python.sh' - } else { - bat label: 'Getting python', - script: 'python/get_python.bat' - } - - if(env.CLEAN_OUTPUT_DIRECTORY.toBoolean() || env.CLEAN_ASSETS.toBoolean()) { - def command = "${pipelineConfig.BUILD_ENTRY_POINT} --platform ${platform} --type clean" - if (env.IS_UNIX) { - sh label: "Running ${platform} clean", - script: "${pipelineConfig.PYTHON_DIR}/python.sh -u ${command}" + dir(ENGINE_REPOSITORY_NAME) { + if(env.IS_UNIX) { + sh label: 'Getting python', + script: 'python/get_python.sh' } else { - bat label: "Running ${platform} clean", - script: "${pipelineConfig.PYTHON_DIR}/python.cmd -u ${command}".replace('/','\\') + bat label: 'Getting python', + script: 'python/get_python.bat' + } + + if(env.CLEAN_OUTPUT_DIRECTORY.toBoolean() || env.CLEAN_ASSETS.toBoolean()) { + def command = "${pipelineConfig.BUILD_ENTRY_POINT} --platform ${platform} --type clean" + if (env.IS_UNIX) { + sh label: "Running ${platform} clean", + script: "${pipelineConfig.PYTHON_DIR}/python.sh -u ${command}" + } else { + bat label: "Running ${platform} clean", + script: "${pipelineConfig.PYTHON_DIR}/python.cmd -u ${command}".replace('/','\\') + } } } } From 37379d9d7601e21c2d20d17d81b3a7295dd8bfed Mon Sep 17 00:00:00 2001 From: Shirang Jia Date: Fri, 9 Apr 2021 18:17:03 -0700 Subject: [PATCH 8/8] Fix indentation in Jenkinsfile --- scripts/build/Jenkins/Jenkinsfile | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/build/Jenkins/Jenkinsfile b/scripts/build/Jenkins/Jenkinsfile index 015dc7cd14..d313cc40ea 100644 --- a/scripts/build/Jenkins/Jenkinsfile +++ b/scripts/build/Jenkins/Jenkinsfile @@ -308,8 +308,8 @@ def PreBuildCommonSteps(Map pipelineConfig, String projectName, String pipeline, // Get python if(env.IS_UNIX) { - sh label: 'Getting python', - script: 'python/get_python.sh' + sh label: 'Getting python', + script: 'python/get_python.sh' } else { bat label: 'Getting python', script: 'python/get_python.bat' @@ -335,7 +335,7 @@ def Build(Map options, String platform, String type, String workspace) { sh label: "Running ${platform} ${type}", script: "${options.PYTHON_DIR}/python.sh -u ${command}" } else { - bat label: "Running ${platform} ${type}", + bat label: "Running ${platform} ${type}", script: "${options.PYTHON_DIR}/python.cmd -u ${command}".replace('/','\\') } } @@ -354,7 +354,7 @@ def TestMetrics(Map options, String workspace, String branchName, String repoNam withCredentials([usernamePassword(credentialsId: "${env.SERVICE_USER}", passwordVariable: 'apitoken', usernameVariable: 'username')]) { def command = "${options.PYTHON_DIR}/python.cmd -u mars/scripts/python/ctest_test_metric_scraper.py -e jenkins.creds.user ${username} -e jenkins.creds.pass ${apitoken} ${cmakeBuildDir} ${branchName} %BUILD_NUMBER% AR ${configuration} ${repoName} " bat label: "Publishing ${buildJobName} Test Metrics", - script: command + script: command } } }