lvgl / lvgl_esp32_drivers

Drivers for ESP32 to be used with LVGL
MIT License
339 stars 284 forks source link

Question: Why is the default buffer size for the ILI9341 larger than for similar display controllers in lvgl_helpers.h? #27

Closed jockm closed 3 years ago

jockm commented 3 years ago

If we look in lvgl_helpers.h where DISP_BUF_SIZE is set/calculated you will see:

#if defined (CONFIG_LV_TFT_DISPLAY_CONTROLLER_ST7789)
#define DISP_BUF_SIZE  (LV_HOR_RES_MAX * 40)
#elif defined CONFIG_LV_TFT_DISPLAY_CONTROLLER_ST7735S
#define DISP_BUF_SIZE  (LV_HOR_RES_MAX * 40)
#elif defined CONFIG_LV_TFT_DISPLAY_CONTROLLER_ST7796S
#define DISP_BUF_SIZE  (LV_HOR_RES_MAX * 40)
#elif defined CONFIG_LV_TFT_DISPLAY_CONTROLLER_HX8357
#define DISP_BUF_SIZE  (LV_HOR_RES_MAX * 40)
#elif defined CONFIG_LV_TFT_DISPLAY_CONTROLLER_SH1107
#define DISP_BUF_SIZE  (LV_HOR_RES_MAX * LV_VER_RES_MAX)
#elif defined CONFIG_LV_TFT_DISPLAY_CONTROLLER_ILI9481
#define DISP_BUF_SIZE  (LV_HOR_RES_MAX * 40)
#elif defined CONFIG_LV_TFT_DISPLAY_CONTROLLER_ILI9486
#define DISP_BUF_SIZE  (LV_HOR_RES_MAX * 40)
#elif defined CONFIG_LV_TFT_DISPLAY_CONTROLLER_ILI9488
#define DISP_BUF_SIZE  (LV_HOR_RES_MAX * 40)
#elif defined CONFIG_LV_TFT_DISPLAY_CONTROLLER_ILI9341
#define DISP_BUF_SIZE  (LV_HOR_RES_MAX * 64)

Why is the DISP_BUF_SIZE 1.6x that of similar controllers? I find it odd because it is very common for for ILI9341 based displays to be 320x240 and 64 doesn't divide cleanly into 240. 64 does however divide cleanly into 320, as in 480x320 displays we see associated the the ILI9486/8.

Also is there a reasoning behind the size of these buffers that could be documented? If there were it would certainly help those who write their own drivers understand what their default buffer size should be

C47D commented 3 years ago

Hi @jockm,

Thanks for your patience, most of the drivers were contributed over time, and as far as I remember the first one was ILI9341, there's no reasoning behind the buffer size, it's mostly the size that worked for the users.

I've seen your other issues about the usage of LVGL on the ESP32-S2 and I will try to help to improve this repo to achieve that. Do you think having the display buffer size available to the user on the menuconfig interface helpful? Maybe an option to use the default one (the one you showed above) or letting the user configure LVGL by themselfs?

I hope you can help us to make this example repo more useful 😃

jockm commented 3 years ago

@C47D I have mixed feelings about menuconfig and am not sure it is always the right way to specify settings for something like LVGL; but at minimum I do think having the ability to specify the buffer size and if there are one or two buffers in menuconfig would be a very good thing

And I would suggest that all magic numbers should be documented, even if to say there is no specific rational why a specific value was chosen. 64 only makes sense for a 240x320 (ie portrait) or somethingx320 (like 480x320) which is quite unusual for the ILI9341. I would suggest making it 40 like the others and having a specific STANDARD_BUFFER_MULTIPLIER constant for it

C47D commented 3 years ago

Thanks for the suggestions @jockm , I will transfer this issues to the lvgl_esp32_drivers repo to solve it over there.

C47D commented 3 years ago

@jockm Can you check if #30 works for you?

jockm commented 3 years ago

Sorry for the late reply, yes that is perfect!

C47D commented 3 years ago

Ok, I will merge it, thanks for the feedback!