From 26aa7495a228670e31ef85201625b7633318fa39 Mon Sep 17 00:00:00 2001 From: Guthrie Adams Date: Tue, 12 Oct 2021 11:26:14 -0500 Subject: [PATCH] Changed preview renderer states to use construction and destruction instead of start and stop functions to make sure everything is shut down cleanly Signed-off-by: Guthrie Adams --- .../PreviewRenderer/PreviewRenderer.h | 14 +------ .../PreviewRenderer/PreviewRendererState.h | 6 --- .../PreviewRenderer/PreviewRenderer.cpp | 42 +++++-------------- .../PreviewRendererCaptureState.cpp | 21 +++------- .../PreviewRendererCaptureState.h | 6 +-- .../PreviewRendererIdleState.cpp | 6 +-- .../PreviewRendererIdleState.h | 4 +- .../PreviewRendererLoadState.cpp | 17 +++----- .../PreviewRendererLoadState.h | 6 +-- 9 files changed, 29 insertions(+), 93 deletions(-) diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/PreviewRenderer/PreviewRenderer.h b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/PreviewRenderer/PreviewRenderer.h index d4fdd3ca1e..dc03ed6715 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/PreviewRenderer/PreviewRenderer.h +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/PreviewRenderer/PreviewRenderer.h @@ -47,17 +47,6 @@ namespace AtomToolsFramework AZ::RPI::ViewPtr GetView() const; AZ::Uuid GetEntityContextId() const; - enum class State : AZ::s8 - { - None, - IdleState, - LoadState, - CaptureState - }; - - void SetState(State state); - State GetState() const; - void ProcessCaptureRequests(); void CancelCaptureRequest(); void CompleteCaptureRequest(); @@ -91,7 +80,6 @@ namespace AtomToolsFramework AZStd::queue m_captureRequestQueue; CaptureRequest m_currentCaptureRequest; - AZStd::unordered_map> m_states; - State m_currentState = PreviewRenderer::State::None; + AZStd::unique_ptr m_state; }; } // namespace AtomToolsFramework diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/PreviewRenderer/PreviewRendererState.h b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/PreviewRenderer/PreviewRendererState.h index 264c37a122..bf68795974 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/PreviewRenderer/PreviewRendererState.h +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/PreviewRenderer/PreviewRendererState.h @@ -23,12 +23,6 @@ namespace AtomToolsFramework virtual ~PreviewRendererState() = default; - //! Start is called when state begins execution - virtual void Start() = 0; - - //! Stop is called when state ends execution - virtual void Stop() = 0; - protected: PreviewRenderer* m_renderer = {}; }; diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRenderer.cpp b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRenderer.cpp index 69a8d357c9..3e4fde4e08 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRenderer.cpp +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRenderer.cpp @@ -79,17 +79,15 @@ namespace AtomToolsFramework m_view->SetViewToClipMatrix(viewToClipMatrix); m_renderPipeline->SetDefaultView(m_view); - m_states[PreviewRenderer::State::IdleState] = AZStd::make_shared(this); - m_states[PreviewRenderer::State::LoadState] = AZStd::make_shared(this); - m_states[PreviewRenderer::State::CaptureState] = AZStd::make_shared(this); - SetState(PreviewRenderer::State::IdleState); + m_state.reset(new PreviewRendererIdleState(this)); } PreviewRenderer::~PreviewRenderer() { PreviewerFeatureProcessorProviderBus::Handler::BusDisconnect(); - SetState(PreviewRenderer::State::None); + m_state.reset(); + m_currentCaptureRequest = {}; m_captureRequestQueue = {}; @@ -120,28 +118,6 @@ namespace AtomToolsFramework return m_entityContext->GetContextId(); } - void PreviewRenderer::SetState(State state) - { - auto stepItr = m_states.find(m_currentState); - if (stepItr != m_states.end()) - { - stepItr->second->Stop(); - } - - m_currentState = state; - - stepItr = m_states.find(m_currentState); - if (stepItr != m_states.end()) - { - stepItr->second->Start(); - } - } - - PreviewRenderer::State PreviewRenderer::GetState() const - { - return m_currentState; - } - void PreviewRenderer::ProcessCaptureRequests() { if (!m_captureRequestQueue.empty()) @@ -150,19 +126,22 @@ namespace AtomToolsFramework m_currentCaptureRequest = m_captureRequestQueue.front(); m_captureRequestQueue.pop(); - SetState(PreviewRenderer::State::LoadState); + m_state.reset(); + m_state.reset(new PreviewRendererLoadState(this)); } } void PreviewRenderer::CancelCaptureRequest() { m_currentCaptureRequest.m_captureFailedCallback(); - SetState(PreviewRenderer::State::IdleState); + m_state.reset(); + m_state.reset(new PreviewRendererIdleState(this)); } void PreviewRenderer::CompleteCaptureRequest() { - SetState(PreviewRenderer::State::IdleState); + m_state.reset(); + m_state.reset(new PreviewRendererIdleState(this)); } void PreviewRenderer::LoadContent() @@ -174,7 +153,8 @@ namespace AtomToolsFramework { if (m_currentCaptureRequest.m_content->IsReady()) { - SetState(PreviewRenderer::State::CaptureState); + m_state.reset(); + m_state.reset(new PreviewRendererCaptureState(this)); return; } diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRendererCaptureState.cpp b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRendererCaptureState.cpp index 9cf228b707..9a807b7d00 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRendererCaptureState.cpp +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRendererCaptureState.cpp @@ -14,32 +14,23 @@ namespace AtomToolsFramework PreviewRendererCaptureState::PreviewRendererCaptureState(PreviewRenderer* renderer) : PreviewRendererState(renderer) { - } - - void PreviewRendererCaptureState::Start() - { - m_ticksToCapture = 1; m_renderer->PoseContent(); AZ::TickBus::Handler::BusConnect(); } - void PreviewRendererCaptureState::Stop() + PreviewRendererCaptureState::~PreviewRendererCaptureState() { - m_renderer->EndCapture(); - AZ::TickBus::Handler::BusDisconnect(); AZ::Render::FrameCaptureNotificationBus::Handler::BusDisconnect(); + AZ::TickBus::Handler::BusDisconnect(); + m_renderer->EndCapture(); } void PreviewRendererCaptureState::OnTick([[maybe_unused]] float deltaTime, [[maybe_unused]] AZ::ScriptTimePoint time) { - if (m_ticksToCapture-- <= 0) + if ((m_ticksToCapture-- <= 0) && m_renderer->StartCapture()) { - // Reset the capture flag if the capture request was successful. Otherwise try capture it again next tick. - if (m_renderer->StartCapture()) - { - AZ::Render::FrameCaptureNotificationBus::Handler::BusConnect(); - AZ::TickBus::Handler::BusDisconnect(); - } + AZ::Render::FrameCaptureNotificationBus::Handler::BusConnect(); + AZ::TickBus::Handler::BusDisconnect(); } } diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRendererCaptureState.h b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRendererCaptureState.h index 190cb919df..e8c6357445 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRendererCaptureState.h +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRendererCaptureState.h @@ -22,9 +22,7 @@ namespace AtomToolsFramework { public: PreviewRendererCaptureState(PreviewRenderer* renderer); - - void Start() override; - void Stop() override; + ~PreviewRendererCaptureState(); private: //! AZ::TickBus::Handler interface overrides... @@ -34,6 +32,6 @@ namespace AtomToolsFramework void OnCaptureFinished(AZ::Render::FrameCaptureResult result, const AZStd::string& info) override; //! This is necessary to suspend capture to allow a frame for Material and Mesh components to assign materials - int m_ticksToCapture = 0; + int m_ticksToCapture = 1; }; } // namespace AtomToolsFramework diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRendererIdleState.cpp b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRendererIdleState.cpp index 800aa03113..c440bafb73 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRendererIdleState.cpp +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRendererIdleState.cpp @@ -13,15 +13,11 @@ namespace AtomToolsFramework { PreviewRendererIdleState::PreviewRendererIdleState(PreviewRenderer* renderer) : PreviewRendererState(renderer) - { - } - - void PreviewRendererIdleState::Start() { AZ::TickBus::Handler::BusConnect(); } - void PreviewRendererIdleState::Stop() + PreviewRendererIdleState::~PreviewRendererIdleState() { AZ::TickBus::Handler::BusDisconnect(); } diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRendererIdleState.h b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRendererIdleState.h index 9e5380e734..f024bd8e30 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRendererIdleState.h +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRendererIdleState.h @@ -20,9 +20,7 @@ namespace AtomToolsFramework { public: PreviewRendererIdleState(PreviewRenderer* renderer); - - void Start() override; - void Stop() override; + ~PreviewRendererIdleState(); private: //! AZ::TickBus::Handler interface overrides... diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRendererLoadState.cpp b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRendererLoadState.cpp index bb858989f7..7e34592095 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRendererLoadState.cpp +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRendererLoadState.cpp @@ -13,31 +13,24 @@ namespace AtomToolsFramework { PreviewRendererLoadState::PreviewRendererLoadState(PreviewRenderer* renderer) : PreviewRendererState(renderer) - { - } - - void PreviewRendererLoadState::Start() { m_renderer->LoadContent(); - m_timeRemainingS = TimeOutS; AZ::TickBus::Handler::BusConnect(); } - void PreviewRendererLoadState::Stop() + PreviewRendererLoadState::~PreviewRendererLoadState() { AZ::TickBus::Handler::BusDisconnect(); } void PreviewRendererLoadState::OnTick(float deltaTime, [[maybe_unused]] AZ::ScriptTimePoint time) { - m_timeRemainingS -= deltaTime; - if (m_timeRemainingS > 0.0f) - { - m_renderer->UpdateLoadContent(); - } - else + if ((m_timeRemainingS += deltaTime) > TimeOutS) { m_renderer->CancelLoadContent(); + return; } + + m_renderer->UpdateLoadContent(); } } // namespace AtomToolsFramework diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRendererLoadState.h b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRendererLoadState.h index 623d6cbdfc..702a01e862 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRendererLoadState.h +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/PreviewRenderer/PreviewRendererLoadState.h @@ -20,15 +20,13 @@ namespace AtomToolsFramework { public: PreviewRendererLoadState(PreviewRenderer* renderer); - - void Start() override; - void Stop() override; + ~PreviewRendererLoadState(); private: //! AZ::TickBus::Handler interface overrides... void OnTick(float deltaTime, AZ::ScriptTimePoint time) override; static constexpr float TimeOutS = 5.0f; - float m_timeRemainingS = TimeOutS; + float m_timeRemainingS = 0.0f; }; } // namespace AtomToolsFramework