Repository navigation
FadeForwards page transition performance - #13161
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces snapshotting to FadeForwardsPageTransitionsBuilder to improve performance during page transitions by rendering a snapshot of the entering and exiting routes instead of the live child. It adds the allowSnapshotting property to control this behavior and includes corresponding tests. A critical issue was identified where widget.animation.isAnimating is used, which will cause a compilation error because the Animation class does not have an isAnimating property. The reviewer suggested checking the animation status instead.
| } | ||
|
|
||
| void _updateSnapshotting() { | ||
| _controller.allowSnapshotting = _useSnapshot && widget.animation.isAnimating; |
There was a problem hiding this comment.
The Animation class does not have an isAnimating property. Using widget.animation.isAnimating will result in a compilation error. Instead, you should check if the animation status is currently forward or reverse.
_controller.allowSnapshotting = _useSnapshot && (widget.animation.status == AnimationStatus.forward || widget.animation.status == AnimationStatus.reverse);There was a problem hiding this comment.
Animation.isAnimating does indeed exist https://api.flutter.dev/flutter/animation/Animation/isAnimating.html
Renzo-Olivares
left a comment
There was a problem hiding this comment.
LGTM overall! Nice find regarding snapshotting in the zoom transition.
Just left a couple of non-blocking questions/nits.
| // This means that animations inside of the child (e.g. ink splashes or | ||
| // progress indicators) are frozen for the duration of the transition, which is | ||
| // the same tradeoff made by [ZoomPageTransitionsBuilder]. If the child contains | ||
| // a platform view, the live child is painted instead (see | ||
| // [SnapshotMode.permissive]). |
There was a problem hiding this comment.
nit: It might be worth mentioning this tradeoff in the public dartdoc on FadeForwardsPageTransitionsBuilder as well (and linking to [PageRoute.allowSnapshotting] in its See also: section, similar to ZoomPageTransitionsBuilder), so developers who notice frozen ink splashes or progress indicators during transitions know why it happens and how to opt out.
| allowSnapshotting: | ||
| !ZoomPageTransitionsBuilder._kProfileForceDisableSnapshotting && | ||
| (route?.allowSnapshotting ?? true), |
There was a problem hiding this comment.
ZoomPageTransitionsBuilder also exposes allowSnapshotting (and allowEnterRouteSnapshotting) on the builder itself so users can opt out globally at the PageTransitionsTheme level without setting allowSnapshotting: false on every PageRoute (e.g., flutter/flutter#122019 / flutter/flutter#118086).
Do we want to keep FadeForwardsPageTransitionsBuilder (and PredictiveBackPageTransitionsBuilder) without those parameters for now to avoid expanding the public API surface until requested, or should we add allowSnapshotting = true for parity?
There was a problem hiding this comment.
Ok I added both allowSnapshotting and allowEnterRouteSnapshotting to both FadeForwardsPageTransitionsBuilder and PredictiveBackPageTransitionsBuilder for parity. The double-boolean API is not great as I mentioned in another comment, but I feel like any breaking cleanup should be done separately, at which point we might as well clean up both, rather than choosing a different API here.
I'm assuming that these two parameters are useful based on the Zoom transition and PR flutter/flutter#118086 which added the "EnterRoute" one. I'm interested in what you think though @Renzo-Olivares .
There was a problem hiding this comment.
I'm assuming that these two parameters are useful based on the Zoom transition and PR flutter/flutter#118086 which added the "EnterRoute" one. I'm interested in what you think though @Renzo-Olivares .
Yeah that was my thinking as well. One interesting caveat is that allowEnterRouteSnapshotting is not being used internally at the moment so the original use-case that motivated that API is no longer present. I wouldn't be opposed to deferring allowEnterRouteSnapshotting until requested and going with only allowSnapshotting.
There was a problem hiding this comment.
Ah you're right about the internal use being gone. I'll omit it on FadeForwards for now.
gaaclarke
left a comment
There was a problem hiding this comment.
Nice, thanks for following up. I just have a few questions.
| /// See also: | ||
| /// | ||
| /// * [_allowSnapshotting], which is the same but for both routes. | ||
| final bool _allowEnterRouteSnapshotting; |
There was a problem hiding this comment.
Do we really need this fine level of control? Is _allowSnapshotting sufficient?
There was a problem hiding this comment.
I almost omitted this, but I decided to include it since ZoomPageTransition has it. It was added in flutter/flutter#118086. @Renzo-Olivares do you have any thoughts?
There was a problem hiding this comment.
Wrote a reply https://github.com/flutter/packages/pull/13161/changes/BASE..2fb11b9332031d09baf4b19bc208bd0a6690a11e#r4262631515 . But generally i'd be okay with deferring allowEnterRouteSnapshotting until requested/needed.
| /// not be snapshotted. | ||
| /// See also: | ||
| /// | ||
| /// * [_allowSnapshotting], which is the same but for both routes. |
There was a problem hiding this comment.
It's worth mentioning the logic used since they are AND'd which isn't clear from the API.
| /// transition. | ||
| /// * [_allowEnterRouteSnapshotting], which is the same but only for the | ||
| /// entering route. | ||
| final bool _allowSnapshotting; |
There was a problem hiding this comment.
The word "allow" is not a definitive command, should this be "_useSnapshotting"?
There was a problem hiding this comment.
I agree this is not the best API, but I'm mirroring the existing ZoomPageTransitionBuilder.allowSnapshotting. The fact that there are 2 booleans that kind of overlap in scope also bothers me a bit (allowEnterRouteSnapshotting).
Since it would be a breaking change to rename the existing allowSnapshotting, I think we should probably use the same name for now, and if we ever get around to making the breaking change, we should rename them both at the same time in the future.
| allowSnapshotting: | ||
| !ZoomPageTransitionsBuilder._kProfileForceDisableSnapshotting && | ||
| (route?.allowSnapshotting ?? true), |
There was a problem hiding this comment.
I'm assuming that these two parameters are useful based on the Zoom transition and PR flutter/flutter#118086 which added the "EnterRoute" one. I'm interested in what you think though @Renzo-Olivares .
Yeah that was my thinking as well. One interesting caveat is that allowEnterRouteSnapshotting is not being used internally at the moment so the original use-case that motivated that API is no longer present. I wouldn't be opposed to deferring allowEnterRouteSnapshotting until requested and going with only allowSnapshotting.
| /// See also: | ||
| /// | ||
| /// * [_allowSnapshotting], which is the same but for both routes. | ||
| final bool _allowEnterRouteSnapshotting; |
There was a problem hiding this comment.
Wrote a reply https://github.com/flutter/packages/pull/13161/changes/BASE..2fb11b9332031d09baf4b19bc208bd0a6690a11e#r4262631515 . But generally i'd be okay with deferring allowEnterRouteSnapshotting until requested/needed.
| const _FadeForwardsPageTransition({ | ||
| required this.animation, | ||
| required this.secondaryAnimation, | ||
| required this._allowSnapshotting, |
There was a problem hiding this comment.
Any reason that these are private named parameters instead of public. I don't really have a preference but I think dart docs are not generated for private named parameters so things like hovering over this._allowSnapshotting would not display any relevant dart docs.
There was a problem hiding this comment.
I will make them public in this PR but then start a discussion about this. I was so excited to use this language feature to make private properties, but you're right about the docs. I made an effort to document the parameters in the constructor, but we should make an official decision here about whether we want to do that or keep things as they are.
Roll Packages from a1ed23c to da40eaf (33 revisions) flutter/packages@a1ed23c...da40eaf 2026-10-15 tarrinneal@gmail.com [pigeon]add support for sealed classes in native interop (flutter/packages#13175) 2026-10-15 tarrinneal@gmail.com [pigeon] Replace full JNI bindings with stubs for API classes that aren't needed (flutter/packages#13173) 2026-10-14 ksanaullah383.khan@gmail.com [material_ui] Fix self-comparison and assert bugs in semantics_tester (flutter/packages#12722) 2026-10-14 magder@google.com [camera_avfoundation] Replace the deprecated 'videoOrientation' with 'videoRotationAngle' on iOS 17+ (flutter/packages#13247) 2026-10-14 zengyan@88.com [material_ui] Fix GridTile crash when the incoming constraints are unbounded (flutter/packages#12586) 2026-10-14 magder@google.com [in_app_purchase_storekit] Remove unused and deprecated "SKDownload" hosted content (flutter/packages#13222) 2026-10-14 55765052+MohanadAbdallah-mv@users.noreply.github.com [material_ui] Add supportingTextPadding to InputDecorator (flutter/packages#12769) 2026-10-14 37883572+edwin-hollen@users.noreply.github.com [vector_graphics_codec] Add support for encoding and decoding paint blur commands (flutter/packages#13148) 2026-10-14 ketulm8@gmail.com [go_router] Fix redirect zone async error handling (flutter/packages#12836) 2026-10-14 fluttergithubbot@gmail.com Sync release-go_router-18.1.0 to main (flutter/packages#13260) 2026-10-14 fluttergithubbot@gmail.com Sync release-cupertino_ui-1.1.3 to main (flutter/packages#13258) 2026-10-14 lsaudon@gmail.com [material_ui] add enableInlinePrediction parameter to TextFormField (flutter/packages#12793) 2026-10-14 fluttergithubbot@gmail.com Sync release-material_ui-1.7.0 to main (flutter/packages#13259) 2026-10-14 sokolovskyi.konstantin@gmail.com [material_ui] Add the Material shapes. (flutter/packages#12771) 2026-10-14 64028623+Ahtsham0715@users.noreply.github.com [material_ui] Add tertiary tap callbacks to InkResponse and InkWell (flutter/packages#12676) 2026-10-14 engine-flutter-autoroll@skia.org Roll Flutter from 4249ccb to 426be96 (44 revisions) (flutter/packages#13256) 2026-10-14 47866232+chunhtai@users.noreply.github.com [material_ui] fixes expanded state in the a11y_assessment home page dropdown (flutter/packages#13219) 2026-10-14 zhongliu88889@gmail.com [VPAT] Fix semantics in a11y_assessments drawer, radio, and rail demos (flutter/packages#13159) 2026-10-14 5684363+tenninebt@users.noreply.github.com [google_maps_flutter] Add onPointOfInterestTap (flutter/packages#11872) 2026-10-14 Gornivv@gmail.com [go_router] Fix shell route key collisions and dynamic branch updates (flutter/packages#12549) 2026-10-14 engine-flutter-autoroll@skia.org Manual roll Flutter from d03768e to 4249ccb (158 revisions) (flutter/packages#13220) 2026-10-14 okorohelijah@google.com [image_picker_ios] Require Flutter 3.47 / iOS 15 and drop deprecated kUTType code paths (flutter/packages#13224) 2026-10-13 louisehsu@google.com [in_app_purchase_storekit] Fixes Storekit 2 flaky tests (flutter/packages#13213) 2026-10-13 stuartmorgan@google.com [local_auth] Convert Android to new Pigeon suspend support (flutter/packages#12894) 2026-10-13 50643541+Mairramer@users.noreply.github.com [material_ui] Add enabled parameter to DropdownButtonFormField (flutter/packages#12610) 2026-10-13 molchanovia.dev@gmail.com [go_router] Link ShellRoute docs to StatefulShellRoute (flutter/packages#13120) 2026-10-13 jmccandless@google.com FadeForwards page transition performance (flutter/packages#13161) 2026-10-13 101952451+eszter-varga@users.noreply.github.com [in_app_purchase_android] Guard async billing callbacks against duplicate listener responses (flutter/packages#12640) 2026-10-13 victor.orozco@cloudsufi.com [google_sign_in] Remove the Objective-C exception catcher (flutter/packages#13156) 2026-10-13 magder@google.com [camera_avfoundation] Remove availability checks below the package minimum iOS 13 (flutter/packages#13214) 2026-10-13 magder@google.com [google_maps_flutter_ios] Remove iOS 14 availability check from example UI tests (flutter/packages#13218) 2026-10-13 stuartmorgan@google.com [google_sign_in] Convert implementation to Kotlin (flutter/packages#13005) 2026-10-13 victor.orozco@cloudsufi.com [image_picker_ios] PR 2/5 Add a PHImageManager image-data seam and migrate plugin native tests to Swift Testing (flutter/packages#13079) 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-flutter-autoroll Please CC flutter-ecosystem@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: 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: ...
`material_ui` 1.8.0 (flutter/packages#13161) enabled route-transition raster snapshotting ([`allowSnapshotting = true`](https://github.com/flutter/packages/blob/623479c2ee9209316460eafb3f3cff61367127a0/packages/material_ui/lib/src/page_transitions_theme.dart#L590-L607)) in `FadeForwardsPageTransitionsBuilder`, causing `game_widget_lifecycle_test.dart` in `flame.test` to fail because `SnapshotWidget` reuses the cached route raster instead of calling `GameRenderBox.paint` a second time during the transition. Example CI failures: - https://ci.chromium.org/b/8667404745333857041 - https://ci.chromium.org/b/8667406041351366033 This temporarily disables `flame.test` to unblock the `flutter/flutter` tree while flame-engine/flame#4125 lands. Part of flutter/flutter#194883
Rolls `flutter/tests` to flutter/tests#508 (`1732eab79f73fe0b9b6ead6030437c74bebd1b2e`), which temporarily disables `registry/flame.test` after `material_ui` 1.8.0 (flutter/packages#13161) enabled route-transition raster snapshotting in `FadeForwardsPageTransitionsBuilder`. Example CI failures: - https://ci.chromium.org/b/8667404745333857041 - https://ci.chromium.org/b/8667406041351366033 Part of flutter#194883
`material_ui` 1.8.0 (flutter/packages#13161) enabled route-transition raster snapshotting ([`allowSnapshotting = true`](https://github.com/flutter/packages/blob/623479c2ee9209316460eafb3f3cff61367127a0/packages/material_ui/lib/src/page_transitions_theme.dart#L590-L607)) in `FadeForwardsPageTransitionsBuilder` on Android, matching `ZoomPageTransitionsBuilder`. In `game_widget_lifecycle_test.dart`, when `tester.tap(find.text('Play'))` pumps a second frame 16ms into the `/game` route transition, `SnapshotWidget` reuses the raster snapshot captured on the first transition frame instead of calling `GameRenderBox.paint` a second time. Example CI failures: - https://ci.chromium.org/b/8667404745333857041 - https://ci.chromium.org/b/8667406041351366033 This disables route-transition snapshotting via `FadeForwardsPageTransitionsBuilder(allowSnapshotting: false)` in `_MyApp` so the game continues rendering live during the transition. Part of flutter/flutter#194883 --------- Co-authored-by: Lukas Klingsbo <me@lukas.fyi>
Rolls `flutter/tests` to flutter/tests#508 (`1732eab79f73fe0b9b6ead6030437c74bebd1b2e`), which temporarily disables `registry/flame.test` after `material_ui` 1.8.0 (flutter/packages#13161) enabled route-transition raster snapshotting in `FadeForwardsPageTransitionsBuilder`. Example CI failures: - https://ci.chromium.org/b/8667404745333857041 - https://ci.chromium.org/b/8667406041351366033 Part of flutter#194883
Roll Packages from a1ed23c to da40eaf (33 revisions) flutter/packages@a1ed23c...da40eaf 2026-10-15 tarrinneal@gmail.com [pigeon]add support for sealed classes in native interop (flutter/packages#13175) 2026-10-15 tarrinneal@gmail.com [pigeon] Replace full JNI bindings with stubs for API classes that aren't needed (flutter/packages#13173) 2026-10-14 ksanaullah383.khan@gmail.com [material_ui] Fix self-comparison and assert bugs in semantics_tester (flutter/packages#12722) 2026-10-14 magder@google.com [camera_avfoundation] Replace the deprecated 'videoOrientation' with 'videoRotationAngle' on iOS 17+ (flutter/packages#13247) 2026-10-14 zengyan@88.com [material_ui] Fix GridTile crash when the incoming constraints are unbounded (flutter/packages#12586) 2026-10-14 magder@google.com [in_app_purchase_storekit] Remove unused and deprecated "SKDownload" hosted content (flutter/packages#13222) 2026-10-14 55765052+MohanadAbdallah-mv@users.noreply.github.com [material_ui] Add supportingTextPadding to InputDecorator (flutter/packages#12769) 2026-10-14 37883572+edwin-hollen@users.noreply.github.com [vector_graphics_codec] Add support for encoding and decoding paint blur commands (flutter/packages#13148) 2026-10-14 ketulm8@gmail.com [go_router] Fix redirect zone async error handling (flutter/packages#12836) 2026-10-14 fluttergithubbot@gmail.com Sync release-go_router-18.1.0 to main (flutter/packages#13260) 2026-10-14 fluttergithubbot@gmail.com Sync release-cupertino_ui-1.1.3 to main (flutter/packages#13258) 2026-10-14 lsaudon@gmail.com [material_ui] add enableInlinePrediction parameter to TextFormField (flutter/packages#12793) 2026-10-14 fluttergithubbot@gmail.com Sync release-material_ui-1.7.0 to main (flutter/packages#13259) 2026-10-14 sokolovskyi.konstantin@gmail.com [material_ui] Add the Material shapes. (flutter/packages#12771) 2026-10-14 64028623+Ahtsham0715@users.noreply.github.com [material_ui] Add tertiary tap callbacks to InkResponse and InkWell (flutter/packages#12676) 2026-10-14 engine-flutter-autoroll@skia.org Roll Flutter from 4249ccb to 426be96 (44 revisions) (flutter/packages#13256) 2026-10-14 47866232+chunhtai@users.noreply.github.com [material_ui] fixes expanded state in the a11y_assessment home page dropdown (flutter/packages#13219) 2026-10-14 zhongliu88889@gmail.com [VPAT] Fix semantics in a11y_assessments drawer, radio, and rail demos (flutter/packages#13159) 2026-10-14 5684363+tenninebt@users.noreply.github.com [google_maps_flutter] Add onPointOfInterestTap (flutter/packages#11872) 2026-10-14 Gornivv@gmail.com [go_router] Fix shell route key collisions and dynamic branch updates (flutter/packages#12549) 2026-10-14 engine-flutter-autoroll@skia.org Manual roll Flutter from d03768e to 4249ccb (158 revisions) (flutter/packages#13220) 2026-10-14 okorohelijah@google.com [image_picker_ios] Require Flutter 3.47 / iOS 15 and drop deprecated kUTType code paths (flutter/packages#13224) 2026-10-13 louisehsu@google.com [in_app_purchase_storekit] Fixes Storekit 2 flaky tests (flutter/packages#13213) 2026-10-13 stuartmorgan@google.com [local_auth] Convert Android to new Pigeon suspend support (flutter/packages#12894) 2026-10-13 50643541+Mairramer@users.noreply.github.com [material_ui] Add enabled parameter to DropdownButtonFormField (flutter/packages#12610) 2026-10-13 molchanovia.dev@gmail.com [go_router] Link ShellRoute docs to StatefulShellRoute (flutter/packages#13120) 2026-10-13 jmccandless@google.com FadeForwards page transition performance (flutter/packages#13161) 2026-10-13 101952451+eszter-varga@users.noreply.github.com [in_app_purchase_android] Guard async billing callbacks against duplicate listener responses (flutter/packages#12640) 2026-10-13 victor.orozco@cloudsufi.com [google_sign_in] Remove the Objective-C exception catcher (flutter/packages#13156) 2026-10-13 magder@google.com [camera_avfoundation] Remove availability checks below the package minimum iOS 13 (flutter/packages#13214) 2026-10-13 magder@google.com [google_maps_flutter_ios] Remove iOS 14 availability check from example UI tests (flutter/packages#13218) 2026-10-13 stuartmorgan@google.com [google_sign_in] Convert implementation to Kotlin (flutter/packages#13005) 2026-10-13 victor.orozco@cloudsufi.com [image_picker_ios] PR 2/5 Add a PHImageManager image-data seam and migrate plugin native tests to Swift Testing (flutter/packages#13079) 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-flutter-autoroll Please CC flutter-ecosystem@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: 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: ...
Rolls `flutter/tests` to flutter/tests#508 (`1732eab79f73fe0b9b6ead6030437c74bebd1b2e`), which temporarily disables `registry/flame.test` after `material_ui` 1.8.0 (flutter/packages#13161) enabled route-transition raster snapshotting in `FadeForwardsPageTransitionsBuilder`. Example CI failures: - https://ci.chromium.org/b/8667404745333857041 - https://ci.chromium.org/b/8667406041351366033 Part of flutter#194883
There's an old performance regression (flutter/flutter#177016) that happened when we changed the default Android page transition to FadeForwards from Zoom. I dismissed it as minor at first, but I think the root problem is that Zoom snapshots itself and FadeForwards doesn't. This PR adds snapshotting to FadeForwards and achieves a 37% speedup on my Pixel 9.
Benchmarking results
Snapshotting reduced raster time by 37% for my Pixel 9 (Impeller on Vulkan) and 22% for my Pixel 2XL (Impeller on OpenGL ES). I ran the transitions_perf test with the following command:
Fixes flutter/flutter#177016