Impeller: Allows R32G32B32A32_SFLOAT images by gaaclarke · Pull Request #177959 · flutter/flutter

gaaclarke · 2025-11-04T00:43:46Z

design doc: https://docs.google.com/document/d/1zpkMutZkqo2GVdMhiKzFURpgN8JDZTjtIVwwqqHKM90/edit?tab=t.0

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.

chinmaygarde

The major thing I didn't quite get was how incoming pixel format could be "optimal".

chinmaygarde · 2025-11-11T19:00:09Z

engine/src/flutter/lib/ui/painting/image_decoder.h

+    kR32G32B32A32Float,
+  };
+
+  struct Options {


Nit: Perhaps rename this to DecodeOptions?

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

chinmaygarde · 2025-11-11T19:10:35Z

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

+    const std::shared_ptr<SkBitmap>& bitmap,
+    std::shared_ptr<ImpellerAllocator> bitmap_allocator,
+    const std::shared_ptr<impeller::Allocator>& allocator) {
+  if (bitmap->alphaType() != SkAlphaType::kUnpremul_SkAlphaType) {


Add a trace event here?

chinmaygarde · 2025-11-11T19:12:21Z

engine/src/flutter/lib/ui/painting/image_decoder_impeller.h

+    impeller::PixelFormat format = impeller::PixelFormat::kUnknown;
+  };
+
+  struct DecompressResult {


Ditto on StatusOr instead instead of an error field.

chinmaygarde · 2025-11-11T19:13:11Z

engine/src/flutter/lib/ui/painting/image_decoder_impeller.h

+  struct DecompressResult {
+    std::shared_ptr<impeller::DeviceBuffer> device_buffer;
+    ImageInfo image_info;
+    std::optional<SkImageInfo> resize_info = std::nullopt;


Optionals are auto initialized to std::nullopt.

chinmaygarde · 2025-11-11T19:14:46Z

engine/src/flutter/lib/ui/painting/image_decoder.h


  using ImageResult = std::function<void(sk_sp<DlImage>, std::string)>;

+  enum PixelFormat {


Can this be called TargetPixelFormat or DecodedPixelFormat? Also, what is kUnknown? Is it the same as kOptimal or kDontCare? Perhaps add some docstrings?

Instead of kOptimal, I like kDontCare instead. With optimal, you have to explain how its optimal. With kDontCare, you are leaving it up to the implementation. Which is whats happening here.

chinmaygarde · 2025-11-11T19:16:53Z

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

+  std::shared_ptr<impeller::DeviceBuffer> buffer =
+      scaled_allocator->GetDeviceBuffer();
+  if (!buffer) {
+    return {.decode_error = "Unable to get device buffer"};


Ditto on statusor.

chinmaygarde · 2025-11-11T19:20:33Z

engine/src/flutter/lib/ui/painting.dart

+
+  /// This is an unspecified pixel format that leaves it to Flutter's discretion
+  /// to choose the optimal pixel format.
+  optimal,


How about kDontCare.

Whatever name you choose, perhaps also specify when someone might want to use this. Something like "Use this when the image is going to be sampled the application doesn't care about the exact format."

Wait, this can't be right. This struct is given to decodeImageFromPixels. This isn't the target pixel format, its the incoming pixel format. How can Flutter divine this format?

Don't Care sounds a bit judgmental, like you've given up on your expectations from a pesky implementation.

kImplementationDependent?
kAny?
kBestForConditions?
kUnspecified?

Or how about just leaving it null to mean "do what you want"? You only set this when you care.

gaaclarke · 2025-11-11T19:30:15Z

The major thing I didn't quite get was how incoming pixel format could be "optimal".

It couldn't be, that would result in a runtime error. The PixelFormat type has 2 uses, it defines the input format for decoding pixels and it defines the target pixel format for the texture. Not all values are valid for for each of those inputs. Alternatively we could have made a different enum "TargetPixelFormat"?

chinmaygarde · 2025-11-11T20:28:47Z

Alternatively we could have made a different enum "TargetPixelFormat"

Ooh. I really like that idea. Instead of conflating the two ideas.

chinmaygarde

The new updates LGTM. I had a concern about device capability checks that I think we can document our way out of.

chinmaygarde · 2025-11-12T19:50:00Z

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

    case kBGR_101010x_XR_SkColorType:
      return impeller::PixelFormat::kB10G10R10XR;
+    case kRGBA_F32_SkColorType:
+      return impeller::PixelFormat::kR32G32B32A32Float;


I am pretty sure this requires a capability check (or am I mistaken). At least for the GL backend I'd think. Perhaps we should fallback to the DontCare case?

Not according to my investigation in gpuinfo.org. I have that in the design doc. I think it says something like 95% compatibility but it also says that for r8g8b8a8. It doesn't seem to go to 100%

gaaclarke · 2025-11-12T21:38:49Z

@chinmaygarde thanks for giving it another look, i responded to all your comments but I forgot to hit "submit" then github lost them.

auto-submit · 2025-11-12T22:20:27Z

autosubmit label was removed for flutter/flutter/177959, because - The status or check suite Linux analyzer_benchmark has failed. Please fix the issues identified (or deflake) before re-applying this label.

The status or check suite Linux analyze has failed. Please fix the issues identified (or deflake) before re-applying this label.

auto-submit · 2025-11-12T23:08:07Z

autosubmit label was removed for flutter/flutter/177959, because - The status or check suite Linux analyzer_benchmark has failed. Please fix the issues identified (or deflake) before re-applying this label.

The status or check suite Linux analyze has failed. Please fix the issues identified (or deflake) before re-applying this label.

auto-submit · 2025-11-13T01:21:39Z

autosubmit label was removed for flutter/flutter/177959, because - The status or check suite Linux analyzer_benchmark has failed. Please fix the issues identified (or deflake) before re-applying this label.

The status or check suite Linux analyze has failed. Please fix the issues identified (or deflake) before re-applying this label.
The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

…177959)

fixes flutter#141289 design doc: https://docs.google.com/document/d/1zpkMutZkqo2GVdMhiKzFURpgN8JDZTjtIVwwqqHKM90/edit?tab=t.0 ## 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.  [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

…177959)

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

gaaclarke changed the title ~~Impeller: Allows R32_SFLOAT images~~ Impeller: Allows R32G32B32A32_SFLOAT images Nov 4, 2025

github-actions bot added the platform-ios iOS applications specifically label Nov 6, 2025

gaaclarke force-pushed the destination-format branch 4 times, most recently from 78430bf to 2a8b146 Compare November 7, 2025 19:15

github-actions bot added platform-ios iOS applications specifically and removed platform-ios iOS applications specifically labels Nov 7, 2025

gaaclarke force-pushed the destination-format branch from ccfe85c to b6ed3d7 Compare November 7, 2025 20:31

github-actions bot added the platform-web Web applications specifically label Nov 10, 2025

gaaclarke marked this pull request as ready for review November 10, 2025 17:54

gaaclarke requested review from chinmaygarde and flar November 10, 2025 17:55

chinmaygarde reviewed Nov 11, 2025

View reviewed changes

gaaclarke requested a review from chinmaygarde November 11, 2025 21:57

chinmaygarde approved these changes Nov 12, 2025

View reviewed changes

gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 12, 2025

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 12, 2025

gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 12, 2025

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 12, 2025

gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 13, 2025

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 13, 2025

gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 13, 2025

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

33a30cb Impeller: Allows R32G32B32A32_SFLOAT images (flutter/flutter#…

a86511f

…177959)

engine-flutter-autoroll mentioned this pull request Nov 14, 2025

Manual roll Flutter from ee23168ddb5d to 5798e1a28622 (106 revisions) flutter/packages#10432

Closed

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

33a30cb Impeller: Allows R32G32B32A32_SFLOAT images (flutter/flutter#…

6051dc1

…177959)

Piinks mentioned this pull request Nov 14, 2025

Flutter Notable Commits - 2025 #161460

Closed

deckerst mentioned this pull request Nov 14, 2025

Ultra HDR support deckerst/aves#838

Open

engine-flutter-autoroll mentioned this pull request Nov 15, 2025

Roll Flutter from 9b5ad8031646 to 5ae70ee7c058 (112 revisions) flutter/packages#10437

Closed

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

33a30cb Impeller: Allows R32G32B32A32_SFLOAT images (flutter/flutter#…

98518e5

…177959)

engine-flutter-autoroll mentioned this pull request Nov 15, 2025

Roll Flutter from 9b5ad8031646 to 5ae70ee7c058 (112 revisions) flutter/packages#10438

Closed

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

33a30cb Impeller: Allows R32G32B32A32_SFLOAT images (flutter/flutter#…

cb1da20

…177959)

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

Roll Flutter from 9b5ad8031646 to 0924c0c2bb0e (117 revisions) flutter/packages#10441

Closed

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

33a30cb Impeller: Allows R32G32B32A32_SFLOAT images (flutter/flutter#…

0eff593

…177959)

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

Roll Flutter from 9b5ad8031646 to 505aa720015a (118 revisions) flutter/packages#10442

Closed

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

33a30cb Impeller: Allows R32G32B32A32_SFLOAT images (flutter/flutter#…

713497d

…177959)

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

Roll Flutter from 9b5ad8031646 to f954fb79dd15 (119 revisions) flutter/packages#10444

Closed

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

33a30cb Impeller: Allows R32G32B32A32_SFLOAT images (flutter/flutter#…

888700c

…177959)

engine-flutter-autoroll mentioned this pull request Nov 17, 2025

Roll Flutter from 9b5ad8031646 to 40f3b2e6e6a9 (123 revisions) flutter/packages#10447

Closed

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

33a30cb Impeller: Allows R32G32B32A32_SFLOAT images (flutter/flutter#…

b38717d

…177959)

engine-flutter-autoroll mentioned this pull request Nov 17, 2025

Roll Flutter from 9b5ad8031646 to ce7d9b9882c0 (125 revisions) flutter/packages#10449

Closed

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

33a30cb Impeller: Allows R32G32B32A32_SFLOAT images (flutter/flutter#…

b361202

…177959)

engine-flutter-autoroll mentioned this pull request Nov 17, 2025

Roll Flutter from 9b5ad8031646 to c546757beb28 (127 revisions) flutter/packages#10451

Closed

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

33a30cb Impeller: Allows R32G32B32A32_SFLOAT images (flutter/flutter#…

8fa7de6

…177959)

engine-flutter-autoroll mentioned this pull request Nov 17, 2025

Roll Flutter from 9b5ad8031646 to f5e649180789 (128 revisions) flutter/packages#10452

Closed

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

33a30cb Impeller: Allows R32G32B32A32_SFLOAT images (flutter/flutter#…

51fe964

…177959)

engine-flutter-autoroll mentioned this pull request Nov 17, 2025

Roll Flutter from 9b5ad8031646 to 77f3aca4907f (132 revisions) flutter/packages#10453

Closed

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

33a30cb Impeller: Allows R32G32B32A32_SFLOAT images (flutter/flutter#…

4a72702

…177959)

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

33a30cb Impeller: Allows R32G32B32A32_SFLOAT images (flutter/flutter#…

7344e05

…177959)

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

33a30cb Impeller: Allows R32G32B32A32_SFLOAT images (flutter/flutter#…

830af34

…177959)


		using ImageResult = std::function<void(sk_sp<DlImage>, std::string)>;

		enum PixelFormat {

Conversation

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

Uh oh!

Pre-launch Checklist

Uh oh!

chinmaygarde left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

gaaclarke commented Nov 11, 2025

Uh oh!

chinmaygarde commented Nov 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 commented Nov 12, 2025

Uh oh!

auto-submit bot commented Nov 12, 2025

Uh oh!

auto-submit bot commented Nov 12, 2025

Uh oh!

auto-submit bot commented Nov 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gaaclarke commented Nov 4, 2025 •

edited

Loading