[wimp] Initial Impeller on Web implementation. #175442
Conversation
|
/gemini review |
There was a problem hiding this comment.
Code Review
This PR introduces the initial implementation of Impeller on the web, codenamed "wimp". It's a significant architectural change that introduces an abstraction layer (RenderContext, DlImage, DlRuntimeEffect) to support both Skia and Impeller backends for skwasm. The changes are well-structured, using conditional compilation based on a use_impeller build flag to select backend-specific implementations. The new "wimp" build variant correctly stubs out features that are not yet implemented (image support, custom shaders) and skips the corresponding tests. The build file modifications across GN, Dart, and JS are consistent with adding this new build configuration. I found one critical issue where a null pointer could be dereferenced in the new code, which I've commented on. Overall, this is a solid foundation for bringing Impeller to the web.
|
@gaaclarke I addressed a number of your comments, but I filed a follow-on issue for updating all the skwasm code to the Google C++ guide. I don't really want to update some of this code to conform to it here and then have the rest of it still not conform. Once this lands, I'll have Gemini take a crack at converting everything over to be consistent the style guide in a separate PR. |
gaaclarke
left a comment
There was a problem hiding this comment.
I don't really want to update some of this code to conform to it here and then have the rest of it still not conform. Once this lands, I'll have Gemini take a crack at converting everything over to be consistent the style guide in a separate PR.
Sounds good to me. Can you fix all the new includes. Anything that is about method names or variable names you can just mark as resolved. The more we can change over in the new code the better. Landing one PR that migrates everything will be a nightmare to land since it will collide with everything.
Can you do me a favor and say "done." or "done, <explanation>" for all the other comments you've addressed (sans the variable/function names). I saw at least one that wasn't addressed yet.
| #include <memory> | ||
|
|
||
| namespace flutter { | ||
| extern std::shared_ptr<DlText> textFromBlob(const sk_sp<SkTextBlob>& blob); |
There was a problem hiding this comment.
extern is redundant on a function declaration as long as all users and implementors include this header. You may have to pull this header into its own module to make that easy.
|
@gaaclarke I'd like to defer fixing up the includes for the follow-on PR. Right now, the skwasm code is a bit awkwardly placed inside of |
gaaclarke
left a comment
There was a problem hiding this comment.
LGTM with the promise that the code will be brought to the google c++ style in a separate PR. I think fixing any new improper includes or "using namespace" would be easy and safe to do here, up to you.
This PR adds an initial implementation of a impeller-based version of skwasm. As of right now, the main things that are missing are: * Image support flutter#175371 * Custom shader support flutter#175431 * Turn on MSAA flutter#175441 * Multithreading is turned off flutter#178749 I plan on implementing these in smaller followup PRs. Towards flutter#174980
Manual roll Flutter from 6a1f5b7 to e25d71b (30 revisions) Manual roll requested by tarrinneal@google.com flutter/flutter@6a1f5b7...e25d71b 2025-12-10 jason-simmons@users.noreply.github.com Ensure that the engine converts std::filesystem::path objects to UTF-8 strings on Windows (flutter/flutter#179528) 2025-12-10 ybz975218925@gmail.com Fix the issue with pinned headers in nested SliverMainAxisGroup. (flutter/flutter#179132) 2025-12-10 Akashefrath@gmail.com added onUserInteractionIfError for form (flutter/flutter#175515) 2025-12-10 39942340+prash4931@users.noreply.github.com Fixed RenderFlex overflow in RouteObserver Example (flutter/flutter#170980) 2025-12-10 engine-flutter-autoroll@skia.org Roll Dart SDK from 17749965ec57 to 077062c5e515 (3 revisions) (flutter/flutter#179691) 2025-12-10 carapacik@gmail.com Manually roll characters (flutter/flutter#179447) 2025-12-10 engine-flutter-autoroll@skia.org Roll Packages from 338ecd3 to 74a5a53 (4 revisions) (flutter/flutter#179693) 2025-12-10 fluttergithubbot@gmail.com Marks Mac_ios draw_arcs_all_stroke_styles_perf_ios__timeline_summary to be unflaky (flutter/flutter#179669) 2025-12-10 jason-simmons@users.noreply.github.com Check for a null cached image in SingleFrameCodec::getNextFrame (flutter/flutter#179483) 2025-12-10 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from _pSztGZvEA3-Ry-GW... to u5vxWTRT0HlxOP5_r... (flutter/flutter#179652) 2025-12-10 robert.ancell@canonical.com Implement flutter/accessibility channel (flutter/flutter#179484) 2025-12-10 engine-flutter-autoroll@skia.org Roll Skia from 82fff05cc621 to e61cc6d073fd (4 revisions) (flutter/flutter#179646) 2025-12-10 ahmedsameha1@gmail.com Make sure that a CupertinoDialogAction doesn't crash in 0x0 environment (flutter/flutter#178956) 2025-12-10 matt.boetger@gmail.com Make SettingsChannel configuration queue not static (flutter/flutter#179636) 2025-12-10 ahmedsameha1@gmail.com Make sure that a CupertinoListSection doesn't crash in 0x0 environment (flutter/flutter#179068) 2025-12-10 ahmedsameha1@gmail.com Make sure that a CupertinoFormSection doesn't crash in 0x0 environment (flutter/flutter#179001) 2025-12-10 ahmedsameha1@gmail.com Make sure that a CupertinoMagnifier doesn't crash in 0x0 environment (flutter/flutter#179206) 2025-12-10 ahmedsameha1@gmail.com Make sure that a Tooltip doesn't crash in 0x0 environment (flutter/flutter#178461) 2025-12-10 ahmedsameha1@gmail.com Make sure that a CupertinoSegmentedControl doesn't crash in 0x0 envir… (flutter/flutter#179544) 2025-12-10 ahmedsameha1@gmail.com Make sure that a CupertinoSlider doesn't crash in 0x0 environment (flutter/flutter#179566) 2025-12-10 ahmedsameha1@gmail.com Make sure that a CupertinoPageScaffold doesn't crash in 0x0 environment (flutter/flutter#179245) 2025-12-09 engine-flutter-autoroll@skia.org Roll Skia from f9e32c28c5c5 to 82fff05cc621 (2 revisions) (flutter/flutter#179641) 2025-12-09 engine-flutter-autoroll@skia.org Roll Dart SDK from 019cb923bf62 to 17749965ec57 (5 revisions) (flutter/flutter#179640) 2025-12-09 116356835+AbdeMohlbi@users.noreply.github.com Enhance documentation for `LocalHistoryEntry` class (flutter/flutter#179223) 2025-12-09 Rusino@users.noreply.github.com WebParagrah: ellipsis (flutter/flutter#178748) 2025-12-09 chingjun@google.com Update the doc on Google Testing to reflect the current state (flutter/flutter#177187) 2025-12-09 jacksongardner@google.com [wimp] Initial Impeller on Web implementation. (flutter/flutter#175442) 2025-12-09 47866232+chunhtai@users.noreply.github.com Relax assertion for adding semantics locale (flutter/flutter#178140) 2025-12-09 45459898+RamonFarizel@users.noreply.github.com Fix Scrollbar drag behavior (flutter/flutter#179199) 2025-12-09 engine-flutter-autoroll@skia.org Roll Skia from 502ee6f2a0d7 to f9e32c28c5c5 (6 revisions) (flutter/flutter#179632) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC stuartmorgan@google.com,tarrinneal@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: 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 ...
This PR adds an initial implementation of a impeller-based version of skwasm. As of right now, the main things that are missing are:
I plan on implementing these in smaller followup PRs.
Towards #174980