[Skia][DisplayList] Map local bounds through the local matrix in DlLocalMatrixImageFilter by krll-kov · Pull Request #193963 · flutter/flutter

krll-kov · 2026-10-07T20:38:29Z

DlLocalMatrixImageFilter::map_local_bounds ignored matrix_, so on Skia raster cached Transform with filterQuality got wrong bounds, and DisplayListBuilder skipped drawing it when it was only partly visible. The bounds are now mapped through matrix_, as map_device_bounds already does.

This line comes from flutter-team-archive/engine#34878, where the review asked for a comparison with Skia (comment). This PR adds it for map_local_bounds in LocalImageFilterBounds, plus a LocalMatrixBounds test. Both fail without the fix

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

gemini-code-assist

Code Review

This pull request updates DlLocalMatrixImageFilter::map_local_bounds to correctly map local bounds by transforming them using the inverse matrix before applying the inner filter, and then transforming the result back. It also adds corresponding unit tests to verify the bounds mapping. Feedback suggests avoiding reference aliasing in map_local_bounds by using separate local variables for the input and output bounds of the inner filter to prevent potential corruption of input values.

gemini-code-assist · 2026-10-07T20:39:46Z

+  DlRect filter_bounds = input_bounds.TransformAndClipBounds(matrix_.Invert());
+  if (!image_filter_->map_local_bounds(filter_bounds, filter_bounds)) {
+    output_bounds = input_bounds;
+    return nullptr;
+  }
+  output_bounds = filter_bounds.TransformAndClipBounds(matrix_);


Passing the same DlRect object (filter_bounds) as both the input_bounds (const reference) and output_bounds (non-const reference) to map_local_bounds can lead to subtle bugs due to reference aliasing. If any implementation of map_local_bounds modifies the output parameter before it finishes reading from the input parameter, the input values will be corrupted.\n\nUsing separate local variables for the input and output bounds of the inner filter avoids this risk entirely.

DlRect filter_input_bounds = input_bounds.TransformAndClipBounds(matrix_.Invert());\n DlRect filter_output_bounds;\n if (!image_filter_->map_local_bounds(filter_input_bounds, filter_output_bounds)) {\n output_bounds = input_bounds;\n return nullptr;\n }\n output_bounds = filter_output_bounds.TransformAndClipBounds(matrix_);

References

C++ code should follow the Google C++ Style Guide and optimize for readability and safety. ^(link)

made 2 variables

gaaclarke

Looking good, can you make the test a bit more clear please?

gaaclarke

lgtm, thanks

gaaclarke · 2026-10-13T16:05:10Z

@flar can you be a second reviewer on this please?

flar · 2026-10-13T21:15:09Z

It basically LGTM, but I'm still wrapping my head around which way the matrices should be applied before/after querying the inner filter and double checking that these conditions are checked in the tests...

flar · 2026-10-13T21:15:39Z

Is it possible to write a golden test from the snippet in the linked issue?

krll-kov · 2026-10-13T23:21:18Z

Is it possible to write a golden test from the snippet in the linked issue?

i'm not sure how to do that, I haven't written golden tests in engine before

From reading a code (not verified by running it), a widget golden from the issue snippet may not hit this path: Scene.toImage seems to flatten the layer tree without a raster cache, and ImageFilterLayer only uses the local matrix filter when its children are raster cached

Where would you suggest putting such a test? If golden that draws with DlLocalMatrixImageFilter directly is enough, I can try that

gaaclarke · 2026-10-13T23:33:12Z

Where would you suggest putting such a test? If golden that draws with DlLocalMatrixImageFilter directly is enough, I can try that

Here's an example, anything that has OpenPlaygroundHere will be a golden:

flutter/engine/src/flutter/impeller/display_list/aiks_dl_basic_unittests.cc

Line 153 in 3454222

