[windows] Implement merged UI and platform thread #162935

knopp · 2025-02-08T13:45:36Z

Original issue: #150525

Thread merging is currently disabled by default.
It is controlled from the application through DartProject:

flutter::DartProject project(L"data");
// Enables running UI isolate on platform thread
project.set_merged_platform_ui_thread(true);

Required changes to windows embedder:

Resize synchronization no longer blocks on condition variable, instead it polls the Flutter run loop (ignoring other windows messages) until the frame is available. This way resize synchronization works with both thread merging enabled and disabled.

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, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
I followed the breaking change policy and added Data Driven Fixes where supported.
All existing and new tests are passing.

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

engine/src/flutter/shell/common/engine.h

engine/src/flutter/fml/task_queue_id.h

engine/src/flutter/shell/platform/embedder/embedder.h

engine/src/flutter/shell/platform/windows/client_wrapper/include/flutter/dart_project.h

engine/src/flutter/shell/platform/windows/flutter_windows_engine.cc

engine/src/flutter/shell/platform/windows/flutter_windows_view.cc

engine/src/flutter/shell/platform/windows/task_runner_window.cc

loic-sharma · 2025-02-11T00:07:21Z

Did you check if the FlutterWindowsEngine::CreateView and FlutterWindowsEngine::RemoveView need to be updated? See:

flutter/engine/src/flutter/shell/platform/windows/flutter_windows_engine.cc

Lines 543 to 558 in 0be3f8f

    
           FlutterEngineResult result = embedder_api_.AddView(engine_, &info); 
        
           if (result != kSuccess) { 
        
             FML_LOG(ERROR) 
        
                 << "Starting the add view operation failed. FlutterEngineAddView " 
        
                    "returned an unexpected result: " 
        
                 << result << ". This indicates a bug in the Windows embedder."; 
        
             FML_DCHECK(false); 
        
             return nullptr; 
        
           } 
        
           // Block the platform thread until the engine has added the view. 
        
           // TODO(loicsharma): This blocks the platform thread eagerly and can 
        
           // cause unnecessary delay in input processing. Instead, this should block 
        
           // lazily only when the app does an operation which needs the view. 
        
           // https://github.com/flutter/flutter/issues/146248 
        
           captures.latch.Wait();

flutter/engine/src/flutter/shell/platform/windows/flutter_windows_engine.cc

Lines 591 to 616 in 0be3f8f

    
           info.remove_view_callback = [](const FlutterRemoveViewResult* result) { 
        
             // This is invoked on an engine thread. If 
        
             // |FlutterRemoveViewResult.removed| is `true`, the engine guarantees the 
        
             // view won't be presented. 
        
             Captures* captures = reinterpret_cast<Captures*>(result->user_data); 
        
             captures->removed = result->removed; 
        
             captures->latch.Signal(); 
        
           }; 
        
           FlutterEngineResult result = embedder_api_.RemoveView(engine_, &info); 
        
           if (result != kSuccess) { 
        
             FML_LOG(ERROR) << "Starting the remove view operation failed. " 
        
                               "FlutterEngineRemoveView " 
        
                               "returned an unexpected result: " 
        
                            << result 
        
                            << ". This indicates a bug in the Windows embedder."; 
        
             FML_DCHECK(false); 
        
             return; 
        
           } 
        
           // Block the platform thread until the engine has removed the view. 
        
           // TODO(loicsharma): This blocks the platform thread eagerly and can 
        
           // cause unnecessary delay in input processing. Instead, this should block 
        
           // lazily only when an operation needs the view. 
        
           // https://github.com/flutter/flutter/issues/146248 
        
           captures.latch.Wait();

