-
Notifications
You must be signed in to change notification settings - Fork 29.8k
[reland] Unify canvas creation and Surface code in Skwasm and CanvasKit #179473
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[reland] Unify canvas creation and Surface code in Skwasm and CanvasKit #179473
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This is a significant and well-executed refactoring of the web engine's rendering layer. Unifying the Surface and Rasterizer implementations and introducing the CanvasProvider abstraction greatly improves the architecture's modularity and maintainability. It's great to see that this work also brings proper WebGL context loss handling to the Skwasm backend. The addition of new tests for context loss is also a very valuable contribution. I have a couple of comments on potential issues I've found.
| final DomEventListener eventListener = createDomEventListener((DomEvent event) { | ||
| onContextLost(); | ||
| // The canvas is no longer usable. | ||
| releaseCanvas(canvas); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The webglcontextlost event listener is missing a call to event.preventDefault(). According to the WebGL specification, if preventDefault() is not called on this event, the browser will not attempt to restore the context, and the webglcontextrestored event will not be fired. This will break WebGL context restoration for the CanvasKit renderer.
| final DomEventListener eventListener = createDomEventListener((DomEvent event) { | |
| onContextLost(); | |
| // The canvas is no longer usable. | |
| releaseCanvas(canvas); | |
| }); | |
| final DomEventListener eventListener = createDomEventListener((DomEvent event) { | |
| event.preventDefault(); | |
| onContextLost(); | |
| // The canvas is no longer usable. | |
| releaseCanvas(canvas); | |
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to call preventDefault() because we don't want the browser to attempt to restore the context. In the case the context is lost, we recreate it ourselves with a new Canvas
engine/src/flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/surface.dart
Show resolved
Hide resolved
mdebbar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this PR still relevant after your PR?
| /// NOTE: `dom.DomOffscreenCanvas` is not fully defined in the provided `dom.dart` | ||
| /// snippet. This implementation assumes it exists and has a similar API to the | ||
| /// standard `OffscreenCanvas`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what this comment means. If there are missing things in dom.dart we should fill the gaps.
| } | ||
|
|
||
| @override | ||
| bool get isDisposed => debugDisposed; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If debugDisposed is now meant to be used in production code, maybe let's rename it to _isDisposed and use the new isDisposed in tests?
Yes, it should be merged with this PR after it lands. |
0bb0a90 to
c04c46c
Compare
Manual roll Flutter from 57c3f8b to 6ff7f30 (83 revisions) Manual roll requested by stuartmorgan@google.com flutter/flutter@57c3f8b...6ff7f30 2025-12-23 engine-flutter-autoroll@skia.org Roll Packages from f28cf2e to 5e3a766 (3 revisions) (flutter/flutter#180232) 2025-12-23 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from CmFPyvSc-K8_WDd5p... to 5EgkVbjGVZmCFPdtR... (flutter/flutter#180230) 2025-12-23 engine-flutter-autoroll@skia.org Roll Skia from db7ec9a14905 to bdb147ae3040 (2 revisions) (flutter/flutter#180222) 2025-12-23 bruno.leroux@gmail.com Add SnackBarTheme (flutter/flutter#180001) 2025-12-23 engine-flutter-autoroll@skia.org Roll Skia from 0b1ba3920f1c to db7ec9a14905 (1 revision) (flutter/flutter#180219) 2025-12-23 engine-flutter-autoroll@skia.org Roll Dart SDK from 31e9f619e18a to 94b05f717ba3 (1 revision) (flutter/flutter#180216) 2025-12-23 engine-flutter-autoroll@skia.org Roll Skia from a3e4f7b9d5f3 to 0b1ba3920f1c (1 revision) (flutter/flutter#180214) 2025-12-23 engine-flutter-autoroll@skia.org Roll Skia from b8517d1e25f7 to a3e4f7b9d5f3 (2 revisions) (flutter/flutter#180211) 2025-12-23 dkwingsmt@users.noreply.github.com [Engine] iOS style blurring (flutter/flutter#175458) 2025-12-23 engine-flutter-autoroll@skia.org Roll Dart SDK from 2243e91acaf2 to 31e9f619e18a (1 revision) (flutter/flutter#180210) 2025-12-22 36861262+QuncCccccc@users.noreply.github.com Add error description for nbsp character(\u202f) (flutter/flutter#178895) 2025-12-22 engine-flutter-autoroll@skia.org Roll Skia from 98c01ea504d7 to b8517d1e25f7 (1 revision) (flutter/flutter#180207) 2025-12-22 116356835+AbdeMohlbi@users.noreply.github.com Small clean up in `LocalizationPlugin` (flutter/flutter#180053) 2025-12-22 engine-flutter-autoroll@skia.org Roll Skia from c5beca8fa90b to 98c01ea504d7 (2 revisions) (flutter/flutter#180202) 2025-12-22 engine-flutter-autoroll@skia.org Roll Dart SDK from cff33b09b24d to 2243e91acaf2 (1 revision) (flutter/flutter#180199) 2025-12-22 116356835+AbdeMohlbi@users.noreply.github.com Remove usages of Android's `AsyncTask` in favor of `java.util.concurrent` (flutter/flutter#180050) 2025-12-22 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from 18ZvfJB61p7Z8HAaC... to CmFPyvSc-K8_WDd5p... (flutter/flutter#180193) 2025-12-22 engine-flutter-autoroll@skia.org Roll Skia from 7b7083ed9d57 to c5beca8fa90b (5 revisions) (flutter/flutter#180187) 2025-12-22 engine-flutter-autoroll@skia.org Roll Dart SDK from 38812d17127d to cff33b09b24d (1 revision) (flutter/flutter#180185) 2025-12-22 engine-flutter-autoroll@skia.org Roll Skia from 0eef18a0e2e6 to 7b7083ed9d57 (1 revision) (flutter/flutter#180184) 2025-12-22 engine-flutter-autoroll@skia.org Roll Dart SDK from 66c8013acbff to 38812d17127d (1 revision) (flutter/flutter#180179) 2025-12-21 engine-flutter-autoroll@skia.org Roll Skia from 6fbc6c75b9bb to 0eef18a0e2e6 (2 revisions) (flutter/flutter#180176) 2025-12-21 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from kGnnY1-fTWwYAnk8e... to 18ZvfJB61p7Z8HAaC... (flutter/flutter#180173) 2025-12-21 engine-flutter-autoroll@skia.org Roll Skia from 1a4ca755288a to 6fbc6c75b9bb (1 revision) (flutter/flutter#180167) 2025-12-20 engine-flutter-autoroll@skia.org Roll Skia from 2ad7452bd9d1 to 1a4ca755288a (1 revision) (flutter/flutter#180160) 2025-12-20 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from oe10epXkqGnv21AbZ... to kGnnY1-fTWwYAnk8e... (flutter/flutter#180158) 2025-12-20 engine-flutter-autoroll@skia.org Roll Skia from b01ad49ea807 to 2ad7452bd9d1 (1 revision) (flutter/flutter#180155) 2025-12-20 engine-flutter-autoroll@skia.org Roll Dart SDK from 8fb1c0c0a8ae to 66c8013acbff (1 revision) (flutter/flutter#180154) 2025-12-20 737941+loic-sharma@users.noreply.github.com Remove unnecessary RadioGroup migration TODOs (flutter/flutter#180105) 2025-12-20 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[reland] Unify canvas creation and Surface code in Skwasm and CanvasKit (#179473)" (flutter/flutter#180152) 2025-12-20 engine-flutter-autoroll@skia.org Roll Skia from 3cc7e81928f0 to b01ad49ea807 (1 revision) (flutter/flutter#180151) 2025-12-20 engine-flutter-autoroll@skia.org Roll Dart SDK from ac95c6e8a31d to 8fb1c0c0a8ae (1 revision) (flutter/flutter#180148) 2025-12-19 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#180146) 2025-12-19 engine-flutter-autoroll@skia.org Roll Skia from fa4434632ce6 to 3cc7e81928f0 (4 revisions) (flutter/flutter#180142) 2025-12-19 1961493+harryterkelsen@users.noreply.github.com [reland] Unify canvas creation and Surface code in Skwasm and CanvasKit (flutter/flutter#179473) 2025-12-19 engine-flutter-autoroll@skia.org Roll Skia from ae5dd72b3591 to fa4434632ce6 (2 revisions) (flutter/flutter#180136) 2025-12-19 45459898+RamonFarizel@users.noreply.github.com Semantics headingLeveldoc update (flutter/flutter#179999) 2025-12-19 matt.kosarek@canonical.com Fix an issue where the semantics announce event may be encoded as either an int32_t or an int64_t depending on its value (flutter/flutter#180071) 2025-12-19 bkonyi@google.com [ Web ] Pass `--enable-experimental-ffi` when compiling WASM tests (flutter/flutter#180127) 2025-12-19 engine-flutter-autoroll@skia.org Roll Dart SDK from cfc117d10d36 to ac95c6e8a31d (1 revision) (flutter/flutter#180130) 2025-12-19 58529443+srujzs@users.noreply.github.com Pass canaryFeatures to BuildSettings (flutter/flutter#180108) 2025-12-19 engine-flutter-autoroll@skia.org Roll Skia from fe2be289c9fe to ae5dd72b3591 (1 revision) (flutter/flutter#180129) 2025-12-19 engine-flutter-autoroll@skia.org Roll Packages from 6f392aa to f28cf2e (1 revision) (flutter/flutter#180124) 2025-12-19 94012149+richardexfo@users.noreply.github.com Set text input purpose and hints on Linux platform (flutter/flutter#180013) ...
…kwasm and CanvasKit (flutter#179473)" (flutter#180152)" This reverts commit 78a6a01.
……kwasm and CanvasKit (#179473)" (#180152)" (#180610) This PR introduces a significant refactoring of the web engine's rendering layer by unifying the Surface and Rasterizer implementations. These components have been moved from being renderer-specific to a generic compositing directory, making the architecture more modular and easier to maintain. The rasterizers are now renderer-agnostic and are provided with renderer-specific surface factories via dependency injection. A new CanvasProvider abstraction has also been introduced to manage the lifecycle of the underlying canvas elements. A key outcome of this work is that the Skwasm backend now correctly handles WebGL context loss events. This was achieved by refactoring SkwasmSurface to allow the Dart side to manage the OffscreenCanvas lifecycle. A communication channel between the main thread and the web worker is now used to gracefully handle context loss and recovery. This effort also included fixing several related bugs around surface sizing, resource cleanup, and callback handling in multi-surface scenarios. To validate these changes, new testing APIs have been added to allow for the creation of renderer-agnostic surface tests. A new test file, surface_context_lost_test.dart, has been added to verify the context loss and recovery behavior across all supported renderers, ensuring the new architecture is robust and reliable. ## 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]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. <!-- Links --> [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
This PR introduces a significant refactoring of the web engine's rendering layer by unifying the Surface and Rasterizer implementations. These components have been moved from being renderer-specific to a generic compositing directory, making the architecture more modular and easier to maintain. The rasterizers are now renderer-agnostic and are provided with renderer-specific surface factories via dependency injection. A new CanvasProvider abstraction has also been introduced to manage the lifecycle of the underlying canvas elements.
A key outcome of this work is that the Skwasm backend now correctly handles WebGL context loss events. This was achieved by refactoring SkwasmSurface to allow the Dart side to manage the OffscreenCanvas lifecycle. A communication channel between the main thread and the web worker is now used to gracefully handle context loss and recovery. This effort also included fixing several related bugs around surface sizing, resource cleanup, and callback handling in multi-surface scenarios.
To validate these changes, new testing APIs have been added to allow for the creation of renderer-agnostic surface tests. A new test file, surface_context_lost_test.dart, has been added to verify the context loss and recovery behavior across all supported renderers, ensuring the new architecture is robust and reliable.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.