[web] Unify Renderer frontend code #173426
Conversation
There was a problem hiding this comment.
Code Review
This pull request is a significant refactoring aimed at unifying the frontend code for the CanvasKit and Skwasm web renderers. The changes are extensive, introducing a new layer-based scene representation and compositing mechanism, which replaces the previous SceneBuilder. Many files have been moved and renamed, and classes have been updated to implement new unified interfaces like LayerCanvas and LayerPicture. The platform view embedding logic has also been modernized to use CSS clip-path instead of SVG clips. Overall, the refactoring appears well-executed and consistent with the goal of code unification. I've found one minor issue to address.
engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/canvas.dart
Outdated
Show resolved
Hide resolved
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request is a large-scale refactoring to unify the frontend code for the CanvasKit and Skwasm web renderers. It introduces a common layer-based scene representation and compositing logic, moving and renaming many files to create a more modular and shared architecture. The changes are extensive and touch many parts of the web engine, from rendering primitives to the scene building and rasterization pipeline. My review focuses on one potential issue found in the new platform view clipping implementation, but overall the refactoring appears well-structured and consistent with its goal.
engine/src/flutter/lib/web_ui/lib/src/engine/platform_views/embedder.dart
Show resolved
Hide resolved
I haven't yet. I plan to run them today. Differences in Skwasm performance are the main remaining risk for this PR so I definitely want to make sure there are no regressions |
engine/src/flutter/lib/web_ui/lib/src/engine/platform_views/embedder.dart
Show resolved
Hide resolved
I ran the benchmarks several times and didn't notice any benchmarks that were consistently slower or faster by a significant amount. It's difficult to tell, though, since the benchmarks that are faster or slower change with each run of the benchmarks. |
Great, thanks for checking. Unless it's really drastic, it can be hard to really make a judgment on perf from a single run of benchmarks due to noise. Let's just keep an eye on skia-perf in the coming days after this lands. |
|
Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change). If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
Golden file changes are available for triage from new commit, Click here to view. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
Golden file changes are available for triage from new commit, Click here to view. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
Unifies the shared code between the two web renderers, CanvasKit and Skwasm.
See https://flutter.dev/go/web-renderer-unification for details.
Fixes #172308
Fixes #142072
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.