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(anim_timeline) avoid calling lv_anim_del(NULL, NULL) #2628

Merged
merged 28 commits into from Oct 15, 2021

Conversation

FASTSHIFT
Copy link
Collaborator

Description of the feature or fix

This PR is about the resolution of #2589.
Avoid calling lv_anim_del(NULL, NULL) to cause all animations to be deleted.

Checkpoints

FASTSHIFT and others added 24 commits June 12, 2021 16:08
Signed-off-by: FASTSHIFT <vifextech@foxmail.com>
Signed-off-by: FASTSHIFT <vifextech@foxmail.com>
Signed-off-by: FASTSHIFT <vifextech@foxmail.com>
Signed-off-by: FASTSHIFT <vifextech@foxmail.com>
…_timeline_set_progress() adds user_data, act_time uses int32_t type
Signed-off-by: FASTSHIFT <vifextech@foxmail.com>
Signed-off-by: FASTSHIFT <vifextech@foxmail.com>
… update lv_anim_timeline_1.c example

Signed-off-by: FASTSHIFT <vifextech@foxmail.com>
Signed-off-by: FASTSHIFT <vifextech@foxmail.com>
Signed-off-by: FASTSHIFT <vifextech@foxmail.com>
Signed-off-by: _VIFEXTech <1290176185@qq.com>
@FASTSHIFT FASTSHIFT changed the title fix(anim-timeline) avoid calling lv_anim_del(NULL, NULL) fix(anim_timeline) avoid calling lv_anim_del(NULL, NULL) Oct 3, 2021
@kisvegabor
Copy link
Member

So far I thought that an animation doesn't make much sens if neither var nor exec_cb is set. But setting only the ready_cb might be also valid use case, especially in case of anim timeline.

An other solution could be to add a dummy callback to exec_cb in lv_anim_timeline_add if it's NULL.

BTW, won't the current solution (in this PR) cause memory leak?

@FASTSHIFT
Copy link
Collaborator Author

So far I thought that an animation doesn't make much sens if neither nor is set. But setting only the might be also valid use case, especially in case of anim timeline.var``exec_cb``ready_cb

An other solution could be to add a dummy callback to in if it's .exec_cb``lv_anim_timeline_add``NULL

BTW, won't the current solution (in this PR) cause memory leak?

Why is there a memory leak, is it lv_anim_timeline or lv_anim?

@kisvegabor
Copy link
Member

Why is there a memory leak, is it lv_anim_timeline or lv_anim?

Because if both var and exec_cb are NULL the animation won't be deleted.

@FASTSHIFT
Copy link
Collaborator Author

Why is there a memory leak, is it lv_anim_timeline or lv_anim?

Because if both var and exec_cb are NULL the animation won't be deleted.

lv_anim will automatically delete the animation when the animation reaches the set end time. No need to actively call lv_anim_del to delete.

@kisvegabor
Copy link
Member

But if an animation is not deleted in lv_anim_timeline_stop it will keep running and it will call its ready_cb. However, the user intention was to make all animations stop.

@FASTSHIFT
Copy link
Collaborator Author

You are right. I thought of a solution: When it is detected that var and exec_cb are NULL, use the pointer of lv_anim_timeline as var, so that the animation can be completely deleted when lv_anim_timeline_stop is called.

@kisvegabor
Copy link
Member

Looks good, thank you!

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

2 participants