TEST_P(AiksTest, CanRenderColorFilterWithInvertColorsDrawPaint) {

krll-kov · 2026-10-14T08:08:03Z

Where would you suggest putting such a test? If golden that draws with DlLocalMatrixImageFilter directly is enough, I can try that

Here's an example, anything that has OpenPlaygroundHere will be a golden:

flutter/engine/src/flutter/impeller/display_list/aiks_dl_basic_unittests.cc

Line 153 in 3454222

TEST_P(AiksTest, CanRenderColorFilterWithInvertColorsDrawPaint) {

Added a test there

gaaclarke · 2026-10-14T15:57:20Z

@flar OP added a golden test, I can't rerequest a review so just a friendly ping

flutter-dashboard · 2026-10-14T17:00:59Z

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 #193963 at sha 8faf499

flar · 2026-10-14T19:16:31Z

Where would you suggest putting such a test? If golden that draws with DlLocalMatrixImageFilter directly is enough, I can try that

Here's an example, anything that has OpenPlaygroundHere will be a golden:

flutter/engine/src/flutter/impeller/display_list/aiks_dl_basic_unittests.cc

Line 153 in 3454222

TEST_P(AiksTest, CanRenderColorFilterWithInvertColorsDrawPaint) {

It can't be an impeller test because Impeller doesn't repro the issue. It has to be a framework test which runs on Skia to trigger the problem.

(To be more specific, you can write a test that uses the filter in Impeller, but the original problem in the Issue was due to the use of the RasterCache which never gets used with Impeller.)

flar · 2026-10-14T19:19:07Z

Is it possible to write a golden test from the snippet in the linked issue?

i'm not sure how to do that, I haven't written golden tests in engine before

From reading a code (not verified by running it), a widget golden from the issue snippet may not hit this path: Scene.toImage seems to flatten the layer tree without a raster cache, and ImageFilterLayer only uses the local matrix filter when its children are raster cached

Where would you suggest putting such a test? If golden that draws with DlLocalMatrixImageFilter directly is enough, I can try that

If that is the only way we produce Goldens then I guess we're out of luck there. Thanks for investigating. I'll check the Impeller golden test to see if it does the trick.

gaaclarke · 2026-10-14T22:18:35Z

@krll-kov if you got back to 3460a82 which i previously approved, sounds like @flar is good with that.

krll-kov · 2026-10-15T06:39:36Z

Where would you suggest putting such a test? If golden that draws with DlLocalMatrixImageFilter directly is enough, I can try that

Here's an example, anything that has OpenPlaygroundHere will be a golden:

flutter/engine/src/flutter/impeller/display_list/aiks_dl_basic_unittests.cc

Line 153 in 3454222

TEST_P(AiksTest, CanRenderColorFilterWithInvertColorsDrawPaint) {

It can't be an impeller test because Impeller doesn't repro the issue. It has to be a framework test which runs on Skia to trigger the problem.

(To be more specific, you can write a test that uses the filter in Impeller, but the original problem in the Issue was due to the use of the RasterCache which never gets used with Impeller.)

Force pushed to 3460a82 for revert, difference was just in history -> add test -> remove test

gaaclarke

lgtm, thanks

gaaclarke · 2026-10-15T16:36:39Z

@flar OP removed the test, please take a look

krll-kov · 2026-10-15T17:31:11Z

@flar OP removed the test, please take a look

Can you rerun please? It did not fail before

flar

LGTM

auto-submit · 2026-10-15T20:45:55Z

autosubmit label was removed for flutter/flutter/193963, because - The status or check suite Dashboard Checks has failed. Please fix the issues identified (or deflake) before re-applying this label.

flutter/flutter@5c7e45a...a3d71d8 2026-10-16 [email protected] 0 Initialize variables in the DlComplexityPathReceiver helper (flutter/flutter#194540) 2026-10-16 [email protected] Roll Fuchsia Test Scripts from V-qQL4158Nw6L-QLi... to 2T18KC7-LDQtmQiLP... (flutter/flutter#194527) 2026-10-16 [email protected] Remove BorderMaskBlur (flutter/flutter#194508) 2026-10-16 [email protected] Roll Skia from a577264dc087 to 470b8816a261 (1 revision) (flutter/flutter#194526) 2026-10-16 [email protected] [ios] Use DFS traversal to find engines of a scene automatically (rather than manually manage the engines list) (flutter/flutter#192457) 2026-10-16 [email protected] [Skia][DisplayList] Map local bounds through the local matrix in DlLocalMatrixImageFilter (flutter/flutter#193963) 2026-10-16 [email protected] [framework] Fix avoid_setters_without_getters warnings on mixin declarations (flutter/flutter#193856) 2026-10-16 [email protected] Roll Dart SDK from 84686ea04815 to 55a734a01fb1 (3 revisions) (flutter/flutter#194522) 2026-10-16 [email protected] Roll Skia from d131b7789482 to a577264dc087 (7 revisions) (flutter/flutter#194514) 2026-10-16 [email protected] Roll Fuchsia GN SDK from PDJz3b3CL31uoLOkD... to kFdyRnojbVy0cxvRZ... (flutter/flutter#194510) 2026-10-16 [email protected] Roll GN to f9eca75ae554 (flutter/flutter#194471) 2026-10-16 [email protected] refactor(flutter_tools): migrate RunCommand and RunCommandBase to OptionDescriptor and OptionBundle (flutter/flutter#194362) 2026-10-15 [email protected] refactor(flutter_tools): finish TestCommand OptionDescriptor and OptionBundle migration (flutter/flutter#194361) 2026-10-15 [email protected] [iOS][a11y] Fix VoiceOver announcement for unfocused password fields (flutter/flutter#192851) 2026-10-15 [email protected] Roll pub packages (flutter/flutter#194477) 2026-10-15 [email protected] refactor(flutter_tools): finish DriveCommand OptionDescriptor migration (flutter/flutter#194360) 2026-10-15 [email protected] [flutter_tools] Roll package:vm_service to 15.4.0 (flutter/flutter#194467) 2026-10-15 [email protected] refactor(flutter_tools): migrate AttachCommand to OptionDescriptor and OptionBundle (flutter/flutter#194359) 2026-10-15 [email protected] [flutter_tools] Inject dependencies into update-packages and package_map (flutter/flutter#194046) 2026-10-15 [email protected] [impeller] parallelize opengles shader compiles (flutter/flutter#194022) 2026-10-15 [email protected] Roll pub packages (flutter/flutter#194469) 2026-10-15 [email protected] Restrict web dev server to loopback by default (flutter/flutter#187595) 2026-10-15 [email protected] Roll Packages from a1ed23c to da40eaf (33 revisions) (flutter/flutter#194459) 2026-10-15 [email protected] [web] Render soft hyphens on Skwasm (flutter/flutter#193850) 2026-10-15 [email protected] refactor(flutter_tools): migrate AssembleCommand to OptionDescriptor (flutter/flutter#194358) 2026-10-15 [email protected] [windows] Add OpenGL golden test against ANGLE at D3D11 feature level 10_0 (flutter/flutter#194321) If this roll has caused a breakage, revert this CL and set the roller to dry run mode using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…calMatrixImageFilter (flutter#193963) Fixes flutter#170535 `DlLocalMatrixImageFilter::map_local_bounds` ignored `matrix_`, so on Skia raster cached `Transform` with `filterQuality` got wrong bounds, and `DisplayListBuilder` skipped drawing it when it was only partly visible. The bounds are now mapped through `matrix_`, as `map_device_bounds` already does. This line comes from flutter-team-archive/engine#34878, where the review asked for a comparison with Skia ([comment](flutter-team-archive/engine#34878 (comment))). This PR adds it for `map_local_bounds` in `LocalImageFilterBounds`, plus a `LocalMatrixBounds` test. Both fail without the fix ## 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]. - [x] I signed the [CLA]. - [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. - [x] All existing and new tests are passing. [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [CLA]: https://cla.developers.google.com/ --------- Co-authored-by: gaaclarke <[email protected]>

filter_bounds

9e2e4f5

github-actions Bot added the engine flutter/engine related. See also e: labels. label Oct 7, 2026

krll-kov marked this pull request as ready for review October 7, 2026 20:38

gemini-code-assist Bot reviewed Oct 7, 2026

View reviewed changes

gemini

67e6818

gaaclarke self-requested a review October 12, 2026 21:46

gaaclarke added the CICD Run CI/CD label Oct 12, 2026

gaaclarke requested changes Oct 12, 2026

View reviewed changes

Comment thread engine/src/flutter/display_list/effects/dl_image_filter_unittests.cc Resolved

Adjusted test

e0935c2

flutter-dashboard Bot removed the CICD Run CI/CD label Oct 13, 2026

Merge branch 'master' into fix-local-matrix-image-filter-bounds

3460a82

krll-kov requested a review from gaaclarke October 13, 2026 08:35

gaaclarke previously approved these changes Oct 13, 2026

View reviewed changes

gaaclarke added the CICD Run CI/CD label Oct 13, 2026

gaaclarke requested a review from flar October 13, 2026 16:04

krll-kov dismissed gaaclarke’s stale review via 8faf499 October 14, 2026 08:07

flutter-dashboard Bot removed the CICD Run CI/CD label Oct 14, 2026

github-actions Bot added the e: impeller Impeller rendering backend issues and features requests label Oct 14, 2026

gaaclarke added the CICD Run CI/CD label Oct 14, 2026

flutter-dashboard Bot added the will affect goldens Changes to golden files label Oct 14, 2026

flar requested changes Oct 14, 2026

View reviewed changes

Comment thread engine/src/flutter/impeller/display_list/aiks_dl_basic_unittests.cc Resolved

flutter-dashboard Bot removed the CICD Run CI/CD label Oct 14, 2026

github-actions Bot removed the e: impeller Impeller rendering backend issues and features requests label Oct 14, 2026

krll-kov requested a review from flar October 14, 2026 20:55

flar added the CICD Run CI/CD label Oct 14, 2026

krll-kov force-pushed the fix-local-matrix-image-filter-bounds branch from 77409bf to 3460a82 Compare October 15, 2026 06:38

flutter-dashboard Bot removed the CICD Run CI/CD label Oct 15, 2026

Merge branch 'master' into fix-local-matrix-image-filter-bounds

036b897

gaaclarke approved these changes Oct 15, 2026

View reviewed changes

gaaclarke added the CICD Run CI/CD label Oct 15, 2026

flar approved these changes Oct 15, 2026

View reviewed changes

gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 15, 2026

auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 15, 2026

Merge branch 'master' into fix-local-matrix-image-filter-bounds

9c27bc0

gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 15, 2026

flutter-dashboard Bot removed the CICD Run CI/CD label Oct 15, 2026

flar added the CICD Run CI/CD label Oct 15, 2026

auto-submit Bot added this pull request to the merge queue Oct 16, 2026

Merged via the queue into flutter:master with commit 132e1c6 Oct 16, 2026
27 checks passed

flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 16, 2026

This was referenced Oct 16, 2026

Roll Flutter from 5c7e45ab4d9c to 2e53c3af41ad (25 revisions) flutter/packages#13288

Closed

Roll Flutter from 5c7e45ab4d9c to a3d71d82160a (26 revisions) flutter/packages#13290

Merged

Conversation

krll-kov commented Oct 7, 2026

Pre-launch Checklist

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

krll-kov Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

gaaclarke commented Oct 13, 2026

Uh oh!

flar commented Oct 13, 2026

Uh oh!

flar commented Oct 13, 2026

Uh oh!

krll-kov commented Oct 13, 2026

Uh oh!

gaaclarke commented Oct 13, 2026

Uh oh!

krll-kov commented Oct 14, 2026

Uh oh!

gaaclarke commented Oct 14, 2026

Uh oh!

flutter-dashboard Bot commented Oct 14, 2026

Uh oh!

flar commented Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flar commented Oct 14, 2026

Uh oh!

Uh oh!

gaaclarke commented Oct 14, 2026

Uh oh!

krll-kov commented Oct 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

gaaclarke commented Oct 15, 2026

Uh oh!

krll-kov commented Oct 15, 2026

Uh oh!

flar left a comment

Choose a reason for hiding this comment

Uh oh!

auto-submit Bot commented Oct 15, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

flar commented Oct 14, 2026 •

edited

Loading

krll-kov commented Oct 15, 2026 •

edited

Loading