diff --git a/shell/platform/windows/angle_surface_manager.cc b/shell/platform/windows/angle_surface_manager.cc index 7005348482856..6d972ae723a49 100644 --- a/shell/platform/windows/angle_surface_manager.cc +++ b/shell/platform/windows/angle_surface_manager.cc @@ -231,8 +231,7 @@ void AngleSurfaceManager::CleanUp() { bool AngleSurfaceManager::CreateSurface(WindowsRenderTarget* render_target, EGLint width, - EGLint height, - bool vsync_enabled) { + EGLint height) { if (!render_target || !initialize_succeeded_) { return false; } @@ -255,13 +254,6 @@ bool AngleSurfaceManager::CreateSurface(WindowsRenderTarget* render_target, surface_width_ = width; surface_height_ = height; render_surface_ = surface; - - if (!MakeCurrent()) { - LogEglError("Unable to make surface current to update the swap interval"); - return false; - } - - SetVSyncEnabled(vsync_enabled); return true; } @@ -277,11 +269,13 @@ void AngleSurfaceManager::ResizeSurface(WindowsRenderTarget* render_target, ClearContext(); DestroySurface(); - if (!CreateSurface(render_target, width, height, vsync_enabled)) { + if (!CreateSurface(render_target, width, height)) { FML_LOG(ERROR) << "AngleSurfaceManager::ResizeSurface failed to create surface"; } } + + SetVSyncEnabled(vsync_enabled); } void AngleSurfaceManager::GetSurfaceDimensions(EGLint* width, EGLint* height) { @@ -342,6 +336,11 @@ EGLSurface AngleSurfaceManager::CreateSurfaceFromHandle( } void AngleSurfaceManager::SetVSyncEnabled(bool enabled) { + if (!MakeCurrent()) { + LogEglError("Unable to make surface current to update the swap interval"); + return; + } + // OpenGL swap intervals can be used to prevent screen tearing. // If enabled, the raster thread blocks until the v-blank. // This is unnecessary if DWM composition is enabled. diff --git a/shell/platform/windows/angle_surface_manager.h b/shell/platform/windows/angle_surface_manager.h index d50dc2a7f1e64..472bb03763940 100644 --- a/shell/platform/windows/angle_surface_manager.h +++ b/shell/platform/windows/angle_surface_manager.h @@ -36,11 +36,11 @@ class AngleSurfaceManager { // Target represents the visual entity to bind to. Width and // height represent dimensions surface is created at. // - // This binds |egl_context_| to the current thread. + // After the surface is created, |SetVSyncEnabled| should be called on a + // thread that can bind the |egl_context_|. virtual bool CreateSurface(WindowsRenderTarget* render_target, EGLint width, - EGLint height, - bool enable_vsync); + EGLint height); // Resizes backing surface from current size to newly requested size // based on width and height for the specific case when width and height do @@ -68,7 +68,7 @@ class AngleSurfaceManager { virtual bool MakeCurrent(); // Unbinds the current EGL context from the current thread. - bool ClearCurrent(); + virtual bool ClearCurrent(); // Clears the |egl_context_| draw and read surfaces. bool ClearContext(); @@ -91,6 +91,12 @@ class AngleSurfaceManager { // If enabled, makes the current surface's buffer swaps block until the // v-blank. + // + // If disabled, allows one thread to swap multiple buffers per v-blank + // but can result in screen tearing if the system compositor is disabled. + // + // This binds |egl_context_| to the current thread and makes the render + // surface current. virtual void SetVSyncEnabled(bool enabled); // Gets the |ID3D11Device| chosen by ANGLE. diff --git a/shell/platform/windows/flutter_window.cc b/shell/platform/windows/flutter_window.cc index 9336974094321..831faeab9cda8 100644 --- a/shell/platform/windows/flutter_window.cc +++ b/shell/platform/windows/flutter_window.cc @@ -373,7 +373,7 @@ ui::AXPlatformNodeWin* FlutterWindow::GetAlert() { return alert_node_.get(); } -bool FlutterWindow::NeedsVSync() { +bool FlutterWindow::NeedsVSync() const { // If the Desktop Window Manager composition is enabled, // the system itself synchronizes with v-sync. // See: https://learn.microsoft.com/windows/win32/dwm/composition-ovw diff --git a/shell/platform/windows/flutter_window.h b/shell/platform/windows/flutter_window.h index 49eb37284f05b..f758c0e71ac20 100644 --- a/shell/platform/windows/flutter_window.h +++ b/shell/platform/windows/flutter_window.h @@ -196,7 +196,7 @@ class FlutterWindow : public KeyboardManager::WindowDelegate, virtual ui::AXPlatformNodeWin* GetAlert() override; // |WindowBindingHandler| - virtual bool NeedsVSync() override; + virtual bool NeedsVSync() const override; // Called to obtain a pointer to the fragment root delegate. virtual ui::AXFragmentRootDelegateWin* GetAxFragmentRootDelegate(); diff --git a/shell/platform/windows/flutter_windows_engine.cc b/shell/platform/windows/flutter_windows_engine.cc index d58a576eae099..70f21c50ab805 100644 --- a/shell/platform/windows/flutter_windows_engine.cc +++ b/shell/platform/windows/flutter_windows_engine.cc @@ -685,7 +685,7 @@ bool FlutterWindowsEngine::MarkExternalTextureFrameAvailable( engine_, texture_id) == kSuccess); } -bool FlutterWindowsEngine::PostRasterThreadTask(fml::closure callback) { +bool FlutterWindowsEngine::PostRasterThreadTask(fml::closure callback) const { struct Captures { fml::closure callback; }; diff --git a/shell/platform/windows/flutter_windows_engine.h b/shell/platform/windows/flutter_windows_engine.h index ff7f6e4a33b2d..aaa7b9540f9dc 100644 --- a/shell/platform/windows/flutter_windows_engine.h +++ b/shell/platform/windows/flutter_windows_engine.h @@ -102,7 +102,7 @@ class FlutterWindowsEngine { bool Run(std::string_view entrypoint); // Returns true if the engine is currently running. - bool running() { return engine_ != nullptr; } + virtual bool running() const { return engine_ != nullptr; } // Stops the engine. This invalidates the pointer returned by engine(). // @@ -141,7 +141,9 @@ class FlutterWindowsEngine { // The ANGLE surface manager object. If this is nullptr, then we are // rendering using software instead of OpenGL. - AngleSurfaceManager* surface_manager() { return surface_manager_.get(); } + AngleSurfaceManager* surface_manager() const { + return surface_manager_.get(); + } WindowProcDelegateManager* window_proc_delegate_manager() { return window_proc_delegate_manager_.get(); @@ -201,7 +203,7 @@ class FlutterWindowsEngine { bool MarkExternalTextureFrameAvailable(int64_t texture_id); // Posts the given callback onto the raster thread. - virtual bool PostRasterThreadTask(fml::closure callback); + virtual bool PostRasterThreadTask(fml::closure callback) const; // Invoke on the embedder's vsync callback to schedule a frame. void OnVsync(intptr_t baton); diff --git a/shell/platform/windows/flutter_windows_view.cc b/shell/platform/windows/flutter_windows_view.cc index 00623ff810e64..b964a6fca2aec 100644 --- a/shell/platform/windows/flutter_windows_view.cc +++ b/shell/platform/windows/flutter_windows_view.cc @@ -36,6 +36,38 @@ bool SurfaceWillUpdate(size_t cur_width, (cur_height != target_height) || (cur_width != target_width); return non_zero_target_dims && not_same_size; } + +/// Update the surface's swap interval to block until the v-blank iff +/// the system compositor is disabled. +void UpdateVsync(const FlutterWindowsEngine& engine, + const WindowBindingHandler& window) { + AngleSurfaceManager* surface_manager = engine.surface_manager(); + if (!surface_manager) { + return; + } + + // Updating the vsync makes the EGL context and render surface current. + // If the engine is running, the render surface should only be made current on + // the raster thread. If the engine is initializing, the raster thread doesn't + // exist yet and the render surface can be made current on the platform + // thread. + auto needs_vsync = window.NeedsVSync(); + if (engine.running()) { + engine.PostRasterThreadTask([surface_manager, needs_vsync]() { + surface_manager->SetVSyncEnabled(needs_vsync); + }); + } else { + surface_manager->SetVSyncEnabled(needs_vsync); + + // Release the EGL context so that the raster thread can use it. + if (!surface_manager->ClearCurrent()) { + FML_LOG(ERROR) + << "Unable to clear current surface after updating the swap interval"; + return; + } + } +} + } // namespace FlutterWindowsView::FlutterWindowsView( @@ -580,15 +612,10 @@ bool FlutterWindowsView::PresentSoftwareBitmap(const void* allocation, void FlutterWindowsView::CreateRenderSurface() { if (engine_ && engine_->surface_manager()) { PhysicalWindowBounds bounds = binding_handler_->GetPhysicalWindowBounds(); - bool enable_vsync = binding_handler_->NeedsVSync(); engine_->surface_manager()->CreateSurface(GetRenderTarget(), bounds.width, - bounds.height, enable_vsync); + bounds.height); - // The EGL context cannot be current on multiple threads. - // Creating the render surface runs on the platform thread and - // makes the EGL context current. Thus, the EGL context must be - // released so that the raster thread can use it for rendering. - engine_->surface_manager()->ClearCurrent(); + UpdateVsync(*engine_, *binding_handler_); resize_target_width_ = bounds.width; resize_target_height_ = bounds.height; @@ -660,24 +687,7 @@ void FlutterWindowsView::UpdateSemanticsEnabled(bool enabled) { } void FlutterWindowsView::OnDwmCompositionChanged() { - AngleSurfaceManager* surface_manager = engine_->surface_manager(); - if (!surface_manager) { - return; - } - - // Update the surface with the new composition state. - // Switch to the raster thread as the render EGL context can only be - // current on a single thread a time. - auto needs_vsync = binding_handler_->NeedsVSync(); - engine_->PostRasterThreadTask([surface_manager, needs_vsync]() { - if (!surface_manager->MakeCurrent()) { - FML_LOG(ERROR) - << "Unable to make surface current to update the swap interval"; - return; - } - - surface_manager->SetVSyncEnabled(needs_vsync); - }); + UpdateVsync(*engine_, *binding_handler_); } void FlutterWindowsView::OnWindowStateEvent(HWND hwnd, WindowStateEvent event) { diff --git a/shell/platform/windows/flutter_windows_view_unittests.cc b/shell/platform/windows/flutter_windows_view_unittests.cc index 39cea644c78dc..ee5c280c6b7ba 100644 --- a/shell/platform/windows/flutter_windows_view_unittests.cc +++ b/shell/platform/windows/flutter_windows_view_unittests.cc @@ -110,8 +110,9 @@ class MockFlutterWindowsEngine : public FlutterWindowsEngine { public: MockFlutterWindowsEngine() : FlutterWindowsEngine(GetTestProject()) {} + MOCK_METHOD(bool, running, (), (const)); MOCK_METHOD(bool, Stop, (), ()); - MOCK_METHOD(bool, PostRasterThreadTask, (fml::closure), ()); + MOCK_METHOD(bool, PostRasterThreadTask, (fml::closure), (const)); private: FML_DISALLOW_COPY_AND_ASSIGN(MockFlutterWindowsEngine); @@ -123,7 +124,7 @@ class MockAngleSurfaceManager : public AngleSurfaceManager { MOCK_METHOD(bool, CreateSurface, - (WindowsRenderTarget*, EGLint, EGLint, bool), + (WindowsRenderTarget*, EGLint, EGLint), (override)); MOCK_METHOD(void, ResizeSurface, @@ -132,6 +133,7 @@ class MockAngleSurfaceManager : public AngleSurfaceManager { MOCK_METHOD(void, DestroySurface, (), (override)); MOCK_METHOD(bool, MakeCurrent, (), (override)); + MOCK_METHOD(bool, ClearCurrent, (), (override)); MOCK_METHOD(void, SetVSyncEnabled, (bool), (override)); private: @@ -879,7 +881,6 @@ TEST(FlutterWindowsViewTest, WindowRepaintTests) { FlutterWindowsView view(std::make_unique(100, 100)); view.SetEngine(engine.get()); - view.CreateRenderSurface(); bool schedule_frame_called = false; modifier.embedder_api().ScheduleFrame = @@ -1221,7 +1222,8 @@ TEST(FlutterWindowsViewTest, TooltipNodeData) { } // Don't block until the v-blank if it is disabled by the window. -TEST(FlutterWindowsViewTest, DisablesVSync) { +// The surface is updated on the platform thread at startup. +TEST(FlutterWindowsViewTest, DisablesVSyncAtStartup) { std::unique_ptr engine = std::make_unique(); auto window_binding_handler = @@ -1229,6 +1231,9 @@ TEST(FlutterWindowsViewTest, DisablesVSync) { std::unique_ptr surface_manager = std::make_unique(); + EXPECT_CALL(*engine.get(), running).WillRepeatedly(Return(false)); + EXPECT_CALL(*engine.get(), PostRasterThreadTask).Times(0); + EXPECT_CALL(*window_binding_handler.get(), NeedsVSync) .WillOnce(Return(false)); @@ -1236,10 +1241,81 @@ TEST(FlutterWindowsViewTest, DisablesVSync) { FlutterWindowsView view(std::move(window_binding_handler)); InSequence s; - EXPECT_CALL(*surface_manager.get(), - CreateSurface(_, _, _, /*vsync_enabled=*/false)) + EXPECT_CALL(*surface_manager.get(), CreateSurface(_, _, _)) + .Times(1) + .WillOnce(Return(true)); + EXPECT_CALL(*surface_manager.get(), SetVSyncEnabled(false)).Times(1); + EXPECT_CALL(*surface_manager.get(), ClearCurrent).WillOnce(Return(true)); + + EXPECT_CALL(*engine.get(), Stop).Times(1); + EXPECT_CALL(*surface_manager.get(), DestroySurface).Times(1); + + modifier.SetSurfaceManager(surface_manager.release()); + view.SetEngine(engine.get()); + + view.CreateRenderSurface(); +} + +// Blocks until the v-blank if it is enabled by the window. +// The surface is updated on the platform thread at startup. +TEST(FlutterWindowsViewTest, EnablesVSyncAtStartup) { + std::unique_ptr engine = + std::make_unique(); + auto window_binding_handler = + std::make_unique>(); + std::unique_ptr surface_manager = + std::make_unique(); + + EXPECT_CALL(*engine.get(), running).WillRepeatedly(Return(false)); + EXPECT_CALL(*engine.get(), PostRasterThreadTask).Times(0); + EXPECT_CALL(*window_binding_handler.get(), NeedsVSync).WillOnce(Return(true)); + + EngineModifier modifier(engine.get()); + FlutterWindowsView view(std::move(window_binding_handler)); + + InSequence s; + EXPECT_CALL(*surface_manager.get(), CreateSurface(_, _, _)) + .Times(1) + .WillOnce(Return(true)); + EXPECT_CALL(*surface_manager.get(), SetVSyncEnabled(true)).Times(1); + EXPECT_CALL(*surface_manager.get(), ClearCurrent).WillOnce(Return(true)); + + EXPECT_CALL(*engine.get(), Stop).Times(1); + EXPECT_CALL(*surface_manager.get(), DestroySurface).Times(1); + + modifier.SetSurfaceManager(surface_manager.release()); + view.SetEngine(engine.get()); + + view.CreateRenderSurface(); +} + +// Don't block until the v-blank if it is disabled by the window. +// The surface is updated on the raster thread if the engine is running. +TEST(FlutterWindowsViewTest, DisablesVSyncAfterStartup) { + std::unique_ptr engine = + std::make_unique(); + auto window_binding_handler = + std::make_unique>(); + std::unique_ptr surface_manager = + std::make_unique(); + + EXPECT_CALL(*engine.get(), running).WillRepeatedly(Return(true)); + EXPECT_CALL(*window_binding_handler.get(), NeedsVSync).WillOnce(Return(true)); + + EngineModifier modifier(engine.get()); + FlutterWindowsView view(std::move(window_binding_handler)); + + InSequence s; + EXPECT_CALL(*surface_manager.get(), CreateSurface(_, _, _)) .Times(1) .WillOnce(Return(true)); + EXPECT_CALL(*engine.get(), PostRasterThreadTask) + .WillOnce([](fml::closure callback) { + callback(); + return true; + }); + EXPECT_CALL(*surface_manager.get(), SetVSyncEnabled(true)).Times(1); + EXPECT_CALL(*surface_manager.get(), ClearCurrent).Times(0); EXPECT_CALL(*engine.get(), Stop).Times(1); EXPECT_CALL(*surface_manager.get(), DestroySurface).Times(1); @@ -1251,7 +1327,8 @@ TEST(FlutterWindowsViewTest, DisablesVSync) { } // Blocks until the v-blank if it is enabled by the window. -TEST(FlutterWindowsViewTest, EnablesVSync) { +// The surface is updated on the raster thread if the engine is running. +TEST(FlutterWindowsViewTest, EnablesVSyncAfterStartup) { std::unique_ptr engine = std::make_unique(); auto window_binding_handler = @@ -1259,16 +1336,24 @@ TEST(FlutterWindowsViewTest, EnablesVSync) { std::unique_ptr surface_manager = std::make_unique(); + EXPECT_CALL(*engine.get(), running).WillRepeatedly(Return(true)); + EXPECT_CALL(*window_binding_handler.get(), NeedsVSync).WillOnce(Return(true)); EngineModifier modifier(engine.get()); FlutterWindowsView view(std::move(window_binding_handler)); InSequence s; - EXPECT_CALL(*surface_manager.get(), - CreateSurface(_, _, _, /*vsync_enabled=*/true)) + EXPECT_CALL(*surface_manager.get(), CreateSurface(_, _, _)) .Times(1) .WillOnce(Return(true)); + EXPECT_CALL(*engine.get(), PostRasterThreadTask) + .WillOnce([](fml::closure callback) { + callback(); + return true; + }); + EXPECT_CALL(*surface_manager.get(), SetVSyncEnabled(true)).Times(1); + EXPECT_CALL(*surface_manager.get(), ClearCurrent).Times(0); EXPECT_CALL(*engine.get(), Stop).Times(1); EXPECT_CALL(*surface_manager.get(), DestroySurface).Times(1); @@ -1290,8 +1375,9 @@ TEST(FlutterWindowsViewTest, UpdatesVSyncOnDwmUpdates) { std::unique_ptr surface_manager = std::make_unique(); + EXPECT_CALL(*engine.get(), running).WillRepeatedly(Return(true)); + EXPECT_CALL(*engine.get(), PostRasterThreadTask) - .Times(2) .WillRepeatedly([](fml::closure callback) { callback(); return true; @@ -1301,13 +1387,13 @@ TEST(FlutterWindowsViewTest, UpdatesVSyncOnDwmUpdates) { .WillOnce(Return(true)) .WillOnce(Return(false)); + EXPECT_CALL(*surface_manager.get(), ClearCurrent).Times(0); + EngineModifier modifier(engine.get()); FlutterWindowsView view(std::move(window_binding_handler)); InSequence s; - EXPECT_CALL(*surface_manager.get(), MakeCurrent).WillOnce(Return(true)); EXPECT_CALL(*surface_manager.get(), SetVSyncEnabled(true)).Times(1); - EXPECT_CALL(*surface_manager.get(), MakeCurrent).WillOnce(Return(true)); EXPECT_CALL(*surface_manager.get(), SetVSyncEnabled(false)).Times(1); EXPECT_CALL(*engine.get(), Stop).Times(1); diff --git a/shell/platform/windows/testing/mock_window_binding_handler.h b/shell/platform/windows/testing/mock_window_binding_handler.h index cea349ab692a8..43be5abc71a43 100644 --- a/shell/platform/windows/testing/mock_window_binding_handler.h +++ b/shell/platform/windows/testing/mock_window_binding_handler.h @@ -40,7 +40,7 @@ class MockWindowBindingHandler : public WindowBindingHandler { MOCK_METHOD(PointerLocation, GetPrimaryPointerLocation, (), (override)); MOCK_METHOD(AlertPlatformNodeDelegate*, GetAlertDelegate, (), (override)); MOCK_METHOD(ui::AXPlatformNodeWin*, GetAlert, (), (override)); - MOCK_METHOD(bool, NeedsVSync, (), (override)); + MOCK_METHOD(bool, NeedsVSync, (), (const override)); private: FML_DISALLOW_COPY_AND_ASSIGN(MockWindowBindingHandler); diff --git a/shell/platform/windows/window_binding_handler.h b/shell/platform/windows/window_binding_handler.h index 698a2c58b97cf..390e9235c5f23 100644 --- a/shell/platform/windows/window_binding_handler.h +++ b/shell/platform/windows/window_binding_handler.h @@ -106,7 +106,7 @@ class WindowBindingHandler { // If true, rendering to the window should synchronize with the vsync // to prevent screen tearing. - virtual bool NeedsVSync() = 0; + virtual bool NeedsVSync() const = 0; }; } // namespace flutter