Apply rect clipping to surface views by gmackall · Pull Request #184471 · flutter/flutter

gmackall · 2026-04-01T18:11:20Z

Upper image is unclipped, bottom is clipped:

With Change	Without Change

…_surfaceviews' into again_we_try_to_fix_surfaceviews

gmackall · 2026-04-01T18:37:29Z

Note in the image that there is a small green bar on the right in the clipped image.

Some thoughts on the source: onDisplayPlatformView uses an int, when the actual position has sub-pixel values. Perhaps there is a rounding error there.
Could also be coming from an off by one error in the calculation of the offset, or perhaps we are rounding down the paths we calculate somehow?

I'm of the opinion we can fix this in a follow up issue, as it is pretty minor and this is a strict improvement to the clipping for surfaceviews, but let me know if you feel otherwise.

gemini-code-assist

Code Review

This pull request introduces clipping support for SurfaceView platform views on Android API 34 and above by applying crops to the SurfaceControl based on the mutator stack. It also adds a corresponding integration test with screenshot comparison. Review feedback highlights that SurfaceControl transactions are currently not being applied or closed, which is critical for functionality and resource management. Additionally, the implementation should be updated to prevent redundant SurfaceHolder callback registrations and to remove unnecessary coordinate validation logic.

gemini-code-assist · 2026-04-01T18:37:35Z

+    if (screenRect.width() < 0 || screenRect.height() < 0) {
+      screenRect.setEmpty();
+    }


This check is redundant. The screenRect is initialized from a RectF using roundOut, which ensures right >= left and bottom >= top. Subsequent intersect operations either maintain this property or result in an empty rect (where width and height are 0) via the setEmpty() call on line 570. The offset operation does not change the dimensions of the rectangle.

gmackall · 2026-04-01T18:52:24Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces clipping support for SurfaceView platform views on Android (API 34+) by applying crops via SurfaceControl transactions and adds a new integration test. Feedback identifies that the SurfaceControl transactions are missing the required apply() call and should be managed using try-with-resources for proper resource disposal. Additionally, it is recommended to use the Flutter-assigned viewId rather than the Android View ID to track pending surface callbacks reliably.

gmackall · 2026-04-07T16:15:40Z

It looks like this is creating golden file image tests on all open PRs right now. They test is suppressed on the dashboard and failing since this landed.

I don't think surpressing is helpful here, and the test is only failing because the gold images weren't approved. I approved them when the pr was initially put up (when the bot made this comment) but for some reason this seems to have been undone after the pr landed

All that needs to be done is for the images to be approved, which looks like it has happened.

gmackall · 2026-04-07T17:37:10Z

Reason for revert: Skia gold seems seriously confused about this PR (likely a bug). I think the only solution we can easily come up with is to just revert and re land

This reverts commit a488da3.

auto-submit · 2026-04-07T17:57:42Z

Unable to create the revert pull request due to ProcessException: Standard error
Pushing to https://github.com/flutter/flutter.git
To https://github.com/flutter/flutter.git
! [rejected] revert_a488da38c3d96e3c1331ba98c2de3a09e484886b -> revert_a488da38c3d96e3c1331ba98c2de3a09e484886b (non-fast-forward)
error: failed to push some refs to 'https://github.com/flutter/flutter.git'
hint: Updates were rejected because the tip of your current branch is behind
hint: its remote counterpart. If you want to integrate the remote changes,
hint: use 'git pull' before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.
Command: git push --verbose origin revert_a488da38c3d96e3c1331ba98c2de3a09e484886b

This reverts commit a488da3.

This reverts commit a488da3 because the bot is failing to do so in #184724 Revert because skia gold seems bugged w.r.t. the original pr, so try relanding it to reset skia gold state

flutter/flutter@9cd60b5...a0924c7 2026-04-07 [email protected] Reland "[data_assets] Cleanup tests" (flutter/flutter#184714) 2026-04-07 [email protected] Use the WindowRegistry in the multiple_windows example app (flutter/flutter#184579) 2026-04-07 [email protected] Introduce command to build a swift package for SwiftPM add to app integration (flutter/flutter#184660) 2026-04-07 [email protected] Have `flutter create` create a pubspec.lock to ensure pinned versions are being used. (flutter/flutter#175352) 2026-04-07 [email protected] [widgets/raw_menu_anchor.dart] Always call onClose and onCloseRequested on descendants before parent. (flutter/flutter#182357) 2026-04-07 [email protected] `WindowsPlugin` should not crash when ffiPlugin enabled (flutter/flutter#184695) 2026-04-06 [email protected] Use full goto.google.com hostname for go/ links (flutter/flutter#184679) 2026-04-06 [email protected] Apply rect clipping to surface views (flutter/flutter#184471) 2026-04-06 [email protected] [A11y] Allow percentage strings like "50%" as `SemanticsValue` for `ProgressIndicator` (flutter/flutter#183670) 2026-04-06 [email protected] Fix invisible accessibility element before scroll view (flutter/flutter#184155) 2026-04-06 [email protected] Roll Skia from 163dfdf500c7 to e264d870a380 (2 revisions) (flutter/flutter#184674) 2026-04-06 [email protected] Keep last character obscured when toggling obscureText (flutter/flutter#183488) 2026-04-06 [email protected] Parse scheme file with XML parser for SwiftPM migrator (flutter/flutter#184525) If this roll has caused a breakage, revert this CL and stop the roller 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

…ter#184728) This reverts commit b213589.

Relands #184471 which [was reverted](#184728) because of suspected skia gold bug where the triage button was not working for the created digests. I also changed the names of the output files just in case, to make sure the state doesn't collide with old.

Fixes flutter#175546 Upper image is unclipped, bottom is clipped: <table width="100%"> <thead> <tr> <th align="center">With Change</th> <th align="center">Without Change</th> </tr> </thead> <tbody> <tr> <td align="center" width="50%"> <img src="https://github.com/user-attachments/assets/c713dd0f-8cfd-4e7d-9070-3bf51a89f50a" style="max-width: 50%;"></img> </td> <td align="center" width="50%"> <img src="https://github.com/user-attachments/assets/fc23ff88-bd64-44d0-a840-e7ba58638943" style="max-width: 50%;"></img> </td> </tr> <tr> <td align="center"> <img src="https://github.com/user-attachments/assets/fef237f1-9e4f-4376-a251-22335cf7a324" style="max-width: 50%;"></img> </td> <td align="center"> <img src="https://github.com/user-attachments/assets/db707bf1-2bd8-4c39-a510-ca45f7730a0a" style="max-width: 50%;"></img> </td> </tr> </tbody> </table>

…er#184728) This reverts commit a488da3 because the bot is failing to do so in flutter#184724 Revert because skia gold seems bugged w.r.t. the original pr, so try relanding it to reset skia gold state

Relands flutter#184471 which [was reverted](flutter#184728) because of suspected skia gold bug where the triage button was not working for the created digests. I also changed the names of the output files just in case, to make sure the state doesn't collide with old.

gmackall added 4 commits April 1, 2026 10:42

finally, this works

e2f6574

format

1efc79b

dangling comment removed

2428d45

add test

a3e8287

github-actions Bot added platform-android Android applications specifically engine flutter/engine related. See also e: labels. team-android Owned by Android platform team labels Apr 1, 2026

Merge branch 'master' into again_we_try_to_fix_surfaceviews

8d2d046

gmackall added the CICD Run CI/CD label Apr 1, 2026

gmackall added 2 commits April 1, 2026 11:27

remove bad comments

5ff934f

Merge remote-tracking branch 'refs/remotes/origin/again_we_try_to_fix…

0e6d909

…_surfaceviews' into again_we_try_to_fix_surfaceviews

github-actions Bot removed the CICD Run CI/CD label Apr 1, 2026

gmackall added 2 commits April 1, 2026 11:33

fix todos

4d923af

format

500a97a

gmackall marked this pull request as ready for review April 1, 2026 18:34

gmackall requested a review from a team as a code owner April 1, 2026 18:34

gmackall added the CICD Run CI/CD label Apr 1, 2026

gemini-code-assist Bot reviewed Apr 1, 2026

View reviewed changes

avoid explosion of callback registration

c5e78d5

github-actions Bot removed the CICD Run CI/CD label Apr 1, 2026

gmackall added the CICD Run CI/CD label Apr 1, 2026

claw back caveat

1aaffb2

gemini-code-assist Bot reviewed Apr 1, 2026

View reviewed changes

Comment thread .../src/flutter/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController2.java

Comment thread .../src/flutter/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController2.java Outdated

github-actions Bot added d: docs/ flutter/flutter/docs, for contributors CICD Run CI/CD and removed CICD Run CI/CD labels Apr 1, 2026

align view id used for hashset management

07806a8

github-actions Bot removed the CICD Run CI/CD label Apr 1, 2026

engine-flutter-autoroll mentioned this pull request Apr 7, 2026

Roll Flutter from 9cd60b512fbc to 3bdf02c1cab0 (11 revisions) flutter/packages#11458

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 7, 2026

a488da38c Apply rect clipping to surface views (flutter/flutter#184471)

4489f48

engine-flutter-autoroll mentioned this pull request Apr 7, 2026

Roll Flutter from 9cd60b512fbc to 92d81d35ea30 (12 revisions) flutter/packages#11460

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 7, 2026

a488da38c Apply rect clipping to surface views (flutter/flutter#184471)

58c4234

engine-flutter-autoroll mentioned this pull request Apr 7, 2026

Roll Flutter from 9cd60b512fbc to a0924c76b0fd (13 revisions) flutter/packages#11462

Closed

gmackall added the revert Autorevert PR (with "Reason for revert:" comment) label Apr 7, 2026

auto-submit Bot pushed a commit that referenced this pull request Apr 7, 2026

Revert "Apply rect clipping to surface views (#184471)"

d82ffe3

This reverts commit a488da3.

auto-submit Bot mentioned this pull request Apr 7, 2026

Reverts "Apply rect clipping to surface views (#184471)" #184724

Closed

auto-submit Bot removed the revert Autorevert PR (with "Reason for revert:" comment) label Apr 7, 2026

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 7, 2026

a488da38c Apply rect clipping to surface views (flutter/flutter#184471)

64225fd

engine-flutter-autoroll mentioned this pull request Apr 7, 2026

Roll Flutter from 9cd60b512fbc to a0924c76b0fd (13 revisions) flutter/packages#11463

Merged

gmackall added the revert Autorevert PR (with "Reason for revert:" comment) label Apr 7, 2026

auto-submit Bot removed the revert Autorevert PR (with "Reason for revert:" comment) label Apr 7, 2026

gmackall added a commit to gmackall/flutter that referenced this pull request Apr 7, 2026

Revert "Apply rect clipping to surface views (flutter#184471)"

902fe07

This reverts commit a488da3.

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 7, 2026

a488da38c Apply rect clipping to surface views (flutter/flutter#184471)

4683b70

engine-flutter-autoroll mentioned this pull request Apr 7, 2026

Roll Flutter from 9cd60b512fbc to a0924c76b0fd (13 revisions) flutter/packages#11464

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 7, 2026

a488da38c Apply rect clipping to surface views (flutter/flutter#184471)

32ab903

engine-flutter-autoroll mentioned this pull request Apr 7, 2026

Roll Flutter from 9cd60b512fbc to b185b02c1d07 (14 revisions) flutter/packages#11465

Closed

gmackall added a commit to gmackall/flutter that referenced this pull request Apr 7, 2026

Reapply "Apply rect clipping to surface views (flutter#184471)" (flut…

4d3a3be

…ter#184728) This reverts commit b213589.

gmackall mentioned this pull request Apr 7, 2026

Reland "Apply rect clipping to surface views" #184732

Merged

Piinks mentioned this pull request Apr 10, 2026

Flutter Notable Commits - 2026 #181433

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Apply rect clipping to surface views#184471

Apply rect clipping to surface views#184471
auto-submit[bot] merged 21 commits into
flutter:masterfrom
gmackall:again_we_try_to_fix_surfaceviews

gmackall commented Apr 1, 2026 •

edited

Loading

Uh oh!

gmackall commented Apr 1, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist Bot Apr 1, 2026

Uh oh!

gmackall commented Apr 1, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

Uh oh!

gmackall commented Apr 7, 2026 •

edited

Loading

Uh oh!

gmackall commented Apr 7, 2026

Uh oh!

auto-submit Bot commented Apr 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

gmackall commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gmackall commented Apr 1, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist Bot Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

gmackall commented Apr 1, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

gmackall commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gmackall commented Apr 7, 2026

Uh oh!

auto-submit Bot commented Apr 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gmackall commented Apr 1, 2026 •

edited

Loading

gmackall commented Apr 7, 2026 •

edited

Loading