(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:55Z

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

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

add tests for dilate/erode

bce07cf

flutter-dashboard bot added the framework label Mar 30, 2022

fzyzcjy mentioned this pull request Mar 30, 2022

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

Open

fzyzcjy marked this pull request as draft Mar 30, 2022

fzyzcjy marked this pull request as ready for review Apr 4, 2022

fzyzcjy added 2 commits Apr 4, 2022

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 Apr 14, 2022

Update image_filter_test.dart

264b33e

Update image_filter_test.dart

c39d7b8

flutter-dashboard bot added the will affect goldens label Apr 14, 2022

Update image_filter_test.dart

e3d3659

goderbauer requested review from flar and dnfield Apr 21, 2022

flar requested changes Apr 21, 2022

View changes

Update image_filter_test.dart

c0f18f4

Merge branch 'flutter:master' into flutter_image_filter

692b271

goderbauer requested a review from flar Apr 27, 2022

flar approved these changes May 3, 2022

View changes

goderbauer approved these changes May 3, 2022

View changes

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

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

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

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

Open

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

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

e611798

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

Apr	MAY	Jun
	03
2021	2022	2023

flutter / flutter Public

(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

fzyzcjy commented Mar 30, 2022

fzyzcjy commented Apr 6, 2022

fzyzcjy commented Apr 6, 2022

dnfield commented Apr 6, 2022

fzyzcjy commented Apr 6, 2022

goderbauer commented Apr 13, 2022 •

edited

fzyzcjy commented Apr 14, 2022

flutter-dashboard bot commented Apr 14, 2022

goderbauer commented Apr 20, 2022

fzyzcjy commented Apr 21, 2022

flutter-dashboard bot commented Apr 21, 2022

fzyzcjy commented Apr 21, 2022

goderbauer commented Apr 21, 2022

flar Apr 21, 2022

flar Apr 21, 2022

fzyzcjy Apr 22, 2022

flar commented Apr 22, 2022

fzyzcjy commented Apr 22, 2022

fzyzcjy commented Apr 22, 2022

flar commented Apr 22, 2022

flar commented Apr 22, 2022

fzyzcjy commented Apr 22, 2022

flar commented Apr 22, 2022

flutter-dashboard bot commented Apr 23, 2022

fzyzcjy commented Apr 23, 2022

flar left a comment

goderbauer left a comment

flutter / flutter Public

(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

Conversation

fzyzcjy commented Mar 30, 2022 • edited

Pre-launch Checklist

fzyzcjy commented Mar 30, 2022

fzyzcjy commented Apr 6, 2022

fzyzcjy commented Apr 6, 2022

dnfield commented Apr 6, 2022

fzyzcjy commented Apr 6, 2022

goderbauer commented Apr 13, 2022 • edited

fzyzcjy commented Apr 14, 2022

flutter-dashboard bot commented Apr 14, 2022

goderbauer commented Apr 20, 2022

fzyzcjy commented Apr 21, 2022

flutter-dashboard bot commented Apr 21, 2022

fzyzcjy commented Apr 21, 2022

goderbauer commented Apr 21, 2022

flar Apr 21, 2022

Choose a reason for hiding this comment

flar Apr 21, 2022

Choose a reason for hiding this comment

fzyzcjy Apr 22, 2022

Choose a reason for hiding this comment

flar commented Apr 22, 2022

fzyzcjy commented Apr 22, 2022

fzyzcjy commented Apr 22, 2022

flar commented Apr 22, 2022

flar commented Apr 22, 2022

fzyzcjy commented Apr 22, 2022

flar commented Apr 22, 2022

flutter-dashboard bot commented Apr 23, 2022

fzyzcjy commented Apr 23, 2022

flar left a comment

goderbauer left a comment

(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

goderbauer commented Apr 13, 2022 •

edited