From 2bec12f0b4d76d9f60d55d057e16cd2788083ade Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Sharma?= <737941+loic-sharma@users.noreply.github.com> Date: Fri, 1 Sep 2023 15:33:01 -0700 Subject: [PATCH] [Windows] Update vsync on raster thread (flutter/engine#45310) ## Background If the Windows system compositor is enabled, the Windows embedder disables the swap interval so that presenting to a surface does not block until the v-blank. If the Windows system compositor is disabled (which is possible on Windows 7), the Windows embedder enables swap interval to prevent screen tearing. Updating the swap interval requires making the GL surface current, which we currently do on the platform thread. However, the raster thread also needs the GL surface for rendering. Our current version of ANGLE allows making a GL surface current on multiple threads. However, the latest version of ANGLE errors if a GL context is made current on multiple threads. This is causing the ANGLE roll to fail ([example](https://ci.chromium.org/ui/p/flutter/builders/try/Windows%20Engine%20Drone/203788/overview)). ## Solution There's two fixes: 1. The GL context is released once the swap interval is updated. This allows the platform thread to set the initial swap interval at start up, before the raster thread is started. This fixes the ANGLE roll. 3. When the system compositor changes, the Windows embedder now switches to the raster thread before updating the swap interval. This ensures that the GL surface is only made current on the raster thread once the raster thread has started. I updated unit tests to cover this scenario and tested this manually on a Windows 7 machine. [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style --- .../shell/platform/windows/angle_surface_manager.cc | 7 +++++++ .../shell/platform/windows/flutter_windows_engine.h | 2 +- .../shell/platform/windows/flutter_windows_view.cc | 7 ++++++- .../platform/windows/flutter_windows_view_unittests.cc | 8 ++++++++ 4 files changed, 22 insertions(+), 2 deletions(-) diff --git a/engine/src/flutter/shell/platform/windows/angle_surface_manager.cc b/engine/src/flutter/shell/platform/windows/angle_surface_manager.cc index 7deafc2d0b..2165130ba1 100644 --- a/engine/src/flutter/shell/platform/windows/angle_surface_manager.cc +++ b/engine/src/flutter/shell/platform/windows/angle_surface_manager.cc @@ -343,6 +343,13 @@ void AngleSurfaceManager::SetVSyncEnabled(bool enabled) { LogEglError("Unable to update the swap interval"); return; } + + if (eglMakeCurrent(egl_display_, EGL_NO_SURFACE, EGL_NO_SURFACE, + EGL_NO_CONTEXT) != EGL_TRUE) { + LogEglError( + "Unable to release the context after updating the swap interval"); + return; + } } bool AngleSurfaceManager::GetDevice(ID3D11Device** device) { diff --git a/engine/src/flutter/shell/platform/windows/flutter_windows_engine.h b/engine/src/flutter/shell/platform/windows/flutter_windows_engine.h index cdbcd375fd..04d33e7583 100644 --- a/engine/src/flutter/shell/platform/windows/flutter_windows_engine.h +++ b/engine/src/flutter/shell/platform/windows/flutter_windows_engine.h @@ -199,7 +199,7 @@ class FlutterWindowsEngine { bool MarkExternalTextureFrameAvailable(int64_t texture_id); // Posts the given callback onto the raster thread. - bool PostRasterThreadTask(fml::closure callback); + virtual bool PostRasterThreadTask(fml::closure callback); // Invoke on the embedder's vsync callback to schedule a frame. void OnVsync(intptr_t baton); diff --git a/engine/src/flutter/shell/platform/windows/flutter_windows_view.cc b/engine/src/flutter/shell/platform/windows/flutter_windows_view.cc index d668d2f447..20ac21558e 100644 --- a/engine/src/flutter/shell/platform/windows/flutter_windows_view.cc +++ b/engine/src/flutter/shell/platform/windows/flutter_windows_view.cc @@ -669,7 +669,12 @@ void FlutterWindowsView::UpdateSemanticsEnabled(bool enabled) { void FlutterWindowsView::OnDwmCompositionChanged() { if (engine_->surface_manager()) { - engine_->surface_manager()->SetVSyncEnabled(binding_handler_->NeedsVSync()); + // Update the surface with the new composition state. + // Switch to the raster thread as this requires making the context current. + auto needs_vsync = binding_handler_->NeedsVSync(); + engine_->PostRasterThreadTask([this, needs_vsync]() { + engine_->surface_manager()->SetVSyncEnabled(needs_vsync); + }); } } diff --git a/engine/src/flutter/shell/platform/windows/flutter_windows_view_unittests.cc b/engine/src/flutter/shell/platform/windows/flutter_windows_view_unittests.cc index 850f2708d8..2c759b84a8 100644 --- a/engine/src/flutter/shell/platform/windows/flutter_windows_view_unittests.cc +++ b/engine/src/flutter/shell/platform/windows/flutter_windows_view_unittests.cc @@ -111,6 +111,7 @@ class MockFlutterWindowsEngine : public FlutterWindowsEngine { MockFlutterWindowsEngine() : FlutterWindowsEngine(GetTestProject()) {} MOCK_METHOD0(Stop, bool()); + MOCK_METHOD(bool, PostRasterThreadTask, (fml::closure)); private: FML_DISALLOW_COPY_AND_ASSIGN(MockFlutterWindowsEngine); @@ -1280,6 +1281,13 @@ TEST(FlutterWindowsViewTest, UpdatesVSyncOnDwmUpdates) { std::unique_ptr surface_manager = std::make_unique(); + EXPECT_CALL(*engine.get(), PostRasterThreadTask) + .Times(2) + .WillRepeatedly([](fml::closure callback) { + callback(); + return true; + }); + EXPECT_CALL(*window_binding_handler.get(), NeedsVSync) .WillOnce(Return(true)) .WillOnce(Return(false));