Skip to content

[Impeller] Fix dilation input bounds when clipping saveLayers - #195652

Merged
auto-submit[bot] merged 4 commits into
flutter:masterfrom
ColdPaleLight:codex/fix-impeller-dilate-source-coverage
Nov 5, 2026
Merged

auto-submit[bot] merged 4 commits into
flutter:masterfrom
ColdPaleLight:codex/fix-impeller-dilate-source-coverage

Conversation

@ColdPaleLight

@ColdPaleLight ColdPaleLight commented Oct 30, 2026 •

Copy link
Copy Markdown
Member

Dilation needs source pixels outside the requested output bounds. Both DisplayList and Impeller inset those bounds instead, so a shape outside the parent clip can disappear even when its dilated result overlaps the clip.

Outset the required input bounds in both implementations. Add coverage and pixel regressions for this case, including transformed bounds, and restore the dilation comparison against Skia's reverse bounds.

fix issue: #195666

Skia reference:
https://fiddle.skia.org/c/86acbc9f1ca0a0a85db41a288e858498

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

List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.

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

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

If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

Dilation needs source pixels outside the requested output bounds. Both
DisplayList and Impeller inset those bounds instead, so a shape outside
the parent clip can disappear even when its dilated result overlaps the
clip.

Outset the required input bounds in both implementations. Add coverage
and pixel regressions for this case, including transformed bounds, and
restore the dilation comparison against Skia's reverse bounds.

Tested on macOS arm64: all 563 DisplayList tests and 41 related Impeller
tests pass, including Metal and MetalSDF with validation enabled.

Skia reference:
https://fiddle.skia.org/c/86acbc9f1ca0a0a85db41a288e858498
@flutter-dashboard flutter-dashboard Bot added the CICD Run CI/CD label Oct 30, 2026
@github-actions github-actions Bot added engine flutter/engine related. See also e: labels. e: impeller Impeller rendering backend issues and features requests labels Oct 30, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request corrects the source and input device bounds calculation for dilation morphology filters. Specifically, it updates DlDilateImageFilter::get_input_device_bounds to use outset_device_bounds instead of inset_device_bounds, and simplifies DirectionalMorphologyFilterContents::GetFilterSourceCoverage to expand the output limit for both dilation and erosion. Additionally, it adds several unit and golden tests to verify the correctness of the bounds calculation and coverage behavior. There are no review comments to address, and I have no further feedback to provide.

@ColdPaleLight
ColdPaleLight requested a review from flar October 30, 2026 06:47

@gaaclarke gaaclarke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looking good, let's just change the test to a golden test.

Move the regression to the Aiks clip tests so Skia Gold compares the whole image, including the strip that dilates into the clip.
@flutter-dashboard

Copy link
Copy Markdown

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 #195652 at sha 2994d94

@flutter-dashboard flutter-dashboard Bot added the will affect goldens Changes to golden files label Nov 3, 2026

@gaaclarke gaaclarke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm!, thanks

@gaaclarke gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 4, 2026
@auto-submit
auto-submit Bot added this pull request to the merge queue Nov 5, 2026
Merged via the queue into flutter:master with commit b6eaa14 Nov 5, 2026
28 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 5, 2026
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Nov 6, 2026
Roll Flutter from 3623d2d34543 to 09175cdeac89 (61 revisions)

flutter/flutter@3623d2d...09175cd

