Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[canvaskit] Do not double-apply ImageFilter transform to children #46336

Merged

Conversation

harryterkelsen
Copy link
Contributor

Fixes an issue seen in the "widgets.image_filter_matrix" golden here: https://flutter-gold.skia.org/search?crs=github&issue=135249&patchsets=3

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 and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

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

@flutter-dashboard
Copy link

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 Hixie or stuartmorgan on 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.

@github-actions github-actions bot added the platform-web Code specifically for the web engine label Sep 27, 2023
final Matrix4 childMatrix = Matrix4.copy(matrix);
childMatrix.translate(_offset.dx, _offset.dy);
prerollContext.mutatorsStack
.pushTransform(Matrix4.translationValues(_offset.dx, _offset.dy, 0.0));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May need to add a golden test if this change doesn't trigger an existing one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. PTAL

Copy link
Contributor

@yjbanov yjbanov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@harryterkelsen harryterkelsen merged commit 1d2fd46 into flutter:main Sep 28, 2023
24 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 28, 2023
zanderso pushed a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 28, 2023
zanderso pushed a commit to flutter/flutter that referenced this pull request Sep 28, 2023
…sions) (#135637)

Manual roll Flutter Engine from f70f65f7a622 to be32dcc9117a (31
revisions)

Manual roll requested by jacksongardner@google.com

Cannot build log URL because revision "be32dcc9117a" is invalid: Luci
builds of "Linux linux_unopt" for
be32dcc9117a462760cb52c18d0d050f067f2463 was STARTED

2023-09-28 jacksongardner@google.com Rollback Dart SDK to 3.2.0-119.
(flutter/engine#46339)
2023-09-28 1961493+harryterkelsen@users.noreply.github.com [canvaskit]
Do not double-apply ImageFilter transform to children
(flutter/engine#46336)
2023-09-28 ychris@google.com Reland "Reverts "[ios] Fix app extension
not able to find assets from… (flutter/engine#46329)
2023-09-27 47866232+chunhtai@users.noreply.github.com Revert "[Impeller]
Fixes stroke path geometry that can draw outside o…
(flutter/engine#46334)
2023-09-27 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from
jQHACBU4Fi2wMElkm... to U334SygIkffMJVmdu... (flutter/engine#46333)
2023-09-27 skia-flutter-autoroll@skia.org Roll Skia from 952e8dd66560 to
b048b468d641 (1 revision) (flutter/engine#46332)
2023-09-27 jason-simmons@users.noreply.github.com [Impeller] Destroy all
per-thread command pools tied to a context before deleting the context
(flutter/engine#46286)
2023-09-27 skia-flutter-autoroll@skia.org Roll Skia from d78aba2524b3 to
952e8dd66560 (1 revision) (flutter/engine#46331)
2023-09-27 jonahwilliams@google.com [Impeller] Match Skia gradient
clamping behavior (and document). (flutter/engine#44825)
2023-09-27 skia-flutter-autoroll@skia.org Roll Dart SDK from
97647bb1666b to 80a965ee48ab (1 revision) (flutter/engine#46330)
2023-09-27 skia-flutter-autoroll@skia.org Roll Skia from ff5474eed6b4 to
d78aba2524b3 (3 revisions) (flutter/engine#46327)
2023-09-27 98614782+auto-submit[bot]@users.noreply.github.com Reverts
"[ios] Fix app extension not able to find assets from unloaded bundle"
(flutter/engine#46328)
2023-09-27 737941+loic-sharma@users.noreply.github.com [Windows] Improve
logic to update swap intervals (flutter/engine#46172)
2023-09-27 jonahwilliams@google.com [Impeller] Fallback to no index
buffer when tesselation count is large, split up nonZero contours.
(flutter/engine#46282)
2023-09-27 skia-flutter-autoroll@skia.org Roll Skia from 4731ccd6342c to
ff5474eed6b4 (1 revision) (flutter/engine#46325)
2023-09-27 jason-simmons@users.noreply.github.com Declare native wrapper
classes in Fuchsia packages as base classes (flutter/engine#46305)
2023-09-27 godofredoc@google.com Remove fuchsia from recipes cq.
(flutter/engine#46324)
2023-09-27 skia-flutter-autoroll@skia.org Roll Dart SDK from
c2a455113e39 to 97647bb1666b (1 revision) (flutter/engine#46323)
2023-09-27 matej.knopp@gmail.com [macOS] Synchronise modifiers from
mouse events for RawKeyboard (flutter/engine#46230)
2023-09-27 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from
Lg6FR6iDnZeV6y-E8... to 6Y22MutFhgL7ua18F... (flutter/engine#46322)
2023-09-27 skia-flutter-autoroll@skia.org Roll Skia from 2991bb799d3f to
4731ccd6342c (1 revision) (flutter/engine#46321)
2023-09-27 jason-simmons@users.noreply.github.com Declare native wrapper
classes in the GPU package as base classes (flutter/engine#46304)
2023-09-27 jason-simmons@users.noreply.github.com Do not call
DrawTextBlob for performance overlay text when using Impeller
(flutter/engine#46307)
2023-09-27 kjlubick@users.noreply.github.com Update to use
GrDirectContexts::MakeGL (flutter/engine#46308)
2023-09-27 skia-flutter-autoroll@skia.org Roll Skia from 0f4f31127ac5 to
2991bb799d3f (1 revision) (flutter/engine#46319)
2023-09-27 skia-flutter-autoroll@skia.org Roll Skia from fc629215398f to
0f4f31127ac5 (1 revision) (flutter/engine#46318)
2023-09-27 skia-flutter-autoroll@skia.org Roll Dart SDK from
6c4eb86ecd25 to c2a455113e39 (1 revision) (flutter/engine#46317)
2023-09-27 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from
OMrTgAfDg9PKXTzq0... to jQHACBU4Fi2wMElkm... (flutter/engine#46314)
2023-09-27 skia-flutter-autoroll@skia.org Roll Skia from 960325c13009 to
fc629215398f (1 revision) (flutter/engine#46313)
2023-09-27 skia-flutter-autoroll@skia.org Roll Skia from 76aecbaea259 to
960325c13009 (2 revisions) (flutter/engine#46311)
2023-09-27 skia-flutter-autoroll@skia.org Roll Dart SDK from
7c3588c05f87 to 6c4eb86ecd25 (3 revisions) (flutter/engine#46310)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from Lg6FR6iDnZeV to 6Y22MutFhgL7
  fuchsia/sdk/core/mac-amd64 from OMrTgAfDg9PK to U334SygIkffM

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC jacksongardner@google.com,rmistry@google.com,zra@google.com on
the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter:
https://github.com/flutter/flutter/issues/new/choose

...
Mairramer pushed a commit to Mairramer/flutter that referenced this pull request Oct 10, 2023
…sions) (flutter#135637)

Manual roll Flutter Engine from f70f65f7a622 to be32dcc9117a (31
revisions)

Manual roll requested by jacksongardner@google.com

Cannot build log URL because revision "be32dcc9117a" is invalid: Luci
builds of "Linux linux_unopt" for
be32dcc9117a462760cb52c18d0d050f067f2463 was STARTED

2023-09-28 jacksongardner@google.com Rollback Dart SDK to 3.2.0-119.
(flutter/engine#46339)
2023-09-28 1961493+harryterkelsen@users.noreply.github.com [canvaskit]
Do not double-apply ImageFilter transform to children
(flutter/engine#46336)
2023-09-28 ychris@google.com Reland "Reverts "[ios] Fix app extension
not able to find assets from… (flutter/engine#46329)
2023-09-27 47866232+chunhtai@users.noreply.github.com Revert "[Impeller]
Fixes stroke path geometry that can draw outside o…
(flutter/engine#46334)
2023-09-27 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from
jQHACBU4Fi2wMElkm... to U334SygIkffMJVmdu... (flutter/engine#46333)
2023-09-27 skia-flutter-autoroll@skia.org Roll Skia from 952e8dd66560 to
b048b468d641 (1 revision) (flutter/engine#46332)
2023-09-27 jason-simmons@users.noreply.github.com [Impeller] Destroy all
per-thread command pools tied to a context before deleting the context
(flutter/engine#46286)
2023-09-27 skia-flutter-autoroll@skia.org Roll Skia from d78aba2524b3 to
952e8dd66560 (1 revision) (flutter/engine#46331)
2023-09-27 jonahwilliams@google.com [Impeller] Match Skia gradient
clamping behavior (and document). (flutter/engine#44825)
2023-09-27 skia-flutter-autoroll@skia.org Roll Dart SDK from
97647bb1666b to 80a965ee48ab (1 revision) (flutter/engine#46330)
2023-09-27 skia-flutter-autoroll@skia.org Roll Skia from ff5474eed6b4 to
d78aba2524b3 (3 revisions) (flutter/engine#46327)
2023-09-27 98614782+auto-submit[bot]@users.noreply.github.com Reverts
"[ios] Fix app extension not able to find assets from unloaded bundle"
(flutter/engine#46328)
2023-09-27 737941+loic-sharma@users.noreply.github.com [Windows] Improve
logic to update swap intervals (flutter/engine#46172)
2023-09-27 jonahwilliams@google.com [Impeller] Fallback to no index
buffer when tesselation count is large, split up nonZero contours.
(flutter/engine#46282)
2023-09-27 skia-flutter-autoroll@skia.org Roll Skia from 4731ccd6342c to
ff5474eed6b4 (1 revision) (flutter/engine#46325)
2023-09-27 jason-simmons@users.noreply.github.com Declare native wrapper
classes in Fuchsia packages as base classes (flutter/engine#46305)
2023-09-27 godofredoc@google.com Remove fuchsia from recipes cq.
(flutter/engine#46324)
2023-09-27 skia-flutter-autoroll@skia.org Roll Dart SDK from
c2a455113e39 to 97647bb1666b (1 revision) (flutter/engine#46323)
2023-09-27 matej.knopp@gmail.com [macOS] Synchronise modifiers from
mouse events for RawKeyboard (flutter/engine#46230)
2023-09-27 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from
Lg6FR6iDnZeV6y-E8... to 6Y22MutFhgL7ua18F... (flutter/engine#46322)
2023-09-27 skia-flutter-autoroll@skia.org Roll Skia from 2991bb799d3f to
4731ccd6342c (1 revision) (flutter/engine#46321)
2023-09-27 jason-simmons@users.noreply.github.com Declare native wrapper
classes in the GPU package as base classes (flutter/engine#46304)
2023-09-27 jason-simmons@users.noreply.github.com Do not call
DrawTextBlob for performance overlay text when using Impeller
(flutter/engine#46307)
2023-09-27 kjlubick@users.noreply.github.com Update to use
GrDirectContexts::MakeGL (flutter/engine#46308)
2023-09-27 skia-flutter-autoroll@skia.org Roll Skia from 0f4f31127ac5 to
2991bb799d3f (1 revision) (flutter/engine#46319)
2023-09-27 skia-flutter-autoroll@skia.org Roll Skia from fc629215398f to
0f4f31127ac5 (1 revision) (flutter/engine#46318)
2023-09-27 skia-flutter-autoroll@skia.org Roll Dart SDK from
6c4eb86ecd25 to c2a455113e39 (1 revision) (flutter/engine#46317)
2023-09-27 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from
OMrTgAfDg9PKXTzq0... to jQHACBU4Fi2wMElkm... (flutter/engine#46314)
2023-09-27 skia-flutter-autoroll@skia.org Roll Skia from 960325c13009 to
fc629215398f (1 revision) (flutter/engine#46313)
2023-09-27 skia-flutter-autoroll@skia.org Roll Skia from 76aecbaea259 to
960325c13009 (2 revisions) (flutter/engine#46311)
2023-09-27 skia-flutter-autoroll@skia.org Roll Dart SDK from
7c3588c05f87 to 6c4eb86ecd25 (3 revisions) (flutter/engine#46310)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from Lg6FR6iDnZeV to 6Y22MutFhgL7
  fuchsia/sdk/core/mac-amd64 from OMrTgAfDg9PK to U334SygIkffM

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC jacksongardner@google.com,rmistry@google.com,zra@google.com on
the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter:
https://github.com/flutter/flutter/issues/new/choose

...
harryterkelsen added a commit that referenced this pull request Oct 23, 2023
…6336)

Fixes an issue seen in the "widgets.image_filter_matrix" golden here:
https://flutter-gold.skia.org/search?crs=github&issue=135249&patchsets=3

## 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] and the [C++,
Objective-C, Java style guides].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I added new tests to check the change I am making or feature I am
adding, or the PR is [test-exempt]. See [testing the engine] for
instructions on writing and running engine tests.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I signed the [CLA].
- [x] All existing and new tests are passing.

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

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/flutter/wiki/Testing-the-engine
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform-web Code specifically for the web engine
Projects
None yet
2 participants