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

lv_disp: invoke scr_load_internal upon completion #5062

Merged
merged 1 commit into from Dec 21, 2023

Conversation

Lzw655
Copy link
Contributor

@Lzw655 Lzw655 commented Dec 20, 2023

Description of the feature or fix

[Version] release/v8.3
[Bugfix] An error involving accessing a null pointer occurs when using the following program:

    lv_obj_t *screen1 = lv_obj_create(NULL);
    lv_obj_set_style_bg_color(screen1, lv_palette_main(LV_PALETTE_BLUE), 0);

    lv_obj_t *screen2 = lv_obj_create(NULL);
    lv_obj_set_style_bg_color(screen2, lv_palette_main(LV_PALETTE_GREEN), 0);

    lv_scr_load_anim(screen1, LV_SCR_LOAD_ANIM_OVER_LEFT, 2000, 0, false);
    lv_scr_load_anim(screen2, LV_SCR_LOAD_ANIM_OVER_RIGHT, 1000, 500, false);

I think the reason is: If call the lv_scr_load_anim() function consecutively twice, and meet the condition time != 0 || delay != 0, on the second call, because scr_load_internal() sets d->scr_to_load = 0, immediately following functions like lv_obj_set_pos() will operate on d->scr_to_load.

Checkpoints

Be sure the following conventions are followed:

  • Follow the Styling guide
  • Prefer enums instead of macros. If inevitable to use defines export them with LV_EXPORT_CONST_INT(defined_value) right after the define.
  • In function arguments prefer type name[] declaration for array parameters instead of type * name
  • Use typed pointers instead of void * pointers
  • Do not malloc into a static or global variables. Instead declare the variable in lv_global_t structure in lv_global.h and mark the variable with (LV_GLOBAL_DEFAULT()->variable) when it's used. See a detailed description here.
  • Widget constructor must follow the lv_<widget_name>_create(lv_obj_t * parent) pattern.
  • Widget members function must start with lv_<module_name> and should receive lv_obj_t * as first argument which is a pointer to widget object itself.
  • structs should be used via an API and not modified directly via their elements.
  • struct APIs should follow the widgets' conventions. That is to receive a pointer to the struct as the first argument, and the prefix of the struct name should be used as the prefix of the function name too (e.g. lv_disp_set_default(lv_disp_t * disp))
  • Functions and structs which are not part of the public API must begin with underscore in order to mark them as "private".
  • Arguments must be named in H files too.
  • To register and use callbacks one of the following needs to be followed (see a detailed description here):
    • For both the registration function and the callback pass a pointer to a struct as the first argument. The struct must contain void * user_data field.
    • The last argument of the registration function must be void * user_data and the same user_data needs to be passed as the last argument of the callback.
    • Callback types not following these conventions should end with xcb_t.

@XuNeo
Copy link
Collaborator

XuNeo commented Dec 20, 2023

Could you add your program to test cases?
See here:

void test_screen_load_no_crash(void)

@Lzw655
Copy link
Contributor Author

Lzw655 commented Dec 20, 2023

@XuNeo The release/v8.3 branch seems to lack this file. Should I make the modifications in the master branch?

@XuNeo
Copy link
Collaborator

XuNeo commented Dec 20, 2023

Should I make the modifications in the master branch?

Yes. Please add another PR with fix and test to master branch.

@Lzw655
Copy link
Contributor Author

Lzw655 commented Dec 20, 2023

Should I make the modifications in the master branch?

Yes. Please add another PR with fix and test to master branch.

Done. #5066

@kisvegabor
Copy link
Member

Thank you! Please add test_screen_load.c from master here as well so that we have a basic confidence about this feature is still working well.

@Lzw655
Copy link
Contributor Author

Lzw655 commented Dec 21, 2023

Thank you! Please add test_screen_load.c from master here as well so that we have a basic confidence about this feature is still working well.

Ok, added it.

@kisvegabor
Copy link
Member

Cool, thank you!

@kisvegabor kisvegabor merged commit b1284c3 into lvgl:release/v8.3 Dec 21, 2023
17 checks passed
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

3 participants