It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

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.

cc @matthew-carroll @xster

The difference is phenomenal!

I have the following problems:

1. How do we verify this is the closest we can get, or I mean, how do we document these magic numbers? If future readers don't know how these numbers are derived, then we can never improve them and are essentially marking them as "magic, don't touch".
2. How do we unit test them? According to my past experience on golden tests, golden tests do not apply the filters (at least the blurring is not applied).

1. I started with a straight saturation filter (from https://docs.rainmeter.net/tips/colormatrix-guide/), but spent a bit of time tweaking until it looked good. I'm not actually sure if we have all of the backdrop convolutions that would be needed to perfectly recreate a surface material, as it doesn't appear to just use a gaussian blur and color filter. However, this is pretty close.

static List<double> buildLightColorFilterMatrix() {
    const double lightLumR = 0.26;
    const double lightLumG = 0.4;
    const double lightLumB = 0.17;
    const double saturation = 2.0;
    const double sr = (1 - saturation) * lightLumR;
    const double sg = (1 - saturation) * lightLumG;
    const double sb = (1 - saturation) * lightLumB;
    return <double>[
      sr + saturation, sg, sb, 0.0, 0.0, //
      sr, sg + saturation, sb, 0.0, 0.0, //
      sr, sg, sb + saturation, 0.0, 0.0, //
      0.0, 0.0, 0.0, 1.0, 0.0, //
    ];
  }

 
  static List<double> buildDarkColorFilterMatrix() {
    const double additive = 0.3;
    const double darkLumR = 0.45;
    const double darkLumG = 0.8;
    const double darkLumB = 0.16;
    const double saturation = 1.7;
    const double sr = (1 - saturation) * darkLumR;
    const double sg = (1 - saturation) * darkLumG;
    const double sb = (1 - saturation) * darkLumB;
    return <double>[
      sr + saturation, sg, sb, 0.0, additive, //
      sr, sg + saturation, sb, 0.0, additive, //
      sr, sg, sb + saturation, 0.0, additive, //
      0.0, 0.0, 0.0, 1.0, 0.0, //
    ];
  }

2. I'm also not sure what the best way to unit test this would be. A basic test would just be looking up the BackdropFilter and printing out the image filter (e.g. ImageFilter.compose(source -> ColorFilter.matrix([1.74, -0.4, -0.17, 0.0, 0.0, -0.26, 1.6, -0.17, 0.0, 0.0, -0.26, -0.4, 1.83, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0]) -> blur(30.0, 30.0, clamp) -> result)). The color filter affects the appearance of the menu a bit more, so a golden may be more useful. I haven't done golden testing yet, so I need to look into it!

About unit test, my idea is:

1. Blurring is hard to test, and is technically not the target of this PR. So we're only testing the change of color through this filter.
2. With these matrices, I assume we can derive what a certain color will be converted to without going through the engine. If this is true...
3. We can set up a few tests, each testing what a certain background color will be converted to by these matrices. Denote them as A and B respectively, therefore B = M A.
4. The expected color (C) can be obtained by drawing a dialog over a solid background of A.
5. B and C will not match exactly, but we can note the true value C in the doc, and expect the distance between B and C to be less than a certain threshold.
6. For example, we pick 27 colors by having R, G, B each being 0, 127, 255, and calculate the average squared distance.

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

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.

Changes reported for pull request #151430 at sha 996d22f

About unit test, my idea is:

1. Blurring is hard to test, and is technically not the target of this PR. So we're only testing the change of color through this filter.
2. With these matrices, I assume we can derive what a certain color will be converted to without going through the engine. If this is true...

I'm not sure if this is easily done without reaching into the engine, but correct me if I'm wrong. ColorFilters appear to be composed and applied directly by skia (at least, as far as I can tell... I'm not too familiar with the native layer of Flutter):

https://api.skia.org/classSkColorFilters.html

I added a few Goldens to test the surface changes, but I'm interested to hear if there is a better way to handle this.

Golden file changes are available for triage from new commit, Click here to view.

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.

Changes reported for pull request #151430 at sha 4aa2544

I think the tests make sense, but here is what it looks like:

Unpainted:

Light:

I'm not sure if the light mode result has the color filter applied or it is just the translucent backdrop, but the unpainted one definitely isn't affected by anything at all. :/

Ok, I've found the reasons.

First, in "unpainted" tests, the two golden files are using the same name and the doc says that the theme shouldn't affect the result. This is incorrect, because the theme does affect the result, since the color filters are different.

Second, in "unpainted" tests, the filters isn't applied at all, because there's nothing within the filter and the layer is dropped. By adding an empty Container to the child, the result is obviously different:

While this shouldn't be a problem for production code (since there'll always be a child), I suggest making PopUpSurface.child required and explain the reason, and add an Container to the child in the test.

// popup_surface_test.dart, L100
            Padding(
              padding: const EdgeInsets.all(16.0),
              child: CupertinoPopupSurface(
                isSurfacePainted: isSurfacePainted,
                child: child,
              ),
            ),

Third, as I pointed in the comment, I wonder why a 3rd layer of 18 strips is painted on top of the pop up surface. Is it used as the master for comparison? I don't think this is very useful though. Without it the result is very obvious and beautiful:

My suggestion: I thought the backdrop filters don't apply to golden images, but it turns out they do. In this case, I still think most of the test cases should be used to test the color change (since that's the most complicated part), and these test should be done without the blurring effect for accuracy, while some separate tests should be used to verify the blurring effect.

• CupertinoPopUpSurface should allow customizing the blurring radius. This is only used for unit tests as below.
• The current unit tests that verify the color change for both themes are great, but they should use a blurring radius of 0, should not have the "top layer" of strips, and there can be a larger variety of colors.
• I suggest we remove the unpainted tests, because in practice the pop up surface is only used with the correct background. If the surface is unpainted, then it's the children's responsibility to paint the correct background afterwards, otherwise the behavior is not guaranteed.
• One or a few additional tests should be used to verify the blurring effect. We'll probably want it to be just black and white to avoid the effect of colors.

Golden file changes are available for triage from new commit, Click here to view.

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.

Changes reported for pull request #151430 at sha 718071e

This should now be updated with skip annotations, i.e.

skip: kIsWasm, // https://github.com/flutter/flutter/issues/152026

I also updated a few tests to make them more specific based on the testing guidelines -- I think I had skipped one of the testing documents from the contributing guidelines, so sorry for the late update! Largest difference was splitting 'Nonpositive blurSigma removes blur' into 'Setting blurSigma to a negative number throws' and 'Setting blurSigma to zero removes blur'.

Thank you for your update! Being unsupported by skwasm is unfortunate but it'll probably be solved in the future and doesn't reduce the value of this PR. I found a fix to the doc and otherwise we're ready to go.

Look perfect :)

I've created an issue to remind us the skwasm issue. #154698