Hero assertion when transitioning routemaps

[markphillips100]

It's difficult to know if this is an issue inherent in what I am doing or if this is actually a flutter bug itself. I'll let others decide I guess.

I've created a public app to demonstrate the issue and it's entirely reproducable. Repo is at https://github.com/markphillips100/herotest. Let me know if you prefer code snippets posted in this issue itself.

The strategy of the app is to show how to switch between responsive layouts; one for mobiles with a single page navigator, and one for wider viewports with a splitview where the left side is the root page and the right side uses it's own page navigator stack (using StackPage). Rotating the device would switch between the layouts and retain the existing route path.

Hero transitions should work within the mobile layout for all pages. For the wider layout, the left side heros should be disabled, and the right side hero transitions should work inside of their own navigator stack.

This all seems to works fine. Navigation okay, and heros isolated to their navigator stack okay. Problem occurs only when rotating from wide layout to mobile layout, and only when the right side has pushed another page above the stack's default path.

The exception I get is an assertion in flutter's heroes.dart file:

      final Animation<double> initial = initialManifest.animation;
      assert(initial != null);
      final HeroFlightDirection type = initialManifest.type;
      assert(type != null);
      switch (type) {
        case HeroFlightDirection.pop:
          return initial.value == 1.0 && initialManifest.isUserGestureTransition
              // During user gesture transitions, the animation controller isn't
              // driving the reverse transition, but should still be in a previously
              // completed stage with the initial value at 1.0.
              ? initial.status == AnimationStatus.completed
              : initial.status == AnimationStatus.reverse;
        case HeroFlightDirection.push:
          return initial.value == 0.0 && initial.status == AnimationStatus.forward;
      }
    }()': is not true.

I've stepped through the assertion logic using Android Studio and it fails because the last line's initial.status is AnimationStatus.declined.

To reproduce just follow these steps:

1. Start an Android emulator (haven't tried iOS) and rotate to landscape mode. Ensure the OS AutoRotate is enabled in settings.
2. Launch app. Should see a split view with "Wide Layout" Page on left and Page Two on right.
3. Press "Select three". Navigates right side to page three.
4. Rotate emulator to portrait. App will respond and throw the exception above.

I've edited the SDK code as a test to include the "declined" status as being valid in the last line, so that the assert returns true. I get no exception when I do this and no visual indication that something is broken.

Having said that, I'd like to understand if there's something fundamentally wrong with what I am doing that would cause this, and if there's something I can do to mitigate the assertion in the first place.

What I know so far:

1. No exceptions occur when a hero does not exist for the same tag on either Page One or Page Two, meaning no pair exists to do any transition.
2. Adding some tag prefix naming scope to the wider layout helps to avoid duplicate tags in subtrees but doesn't resolve this issue specifically.
3. The issue is triggered from a hero transition started by a push to Page Two using the material app controller scope, not the wide layout scope.

[markphillips100]

I've found a workaround and updated the herotest app to demonstrate it.

Basically there seems to be some conflict between SDK HeroController and Navigator. The former makes the animation status assertion during it's void start(_HeroFlightManifest initialManifest) method without first filtering out offending toRoute parameters in its _maybeStartHeroTransition method - seems a likely spot to put it IMO.

And from the Navigator perspective, even though the route that causes the issues is an "add" rather than a "push" (because it's not a top-level page) the HeroController's didPush is still called which triggers the _maybeStartHeroTransition flow.

So, the workaround is to have a custom derived HeroController and override didPush and filter the toRoute animation status and don't call the super if status is already dismissed:

class CustomHeroController extends HeroController {
  CustomHeroController({ createRectTween })
  : super(createRectTween: createRectTween);

  static HeroController createMaterialHeroController() {
    return CustomHeroController(createRectTween: (Rect? begin, Rect? end) {
        return MaterialRectArcTween(begin: begin, end: end);
      },
    );
  }

  @override
  void didPush(Route<dynamic> route, Route<dynamic>? previousRoute) {
    if (route is PageRoute<dynamic> && route.animation?.status == AnimationStatus.dismissed) {
      return;
    }
    super.didPush(route, previousRoute);
  }
}

Then use the builder variant of RoutemasterDelegate.builder to provide a new hero controller scope and specify an instance of the new controller (instantiated outside of the build method):

class MyApp extends StatelessWidget {

  // Use this controller to support bypass possible hero animation if the toRoute animation is already dismissed.
  final mainHeroController = CustomHeroController.createMaterialHeroController();
  // Use this controller to show assertion.
  // final mainHeroController = MaterialApp.createMaterialHeroController();

  MyApp({Key? key}) : super(key: key);

  // This widget is the root of your application.
  @override
  Widget build(BuildContext context) {
    return MaterialApp.router(
      title: 'Hero Rotation Demo',
      routeInformationParser: const RoutemasterParser(),
      routeInformationProvider: PlatformRouteInformationProvider(initialRouteInformation: const RouteInformation(location: "/layout")),
      routerDelegate: RoutemasterDelegate.builder(
        navigatorBuilder: (context, stack) {
          return HeroControllerScope(
            controller: mainHeroController,
            child: PageStackNavigator(
              stack: stack,
            ),
          );
        },
        routesBuilder: (context) {
....
        },
      ),
    );
  }
}

[markphillips100]

I've also submitted the issue with Flutter repo too. It's been triaged so will see if they come up with something.