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

Fix one frame missing #529

Merged
merged 1 commit into from
Sep 19, 2022
Merged

Fix one frame missing #529

merged 1 commit into from
Sep 19, 2022

Conversation

mmaciola
Copy link
Contributor

@mmaciola mmaciola commented Sep 14, 2022

As mStartFrame and mEndFrame was counted from 0 and totalFrame() was
calculated as a difference, there were always one frame missing and
the animation rescaled. This patch adds one to the total frames count.
As there is 0.01 subtracted (copied from android implementation, needed
for some resources where frame no is non-whole number), added +2, not +1.
It fixes the problem for all tested resources- requested and all in example/resource.

issue: #527

Comment on lines 542 to 543
size_t totalFrame() const { return mEndFrame - mStartFrame + 2; }
long frameDuration() const { return mEndFrame - mStartFrame; }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also searched for code related to this issue in android implement.
and I found the code for -0.1f. (https://github.com/airbnb/lottie-android/blob/456332497619eee69fcf82a8005ed58c95761a5c/lottie/src/main/java/com/airbnb/lottie/parser/LottieCompositionMoshiParser.java#L69 )

And I also found implementations for duration.
(https://github.com/airbnb/lottie-android/blob/da7d284002fccd4226877ea3c4107e2c7246f107/lottie/src/main/java/com/airbnb/lottie/LottieComposition.java#L216)

But I couldn't find it for totalFrame.
I'm not sure if adding 2 frame to totalFrame will cover all scenarios.
but when I tested on a few resources there were no problems.

What do you think? @hermet @smohantty

@mmaciola
Copy link
Contributor Author

@JSUYA Thanks for review. I have force pushed a change.
I changed the condition < to <= so no need for +2 but +1 and I added lround as long type is used, not float as in android implementation.

As mStartFrame and mEndFrame was counted from 0 and totalFrame() was
calculated as a difference, there were always one frame missing and
the animation rescaled. This patch adds one to the total frames count.

issue: Samsung#527
@mmaciola
Copy link
Contributor Author

As lround(), no need for - 0.01f

Copy link
Collaborator

@JSUYA JSUYA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@JSUYA JSUYA merged commit 12facb8 into Samsung:master Sep 19, 2022
rlaguilar added a commit to rlaguilar/rlottie that referenced this pull request Nov 15, 2022
* lottiemodel: Improve opacity calculation for gradient stop

Basically, Graeidnt stop's color and opacity are provided as separate arrays.
Stop position and opacity position do not match each other.
Existing code is a sequential approach. It caused problems in various cases of positions.
The improved logic repeats the loop, but no exceptions are raised. It's not complicated, it's simple.

This code referenced the lottie-android library.
https://github.com/airbnb/lottie-android/blob/master/lottie/src/main/java/com/airbnb/lottie/parser/GradientColorParser.java

* replace uint ushort and uchar typedefs with uint32 uint16 and uint8
-- added format script to run clang format on latest commit.

* replace unsigned int with uint32_t in Int()

Signed-off-by: wangxuedong <wangxuedong@xiaomi.com>
Change-Id: I4390c12fcf3aefccfe20290436b0dad96877008c

* Add lottie_init() and lottie_shutdown() c api.

To support dynamic loading and unloading of rlottie library safely
we need to deallocate the resource cache as well as safely shutdown all the
worker threads.
current patch only stops the Render and Rle task schedulers when lottie_shutdown is called.

Things yet to be implemented during shutdown phase
- Unload image loader if loaded dynamically.
- Check if we can release some cache resources.
- Currently multiple load and unload of rlottie library will not work as we are not starting the
  scheduler again when lottie_init() called multiple time in the same process.

* add formal parameter void to lottie_init() and lottie_shutdown()

Signed-off-by: wangxuedong <wangxuedong@xiaomi.com>
Change-Id: I40dbe4b07f9581e5042e468d3860e135622580c8

* Ignore animations with objects of unspecified type

* Check border of color table while generating gradient

* Delete accidentally committed Vim swap file

* Fix one frame missing (Samsung#529)

As mStartFrame and mEndFrame was counted from 0 and totalFrame() was
calculated as a difference, there were always one frame missing and
the animation rescaled. This patch adds one to the total frames count.

issue: Samsung#527

* example: lottieviewer - fixed frame no (Samsung#528)

There is a problem (issue Samsung#527) with totalFrame() in rlottie.
As lottieviewer didn't showed the actual frame number, but calculated it from
progress, it camouflaged the problem. Now the actual frame number and the
total number of frames are displayed.

Signed-off-by: wangxuedong <wangxuedong@xiaomi.com>
Co-authored-by: JunsuChoi <jsuya.choi@samsung.com>
Co-authored-by: Subhransu Mohanty <sub.mohanty@samsung.com>
Co-authored-by: wangxuedong <wangxuedong@xiaomi.com>
Co-authored-by: Nicholas Guriev <nicholas@guriev.su>
Co-authored-by: Michal Maciola <71131832+mmaciola@users.noreply.github.com>
rlaguilar pushed a commit to rlaguilar/rlottie that referenced this pull request Nov 15, 2022
As mStartFrame and mEndFrame was counted from 0 and totalFrame() was
calculated as a difference, there were always one frame missing and
the animation rescaled. This patch adds one to the total frames count.

issue: Samsung#527
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants