Repository navigation
iOS: Recomposite platform views only if their parameters change - #196287
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors FlutterPlatformViewsController to use Objective-C collections instead of C++ standard library collections for tracking platform views and views to dispose. It introduces FlutterPlatformViewInfo to encapsulate the platform view hierarchy and optimizes frame submission by only recompositing views whose parameters have changed since the previous frame. Additionally, new unit tests are added to verify these recomposition behaviors. Feedback suggests replacing the use of std::unordered_map::at with find during frame submission to prevent potential crashes in environments where C++ exceptions are disabled.
|
|
||
| if (self.currentCompositionParams.count(viewId) == 1 && | ||
| self.currentCompositionParams[viewId] == *params.get()) { | ||
| // Do nothing if the params didn't change. |
There was a problem hiding this comment.
Is this logic removed?
There was a problem hiding this comment.
Moved to submitFrame: in the loop on line 783 (under the "Recomposite only the views..." comment). See the comment there for the reason.
TL;DR the params aren't actually complete here: the backdrop filter/clip mutators are appended afterwards via the push*ToVisitedPlatformViews: calls, so we need to do the check after that, hence in submitFrame:.
| self.visitedPlatformViews.clear(); | ||
| } | ||
|
|
||
| - (void)submitFrame:(std::unique_ptr<flutter::SurfaceFrame>)background_frame |
There was a problem hiding this comment.
are these APIs called on raster thread?
There was a problem hiding this comment.
If you mean submitFrame:, yep!
If you mean reset above it, nope. Good catch. The doc comment is a lie (I didn't change anything but it was already wrong). Turns out the only caller is PlatformViewIOS::OnPreEngineRestart which happens on hot restart on the platform thread. Fixed.
| auto previous = self.previousCompositionParams.find(viewId); | ||
| if (previous == self.previousCompositionParams.end() || | ||
| previous->second != self.currentCompositionParams.at(viewId)) { | ||
| viewsToRecomposite.insert(viewId); |
There was a problem hiding this comment.
If I read it right, in previous code, we keep the previous state and compute delta when current state comes in. In current code, we keep both previous state and current state, and compute delta when submit frame?
Both seem reasonable to me. Is this the reason we make the change here?
currentCompositionParams
is now per-frame state that we clear at the same time as the composition
order.
There was a problem hiding this comment.
Yep exactly. There are a few reasons for doing it here:
- The set of views to recomposit wasn't cleared between frames, so once a view's params changed we actually recomposited it unnecessarily every frame after that.
- Plus the note above; the params aren't final in preroll so we need to do it after any backdrop filter/clip mutators are applied to the params after that and this was the most reasonable place I could think of.
`FlutterPlatformViewsController` tracks a set of views to recomposite such that we only call `compositeView:withParams:` for views whose composition parameters changed since the previous frame. It turns out that it didn't actually do that: * We only ever cleared the set in `reset`. Once a view's parameters had changed once, we recomposited it on every subsequent frame until hot restart. This has been the case since the set was introduced in flutter-team-archive/engine#9651. * We compared at preroll, but the visited-platform-view callbacks (`pushFilterToVisitedPlatformViews:` and the clip equivalents) append mutators to the stored parameters afterwards. The next frame's incoming parameters _never_ matched the stored copy, so we recomposited any view beneath a backdrop filter on every frame regardless, allocating a fresh `UIVisualEffectView` each time. This moves the parameter comparison to `submitFrame:`, after the callbacks have run, and compares each view's final parameters against the ones we submitted in the previous frame. `currentCompositionParams` is now per-frame state that we clear at the same time as the composition order. Views with no previous entry are always composited. That covers views recreated under a disposed ID since `onCreate:` doesn't insert an entry for any view ID that is still registered. We only unregister an ID when we present a frame that no longer includes it. `computeViewsToDispose` runs on the platform thread but was reading the composition order from (and erased entries in) state managed by the raster-thread. It now takes the composition order and no longer touches any raster-side maps. `compositeView:withParams:` is idempotent, so nothing changes in terms of the pixels. We just call it less often. Also added new tests to cover the unchanged, changed, filtered, and recreated cases. Issue: flutter#112232
8fa2da5 to
af8f7bf
Compare
flutter/flutter@b98afb5...9c427b5 2026-11-12 [email protected] Roll Skia from d67d0f22e3ec to 9aa2513a4ac9 (2 revisions) (flutter/flutter#196429) 2026-11-12 [email protected] [flutter_tools] Move CpuArch to flutter_tools_core and define HostPlatform by osName and cpuArch (flutter/flutter#195841) 2026-11-12 [email protected] Roll Skia from 23e7e63789b8 to d67d0f22e3ec (2 revisions) (flutter/flutter#196424) 2026-11-12 [email protected] Uber compactify (flutter/flutter#196392) 2026-11-12 [email protected] Roll Skia from 3d7e335da165 to 23e7e63789b8 (12 revisions) (flutter/flutter#196422) 2026-11-12 [email protected] Roll Dart SDK from 13af329ea4f2 to e5add7a25b32 (1 revision) (flutter/flutter#196413) 2026-11-12 [email protected] Don't terminate the shared EGL display when an engine is disposed (flutter/flutter#194520) 2026-11-11 [email protected] Adds manifest flag to turn on SDFs for Android. (flutter/flutter#196056) 2026-11-11 [email protected] Remove bringup from Linux_pixel_7pro integration_ui_keyboard_inset_jump (flutter/flutter#196334) 2026-11-11 [email protected] Uses system default locale for text rendering when preferred locale isn't supported (flutter/flutter#195997) 2026-11-11 [email protected] [android]: prewarms shaders when the context is created (flutter/flutter#196069) 2026-11-11 [email protected] iOS: Recomposite platform views only if their parameters change (flutter/flutter#196287) 2026-11-11 [email protected] [flutter_tools] Forward encoding in ErrorHandlingFile.readAsStringSync (flutter/flutter#196136) 2026-11-11 [email protected] Roll Skia from b89c24bbab8c to 3d7e335da165 (8 revisions) (flutter/flutter#196390) 2026-11-11 [email protected] Add Flutter Gardener instructions on how to disable a test (flutter/flutter#196312) 2026-11-11 [email protected] [Impeller] Enable malioc analysis for framebuffer_blend shaders (flutter/flutter#196343) 2026-11-11 [email protected] [flutter_tools] Migrate general.shard Android, asset, and build tests off globals (flutter/flutter#196125) 2026-11-11 [email protected] Roll Dart SDK from 8f15c4ca2d41 to 13af329ea4f2 (1 revision) (flutter/flutter#196386) 2026-11-11 [email protected] [Impeller] Do not clamp the local coverage limit to the root render target (flutter/flutter#195860) 2026-11-11 [email protected] [Impeller] Use separate uniform buffers on VMware SVGA3D (flutter/flutter#196022) 2026-11-11 [email protected] [flutter_tools] Introduce AndroidSdkProvider for lazy native_assets SDK lookup (flutter/flutter#196122) 2026-11-11 [email protected] Moves linux to ThinLTO. (flutter/flutter#196314) 2026-11-11 [email protected] [flutter_tools] Migrate bin/fuchsia_tester.dart off globals and runInContext (flutter/flutter#196074) 2026-11-11 [email protected] Changes the tolerances for frame_rate_main_test.dart. (flutter/flutter#196304) 2026-11-11 [email protected] Roll Packages from 08faf64 to 653628e (16 revisions) (flutter/flutter#196381) 2026-11-11 [email protected] Allow SliverCrossAxisGroup children to lay out with zero cross-axis extent instead of asserting (flutter/flutter#191287) 2026-11-11 [email protected] Roll Skia from cc4c54f752ba to b89c24bbab8c (4 revisions) (flutter/flutter#196375) 2026-11-11 [email protected] [Linux] Fix layer offset when compositing with glBlitFramebuffer (flutter/flutter#196189) 2026-11-11 [email protected] [flutter_tools] Inject CrashReportSender into SwiftPM migration (flutter/flutter#196315) 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
FlutterPlatformViewsControllertracks a set of views to recompositesuch that we only call
compositeView:withParams:for views whosecomposition parameters changed since the previous frame. It turns out
that it didn't actually do that:
reset. Once a view's parameters hadchanged once, we recomposited it on every subsequent frame until hot
restart. This has been the case since the set was introduced in
Move the mutators stack handling to preroll flutter-team-archive/engine#9651.
(
pushFilterToVisitedPlatformViews:and the clip equivalents) appendmutators to the stored parameters afterwards. The next frame's
incoming parameters never matched the stored copy, so we
recomposited any view beneath a backdrop filter on every frame
regardless, allocating a fresh
UIVisualEffectVieweach time.This moves the parameter comparison to
submitFrame:, after thecallbacks have run, and compares each view's final parameters against
the ones we submitted in the previous frame.
currentCompositionParamsis now per-frame state that we clear at the same time as the composition
order.
Views with no previous entry are always composited. That covers views
recreated under a disposed ID since
onCreate:doesn't insert an entryfor any view ID that is still registered. We only unregister an ID when
we present a frame that no longer includes it.
computeViewsToDisposeruns on the platform thread but was reading thecomposition order from (and erased entries in) state managed by the
raster-thread. It now takes the composition order and no longer touches
any raster-side maps.
compositeView:withParams:is idempotent, so nothing changes in termsof the pixels. We just call it less often.
Also added new tests to cover the unchanged, changed, filtered, and
recreated cases.
Issue: #112232
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-assistbot 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.