ColorFilter.saturation #167898

lukepighetti · 2025-04-27T20:32:47Z

I'm not sure how to update the Goldens but here are the three I have locally. Test was written to ensure that transparency is preserved.

golden files

ColorFilter.saturation(1)

ColorFilter.saturation(0.5)

ColorFilter.saturation(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.

flutter-dashboard · 2025-04-28T13:40:02Z

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

lukepighetti · 2025-04-28T13:40:30Z

trying to understand the intricacies of what parts of the codebase need to be updated. builds are failing on web and it's not clear to me what the next steps are. will keep trying to apply pressure on this

benthillerkus · 2025-04-28T14:55:21Z

There's also this file where a test could be added:
https://github.com/lukepighetti/flutter/blob/1e29389a7c1b0456feadf74914fb6de05e34e048/engine/src/flutter/testing/dart/color_filter_test.dart

Unfortunately it's in the engine and running the test isn't as easy as pressing the run button in VSCode (and setting everything up correctly seems really complicated), so I can' confirm, but I think this would do:

/// [red] with a 0.5 saturation adjustment layer in Affinity Photo
// Might not match up with the result of the matrix calc exactly
const int redHalfSaturation = 0xFF853F2D;

....

  group('ColorFilter - saturation', () {
    test('saturation - 0', () async {
      final Paint paint =
          Paint()
            ..color = green
            ..colorFilter = const ColorFilter.saturation(0);
  
      Uint32List bytes = await getBytesForPaint(paint);
      expect(bytes[0], greenGreyscaled);
  
      // TODO(135699): enable this
      if (impellerEnabled) {
        return;
      }
  
      paint.invertColors = true;
      bytes = await getBytesForPaint(paint);
      expect(bytes[0], greenInvertedGreyscaled);
    });
  
    test('saturation - 1 - transparent', () async {
      final Paint paint =
          Paint()
            ..color = transparent
            ..colorFilter = const ColorFilter.saturation(1);
  
      Uint32List bytes = await getBytesForPaint(paint);
      expect(bytes[0], transparent);
    });
  
    test('saturation - 1 - green', () async {
      final Paint paint =
          Paint()
            ..color = green
            ..colorFilter = const ColorFilter.saturation(1);
  
      Uint32List bytes = await getBytesForPaint(paint);
      expect(bytes[0], green);
    });
  
    test('saturation - 1 - red', () async {
      final Paint paint =
          Paint()
            ..color = red
            ..colorFilter = const ColorFilter.saturation(1);
  
      Uint32List bytes = await getBytesForPaint(paint);
      expect(bytes[0], red);
    });
  
    test('saturation - 0.5 - red', () async {
      final Paint paint =
          Paint()
            ..color = red
            ..colorFilter = const ColorFilter.saturation(0.5);
  
      Uint32List bytes = await getBytesForPaint(paint);
      expect(bytes[0], redHalfSaturation);
    });
  });

benthillerkus · 2025-04-28T15:32:37Z

trying to understand the intricacies of what parts of the codebase need to be updated. builds are failing on web and it's not clear to me what the next steps are. will keep trying to apply pressure on this

https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8716341066034125713/+/u/build_wasm_release_flutter_web_sdk:flutter_web_sdk_archive/stdout?format=raw

org-dartlang-sdk:///lib/ui/painting.dart:446:61: Error: Couldn't find constructor 'engine.EngineColorFilter.saturation'.
  const factory ColorFilter.saturation(double saturation) = engine.EngineColorFilter.saturation;
                                                            ^
org-dartlang-sdk:///lib/ui/painting.dart:446:61: Error: Redirection constructor target not found: 'engine.EngineColorFilter.saturation'
  const factory ColorFilter.saturation(double saturation) = engine.EngineColorFilter.saturation;

This is what's failing which is fair, because there is indeed no such thing https://github.com/lukepighetti/flutter/blob/1e29389a7c1b0456feadf74914fb6de05e34e048/engine/src/flutter/lib/web_ui/lib/src/engine/color_filter.dart#L110

But this class is really curious, it seems to be a copy paste of the section from ui/painting.dart, just with the internal fields made public.
Since the matrix is hidden on the ColorFilter, we can't easily just pass it into the EngineColorFilter.matrix factory, so the .saturation factory would probably have to be copy pasted 1:1 to the EngineColorFilter?

Piinks

Thanks for sending a PR! 🎊
See comment below to get this working on web.

Piinks · 2025-04-28T17:19:55Z

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

  const factory ColorFilter.matrix(List<double> matrix) = engine.EngineColorFilter.matrix;
  const factory ColorFilter.linearToSrgbGamma() = engine.EngineColorFilter.linearToSrgbGamma;
  const factory ColorFilter.srgbToLinearGamma() = engine.EngineColorFilter.srgbToLinearGamma;
+  const factory ColorFilter.saturation(double saturation) = engine.EngineColorFilter.saturation;


Would be great if Github did not lose said comment. 😅
See engine/src/flutter/lib/web_ui/lib/src/engine/color_filter.dart
You need to define EngineColorFilter.saturation
Search for EngineColorFilter.srgbToLinearGamma as an example.

Piinks · 2025-04-28T17:23:46Z

I'm not sure how to update the Goldens

Once CI is green, the bot will check for golden file changes and leave a comment to the dashboard where we can check them out. 👍

Piinks · 2025-07-08T22:57:06Z

Hey @lukepighetti! Greetings from stale PR triage 👋
Is this one still on your radar?

Piinks · 2025-09-15T22:01:04Z

Unfortunately since we have not heard back we'll have to regrettably close this PR. I have marked the issue with a the partial patch label in case anyone wants to pick this up and address the feedback. Thanks!

@lukepighetti

@lukepighetti

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. engine flutter/engine related. See also e: labels. labels Apr 27, 2025

ColorFilter.saturation()

081ab97

lukepighetti force-pushed the lukepighetti/color-filter-apr-27 branch from 28ae2f1 to 081ab97 Compare April 27, 2025 23:30

lukepighetti marked this pull request as ready for review April 27, 2025 23:34

lukepighetti mentioned this pull request Apr 27, 2025

[FR] Add Desaturate widget #166589

Closed

lukepighetti changed the title ~~WIP: ColorFilter.saturation~~ ColorFilter.saturation Apr 27, 2025

lukepighetti added 2 commits April 28, 2025 06:22

analyzer nits

9d3b7b9

web_ui constructor

1e29389

github-actions bot added the platform-web Web applications specifically label Apr 28, 2025

lukepighetti marked this pull request as draft April 28, 2025 13:39

Piinks reviewed Apr 28, 2025

View reviewed changes

Piinks closed this Sep 15, 2025

ksokolovskyi mentioned this pull request Oct 3, 2025

Add saturation ColorFilter. #176464

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

ColorFilter.saturation #167898

ColorFilter.saturation #167898

Uh oh!

lukepighetti commented Apr 27, 2025 •

edited

Loading

Uh oh!

flutter-dashboard bot commented Apr 28, 2025

Uh oh!

lukepighetti commented Apr 28, 2025

Uh oh!

benthillerkus commented Apr 28, 2025

Uh oh!

benthillerkus commented Apr 28, 2025

Uh oh!

Piinks left a comment

Uh oh!

Piinks Apr 28, 2025

Uh oh!

Piinks commented Apr 28, 2025

Uh oh!

Piinks commented Jul 8, 2025

Uh oh!

Piinks commented Sep 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ColorFilter.saturation #167898

ColorFilter.saturation #167898

Uh oh!

Conversation

lukepighetti commented Apr 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

flutter-dashboard bot commented Apr 28, 2025

Uh oh!

lukepighetti commented Apr 28, 2025

Uh oh!

benthillerkus commented Apr 28, 2025

Uh oh!

benthillerkus commented Apr 28, 2025

Uh oh!

Piinks left a comment

Choose a reason for hiding this comment

Uh oh!

Piinks Apr 28, 2025

Choose a reason for hiding this comment

Uh oh!

Piinks commented Apr 28, 2025

Uh oh!

Piinks commented Jul 8, 2025

Uh oh!

Piinks commented Sep 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lukepighetti commented Apr 27, 2025 •

edited

Loading