diff --git a/garnet/lib/ui/gfx/engine/default_frame_scheduler.cc b/garnet/lib/ui/gfx/engine/default_frame_scheduler.cc index 66685fdbd220142eecdacd793a553554a64473b0..de0c9ce6d7e1daaa404b3b83e1fd56ccdc62b986 100644 --- a/garnet/lib/ui/gfx/engine/default_frame_scheduler.cc +++ b/garnet/lib/ui/gfx/engine/default_frame_scheduler.cc @@ -176,6 +176,8 @@ void DefaultFrameScheduler::MaybeRenderFrame(async_dispatcher_t*, // TODO(SCN-1091): this is a very conservative approach that may result in // excessive rendering. + // TODO(SCN-1337) Remove the render_pending_ check, and pipeline frames within + // a VSYNC interval. if (currently_rendering_) { render_pending_ = true; return; @@ -189,12 +191,13 @@ void DefaultFrameScheduler::MaybeRenderFrame(async_dispatcher_t*, << "DefaultFrameScheduler: calling RenderFrame presentation_time=" << presentation_time << " frame_number=" << frame_number_; } - TRACE_INSTANT("gfx", "Render start", TRACE_SCOPE_PROCESS, "Expected presentation time", presentation_time, "frame_number", frame_number_); - delegate_.session_updater->NewFrame(); + // Ratchet the Present callbacks to signal that all outstanding Present() + // calls until this point are applied to the next Scenic frame. + delegate_.session_updater->RatchetPresentCallbacks(); auto frame_timings = fxl::MakeRefCounted<FrameTimings>(this, frame_number_, presentation_time); @@ -261,7 +264,7 @@ bool DefaultFrameScheduler::ApplyScheduledSessionUpdates( } auto update_results = delegate_.session_updater->UpdateSessions( - std::move(sessions_to_update), presentation_time); + std::move(sessions_to_update), presentation_time, frame_number_); // Push updates that didn't have their fences ready back onto the queue to be // retried next frame. @@ -292,8 +295,7 @@ void DefaultFrameScheduler::OnFramePresented(const FrameTimings& timings) { timings.frame_number()); } else { if (TRACE_CATEGORY_ENABLED("gfx")) { - // Log trace data. - // TODO(SCN-400): just pass the whole Frame to a listener. + // Log trace data.. zx_duration_t target_vs_actual = timings.actual_presentation_time() - timings.target_presentation_time(); diff --git a/garnet/lib/ui/gfx/engine/engine.cc b/garnet/lib/ui/gfx/engine/engine.cc index 6e2bacff82a2bd8ba7876facdb5bd3db7ae1fef4..362992fe119929cc02aea2ac12a5dfa2fc89074b 100644 --- a/garnet/lib/ui/gfx/engine/engine.cc +++ b/garnet/lib/ui/gfx/engine/engine.cc @@ -61,7 +61,6 @@ Engine::Engine(sys::ComponentContext* component_context, session_manager_(std::move(session_manager)), frame_scheduler_(std::move(frame_scheduler)), has_vulkan_(escher_ && escher_->vk_device()), - command_context_(CreateCommandContext(/* frame number */ 0)), inspect_object_(std::move(inspect_object)), weak_factory_(this) { FXL_DCHECK(frame_scheduler_); @@ -87,7 +86,6 @@ Engine::Engine( session_manager_(std::move(session_manager)), frame_scheduler_(std::move(frame_scheduler)), has_vulkan_(escher_ && escher_->vk_device()), - command_context_(CreateCommandContext(/* frame number */ 0)), weak_factory_(this) { FXL_DCHECK(frame_scheduler_); FXL_DCHECK(display_manager_); @@ -144,17 +142,17 @@ std::optional<HardwareLayerAssignment> GetHardwareLayerAssignment( }; } -CommandContext Engine::CreateCommandContext(uint64_t frame_number_for_tracing) { - return CommandContext(has_vulkan() ? escher::BatchGpuUploader::New( - escher_, frame_number_for_tracing) - : nullptr); +CommandContext Engine::CreateCommandContext(uint64_t trace_id) { + return CommandContext(has_vulkan() + ? escher::BatchGpuUploader::New(escher_, trace_id) + : nullptr); } // Applies scheduled updates to a session. If the update fails, the session is // killed. Returns true if a new render is needed, false otherwise. SessionUpdater::UpdateResults Engine::UpdateSessions( std::unordered_set<SessionId> sessions_to_update, - zx_time_t presentation_time) { + zx_time_t presentation_time, uint64_t trace_id) { SessionUpdater::UpdateResults update_results; for (auto session_id : sessions_to_update) { auto session_handler = session_manager_->FindSessionHandler(session_id); @@ -169,8 +167,12 @@ SessionUpdater::UpdateResults Engine::UpdateSessions( auto session = session_handler->session(); + if (!command_context_) { + command_context_ = + std::make_optional<CommandContext>(CreateCommandContext(trace_id)); + } auto apply_results = session->ApplyScheduledUpdates( - &command_context_, presentation_time, needs_render_count_); + &(command_context_.value()), presentation_time, needs_render_count_); // If update fails, kill the entire client session. if (!apply_results.success) { @@ -201,7 +203,7 @@ SessionUpdater::UpdateResults Engine::UpdateSessions( return update_results; } -void Engine::NewFrame() { +void Engine::RatchetPresentCallbacks() { while (!callbacks_this_frame_.empty()) { pending_callbacks_.push(std::move(callbacks_this_frame_.front())); callbacks_this_frame_.pop(); @@ -234,8 +236,8 @@ bool Engine::RenderFrame(const FrameTimingsPtr& timings, } // Flush work to the gpu. - command_context_.Flush(); - command_context_ = CreateCommandContext(frame_number + 1); + command_context_->Flush(); + command_context_.reset(); UpdateAndDeliverMetrics(presentation_time); diff --git a/garnet/lib/ui/gfx/engine/engine.h b/garnet/lib/ui/gfx/engine/engine.h index 884a39d0d5a5ebd486a40cb96da7795b176f27c4..2d75cae98e3d7b229d803efc6c734827f60b2fef 100644 --- a/garnet/lib/ui/gfx/engine/engine.h +++ b/garnet/lib/ui/gfx/engine/engine.h @@ -132,18 +132,19 @@ class Engine : public SessionUpdater, public FrameRenderer { // killed. Returns true if a new render is needed, false otherwise. SessionUpdater::UpdateResults UpdateSessions( std::unordered_set<SessionId> sessions_to_update, - zx_time_t presentation_time) override; + zx_time_t presentation_time, uint64_t trace_id = 0) override; // |SessionUpdater| // - // Signals the start of a new frame, pushing all updates for the previous - // frame onto the pending callback queue. - void NewFrame() override; + // Signals that all present calls prior to this point are included in the + // next frame. This must be called before RenderFrame() to be able to signal + // the present callbacks that contributed to that next frame. + void RatchetPresentCallbacks() override; // |SessionUpdater| // - // Triggers the corresponding callbacks for each session that had an update in - // the last frame. + // Triggers the corresponding callbacks for each session that had an update + // since the last ratchet point. void SignalSuccessfulPresentCallbacks(PresentationInfo) override; // |FrameRenderer| @@ -168,7 +169,7 @@ class Engine : public SessionUpdater, public FrameRenderer { void InitializeInspectObjects(); // Creates a command context. - CommandContext CreateCommandContext(uint64_t frame_number_for_tracing); + CommandContext CreateCommandContext(uint64_t trace_id); // Takes care of cleanup between frames. void EndCurrentFrame(uint64_t frame_number); @@ -231,7 +232,7 @@ class Engine : public SessionUpdater, public FrameRenderer { std::queue<OnPresentedCallback> callbacks_this_frame_; std::queue<OnPresentedCallback> pending_callbacks_; - CommandContext command_context_; + std::optional<CommandContext> command_context_; inspect::Object inspect_object_; inspect::LazyStringProperty inspect_scene_dump_; diff --git a/garnet/lib/ui/gfx/engine/frame_scheduler.h b/garnet/lib/ui/gfx/engine/frame_scheduler.h index 452598c0152efca55d5cf9d3d1b88ea6b41a6ab7..2413f648d9a7d487a0cc4d772df41d15b0150cb9 100644 --- a/garnet/lib/ui/gfx/engine/frame_scheduler.h +++ b/garnet/lib/ui/gfx/engine/frame_scheduler.h @@ -5,14 +5,14 @@ #ifndef GARNET_LIB_UI_GFX_ENGINE_FRAME_SCHEDULER_H_ #define GARNET_LIB_UI_GFX_ENGINE_FRAME_SCHEDULER_H_ -#include <unordered_set> - -#include "garnet/lib/ui/gfx/id.h" - #include <fuchsia/ui/scenic/cpp/fidl.h> #include <lib/zx/time.h> #include <src/lib/fxl/memory/weak_ptr.h> +#include <unordered_set> + +#include "garnet/lib/ui/gfx/id.h" + namespace scenic_impl { namespace gfx { @@ -44,15 +44,17 @@ class SessionUpdater { // false otherwise. virtual UpdateResults UpdateSessions( std::unordered_set<SessionId> sessions_to_update, - zx_time_t presentation_time) = 0; + zx_time_t presentation_time, uint64_t trace_id) = 0; - // Signals the start of a new frame. - virtual void NewFrame() = 0; + // Creates a ratchet point for the updater. All present calls that were + // updated before this point will be signaled with the next call to + // SignalSuccessfulPresentCallbacks(). + virtual void RatchetPresentCallbacks() = 0; - // Signal that all updates before the current frame have been presented. The - // signaled callbacks are every successful present between the last time + // Signal that all updates before the last ratchet point have been presented. + // The signaled callbacks are every successful present between the last time // SignalSuccessfulPresentCallbacks was called and the most recent call to - // NewFrame(). + // RatchetPresentCallbacks(). virtual void SignalSuccessfulPresentCallbacks( fuchsia::images::PresentationInfo) = 0; }; diff --git a/garnet/lib/ui/gfx/tests/default_frame_scheduler_unittest.cc b/garnet/lib/ui/gfx/tests/default_frame_scheduler_unittest.cc index af22914fb9556edf91c33a12749a32053d21d654..e0cca85192e76dce0822e653fe9515a191276f5c 100644 --- a/garnet/lib/ui/gfx/tests/default_frame_scheduler_unittest.cc +++ b/garnet/lib/ui/gfx/tests/default_frame_scheduler_unittest.cc @@ -3,6 +3,7 @@ // found in the LICENSE file. #include <lib/gtest/test_loop_fixture.h> + #include "garnet/lib/ui/gfx/tests/frame_scheduler_test.h" namespace scenic_impl { @@ -63,7 +64,9 @@ TEST_F(FrameSchedulerTest, SinglePresent_ShouldGetSingleRenderCall) { auto scheduler = CreateDefaultFrameScheduler(); EXPECT_EQ(mock_updater_->update_sessions_call_count(), 0u); + EXPECT_EQ(mock_updater_->ratchet_present_call_count(), 0u); EXPECT_EQ(mock_renderer_->render_frame_call_count(), 0u); + EXPECT_EQ(mock_updater_->signal_previous_frames_presented_call_count(), 0u); scheduler->ScheduleUpdateForSession(Now().get(), /* session id */ 1); @@ -75,12 +78,15 @@ TEST_F(FrameSchedulerTest, SinglePresent_ShouldGetSingleRenderCall) { // Present should have been scheduled and handled. EXPECT_EQ(mock_updater_->update_sessions_call_count(), 1u); + EXPECT_EQ(mock_updater_->ratchet_present_call_count(), 1u); EXPECT_EQ(mock_renderer_->render_frame_call_count(), 1u); + EXPECT_EQ(mock_updater_->signal_previous_frames_presented_call_count(), 0u); // End the pending frame. EXPECT_EQ(mock_renderer_->pending_frames(), 1u); mock_renderer_->EndFrame(/* frame index */ 0); EXPECT_EQ(mock_renderer_->pending_frames(), 0u); + EXPECT_EQ(mock_updater_->signal_previous_frames_presented_call_count(), 1u); // Wait for a very long time. RunLoopFor(zx::sec(10)); @@ -88,6 +94,7 @@ TEST_F(FrameSchedulerTest, SinglePresent_ShouldGetSingleRenderCall) { // No further render calls should have been made. EXPECT_EQ(mock_updater_->update_sessions_call_count(), 1u); EXPECT_EQ(mock_renderer_->render_frame_call_count(), 1u); + EXPECT_EQ(mock_updater_->signal_previous_frames_presented_call_count(), 1u); } TEST_F(FrameSchedulerTest, PresentsForTheSameFrame_ShouldGetSingleRenderCall) { @@ -171,7 +178,9 @@ TEST_F(FrameSchedulerTest, auto scheduler = CreateDefaultFrameScheduler(); EXPECT_EQ(mock_updater_->update_sessions_call_count(), 0u); + EXPECT_EQ(mock_updater_->ratchet_present_call_count(), 0u); EXPECT_EQ(mock_renderer_->render_frame_call_count(), 0u); + EXPECT_EQ(mock_updater_->signal_previous_frames_presented_call_count(), 0u); SessionId session_id = 1; @@ -183,9 +192,11 @@ TEST_F(FrameSchedulerTest, RunLoopFor(zx::duration(fake_display_->GetVsyncInterval())); EXPECT_EQ(mock_updater_->update_sessions_call_count(), 1u); + EXPECT_EQ(mock_updater_->ratchet_present_call_count(), 1u); EXPECT_EQ(mock_renderer_->render_frame_call_count(), 1u); EXPECT_EQ(mock_renderer_->pending_frames(), 1u); + EXPECT_EQ(mock_updater_->signal_previous_frames_presented_call_count(), 0u); // Schedule another update for now. scheduler->ScheduleUpdateForSession(now, session_id); @@ -194,16 +205,22 @@ TEST_F(FrameSchedulerTest, // Updates should be applied, but not rendered. EXPECT_EQ(mock_updater_->update_sessions_call_count(), 2u); + EXPECT_EQ(mock_updater_->ratchet_present_call_count(), 1u); EXPECT_EQ(mock_renderer_->render_frame_call_count(), 1u); + EXPECT_EQ(mock_updater_->signal_previous_frames_presented_call_count(), 0u); // End previous frame. EXPECT_EQ(mock_renderer_->pending_frames(), 1u); mock_renderer_->EndFrame(/* frame index */ 0); + EXPECT_EQ(mock_updater_->signal_previous_frames_presented_call_count(), 1u); RunLoopFor(zx::duration(fake_display_->GetVsyncInterval())); - // Second render should have occured. + // Second render should have occurred. + EXPECT_EQ(mock_updater_->ratchet_present_call_count(), 2u); EXPECT_EQ(mock_renderer_->render_frame_call_count(), 2u); + mock_renderer_->EndFrame(/* frame index */ 0); + EXPECT_EQ(mock_updater_->signal_previous_frames_presented_call_count(), 2u); } TEST_F(FrameSchedulerTest, RenderCalls_ShouldNotExceed_MaxOutstandingFrames) { diff --git a/garnet/lib/ui/gfx/tests/frame_scheduler_mocks.cc b/garnet/lib/ui/gfx/tests/frame_scheduler_mocks.cc index 9e8bcee3e1a846b21cbd2d30c989c2988a98a18b..211189f30fb56bd73c48d43d4b31f38901718458 100644 --- a/garnet/lib/ui/gfx/tests/frame_scheduler_mocks.cc +++ b/garnet/lib/ui/gfx/tests/frame_scheduler_mocks.cc @@ -12,7 +12,7 @@ namespace test { SessionUpdater::UpdateResults MockSessionUpdater::UpdateSessions( std::unordered_set<SessionId> sessions_to_update, - zx_time_t presentation_time) { + zx_time_t presentation_time, uint64_t trace_id) { ++update_sessions_call_count_; return update_sessions_return_value_; } diff --git a/garnet/lib/ui/gfx/tests/frame_scheduler_mocks.h b/garnet/lib/ui/gfx/tests/frame_scheduler_mocks.h index 38aaf2500383e56ce6d04f5eacc67e5ead562b9c..0a9ca130e51ac37f6bbcf11b99ea92a69c2861a7 100644 --- a/garnet/lib/ui/gfx/tests/frame_scheduler_mocks.h +++ b/garnet/lib/ui/gfx/tests/frame_scheduler_mocks.h @@ -59,9 +59,9 @@ class MockSessionUpdater : public SessionUpdater { SessionUpdater::UpdateResults UpdateSessions( std::unordered_set<SessionId> sessions_to_update, - zx_time_t presentation_time) override; + zx_time_t presentation_time, uint64_t trace_id = 0) override; - void NewFrame() override { ++new_frame_call_count_; } + void RatchetPresentCallbacks() override { ++ratchet_present_call_count_; } void SignalSuccessfulPresentCallbacks( fuchsia::images::PresentationInfo) override { @@ -75,7 +75,7 @@ class MockSessionUpdater : public SessionUpdater { uint32_t update_sessions_call_count() { return update_sessions_call_count_; } - uint32_t new_frame_call_count() { return new_frame_call_count_; } + uint32_t ratchet_present_call_count() { return ratchet_present_call_count_; } uint32_t signal_previous_frames_presented_call_count() { return signal_previous_frames_presented_call_count_; @@ -91,7 +91,7 @@ class MockSessionUpdater : public SessionUpdater { uint32_t update_sessions_call_count_ = 0; uint32_t signal_previous_frames_presented_call_count_ = 0; - uint32_t new_frame_call_count_ = 0; + uint32_t ratchet_present_call_count_ = 0; fxl::WeakPtrFactory<MockSessionUpdater> weak_factory_; // must be last }; diff --git a/garnet/public/lib/escher/renderer/batch_gpu_uploader.cc b/garnet/public/lib/escher/renderer/batch_gpu_uploader.cc index daf7f5f9c86747097b930a5587723d67877cd93f..84e2345a0b67b146e39a5a034220ea1cd6f9f4a5 100644 --- a/garnet/public/lib/escher/renderer/batch_gpu_uploader.cc +++ b/garnet/public/lib/escher/renderer/batch_gpu_uploader.cc @@ -14,7 +14,7 @@ namespace escher { /* static */ std::unique_ptr<BatchGpuUploader> BatchGpuUploader::New( - EscherWeakPtr weak_escher, int64_t frame_trace_number) { + EscherWeakPtr weak_escher, uint64_t frame_trace_number) { if (!weak_escher) { // This class is not functional without a valid escher. FXL_LOG(WARNING) << "Error, creating a BatchGpuUploader without an escher."; @@ -131,7 +131,7 @@ CommandBufferPtr BatchGpuUploader::Reader::TakeCommandsAndShutdown() { } BatchGpuUploader::BatchGpuUploader(EscherWeakPtr weak_escher, - int64_t frame_trace_number) + uint64_t frame_trace_number) : escher_(std::move(weak_escher)), frame_trace_number_(frame_trace_number) { FXL_DCHECK(escher_); } diff --git a/garnet/public/lib/escher/renderer/batch_gpu_uploader.h b/garnet/public/lib/escher/renderer/batch_gpu_uploader.h index 7890deb2bb56e5649c28d060321829bd97a9a702..dfa59e25b723199eeee2ff602d7d920eea867cd2 100644 --- a/garnet/public/lib/escher/renderer/batch_gpu_uploader.h +++ b/garnet/public/lib/escher/renderer/batch_gpu_uploader.h @@ -28,9 +28,9 @@ namespace escher { class BatchGpuUploader { public: static std::unique_ptr<BatchGpuUploader> New(EscherWeakPtr weak_escher, - int64_t frame_trace_number = 0); + uint64_t frame_trace_number = 0); - BatchGpuUploader(EscherWeakPtr weak_escher, int64_t frame_trace_number = 0); + BatchGpuUploader(EscherWeakPtr weak_escher, uint64_t frame_trace_number = 0); ~BatchGpuUploader(); // Provides a pointer in host-accessible GPU memory, and methods to copy this @@ -136,7 +136,7 @@ class BatchGpuUploader { EscherWeakPtr escher_; bool is_initialized_ = false; // The trace number for the frame. Cached to support lazy frame creation. - const int64_t frame_trace_number_; + const uint64_t frame_trace_number_; // Lazily created when the first Reader or Writer is acquired. BufferCacheWeakPtr buffer_cache_; FramePtr frame_;