Skip to content

Stop compiling the PorterDuffBlend kScreen variant - #194538

Merged
auto-submit[bot] merged 2 commits into
flutter:masterfrom
walley892:delete_more_shaders
Oct 17, 2026
Merged

auto-submit[bot] merged 2 commits into
flutter:masterfrom
walley892:delete_more_shaders

Conversation

@walley892

@walley892 walley892 commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Stops compiling the PorterDuffBlend kScreen variant.

This variant is unused, and the getter - GetScreenBlendPipeline has no callers in the codebase. When it was initially added in flutter-team-archive/engine#41098, it was also unused.

Part of #194511

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools.
  • 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 in-code documentation (doc comments with ///).
  • If this PR introduces a new feature or capability, I created and linked a website documentation issue or PR in [flutter/website] (or verified none is needed).
  • 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.

@github-actions github-actions Bot added engine flutter/engine related. See also e: labels. e: impeller Impeller rendering backend issues and features requests labels Oct 16, 2026
@walley892

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request removes the screen blend pipeline and its associated methods, constants, and configurations from the Impeller content context. A review comment suggests using 'const auto' instead of hardcoding the array size for 'porter_duff_constants' to improve maintainability.

@walley892 walley892 added the CICD Run CI/CD label Oct 16, 2026
@walley892
walley892 requested a review from gaaclarke October 16, 2026 06:40
@walley892
walley892 marked this pull request as ready for review October 16, 2026 06:40
@flutter-dashboard

Copy link
Copy Markdown

It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If both you and the PR author believe this PR qualifies for a test exemption, escalate to the tech lead for the changed area to review and apply the test-exempt label. All reviewers should feel empowered to ask for tests before escalating.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request removes the screen blend pipeline and its associated methods and constants from ContentContext. The feedback suggests using auto for the porter_duff_constants declaration to avoid hardcoding the array size and improve maintainability.

Comment on lines +729 to 730
const std::array<std::vector<Scalar>, 14> porter_duff_constants =
GetPorterDuffSpecConstants(supports_decal);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Using auto here avoids duplicating the hardcoded array size 14 and improves maintainability if the number of Porter-Duff constants changes in the future.

Suggested change
const std::array<std::vector<Scalar>, 14> porter_duff_constants =
GetPorterDuffSpecConstants(supports_decal);
const auto porter_duff_constants =
GetPorterDuffSpecConstants(supports_decal);

@walley892 walley892 added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 16, 2026
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 16, 2026
@auto-submit

auto-submit Bot commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

autosubmit label was removed for flutter/flutter/194538, because - This pull request has the missing-tests label and does not have the test-exempt label. Please add tests or obtain a test exemption from a tech lead before re-applying the autosubmit label.

@walley892 walley892 added autosubmit Merge PR when tree becomes green via auto submit App test-exempt Exempt from tests. Overrides `missing-tests` label labels Oct 16, 2026
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 16, 2026
@auto-submit

auto-submit Bot commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

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

@walley892 walley892 added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 16, 2026
@auto-submit
auto-submit Bot added this pull request to the merge queue Oct 16, 2026
Merged via the queue into flutter:master with commit 6b2162e Oct 17, 2026
42 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 17, 2026
suryaKommana2662 pushed a commit to suryaKommana2662/flutter that referenced this pull request Oct 29, 2026
Stops compiling the PorterDuffBlend kScreen variant.

This variant is unused, and the getter - GetScreenBlendPipeline has no
callers in the codebase. When it was initially added in
flutter-team-archive/engine#41098, it was also unused.

Part of flutter#194511

## 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 in-code documentation (doc comments with
`///`).
- [x] If this PR introduces a new feature or capability, I created and
linked a website documentation issue or PR in [flutter/website] (or
verified none is needed).
- [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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICD Run CI/CD e: impeller Impeller rendering backend issues and features requests engine flutter/engine related. See also e: labels. missing-tests test-exempt Exempt from tests. Overrides `missing-tests` label

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants