Skip to content

Conversation

@gaaclarke
Copy link
Member

@gaaclarke gaaclarke commented Nov 17, 2025

fixes #178539

This allows users to chain together fragment shaders with pixel formats other than rgba8.

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-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.

@github-actions github-actions bot added engine flutter/engine related. See also e: labels. e: impeller Impeller rendering backend issues and features requests labels Nov 17, 2025
@gaaclarke gaaclarke force-pushed the format-to-image-sync branch from e1bc825 to 1714a6b Compare November 19, 2025 23:33
@gaaclarke gaaclarke force-pushed the format-to-image-sync branch from 60122bd to a8b932c Compare November 21, 2025 00:54
@gaaclarke gaaclarke marked this pull request as ready for review November 21, 2025 18:35
Copy link
Contributor

@chinmaygarde chinmaygarde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some minor nits.

render_target_size.height *= scale_factor;
}

std::optional<impeller::PixelFormat> impeller_pixel_format;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Hoist into a little helper.

const std::shared_ptr<Texture>& existing_depth_stencil_texture =
nullptr) override;
const std::shared_ptr<Texture>& existing_depth_stencil_texture = nullptr,
std::optional<PixelFormat> target_pixel_format = std::nullopt) override;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oof. These argument lists are getting kinda large aren't they? I remember having a bug for to create a RenderTargetBuilder. I think we even added one but had to back it out because it didn't quite do the right thing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, I thought the same thing. I have a few other things I want to get to so I didn't incorporate as much refactoring here as I was thinking.

color0_tex_desc.storage_mode = color_attachment_config.storage_mode;
color0_tex_desc.format = pixel_format;
color0_tex_desc.format =
target_pixel_format.has_value()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: target_pixel_format.value_or(context.GetCapabilities()->GetDefaultColorFormat());

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And elsewhere.

@github-actions github-actions bot added the platform-web Web applications specifically label Nov 24, 2025
@gaaclarke gaaclarke added autosubmit Merge PR when tree becomes green via auto submit App and removed platform-web Web applications specifically labels Nov 24, 2025
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 24, 2025
@auto-submit
Copy link
Contributor

auto-submit bot commented Nov 24, 2025

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

@gaaclarke gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 24, 2025
@github-actions github-actions bot added the platform-web Web applications specifically label Nov 24, 2025
@gaaclarke gaaclarke force-pushed the format-to-image-sync branch from ccfe34f to f79ae2a Compare November 24, 2025 21:33
@auto-submit
Copy link
Contributor

auto-submit bot commented Nov 25, 2025

autosubmit label was removed for flutter/flutter/178691, 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.

@gaaclarke gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 25, 2025
@flutter-dashboard
Copy link

CI had a failure that stopped further tests from running. We need to investigate to determine the root cause.

SHA at time of execution: 33368de.

Possible causes:

  • Configuration Changes: The .ci.yaml file might have been modified between the creation of this pull request and the start of this test run. This can lead to ci yaml validation errors.
  • Infrastructure Issues: Problems with the CI environment itself (e.g., quota) could have caused the failure.

A blank commit, or merging to head, will be required to resume running CI for this PR.

Error Details:

GitHub Error: You have exceeded a secondary rate limit. Please wait a few minutes before you try again. For more on scraping GitHub and how it may affect your rights, please review our Terms of Service (https://docs.github.com/en/site-policy/github-terms/github-terms-of-service) If you reach out to GitHub Support for help, please include the request ID D832:173C34:12151:4FF44:692627B7.

Stack trace:

#0      GitHub.handleStatusCode (package:github/src/common/github.dart:486:5)
#1      GitHub.request (package:github/src/common/github.dart:422:7)
<asynchronous suspension>
#2      GitHub.requestJson (package:github/src/common/github.dart:323:22)
<asynchronous suspension>
#3      RetryOptions.retry (package:retry/retry.dart:131:16)
<asynchronous suspension>
#4      LuciBuildService.scheduleTryBuilds (package:cocoon_service/src/service/luci_build_service.dart:246:24)
<asynchronous suspension>
#5      Scheduler._runCiTestingStage (package:cocoon_service/src/service/scheduler.dart:1253:9)
<asynchronous suspension>
#6      Scheduler.proceedToCiTestingStage (package:cocoon_service/src/service/scheduler.dart:1314:7)
<asynchronous suspension>
#7      Scheduler._closeSuccessfulEngineBuildStage (package:cocoon_service/src/service/scheduler.dart:1121:5)
<asynchronous suspension>
#8      Scheduler.processCheckRunCompleted (package:cocoon_service/src/service/scheduler.dart:1054:9)
<asynchronous suspension>
#9      PresubmitLuciSubscription.post (package:cocoon_service/src/request_handlers/presubmit_luci_subscription.dart:135:7)
<asynchronous suspension>
#10     RequestHandler.service (package:cocoon_service/src/request_handling/request_handler.dart:45:20)
<asynchronous suspension>
#11     SubscriptionHandler.service (package:cocoon_service/src/request_handling/subscription_handler.dart:140:5)
<asynchronous suspension>
#12     createServer.<anonymous closure> (package:cocoon_service/server.dart:339:7)
<asynchronous suspension>

@auto-submit
Copy link
Contributor

auto-submit bot commented Nov 25, 2025

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

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 25, 2025
@gaaclarke gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 1, 2025
@auto-submit
Copy link
Contributor

auto-submit bot commented Dec 1, 2025

autosubmit label was removed for flutter/flutter/178691, 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 auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Dec 1, 2025
@gaaclarke gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 1, 2025
@gaaclarke
Copy link
Member Author

Merging main to hopefully help with presubmit tasks that were stuck queued.

@auto-submit
Copy link
Contributor

auto-submit bot commented Dec 2, 2025

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

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Dec 2, 2025
@gaaclarke gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 2, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Dec 2, 2025
Merged via the queue into flutter:master with commit d57f6f8 Dec 2, 2025
181 of 182 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Dec 2, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 3, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 3, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Dec 3, 2025
flutter/flutter@5545bb3...e274574

2025-12-03 engine-flutter-autoroll@skia.org Roll Skia from 20829e37dfb8 to db4c79d41513 (1 revision) (flutter/flutter#179401)
2025-12-03 engine-flutter-autoroll@skia.org Roll Skia from adc7ea94cada to 20829e37dfb8 (6 revisions) (flutter/flutter#179385)
2025-12-03 pateltirth454@gmail.com Refactor GetShaderClipDepth for clarity (flutter/flutter#179110)
2025-12-03 engine-flutter-autoroll@skia.org Roll Skia from 3b339a83959b to adc7ea94cada (1 revision) (flutter/flutter#179376)
2025-12-03 engine-flutter-autoroll@skia.org Roll Dart SDK from eb743a1d4ade to 0bb365d7ac74 (7 revisions) (flutter/flutter#179372)
2025-12-03 nebkat@gmail.com feat: Add `mainAxisExtent` parameter to `GridView` constructors (flutter/flutter#176927)
2025-12-03 engine-flutter-autoroll@skia.org Roll Skia from eb01fff20df8 to 3b339a83959b (4 revisions) (flutter/flutter#179371)
2025-12-02 rmolivares@renzo-olivares.dev Fix crash when text editing value changes between scrolls (flutter/flutter#179163)
2025-12-02 engine-flutter-autoroll@skia.org Roll Skia from 6bd3b06b1e08 to eb01fff20df8 (3 revisions) (flutter/flutter#179362)
2025-12-02 timmaffett@gmail.com Adds Impellerc flatbuffer format versioning. (flutter/flutter#175470)
2025-12-02 30870216+gaaclarke@users.noreply.github.com Adds format argument to Picture.toImageSync (flutter/flutter#178691)
2025-12-02 6655696+guidezpl@users.noreply.github.com Delete disabled workflow and add missing permissions key to workflow (flutter/flutter#178911)
2025-12-02 mdebbar@google.com [web] Fix some gn warnings (flutter/flutter#178313)
2025-12-02 engine-flutter-autoroll@skia.org Roll Skia from 45337c4e919d to 6bd3b06b1e08 (4 revisions) (flutter/flutter#179353)
2025-12-02 louisehsu@google.com [ios] Reland Dynamic Content Resizing (flutter/flutter#179153)
2025-12-02 sokolovskyi.konstantin@gmail.com [web] Fix onTextScaleFactorChanged not getting called. (flutter/flutter#178862)
2025-12-02 engine-flutter-autoroll@skia.org Roll Packages from c8be05d to 148dcd2 (9 revisions) (flutter/flutter#179343)

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 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
reidbaker pushed a commit to AbdeMohlbi/flutter that referenced this pull request Dec 10, 2025
fixes flutter#178539

This allows users to chain together fragment shaders with pixel formats
other than rgba8.

## 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.

<!-- Links -->
[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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

e: impeller Impeller rendering backend issues and features requests engine flutter/engine related. See also e: labels. platform-web Web applications specifically

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add TargetPixelFormat to toImageSync

2 participants