better theming support

[Xeverous]

I like the support of being able to set custom theme object, but not all variables which impact drawing are there

elements/lib/include/elements/support/theme.hpp

Lines 13 to 20 in 2aea4fc

	class theme
	{
	public:
	color panel_color = rgba(28, 30, 34, 192);
	color frame_color = rgba(220, 220, 220, 80);
	float frame_corner_radius = 3.0;
	float frame_stroke_width = 1.0;

For example, the button has its own radius variable:

elements/lib/include/elements/element/gallery/button.hpp

Lines 24 to 26 in 2aea4fc

	struct basic_button_body : public element
	{
	constexpr static float corner_radius = 4.0;

and some color changes too:

elements/lib/include/elements/element/gallery/button.hpp

Lines 38 to 44 in 2aea4fc

	template <typename Button, typename Label>
	inline Button make_button(
	Label&& label
	, color body_color = get_theme().default_button_color)
	{
	auto btn_body_off = basic_button_body(body_color.level(0.9));
	auto btn_body_on = basic_button_body(body_color.opacity(0.5));

Could we move all such variables to the theme so that one can create a theme which changes color and corners of all elements? I would like to be able to create eg a full square-like UI as on this screen.

[djowel]

Yes, we can do that. Absolutely!

[Xeverous]

What's the reason behind -1 here? If I try corner_radius = 0 will something break?

elements/lib/src/support/draw_utils.cpp

Lines 81 to 104 in 2aea4fc

	void draw_button(canvas& cnv, rect bounds, color c, float corner_radius)
	{
	auto gradient = canvas::linear_gradient{
	bounds.top_left(),
	bounds.bottom_left()
	};
	gradient.add_color_stop({ 0.0, rgba(255, 255, 255, 32) });
	gradient.add_color_stop({ 1.0, rgba(0, 0, 0, 32) });
	cnv.fill_style(gradient);
	cnv.begin_path();
	cnv.round_rect(bounds.inset(1, 1), corner_radius-1);
	cnv.fill_style(c);
	cnv.fill();
	cnv.round_rect(bounds.inset(1, 1), corner_radius-1);
	cnv.fill_style(gradient);
	cnv.fill();
	cnv.begin_path();
	cnv.round_rect(bounds.inset(0.5, 0.5), corner_radius-0.5);
	cnv.stroke_style(rgba(0, 0, 0, 48));
	cnv.stroke();
	}

[djowel]

What's the reason behind -1 here?

Pseudo shadows with varying transparency; purely aesthetics. Same for -0.5.

If I try corner_radius = 0 will something break?

It should not; but try it out perhaps you'll uncover a bug. Also, I'm not sure if the code implementing round rects is optimized. Cairo does not have direct support for it; so it's all done in the canvas class.

[djowel]

It should not; but try it out perhaps you'll uncover a bug. Also, I'm not sure if the code implementing round rects is optimized. Cairo does not have direct support for it; so it's all done in the canvas class.

Ah thinking about it again, there should be some checks here and constrain the corner_radius if it is allowed to be parametizable in the gallery.

[Xeverous]

Got any objections for moving almost all color/geometry information into default theme? I would like to have much higher control over the look and experiment a bit (perhaps even make a theme-editor example to see changes on the go).

Currently majority of drawing functions have hardcoded values. I would like to move all or almost all of them into the theme type (it would grow significantly). The only things I would rather not touch are non-vanilla premade widgets like knobs which are intended to always look like real knobs.

See #62 for an example change which moves 32 opacity.

[djowel]

No objections here. I think my only concern is how to organize the theme to make it easily understandable. But we can cross that bridge when we get there.

[Xeverous]

Do you have any recommendations where theme object should/should not be used? I see 3 possible ways:

1. refactor draw() to pass the theme object as a parameter
2. widgets access theme upon draw() call and give all color/geometry info into draw utils
3. widgets call draw utils and draw utils acess the theme

Currently it's a mix of 2 and 3. Thinking whether only 1 or only 2 or only 3 would be better.

[djowel]

Do you have any recommendations where theme object should/should not be used? I see 3 possible ways:

1. refactor draw() to pass the theme object as a parameter
2. widgets access theme upon draw() call and give all color/geometry info into draw utils
3. widgets call draw utils and draw utils acess the theme

Currently it's a mix of 2 and 3. Thinking whether only 1 or only 2 or only 3 would be better.

We used to have 1, but not anymore, because the theme is now globally accessible even at construction time via get_theme() For example:

https://github.com/cycfi/elements/blob/master/lib/include/elements/element/gallery/button.hpp#L41

[Xeverous]

I don't think such at-construction use of theme is good. You need to recreate the widget to apply theme changes - I would prefer it to be only 1 or only 2 or only 3.

[djowel]

By design, the theme should be valid at construction/elements-build time. I thought about this for quite some time until I arrived at that decision. There are problems with being able to dynamically change the theme on the fly. For one, there are things that are needed at construction time, for optimization reasons. Do you recall the issue with the labels' fonts in the ctor and at draw time? In such cases, the font should be known and set at ctor time, not draw time. The thing that you uncovered is a remnant of the "old" ways where the theme is only available at draw time in the context, and so I had to set the font every time it draws (and that I am fixing now).

Moreover, if needed, a rebuild/reboot of the elements view should be effortless, as can be seen in the examples. Basically, you just re-construct the root element held by the view. The lightweight elements hierarchy is very efficient to construct!

BTW, in case you missed it, there's a way to "override" a specific theme. You can see that in action here:

https://github.com/cycfi/elements/blob/develop/lib/src/element/menu.cpp#L201

For reference:

https://github.com/cycfi/elements/blob/develop/lib/include/elements/support/theme.hpp#L133

[Xeverous]

I'm fine with reconstructing the root element, but not sure if it will actually apply the change. If the root element holds a shared pointer to some widget hierarchy, I think you need to reconstruct the entire hierarchy to apply the theme changes. Am I right?

BTW, in case you missed it, there's a way to "override" a specific theme. You can see that in action here:

This should be documented on Wiki. But then it means we can use the theme in arbitrary places in code since its a globally accessible object.

• Do you want to allow to use theme object in any place (at any time past static init before main)?
• Do you want to allow run-time theme change, provided the library user reconstructs widget hierarchy?

[djowel]

I'm fine with reconstructing the root element, but not sure if it will actually apply the change. If the root element holds a shared pointer to some widget hierarchy, I think you need to reconstruct the entire hierarchy to apply the theme changes. Am I right?

You rebuild the view content. For example, here's the point:
https://github.com/cycfi/elements/blob/master/examples/buttons/main.cpp#L137

In that example, just call:

   view_.content(
      {
         share(make_controls(view_)),
         share(background)
      }
   );

and it will rebuild the entire thing, with all the shared stuff linking just fine. The root (content) is the root and cannot/should not link to anything. It can't because the view owns it privately.

BTW, in case you missed it, there's a way to "override" a specific theme. You can see that in action here:

This should be documented on Wiki. But then it means we can use the theme in arbitrary places in code since its a globally accessible object.

• Do you want to allow to use theme object in any place (at any time past static init before main)?
• Do you want to allow run-time theme change, provided the library user reconstructs widget hierarchy?

• Yes, that should be in the docs when we get the documenting the theme. The theme is a very important part of elements.
• The theme should be valid at view construct time when the root element (aka view content) is set by the client. That happens after window construction. Never before main. So the rule here is NEVER hold an element globally. It should always be constructed and passed on to the view for lifetime management.
• Yes, we should allow run-time theme change, globally and locally via override_theme. override_theme uses the RAII idiom and restores the original theme value on scope exit --a very powerful concept (e.g. I use it to disable a menu item visually and many more uses to come).
• Always defer theme access as much as possible at draw time to allow dynamic overriding. The ctors access to the theme may only be as hint, so it knows what to do and perhaps hold on potentially heavy things fonts. For example, the label's size is not an absolute size, but a scale that is multiplied by the theme's label font size:

https://github.com/cycfi/elements/blob/master/lib/src/element/misc.cpp#L119

Aside: you might notice that a one line above is: _font.c_str() which is actually a bad idea because it does not allow overriding. I am thinking of a ways to fix that. It might be that theme at ctor time held in a member variable conflicts with theme overriding after all. Hah! Let's ponder on it some more...

I hope I am making sense, almost thinking out loud! Let's talk about this some more in our next voice call.

[djowel]

Aside: you might notice that a one line above is: _font.c_str() which is actually a bad idea because it does not allow overriding. I am thinking of a ways to fix that. It might be that theme at ctor time held in a member variable conflicts with theme overriding after all. Hah! Let's ponder on it some more...

Ah got it!... See the latest labels work-in-progress code here:

https://github.com/cycfi/elements/blob/text_revisit/lib/src/element/misc.cpp#L117)

1. The theme should hold actual font objects, not strings.
2. The _font member in the label should be optional (e.g. std::optional<font>). That way, we use the theme's label font if it is not set, and use the user supplied font at its ctor otherwise.

[Xeverous]

You rebuild the view content. For example, here's the point:

I get the example but I mean such situation:

    auto ptr = share(make_controls(view_));
    view_.content(ptr, share(background));

    // change theme...

    view_.content(ptr, share(background));

Will this reapply the theme? I think not. I think I need to call make_controls again to change the theme.

Always defer theme access as much as possible at draw time to allow dynamic overriding. The ctors access to the theme may only be as hint, so it knows what to do and perhaps hold on potentially heavy things fonts.

That's exactly what I was asking for - a recommendation when a theme object should be used.

1. The theme should hold actual font objects, not strings.
2. The _font member in the label should be optional (e.g. std::optional). That way, we use the theme's label font if it is not set, and use the user supplied font at its ctor otherwise.

Shouldn't this be a sort of font pointer/reference? If I have multiple labels, which all use the same but non-standard font I don't want to construct multiple expensive font objects.

[djowel]

You rebuild the view content. For example, here's the point:

I get the example but I mean such situation:

    auto ptr = share(make_controls(view_));
    view_.content(ptr, share(background));

    // change theme...

    view_.content(ptr, share(background));

Will this reapply the theme? I think not. I think I need to call make_controls again to change the theme.

Just don't do that :-) The elements inside a view should be self-contained. Yes, just call make_controls again. Elements construction should be fast.

Always defer theme access as much as possible at draw time to allow dynamic overriding. The ctors access to the theme may only be as hint, so it knows what to do and perhaps hold on potentially heavy things fonts.

That's exactly what I was asking for - a recommendation when a theme object should be used.

Ideally, anywhere where there's a context.

1. The theme should hold actual font objects, not strings.
2. The _font member in the label should be optional (e.g. std::optional). That way, we use the theme's label font if it is not set, and use the user supplied font at its ctor otherwise.

Shouldn't this be a sort of font pointer/reference? If I have multiple labels, which all use the same but non-standard font I don't want to construct multiple expensive font objects.

The font is a ref-counted handle. Same fonts are shared:
https://github.com/cycfi/elements/blob/text_revisit/lib/src/support/font.cpp

[djowel]

Oh, we might not support synthesizing styles.

Note: On Mac with Cocoa, Bold, Italic and BoldItalic must have the proper font, or the style is simply not available. It was not always the case. Carbon used to synthesize these styles by playing on weight and slant. Windows still synthesizes Bold, Italic and BoldItalic when the specific stylistic variation font is not present.

But that practice is going out of style, IMO. Synthesized styles will never be as good as true weighted fonts (e.g. bold) and italics. Modern fonts support many style variations.

https://docs.huihoo.com/apple/wwdc/2012/session_226__core_text_and_fonts.pdf
"CoreText does not synthesize font styles (bold, italic) • Clients may choose to do something for compatibility reasons ■ Italic: Skewed font matrix ■ Bold: No best approach"

[Xeverous]

Wow, the image looks really good.

But that practice is going out of style, IMO. Synthesized styles will never be as good as true weighted fonts (e.g. bold) and italics. Modern fonts support many style variations.

Does it mean elements will not support rendering fonts with such styles if they are not in the font itself? I'm fine with it, jus no bad limitations like "not being able to use the font at all when a subset of styles is not in it".

OK, that worked. Simply: fontconfig+cairo. Now I need to fugure out how to get fontconfig into the windows build. Perhaps https://github.com/tgoyne/fontconfig will help.

fontconfig is already a dependency of cairo. I have already built it. Now just need to add it to the CMake recipe.

[djowel]

Wow, the image looks really good.

But that practice is going out of style, IMO. Synthesized styles will never be as good as true weighted fonts (e.g. bold) and italics. Modern fonts support many style variations.

Does it mean elements will not support rendering fonts with such styles if they are not in the font itself? I'm fine with it, jus no bad limitations like "not being able to use the font at all when a subset of styles is not in it".

Did you know that Adobe graphics programs such as Photoshop and Illustrator do not fake bold and italic fonts? And indeed, the MacOS, known for its superiority in typography (and graphics) stopped doing so in the OS level. It makes sense, if you think about it. Faking bold and italics were a necessity in the old days when we have limited fonts available and computer typography was in its infancy.

I am an artist and I want Elements to have superiority in aesthetics and design. I have a keen eye on such things and I dislike faking things, especially now that you do not have to because there are a LOT of superb font families available.

Perhaps these articles will be more convincing:
Say No to Faux Bold
The Right Bold & Italic for the Right Job

OK, that worked. Simply: fontconfig+cairo. Now I need to fugure out how to get fontconfig into the windows build. Perhaps https://github.com/tgoyne/fontconfig will help.

fontconfig is already a dependency of cairo. I have already built it. Now just need to add it to the CMake recipe.

Wonderful!

[Xeverous]

I am an artist and I want Elements to have superiority in aesthetics and design. I have a keen eye on such things and I dislike faking things

I like this approach. I know that fake bold does not look really good (already seen it a couple of times when experimenting with bold in CSS, which has 8 levels IIRC - lots of them look the same or have very irregular line thickness depending on the font).

I like your attention to details. Recently found an article that was about how many UIs commit the mistake of non-assymetric ranges (where asymetric ranges are standard, basically [first, last) where the dist = last - first and last points 1 past actual last object). Because of this mistake, it was impossible to either move a widget to the top or bottom without 2+ drags and drops (always appeared 1-off the maximum due to lack of "1-past last element" logic for cursor moves).

[djowel]

I like this approach. I know that fake bold does not look really good (already seen it a couple of times when experimenting with bold in CSS, which has 8 levels IIRC - lots of them look the same or have very irregular line thickness depending on the font).

Indeed, a well-designed application should not have any fake fonts in its UI.

[Xeverous]

How about a theme editor example? I noticed some colors don't play nicely (eg black background because a lot of code uses color.level() and anything multiplied by 0 is 0) and I think it would be good to have a playing ground for experimenting with different themes. This could also help catching some bugs in case some elements have hardcoded color values or use the theme object incorrectly.

[djowel]

That will be interesting!

[Xeverous]

Somewhat a blind guess but I think we will need to remove all of .level() calls in any drawing code and replace them with theme-supplied values.

This is because:

• .level(x) with x > 1.0 has no effect when the color is white or close to white and x is big enough
• .level(x) with x < 1.0 has no effect when the color is black or close to black and x is small enough

Also, have you thought about making theme not to be a global object? Eg be it a parameter of the draw function? Are there any strong reasons why the theme is global?

[djowel]

Somewhat a blind guess but I think we will need to remove all of .level() calls in any drawing code and replace them with theme-supplied values.

Good point! I'll need to think about this some more.

Also, have you thought about making theme not to be a global object? Eg be it a parameter of the draw function? Are there any strong reasons why the theme is global?

I recall you asked this before? Yes, that was originally a part of the context. But remember the construction things? The theme object somehow needs to be available at construction time on many gallery items.

[Xeverous]

I recall you asked this before?

Yes, even in this issue...

The theme object somehow needs to be available at construction time on many gallery items.

Right, this complicates things. But why is it needed? I'm interested what exactly is taken from the theme. If theme is needed at construction, this means that some elements keep theme data in their state - do we really want that? That state is only used in drawing.

[Xeverous]

grep -rn -C 5 get_theme lib/include/ is telling me that the factories are using fonts, colors and margins to construct element hierarchies. Theme-supplied values at this point are not being used, they are just passed to constructors.

IMO this creates 2 problems:

• it blocks theme from not being global
• if elements are reading theme data at construction, this means that changing the theme at runtime will not affect them because they are not rereading theme on drawing; one must reconstruct element hierarchy to let theme change take effect

[djowel]

• it blocks theme from not being global

Yes, and I don't see any other better solution right now. Believe me, I've not taken this lightly when I moved to a global theme. Show me an alternative solution and I will consider it: pick one example in the gallery and try to come up with a non-global theme.

• if elements are reading theme data at construction, this means that changing the theme at runtime will not affect them because they are not rereading theme on drawing; one must reconstruct element hierarchy to let theme change take effect

Yes, and we already discussed that before as well.

[Xeverous]

Show me an alternative solution and I will consider it

Fine. This "prove me by PR" way of dealing with things has been working quite well recently.

The task seems simple: just move all of the state that is accessed in gallery element factories to respective drawing functions. There is just 1 problem: some widgets are keeping some state by design. An input box, frame etc will always use the same theme variable. But some elements (eg button) holds the choosen color as a part of its state and each button can have different color regardless of the theme.

To solve this problem, we would need buttons etc to either:

• store state as abstracted information (eg enum mapping to specific theme variable)
• not store any state, having a separate button type for each color, different button types would read different theme variables

Both are problematic, because they do not support arbitrary number of custom-color buttons. And it can be easily predicted that some library users will want to have specific buttons in certain colors regardless of the theme.

[Xeverous]

Any work related to making theme non-global is secondary priority, first I would like to develop a good theme editor (with example widgets) in order to test theme and detect all kinds of theme inconsistencies/limitations/bugs. I will create a separate branch for this.

Current work notes:

• theme::controls_color and theme::icon_button_color are dead code (not used anywhere apart from theme object itself)
• I like to notice and implement some patterns/regularity/consistency and I think theme is a thing that can benefit a lot from such approach - it already contains some variables that are tied to specific widgets and grouped together.

I will post more conclusions as I experiment more with theme editor example.

[Xeverous]

This appeared recently:

lib\src\element\gallery\check_box.cpp: In member function 'virtual void cycfi::elements::check_box_element::draw(const cycfi::elements::context&)':
lib\src\element\gallery\check_box.cpp:15:19: warning: variable 'indicator_color' set but not used [-Wunused-but-set-variable]
   15 |       color const indicator_color = theme_.indicator_color;
      |                   ^~~~~~~~~~~~~~~