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

add anim timeline #2309

Merged
merged 15 commits into from
Jul 19, 2021
Merged

add anim timeline #2309

merged 15 commits into from
Jul 19, 2021

Conversation

FASTSHIFT
Copy link
Collaborator

@FASTSHIFT FASTSHIFT commented Jun 11, 2021

Description of the feature or fix

Realize the timeline, support forward and reverse playback of the animation group, support dynamic adjustment of progress.
anim-timeline

Checkpoints

lv_anim_timeline.example.mp4

@FASTSHIFT FASTSHIFT mentioned this pull request Jun 11, 2021
19 tasks
@FASTSHIFT
Copy link
Collaborator Author

@kisvegabor
I don’t know if it’s appropriate to put it under extra/widgets. Do you have any good suggestions?

@FASTSHIFT FASTSHIFT changed the title add anim_timeline add anim timeline Jun 11, 2021
@FASTSHIFT FASTSHIFT force-pushed the anim-timeline branch 2 times, most recently from 6b3fb28 to 2efd8c8 Compare June 11, 2021 03:33
Signed-off-by: FASTSHIFT <vifextech@foxmail.com>
Copy link
Member

@kisvegabor kisvegabor left a comment

Choose a reason for hiding this comment

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

Wow, you have added the progress too. My dream came true 😍

The implementation looks very clean and professional.

Could update the documentation too? Just 1-2 paragraphs about what an anim. time line is and basics of its usage.

I don’t know if it’s appropriate to put it under extra/widgets. Do you have any good suggestions?

I think it's such a useful and great feature can be added right into the core lvgl (src/misc/lv_anim_timeline.c/h). And it's only ~100 lines of code so won't bloat lvgl.

src/extra/widgets/anim_timeline/lv_anim_timeline.c Outdated Show resolved Hide resolved
src/extra/widgets/anim_timeline/lv_anim_timeline.h Outdated Show resolved Hide resolved
Signed-off-by: FASTSHIFT <vifextech@foxmail.com>
@FASTSHIFT FASTSHIFT force-pushed the anim-timeline branch 5 times, most recently from c91ed38 to 8a5dfe3 Compare June 15, 2021 09:15
docs/overview/animation.md Outdated Show resolved Hide resolved
@FASTSHIFT
Copy link
Collaborator Author

@kisvegabor
The document has been uploaded.

@FASTSHIFT FASTSHIFT force-pushed the anim-timeline branch 3 times, most recently from 7288b9c to b22f5ac Compare June 16, 2021 03:03
@FASTSHIFT
Copy link
Collaborator Author

FASTSHIFT commented Jun 16, 2021

@kisvegabor
Can lv_anim add the reverse function? The current timeline reverse playback function is simply to swap the start value and end value, but the path is still forward, I hope it can be reversed on the path.

@FASTSHIFT FASTSHIFT force-pushed the anim-timeline branch 4 times, most recently from 3b728c7 to 9786493 Compare June 18, 2021 06:41
@FASTSHIFT
Copy link
Collaborator Author

@kisvegabor
Is there anything else that needs to be modified?

@amirgon
Copy link
Contributor

amirgon commented Jun 20, 2021

Add Micropython example?
By trying to implement a Micropython example, issues with the API could become clearer.

@kisvegabor In general, I think that asking for a Micropython example for new widgets makes sense both for documentation and for ensuring API compatibility.

@kisvegabor
Copy link
Member

@kisvegabor In general, I think that asking for a Micropython example for new widgets makes sense both for documentation and for ensuring API compatibility.

That's true. @uraich could you take a look at this new feature and the example?

@FASTSHIFT
The CI error can be solved by using exec_cb's like:

static void set_width(void * var, int32_t v)
{
  lv_obj_set_width(var, v);
}

@uraich
Copy link
Contributor

uraich commented Jun 20, 2021

@kisvegabor In general, I think that asking for a Micropython example for new widgets makes sense both for documentation and for ensuring API compatibility.

That's true. @uraich could you take a look at this new feature and the example?

@FASTSHIFT
The CI error can be solved by using exec_cb's like:

static void set_width(void * var, int32_t v)
{
  lv_obj_set_width(var, v);
}

