(Test-only) Add tests for new `ImageFilter.dilate`/`ImageFilter.erode` in flutter engine #101036

fzyzcjy · 2022-03-30T10:48:14Z

Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.
Please see #100830

List which issues are fixed by this PR. You must list at least one issue.
#100830

Related: flutter/engine#32334

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

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.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

fzyzcjy · 2022-03-30T10:49:42Z

This should fail until flutter/engine#32334 is merged and flutter/flutter repo uses the latest flutter/engine.

fzyzcjy · 2022-04-06T01:43:57Z

We know dilate/erode is not implemented for the web platform, just like the compose filter. So what should I do (skip tests when in web?)?

══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following UnimplementedError was thrown running a test:
ImageFilter.dilate not implemented for web platform.

When the exception was thrown, this was the stack:
../dart-sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/errors.dart 251:49  throw_
../lib/ui/painting.dart 411:5                                                    dilate
image_filter_test.dart.js 428:133                                                <fn>
../dart-sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart 84:54            runBody
../dart-sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart 123:5            _async
image_filter_test.dart.js 427:80                                                 <fn>
../packages/flutter_test/src/matchers.dart.js 4522:19                            <fn>

fzyzcjy · 2022-04-06T01:44:29Z

/cc @dnfield

dnfield · 2022-04-06T03:19:14Z

Skip these on web with a reference to the bug filed to implement them for web.

fzyzcjy · 2022-04-06T03:56:01Z

Thanks, I will do that.

goderbauer · 2022-04-13T21:22:58Z

@fzyzcjy Can you update the tests per the suggestion above to make the checks happy?

fzyzcjy · 2022-04-14T00:10:32Z

Sure. I forgot it

flutter-dashboard · 2022-04-14T01:07:52Z

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 #101036 at sha c39d7b8

goderbauer · 2022-04-20T21:34:51Z

@fzyzcjy Can you take a look at the golden images linked in the previous comment to confirm that that's what you expected them to look like? One of the images looks empty, which seems odd...

fzyzcjy · 2022-04-21T00:23:06Z

@goderbauer I guess it is right, just b/c it erodes too much. I have updated the test so it will be less strange.

flutter-dashboard · 2022-04-21T01:04:39Z

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 #101036 at sha e3d3659

fzyzcjy · 2022-04-21T03:10:40Z

The gold looks better. (Notice it has very very thin lines)

goderbauer · 2022-04-21T16:06:14Z

Thanks, @fzyzcjy

@dnfield or @flar could you review this one (and also approve the goldens) since you reviewed the upstream PR in the engine?

flar · 2022-04-21T18:45:50Z

packages/flutter/test/widgets/image_filter_test.dart

+        child: ImageFiltered(
+          // Do not erode too much, otherwise we will see nothing left.
+          imageFilter: ImageFilter.erode(radiusX: 1.0, radiusY: 1.0),
+          child: const Placeholder(),


The place holder by default draws a box with diagonals with a stroke width of 2 and doesn't survive the erode process. Try increasing the stroke width on both to a reasonable size so it doesn't erode completely for the golden image.

Actually, the dilate filter shows up fine in its golden, it's definitely more than 2 pixels wide, but the erode filter creates a blank image.

stroke width modified.

flar · 2022-04-22T01:05:42Z

Thanks, @fzyzcjy

@dnfield or @flar could you review this one (and also approve the goldens) since you reviewed the upstream PR in the engine?

I still don't see anything rendered in the erode test even if I click on it to see it full size. Am I clicking on the wrong link?

Actually, it looks like I was reviewing the goldens from before I requested a larger stroke width. Maybe they haven't been updated yet?

fzyzcjy · 2022-04-22T01:09:51Z

I still don't see anything rendered in the erode test even if I click on it to see it full size. Am I clicking on the wrong link?
Actually, it looks like I was reviewing the goldens from before I requested a larger stroke width. Maybe they haven't been updated yet?

I guess the CI is even not executed after modifying strokewidth...

fzyzcjy · 2022-04-22T01:10:38Z

I guess the failure (and seems skipped executing all tests related to golden) is not caused by me. So maybe need to wait

flar · 2022-04-22T01:24:12Z

I guess the failure (and seems skipped executing all tests related to golden) is not caused by me. So maybe need to wait

I mentioned it on the infra Discord chat to see if someone can poke it.

flar · 2022-04-22T01:24:48Z

I would just LGTM to let it merge, but I'm worried it may merge with bad goldens and cause problems down the line.

fzyzcjy · 2022-04-22T01:52:23Z

Take your time. Since this is a test-only PR I am not that urgent :)

flar · 2022-04-22T21:54:20Z

It looks like you need to rebase in order to get the ci.yaml to gel with the latest CI recipes and then hopefully final goldens will be generated.

flutter-dashboard · 2022-04-23T00:25:28Z

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 #101036 at sha 692b271

fzyzcjy · 2022-04-23T00:27:08Z

Now seems better

flar

LGTM

goderbauer

LGTM

…er.erode` in flutter engine (flutter/flutter#101036)

darrenaustin · 2022-05-03T23:41:44Z

Where the goldens for this approved? It looks like this is breaking the build:

https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8815144942697521329/+/u/run_test.dart_for_framework_tests_shard_and_subshard_widgets/test_stdout

The following _Exception was thrown while running async test code:
Exception: Skia Gold received an unapproved image in post-submit
testing. Golden file images in flutter/flutter are triaged
in pre-submit during code review for the given PR.

Visit https://flutter-gold.skia.org// to view and approve
the image(s), or revert the associated change. For more
information, visit the wiki:
https://github.com/flutter/flutter/wiki/Writing-a-golden-file-test-for-package:flutter

Debug information for Gold:
stdout: Given image with hash 94028cfacbe41a55af47edcddeff6540 for test widgets.image_filter_erode
Untriaged or negative image:
https://flutter-gold.skia.org/detail?test=widgets.image_filter_erode&digest=94028cfacbe41a55af47edcddeff6540


stderr: Test: widgets.image_filter_erode FAIL

When the exception was thrown, this was the stack:
#0      SkiaGoldClient.imgtestAdd (package:flutter_goldens_client/skia_client.dart:216:7)
<asynchronous suspension>
<asynchronous suspension>
(elided one frame from package:stack_trace)

Can we just approve the new goldens? If not I will need to revert this to get the tree green again.

fzyzcjy · 2022-05-04T00:46:54Z

I do not have permission to approve the goldens, but seems that @flar has said that the goldens are ok, and he may have permission to approve

darrenaustin · 2022-05-04T01:20:57Z

It looks like it has been resolved and the tree is green again, so it's all good now.

flar · 2022-05-04T06:55:33Z

I did approve the goldens, so I'm not sure what happened.

…` in flutter engine (flutter#101036)

add tests for dilate/erode

bce07cf

flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label Mar 30, 2022

fzyzcjy mentioned this pull request Mar 30, 2022

Proposal to Create ImageFilter.dilate/ImageFilter.erode #100830

Closed

fzyzcjy marked this pull request as draft March 30, 2022 10:49

fzyzcjy marked this pull request as ready for review April 4, 2022 23:39

fzyzcjy added 2 commits April 5, 2022 07:40

Merge branch 'flutter:master' into flutter_image_filter

7fbabc2

Merge branch 'flutter:master' into flutter_image_filter

cba5e1b

fzyzcjy changed the title ~~Add tests for new ImageFilter.dilate/ImageFilter.erode in flutter engine~~ (Test-onlyu) Add tests for new ImageFilter.dilate/ImageFilter.erode in flutter engine Apr 6, 2022

fzyzcjy changed the title ~~(Test-onlyu) Add tests for new ImageFilter.dilate/ImageFilter.erode in flutter engine~~ (Test-only) Add tests for new ImageFilter.dilate/ImageFilter.erode in flutter engine Apr 6, 2022

Merge branch 'flutter:master' into flutter_image_filter

226ca65

fzyzcjy mentioned this pull request Apr 14, 2022

ImageFilter.dilate/ImageFilter.erode not supported on the web. #101874

Closed

fzyzcjy added 2 commits April 14, 2022 08:12

Update image_filter_test.dart

264b33e

Update image_filter_test.dart

c39d7b8

flutter-dashboard bot added the will affect goldens Changes to golden files label Apr 14, 2022

Update image_filter_test.dart

e3d3659

goderbauer requested review from dnfield and flar April 21, 2022 16:06

flar requested changes Apr 21, 2022

View reviewed changes

Update image_filter_test.dart

c0f18f4

Merge branch 'flutter:master' into flutter_image_filter

692b271

goderbauer requested a review from flar April 27, 2022 21:50

flar approved these changes May 3, 2022

View reviewed changes

goderbauer approved these changes May 3, 2022

View reviewed changes

goderbauer added the waiting for tree to go green label May 3, 2022

fluttergithubbot merged commit 896e5b3 into flutter:master May 3, 2022

engine-flutter-autoroll mentioned this pull request May 3, 2022

Roll Flutter from b4f8d7c79296 to 896e5b332d36 (1 revision) flutter/packages#1737

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 3, 2022

896e5b3 (Test-only) Add tests for new ImageFilter.dilate/`ImageFilt…

e611798

…er.erode` in flutter engine (flutter/flutter#101036)

engine-flutter-autoroll mentioned this pull request May 3, 2022

Roll Flutter from b4f8d7c79296 to 896e5b332d36 (1 revision) flutter/plugins#5611

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request May 3, 2022

896e5b3 (Test-only) Add tests for new ImageFilter.dilate/`ImageFilt…

0fbd49b

…er.erode` in flutter engine (flutter/flutter#101036)

egramond pushed a commit to egramond/flutter that referenced this pull request May 5, 2022

(Test-only) Add tests for new ImageFilter.dilate/`ImageFilter.erode…

8129527

…` in flutter engine (flutter#101036)

Piinks mentioned this pull request May 6, 2022

Windows framework_tests_widgets is 2.00% flaky #103063

Closed

fzyzcjy mentioned this pull request May 13, 2022

for updateGoldens, give a GUI to check golden files git diff fzyzcjy/flutter_convenient_test#201

Closed

(Test-only) Add tests for new ImageFilter.dilate/ImageFilter.erode in flutter engine #101036

(Test-only) Add tests for new ImageFilter.dilate/ImageFilter.erode in flutter engine #101036

Uh oh!

Conversation

fzyzcjy commented Mar 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

fzyzcjy commented Mar 30, 2022

Uh oh!

fzyzcjy commented Apr 6, 2022

Uh oh!

fzyzcjy commented Apr 6, 2022

Uh oh!

dnfield commented Apr 6, 2022

Uh oh!

fzyzcjy commented Apr 6, 2022

Uh oh!

goderbauer commented Apr 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fzyzcjy commented Apr 14, 2022

Uh oh!

flutter-dashboard bot commented Apr 14, 2022

Uh oh!

goderbauer commented Apr 20, 2022

Uh oh!

fzyzcjy commented Apr 21, 2022

Uh oh!

flutter-dashboard bot commented Apr 21, 2022

Uh oh!

fzyzcjy commented Apr 21, 2022

Uh oh!

goderbauer commented Apr 21, 2022

Uh oh!

flar Apr 21, 2022

Choose a reason for hiding this comment

Uh oh!

flar Apr 21, 2022

Choose a reason for hiding this comment

Uh oh!

fzyzcjy Apr 22, 2022

Choose a reason for hiding this comment

Uh oh!

flar commented Apr 22, 2022

Uh oh!

fzyzcjy commented Apr 22, 2022

Uh oh!

fzyzcjy commented Apr 22, 2022

Uh oh!

flar commented Apr 22, 2022

Uh oh!

flar commented Apr 22, 2022

Uh oh!

fzyzcjy commented Apr 22, 2022

Uh oh!

flar commented Apr 22, 2022

Uh oh!

flutter-dashboard bot commented Apr 23, 2022

Uh oh!

fzyzcjy commented Apr 23, 2022

Uh oh!

flar left a comment

Choose a reason for hiding this comment

Uh oh!

goderbauer left a comment

Choose a reason for hiding this comment

Uh oh!

darrenaustin commented May 3, 2022

Uh oh!

fzyzcjy commented May 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

darrenaustin commented May 4, 2022

Uh oh!

flar commented May 4, 2022

Uh oh!

Reviewers

Assignees

Labels

(Test-only) Add tests for new `ImageFilter.dilate`/`ImageFilter.erode` in flutter engine #101036

(Test-only) Add tests for new `ImageFilter.dilate`/`ImageFilter.erode` in flutter engine #101036

fzyzcjy commented Mar 30, 2022 •

edited

Loading

goderbauer commented Apr 13, 2022 •

edited

Loading

fzyzcjy commented May 4, 2022 •

edited

Loading