Baseflow / LottieXamarin

Render After Effects animations natively on Android, iOS, MacOS and TvOS for Xamarin
https://baseflow.com
Apache License 2.0
1.22k stars 261 forks source link

Scale is not working #299

Closed ChristopherStephan closed 3 years ago

ChristopherStephan commented 3 years ago

🐛 Bug Report

With version 4.0.0 the property "Scale" seems to be broken: the animation is not scaled when changing the property's value. With version 3.1.3 the scaling was working. This bug affects Android and iOS.

A git bisect on branch develop identified several possible first broken commits, from which only 297982db1965dc27904da6d5a40ed3efd22bec5e proved that Scale is not working (other commits listed did not show the Lottie animation):

git bisect start
HEAD is now at 31555da Update versions
git bisect good 31555da0e80bac88ff182d99ec96fcbf45a3c4b7
git bisect bad b1e1988d243552b74b48b620daa3103643e00f3d
Bisecting: 24 revisions left to test after this (roughly 5 steps)
[0e43e58ae5cb752c5e7408a7d073d6d5e0b25bd1] Update props
git bisect bad
Bisecting: 12 revisions left to test after this (roughly 4 steps)
[5dbda688934579e0b87e7ba2d6637ca07900bbac] Update Android
git bisect good
Bisecting: 6 revisions left to test after this (roughly 3 steps)
[2573192210fcbb97256309a1d0a0a6db97ff3762] Cleanup
git bisect skip
Bisecting: 6 revisions left to test after this (roughly 3 steps)
[297982db1965dc27904da6d5a40ed3efd22bec5e] Android updates
git bisect bad
Bisecting: 3 revisions left to test after this (roughly 2 steps)
[729008759e17e3946aac1f61b65a1065688c57c2] Add new properties
git bisect skip
Bisecting: 3 revisions left to test after this (roughly 2 steps)
[ade55b1a4a987e722445e824ba73f422970aed96] More fixes
git bisect skip
Bisecting: 3 revisions left to test after this (roughly 2 steps)
[7953b4f347fcbc30bc732b9ed917626787ca3994] More fixes
git bisect skip
Bisecting: 3 revisions left to test after this (roughly 2 steps)
[b1f530d62c96cd1430b1654588718ba5feae8213] Case sensitive
 git bisect good
There are only 'skip'ped commits left to test.
The first bad commit could be any of:
7953b4f347fcbc30bc732b9ed917626787ca3994
ade55b1a4a987e722445e824ba73f422970aed96
2573192210fcbb97256309a1d0a0a6db97ff3762
729008759e17e3946aac1f61b65a1065688c57c2
297982db1965dc27904da6d5a40ed3efd22bec5e
We cannot bisect more!

Expected behavior

Scale should take effect on the animation view.

Reproduction steps

  <forms:AnimationView
            Animation="LottieLogo1.json"
            Scale="1.8"
            AnimationSource="EmbeddedResource"
            WidthRequest="300"
            HeightRequest="300"/>

Configuration

Version: 4.0.6 (version 4.0.0 introduced the bug)

Platform:

martijn00 commented 3 years ago

Can you make a PR?

ChristopherStephan commented 3 years ago

Probably not. I tried to debug on commit 0811327837b259d2c1c3ec50a9925263e9213775 why Scale is not working but I guess the bug was introduced by a major refactoring, that I do not completely understand.

I guess that this is the root of the problem: Inside the OnElementPropertyChanged of Lottie.Forms.Platforms.Ios.AnimationViewRenderer reacting to Scale changes is commented out:

//if (e.PropertyName == AnimationView.ScaleProperty.PropertyName)
//    _animationView.Scale = Element.Scale;

LOTAnimationView does not contain a definition for it, so it cannot simply be commented in again:

'LOTAnimationView' does not contain a definition for 'Scale' and no accessible extension method 'Scale' accepting a first argument of type 'LOTAnimationView' could be found (are you missing a using directive or an assembly reference?)

I get many of these kind of errors, but still the solution compiles:

Screenshot 2020-11-08 at 19 50 52
martijn00 commented 3 years ago

https://github.com/Baseflow/LottieXamarin/blob/develop/Lottie.Forms/AnimationView.cs#L56 Scale is overwritten with the new keyword here. Maybe that was used in the past?

joshardt commented 3 years ago

I use ScaleX and ScaleY as a workaround. Works perfectly.

martijn00 commented 3 years ago

See if this fixes it https://github.com/Baseflow/LottieXamarin/commit/9e5c61604132b0485fb693b6784bcc886ccc51d5