Which is the example? I pulled the latest changes of lv_microprython, lv_bindings and lvgl but could not find the anim_timeline example. Has it been merged?

@amirgon
Copy link
Contributor

amirgon commented Jun 20, 2021

Which is the example? I pulled the latest changes of lv_microprython, lv_bindings and lvgl but could not find the anim_timeline example. Has it been merged?

I think that this new feature and the C example are only part of this PR and not in LVGL yet.

Signed-off-by: FASTSHIFT <vifextech@foxmail.com>
@kisvegabor
Copy link
Member

I also like the use of lv_ainm_t. It allows reusing the individual animations in different timelines too.

@FASTSHIFT
Copy link
Collaborator Author

@kisvegabor Is there anything else to change?

@uraich
Copy link
Contributor

uraich commented Jul 12, 2021

I tried to upgrade and recompile lv_micropython with your fork but get this error:
image
I wanted to give the MP port of your examples another try.

@amirgon
Copy link
Contributor

amirgon commented Jul 12, 2021

@uraich Did you switch to master branch?
dev-8.0 is now obsolete and we are using master. (See this announcement)

@uraich
Copy link
Contributor

uraich commented Jul 12, 2021

When I use lvgl master everything works fine. If I use the fork with anim-timeline I have this problem.

@amirgon
Copy link
Contributor

amirgon commented Jul 12, 2021

When I use lvgl master everything works fine. If I use the fork with anim-timeline I have this problem.

Try to merge master into your branch.

@amirgon
Copy link
Contributor

amirgon commented Jul 12, 2021

@uraich I just tried to build it on my side and I don't see any errors.
I pulled latest from https://github.com/FASTSHIFT/lvgl/tree/anim-timeline and merged lvgl/master into it.
It builds with no errors and I can see the new anim timeline functions:

>>> 
>>> lv.anim_timeline_
anim_timeline_add               anim_timeline_create
anim_timeline_del               anim_timeline_get_playtime
anim_timeline_get_reverse       anim_timeline_set_progress
anim_timeline_set_reverse       anim_timeline_start

@embeddedt / @kisvegabor I think you need to approve the workflows for this PR

@FASTSHIFT
Copy link
Collaborator Author

@kisvegabor

@kisvegabor
Copy link
Member

I've updated the workflow files and now it passes.

If nothing is left from the MP side we can merge!

@amirgon
Copy link
Contributor

amirgon commented Jul 16, 2021

If nothing is left from the MP side we can merge!

To really be sure we need a MP example up and running, including "delete" which is handled differently on MP.
@uraich were you eventually able to build MP and try this? If not, I'll try to find some time next week to create the MP example.

@uraich
Copy link
Contributor

uraich commented Jul 17, 2021

Well, I managed to compile lv_micropython with anim-timeline included, but I get
image
when trying to create an anim_timeline with
anim_timeline = lv.anim_timeline_t()
How do I have to do this correctly? I am also not too sure if passing the object in user_data works in MP.
@amirgon
I have uploaded the translated examples to https://github.com/uraich/lv_mpy_examples_v8/tree/main/anim
so you should have a good starting point if you want to experiment yourself.

@amirgon
Copy link
Contributor

amirgon commented Jul 17, 2021

when trying to create an anim_timeline with
anim_timeline = lv.anim_timeline_t()
How do I have to do this correctly?

lv_anim_timeline_t is an opaque struct so there is no corresponding MP struct:

typedef struct _lv_anim_timeline_t lv_anim_timeline_t;

The anim_timeline functions are available directly under lvgl module.

For example:

anim_timeline = lv.anim_timeline_create()
lv.anim_timeline_add(anim_timeline, start_time, a)

I am also not too sure if passing the object in user_data works in MP.

I'm not sure what you mean, could you explain?

@uraich
Copy link
Contributor

uraich commented Jul 18, 2021

