Fix load and play issues between states

[youngjoo-sonos]

During the course of using flare, I've been encountered glitches every now and then such as #136 , #112 and more. Here are the fixes for some of them.
Most issues that I had were coming from state to sate in which we are particularly switching to a different flare file and a different animation or continuously using same flare animation linearly across pages. During that transition, we needed to figure out how we can handle the flareplugin loading process since it is asynchronous and a bit hard to understand.
The most painful issues to me were rendering empty frame first between state changes or failing to resume linearly between pages.

1. Add a feature of loading the flare asset immediately if it can. From now on, if it can't load immediately, then it loads asynchronously as before. To fix that, I added loadImmediately(). It gets called before load() to see if it can load from the cache immediately. If it can't, then it always subsequently calls load() as before.

2. _isLoading variable is not always correct. (Even FlareActor can call load() directly which wouldn't let the base class know that the call is occurring.) To fix that, load() won't be called if the node is not mounted.

3. The load method was incorrectly setting things up which would cause the first frame to play the first animation frame while in _instanceArtboard(). The issue was that the animation layers were not loaded, and the controller was not being called to advance on the first frame. That caused the animation failed to resume from where it left off. With this fix, now the controller keeps the flare continuing right from where it left off.

4. The flare plugin is continually drawing every single frame even though the controller said it was inactive. To fix that, I split ((_controller?.isActive?.value ?? false) || _animationLayers.isNotEmpty); into two separate conditions (_controller?.isActive?.value ?? true) && _animationLayers.isNotEmpty; Note: I set ?? true instead of ?? false to honor when _animationLayers.isNotEmpty is true.

[yjoo9317]

@luigi-rosso
Let me know what you think about the change.

[umberto-sonnino]

Thanks a lot for your contribution!
This is all great feedback, I'm trying to review it and understand what we can improve in the current implementation; could you provide some example files & code to reproduce the issues you're having?

In particular:

1. I believe that to be already in place with the current implementation: the call to await cachedActor() will try to fetch a copy of the actor in cache if already present.
   For 2. 3. and 4. some reproducibles would make it possible to debug it.

[dannyvalentesonos]

@umberto-sonnino The code that is in place currently does not work correctly in the case where there is a valid cached actor, because the method is an async method, and the await keyword is causing issues that one must understand exists due to the nature of the async APIs in dart.

If you look at https://dart.dev/codelabs/async-await#execution-flow-with-async-and-await, you will see the following note:

Execution flow with async and await
An async function runs synchronously until the first await keyword. This means that within an async function body, all synchronous code before the first await keyword executes immediately.

 Note: Before Dart 2.0, an async function returned immediately, without executing any code within the async function body.

This is very important to understand what is going on. As soon as an await keyword is met, the execution of the method halts and execution resumes from the following instruction after the calling site. This means, that the code following the await in the flare plugin will only run in the next mircrotask, which means that a full frame will be rendered before the plugin's method code resumes.

This is why the changes we are proposing actually fixes all the glitches with flares disappearing for one frame whenever the flare render object must be unattached and reattached to a different node in the render tree. The fix simply allows the plugin to load everything synchronously, without using any await keywords anywhere when a cached actor exists.

I hope that explains the need for the changes. Let me know if you have any other questions.

Thanks!

[yjoo9317]

@umberto-sonnino
Here is the sample code.
I've tried to minimize the size as much as I can.

The archive file that I attached here includes /lib, /animations, /plugins and pubspec.yaml.
You can copy and paste directories and pub file into your test sample app.

1. /lib contains the source files
2. /animations contains flare animation file
3. /plugins contains fixed flare plugin
4. with pubspec file, you can switch around flare and local flare when to run it.
   flare_flutter: ^1.6.2
   #flare_flutter:
   #path: plugins/Flare-Flutter/flare_flutter

If you run the sample app,
you can press swap button at the bottom which simply sets state again.
Then you will see the glitches.
(Expected: The animation should keep running seamlessly)

Let me know if you need anything else.
Archive.zip

[luigi-rosso]

I'm sorry I haven't been able to look at this yet, but I wanted to share writeup we did a while ago which touches on some of these points:
https://github.com/2d-inc/Flare-Flutter/blob/sync_load/loading_strategies.md

Thanks for the submission @youngjoo-sonos and @umberto-sonnino for looking into this! I hope to catch up soon.

[luigi-rosso]

I see the intention here, this is great. It boils down to performing a fully synchronous load when possible, if not, quickly detect it and fall back to an async load. This prevents popping when first rendering a FlareActor widget that already has the Flare file in the cache.

I love this idea, and gives me another idea...I'm going to re-work this a little bit. I'll post updates here soon.

[luigi-rosso]

I need to do more testing (feel free to try this branch), but the idea is that you can also use a FlareCacheBuilder anywhere and display certain parts of your app only when the cache is warm (say you want to show a loading message or something else):

506837e

I also implemented a lot of the ideas from this PR slightly differently to match the wording and style of the rest of the codebase, but the concept is very similar.

I still need to better test the controller active state.

[dannyvalentesonos]

@luigi-rosso I think this looks good and I like the idea.

This change doesn't fix the issue with the flare animation not resuming at the correct location, from where it was before it gets moved in the widget tree, nor does it fix the problem with the controller stopping the animation, yet the flare widget keeps processing an animation on every frame draw.
Do you plan on incorporating those fixes in a different change?

Thanks!

[luigi-rosso]

Hey @dannyvalentesonos, yes we're definitely planning on fixing those issues too. I want to look at them more deeply as there are specific use cases that expect the controller to work the way it currently does. I want to make sure I see the problems clearly and how the fixes impact all the use cases, so it's taking a little longer. Thanks to @yjoo9317 for the sample. I'm hoping to get this resolved today.

[luigi-rosso]

Ok give that branch another shot:
https://github.com/2d-inc/Flare-Flutter/tree/optimal_load

I made some changes to ensure the cold path isn't taken when loading is impossible (happens when recycling the widget). Thanks @yjoo9317, your example clearly showed this.

The only other item that I haven't addressed is changing the isPlaying check.

((_controller?.isActive?.value ?? false) || _animationLayers.isNotEmpty)

It's important that the ?? value is false so that rendering continues if the animationLayers are not empty. animationLayers are set when FlareActor is still mixing an active animation. This is your change:

((_controller?.isActive?.value ?? true) && _animationLayers.isNotEmpty)

This wouldn't have the desired outcome as the controller would only keep playing if it's active and it's also mixing an animation from the FlareActor, which often is not the case when using a controller. Often you use a controller with no base animation, wanting the controller to provide all the animation.

Could you send me a separate example showing the case you're running into? I suspect there's an animation playing that's forcing the controller to keep advancing.

[yjoo9317]

/// We're playing if we're not paused and our controller is active (or
/// there's no controller) or there are animations running.

[luigi-rosso]

The or is important there.

Flare needs to keep playing if either the controller is active or the FlareActor has animations still mixing (_animationLayers.isNotEmpty). By changing it to an and you are forcing it to only play when all those conditions are true, which is often not the case with controllers.

I suspect that in your use case you're trying to mix an animation via FlareActor and use a controller with it. If that's the case, you need to make sure the FlareActor animation is not looping. Otherwise, it will keep playing forever (and your controller will advance in sync with it). Send me an example, happy to help fix it.

[luigi-rosso]

The flare plugin is continually drawing every single frame even though the controller said it was inactive.

I think this is the fundamental misunderstanding: The controller doesn't get the final say on whether the animation is advancing or not, that's because there are other forces at work advancing the animation. For example, if you provide an animation name to the FlareActor. Those animations are not mutually exclusive to a controller (see the penguin example where both are used).

The controller can let FlareActor know that it no longer needs to advance, and if FlareActor is done too, then no further drawing will occur.

If either still needs to draw, then both must advance.

[yjoo9317]

My suspicion was the controller, maybe hugely influence from the experience of video_controller, is inactive in which we think it might reach the end and return false, anyhow, it means the controller is not active means the animation should inactive meaning no more animation. For example, if we are running still image with flare, it still calls advance.. not sure it is correct behavior. Also, by putting or there, if the animation is empty but controller is active, then it runs it. Is still valid case to run?

[luigi-rosso]

Yes, that's a valid case.

There are times you want just the controller to drive the entire experience (no animation mixing from FlareActor). For example, take a look at Teddy:

Flare-Flutter/example/teddy/lib/main.dart

Lines 77 to 82 in 9ee95b9

	child: FlareActor(
	"assets/Teddy.flr",
	shouldClip: false,
	alignment: Alignment.bottomCenter,
	fit: BoxFit.contain,
	controller: _teddyController,

Teddy only uses one controller, no animation from FlareActor.

In some other cases you want both a controller and a base animation like with the penguin:

Flare-Flutter/example/penguin_dance/lib/main.dart

Lines 60 to 65 in 9ee95b9

	child: FlareActor("assets/Penguin.flr",
	alignment: Alignment.center,
	isPaused: _isPaused,
	fit: BoxFit.cover,
	animation: "walk",
	controller: this)),

If only the base animation advances, then the controller will not be applied and the visuals will glitch as they pop to not using the controller. Instead, the controller keeps running if the FlareActor needs it to.

If you're seeing undesirable playback, make sure FlareActor isn't still mixing an animation.

[yjoo9317]

Basically, the controller says stop, it must stop. With that expression, there is a case where it won't stop. Then we hardly call it controller usually. Maybe I am misunderstanding the concept behind.

[yjoo9317]

Ok so I am trying to understand the bigger concept here. For example, there is a page in which there are background animation running (like cloud....which is just looping) and a main animation controlled by the controller on top. (I've never mixed them up like that though). In this case the controller becomes inactive (main animation reached the end), but there are still background animation running. Is it something you are talking about here?

[luigi-rosso]

If you're using a single FlareActor to compose that scene, then yes the controller will always need to run as the clouds are animating.

One option is to break the scene into two files and use two FlareActors stacked over each other. One always looping the animation and the other just controlling the portion that can stop.

An even better way to tackle something custom (like a scene with lots of animations or a scene composed of multiple Flare files) is by using a custom FlareRenderBox with your own LeafRenderObjectWidget. We are working on some more complex examples (like games) that include complex logic like this (where a scene is composed of multiple files) but we don't have anything public yet.

Flare-Actor can't solve every case, it's a general tool to solve common scenarios. We want to encourage advanced users of Flare-Flutter to be comfortable making custom widgets as necessary. For example, here is one such widget where you can directly pass in the artboard to render instead of a filename: https://github.com/luigi-rosso/flare_flutter_shared_artboard/blob/master/lib/flare_artboard.dart

Here's another one that does some custom manipulation of the nodes to 3d tilt them:
https://github.com/2d-inc/tilt_card/blob/master/lib/flare_tilt_widget.dart

The flare_flutter library isn't just limited by what FlareActor can do. In fact, we're thinking of moving FlareActor into a new package/library in the future to specifically keep flare_flutter small and efficient for users who just want to build their own custom widgets for their very specific scenarios.

If you have another scenario you were trying to solve that led you to this problem, please feel free to contact me/share the source. I would like to help you solve it efficiently!

[luigi-rosso]

And in the meantime, please do try if https://github.com/2d-inc/Flare-Flutter/tree/optimal_load fixes the rest of the issues so we can get that merged!

[yjoo9317]

With current setting, I got these outputs endlessly..
the animation's duration is 3 seconds and it deactivates at the end.
But look at the following, it kept calling advance() and printing out same message over and over.
@luigi-rosso it seems serious issue to me.
Change isLooping to false then you will see it keeps advancing.

I/flutter (22895): >>>> FLARE deactivate [1] no more advance..
I/flutter (22895): >> animation duration 3.0 vs 3.0077009999999995
I/flutter (22895): >>>> FLARE deactivate [1] no more advance..
I/flutter (22895): >> animation duration 3.0 vs 3.0077009999999995
I/flutter (22895): >>>> FLARE deactivate [1] no more advance..```

[yjoo9317]

@luigi-rosso
I will test the latest with our internal flows to see if it fixes the issues as you suggested. I will let you know tomorrow. In meantime, please can you look at the issue of keeping calling advance() issue please?

[luigi-rosso]

Sure! @yjoo9317, can you post the code for that example? The one that prints the animation duration 3.0 vs 3.0077009999999995? I'd like to see how you set up the controller and FlareActor.

[yjoo9317]

@luigi-rosso
as I mentioned above, you don't need another example to reproduce the continuous advance calls.
You can easily reproduce with the example that I gave to you.
Just set isLooping false in flare_text_homepage.dart.
And I set the comments in advance() of flare_widget.dart as following.

If you un it, then you will see the following ouputs.

I/flutter (30574): >>> FLARE advance called...
I/chatty (30574): uid=10331(io.flutter.flare_test_app) 1.ui identical 85 lines
I/flutter (30574): >>> FLARE advance called...
I/flutter (30574): >> Deactivate: reached the end of animation
I/flutter (30574): >>> FLARE advance called...
I/flutter (30574): >> Deactivate: animation duration 2.0 vs 2.000498
I/flutter (30574): >>> FLARE advance called...
I/flutter (30574): >> Deactivate: animation duration 2.0 vs 2.000498
I/flutter (30574): >>> FLARE advance called...
I/flutter (30574): >> Deactivate: animation duration 2.0 vs 2.000498
I/flutter (30574): >>> FLARE advance called...
I/flutter (30574): >> Deactivate: animation duration 2.0 vs 2.000498
I/flutter (30574): >>> FLARE advance called...
I/flutter (30574): >> Deactivate: animation duration 2.0 vs 2.000498
....

[luigi-rosso]

Sorry, that wasn't clear that it was the same code (I couldn't see any of the output you were referring to). I now understand you added the print statements separately to evidence the issue.

The problem is that the animation name is being passed to FlareActor. This causes both FlareActor and your controller to apply the animation. It'll never stop, and it's further wasteful because both FlareActor and the controller are doing the same thing. FlareActor only reads the animation's internal looping value (which is true) so it keeps playing the animation (which must also keep advancing the controller). You can fix this by removing the animation parameter from FlareActor, since you're applying the animation yourself in the controller.

    Widget flare = FlareActor(
        "animations/${widget.asset}",
        alignment: Alignment.center,
        fit: widget.fit ?? BoxFit.contain,
        controller: this,
        //animation: widget.animationName,
        sizeFromArtboard: true
    );

Now the advance function will stop after the animation completes.

I think this also comes from a misunderstanding of how the controller works in tandem with FlareActor. We need to do a better job documenting this.

[yjoo9317]

@luigi-rosso I've confirmed that it stopped calling. Thank you for that.

But frankly I cannot say I am comfortable with this.
I admit that if there are things that I didn't know about that then it would be all my bad.
Examples that I've seen are like that and I started working based on those examples.
However by seeing controller can be overridden by flr file's internal property, the reason for using the controller becomes bland.

[luigi-rosso]

Glad that worked!

I'm sorry for the frustration you're experiencing. All the examples that use a base animation from FlareActor do so with a specific intention, I'm happy to outline why each one does if you have questions about specific ones. For example, the penguin does it because there is always a base walk cycle looping, so might as well let FlareActor drive it while the controller simply mixes the head movement on top of it. The Teddy example uses no animation on FlareActor and simply lets the controls drive everything. Changing the logic of FlareActor to solely respect the controller would break a lot of other apps in production today and would be a fundamental shift from how the system currently works.

The controller is meant to augment how FlareActor works, but FlareActor is the final decision maker for whether the widget needs to repaint (and advance). This is because FlareActor is designed to be extremely high level, offering solutions for a wide variety of problems.

You're looking for direct control of the entire animation, which is awesome. We encourage that. But to do this you should either not supply an animation to FlareActor, or build a light-weight widget that only uses a controller to drive the animation. Something similar to the FlareArtboard example widget I linked earlier.

FlareActor is meant to be a generic tool to get you quickly started using Flare with Flutter. To go deep, we strongly recommend you build the widgets you need for your use case. We need to do a better job of showing how to do this. It'll keep both of our codebases lighter weight and more efficient.

[luigi-rosso]

The changes from the optimal_loading branch have now been merged and published in flare_flutter: 1.6.4. Please give that a try!

Thanks again for all your efforts with contributing back to Flare, it is immensely appreciated.

If you'd like to keep the discussion of the controller driven animations vs FlareActor driven ones going, I'd suggest maybe we open a separate issue? Or perhaps we can do it over email, although I do like the idea of doing it publicly so the community can see where changes to the API originated.

[yjoo9317]

@luigi-rosso
Thanks for the quick release.
I will try the 1.6.4 and test internal flows to see if it works.
When I tried with optimal_load branch, it worked.
So I don't expect to see any flaw.

[yjoo9317]

@luigi-rosso
I tested with latest release, the animations in our internal flows played really well.
FlareCacheBuilder was awesome.
Thanks.

[dannyvalentesonos]

@luigi-rosso We're seeing an exception being thrown in our app from the new flare code.

Unhandled Exception: NoSuchMethodError: The method 'inheritFromWidgetOfExactType' was called on null.
	Receiver: null
	Tried calling: inheritFromWidgetOfExactType(DefaultAssetBundle)
	#0      Object.noSuchMethod (dart:core-patch/object_patch.dart:51:5)
	#1      DefaultAssetBundle.of (package:flutter/src/widgets/basic.dart:5435:47)
	#2      _FlareCacheBuilderState.bundle (package:flare_flutter/flare_cache_builder.dart:43:48)
	#3      _FlareCacheBuilderState._updateWarmth (package:flare_flutter/flare_cache_builder.dart:73:24)
	#4      _FlareCacheBuilderState._warmup.<anonymous closure> (package:flare_flutter/flare_cache_builder.dart:58:11)
	#5      _rootRunUnary (dart:async/zone.dart:1132:38)
	#6      _CustomZone.runUnary (dart:async/zone.dart:1029:19)
	#7      _FutureListener.handleValue (dart:async/future_impl.dart:137:18)
	#8      Future._propagateToListeners.handleValueCallback (dart:async/future_impl.dart:678:45)
	#9      Future._propagateToListeners (dart:async/future_impl.dart:707:32)
	#10     Future._completeWithValue (dart:async/future_impl.dart:522:5)
	#11     _AsyncAwaitCompleter.complete (dart:async-patch/async_patch.dart:30:15)
	#12     _completeOnAsyncReturn (dart:async-patch/async_patch.dart:288:13)
	#13     Cache.getAsset (package:flare_flutter/cache.dart)
	<asynchronous suspension>
	#14     cachedActor (package:flare_flutter/flare_cache.dart:35:16)
	<asynchronous suspension>
	#15     _FlareCacheBuilderState._warmup (package:flare_flutter/flare_cache_builder.dart:52:9)
	#16     _FlareCacheBuilderState.initState (package:flare_flutter/flare_cache_builder.dart:25:5)
	#17     StatefulElement._firstBuild (package:flutter/src/widgets/framework.dart:4085:58)
	#18     ComponentElement.mount (package:flutter/src/widgets/framework.dart:3936:5)
	#19     Element.inflateWidget (package:flutter/src/widgets/framework.dart:3109:14)

[umberto-sonnino]

@luigi-rosso We're seeing an exception being thrown in our app from the new flare code.

@dannyvalentesonos could you share what you're doing, i.e. Flare files and code? That'd help us trace any issues!

[dannyvalentesonos]

We can't share these particular files as the project is huge, and cannot be shared.
However, I've found the remedy. Here's a change that fixes the issue, but I'm not sure if it's the correct solution. In flare_cache_builder.dart, changing

void _warmup() {
    if (_updateWarmth()) {
      return;
    }

    for (final filename in widget.filenames) {
      if (getWarmActor(bundle, filename) == null) {
        cachedActor(bundle, filename).then((FlareCacheAsset asset) {
          if (mounted && asset != null) {
            _assets.add(asset);
            asset.ref();
          }

          _updateWarmth();
        });
      }
    }
  }

to

void _warmup() {
    if (_updateWarmth()) {
      return;
    }

    for (final filename in widget.filenames) {
      if (getWarmActor(bundle, filename) == null) {
        cachedActor(bundle, filename).then((FlareCacheAsset asset) {
          if (mounted && asset != null) {
            _assets.add(asset);
            asset.ref();
            _updateWarmth();
          }
        });
      }
    }
  }

fixes the problem. The issue is that when the cache calls the closure back, the widget is no longer mounted, but _updateWarmth is called anyway, which tries to get the DefaultAssetBundle.of(context) but our context is not valid because we're not mounted.

[dannyvalentesonos]

@luigi-rosso @umberto-sonnino Any chance we can get a fix for this?
Thanks!

[luigi-rosso]

@dannyvalentesonos can you try cloning the warmth_fix branch, update your pubspec to point to the relative path of that flare_flutter folder, and see if that fixes the issue for you? If it does, I can get a new flare_flutter published shortly!

[dannyvalentesonos]

@luigi-rosso Thanks. That fixed the exception! And I don't see any regressions with all our cases. Once you get it deployed, we'll do a better regression test to make sure, but all looks good.

Thanks for acting quickly!

[luigi-rosso]

flare_flutter: 1.6.5 should be up, please let me know if you run into any issues!