Implements decodeImageFromPixelsSync by gaaclarke · Pull Request #179519 · flutter/flutter

gaaclarke · 2025-12-05T22:11:00Z

This doesn't implement the following. They can be implemented in later PRs.

a skia implementation (maybe won't implement?)
a web implementation
resizing
target pixel format

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
I followed the breaking change policy and added Data Driven Fixes where supported.
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. 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.

gaaclarke · 2025-12-08T21:07:36Z

Hmm, the linux test runs successfully for me locally.

gaaclarke · 2025-12-11T22:08:02Z

@flar @chinmaygarde friendly ping

chinmaygarde

The ThrowException not unwinding the stack is the big red flag. But past that, the rest looks good to me!

chinmaygarde · 2025-12-11T22:46:11Z

engine/src/flutter/lib/ui/painting/image.cc

+                                            Dart_Handle raw_image_handle) {
+  auto* dart_state = UIDartState::Current();
+  if (!dart_state) {
+    Dart_ThrowException(tonic::ToDart("Dart state is null."));


IIRC, Dart_ThrowException doesn't unwind the C++ stack. So you can't early return and reasonably expect to have RAII'ed stuff cleaned up. Instead of multiple Dart_ThrowExceptions, you could just return a StatusOr or an optional with an error message, propagate that up and just call a single Dart_ThrowException. This reason is why we have so many instances where we just log the error. I think we can do better now with absl and the new patterns.

cc @jason-simmons who might also remember more about the stack unwinding stuff (or lack therof).

This was following the pattern in canvas.cc. I thought it was setting some state in the dart vm and not performing any jump.

example:

flutter/engine/src/flutter/lib/ui/painting/canvas.cc

Line 385 in 2e5ed1a

Dart_ThrowException(

Ahh, you're right. We should fix this in canvas too. Here's from the dart documentation:

* Dart_PropagateError and Dart_ThrowException do not return. Instead * they transfer control non-locally using a setjmp-like mechanism. * This can be inconvenient if you have resources that you need to * clean up before propagating the error.

Done, I split out a helper function that returns static strings as errors. I also fixed a potential problem in canvas.cc where there was a c++ struct on the stack when we threw.

We should probably remove those return statements after the throws too, those are confusing.

We should fix this in canvas too... I also fixed a potential problem in canvas.cc where there was a c++ struct on the stack when we threw...

Yeah. I am sure we have missed a couple of other spots in review. I was also considering just banning throwing of exception from native code. Instead returning an error string to Dart code that can then throw the exception.

remove those return statements after the throws too, those are confusing

Yeah. Perhaps we can decorate the call with [[noreturn]]. That way, the compiler will warn about redundant code the author didn't realize was redundant.

chinmaygarde · 2025-12-11T22:47:14Z

engine/src/flutter/lib/ui/painting/image.cc

+    sk_data = SkData::MakeWithCopy(pixels.data(), pixels.num_elements());
+    sk_image = SkImages::RasterFromData(image_info, sk_data, row_bytes);
+    if (!sk_image) {
+      Dart_ThrowException(tonic::ToDart("Failed to create image from pixels."));


Nothing after this ThrowException will run including RAII cleanups, so the data and image above will be leaked. Hence the recommendation to only call Dart_ThrowException once.

chinmaygarde · 2025-12-11T22:52:53Z

engine/src/flutter/lib/ui/painting/pixel_deferred_image_gpu_impeller.cc

+
+void PixelDeferredImageGPUImpeller::ImageWrapper::SnapshotImage(
+    sk_sp<SkImage> image) {
+  fml::TaskRunner::RunNowOrPostTask(


This may only be called once right? Perhaps guard against incorrect use?

flar

LGTM with a couple of verifying questions...

engine/src/flutter/lib/ui/painting/pixel_deferred_image_gpu_impeller.cc

engine/src/flutter/testing/dart/decode_image_from_pixels_sync_test.dart

Roll Flutter from 6e1aa82 to d81baab (47 revisions) flutter/flutter@6e1aa82...d81baab 2025-12-16 [email protected] Revert "chore: linux fuchsia tests are flaking" (flutter/flutter#179897) 2025-12-15 [email protected] add default-linux-sysroot to gn frontend args (flutter/flutter#179303) 2025-12-15 [email protected] Roll pub packages (flutter/flutter#179751) 2025-12-15 [email protected] Roll Skia from 783ce2077e46 to 6903a4e65c3f (2 revisions) (flutter/flutter#179903) 2025-12-15 [email protected] Add `Adwaita Sans` as a font fallback on Linux (flutter/flutter#179144) 2025-12-15 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Unmodified android sdk bundle (#179647)" (flutter/flutter#179904) 2025-12-15 [email protected] Update window settings as they change rather than the more outdated "Apply" pattern. (flutter/flutter#179861) 2025-12-15 49699333+dependabot[bot]@users.noreply.github.com Bump dessant/lock-threads from 5.0.1 to 6.0.0 in the all-github-actions group (flutter/flutter#179901) 2025-12-15 [email protected] Roll Skia from 9decbd4c216b to 783ce2077e46 (2 revisions) (flutter/flutter#179896) 2025-12-15 [email protected] Unmodified android sdk bundle (flutter/flutter#179647) 2025-12-15 [email protected] [web] Fix `resizeToAvoidBottomInset` on Android web (flutter/flutter#179581) 2025-12-15 [email protected] Suppress deprecation warning for AChoreographer_postFrameCallback (flutter/flutter#178580) 2025-12-15 [email protected] remove unnecessary virtual destructor from VertexDescriptor (flutter/flutter#178682) 2025-12-15 [email protected] Roll Dart SDK from b245f6022727 to 20d114f951db (1 revision) (flutter/flutter#179893) 2025-12-15 [email protected] [Impeller] Delete GLES framebuffer objects only on the thread where they were created (flutter/flutter#179768) 2025-12-15 [email protected] [Impeller] Convert paths in ImpellerC command line flags to std::filesystem::path objects (flutter/flutter#179717) 2025-12-15 [email protected] Roll Skia from 5e1ff611b36b to 9decbd4c216b (2 revisions) (flutter/flutter#179890) 2025-12-15 [email protected] Roll Fuchsia Linux SDK from DtfDWAx6t_CsD2e1W... to 433KtmJvbMyaDMJvD... (flutter/flutter#179889) 2025-12-15 [email protected] Remove unnecessary unboxing in `FlutterLoader.java‎` (flutter/flutter#179869) 2025-12-15 [email protected] Roll Skia from f04f8ca3b284 to 5e1ff611b36b (1 revision) (flutter/flutter#179882) 2025-12-15 [email protected] Roll Packages from 0ac7a03 to 2cd921c (1 revision) (flutter/flutter#179885) 2025-12-15 [email protected] [ Widget Preview ] Pass DTD URI as a constant in a generated file (flutter/flutter#179821) 2025-12-15 [email protected] Bump minSdk to `24` in `engine` (flutter/flutter#175508) 2025-12-15 [email protected] Roll Dart SDK from bca8bb37d95f to b245f6022727 (1 revision) (flutter/flutter#179879) 2025-12-15 [email protected] Roll Skia from eb1347d18957 to f04f8ca3b284 (1 revision) (flutter/flutter#179876) 2025-12-15 [email protected] Roll Skia from 41bcfb848b13 to eb1347d18957 (1 revision) (flutter/flutter#179871) 2025-12-15 [email protected] Roll Skia from d194c3b7a9a9 to 41bcfb848b13 (6 revisions) (flutter/flutter#179866) 2025-12-14 [email protected] Remove obsolete windowing channel (flutter/flutter#179718) 2025-12-14 [email protected] Roll Fuchsia Linux SDK from SCLq31whvg8nJvuYE... to DtfDWAx6t_CsD2e1W... (flutter/flutter#179856) 2025-12-14 [email protected] Roll Skia from 5d98ce0af031 to d194c3b7a9a9 (1 revision) (flutter/flutter#179854) 2025-12-13 [email protected] Roll Skia from f9b242d9a365 to 5d98ce0af031 (1 revision) (flutter/flutter#179843) 2025-12-13 [email protected] Roll Skia from abe90c72cf56 to f9b242d9a365 (1 revision) (flutter/flutter#179838) 2025-12-13 [email protected] Roll Fuchsia Linux SDK from fppT9ZrwbFx7iYrIh... to SCLq31whvg8nJvuYE... (flutter/flutter#179836) 2025-12-13 [email protected] Roll Dart SDK from 34654f2a3baa to bca8bb37d95f (1 revision) (flutter/flutter#179835) 2025-12-13 [email protected] Roll Skia from 9e7c893bb593 to abe90c72cf56 (1 revision) (flutter/flutter#179830) 2025-12-13 [email protected] Roll Dart SDK from f105c2168574 to 34654f2a3baa (1 revision) (flutter/flutter#179823) 2025-12-13 [email protected] Add FilterQuality parameter to FragmentShader.setImageSampler (flutter/flutter#179760) 2025-12-13 [email protected] Add profiling counts to pipeline uses (flutter/flutter#179374) 2025-12-13 [email protected] Roll Skia from edf52cb27f29 to 9e7c893bb593 (3 revisions) (flutter/flutter#179819) 2025-12-12 [email protected] Update Skwasm to engine style guidelines. (flutter/flutter#179756) 2025-12-12 [email protected] Roll Dart SDK from 9a65db770758 to f105c2168574 (5 revisions) (flutter/flutter#179814) 2025-12-12 [email protected] Roll Skia from f23b00a407a4 to edf52cb27f29 (5 revisions) (flutter/flutter#179807) 2025-12-12 [email protected] Use depth buffer to implement geometry overdraw protection (flutter/flutter#179426) 2025-12-12 [email protected] [flutter_tool] Force UTF-8 character set for dev (flutter/flutter#179419) 2025-12-12 [email protected] Roll Skia from e66816c3645e to f23b00a407a4 (2 revisions) (flutter/flutter#179798) 2025-12-12 [email protected] Implements decodeImageFromPixelsSync (flutter/flutter#179519) ...

gaaclarke marked this pull request as draft December 5, 2025 22:11

github-actions bot added the engine flutter/engine related. See also e: labels. label Dec 5, 2025

This comment was marked as outdated.

Sign in to view

gaaclarke added 5 commits December 5, 2025 15:37

Implements decodeImageFromPixelsSync

92de78a

updated testing

17d908f

removed optional arguments for now

330a316

cleanup

67ee437

pulled out pixelformat enum

a666217

gaaclarke force-pushed the decodeImagePixelsSync branch from 22d04c5 to a666217 Compare December 5, 2025 23:37

gaaclarke added 5 commits December 5, 2025 15:40

docstring

69cbdae

lint

84ab671

added assert

6187f77

fixed threading on test

e389823

added web api

f39fe97

gaaclarke marked this pull request as ready for review December 8, 2025 19:07

github-actions bot added the platform-web Web applications specifically label Dec 8, 2025

cleanup

a2878de

gaaclarke requested review from chinmaygarde and flar December 8, 2025 19:09

gaaclarke added 3 commits December 8, 2025 13:10

have a tighter scope on the Uint8List

eee8657

dart analyze

d6de5cb

dart analyze 2

cf0ea10

chinmaygarde reviewed Dec 11, 2025

View reviewed changes

chinmay feedback

8e5b265

gaaclarke requested a review from chinmaygarde December 11, 2025 23:40

flar approved these changes Dec 12, 2025

View reviewed changes

engine/src/flutter/lib/ui/painting/pixel_deferred_image_gpu_impeller.cc Show resolved Hide resolved

engine/src/flutter/testing/dart/decode_image_from_pixels_sync_test.dart Show resolved Hide resolved

chinmaygarde approved these changes Dec 12, 2025

View reviewed changes

gaaclarke added this pull request to the merge queue Dec 12, 2025

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 16, 2025

feaec27 Implements decodeImageFromPixelsSync (flutter/flutter#179519)

654548d

engine-flutter-autoroll mentioned this pull request Dec 16, 2025

Roll Flutter from 6e1aa823523d to d81baabfec4c (47 revisions) flutter/packages#10637

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 17, 2025

feaec27 Implements decodeImageFromPixelsSync (flutter/flutter#179519)

60fd335

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 17, 2025

feaec27 Implements decodeImageFromPixelsSync (flutter/flutter#179519)

e27031e

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 17, 2025

feaec27 Implements decodeImageFromPixelsSync (flutter/flutter#179519)

8507f4a

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 18, 2025

feaec27 Implements decodeImageFromPixelsSync (flutter/flutter#179519)

4f802b8

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 18, 2025

feaec27 Implements decodeImageFromPixelsSync (flutter/flutter#179519)

33a9b13

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 18, 2025

feaec27 Implements decodeImageFromPixelsSync (flutter/flutter#179519)

098e88e

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 18, 2025

feaec27 Implements decodeImageFromPixelsSync (flutter/flutter#179519)

85785bc

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 18, 2025

feaec27 Implements decodeImageFromPixelsSync (flutter/flutter#179519)

3c9e3fb

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 18, 2025

feaec27 Implements decodeImageFromPixelsSync (flutter/flutter#179519)

1dcb95d

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 19, 2025

feaec27 Implements decodeImageFromPixelsSync (flutter/flutter#179519)

229b8d2

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 19, 2025

feaec27 Implements decodeImageFromPixelsSync (flutter/flutter#179519)

5012727

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 19, 2025

feaec27 Implements decodeImageFromPixelsSync (flutter/flutter#179519)

807b077

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 19, 2025

feaec27 Implements decodeImageFromPixelsSync (flutter/flutter#179519)

d14564b

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 19, 2025

feaec27 Implements decodeImageFromPixelsSync (flutter/flutter#179519)

c453b3d

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 20, 2025

feaec27 Implements decodeImageFromPixelsSync (flutter/flutter#179519)

8dfe6cb

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 20, 2025

feaec27 Implements decodeImageFromPixelsSync (flutter/flutter#179519)

a928d7b

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 21, 2025

feaec27 Implements decodeImageFromPixelsSync (flutter/flutter#179519)

81288fe

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 21, 2025

feaec27 Implements decodeImageFromPixelsSync (flutter/flutter#179519)

7a90f19

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 21, 2025

feaec27 Implements decodeImageFromPixelsSync (flutter/flutter#179519)

3287f84

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 22, 2025

feaec27 Implements decodeImageFromPixelsSync (flutter/flutter#179519)

4216838

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 22, 2025

feaec27 Implements decodeImageFromPixelsSync (flutter/flutter#179519)

a6ae1c4

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 22, 2025

feaec27 Implements decodeImageFromPixelsSync (flutter/flutter#179519)

b5cfebb

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 23, 2025

feaec27 Implements decodeImageFromPixelsSync (flutter/flutter#179519)

1fd5bec

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 23, 2025

feaec27 Implements decodeImageFromPixelsSync (flutter/flutter#179519)

a5906b1

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 23, 2025

feaec27 Implements decodeImageFromPixelsSync (flutter/flutter#179519)

aa1b62e

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 11, 2026

feaec27 Implements decodeImageFromPixelsSync (flutter/flutter#179519)

40a137e

Conversation

gaaclarke commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

This comment was marked as outdated.

Uh oh!

gaaclarke commented Dec 8, 2025

Uh oh!

gaaclarke commented Dec 11, 2025

Uh oh!

chinmaygarde left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gaaclarke Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gaaclarke Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

flar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gaaclarke commented Dec 5, 2025 •

edited

Loading

gaaclarke Dec 11, 2025 •

edited

Loading

gaaclarke Dec 11, 2025 •

edited

Loading