lvgl / lvgl

Embedded graphics library to create beautiful UIs for any MCU, MPU and display type.
https://lvgl.io
MIT License
17.55k stars 3.3k forks source link

Reduce deviations from CSS #2156

Closed puzrin closed 3 years ago

puzrin commented 3 years ago

Introduce the problem

v8 api accumulating to many deviations from CSS. Problem is with development costs.

  1. When strict subset implemented - experience of original authors is reused (indirectly). And many pitfails (public and private) are avoided automatically.
  2. When something custom invented, it may take years to find working balance.

Note, i do not insist on CSS. QT or something else could be good to. But it worth to decide what to follow, and keep direction.

Examples and cases

  1. "state" should be part of style, not object.
  2. Too many duplicating properties like LV_STYLE_ARC_COLOR, LV_STYLE_ARC_OPA and so on.
    • Consider arc as "rectangle with arc inside", then it's obvious to use standard properties LV_STYLE_COLOR / LV_STYLE_OPACITY as with any <div> / <span>
    • Verify CSS styles/names for web and drop non-standard (except specific cases like gradient in background image)
  3. "partitions" seems to be inspired by old widgets, and is alien to CSS/DOM
    • Rework widgets structure. For example, use sub-nodes to carry extra styles/drawers, and vendor-styles for the rest.
  4. LV_STYLE_CONTENT_* looks alien. If those are for hidden text node of <div>, then <div> has enough standard css properties to arrange text. No need to invent specific *_CONTENT_* things.
  5. Layout - not from CSS. I'd expect display property and so on.
    • may be this can work, but i don't understand reasons for deviation.

Suggested solution

See comments above

scottandrew commented 3 years ago

My 2cents and see both sides of it. Want to just play Devil's advocate as I am learning V8 and LVGL.

  • "state" should be part of style, not object.

Wouldn't this then break local styles?? If you are strictly part of the style then you can't tweak things easily based on an event like a value change (take a gauge that colors it self green, yellow, red based on value changes) or even changes text styles based on value. I like the idea of being able to do hyper local changes based on events not have to flip out the whole style.

  • Too many duplicating properties like LV_STYLE_ARC_COLOR, LV_STYLE_ARC_OPA and so on.

This threw me off a bit because we have now have a bunch of object calls that set style too. I don't mind arc having some wrapper functions to handle indicator and background art styling. That its local on the arc object. But I also think this was meant so that the draw calls can be reused and the styling is hyper local to the object (I thought charing the background color no the race was weird to color the underlying arc.).

  • Consider arc as "rectangle with arc inside", then it's obvious to use standard properties LV_STYLE_COLOR / LV_STYLE_OPACITY as with any <div> / <span>

But its an rectangle with two arcs that should be able to be individually styled.

  • Verify CSS styles/names for web and drop non-standard (except specific cases like gradient in background image)

    • "partitions" seems to be inspired by old widgets, and is alien to CSS/DOM
  • Rework widgets structure. For example, use sub-nodes to carry extra styles/drawers, and vendor-styles for the rest.

This should save memory and the amount of styling you have to do correct? This is implementing a cascading from the screen down?

While I agree with consistency and I think some of the API could be cleaned up. Coming from a mobile world and now doing CSS/JS/ReactJS I hate CSS and HTML and long for iOS, Android, etc. I think the library needs to balance the act of being concise and easy to use. It should also take a bit of inspiration from QT, GTK , iOS, Win32, Mac, things that have been around and have been made to do GUIs for many many years that I think are a bit more concise than HTML/CSS.

I am working on C++ wrapper that uses lambada's, smart pointers, and moves local styling on the objects to abstract away parts etc.. I probably would have worked on more type safe C wrappers as well for local styles. I am not sure the APIs are concise enough. Instead of object_set_style_arcxxxx.. Should it be in the arc header and arc_setstyle? So you are context aware? This how I have been solving the issue in my C++ wrapper.

class Arc: public Object {
protected:
    Arc(lv_obj_t* parent): Object(lv_arc_create(parent, NULL)){};

public:
    static std::shared_ptr<Arc> create(SharedObject parent);

    void setEndAngle(uint16_t angle);
    void setBackgroundAngle(uint16_t startAngle, uint16_t endAngle);

    void setValue(int16_t value);
    void setAdjustable(bool adjustable);

    // style helpers..
    void setBackgroundArcWidth(lv_coord_t width);
    void setIndicatorWidth(lv_coord_t width);

    void setBackgroundArcColor(Color& color);
    void setIndicatorColor(Color& color);
};
void TankGuage::setValue(int16_t value) {
  char percent[5] = "";
  char remaining[60] = "";
  sprintf(percent, "%d%%", value);

  int gallonsRemaining = floor(TANKGALLONS * ((100 - value) / 100.0));
  sprintf(remaining, "%d gal\nremaining", gallonsRemaining);

  percentageLabel->setText(percent);
  gallonsRemaingLevel->setText(remaining);

  // depending on the level we change colors.
  if (value < warningLevel) {
    setBackgroundArcColor(goodBackground);
    setIndicatorColor(goodIndicator);
  } else if (value < crticalLevel) {
    setBackgroundArcColor(warningBackground);
    setIndicatorColor(warningIndicator);
  } else {
    setBackgroundArcColor(criticalBackground);
    setIndicatorColor(criticalIndicator);
  }

    Arc::setValue(value);
}

Coming from a mobile world is a lot nicer. Nice thing is we aren't taking a huge amount of memory. We have a bit of overhead for the C++ classes and STL. But there is nothing out of the box or much weird (storying self in user_data)..

puzrin commented 3 years ago

Wouldn't this then break local styles?

Local styles is just a syntax sugar. It adds unique "hidden" style to each object and set it's properties via separate methods. Thia cannot break anything.

But its an rectangle with two arcs that should be able to be individually styled.

You missunderstand [desired] DOM model. There are no rectangle with 2 arcs, where are 2 rectangles with one arc in each.

I think the library needs to balance the act of being concise and easy to use. It should also take a bit of inspiration from QT, GTK , iOS, Win32, Mac, things that have been around and have been made to do GUIs for many many years that I think are a bit more concise than HTML/CSS.

If you scan my previous postings, i never said dom/css are best possible option. I said "any approach is ok, if followed exactly without deviations". But i have experience with dom/css only. If anyone has skills of software architect, and can curate different approach with appropriate consistency - i'll be ok.

My only wish is to get result of guaranteed quality in predictable time.

This is implementing a cascading from the screen down?

No cascading supported. That's tradeoff for simplicity. This will be partially compensated with dom iterators.

I am working on C++ wrapper that uses lambada's, smart pointers, and moves local styling on the objects to abstract away parts etc..

I don't know if full cpp wrapper possible, but i know exactly what i wish from cpp - chained syntax. That's useful to assign styles and properties. Other things are less critical.

puzrin commented 3 years ago

@kisvegabor clarification for "state in style"

In css i can define:

.btn {
  color: red;
}

.btn:active {
  color: green;
}

Then i say <div class="btn">... and everything assigned to div at once. It would be nice to have something similar for lvgl.

scottandrew commented 3 years ago

But its an rectangle with two arcs that should be able to be individually styled.

You missunderstand [desired] DOM model. There are no rectangle with 2 arcs, where are 2 rectangles with one arc in each.

Probably. What looks like is 2 arcs in one frame. (background and indicator) maybe I am misunderstanding the draw code.

I think the library needs to balance the act of being concise and easy to use. It should also take a bit of inspiration from QT, GTK , iOS, Win32, Mac, things that have been around and have been made to do GUIs for many many years that I think are a bit more concise than HTML/CSS.

If you scan my previous postings, i never said dom/css are best possible option. I said "any approach is ok, if followed exactly without deviations". But i have experience with dom/css only. If anyone has skills of software architect, and can curate different approach with appropriate consistency - i'll be ok.

My only wish is to get result of guaranteed quality in predictable time.

👍

This is implementing a cascading from the screen down?

No cascading supported. That's tradeoff for simplicity. This will be partially compensated with dom iterators.

👍

I don't know if full cpp wrapper possible, but i know exactly what i wish from cpp - chained syntax. That's useful to assign styles and properties. Other things are less critical.

My wrapper is not fully but its should be fairly functional for my needs at least. Maybe I'll put it out there as a binding later.

puzrin commented 3 years ago

Probably. What looks like is 2 arcs in one frame. (background and indicator) maybe I am misunderstanding the draw code.

There is problem with widgets architecture. It's too monolithic with bulk of custom properties (comes from previous lvgl versions). Current sources can not be reference to "what is desired".

If we follow DOM/CSS model from web, then every end-node is rectangle with custom drawer. Custom drawer can draw arc inside. But since container is standard, we can use standard properties "color", "opacity" etc.

To fix this, we need to review every widget and split when required to some subnodes. For example, "knob" -> [rotator, marks, text]. That will reduce number of custom properties to zero or almost zero.

kisvegabor commented 3 years ago

@puzrin You have mentioned a couple of times that you prefer BEM. I've read a lot about BEM vs atomic CSS (e.g. Tailwind) and found that both are popular, and one is not clearly better than the other. I think the Taliwind-like way of thinking is simpler and easier to use on UI's with less screen and lower complexities (like the embedded UI's usually are). In Tailwind you can do this:

<button class="bg-purple-600 hover:bg-purple-700> 

It's quite close how you can do it with LVGL:

lv_obj_add_style(button, LV_PART_MAIN, LV_STATE_DEFAULT, &bg_purple_600);
lv_obj_add_style(button, LV_PART_MAIN, LV_STATE_HOVER, &bg_purple_700);

To allow "normal" CSS pseudo-class and pseudo-element method I can imagine something like this:

lv_style_pack_t button_pack;
lv_style_pack_add(&button_pack, LV_PART_MAIN, LV_STATE_DEFAULT, &bg_purple_600);
lv_style_pack_add(&button_pack, LV_PART_MAIN, LV_STATE_HOVER, &bg_purple_700);

lv_obj_add_style_pack(button, &button_pack);

Regarding the LVGL parts, they are like pseudo-elements. E.g. a progress bar can be styled via pseudo-elements. It's the same how the slider can be styled via LV_PART_MAIN/INDICATOR/KNOB.

The content-related properties can be grouped like below. I agree that it's not a typical use case that there are an arc and line on the same container and you want different styles for them. Having a text on the middle of the arc with a different color than the arc itself is reasonable. In this case - of course - you can style the text independently.

Correct me if I'm wrong, but in HTML/CSS there are only rectangles, images, and texts and no arc and line. Therefore HTML/CSS is cleaner in this regards (but more difficult to use)

An other thing: this way not only text color will be inherited but arc and line color too. It sounds reasonable, I've just wanted to mention it.

Similar properties:

LV_STYLE_ARC_COLOR
LV_STYLE_TEXT_COLOR
LV_STYLE_LINE_COLOR
LV_STYLE_IMG_RECOLOR

LV_STYLE_LINE_COLOR_FILTERED     //only helpers, doesn't really matter now
LV_STYLE_ARC_COLOR_FILTERED
LV_STYLE_TEXT_COLOR_FILTERED
LV_STYLE_IMG_RECOLOR_FILTERED

LV_STYLE_TEXT_OPA
LV_STYLE_ARC_OPA
LV_STYLE_LINE_OPA
LV_STYLE_IMG_OPA

LV_STYLE_LINE_WIDTH
LV_STYLE_ARC_WIDTH

LV_STYLE_LINE_ROUNDED
LV_STYLE_ARC_ROUNDED

Unique properties:

LV_STYLE_LINE_DASH_WIDTH
LV_STYLE_LINE_DASH_GAP

LV_STYLE_ARC_IMG_SRC

LV_STYLE_TEXT_FONT
LV_STYLE_TEXT_LETTER_SPACE
LV_STYLE_TEXT_LINE_SPACE
LV_STYLE_TEXT_DECOR
LV_STYLE_TEXT_ALIGN

LV_STYLE_IMG_RECOLOR_OPA   //intensity
puzrin commented 3 years ago

I think the Taliwind-like way of thinking is simpler and easier to use on UI's with less screen and lower complexities (like the embedded UI's usually are).

At the moment of style design i verified "at least one methodology works". If more works - i see no problem when everyone do selection according to his preferences.

It's quite close how you can do it with LVGL:

lv_obj_add_style(button, LV_PART_MAIN, LV_STATE_DEFAULT, &bg_purple_600);
lv_obj_add_style(button, LV_PART_MAIN, LV_STATE_HOVER, &bg_purple_700);

I do not agree. As i said before, state is part of style definition, not of object assign. Example above has "broken" data flow.

To allow "normal" CSS pseudo-class and pseudo-element method I can imagine something like this:

lv_style_pack_t button_pack;
lv_style_pack_add(&button_pack, LV_PART_MAIN, LV_STATE_DEFAULT, &bg_purple_600);
lv_style_pack_add(&button_pack, LV_PART_MAIN, LV_STATE_HOVER, &bg_purple_700);

lv_obj_add_style_pack(button, &button_pack);

Regarding the LVGL parts, they are like pseudo-elements. E.g. a progress bar can be styled via pseudo-elements. It's the same how the slider can be styled via LV_PART_MAIN/INDICATOR/KNOB.

I do not understand reason of discussing all that. I can help to follow CSS because i know how to do that and can predict cost/quality in long term. And i can not help with diverged things. As i said before - it's normal to extend core api with sugar methods, but not normal to change core methods for custom things.

Correct me if I'm wrong, but in HTML/CSS there are only rectangles, images, and texts and no arc and line. Therefore HTML/CSS is cleaner in this regards (but more difficult to use)

For example, you can insert SVG image (via data-uri) and declare arc inside (and CSS can be used to style svg internals).

Unique properties:

LV_STYLE_LINE_DASH_WIDTH
LV_STYLE_LINE_DASH_GAP

LV_STYLE_ARC_IMG_SRC

LV_STYLE_TEXT_FONT
LV_STYLE_TEXT_LETTER_SPACE
LV_STYLE_TEXT_LINE_SPACE
LV_STYLE_TEXT_DECOR
LV_STYLE_TEXT_ALIGN

LV_STYLE_IMG_RECOLOR_OPA   //intensity

As far as i understand, LV_STYLE_TEXT_* are global, not widget-specific (if widget has text as separate node). So we coming to very short list

LV_STYLE_LINE_DASH_WIDTH
LV_STYLE_LINE_DASH_GAP

LV_STYLE_ARC_IMG_SRC

LV_STYLE_IMG_RECOLOR_OPA   //intensity

Only 4 vendor-properties for one concrete "advanced widget". Not a big deal, IMO. May be LV_STYLE_ARC_IMG_SRC & LV_STYLE_IMG_RECOLOR_OPA can be dropped too. But that's minor details.

puzrin commented 3 years ago

Seems i misunderstood term "arc" in context of lvgl. It's not any elliptic arc, but segment of circle with some nice styling.

Then look at this https://www.google.com/search?q=css+gauge&tbm=isch (mostly just for fun).

kisvegabor commented 3 years ago

I do not agree. As i said before, state is part of style definition, not of object assign. Example above has "broken" data flow.

Do you think "part" also should be part of the styles (similarly to pseudo-elements)?

As far as i understand, LV_STYLETEXT* are global, not widget-specific (if widget has text as separate node). So we coming to very short list

Yes, TEXT is not widget specific.

kisvegabor commented 3 years ago

@embeddedt @xiaoxiang781216 What do you think about the style part? In summary which one is better?

1, State and part in style

lv_style_set_bg_color(&style1, lv_color_red());
lv_style_set_state(&style1, LV_STATE_PRESSED);
lv_style_set_part(&style1, LV_PART_INDICATOR);

lv_obj_add_style(bar1, &style1)

2. State and part in widget

lv_style_set_bg_color(&style1, lv_color_red());

lv_obj_add_style(bar1, LV_PART_INDICATOR, LV_STATE_DEFAULT, &style1)

1) is better becasue it's like CSS but 2) makes the styles more reusable (e.g. style_red can be used as bar indictor, or pressed button style )

puzrin commented 3 years ago

Do you think "part" also should be part of the styles (similarly to pseudo-elements)?

I think it should nor exist at all in core methods. That's said in first post.

kisvegabor commented 3 years ago

@puzrin One thing just came to my mind: even if the states were saved in styles it'd differ from CSS in an important aspect. Compare these:

.style1 {...}
.style1:focus {...}
.style1::placeholder{...}
.style1::placeholder:disabled{...}

<input class="style1" ...>
lv_style_t style1; //... add props
lv_style_t style1_focus; //... add props and set focused state
lv_style_t style1_placeholder; //... add props and set part
lv_style_t style1_placeholder_disabled; //... add props and set part and disabled state

lv_obj_t * ta = lv_textarea_create(...);
lv_obj_add_style(ta, &style1);
lv_obj_add_style(ta, &style1_focus);
lv_obj_add_style(ta, &style1_placeholder);
lv_obj_add_style(ta, &style1_placeholder_disabled);

The key is in C the styles need to be added one by one. If you need a new style (e.g. style1_pressed) it needs to be added to every object where the other style1_..s are added. In addition, you need to name all the styles and make them globally available.

So it seems the above proposed style_pack makes sense with a minor modification:

lv_style_pack_t red_btn;
//Option1
lv_style_t * s = lv_style_pack_add_style(&red_btn, LV_PART_MAIN, LV_STATE_DEFAULT);
//Option2
lv_style_t * s = lv_style_pack_add_style(&red_btn);
lv_style_set_state(s, LV_STATE_PRESSED);
lv_style_set_part(s, LV_PART_KNOB);

// ...add props and new styles

lv_obj_add_style_list(obj, &red_btn);

What do you think?

puzrin commented 3 years ago
.style1 {...}
.style1:focus {...}
.style1::placeholder{...}
.style1::placeholder:disabled{...}

<input class="style1" ...>

Probably, this may be invalid combination in current context, because it asks indirectly about unsupported cascading (for placeholder). I think, prior to ask about ::placeholder, ::before/::after should be arranged.

The most used cases are:

Then this signatures could be convenient:

(probably, more methods for each value type, not principal).


What to do with ::placeholder is a separate question.

kisvegabor commented 3 years ago

The problem is not with parts but the fact that all styles need to be added only by one instead of class="my_class". Our current approach would look like this in CSS: class="my_class my_class:hover my_class:focus"

embeddedt commented 3 years ago

all styles need to be added only by one

That's quite verbose. I agree that we need a solution for that - not sure if the style_pack idea is the right one though. I liked the v7 approach but it might not be conducive to the optimizations in v8.

puzrin commented 3 years ago

The problem is not with parts but the fact that all styles need to be added only by one instead of class="my_class".

There are 2 problems. Main one is: user has to specify style-apecific thing (state) on style assign.

Our current approach would look like this in CSS: class="my_class my_class:hover my_class:focus"

https://github.com/lvgl/lvgl/issues/2156#issuecomment-808895071 here i described method how to describe all states in one style (before assign to object).

puzrin commented 3 years ago

That's quite verbose. I agree that we need a solution for that - not sure if the style_pack idea is the right one though. I liked the v7 approach but it might not be conducive to the optimizations in v8.

@embeddedt problem is not of esthetic kind. It's about proper data grouping & isolation. When you define state, it should be inside of style (class). Defining it at moment of object assign is "broken isolation", because instead of [style] we pass [style, state_from_style_internals]

When you to architecture design, it should be verified in multiple dimentions:

Current architecture may be have nice api (subjective), but has not nice data flow. Proper (optimal) data flow is usually very important, because it cause more nice signatures.

puzrin commented 3 years ago

About placeholder. If we consider <input> as "composite widget" (placeholder is child of <input>, with reference for direct access), then we can style is as usual node (and access as input->placeholder for simplicity).

That's a bit hacky, but limited by input node scope and will not spread to all architecture. I mean, if hack does not affect other parts - it's not too bad.

kisvegabor commented 3 years ago

@puzrin

lv_style_add_property(style, property_id, value) lv_style_add_state_property(style, property_id, state_id, value)

This API doesn't work because value can have several types.

 lv_style_add_property(*style, BG_COLOR, color);
 lv_style_add_property(*style, BORDER_WIDTH, 3);
 lv_style_add_property(*style, FONT, &my_font);

Instead of "style pack" what about this:

lv_style_t red_btn;
lv_style_set_bg_color(&red_btn, red);

lv_style_t * s = lv_style_add(&style, LV_PART_MAIN, LV_STATE_PRESSED); //Needs better name (extend or allocate?)
lv_style_set_bg_color(s, dark_red);

lv_obj_add_style(btn, &red_btn);

The styles can store the extra styles in a single linked list.


I still see that parts are the same as pseudo-elements in CSS (see they are not evil) and they save a lot of memory for several objects. Therefore I wouldn't like to remove them.

puzrin commented 3 years ago

This API doesn't work because value can have several types.

 lv_style_add_property(*style, BG_COLOR, color);
 lv_style_add_property(*style, BORDER_WIDTH, 3);
 lv_style_add_property(*style, FONT, &my_font);

My samples are not pure C, those are in meta-language to illustrate approach:

Please, extend with "typed props" functions appropriately.

lv_style_t * s = lv_style_add(&style, LV_PART_MAIN, LV_STATE_PRESSED); //Needs better name (extend or allocate?)
lv_style_set_bg_color(s, dark_red);

This still does not solve issue how to store props for multiple states in style. See my proposal above. Just do several _add functions, with and without state. That should be convenient IMO.

I still see that parts are the same as pseudo-elements in CSS (see they are not evil) and they save a lot of memory for several objects. Therefore I wouldn't like to remove them.

That's not correct logic to refer not implemented before/after as "working well". I don't know how to pass you all volume of my CSS experience to prove your extension is not nice. But even if i agree with you, PART is used rare, only in "extended" widgets (subject for rework). It will be annoying to pass dummy value of PART everywhere.

In worst case, you could create separate functions (with PART param). That will get 2 benefits:

Here are several problems, not fatal each, but not nice in combination:

In your proposal kludges are spreaded to all core. I propose to keep those local as much as possible, at least.

kisvegabor commented 3 years ago

This still does not solve issue how to store props for multiple states in style. See my proposal above. Just do several _add functions, with and without state. That should be convenient IMO.

With types it'd look like this (with simplified function names):

 lv_style_set_color(*style, LV_STYLE_BG_COLOR, color);
 lv_style_set_int(*style, LV_STYLE_BORDER_WIDTH, 3);
 lv_style_set_pointer(*style, LV_STYLE_FONT, &my_font);

 lv_style_set_state_color(*style, LV_STYLE_BG_COLOR, color);
 lv_style_set_state_int(*style, LV_STYLE_BORDER_WIDTH, 3);
 lv_style_set_state_pointer(*style, LV_STYLE_FONT, &my_font);

Now it look like this with more functions but less parameters:

 lv_style_set_bg_color(*style, color);

Assuming we leave parts:

 lv_style_set_bg_color_state(*style, state, color);
 lv_style_set_bg_color_part(*style, part, color);
 lv_style_set_bg_color_state_part(*style, state, part, color);

We can also make something like this:

 lv_style_set_bg_color(*style, color, 0);  //default state, main part
 lv_style_set_bg_color(*style, color, LV_STATE_PRESSED);
 lv_style_set_bg_color(*style, color, LV_PART_KNOB);
 lv_style_set_bg_color(*style, color, LV_STATE_PRESSED | LV_PART_KNOB);
embeddedt commented 3 years ago

I like the last option (bitmask for states and parts).

puzrin commented 3 years ago

I like idea to hide parts behind mask. That's still useable and keeps potential for future cleanup.

See tweaked names and params order. That's not principal (i will not insist if you prefer different names/order).

// most popular
lv_style_set_bg_color(*style, color);

// universal but more rare
lv_style_set_state_bg_color(*style, color, state);

// the same, with unobtrusive "parts" support
lv_style_set_bg_color_state(*style, color, state | LV_PART_KNOB);
embeddedt commented 3 years ago

@puzrin Sounds good. I would prefer lv_style_set_bg_color (no part/state) and lv_style_set_bg_color_ex (with part & state) for names, as appending _state could be ambiguous if we later have a style property ending in _state.

kisvegabor commented 3 years ago

Great :slightly_smiling_face:

@puzrin Adding state would double the number of functions there are already many.

puzrin commented 3 years ago

@puzrin Sounds good. I would prefer lv_style_set_bg_color (no part/state) and lv_style_set_bg_color_ex (with part & state) for names, as appending _state could be ambiguous if we later have a style property ending in _state.

I agree with lv_style_set_bg_color & lv_style_set_bg_color_ex. Useable.

@puzrin Adding state would double the number of functions there are already many.

Not critical, IMO. Metrics are "usability" and "costs". Costs are ~ zero. Usability - "easy to skip when read docs" (no need to keep in mind multiple small things).

kisvegabor commented 3 years ago

It increases the API size which increases the size of Micropython binding (already too large) and makes autocompletion less usable.

embeddedt commented 3 years ago

Can't we use inline functions? Ideally MicroPython should use only one of the variants + default arguments.

kisvegabor commented 3 years ago

We can use inline functions but I don't think the binding can detect extended functions. We can't indicate the default of the omitted parameter(s) in C.

embeddedt commented 3 years ago

@puzrin Do you think it's acceptable to have the extended function only and provide 0 as the last argument for the default part & state? That would be the simplest compromise.

puzrin commented 3 years ago

@puzrin Do you think it's acceptable to have the extended function only and provide 0 as the last argument for the default part & state? That would be the simplest compromise.

In general, i consider such things as nasty (causing dirty user code). BUT (!) i have no wide experience with C99 and not sure all my practices can be propagated here (due language specifics and limits).

lv_style_set_bg_color(*style, color, 0) is not too horrible, at least. I suggest to temporary accept that, and polish later after other fixes.

kisvegabor commented 3 years ago

Okay, I'll make some experimets with that today or tomorrow.

kisvegabor commented 3 years ago

I've started to work on it here is what I found:

To be honest, I'm a little bit puzzled now. I'd like to live the styles as they are because I see clear advantages, but I know that you find it important to change them.

Default size handling and combing text/arc/line properties are still missing. I rather work on that now and we will see what to do with the states.

puzrin commented 3 years ago

What I like about the current system is it's very easy to reuse styles. E.g. radius_10 can be added to anything and any state and part. With the suggested direction styles becomes bulky and repetitive again.

What exactly do you need (from real world)?

Note, "state-less" style is propagated to all "states" by default.

kisvegabor commented 3 years ago

What exactly do you need (from real world)?

As I've already mentioned I believe the atomic CSS approach fits better our use case. Imagine a blue_bg style. Now in LVGL you can use it anywhere:

In CSS normally it'd require 6 different classes with the same content.

It's conceptually the same but we can change

lv_obj_add_style(obj, LV_PART_KNOB, LV_STATE_PRESSED, &style);
lv_obj_set_style_bg_color(obj, LV_PART_KNOB, LV_STATE_PRESSED, red);   //Local style

to

lv_obj_add_style(obj, &style, LV_PART_KNOB | LV_STATE_PRESSED);  //0 for deafult
lv_obj_set_style_bg_color(obj, red. LV_PART_KNOB | LV_STATE_PRESSED);

Default sizes I got an idea that we can store the default sizes in classes. We can use auto and 100% where possible, but e.g. for the switch, an absolute size should be set.

This way even if the user removes all the styles, the default sizes are still available from the class.

Simplify properties We will have color and opa for text, arc, and line.

puzrin commented 3 years ago

What exactly do you need (from real world)?

As I've already mentioned I believe the atomic CSS approach fits better our use case. Imagine a blue_bg style. Now in LVGL you can use it anywhere:

  • add to a button's default state
  • use it as focused color on an other button
  • use on the checkbox's tickbox's checked state
  • use on the slider's indicator
  • add to the keyboard's button checked state
  • add to the table cell's focused state

In CSS normally it'd require 6 different classes with the same content.

It's conceptually the same but we can change

lv_obj_add_style(obj, LV_PART_KNOB, LV_STATE_PRESSED, &style);
lv_obj_set_style_bg_color(obj, LV_PART_KNOB, LV_STATE_PRESSED, red);   //Local style

to

lv_obj_add_style(obj, &style, LV_PART_KNOB | LV_STATE_PRESSED);  //0 for deafult
lv_obj_set_style_bg_color(obj, red. LV_PART_KNOB | LV_STATE_PRESSED);

I'm completely lost. We discussed signature of "add property to style (with optional state)". All we need is define signature of "add class [style] to object". What is "atomic CSS approach" and why it should be taken into account here? If that's methodology - it's higher abstraction thing, completely isolated from styles data flow and api.

I can't understand, why you refer to old (deviated) signatures. Those can not be source of useful info (for new design). Just forget those and to "right" in single step.

Let's start from begining, "blue_bg" class. I suggest more obvious thing instead => "padding-1", "padding-2"... - added when you wish to increase object space. That's called "mixin" (when you merge multiple sources):

<button "my-btn padding-2">go!</button>

https://getbootstrap.com/docs/4.5/utilities/spacing/

What's wrong?

  • color is not a good name on its own, I'm pretty sure users won't think that it should be used to color a label. Do you know a good alternative name? E.g. color_content?

IMO "colors" is ok. New users will have to learn once - "color is for content". I'd prefer to keep names when possible, until absolutely impossible (example: "class" => "style" in lvgl).

  • if opa is used for text, arc, line, how to call the "global opacity" property? (opacity in CSS)

IMO "opacity" is ok, if you drop all related spec deviations. AFAIK, instead of "global opacity" lvgl had something specific for concrete widgets (probably you already removed that).

kisvegabor commented 3 years ago

<button "my-btn padding-2">go! https://getbootstrap.com/docs/4.5/utilities/spacing/

What's wrong?

Let's say you have a bg-light style. In CSS how would you use it as:

As mixins are not supported by plain CSS we can't use it as an argument that in CSS you can solve these issues with mixins.

So we can choose from 2 bad:

  1. Go with the CSS approach even if know that it has problems.
  2. Go with the current approach and take the risk of doing something new.

I like 2) more (but I'm biased to my own child :stuck_out_tongue: ). Knowing that "no one" uses plain CSS we should be very critical with it. I agree with some parts but not so much with the others. Just out of curiosity, can you mention real-life issues with storing state/part in the object?

IMO "colors" is ok. New users will have to learn once - "color is for content". I'd prefer to keep names when possible, until absolutely impossible (example: "class" => "style" in lvgl).

Okay, I'll try how it looks.

IMO "opacity" is ok, if you drop all related spec deviations. AFAIK, instead of "global opacity" lvgl had something specific for concrete widgets (probably you already removed that).

In CSS text opacity can be set with RGBA colors but we don't have RGBA colors in styles yet.

"Global opacity" means set the opacity of an object all its children.

kisvegabor commented 3 years ago

I've started to remove and refactor the discussed properties. Two things came up:

I couldn't decide if these properties should be inherited or not (there are no CSS counterparts):

    LV_STYLE_IMG_COLOR, //Color overlay on image
    LV_STYLE_IMG_COLOR_INTENSITY,

    LV_STYLE_ARC_IMG_SRC ,

    LV_STYLE_LINE_CAPS,
    LV_STYLE_LINE_DASH_WIDTH ,
    LV_STYLE_LINE_DASH_GAP,

LV_STLYE_WIDTH conflicts with LV_STYLE_LINE/ARC_WIDTH if I remove the LINE/ARC words. Do you have any ideas?

puzrin commented 3 years ago

Let's say you have a bg-light style. In CSS how would you use it as:

  • button background? Clear <button class="bg-light">text</button>
  • scrollbar track? Needs ::-webkit-scrollbar-track where you need to add the same color again. But you can't extract the color from bg-light
  • progress bar background? Same issue as with scrollbar but with progress[value]::-webkit-progress-bar

As mixins are not supported by plain CSS we can't use it as an argument that in CSS you can solve these issues with mixins.

IMO this example is not relevant. There is no need to duplicate background to virtual nodes (::...) [or sub-nodes if widgets reworked]. It's assigned to root only, and other parts stay with transparent one. I speak about real world, not imaginated cases.

If you take another example, color - it's propageted down, also not a problem.

So we can choose from 2 bad:

  1. Go with the CSS approach even if know that it has problems.
  2. Go with the current approach and take the risk of doing something new.

Since example is artificial, (1) continues to stay a good choice.

Just out of curiosity, can you mention real-life issues with storing state/part in the object?

For me this is out of scope. I think in domain of following CSS (reasons were declared). Everything else is waste of time :). If you decide to make different thing - i will accept your choice but will not be able to help. Not because i don't wish, but because it's huge effort to rebalance new system. With CSS-like i'm sure about my recommendation.

In CSS text opacity can be set with RGBA colors but we don't have RGBA colors in styles yet.

Then we should have it :). AFAIK, that's scheduled for v9. If we can't have it now, i suggest to temporary crop related features until RGBA support complete. IMO, that will not be too big [temporary] disadvantage of v8.

"Global opacity" means set the opacity of an object all its children.

AFAIK, that's exactly how CSS opacity property works.

puzrin commented 3 years ago

I couldn't decide if these properties should be inherited or not (there are no CSS counterparts):

    LV_STYLE_IMG_COLOR, //Color overlay on image
    LV_STYLE_IMG_COLOR_INTENSITY,

    LV_STYLE_ARC_IMG_SRC ,

    LV_STYLE_LINE_CAPS,
    LV_STYLE_LINE_DASH_WIDTH ,
    LV_STYLE_LINE_DASH_GAP,

LV_STLYE_WIDTH conflicts with LV_STYLE_LINE/ARC_WIDTH if I remove the LINE/ARC words. Do you have any ideas?

kisvegabor commented 3 years ago

IMO this example is not relevant. There is no need to duplicate background to virtual nodes (::...) [or sub-nodes if widgets reworked]. It's assigned to root only, and other parts stay with transparent one. I speak about real world, not imaginated cases.

I've intentionally chosen real-life CSS examples not LVGL examples based on not-implemented features.

In CSS text opacity can be set with RGBA colors but we don't have RGBA colors in styles yet.

Then we should have it :). AFAIK, that's scheduled for v9. If we can't have it now, i suggest to temporary crop related features until RGBA support complete.

How do you mean "crop"? bg/border/outline_opa is required some form. (Now with BG_COLOR + BG_OPA which is acceptable for now). IMO for CSS opacity we can use OPA_GLOBAL in v8.

LV_STLYE_WIDTH conflicts with LV_STYLE_LINE/ARC_WIDTH if I remove the LINE/ARC words. Do you have any ideas?

Could you comment on this?

LV_STYLE_IMG_COLOR, LV_STYLE_IMG_COLOR_INTENSITY - seems deviation from CSS, subject to drop

It seems it can be merged to filter.

LV_STYLE_ARC_IMG_SRC - i don't know arc details, difficult to comment. Background image? AFAIK arc is ~ segment of circle's border. How image is related to that?

It's an image from which the arc can be masked out. It allows using any complex gradient as the arc.

LV_STYLE_LINE_CAPS, LV_STYLE_LINE_DASH_WIDTH, LV_STYLE_LINE_DASH_GAP - at first glance, line is not needed at all, because solved via border of

(no line => no props) https://www.w3schools.com/cssref/pr_border-style.asp. If i missunderstand what are lines for in lvgl - please give me a link to demo. I need to see visually, what is that.

The line can be skewed too. Needed e.g. for a line chart.

puzrin commented 3 years ago

I've intentionally chosen real-life CSS examples not LVGL examples based on not-implemented features.

I need proof why this example is from real world. Technically possible != from real world.

How do you mean "crop"? bg/border/outline_opa is required some form. (Now with BG_COLOR + BG_OPA which is acceptable for now). IMO for CSS opacity we can use OPA_GLOBAL in v8.

I mean drop features until v9, instead of adding kludges.

LV_STLYE_WIDTH conflicts with LV_STYLE_LINE/ARC_WIDTH if I remove the LINE/ARC words. Do you have any ideas?

Could you comment on this?

CSS in HTML does not operate with "brush" (to draw line, point), and so, has no "width". Low level drawing is possible via SVG only. IMO lvgl mixes completely different abstraction layers (rect objects and drawing primitives). This needs rework.

LV_STYLE_IMG_COLOR, LV_STYLE_IMG_COLOR_INTENSITY - seems deviation from CSS, subject to drop

It seems it can be merged to filter.

filter is unstable feature, known only by geeks. Could you explain what those features mean? I need to understand why those were added and how often are those used.

LV_STYLE_ARC_IMG_SRC - i don't know arc details, difficult to comment. Background image? AFAIK arc is ~ segment of circle's border. How image is related to that?

It's an image from which the arc can be masked out. It allows using any complex gradient as the arc.

Is image added to hack radial gradient? https://stackoverflow.com/questions/64148997/how-to-create-a-curved-gauge-having-linear-gradient-as-background.

I'm not ready to reply if "background-image" is relevant. Formally, background occupies rectangle, then arc should be clip-mask. But clip-mask is too much for weak hardware & low memory.

Probably, keeping LV_STYLE_ARC_IMG_SRC as vendor property is optimal. I'd suggest rename to LV_STYLE_ARC_BG_IMG_SRC

The line can be skewed too. Needed e.g. for a line chart.

If it's a part of specific widget, it worth to limit is by widget scope. There are two possibilities:

  1. Add vendor-prefixes for that widget
  2. Organize custome drawer for easy override

I usually prefer (2), because number of styles can grow infinite.

kisvegabor commented 3 years ago

I need proof why this example is from real world. Technically possible != from real world.

I don't understand why isn't it real-world to set the same color on different elements.

I mean drop features until v9, instead of adding kludges.

People certainly want to use opacity.

CSS in HTML does not operate with "brush" (to draw line, point), and so, has no "width". Low level drawing is possible via SVG only. IMO lvgl mixes completely different abstraction layers (rect objects and drawing primitives). This needs rework.

Line, polygon, arc are graphics primitive (besides rectangle, circle, and text). The fact that they have no built-in support in HTML/CSS is rather a sign of problem than a virtue. The abstraction layers are pretty clear in LVGL:

  1. primitive drawing
  2. widgets
  3. user app

filter is unstable feature, known only by geeks. Could you explain what those features mean? I need to understand why those were added and how often are those used.

Image recolor (which can be a filter) is added to save using multiple images/icons for released, pressed (e.g. slightly darker), focused (e.g. blueish) states.

Is image added to hack radial gradient? https://stackoverflow.com/questions/64148997/how-to-create-a-curved-gauge-having-linear-gradient-as-background.

Not only but mainly yes. The arc's image can have any complexity, e.g. adding an inner shadow, glow, special border, etc.

I'm not ready to reply if "background-image" is relevant. Formally, background occupies rectangle, then arc should be clip-mask. But clip-mask is too much for weak hardware & low memory.

The hearth of LVGL's drawing engine is clip masking. It's quite fast and uses only a little memory. Every arcs and skewed lines are clipped from a rectangle.

If it's a part of specific widget, it worth to limit is by widget scope. There are two possibilities:

  1. Add vendor-prefixes for that widget
  2. Organize custome drawer for easy override I usually prefer (2), because number of styles can grow infinite.

"Line" is still a graphics primitive that should be available for the user on its own.


I feel like we don't get closer (rather even further) to an agreement with the remaining questions. As finally somethings needs to be chosen and someone needs to take responsibility for the decision I'm about going with:

embeddedt commented 3 years ago

I agree with @kisvegabor. The fact that you can do anything with CSS by assembling many widgets does not mean that LVGL should behave exactly the same way. The gauge implementation @puzrin linked above is a good example. That gauge required 3 divs each with a non-trivial amount of styles. In comparison, LVGL's gauge (at least in v7) used an arc and a rectangle. It accomplishes the same thing while being much more intuitive and still customizable.

There is a difference between adopting a CSS-like styling solution and rearchitecting everything to work like HTML.

excitedbox commented 3 years ago

I kinda lost track of progress these last few months and decided to check in on things. I got sidetracked by a massive work project (A WYSIWYG Appbuilder) that has taken me deep into the bowels of css.

This discussion all seems to point back to my previously proposed solution of using a hash table (during boot ONLY) and pointer arrays during runtime. (Although @scottandrew C++ method is the "proper" way of doing it since mine is really just implementing C++ features).

My previous suggestion supported 100% 1:1 CSS parity while reducing memory overhead and reducing ALL FUTURE development effort. Right now you need to implement functions for every feature in every widget while the proposed solution lets you create 1 function that applies to every widget.

I had done some testing with 30 widgets that had 30 parts such as labels, input boxes, table rows, etc. each using 10 styles, and it used around 1KB of memory.

I can only keep pointing back to the previous proposal as a solution for this 6 month long discussion about how to improve the style system.

By creating functions such as set_color instead of set_arc_color for each style and property and assigning them a 1 byte ID/hash you can convert all CSS code into function pointers with about 200 lines of code. After the CSS is parsed you unload the hash table from memory and run purely using the arrays of pointers.

For each widget instance you maintain an array of settings using the same method. Each setting will have a byte ID and when you iterate through them, you end up with a perfect description of your widget.

Inheritance as @puzrin is talking about is handled through the CSS parsing which creates your array of widget settings. ie. when the parser. (This guide from LLVM Compiler explains how to do inheritance ) AND ( Here is a C example on the Operator precedence parsing wiki ) You would need to modify this to create dynamic precedence based on relationship instead of type. This way a Parents style can carry over to children.

This system is really very simple and in addition to MUCH simpler Lvgl usage for end users, the speed improvements, the memory reduction, code reduction, you also save time on any future features you want to implement.

Imagine you want to allow users to use JPG images in a widget label. Right now each widget would need to have that feature added. With my system you add a single function to draw an image and add a style (a color even) with a pointer to that function and now any widget can display an image not just in the label, but anywhere you can set a color.

I suggest looking at the old threads because once you understand what I was suggesting you will see that it makes heaps of sense.

excitedbox commented 3 years ago

@kisvegabor HTML doesn't use the primitives because there are better solutions not because they had problems. There is the Canvas for UI, SVG for images, and WebGL for Graphics. There was no real reason to add another method of drawing. CSS shapes are still used though and there are additions being made to the spec right now Shapes Level 1. AND Shapes level 2 Spec.

HTML is a constantly developing thing, so there are some parts that are outdated and no longer used or parts that were used as a hack before a real implementation existed.

I was actually very surprised that lvgl doesn't use graphics primitives because it saves you having to repeat yourself for every widget. Instead of drawing a Button you can draw a shape and attach an action. This also makes styling much easier since adding a border is just another square in a different color. No need for another function.

There are entire GUI systems that use less code than lvgl uses for some of the more complex widgets. Remember the 3d rendering examples I showed you a few weeks ago? The reason their performance is so good is that they are written primitives that reuse their code. You could run some pretty complex programs in Win95 and that used less resources than what our MCUs have today.

embeddedt commented 3 years ago

@excitedbox Hmm... I would say that LVGL is still using graphics primitives; it's just that you interact with the widgets, not the primitives themselves.

excitedbox commented 3 years ago

Yes I know. They are treated as second class citizens though. This created a lot of repetition in the way the code is written.

As I suggested above it is more efficient to treat everything as a primitive. Like in Physics formulas you use symbols to represent the answers to other formulas and to solve for something you need to first solve all the other equations. You don't write out everything as 1 long equation every time.

Edit: That is a bad example. What I mean is that it eliminates repetition such as "void lv_chart_set_value_by_id" you could have one "set_value_by_id" function for ALL of lvgl instead of repeating it for many widgets.

kisvegabor commented 3 years ago

I had done some testing with 30 widgets that had 30 parts such as labels, input boxes, table rows, etc. each using 10 styles, and it used around 1KB of memory.

Could you share the code?

kisvegabor commented 3 years ago

In v8 we have changed a lot of things to be closer to CSS. The rest of the topics should be postponed to a next iteration so I close this issue.

Thank you very much for sharing your ideas and contributing.