2026-11-06 [email protected] Roll Skia from ae35b7b2221f to 793b8ac73d4b (3 revisions) (flutter/flutter#196091)
2026-11-06 [email protected] [web] Rasterize scenes at the size passed to `FlutterView.render` (flutter/flutter#196052)
2026-11-06 [email protected] Roll Skia from 4aa043a81e0c to ae35b7b2221f (1 revision) (flutter/flutter#196086)
2026-11-06 [email protected] Roll Fuchsia Linux SDK from Fqe3UlLwAohsPbsCS... to KIc0XZbhLmvDpTluZ... (flutter/flutter#196085)
2026-11-06 [email protected] Allow a zero scale change in InteractiveViewer (flutter/flutter#194949)
2026-11-06 [email protected] Roll Skia from dd5c2553a856 to 4aa043a81e0c (2 revisions) (flutter/flutter#196079)
2026-11-06 [email protected] [web] Keep ImageFilter.blur bounds in equality and description (flutter/flutter#196013)
2026-11-06 [email protected] iOS: Remove unused PlatformViewIOS setup from FlutterPlatformViewsTest (flutter/flutter#196033)
2026-11-06 [email protected] Serialize LoggerTests that mutate the shared Logger (flutter/flutter#196028)
2026-11-06 [email protected] Use reverseDuration for the AnimatedCrossFade size animation (flutter/flutter#194860)
2026-11-05 [email protected] Roll Skia from a5aacd103e68 to dd5c2553a856 (8 revisions) (flutter/flutter#196073)
2026-11-05 [email protected] Fix ListWheelScrollView null check error when it reads itemIndex before content dimensions are set (flutter/flutter#194859)
2026-11-05 [email protected] Allow zero cross axis extent in SliverGridDelegateWithMaxCrossAxisExtent (flutter/flutter#194858)
2026-11-05 [email protected] Revert: Allow color delta tolerance in in-memory fuzzy image comparator (flutter/flutter#196067)
2026-11-05 [email protected] Revert: Mitigate snapshot race in 3D transform test by reducing loop density (flutter/flutter#196066)
2026-11-05 [email protected] Delegate createStream in ResizeImage and ScrollAwareImageProvider (flutter/flutter#194756)
2026-11-05 [email protected] [iOS] Introduce FlutterShell and move thread host and profiler (flutter/flutter#195388)
2026-11-05 [email protected] [web] Support FragmentShader.getImageSampler on CanvasKit and Skwasm (flutter/flutter#195854)
2026-11-05 [email protected] Deprecates setFloat API (flutter/flutter#195505)
2026-11-05 [email protected] [flutter_tools] Remove public GlobalToolContext and globals from runner (flutter/flutter#195561)
2026-11-05 [email protected] Defer TLHC PlatformViewWrapper frame post and avoid initial 1x1 buffer resize (flutter/flutter#195861)
2026-11-05 [email protected] Roll Skia from b5abed66da86 to a5aacd103e68 (17 revisions) (flutter/flutter#196054)
2026-11-05 [email protected] Made `FlutterWindowControllerSizeTest.SizedToContentNotResizable` more robust (flutter/flutter#195845)
2026-11-05 [email protected] Update docs for contributor ladder (flutter/flutter#196011)
2026-11-05 [email protected] [Impeller] Use the transfer stage for the Vulkan readback barrier (flutter/flutter#194991)
2026-11-05 [email protected] Remove Mac mac_unopt builder (flutter/flutter#196027)
2026-11-05 [email protected] Updated Instructions for Determining Gradle Version for `gradle_dists` in CIPD (flutter/flutter#193310)
2026-11-05 [email protected] Move ios_app_with_extensions_test into the tool integration shard (flutter/flutter#195609)
2026-11-05 [email protected] [web] Respect Uint8List view bounds when detecting animated WebP/GIF (flutter/flutter#196010)
2026-11-05 [email protected] Add platform_views_layout to build_tests so iOS/Android build breaks are caught in presubmit (flutter/flutter#192985)
2026-11-05 [email protected] [flutter_tools] Detect macOS TCC permission crashes and provide diagnostic guidance (flutter/flutter#192432)
2026-11-05 [email protected] Roll Packages from 670b30e to 0478162 (15 revisions) (flutter/flutter#196047)
2026-11-05 [email protected] [web] Support Skwasm rendering without bitmap capture (flutter/flutter#195103)
2026-11-05 [email protected] [flutter_tools] Reland fix for local CanvasKit asset paths on Windows (flutter/flutter#195770)
2026-11-05 [email protected] Roll Skia from f96be51af95d to b5abed66da86 (14 revisions) (flutter/flutter#196017)
2026-11-05 [email protected] Split chrome web engine test drone (flutter/flutter#195873)
2026-11-05 [email protected] Roll Dart SDK from c9153304136a to 6623b84628f3 (1 revision) (flutter/flutter#196026)
2026-11-05 [email protected] Roll ICU from 4fe960221710 to a0ea93436c52 (6 revisions) (flutter/flutter#196025)
2026-11-05 [email protected] Add more guidance in README and AGENTS for contributing to flutter_tools (flutter/flutter#195998)
2026-11-05 [email protected] Roll Fuchsia Linux SDK from p0HDXLISPIElJfSz7... to Fqe3UlLwAohsPbsCS... (flutter/flutter#196019)
2026-11-05 [email protected] Roll Dart SDK from e362a23e9738 to c9153304136a (18 revisions) (flutter/flutter#196014)
2026-11-05 [email protected] [Impeller] Fix dilation input bounds when clipping saveLayers (flutter/flutter#195652)
2026-11-05 [email protected] Add `variantOf` to the M3E migration checklist (flutter/flutter#195099)
2026-11-05 [email protected] [flutter_tools] Eliminate globals from remaining subsystem files (flutter/flutter#195555)
2026-11-05 [email protected] Fix gap flipping when dragging variable height reorderable items (flutter/flutter#194478)
2026-11-05 [email protected] [A11y] Send un-merged SemanticsNodes in the same update that first references them (flutter/flutter#195829)
...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICD Run CI/CD e: impeller Impeller rendering backend issues and features requests engine flutter/engine related. See also e: labels. will affect goldens Changes to golden files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants