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_obj_set_property() with selector #6166

Open
hyansuper opened this issue May 2, 2024 · 12 comments · May be fixed by #6275
Open

lv_obj_set_property() with selector #6166

hyansuper opened this issue May 2, 2024 · 12 comments · May be fixed by #6275
Labels

Comments

@hyansuper
Copy link

Introduce the problem

can I set property for different state/part with code described in https://docs.lvgl.io/master/others/obj_property.html#a-step-further.
for example, I want to specify PART_INDECATOR color property in the array, how to do it?

Proposal

No response

@hyansuper
Copy link
Author

Oh, I think I figure it out:

lv_property_t props[] = {
    { .id = LV_STYLE_BG_COLOR | LV_PART_INDICATOR, .color = (lv_color_t){.red = 0x11, .green = 0x22, .blue = 0x33}, },
}

LV_OBJ_SET_PROPERTY_ARRAY(obj, props);

This is really neat!

@hyansuper
Copy link
Author

There's a small error:
LV_PROPERTY_ID_BUILTIN_LAST should be larger than LV_PROPERTY_IMAGE_END

@kisvegabor kisvegabor reopened this May 3, 2024
@kisvegabor
Copy link
Member

Hi @hyansuper,

Thanks you for opening this issue.

LV_STYLE_BG_COLOR | LV_PART_INDICATOR

I can't see in the code where/why it should work. See here.

@XuNeo Is the set_property feature still interesting for you? If so we can plan to finish it and make it more compete.

@hyansuper
Copy link
Author

LV_STYLE_BG_COLOR | LV_PART_INDICATOR

my mistake, that won't work😂

@XuNeo
Copy link
Collaborator

XuNeo commented May 4, 2024

I have looked into the issue, and found out no easy solution. Let's discuss.
There are two main solutions.

  1. add selector to all property APIs. There could be requirement to specify selector in a const property array. Thus it may also required to add selector field in lv_property_t.
    Note that selector is only used when id is style ID.
typedef struct {
    lv_prop_id_t id;                /**< property ID, includes style ID(lv_style_prop_t) and widgets property ID.*/
    lv_style_selector_t selector;   /**< style selector, only used for when id is style ID*/
    union {
        int32_t num;                /**< Number integer number (opacity, enums, booleans or "normal" numbers)*/
        const void * ptr;           /**< Constant pointers  (font, cone text, etc)*/
        lv_color_t color;           /**< Colors*/
        lv_value_precise_t precise; /**< float or int for precise value*/
        lv_style_value_t _style;    /**< A place holder for style value which is same as property value.*/
    };
} lv_property_t;

lv_result_t lv_obj_set_property(lv_obj_t * obj, const lv_property_t * value, lv_style_selector_t selector);
lv_property_t lv_obj_get_property(lv_obj_t * obj, lv_prop_id_t id, lv_style_selector_t selector);
  1. Remove style ID support from property API, lv_obj_set_local_style_prop can be used directly. Widget general properties and styles are two categories and they deserve two sets of API.
lv_result_t lv_obj_set_property(lv_obj_t * obj, const lv_property_t * value);
void lv_obj_set_local_style_prop(lv_obj_t * obj, lv_style_prop_t prop, lv_style_value_t value,
                                 lv_style_selector_t selector);

I vote for 2, since mixing them looks really neat.

Oh, I think I figure it out:

lv_property_t props[] = {
    { .id = LV_STYLE_BG_COLOR | LV_PART_INDICATOR, .color = (lv_color_t){.red = 0x11, .green = 0x22, .blue = 0x33}, },
}

LV_OBJ_SET_PROPERTY_ARRAY(obj, props);

This is really neat!

@hyansuper
Copy link
Author

hyansuper commented May 4, 2024

lv_property_t props[] = {
{ .id = LV_STYLE_BG_COLOR | LV_PART_INDICATOR, .color = (lv_color_t){.red = 0x11, .green = 0x22, .blue = 0x33}, },
}

Do you mean to make this usable?
I like this solution too, you already mixed data type into id by bitwise operation, we can also mix in selectors

Widget general properties and styles are two categories and they deserve two sets of API.

I am not sure, is label_long_mode considered a property or a style? to me it controls how label looks, just like label size.

@XuNeo
Copy link
Collaborator

XuNeo commented May 4, 2024

you already mixed data type into id by bitwise operation, we can also mix in selectors

The issue is that selector is 32bit long(state + part). So id alone is not enough.

I am not sure, is label_long_mode considered a property or a style? to me it controls how label looks, just like label size.

It feels like style is a special property one widget can have. Widget can also have normal properties like label long mode etc.

Do you mean to make this usable?

I mean that we should drop style prop ID support from API lv_obj_set_property and call lv_obj_set_local_style_prop (with some helpers) if you want to set a style value.

@hyansuper
Copy link
Author

Hi @XuNeo , I hope you keep the property API that unifies style and properties.

It's ok if selectors can't be used. I can add 'LV_PROPERTY_BAR_INDICATOR_COLOR' if I need it. just like you did in lv_image.c.

With lv_obj_set_property(), I can make my device swap UI easily by parsing a portable bin file that contains a list of property id-value pairs.

@lvgl-bot
Copy link

We need some feedback on this issue.

Now we mark this as "stale" because there was no activity here for 14 days.

Remove the "stale" label or comment else this will be closed in 7 days.

@lvgl-bot lvgl-bot added the stale label May 21, 2024
@XuNeo
Copy link
Collaborator

XuNeo commented May 21, 2024

Not stale.

Hi @kisvegabor ,

What do you think of adding selector filed to property struct and APIs?

@XuNeo
Copy link
Collaborator

XuNeo commented May 21, 2024

Hi @XuNeo , I hope you keep the property API that unifies style and properties.

Since style is a special property, how about change the value of property for style, to include state and part selectors?

typedef struct {
    lv_prop_id_t id;
    union {
        int32_t num;                /**< Number integer number (opacity, enums, booleans or "normal" numbers)*/
        const void * ptr;           /**< Constant pointers  (font, cone text, etc)*/
        lv_color_t color;           /**< Colors*/
        lv_value_precise_t precise; /**< float or int for precise value*/
        struct {
            uint32_t selector;
            lv_style_value_t _style;
        } style;
    };
} lv_property_t;

The struct size will 4bytes longer of cause.

@hyansuper
Copy link
Author

Sounds good to me😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants