Turn on fragment shader equality test #180784

gaaclarke · 2026-01-10T00:13:52Z

I manually tested against #163302 which still works.

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.

gemini-code-assist

Code Review

This pull request enables an equality test for fragment shaders, which was previously disabled. The main change is in ImageFilter::equals, which now performs a deep comparison of DlImageFilter objects, considering their underlying properties like uniforms, instead of just comparing pointers. This fixes an issue where different ImageFilter instances with identical shader configurations were not considered equal. The implementation of ImageFilter::equals is updated to take Dart_Handles and perform a more robust check. A corresponding test in fragment_shader_test.dart is re-enabled for Impeller to verify this behavior.

My review includes suggestions to simplify the C++ equality check logic and improve parameter naming for clarity.

gemini-code-assist · 2026-01-10T00:16:01Z

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

+  if (a->filter_ == b->filter_) {
+    return true;
+  }
+  if (!a->filter_ || !b->filter_) {
+    return false;
+  }
+  return *a->filter_ == *b->filter_;


The equality check logic for filter_ can be simplified. After ensuring a and b are valid, you can check if both a->filter_ and b->filter_ are valid pointers. If they are, you can perform the deep comparison via *a->filter_ == *b->filter_. Otherwise, a simple shared_ptr comparison a->filter_ == b->filter_ correctly handles all other cases (both null, or one null).

if (a->filter_ && b->filter_) { return *a->filter_ == *b->filter_; } return a->filter_ == b->filter_;

gemini-code-assist · 2026-01-10T00:16:01Z

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

  void initComposeFilter(ImageFilter* outer, ImageFilter* inner);
  void initShader(ReusableFragmentShader* shader);
-  static bool equals(ImageFilter* a, ImageFilter* b);
+  static bool equals(Dart_Handle a, Dart_Handle b);


For clarity and consistency with the implementation in image_filter.cc, consider renaming the parameters to a_handle and b_handle to make it clear that they are Dart handles. This aligns with the Google C++ Style Guide's recommendation for descriptive parameter names.

Suggested change

static bool equals(Dart_Handle a, Dart_Handle b);

static bool equals(Dart_Handle a_handle, Dart_Handle b_handle);

References

The repository style guide specifies adherence to the Google C++ Style Guide. The guide recommends using descriptive names for function parameters. ^(link)

Seems like a good suggestion, especially because the implementation has other a and b variables.

gaaclarke · 2026-01-10T00:16:42Z

looks like @jason-simmons is reverting the base for this change, it will have to wait until that gets relanded to land

gaaclarke · 2026-01-10T00:56:10Z

reland happening in #180788

b-luk

LGTM

b-luk · 2026-01-10T01:49:33Z

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

  void initComposeFilter(ImageFilter* outer, ImageFilter* inner);
  void initShader(ReusableFragmentShader* shader);
-  static bool equals(ImageFilter* a, ImageFilter* b);
+  static bool equals(Dart_Handle a, Dart_Handle b);


Seems like a good suggestion, especially because the implementation has other a and b variables.

auto-submit · 2026-01-12T19:40:06Z

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

flutter/flutter@d81cd3e...793b0b8 2026-01-13 ahmedsameha1@gmail.com Make sure that a FormField doesn't crash at 0x0 environment (flutter/flutter#180810) 2026-01-13 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from VYeyMPe1lyCtlcl-V... to vTYb37OeUqZRxpiiP... (flutter/flutter#180880) 2026-01-13 ahmedsameha1@gmail.com Make sure that a DisplayFeatureSubScreen doesn't crash in 0x0 environ… (flutter/flutter#180357) 2026-01-13 ahmedsameha1@gmail.com Make sure that a DragTarget doesn't crash in 0x0 environment (flutter/flutter#180422) 2026-01-13 goderbauer@google.com Manually roll test dependencies (flutter/flutter#180886) 2026-01-12 engine-flutter-autoroll@skia.org Roll Skia from f70bcbf1b090 to 714d0af2eda7 (2 revisions) (flutter/flutter#180866) 2026-01-12 jason-simmons@users.noreply.github.com Roll libpng to version 1.6.53 (flutter/flutter#180712) 2026-01-12 30870216+gaaclarke@users.noreply.github.com Turn on fragment shader equality test (flutter/flutter#180784) 2026-01-12 116356835+AbdeMohlbi@users.noreply.github.com Update `PlatformPlugin` to not call `setStatusBarColor`, `setNavigationBarColor`, `setNavigationBarDividerColor` when disabled (flutter/flutter#180061) 2026-01-12 engine-flutter-autoroll@skia.org Roll Skia from a650ce2b0d50 to f70bcbf1b090 (1 revision) (flutter/flutter#180860) 2026-01-12 augustocesarperin@gmail.com Fix RawAutocomplete unmounted crash during async optionsBuilder (flutter/flutter#180824) 2026-01-12 116356835+AbdeMohlbi@users.noreply.github.com Fix `documentation member not recognized` because of missing import in `FlutterEngine.java` (flutter/flutter#180731) 2026-01-12 engine-flutter-autoroll@skia.org Roll Dart SDK from 42fd9ef68c1a to 34318de9874b (1 revision) (flutter/flutter#180854) 2026-01-12 gowsik.andro@gmail.com Add API sample and docs for Expansible widget (flutter/flutter#180273) 2026-01-12 engine-flutter-autoroll@skia.org Roll Skia from 487a9943210b to a650ce2b0d50 (2 revisions) (flutter/flutter#180849) 2026-01-12 116356835+AbdeMohlbi@users.noreply.github.com Improve code quality in `BinaryMessenger.java` (flutter/flutter#180733) 2026-01-12 30870216+gaaclarke@users.noreply.github.com Reland `Enabled some disabled impeller fragment shader dart tests` (flutter/flutter#180788) 2026-01-12 bkonyi@google.com [ Tool ] Fix `flutter run -d all` crash (flutter/flutter#180845) 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,tarrinneal@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

fixes flutter#180762 I manually tested against flutter#163302 which still works. ## 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

github-actions bot added the engine flutter/engine related. See also e: labels. label Jan 10, 2026

gaaclarke requested review from b-luk and jason-simmons January 10, 2026 00:15

gemini-code-assist bot reviewed Jan 10, 2026

View reviewed changes

b-luk approved these changes Jan 10, 2026

View reviewed changes

gaaclarke added 3 commits January 12, 2026 10:30

Renabled disabled fragment shader equality test.

94d683b

made the code do what the test said it should do and what makes sense

af783a6

gemini feedback

f14a9b6

gaaclarke force-pushed the turn-on-fragment-shader-equality-test branch from a8c8698 to f14a9b6 Compare January 12, 2026 18:33

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

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

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

auto-submit bot added this pull request to the merge queue Jan 12, 2026

Merged via the queue into flutter:master with commit 82d232d Jan 12, 2026
183 checks passed

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 12, 2026

engine-flutter-autoroll mentioned this pull request Jan 12, 2026

Manual roll Flutter from 3134be83dc81 to 82d232df03e3 (22 revisions) flutter/packages#10781

Closed

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

82d232df0 Turn on fragment shader equality test (flutter/flutter#180784)

272097d

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

82d232df0 Turn on fragment shader equality test (flutter/flutter#180784)

a5bc67c

engine-flutter-autoroll mentioned this pull request Jan 13, 2026

Roll Flutter from d81cd3e6f655 to 96fc83154d4d (14 revisions) flutter/packages#10783

Closed

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

82d232d Turn on fragment shader equality test (flutter/flutter#180784)

2a89f58

engine-flutter-autoroll mentioned this pull request Jan 13, 2026

Roll Flutter from d81cd3e6f655 to 793b0b89e82e (18 revisions) flutter/packages#10784

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Turn on fragment shader equality test #180784

Turn on fragment shader equality test #180784

gaaclarke commented Jan 10, 2026

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Jan 10, 2026

Uh oh!

gemini-code-assist bot Jan 10, 2026

Uh oh!

b-luk Jan 10, 2026

Uh oh!

gaaclarke Jan 12, 2026

Uh oh!

gaaclarke commented Jan 10, 2026

Uh oh!

gaaclarke commented Jan 10, 2026

Uh oh!

b-luk left a comment

Uh oh!

b-luk Jan 10, 2026

Uh oh!

auto-submit bot commented Jan 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	static bool equals(Dart_Handle a, Dart_Handle b);
	static bool equals(Dart_Handle a_handle, Dart_Handle b_handle);

Turn on fragment shader equality test #180784

Turn on fragment shader equality test #180784

Conversation

gaaclarke commented Jan 10, 2026

Pre-launch Checklist

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Jan 10, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 10, 2026

Choose a reason for hiding this comment

Uh oh!

b-luk Jan 10, 2026

Choose a reason for hiding this comment

Uh oh!

gaaclarke Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

gaaclarke commented Jan 10, 2026

Uh oh!

gaaclarke commented Jan 10, 2026

Uh oh!

b-luk left a comment

Choose a reason for hiding this comment

Uh oh!

b-luk Jan 10, 2026

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented Jan 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants