Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Add support for AnimatedVisualPlayer #131

Open
ShankarBUS opened this issue Jul 8, 2020 · 9 comments
Open

Proposal: Add support for AnimatedVisualPlayer #131

ShankarBUS opened this issue Jul 8, 2020 · 9 comments
Assignees
Labels
enhancement New feature or request

Comments

@ShankarBUS
Copy link
Contributor

ShankarBUS commented Jul 8, 2020

Summary

The proposal is to add support for AnimatedVisualPlayer

Rationale

As a part of creating a toolkit for this library, I found out that WCT has LottieVisualSource which depends on WinUI's IAnimatedVisualSource and AnimatedVisualPlayer.

But this library doesn't have any support for AnimatedVisualPlayer. The goal of this library is to provide WPF everything that WinUI provides UWP. So having a support for AnimatedVisualPlayer would help accomplishing the goal.

For your part, Uno Platform already support and have the source code of AnimatedVisualPlayer so porting it won't be that hard on your side.

For my part, there is a library called LottieSharp which has a LottieAnimationView control that can be embedded inside AnimatedVisualPlayer once it is supported. And Uno Platform also has the source code for LottieVisualSource

Note : The APIs in the source code provided by Uno Platform is different from WinUI. So porting AnimatedVisualPlayer directly from WinUI will be a good idea. But it has dependency on Composition. What should we do?

There may be some differences in API due to the Compositor dependency in them, but a basic support like what Uno Platform provides would be good.

If you find this as out-of-scope or hard to maintain/implement or not necessary, you can close this issue

@ShankarBUS
Copy link
Contributor Author

ShankarBUS commented Jul 8, 2020

I tried to port AnimatedVisualPlayer from Uno Platform and I have a working control. But I have to a add Child property to AnimatedVisualPlayer (since its a FrameworkElement we can't add visual child from outside) to host the LottieAnimationView inside it.

WinUI doesn't have this child property the AnimatedVisualPlayer gets the visual child from the animated visual source using an interface called IAnimatedVisualSource. The AnimatedVisualPlayer in WinUI manages everything by itself. This is due to the mismatch of code in Uno Platform and WinUI.

@Kinnara Kinnara added the enhancement New feature or request label Jul 12, 2020
@Kinnara
Copy link
Owner

Kinnara commented Jul 12, 2020

This would definitely be useful, especially considering that Lottie is seeing more use in WinUI (e.g. for new ProgressRing animations). Given the dependencies of LottieSharp (net461, DotNetZip, Newtonsoft.Json, and SharpDX), maybe it makes sense to make it (potentially together with the new ProgressRing) a standalone library in the toolkit?

@ShankarBUS
Copy link
Contributor Author

The new ProgressRing uses a IAnimatedVisualSource called ProgressRingIndeterminate for its templated AnimatedVisualPlayer element. The IAnimatedVisualSource is generated by Lottie-Gen and is different from LottieVisualSource.

The progress ring visual (which uses composition shapes 😭) is a generated c++ code while the LottieVisualSource uses a JSON file to generate visuals (same composition shapes but we somehow can do with this LottieAnimationView from LottieSharp).

There must be support for AnimatedVisualPlayer in the main library first.

And I will somehow manage to create the LottieVisualSource.

I'm obligated to say how Uno Platform tackles the ProgressRing thing, they have the new ProgressRing with a AnimatedVisualPlayer but it will only work if the user install another nuget package called Uno.UI.Lottie. We can do the same for this.

Because having two controls with same name will be confusing. If the user have the lottie package they'll have the new UI else the control should fallback to the old UI

@ShankarBUS
Copy link
Contributor Author

Given the dependencies of LottieSharp (net461, DotNetZip, Newtonsoft.Json, and SharpDX), maybe it makes sense to make it (potentially together with the new ProgressRing) a standalone library in the toolkit?

I don't understand what this means. Yes, I gonna create a separate package called ModernWpf.Toolkit.UI.Lottie which will take dependencies on this library and LottieSharp (->DotNetZip, Newtonsoft.Json, and SharpDX). This package will be similar to Microsoft.Toolkit.Uwp.UI.Lottie (Nuget) and will contain the LottieVisualSource this can be used with the AnimatedVisualPlayer if that could be present in this library

Are you saying that the AnimatedVisualPlayer should also be in the toolkit? Hmm... nice idea, which is possible.

But wouldn't that be unnecessary since users can directly place LottieAnimationView from LottieSharp and control it directly without some in-betweens. Having AnimatedVisualPlayer in the toolkit is useless, directly using LottieSharp will be more efficient

This library won't depend on them, this library just needs support for AnimatedVisualPlayer, which won't need any additional dependencies.

My original point was this lib must have AnimatedVisualPlayer so that users can have benefits of what WinUI provides.
Only the LottieVisualSource should be in the new separate package. If AnimatedVisualPlayer is implemented in this library it will be beneficial for this lib, since there could be some new controls coming to WinUI using AnimatedVisualPlayer like the new AnimatedIcon control in proposal.

If you really believe that the AnimatedVisualPlayer control should be in the toolkit then closing this issue will be a good idea.
Thank you for creating this lib ✌.

@ShankarBUS
Copy link
Contributor Author

ShankarBUS commented Jul 25, 2020

Hey @Kinnara, I kinda reproduced the new ProgressRing (Indeterminate) [WinUI] just using xaml shapes (no Lottie, no nothing) in one of my repo's branch.

ProgressRing
(Left : MS Edge, Right : My repro. Note: Edge's progress ring has been scaled to 100px for clear view)

I reproduced it from the new Microsoft Edge's Clear Browsing Data Page instead of WinUI. Since Edge uses SVG Shapes and CSS Animations instead of Composition Shapes and Composition Animations as in WinUI it was easy to recreate it from Edge.

But for the Determinate ProgressRing with Lottie I have no idea yet.

Thus, I have come to a conclusion.

The AnimatedVisualPlayer control is necessary incase of adding the new progress ring control to this lib. And it can be done. It's easy, it doesn't depend on anything else.

So these are my suggestions :

Mechanism of AnimatedVisualPlayer

  • The AnimatedVisualPlayer passes its compositor to its source (i.e. IAnimatedVisualSource) through IAnimatedVisualSource.TryCreateAnimatedVisual(Compositor, Object) method which returns a IAnimatedVisual.
  • The AnimatedVisualPlayer will host a visual element from the previously fetched IAnimatedVisual through IAnimatedVisual.RootVisual
  • The Compositor has a Progress scalar property set inside by the AnimatedVisualPlayer which will control all the Composition animations of the IAnimatedVisual (the IAnimatedVisualSource has no purpose other than providing the IAnimatedVisual)

We can do most of these except for the Compositor part which requires sacrifice in consistency with WinUI's APIs

So for that we can create a faux Compositor like object that will only have the progress [0.0f to 1.0f] property and the progress property can be used to seek the Storyboard Animations (instead of Composition Animations) of the IAnimatedVisual.

@ShankarBUS ShankarBUS reopened this Jul 25, 2020
@ShankarBUS
Copy link
Contributor Author

ShankarBUS commented Jul 25, 2020

Hey @Kinnara, see commit 322e93b I have added a Slider and two buttons to seek, play and pause. In my demo the storyboard was started after load and it handles its own clock, just like a normal storyboard. But once the AnimatedVisualPlayer is implemented, it should handle everything (progress, play, pause, etc.) not the storyboard itself.

Once the AnimatedVisualPlayer is implemented, the code must be changed in way that the storyboard should be paused immediately after starting it (Like this spinInfinite.Begin(); spinInfinite.Pause();, calling Begin() is mandatory for the storyboard to work) and let the AnimatedVisualPlayer do all the work.
See my example on how seeking works. The playing and pausing should be handled by the AnimatedVisualPlayer, the storyboard should only listen to the progress of the faux Compositor provided through IAnimatedVisualSource.TryCreateAnimatedVisual(Compositor, Object) by the AnimatedVisualPlayer.

Thus, proven it will work and mostly be similar API-wise and UI-wise

Edit :

My sample is consistent only with MS Edge, the WinUI progressring is rotated -90 degrees and have a different stroke thickness

@Kinnara Kinnara self-assigned this Aug 1, 2020
@ShankarBUS
Copy link
Contributor Author

Hey @Kinnara, I would like to implement this feature myself and contribute to this project.

Am I allowed to open a PR? Would you like to see this implemented?

You don't seem to be that active on GitHub for the last 6 months. if you have a tight schedule and don't have free time, I can add this to the toolkit instead.

What do you say?

@Kinnara
Copy link
Owner

Kinnara commented Oct 12, 2020

Hi @ShankarBUS, of course you are allowed to open a PR, and it's very welcome! My free time is indeed quite limited, but I'll do my best to ensure such PRs get reviewed and merged within a reasonable time frame.

@ShankarBUS
Copy link
Contributor Author

Ok then, I'll start my work.

Thanks for spending your spare time to keep this library alive 👍.

TBH, I'm impressed by your extraordinary talent 👏👏👏.

Hope you have the determination & will to keep this library well. Take care of yourself my friend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants