[Windows] Set swap interval on raster thread after startup #47787
Conversation
loic-sharma
force-pushed
the
windows_vsync_post_to_raster
branch
2 times, most recently
from
11eb8b5 to
a17d591
Compare
loic-sharma
force-pushed
the
windows_vsync_post_to_raster
branch
2 times, most recently
from
ea03f59 to
5f1ca09
Compare
| // 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. |
There was a problem hiding this comment.
Is this operation safe/effective on either the raster or platform thread? Or in other words, should we expect it to work both before and after the engine initializes? And if so, what is the reasoning for posting it as a task only when it is possible to do so?
There was a problem hiding this comment.
Is this operation safe/effective on either the raster or platform thread?
No. The EGL context cannot be current on multiple threads. Since the raster thread uses the EGL context without any synchronization, other threads shouldn't use the EGL context at all while the raster thread exists. In other words, this operation must not run on the platform thread while the engine is running and the raster thread exists.
However, the EGL context is unused before the engine is initialized. Thus, this operation is safe on the platform thread during startup.
should we expect it to work both before and after the engine initializes?
Yup
There was a problem hiding this comment.
What is achieved by running it on the platform thread when the engine is not initialized? Do we want to make the EGL context current at all on the platform thread in the case where there is not yet any raster thread?
There was a problem hiding this comment.
Our end goal is to set the surface's swap interval to 0. Without this, you'd see Windows takes ~17ms - the full frame budget - to render each surface as EGL's swap buffer blocks until the v-blank. This would prevent us from rendering multiple views using a single raster thread on Windows.
To update the swap interval, you first must first make the EGL context current.
Ideally, only the raster thread would make the EGL context current. However, doing so would require us to update the swap intervals in 3 locations:
- When a view is created and the engine is running, we can update its surface's swap interval immediately
- When the engine is started, we can update surfaces that were created before the engine was started
- When the system compositor changes, we need to update surfaces' swap intervals (the blocking until v-blank prevents screen tearing in this scenario)
Using the platform thread allows us to remove that 2nd location by allowing us to update the swap interval whenever a view is created.
|
Can we land this? Or are there still open questions. |
|
@chinmaygarde Apologies, I was out on vacation. All outstanding feedback has been addressed, I should be able to merge this after I've rebased on top of the latest |
loic-sharma
force-pushed
the
windows_vsync_post_to_raster
branch
from
5f1ca09 to
563946c
Compare
loic-sharma
force-pushed
the
windows_vsync_post_to_raster
branch
from
563946c to
9fb47cb
Compare
loic-sharma
added
the
autosubmit
…139716) flutter/engine@347f506...82de334 2023-12-07 [email protected] Revert "Replace use of Fontmgr::RefDefault with explicit creation calls" (flutter/engine#48755) 2023-12-07 [email protected] Roll Skia from 2abb01e18ab3 to 8ebf43ba1c09 (1 revision) (flutter/engine#48761) 2023-12-07 [email protected] Roll Dart SDK from dbfe4e3f867e to be8a95b6717d (1 revision) (flutter/engine#48757) 2023-12-06 [email protected] Remove obsolete properties. (flutter/engine#48753) 2023-12-06 [email protected] Roll Skia from 7ff0103760d0 to 2abb01e18ab3 (1 revision) (flutter/engine#48751) 2023-12-06 [email protected] Roll Skia from 570103e08087 to 7ff0103760d0 (1 revision) (flutter/engine#48748) 2023-12-06 [email protected] Roll Skia from 326bdc97ac40 to 570103e08087 (1 revision) (flutter/engine#48746) 2023-12-06 [email protected] Roll Dart SDK from 6eb13603c212 to dbfe4e3f867e (1 revision) (flutter/engine#48745) 2023-12-06 [email protected] [Impeller] Store Buffer/Texture bindings in vector instead of map. (flutter/engine#48719) 2023-12-06 [email protected] Roll Skia from 33cba437bf70 to 326bdc97ac40 (2 revisions) (flutter/engine#48743) 2023-12-06 [email protected] [Impeller] Provide the clear color to an advanced blend if it was optimized out (flutter/engine#48646) 2023-12-06 [email protected] [Windows] Set swap interval on raster thread after startup (flutter/engine#47787) 2023-12-06 [email protected] Roll Skia from 384d14063dc1 to 33cba437bf70 (3 revisions) (flutter/engine#48740) 2023-12-06 [email protected] [Windows] Refactor the GLES proc table (flutter/engine#48688) 2023-12-06 [email protected] Replace use of Fontmgr::RefDefault with explicit creation calls (flutter/engine#48571) 2023-12-06 [email protected] [Impeller] disable entity culling by default. (flutter/engine#48717) 2023-12-06 [email protected] Roll Skia from 23e1cb20a6b5 to 384d14063dc1 (1 revision) (flutter/engine#48738) 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 [email protected],[email protected],[email protected] 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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
4 participants
The EGL context can only be used by a single thread at a time. Currently:
FlutterViewControlleris createdIn a multi-view world, a
FlutterViewControllercan be created in parallel to a rendering operation. This results in multiple threads attempting to use the EGL context in parallel, which can crash (see flutter/flutter#137973).This change configures the render surface on the raster thread if the raster thread exists (aka the engine is running).
Addresses flutter/flutter#137973
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.