[Windows] Update vsync on raster thread #45310

loic-sharma · 2023-08-31T01:15:02Z

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).

Solution

There's two fixes:

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.
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.

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the CLA.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

loic-sharma · 2023-08-31T16:04:23Z

shell/platform/windows/angle_surface_manager.cc

+    LogEglError(
+        "Unable to release the context after updating the swap interval");
+    return;
+  }


This is what fixes the ANGLE roll. See: #45064

This is covered by existing tests and should be causing failures as it causes the context to be current on multiple threads.

yaakovschectman

LGTM

loic-sharma · 2023-09-01T20:50:28Z

shell/platform/windows/flutter_windows_view_unittests.cc

@@ -111,6 +111,7 @@ class MockFlutterWindowsEngine : public FlutterWindowsEngine {
  MockFlutterWindowsEngine() : FlutterWindowsEngine(GetTestProject()) {}

  MOCK_METHOD0(Stop, bool());


See: #45307

cbracken

…sions) (#133924) Manual roll requested by zra@google.com flutter/engine@489c399...e496eec 2023-09-02 skia-flutter-autoroll@skia.org Roll Skia from 2d8849f9f0cc to 15f77147a3ec (1 revision) (flutter/engine#45414) 2023-09-02 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from OF4TS05qlWCjukWw6... to MesZPNdj-uw8VdCyV... (flutter/engine#45413) 2023-09-02 dkwingsmt@users.noreply.github.com Remove --disable-service-auth-codes (flutter/engine#45356) 2023-09-02 bdero@google.com [Impeller] Import cstring for memcpy. (flutter/engine#45408) 2023-09-02 skia-flutter-autoroll@skia.org Roll Dart SDK from cdf1ce0c6d7e to a5c7102af509 (1 revision) (flutter/engine#45412) 2023-09-02 skia-flutter-autoroll@skia.org Roll ANGLE from 179bd7762ffa to ebf1e7163216 (1 revision) (flutter/engine#45411) 2023-09-02 dkwingsmt@users.noreply.github.com Remove deprecated MOCK_METHODx calls (flutter/engine#45307) 2023-09-02 jonahwilliams@google.com [Impeller] Better demonstrate blur and draw picture? (flutter/engine#45388) 2023-09-02 jonahwilliams@google.com [Impeller] Make paths externally immutable, update all tests to use PathBuilder to create Path. (flutter/engine#45393) 2023-09-02 skia-flutter-autoroll@skia.org Roll ANGLE from 962fdf7b7882 to 179bd7762ffa (1 revision) (flutter/engine#45409) 2023-09-02 flar@google.com Cull the RTree bounds when they are forwarded in DrawDisplayList (flutter/engine#45358) 2023-09-02 skia-flutter-autoroll@skia.org Roll Skia from fedff79a6afc to 2d8849f9f0cc (3 revisions) (flutter/engine#45407) 2023-09-02 jonahwilliams@google.com [impeller] premultiply vertices colors. (flutter/engine#45406) 2023-09-01 skia-flutter-autoroll@skia.org Roll ANGLE from 6a09e41ce6ea to 962fdf7b7882 (224 revisions) (flutter/engine#45400) 2023-09-01 skia-flutter-autoroll@skia.org Roll Skia from 22ae23891e8e to fedff79a6afc (1 revision) (flutter/engine#45405) 2023-09-01 30870216+gaaclarke@users.noreply.github.com [Impeller] turned on validations for all debug builds (flutter/engine#45350) 2023-09-01 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from sk7JBGzW1Jw10Wy-T... to OF4TS05qlWCjukWw6... (flutter/engine#45403) 2023-09-01 skia-flutter-autoroll@skia.org Roll Skia from 2c0405489966 to 22ae23891e8e (1 revision) (flutter/engine#45402) 2023-09-01 737941+loic-sharma@users.noreply.github.com [Windows] Update vsync on raster thread (flutter/engine#45310) 2023-09-01 skia-flutter-autoroll@skia.org Roll Dart SDK from a2ea759c16cc to cdf1ce0c6d7e (1 revision) (flutter/engine#45397) 2023-09-01 skia-flutter-autoroll@skia.org Roll Skia from f3f6c733c7e6 to 2c0405489966 (1 revision) (flutter/engine#45396) 2023-09-01 skia-flutter-autoroll@skia.org Roll Skia from 02fa14799c6c to f3f6c733c7e6 (1 revision) (flutter/engine#45394) 2023-09-01 skia-flutter-autoroll@skia.org Roll Skia from d5d3b0d4ee77 to 02fa14799c6c (2 revisions) (flutter/engine#45392) 2023-09-01 41930132+hellohuanlin@users.noreply.github.com [ios][ios17][text_input]fix text input system highlight in iOS 17 Beta 7 with firstRectForRange (flutter/engine#45303) 2023-09-01 skia-flutter-autoroll@skia.org Roll Skia from d6266ef14a7e to d5d3b0d4ee77 (2 revisions) (flutter/engine#45389) 2023-09-01 skia-flutter-autoroll@skia.org Roll Dart SDK from 0c121a6431cc to a2ea759c16cc (1 revision) (flutter/engine#45384) Also rolling transitive DEPS: fuchsia/sdk/core/mac-amd64 from sk7JBGzW1Jw1 to MesZPNdj-uw8 If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC aaclarke@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

This also reverts the ANGLE roll (for which the original fix was landed) to 48e2c605adcd5bcc1622b18f357c7a73ebfb3543. fixes: flutter/flutter#134262 This reverts commit 708d82d.

This also reverts the ANGLE roll (for which the original fix was landed) to 48e2c605adcd5bcc1622b18f357c7a73ebfb3543. fixes: flutter/flutter#134262 This reverts commit 708d82d. [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

This relands #45310 to unblock the ANGLE roll with the fix for flutter/flutter#134262. ## Background ### Swap interval 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. ### GL context threading Our current version of ANGLE allows making a GL context 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)). The Windows embedder has two GL context threading issues: 1. At startup, the platform thread creates and binds the GL context. This change ensures the GL context is released from the platform thread so that the raster thread can use the GL context for rendering. 2. When the system compositor updates, the GL context is bound to the platform thread to update the swap interval. This change ensures the swap interval update happens on the raster thread. ### Window resizing Resizing the window recreates the GL surface and resets the swap interval. The previous fix released the current GL context after updating the swap interval (this ensured the platform thread released the GL context at startup). This broke window resizing as it caused the engine to "lose" its GL context during rendering (see flutter/flutter#134262). This reland releases the GL context only if on the startup case. [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

This also reverts the ANGLE roll (for which the original fix was landed) to 48e2c605adcd5bcc1622b18f357c7a73ebfb3543. fixes: flutter/flutter#134262 This reverts commit 708d82d. [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

This relands #45310 to unblock the ANGLE roll with the fix for flutter/flutter#134262. ## Background ### Swap interval 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. ### GL context threading Our current version of ANGLE allows making a GL context 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)). The Windows embedder has two GL context threading issues: 1. At startup, the platform thread creates and binds the GL context. This change ensures the GL context is released from the platform thread so that the raster thread can use the GL context for rendering. 2. When the system compositor updates, the GL context is bound to the platform thread to update the swap interval. This change ensures the swap interval update happens on the raster thread. ### Window resizing Resizing the window recreates the GL surface and resets the swap interval. The previous fix released the current GL context after updating the swap interval (this ensured the platform thread released the GL context at startup). This broke window resizing as it caused the engine to "lose" its GL context during rendering (see flutter/flutter#134262). This reland releases the GL context only if on the startup case. [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

[Windows] Update vsync on raster thread

428ac16

github-actions bot added affects: desktop platform-windows labels Aug 31, 2023

loic-sharma commented Aug 31, 2023

View reviewed changes

loic-sharma marked this pull request as ready for review August 31, 2023 22:47

loic-sharma requested review from cbracken and yaakovschectman August 31, 2023 22:48

yaakovschectman approved these changes Sep 1, 2023

View reviewed changes

Update

47d6350

loic-sharma commented Sep 1, 2023

View reviewed changes

cbracken approved these changes Sep 1, 2023

View reviewed changes

Fix

c9c46d1

loic-sharma added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 1, 2023

auto-submit bot merged commit 708d82d into flutter:main Sep 1, 2023

loic-sharma deleted the windows_update_vsync_on_raster_thread branch September 1, 2023 22:37

engine-flutter-autoroll mentioned this pull request Sep 2, 2023

Roll Flutter Engine from 489c399e3dc6 to e496eec40e21 (26 revisions) flutter/flutter#133909

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 2, 2023

708d82dc8 [Windows] Update vsync on raster thread (flutter/engine#45310)

1a634e5

engine-flutter-autoroll mentioned this pull request Sep 2, 2023

Roll Flutter Engine from 489c399e3dc6 to e496eec40e21 (26 revisions) flutter/flutter#133916

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 2, 2023

708d82dc8 [Windows] Update vsync on raster thread (flutter/engine#45310)

c35dd5b

engine-flutter-autoroll mentioned this pull request Sep 2, 2023

Manual roll Flutter Engine from 489c399e3dc6 to e496eec40e21 (26 revisions) flutter/flutter#133924

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 2, 2023

708d82dc8 [Windows] Update vsync on raster thread (flutter/engine#45310)

e2f2a0c

gnprice mentioned this pull request Sep 11, 2023

Window resizing is broken on windows desktop. flutter/flutter#134262

Closed

This was referenced Sep 21, 2023

[Desktop] Add integration tests for window resizing flutter/flutter#135268

Open

[Windows] Improve logic to update swap intervals #46172

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Windows] Update vsync on raster thread #45310

[Windows] Update vsync on raster thread #45310

loic-sharma commented Aug 31, 2023 •

edited

Loading

loic-sharma Aug 31, 2023

yaakovschectman left a comment

loic-sharma Sep 1, 2023

cbracken left a comment

		@@ -111,6 +111,7 @@ class MockFlutterWindowsEngine : public FlutterWindowsEngine {
		MockFlutterWindowsEngine() : FlutterWindowsEngine(GetTestProject()) {}

		MOCK_METHOD0(Stop, bool());

[Windows] Update vsync on raster thread #45310

[Windows] Update vsync on raster thread #45310

Conversation

loic-sharma commented Aug 31, 2023 • edited Loading

Background

Solution

Pre-launch Checklist

loic-sharma Aug 31, 2023

Choose a reason for hiding this comment

yaakovschectman left a comment

Choose a reason for hiding this comment

loic-sharma Sep 1, 2023

Choose a reason for hiding this comment

cbracken left a comment

Choose a reason for hiding this comment

loic-sharma commented Aug 31, 2023 •

edited

Loading