search

lvgl / lv_binding_micropython

LVGL binding for MicroPython
MIT License
259 stars 166 forks source link

Update master branch to LVGL v9 version #311

Closed PGNetHun closed 10 months ago

PGNetHun commented 10 months ago
PGNetHun commented 10 months ago

@kisvegabor , @XuNeo , @kdschlosser Please review this PR before I merge it to master.

(PS: to have a clean Git history, it will be a squash merge)

PGNetHun commented 10 months ago

@sam0910 : this PR contains the latest LVGL with fixed display drivers (including the fix for your feedback: https://github.com/lvgl/lv_binding_micropython/pull/298#issuecomment-1880908299). Please test this multi-instance branch on your device, and if it works, then give an approve to this PR. Thank you!

kisvegabor commented 10 months ago

Waiting for @sam0910's approval to merge.

PGNetHun commented 10 months ago

This PR works on M5Core2 device (ESP32 SPIRAM, ILI9342C display)

kdschlosser commented 10 months ago

and would you please separate the display drivers into their own files and import the base class into the file. The amount of memory that gets consumed because of importing a bunch of code for drivers that are not being used is simply out of control.

Also get rid of the class level attribute for the init parameters and put them into a function. they get used only once and to have them sitting there eating up memory after the initialization is just crazy. If the init parameters are set in the __init__ function of the driver you can delete them after they are used. But if they are set at the class level then you cannot delete them because if a user has more than one display attached that is the same the init commands will be gone and will not be able to be used when the second display is initialized.

kisvegabor commented 10 months ago

I suggest merging this PR if it's doesn't break anything, and send smaller ones after that.

kdschlosser commented 10 months ago

and TY for fixing that error in gen_mpy. I mentioned the problem as you know and it was never corrected.

kdschlosser commented 10 months ago

since LVGL internally no longer changes things like structures or unions based on the color depth why is there still a reliance on LV_COLOR_DEPTH??

It sets the NATIVE color format enumerations and it is used to set _LV_COLOR_NATIVE_WITH_ALPHA_SIZE which is used in LV_IMAGE_BUF_SIZE_TRUE_COLOR_ALPHA. This macro is not accessible in the binding nor is it used at all in LVGL's internal code.

the color depth is also used in LV_IMAGE_BUF_SIZE_TRUE_COLOR_CHROMA_KEYED and LV_IMAGE_BUF_SIZE_TRUE_COLOR and none of those macros are used anywhere in LVGL code. Those macros are not exposed in the binding and will never get used. So setting LV_COLOR_DEPTH does not need to be done at all. It's is pointless to do so. It adds additional complexity and it also forces the binding to be a compile time thing instead of a runtime thing. If LV_COLOR_DEPTH is not used at all then a user will not have ro compile and flash their firmware if they want to change out to a different display. All they have to do is upload the new python source file that has the new driver code in it. That's it. The sole point to using python is rapid development. having to recompile firmware and flash firmware if changing a piece of hardware that is attached kind of negates the point to using it.

LV_COLOR_DEPTH also sets antialiasing in the display driver but that is easily changed in python code.

XuNeo commented 10 months ago

It's better to use the lv_draw_buf_create and set the display with draw buffer. Unfortunately, the PR https://github.com/lvgl/lvgl/pull/5204 is still ongoing.

After that PR, it's recommended to use below method(and equivalent in python)

  lv_draw_buf_t * buf1 = lv_draw_buf_create(hor_res, ver_res, cf, 0);

  lv_display_set_draw_buffers(disp, buf1, NULL);
sam0910 commented 10 months ago

lv_binding_micropython with lvgl@a9960c62 is working without any issue :) Thank you so much, and I am very excited that I could use the latest v9, with great new features.

I have not measured the detail, but the rendering speed is better than v8. I could feel it :)

kisvegabor commented 10 months ago

I have not measured the detail, but the rendering speed is better than v8. I could feel it :)

Ahhh, what a relief :slightly_smiling_face:

kisvegabor commented 10 months ago

We can remove LV_IMAGE_BUF_SIZE_ as they are not used anymore. See 5244

LV_COLOR_DEPTH still affects a few minor places and we have no time to remove it for v9. We can get beck to this in the near future.

PGNetHun commented 10 months ago

and would you please separate the display drivers into their own files and import the base class into the file. The amount of memory that gets consumed because of importing a bunch of code for drivers that are not being used is simply out of control.

Also get rid of the class level attribute for the init parameters and put them into a function. they get used only once and to have them sitting there eating up memory after the initialization is just crazy. If the init parameters are set in the init function of the driver you can delete them after they are used. But if they are set at the class level then you cannot delete them because if a user has more than one display attached that is the same the init commands will be gone and will not be able to be used when the second display is initialized.

Next step in a new PR.

PGNetHun commented 10 months ago

The new changes are tested on ST7789 display (ESP32), and it works. For me the performance (FPS) is worse than with LVGL 8.3, but I will investigate this issue later.

@sam0910 , could you please test again on your device this updated PR? Thank you!

sam0910 commented 10 months ago

The new changes are tested on ST7789 display (ESP32), and it works. For me the performance (FPS) is worse than with LVGL 8.3, but I will investigate this issue later.

@sam0910 , could you please test again on your device this updated PR? Thank you!

Update ili9xxx, st77xx drivers and examples, remove lv.COLOR_DEPTH is looking great! no issues!

it might not be relevant with this PR, but I found a roller widget has 0ms anim_time by default. so user set roller.set_selected(1, lv.ANIM.ON) they cannot see the anim, unless they set roller.set_style_anim_time(1000, lv.PART.MAIN)

Never mind if it causes by setting LV_USE_THEME_SIMPLE 1(LV_USE_THEME_DEFAULT 0) which is I am using :)

kisvegabor commented 10 months ago

Please note this merged PR: https://github.com/lvgl/lvgl/pull/5204 There are no other planned API changes before v9.

PGNetHun commented 10 months ago

Please note this merged PR: lvgl/lvgl#5204 There are no other planned API changes before v9.

Get stuck with an issue about allocating ESP32 DMA-capable memory for display buffers: https://github.com/lvgl/lv_binding_micropython/issues/313

PGNetHun commented 10 months ago

Please note this merged PR: lvgl/lvgl#5204 There are no other planned API changes before v9.

Get stuck with an issue about allocating ESP32 DMA-capable memory for display buffers: #313

Problem solved in another PR, and it is merged to this multi-instance branch.

Tested on real ESP32 devices with ST7789 and ILI9342C displays.

PGNetHun commented 10 months ago

@kisvegabor , @XuNeo , @kdschlosser , Could you please review & approve this big PR? I don't want to add any more commits to it, so it is ready to merge to master branch. After this, every new PRs should target master. (and prepare the work to upgrade MicroPython to v1.22)

PS: this PR is tested on M5Core2 ESP32 device, and it works

PGNetHun commented 10 months ago

Reminder: disable LV_USE_ARABIC_PERSIAN_CHARS see: https://github.com/lvgl/lvgl/pull/5386

UPDATE: it is already disabled

PGNetHun commented 10 months ago

To have a linear and clean git history on master branch, use "Squash and merge"

kisvegabor commented 10 months ago

@PGNetHun feel free to merge it when it feels appropriate.