I think the current logic still works with merged threads if UI tasks are executed immediately on the platform thread (as the embedder's signal will be triggered before the embedder waits on the signal). Could you confirm that's correct? Also, could you update the comments to explain the behavior when threads are merged since it won't be obvious.

cc @hbatagelo who's working on multi-window

engine/src/flutter/shell/platform/windows/flutter_windows_view.cc

knopp · 2025-02-11T01:29:19Z

Please don't review the embedder changes :) I'll rebase this PR as soon as #162944 lands (tree is red) and everything embedder related will be gone from this PR.

loic-sharma · 2025-02-11T02:17:25Z

Please don't review the embedder changes :)

Sorry about that, I didn't realize this PR was stacked on another one until after I finished reviewing this one.

knopp · 2025-02-11T12:50:50Z

Did you check if the FlutterWindowsEngine::CreateView and FlutterWindowsEngine::RemoveView need to be updated? See:

flutter/engine/src/flutter/shell/platform/windows/flutter_windows_engine.cc

Lines 543 to 558 in 0be3f8f

FlutterEngineResult result = embedder_api_.AddView(engine_, &info);

if (result != kSuccess) {

FML_LOG(ERROR)

<< "Starting the add view operation failed. FlutterEngineAddView "

"returned an unexpected result: "

<< result << ". This indicates a bug in the Windows embedder.";

FML_DCHECK(false);

return nullptr;

}

// Block the platform thread until the engine has added the view.

// TODO(loicsharma): This blocks the platform thread eagerly and can

// cause unnecessary delay in input processing. Instead, this should block

// lazily only when the app does an operation which needs the view.

// https://github.com/flutter/flutter/issues/146248

captures.latch.Wait();

flutter/engine/src/flutter/shell/platform/windows/flutter_windows_engine.cc

Lines 591 to 616 in 0be3f8f

info.remove_view_callback = [](const FlutterRemoveViewResult* result) {

// This is invoked on an engine thread. If

// |FlutterRemoveViewResult.removed| is `true`, the engine guarantees the

// view won't be presented.

Captures* captures = reinterpret_cast<Captures*>(result->user_data);

captures->removed = result->removed;

captures->latch.Signal();

};

FlutterEngineResult result = embedder_api_.RemoveView(engine_, &info);

if (result != kSuccess) {

FML_LOG(ERROR) << "Starting the remove view operation failed. "

"FlutterEngineRemoveView "

"returned an unexpected result: "

<< result

<< ". This indicates a bug in the Windows embedder.";

FML_DCHECK(false);

return;

}

// Block the platform thread until the engine has removed the view.

// TODO(loicsharma): This blocks the platform thread eagerly and can

// cause unnecessary delay in input processing. Instead, this should block

// lazily only when an operation needs the view.

// https://github.com/flutter/flutter/issues/146248

captures.latch.Wait();

I think the current logic still works with merged threads if UI tasks are executed immediately on the platform thread (as the embedder's signal will be triggered before the embedder waits on the signal). Could you confirm that's correct? Also, could you update the comments to explain the behavior when threads are merged since it won't be obvious.

cc @hbatagelo who's working on multi-window

This will likely deadlock when running with merged platform threads. There might be also other things. That's why I'd first want to merge this with merged threads disabled by default. I have yet to test the canonical PR with merged threads. I'd assume there's going to be some work needed.

…utter#162935)

Manual roll Flutter from 39b4951 to 911aa75 (56 revisions) Manual roll requested by stuartmorgan@google.com flutter/flutter@39b4951...911aa75 2025-02-21 bdero@google.com Remove legacy scenec stuff from flutter_tool (flutter/flutter#163569) 2025-02-21 68449066+zijiehe-google-com@users.noreply.github.com [fuchsia] reorder tests in test_suites.yaml and update the comment (flutter/flutter#163799) 2025-02-21 engine-flutter-autoroll@skia.org Roll Dart SDK from 7ceb3be8ed0a to c5e582f15b6c (2 revisions) (flutter/flutter#163844) 2025-02-21 engine-flutter-autoroll@skia.org Roll Packages from a9b9172 to 5a721da (2 revisions) (flutter/flutter#163848) 2025-02-21 engine-flutter-autoroll@skia.org Roll Dart SDK from ad642ee6f94b to 7ceb3be8ed0a (2 revisions) (flutter/flutter#163832) 2025-02-21 matanlurey@users.noreply.github.com Update how to use VSCode with `clangd` after the monorepo merge (flutter/flutter#163671) 2025-02-21 matanlurey@users.noreply.github.com Reland #163711 after #163780 (flutter/flutter#163812) 2025-02-21 robert.ancell@canonical.com Fix incorrect function name in FlutterEngineRun documentation (flutter/flutter#163696) 2025-02-21 engine-flutter-autoroll@skia.org Roll Dart SDK from 557cbf2c7da6 to ad642ee6f94b (5 revisions) (flutter/flutter#163794) 2025-02-21 1961493+harryterkelsen@users.noreply.github.com [canvaskit] Use `visualViewport.scale` to determine device pixel ratio. (flutter/flutter#163688) 2025-02-21 flar@google.com [DisplayList] Migrate rendering tests and benchmarks to DL geometry (flutter/flutter#163766) 2025-02-21 matanlurey@users.noreply.github.com Make `releaseMode` explicit, inform `determineDevDependencies` entirely on the flag (flutter/flutter#163780) 2025-02-21 matanlurey@users.noreply.github.com Ensure `exclude_dev_dependencies_test` runs if `flutter_tools` changes. (flutter/flutter#163768) 2025-02-21 nabilamevia2003@gmail.com WebKit should respect TextCapitalization.words (flutter/flutter#161027) 2025-02-21 victorsanniay@gmail.com CupertinoSliverNavigationBar.search fidelity updates (flutter/flutter#163089) 2025-02-21 65627237+lucas-goldner@users.noreply.github.com feat: Add selectionColor property to selectable text (flutter/flutter#162177) 2025-02-20 matanlurey@users.noreply.github.com Restore `linux_android_emulator_tests` to CI. (flutter/flutter#163747) 2025-02-20 yjbanov@google.com introduce system color palette (flutter/flutter#163335) 2025-02-20 kevmoo@users.noreply.github.com [web] DRY up access to headers required for multi-threaded WebAssembly (flutter/flutter#163555) 2025-02-20 stanleycocoa@gmail.com fix(CupertinoDatePicker): font is inconsistent (flutter/flutter#162932) 2025-02-20 reidbaker@google.com Create Android-API-And-Related-Versions.md (flutter/flutter#163556) 2025-02-20 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Avoid implicitly setting `determineDevDependencies: true` (it's not a safe operation) (#163711)" (flutter/flutter#163762) 2025-02-20 56849473+mariamhas@users.noreply.github.com Revert "Create g3_bug.yml" (flutter/flutter#163757) 2025-02-20 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Make Flutter version information accessible at runtime (#140783)" (flutter/flutter#163753) 2025-02-20 magder@google.com Skip Xcode install on mac_mokey Android tests (flutter/flutter#163685) 2025-02-20 engine-flutter-autoroll@skia.org Roll Dart SDK from 2cecb16348e4 to 557cbf2c7da6 (3 revisions) (flutter/flutter#163705) 2025-02-20 matanlurey@users.noreply.github.com Add `FlutterVersion.engineCommitDate`, helps signal engine artifact SHA issues (flutter/flutter#163652) 2025-02-20 matanlurey@users.noreply.github.com Avoid implicitly setting `determineDevDependencies: true` (it's not a safe operation) (flutter/flutter#163711) 2025-02-20 matanlurey@users.noreply.github.com It is now safe to call `getBuildMode().release` (flutter/flutter#163712) 2025-02-20 jonahwilliams@google.com [iOS] remove Skia interfaces from iOS platform code. (flutter/flutter#163505) 2025-02-20 ueman@users.noreply.github.com Make Flutter version information accessible at runtime (flutter/flutter#140783) 2025-02-20 matej.knopp@gmail.com [windows] Implement merged UI and platform thread (flutter/flutter#162935) 2025-02-20 31642846+dbspoudel@users.noreply.github.com `CircularProgressIndicator` throws null exception (flutter/flutter#163356) 2025-02-20 ybz975218925@gmail.com Fix the issue where DropdownMenu does not gain focus when tapped. (flutter/flutter#162874) 2025-02-20 codefu@google.com Check for tracked engine.version before overriding (flutter/flutter#163672) 2025-02-20 ybz975218925@gmail.com showDialogs adds a `requestFocus` parameter. (flutter/flutter#162928) 2025-02-20 polinach@google.com Create helper method to dispatch object creation and disposal. (flutter/flutter#163637) 2025-02-20 15619084+vashworth@users.noreply.github.com Redistribute iOS TESTOWNERS (flutter/flutter#163653) 2025-02-20 31859944+LongCatIsLooong@users.noreply.github.com enclosingElement -> enclosingElement3 (flutter/flutter#163681) 2025-02-19 bkonyi@google.com [ Widget Preview ] Move `preview_detector_test.dart` from `general.shard` to `commands.shard` (flutter/flutter#163619) 2025-02-19 jonahwilliams@google.com [iOS] always post new task during gesture dispatch. (flutter/flutter#163646) 2025-02-19 biggs0125@gmail.com Allow flutter tools to detach a running Chrome session (flutter/flutter#163349) 2025-02-19 flar@google.com Reland "[Impeller] add support for rational bezier conics to Path (#63282)" (flutter/flutter#163645) 2025-02-19 yjbanov@google.com [web:a11y] wheel events switch to pointer mode (flutter/flutter#163582) ...

#162935 added a boolean setting to allow merged platform and UI thread. Using boolean doesn't allow for opting out of the behavior once we enable it by default, which is something we will likely want to allow, at least for a period of time. This PR replaces the boolean with following enum: ```cpp // Configures the thread policy for running the UI isolate. typedef enum { // Default value. Currently will run the UI isolate on separate thread, // later will be changed to running the UI isolate on platform thread. Default, // Run the UI isolate on platform thread. RunOnPlatformThread, // Run the UI isolate on a separate thread. RunOnSeparateThread, } FlutterDesktopUIThreadPolicy; ``` ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord].  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: Loïc Sharma <737941+loic-sharma@users.noreply.github.com>

…utter#162935)

) Manual roll Flutter from 39b4951 to 911aa75 (56 revisions) Manual roll requested by stuartmorgan@google.com flutter/flutter@39b4951...911aa75 2025-02-21 bdero@google.com Remove legacy scenec stuff from flutter_tool (flutter/flutter#163569) 2025-02-21 68449066+zijiehe-google-com@users.noreply.github.com [fuchsia] reorder tests in test_suites.yaml and update the comment (flutter/flutter#163799) 2025-02-21 engine-flutter-autoroll@skia.org Roll Dart SDK from 7ceb3be8ed0a to c5e582f15b6c (2 revisions) (flutter/flutter#163844) 2025-02-21 engine-flutter-autoroll@skia.org Roll Packages from a9b9172 to 5a721da (2 revisions) (flutter/flutter#163848) 2025-02-21 engine-flutter-autoroll@skia.org Roll Dart SDK from ad642ee6f94b to 7ceb3be8ed0a (2 revisions) (flutter/flutter#163832) 2025-02-21 matanlurey@users.noreply.github.com Update how to use VSCode with `clangd` after the monorepo merge (flutter/flutter#163671) 2025-02-21 matanlurey@users.noreply.github.com Reland #163711 after #163780 (flutter/flutter#163812) 2025-02-21 robert.ancell@canonical.com Fix incorrect function name in FlutterEngineRun documentation (flutter/flutter#163696) 2025-02-21 engine-flutter-autoroll@skia.org Roll Dart SDK from 557cbf2c7da6 to ad642ee6f94b (5 revisions) (flutter/flutter#163794) 2025-02-21 1961493+harryterkelsen@users.noreply.github.com [canvaskit] Use `visualViewport.scale` to determine device pixel ratio. (flutter/flutter#163688) 2025-02-21 flar@google.com [DisplayList] Migrate rendering tests and benchmarks to DL geometry (flutter/flutter#163766) 2025-02-21 matanlurey@users.noreply.github.com Make `releaseMode` explicit, inform `determineDevDependencies` entirely on the flag (flutter/flutter#163780) 2025-02-21 matanlurey@users.noreply.github.com Ensure `exclude_dev_dependencies_test` runs if `flutter_tools` changes. (flutter/flutter#163768) 2025-02-21 nabilamevia2003@gmail.com WebKit should respect TextCapitalization.words (flutter/flutter#161027) 2025-02-21 victorsanniay@gmail.com CupertinoSliverNavigationBar.search fidelity updates (flutter/flutter#163089) 2025-02-21 65627237+lucas-goldner@users.noreply.github.com feat: Add selectionColor property to selectable text (flutter/flutter#162177) 2025-02-20 matanlurey@users.noreply.github.com Restore `linux_android_emulator_tests` to CI. (flutter/flutter#163747) 2025-02-20 yjbanov@google.com introduce system color palette (flutter/flutter#163335) 2025-02-20 kevmoo@users.noreply.github.com [web] DRY up access to headers required for multi-threaded WebAssembly (flutter/flutter#163555) 2025-02-20 stanleycocoa@gmail.com fix(CupertinoDatePicker): font is inconsistent (flutter/flutter#162932) 2025-02-20 reidbaker@google.com Create Android-API-And-Related-Versions.md (flutter/flutter#163556) 2025-02-20 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Avoid implicitly setting `determineDevDependencies: true` (it's not a safe operation) (#163711)" (flutter/flutter#163762) 2025-02-20 56849473+mariamhas@users.noreply.github.com Revert "Create g3_bug.yml" (flutter/flutter#163757) 2025-02-20 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Make Flutter version information accessible at runtime (#140783)" (flutter/flutter#163753) 2025-02-20 magder@google.com Skip Xcode install on mac_mokey Android tests (flutter/flutter#163685) 2025-02-20 engine-flutter-autoroll@skia.org Roll Dart SDK from 2cecb16348e4 to 557cbf2c7da6 (3 revisions) (flutter/flutter#163705) 2025-02-20 matanlurey@users.noreply.github.com Add `FlutterVersion.engineCommitDate`, helps signal engine artifact SHA issues (flutter/flutter#163652) 2025-02-20 matanlurey@users.noreply.github.com Avoid implicitly setting `determineDevDependencies: true` (it's not a safe operation) (flutter/flutter#163711) 2025-02-20 matanlurey@users.noreply.github.com It is now safe to call `getBuildMode().release` (flutter/flutter#163712) 2025-02-20 jonahwilliams@google.com [iOS] remove Skia interfaces from iOS platform code. (flutter/flutter#163505) 2025-02-20 ueman@users.noreply.github.com Make Flutter version information accessible at runtime (flutter/flutter#140783) 2025-02-20 matej.knopp@gmail.com [windows] Implement merged UI and platform thread (flutter/flutter#162935) 2025-02-20 31642846+dbspoudel@users.noreply.github.com `CircularProgressIndicator` throws null exception (flutter/flutter#163356) 2025-02-20 ybz975218925@gmail.com Fix the issue where DropdownMenu does not gain focus when tapped. (flutter/flutter#162874) 2025-02-20 codefu@google.com Check for tracked engine.version before overriding (flutter/flutter#163672) 2025-02-20 ybz975218925@gmail.com showDialogs adds a `requestFocus` parameter. (flutter/flutter#162928) 2025-02-20 polinach@google.com Create helper method to dispatch object creation and disposal. (flutter/flutter#163637) 2025-02-20 15619084+vashworth@users.noreply.github.com Redistribute iOS TESTOWNERS (flutter/flutter#163653) 2025-02-20 31859944+LongCatIsLooong@users.noreply.github.com enclosingElement -> enclosingElement3 (flutter/flutter#163681) 2025-02-19 bkonyi@google.com [ Widget Preview ] Move `preview_detector_test.dart` from `general.shard` to `commands.shard` (flutter/flutter#163619) 2025-02-19 jonahwilliams@google.com [iOS] always post new task during gesture dispatch. (flutter/flutter#163646) 2025-02-19 biggs0125@gmail.com Allow flutter tools to detach a running Chrome session (flutter/flutter#163349) 2025-02-19 flar@google.com Reland "[Impeller] add support for rational bezier conics to Path (#63282)" (flutter/flutter#163645) 2025-02-19 yjbanov@google.com [web:a11y] wheel events switch to pointer mode (flutter/flutter#163582) ...

github-actions bot added engine flutter/engine repository. See also e: labels. platform-windows Building on or for Windows specifically a: desktop Running on desktop labels Feb 8, 2025

knopp changed the title ~~[windows] Implement merged UI and platform thread~~ [wip] [windows] Implement merged UI and platform thread Feb 8, 2025

knopp force-pushed the windows_merged_ui_thread branch from c81c046 to d2233c3 Compare February 8, 2025 15:10

flutter deleted a comment from flutter-dashboard bot Feb 8, 2025

This comment was marked as resolved.

Sign in to view

knopp force-pushed the windows_merged_ui_thread branch 2 times, most recently from 3238d1e to dc79a94 Compare February 8, 2025 15:42

knopp changed the title ~~[wip] [windows] Implement merged UI and platform thread~~ [windows] Implement merged UI and platform thread Feb 8, 2025

knopp requested review from loic-sharma and mattkae February 8, 2025 15:43

knopp force-pushed the windows_merged_ui_thread branch from dc79a94 to 2373a96 Compare February 8, 2025 15:46

knopp mentioned this pull request Feb 8, 2025

[Embedder] Implement merged platform and UI thread #162944

Merged

9 tasks

knopp force-pushed the windows_merged_ui_thread branch from 2373a96 to 0be3f8f Compare February 10, 2025 10:49