@amirgon : Thanks for the explication.
In MP user_data must be a dictionary or None. Instead of passing the obj through user_data of the anim object, I pass the object directly as a parameter to set_width and set_height through a lambda function.
Comparing lv_example_anim_timeline_1.c to lv_example_anim_timeline_2.c, I don't see much difference and both behave exactly alike. In addition, translation to MP would result in the same code. I therefore only supply lv_example_anim_timeline_1.py. Please correct me if I am wrong.
The new version of lv_example_anim_timeline_1.py now works the same way as lv_example_anim_timeline_1/2.c. I uploaded it to my private github repo (see mail above).
As soon as anim_timeline will be merged into lvgl master, I can create a PR to include this example in the master branch.

@amirgon
Copy link
Contributor

amirgon commented Jul 18, 2021

Instead of passing the obj through user_data of the anim object, I pass the object directly as a parameter to set_width and set_height through a lambda function.

This is the best practice. No need to use user_data directly in most cases. It is used internally by the bindings.

Comparing lv_example_anim_timeline_1.c to lv_example_anim_timeline_2.c, I don't see much difference and both behave exactly alike.

@FASTSHIFT Why 2 examples are provided? Could you explain the difference between them?

The new version of lv_example_anim_timeline_1.py now works the same way as lv_example_anim_timeline_1/2.c. I uploaded it to my private github repo (see mail above).
As soon as anim_timeline will be merged into lvgl master, I can create a PR to include this example in the master branch.

@uraich If you want you can even add it to this PR so it would be pushed and tested together.
You could open a PR on https://github.com/FASTSHIFT/lvgl/tree/anim-timeline, or just ask someone with write access (@FASTSHIFT / @embeddedt / @kisvegabor) to add it directly.


One thing that is not tested in the examples is the deletion of a timeline (lv_anim_timeline_del).
This is especially important for Micropython because deletion must be handled differently using lv_anim_custom_del:

#if LV_ANIM_TIMELINE_CUSTOM_EXEC
lv_anim_custom_del(at->anim_dsc[i].new_anim, (lv_anim_custom_exec_cb_t)a->exec_cb);
#else
lv_anim_del(a->var, a->exec_cb);
#endif

that, however, also requires enabling LV_ANIM_TIMELINE_CUSTOM_EXEC in lv_conf.h on lv_binding_micropython

@FASTSHIFT why not always use lv_anim_custom_del? What is the advantage of calling lv_anim_del instead? If we could always use lv_anim_custom_del then we wouldn't need LV_ANIM_TIMELINE_CUSTOM_EXEC at all, right?

@FASTSHIFT
Copy link
Collaborator Author

FASTSHIFT commented Jul 19, 2021

@amirgon @uraich
The reason for adding lv_example_anim_timeline_2 is to demonstrate the use of user_data to pass anim object. Since lv_example_anim_timeline_1 can work, I deleted lv_example_anim_timeline_2.
Using LV_ANIM_TIMELINE_CUSTOM_EXEC does not seem to make much sense, I also deleted it.
I added an example of lv_anim_timeline_del in lv_example_anim_timeline_1.

… update lv_anim_timeline_1.c example

Signed-off-by: FASTSHIFT <vifextech@foxmail.com>
@kisvegabor
Copy link
Member

I fixed the CI error in the C+C++ test. The MP test fails because this PR contains an older version of LVGL and it's not compatible the MP run by the CI.

I can merge lvgl master here but it will bloat the commit history. So I suggest merging it and if there are any issues fix them in a new PR.

@amirgon
Copy link
Contributor

amirgon commented Jul 19, 2021

I can merge lvgl master here but it will bloat the commit history.

If you rebase this PR on master you would fix it without bloating the commit history.

@kisvegabor
Copy link
Member

If you rebase this PR on master you would fix it without bloating the commit history.

Thanks, done!

As the tests pass I merge it.

Thank you very much for this great feature!

@kisvegabor kisvegabor merged commit 690b354 into lvgl:master Jul 19, 2021
@amirgon
Copy link
Contributor

amirgon commented Jul 19, 2021

The new version of lv_example_anim_timeline_1.py now works the same way as lv_example_anim_timeline_1/2.c. I uploaded it to my private github repo (see mail above).
As soon as anim_timeline will be merged into lvgl master, I can create a PR to include this example in the master branch.

@uraich when you add lv_example_anim_timeline_1.py into lvgl master, could you also add the usage of lv_anim_timeline_del as in the C example?

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.

None yet

4 participants