Fix hcpp cliprect being behind by 1 frame when scrolling - #189946
Merged
auto-submit[bot] merged 7 commits intoJul 28, 2026
Merged
Fix hcpp cliprect being behind by 1 frame when scrolling#189946auto-submit[bot] merged 7 commits into
auto-submit[bot] merged 7 commits into
Conversation
github-actions
Bot
added
platform-android
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the Android external view embedder to swap transactions before ending a frame, adds a corresponding unit test, and introduces a mechanism in PlatformViewsController2 to track and apply pending surface clips using a new PendingSurfaceClip class. The review feedback highlights a critical issue where the created SurfaceControl.Transaction is never applied, and suggests refactoring the lambda in external_view_embedder_2.cc to use the captured jni_facade for safety, as well as simplifying the logic for updating pendingSurfaceClips to avoid redundant operations.
Comment on lines
603
to
617
| float opacity = mutatorsStack.getFinalOpacity(); | ||
| if (viewsWithPendingSurfaceCallback.contains(viewId)) { | ||
| pendingSurfaceClips.put(viewId, new PendingSurfaceClip(opacity, screenRect)); | ||
| } | ||
| SurfaceControl sc = surfaceView.getSurfaceControl(); | ||
| if (sc == null) { | ||
| pendingSurfaceClips.put(viewId, new PendingSurfaceClip(opacity, screenRect)); | ||
| if (viewsWithPendingSurfaceCallback.contains(viewId)) { | ||
| return; | ||
| } | ||
| viewsWithPendingSurfaceCallback.add(viewId); | ||
| SurfaceHolder.Callback cb = | ||
| createSurfaceClipCallback(surfaceView, opacity, screenRect, viewId); | ||
| SurfaceHolder.Callback cb = createSurfaceClipCallback(surfaceView, viewId); | ||
| surfaceView.getHolder().addCallback(cb); | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
The logic for updating pendingSurfaceClips can be simplified to avoid redundant put operations when sc == null and viewsWithPendingSurfaceCallback already contains the viewId.
float opacity = mutatorsStack.getFinalOpacity();
SurfaceControl sc = surfaceView.getSurfaceControl();
if (sc == null || viewsWithPendingSurfaceCallback.contains(viewId)) {
pendingSurfaceClips.put(viewId, new PendingSurfaceClip(opacity, screenRect));
}
if (sc == null) {
if (viewsWithPendingSurfaceCallback.contains(viewId)) {
return;
}
viewsWithPendingSurfaceCallback.add(viewId);
SurfaceHolder.Callback cb = createSurfaceClipCallback(surfaceView, viewId);
surfaceView.getHolder().addCallback(cb);
return;
}…prect_behindby1frame' into fix_hcpp_scroll_cliprect_behindby1frame
reidbaker
previously approved these changes
Jul 24, 2026
reidbaker
left a comment
reidbaker
left a comment
Contributor
There was a problem hiding this comment.
LGTM but I feel far from qualified to critique your .cc code. I would encourage you to get a readability review.
mboetger
approved these changes
Jul 27, 2026
jmagman
approved these changes
Jul 27, 2026
gmackall
added
the
autosubmit
Member
Author
I am going to merge this one because it's a one liner outside of the test (hopefully that's fine), but will do for any more involved prs |
flutter-dashboard
Bot
removed
the
autosubmit
auto-submit Bot
pushed a commit
to flutter/packages
that referenced
this pull request
Jul 28, 2026
flutter/flutter@9988960...0f02463 2026-07-28 [email protected] Roll Fuchsia Test Scripts from E8hJ1AfK8CtGtaES0... to 1frGe_KltAJKkeyPg... (flutter/flutter#190134) 2026-07-28 [email protected] iOS: Reject merged-platform-ui-thread=mergeAfterLaunch (flutter/flutter#190051) 2026-07-28 [email protected] iOS: Migrate TaskRunner tests to Swift Testing (flutter/flutter#190055) 2026-07-28 [email protected] Run Mac golden tests on ARM bots (flutter/flutter#189465) 2026-07-28 [email protected] iOS,macOS: Rename Swift test files to end in Tests.swift (flutter/flutter#190063) 2026-07-28 [email protected] Fix hcpp cliprect being behind by 1 frame when scrolling (flutter/flutter#189946) 2026-07-28 [email protected] Roll Fuchsia Linux SDK from vpboK5fPPIoFteqRq... to OZkZC_2CZ_G5rbMIS... (flutter/flutter#190115) 2026-07-27 [email protected] Add Ishaq Hassan to AUTHORS (flutter/flutter#190064) 2026-07-27 [email protected] [wimp] fixes ubo padding size issue (flutter/flutter#189958) 2026-07-27 [email protected] Roll pub packages (flutter/flutter#189872) 2026-07-27 [email protected] Move tool host_cross_arch tests into different shards (flutter/flutter#189470) 2026-07-27 49699333+dependabot[bot]@users.noreply.github.com Bump actions/labeler from 6.2.0 to 7.0.0 in the all-github-actions group (flutter/flutter#190099) 2026-07-27 [email protected] [ios]do not nuke user input path when running uiscene integration test (flutter/flutter#186436) 2026-07-27 [email protected] ci: verify_binaries_pre_codesigned part 2 (flutter/flutter#190078) 2026-07-27 [email protected] Roll Abseil to ff6e8ce3e932 (flutter/flutter#189998) 2026-07-27 [email protected] Android_hardware_smoke_test: clean up golden copy in CI (flutter/flutter#189948) 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] 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, the clipping was behind by 1 frame.
This is because
onDisplayPlatformView2()was getting called afterjni_facade->swapTransaction(), and so was adding new pending transactions after the swap from pending to active (and so the clips were lagging for a frame, waiting till next frame to be made active). We need to calljni_facade->swapTransaction()last.The ordering (c++ bug) was introduced in #181009 in combination with #184732
See videos:
Screen_recording_20260723_130531.mp4
Screen_recording_20260723_130847.mp4