refactor(web): Unify Image on Skwasm and CanvasKit#187873
Conversation
Introduces EngineImage and BackendImage, with EngineImage handling all of the shared logic for implementing an Image in the Flutter Engine, and BackendImage as a thin wrapper around the native Image object. Another step towards flutter#175630
There was a problem hiding this comment.
Code Review
This pull request refactors the web engine's image representation by introducing EngineImage as a frontend wrapper that delegates to a BackendImage (such as CkImageDelegate or SkwasmImage), and extracts image source handling into a dedicated ImageSource hierarchy. Feedback on these changes includes addressing a potential memory leak in CanvasKit's scaleImage where a temporary EngineImage is not disposed, optimizing Uint8ClampedList.view instantiation in EngineImage.toByteData to avoid an intermediate Uint8List view, and updating several documentation comments in Skwasm's image.dart to use /// instead of // to comply with the style guide.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the web engine's image architecture by introducing a unified EngineImage class and BackendImage interface, migrating CanvasKit and Skwasm backends to this new model, and moving image source implementations to a dedicated primitives file. The review feedback identifies several opportunities to improve robustness and resource management: throwing a StateError when attempting to clone, check, or extract byte data from a disposed EngineImage to prevent native crashes in release mode; wrapping drawing operations in scaleImage in a try-finally block to prevent native SkImage leaks; adding defensive checks in ImageSource.release to prevent reference count underflow; and simplifying type casting and target dimension calculations.
| if (filterQuality == ui.FilterQuality.high) { | ||
| skCanvas.drawImageCubic( | ||
| (image as CkImage).skImage, | ||
| ((image as EngineImage).backendImage as CkImageDelegate).skImage, |
There was a problem hiding this comment.
We are doing a lot of these verbose type casts. I'm wonder if it would help to do something like:
class EngineImage<T extends BackendImage> implements ui.Image, StackTraceDebugger {
EngineImage(T backendImage, this.width, this.height, {this.imageSource}) {
box = CountedRef<EngineImage, T>(...);
...
}
...
T get backendImage => box.nativeObject;
...
}Then, the above line of code becomes like:
(image as EngineImage<CkImageDelegate>).backendImage.skImageand asserts becomes:
assert(
image is EngineImage<CkImageDelegate>,
'...',
); There was a problem hiding this comment.
I considered adding a type parameter, but the problem is that we don't know the type of the backend image when we create the EngineImage.
| final imageInfo = SkImageInfo( | ||
| alphaType: canvasKit.AlphaType.Unpremul, | ||
| colorType: canvasKit.ColorType.RGBA_8888, | ||
| colorSpace: SkColorSpaceSRGB, | ||
| width: sourceRect.width, | ||
| height: sourceRect.height, | ||
| ); | ||
| final Uint8List imageBytes = generateParagraphImage(); | ||
| final SkImage? skImage = canvasKit.MakeImage( | ||
| imageInfo, | ||
| imageBytes, | ||
| (4 * sourceRect.width).toInt(), | ||
| ); | ||
|
|
||
| if (skImage == null) { | ||
| throw Exception('Failed to convert text image bitmap to an SkImage.'); | ||
| } | ||
| _singleImageCache = CkImage(skImage); | ||
| _singleImageCache = EngineImage( | ||
| CkImageDelegate(skImage), | ||
| skImage.width().toInt(), | ||
| skImage.height().toInt(), | ||
| ); |
There was a problem hiding this comment.
Off topic, but kinda related: is there a way to make this code renderer-agnostic? Then we can reuse it for Skwasm later.
There was a problem hiding this comment.
Yes, that's what I'm working on now. I plan on unifying the Codec and various decoding methods.
|
autosubmit label was removed for flutter/flutter/187873, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label. |
flutter/flutter@3a0420c...b10d0f1 2026-06-17 mr-peipei@web.de Skip platform-specific plugin registration if no platforms enabled (flutter/flutter#186304) 2026-06-17 engine-flutter-autoroll@skia.org Roll Packages from 8286d39 to 6ce00a8 (1 revision) (flutter/flutter#188109) 2026-06-17 engine-flutter-autoroll@skia.org Roll Skia from 79f93fd5f36e to 5d19002eb73e (1 revision) (flutter/flutter#188108) 2026-06-17 simon@journeyapps.com Import `dart:_js_interop_wasm` in addition to `dart:_wasm` to convert between `JSAny` and `WasmExternRef?` (flutter/flutter#186974) 2026-06-17 engine-flutter-autoroll@skia.org Roll Dart SDK from f811ecae9ca0 to e39bde5b1bfc (2 revisions) (flutter/flutter#188107) 2026-06-17 engine-flutter-autoroll@skia.org Roll Skia from 026f6a6be2b9 to 79f93fd5f36e (1 revision) (flutter/flutter#188105) 2026-06-17 engine-flutter-autoroll@skia.org Roll Dart SDK from 462bf0a1d489 to f811ecae9ca0 (1 revision) (flutter/flutter#188099) 2026-06-17 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from VeLhhlDcod09NR4Hb... to or21OEdGtairm6nl9... (flutter/flutter#188098) 2026-06-17 engine-flutter-autoroll@skia.org Roll Skia from 2ffd155313f5 to 026f6a6be2b9 (10 revisions) (flutter/flutter#188097) 2026-06-17 engine-flutter-autoroll@skia.org Roll Dart SDK from 824b4b48b6d4 to 462bf0a1d489 (1 revision) (flutter/flutter#188093) 2026-06-17 jason-simmons@users.noreply.github.com Manual Dart roll from f6c31f4c3a63 to 824b4b48b6d4 (flutter/flutter#188023) 2026-06-17 awolff@google.com Add a platform view test to android_hardware_smoke_test (flutter/flutter#188069) 2026-06-17 44747303+theprantadutta@users.noreply.github.com [flutter_tools] Format empty app template with latest dart format (flutter/flutter#187443) 2026-06-16 49699333+dependabot[bot]@users.noreply.github.com Bump the all-github-actions group across 1 directory with 3 updates (flutter/flutter#188086) 2026-06-16 engine-flutter-autoroll@skia.org Roll Skia from d7196b0b4939 to 2ffd155313f5 (9 revisions) (flutter/flutter#188081) 2026-06-16 43089218+chika3742@users.noreply.github.com Prevent downgrading `project.pbxproj` when greater version number (flutter/flutter#186250) 2026-06-16 magder@google.com Only allow dependabot to autoupdate GitHub-owned actions (flutter/flutter#187936) 2026-06-16 matt.boetger@gmail.com Fall back to source AndroidManifest.xml if AAPT fails or returns garbage (flutter/flutter#187197) 2026-06-16 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#187769) 2026-06-16 jason-simmons@users.noreply.github.com [Impeller] Move queue submission into a callback that is invoked by FenceWaiterVK::AddFence only if it can accept the fence (flutter/flutter#187761) 2026-06-16 jhy03261997@gmail.com Reland [a11y] Map some framework semantics roles to android classes. (flutter/flutter#188037) 2026-06-16 1961493+harryterkelsen@users.noreply.github.com refactor(web): Unify Image on Skwasm and CanvasKit (flutter/flutter#187873) 2026-06-16 30870216+gaaclarke@users.noreply.github.com Adds arm64 variant of impeller devicelab tests for windows. (flutter/flutter#188053) 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 bmparr@google.com,stuartmorgan@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
|
Reason for revert: causes google3 test failure |
…187873)" (flutter#188124) <!-- start_original_pr_link --> Reverts: flutter#187873 <!-- end_original_pr_link --> <!-- start_initiating_author --> Initiated by: harryterkelsen <!-- end_initiating_author --> <!-- start_revert_reason --> Reason for reverting: causes google3 test failure <!-- end_revert_reason --> <!-- start_original_pr_author --> Original PR Author: harryterkelsen <!-- end_original_pr_author --> <!-- start_reviewers --> Reviewed By: {mdebbar} <!-- end_reviewers --> <!-- start_revert_body --> This change reverts the following previous change: Introduces EngineImage and BackendImage, with EngineImage handling all of the shared logic for implementing an Image in the Flutter Engine, and BackendImage as a thin wrapper around the native Image object. Another step towards flutter#175630 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [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]. If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance. **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 [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [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 <!-- end_revert_body --> Co-authored-by: auto-submit[bot] <flutter-engprod-team@google.com>
…187873)" (flutter#188124) <!-- start_original_pr_link --> Reverts: flutter#187873 <!-- end_original_pr_link --> <!-- start_initiating_author --> Initiated by: harryterkelsen <!-- end_initiating_author --> <!-- start_revert_reason --> Reason for reverting: causes google3 test failure <!-- end_revert_reason --> <!-- start_original_pr_author --> Original PR Author: harryterkelsen <!-- end_original_pr_author --> <!-- start_reviewers --> Reviewed By: {mdebbar} <!-- end_reviewers --> <!-- start_revert_body --> This change reverts the following previous change: Introduces EngineImage and BackendImage, with EngineImage handling all of the shared logic for implementing an Image in the Flutter Engine, and BackendImage as a thin wrapper around the native Image object. Another step towards flutter#175630 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [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]. If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance. **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 [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [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 <!-- end_revert_body --> Co-authored-by: auto-submit[bot] <flutter-engprod-team@google.com>
Introduces EngineImage and BackendImage, with EngineImage handling all of the shared logic for implementing an Image in the Flutter Engine, and BackendImage as a thin wrapper around the native Image object. Another step towards flutter#175630 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [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]. If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance. **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 [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [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
…187873)" (flutter#188124) <!-- start_original_pr_link --> Reverts: flutter#187873 <!-- end_original_pr_link --> <!-- start_initiating_author --> Initiated by: harryterkelsen <!-- end_initiating_author --> <!-- start_revert_reason --> Reason for reverting: causes google3 test failure <!-- end_revert_reason --> <!-- start_original_pr_author --> Original PR Author: harryterkelsen <!-- end_original_pr_author --> <!-- start_reviewers --> Reviewed By: {mdebbar} <!-- end_reviewers --> <!-- start_revert_body --> This change reverts the following previous change: Introduces EngineImage and BackendImage, with EngineImage handling all of the shared logic for implementing an Image in the Flutter Engine, and BackendImage as a thin wrapper around the native Image object. Another step towards flutter#175630 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [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]. If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance. **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 [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [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 <!-- end_revert_body --> Co-authored-by: auto-submit[bot] <flutter-engprod-team@google.com>

Introduces EngineImage and BackendImage, with EngineImage handling all of the shared logic for implementing an Image in the Flutter Engine, and BackendImage as a thin wrapper around the native Image object.
Another step towards #175630
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.
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.