"Readable" LV_COLOR_DEPTH option in lv_conf.h

[C47D]

Hi,

@embeddedt I think you can help me out on this one, I'm comparing the current lv_conf.h with the lv_conf_internal.h. This is because we want to handle all configurations inside lv_conf_internal.h so we don't need lv_conf.h to use LVGL as an ESP-IDF component.

Here's the issue:

Currently the LV_COLOR_DEPTH configuration looks like this in lv_conf.h:

/* Color depth:
 * - 1:  1 byte per pixel
 * - 8:  RGB233
 * - 16: RGB565
 * - 32: ARGB8888
 */
#if defined (CONFIG_LV_COLOR_DEPTH_1)
#define LV_COLOR_DEPTH     1
#elif defined (CONFIG_LV_COLOR_DEPTH_8)
#define LV_COLOR_DEPTH     8
#elif defined (CONFIG_LV_COLOR_DEPTH_16)
#define LV_COLOR_DEPTH     16
#elif defined (CONFIG_LV_COLOR_DEPTH_32)
#define LV_COLOR_DEPTH     32
#endif

In lv_conf_internal.h:

/* Color depth:
 * - 1:  1 byte per pixel
 * - 8:  RGB332
 * - 16: RGB565
 * - 32: ARGB8888
 */
#ifndef LV_COLOR_DEPTH
#  ifdef CONFIG_LV_COLOR_DEPTH
#    define LV_COLOR_DEPTH CONFIG_LV_COLOR_DEPTH
#  else
#    define  LV_COLOR_DEPTH     16
#  endif
#endif

To make them look alike we need to define CONFIG_LV_COLOR_DEPTH as an integer out of a Kconfig choice (which I don't think is possible), but applying this workaround it is.

Applying the workaround to our Kconfig it looks like this:

choice LV_COLOR_DEPTH_BOOL
        prompt "Color depth."
        default LV_COLOR_DEPTH_16
        help
            Color depth to be used.

        config LV_COLOR_DEPTH_32
            bool "32: ARGB8888"
        config LV_COLOR_DEPTH_16
            bool "16: RGB565"
        config LV_COLOR_DEPTH_8
            bool "8: RGB232"
        config LV_COLOR_DEPTH_1
            bool "1: 1 byte per pixel"
    endchoice

# Without prompt the user don't see this configuration but the symbol is generated
config LV_COLOR_DEPTH
   int
   default 16
   default 1 if LV_COLOR_DEPTH_1
   default 8 if LV_COLOR_DEPTH_8
   default 16 if LV_COLOR_DEPTH_16
   default 32 if LV_COLOR_DEPTH_32

And the generated symbols:

#define CONFIG_LV_COLOR_DEPTH_16 1
#define CONFIG_LV_COLOR_DEPTH 16

New lv_conf.h

/* Color depth:
 * - 1:  1 byte per pixel
 * - 8:  RGB233
 * - 16: RGB565
 * - 32: ARGB8888
 */
#define LV_COLOR_DEPTH     CONFIG_LV_COLOR_DEPTH

Which pretty much matches the lv_conf_internal.h and personally I think it's much easier to read.

What do you think @embeddedt and @kisvegabor? Should I apply this change and maybe find more places to apply it to?

[C47D]

Same applies to options like this:

/* 1: Use other blend modes than normal (`LV_BLEND_MODE_...`)*/
#if defined CONFIG_LV_FEATURE_USE_BLEND_MODES
   #define LV_USE_BLEND_MODES      1
#else
    #define LV_USE_BLEND_MODES      0
#endif

CONFIG_LV_FEATURE_USE_BLEND_MODES should have the value 0 or 1, using the workaround we can make that:

/* 1: Use other blend modes than normal (`LV_BLEND_MODE_...`)*/
#define LV_USE_BLEND_MODES      CONFIG_LV_FEATURE_USE_BLEND_MODES

Again, much easier to read, but this work will increment the Kconfig size and work on it.

[kisvegabor]

Hi Carlos,

I think it's important to keep lv_conf_internal.h generic and make it possible to generate is from lv_conf.h. Therefore I like your workaround for LV_COLRO_DEPTH and other special cases.

However, I think we should handle simple bool values in lv_conf_internal.h automatically as there are a lot of them.

In lvgl parts are guarded like

#if LV_USE_BLEND_MODES
...
#endif

So setting LV_USE_BLEND_MODES false or true will still work. Or isn't this the case?

[embeddedt]

Kconfig doesn't output any define if a boolean is false, so I think what you would end up with is LV_USE_BLEND_MODES having an empty value. Usually the compiler will complain about that if you try to use it in an #if statement.

[kisvegabor]

Now it looks like this in lv_conf_internak.h:

#ifndef LV_USE_BLEND_MODES
#  ifdef CONFIG_LV_USE_BLEND_MODES
#    define LV_USE_BLEND_MODES CONFIG_LV_USE_BLEND_MODES
#  else
#    define  LV_USE_BLEND_MODES      1
#  endif
#endif

So if the a value is not defined then we end up with the default value of LVGL.
Probably we should make a policy to have the undefined values to 0/false/disabled.

[C47D]

Therefore I like your workaround for LV_COLRO_DEPTH and other special cases.

Ok, will start pushing changes to the Kconfig where the workaround fits.

[stale]

This issue or pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[C47D]

I'm starting to hate this bot... This issue should be solved once we get the Kconfig file into lvgl.

[embeddedt]

You can use the pinned label to disable the bot on certain issues. I've added it to this one.

[kisvegabor]

You can also modify the settings of the bot here:
https://github.com/lvgl/lv_port_esp32/blob/master/.github/stale.yml