Add header property to labels, revert non headers to regular font.

[reduz]

• When designing UIs, labels are often be used as headers to give section names.
• This PR makes it possible to do so where relevant.
• Labels are also non-bold again by default, only bold when marked as headers.

This PR should fix all the problems of bold fonts where they should not be, I have been checking one by one all the usages.

The header size can be chosen from presets (Small, Medium, Large). This allows for creating modern UIs with Godot with reusable preset sizes more efficiently, as per these examples which use multiple header sizes:

and should work as a base for ourselves for modernizing Godot UI.

Godot never supported this in the past because our font support was limited, but this is a common feature of modern UIs and the fact we now support variable font sizes properly is main the reason for it.

Q: Why not a workaround using theme overrides or theme custom types?
A: After checking several applications, headers with multiple sizes are an integral part of modern ones, and are thoroughly used across user interfaces (see examples above). Given this is a very commonly used feature, it should be core and not a workaround (workarounds are designed for corner cases). Additionally, it is common for non-programmers to build user interfaces in game development teams, so having the ability to do this easily (without resorting to code or having to remember strings) is important.

[KoBeWi]

It would be nice if the PR included docs for the new properties.

[reduz]

@KoBeWi done

[KoBeWi]

I went over some dialogs and this should likely be header too

Also by documenting properties I meant theme items too (basically anything that was added and doesn't have description). But maybe leaving it for another PR is ok too.

[DarkKilauea]

You might want to consider setting up a set of fonts and sizes to be used semantically across the editor. As an example, UIKit used to have header sizes, but switched to a semantic system with fonts for titles, subtitles, captions, bodies, and more. See: https://developer.apple.com/documentation/uikit/uifont/textstyle

Likewise: Small, Medium, and Large may not help the user know which header size should be used in which situation.

I'm concerned that creating a special property for "headers" ignores the styling you might want to enforce with fonts across the editor and creates an API that you'll need to support throughout 4.x (since it's added directly to Label and isn't specific to the editor). A proposal might be warranted here.

[reduz]

@DarkKilauea We discussed this, but my point is that the aim of this feature is so it can be used mostly from the UI in a relatively easy to use matter by just changing something in the inspector. Doing it as suggested in #49347 requires to manually put text in the inspector, which is not user friendly.

[KoBeWi]

Looks ok. I still prefer my implementation slightly more, but either one is fine.

[reduz]

@KoBeWi you still did a great job 👍 sorry for being so obsessive with user friendliness.

[menip]

Variable font size question, not related to this header implementation

  the fact we now support variable font sizes properly

In terms of rendering? Or will we be able to set font size directly on label without adding custom fonts now?

[YuriSizov]

@menip Font size is already a separate property in master, on every control.

[CsloudX]

I think font was a property for all controls, not just for label. if label has this features, why other controls not ?
Maybe all control has the font property, user can change size, color, bold etc. not set as header.

[groud]

I'm still not sold about going this route over @KoBeWi 's PR one. It kind of changes the paradigm we use for theming, which introduces inconsistencies which makes things harder to understand for newcomers. Not even mentioning that people will ask for a way to change the colors of headers too.

Also, there's already a way to define custom theming for controls (without overriding each component) implemented by #47544. It seems specifically built for this exact use case, so I think it might make sense to use it instead.

But that being said, I agree that the current system isn't ideal. Using strings as identifiers isn't great. But IMO that could be changed. Maybe we could use integers instead, with a set of built-in constant identifiers and ways to register custom ones for example. I also agree on the fact that this is a simple change, and that it might make sense to be able to easily define a label as header easily, which this PR implements better than @KoBeWi 's one.

However, it kind of disappoints me to go the hackish route instead of finding a way to make the theming system more intuitive... Ideally changing a label's theme to "header" should be as simple as selecting an entry in a dropdown menu, but that should be the same for all basic theming elements IMO, not only for setting a label as a header.

[KoBeWi]

What if we could add selectable custom types to Theme? Something like theme.add_custom_type("Label", "Header") and then we could select it from a list instead of typing:

It's basically my approach, but with the string problem solved.

[YuriSizov]

What if we could add selectable custom types to Theme? Something like theme.add_custom_type("Label", "Header") and then we could select it from a list instead of typing:

It's basically my approach, but with the string problem solved.

@KoBeWi The problem is not with setting it in a Theme resource. The problem is that in the inspector for a specific control we have no idea what themes are applied to it. Theme can be applied to one or many parents in that scene, or parents in the running project where that scene is instanced, or it can be the Project-level theme, or it can be the default theme. And in actuality it will be a combination of all of the above.

Only the latter two can be parsed for the inspector somewhat reliably, and we can do that. But intermediate themes are impossible to fetch for this purpose due to how dynamic theming in Godot is.

[reduz]

This is such a stupid change that, to be completely honest, I don't understand the opposition to it. As I explained, I am the one doing the work of fixing all the labels and the headers and I prefer to implement it this way. I don't like to use strings that I have to guess. In C++ within the engine this is tolerated because it's easier, but in anything exposed to the outside (GDScript, Mono, etc) we have been replacing those by keywords in every area of the engine for the past two years. Ideally that should happen to Theme too (see proposal godotengine/godot-proposals#2832).

In this case though, if text was used, it would be impossible to replace by a keyword. The excuse that this will only be used internally in the editor so we should make an exception is moot, because if you want to use headers in a regular application or UI (which we can now more easily because of variable font sizes), you would need to be forced to put text manually in every place, or creating a script. Both solutions are much worse than what is proposed in this PR.

[reduz]

@groud The system used integers before in previous versions of the engine (before it was named Godot):

https://github.com/reduz/chibitracker/blob/master/gui/base/frame.h#L25

It was changed to strings because it does not scale. Generally strings are not a problem on the C++ side, what is important is that they are not exposed as such on the higher level languages because this kills usability. See my proposal on the comment above to solve this.

[reduz]

@KoBeWi

What if we could add selectable custom types to Theme? Something like theme.add_custom_type("Label", "Header") and then we could select it from a list instead of typing:

This is difficult because, as @pycbouh mentions, you don' t exactly know what is there. Again, the theme system is limited and should be overhauled and possibly redesigned at some point. The challenge is how to do this without incurring in adding a huge amount of complexity just for a very simple use case like this.

In any case, I believe I went through all the possible alternative solutions in my head already so I am confident this one is the best I can come up to in the context of the current theme system. I suggest this is merged for the time being, given without major changes to themeing, the other options are worse. It also fixes for the time being the bold fonts, which many are complaining about.

Afterwards, we can discuss together a new theme system where keywords can be used, and where the "presets" to override controls can be exposed better to the UI, which seems like it would be the ideal solution.

[groud]

As I explained, I am the one doing the work of fixing all the labels and the headers and I prefer to implement it this way. I don't like to use strings that I have to guess.

Well, sorry but to me that's not an argument. If we changed all APIs only according to our usage as developers, it would be quite bad IMO.

In this case though, if text was used, it would be impossible to replace by a keyword. The excuse that this will only be used internally in the editor so we should make an exception is moot, because if you want to use headers in a regular application or UI (which we can now more easily because of variable font sizes), you would need to be forced to put text manually in every place, or creating a script. Both solutions are much worse than what is proposed in this PR.

Yeah, that's basically what anyone has to do already for any other theming. As I said, I agree this is not good. It's not easy to use at all and needs a rework. What I say is that your PR only fix this issue for Label headers, while it is a global problem.

I'm not 100% against this solution, but well, it's a bandage on a wooden leg that we will need to keep compatibility for even if we come up with a better solution for theming in general. Consequently, I am not sure that the little gain we get in usability from using an enum instead of typing text is worth the difficulties we will have to face due to those workarounds.

[reduz]

@groud I like that we agree that the current state of affairs regarding to theming is not good. This is why I suggest lets merge this for now (so fonts go back to normal, at least), then work on a proposal to revamp the theme system. I like the solution @KoBeWi proposed, but its not doable as-is, and we also need to figure out how to export theme properties as keywords for the higher level languages.

[KoBeWi]

Only the latter two can be parsed for the inspector somewhat reliably, and we can do that. But intermediate themes are impossible to fetch for this purpose due to how dynamic theming in Godot is.

Is it really? Controls already display properly in the editor, so they can determine the correct theme. The list could be fetched from the node. The new theme editor already allows adding theme items for arbitrary types, so we could just check the Theme tree for any properties that are assigned to custom type and add the type to the list. The only problem is that these types don't have hierarchy, so you can't make the Header be Label sub-type.

[YuriSizov]

Is it really? Controls already display properly in the editor, so they can determine the correct theme.

You probably add your theme to every scene manually for that. Or maybe you set the project level theme. If you set your theme to the topmost control node (basically like gui_base in the editor) and let it propagate, other scenes will not be able to pick on that, obviously. Like, each dedicated scene can be instanced at an arbitrary point of the scene tree of your running project. And each of these points can have it's own theme property set up. So the look will differ depending on the point of the mount.

The list could be fetched from the node.

It cannot be. A dedicated scene is a dedicated scene, it has no information on how it will be used. Again, some of the information can be fetched from the control itself, a project level theme or a default theme. If we try, we can even get themes from local parents. But that would only work if you manually set theme for each of your scenes, which is not required in practice, unless you want in-editor previews to look absolutely correct. In practice a theme can be propagated from a control way outside of your edited theme when the project is run.

We can, of course, still support what we can as long as we allow manual input for theme_custom_type if suggested options don't satisfy the user.

The only problem is that these types don't have hierarchy, so you can't make the Header be Label sub-type.

That is indeed a problem currently, but we can still show all the types. Beats not showing any. And as a matter of the fact, I've planned for types to eventually be dependent on each other. Which is why get_type_dependencies() is a part of the Theme class.

godot/scene/resources/theme.cpp

Lines 1361 to 1369 in 3041bec

	void Theme::get_type_dependencies(const StringName &p_theme_type, List<StringName> *p_list) {
	ERR_FAIL_NULL(p_list);
	StringName class_name = p_theme_type;
	while (class_name != StringName()) {
	p_list->push_back(class_name);
	class_name = ClassDB::get_parent_class_nocheck(class_name);
	}
	}

At the moment it only does the old logic for class hierarchy but we can also add type dependency there and store it in the Theme resource. I've prepared for this 🙃

[reduz]

@pycbouh @KoBeWi Just to get you in line with what I am thinking. When I propose that theme properties should be accessible as keywords, what I mean is that we should probably have a central theme class that manages this, that resembles more of a singleton and that contains the default theme values. Currently we do have this (the " default theme" which is kind of a singleton) but its still just a Theme.

The idea would be that the theme singleton would no longer be a Theme, but something that contains the base structure for all themes. If you want to add an override for something, a custom type, custom property, or whathever, you do there. Themes would only be able to override what is there.

If you want to add custom ones in Theme (not the central one), similarly to how you can add user signals, you can but those would no longer be exposed as properties. Most of the editor stuff will probably go that route, so it does not pollute regular theme editing.

For custom theme types, this would be a global project setting separate from theme editing (which would let you edit them if you want). This would allow to show the proper available overrides in the inspector, as @KoBeWi proposes.

All this said, for this very specific use case, the header one still needs to be part of the default one, because VBoxContainer uses it in add_margin_child(), so it will still not be an editor-only thing. It may not be a property of label, but it will need to still exist.

[YuriSizov]

@reduz I'm not sure that a singleton is a good idea. Maybe on paper it sounds like a reasonable reorganization, but my main issue after working with Themes in projects and working with Themes in engine is that there is a gap between theming in engine and in user projects. In the engine everything is set up for classes to have their own "theme space", a set of theme properties. Like the code I've linked above is targeted to work with the ClassDB.

This is something that is not accessible to the end users. They cannot seamlessly integrate a new node type into this system, which is why I've designed that custom type property. And while it helps with that a bit, it's still far from ideal and reusable. So if you want to rework Themes please consider a solution that would make it easier for users to create new reusable nodes that can define their own set of theme items. And it should work in a way that a node like that shared on the AssetLib needs to completely encapsulate everything in its scenes and its code. This cannot work well if you require users of such nodes to go to the project settings and input some types there.

Which is why I initially talked about making all controls expose "contracts" for the theme items they accept. So it's a control-side logic, not a global singleton logic. Or if it is a global singleton, I'd prefer a declarative way to interact with it (like get_property_list) instead of ThemeSingleton->add_type.

[reduz]

@pycbouh I see, I did not think about add-ons. Maybe the simplest solution in this case would simply to be able to somehow more than one default theme? Then the theme editor lets you also choose to edit those values too in a new theme.

[reduz]

OK, from discussions here and on IRC, it seems like having theme extensions and type variations may be a better solution to all the problems discussed. This still depends on features that are not there (editor scanning extensions and listing them, and theme registering them on load.). I will work on this and re-make this PR to use them, so I am converting it to draft for the time being.

[Xrayez]

To be honest, looking at the code, this makes it more readable and/or gives better intention at least. I'm not particularly versed in GUI development with Godot, but I do find this kind of approach intuitive for such a common use case as displaying headers.

To make comparisons with web development, this kind of reminds me of using HTML's heading tags. While it's possible to use CSS styling to adjust literally everything, webdevs do use <h1>...<h6> tags to adjust semantic size rather than physical size, so to speak. Same applies to markdown #, ##, ### etc. The only difference is that Godot's GUI system is more imperative than declarative, but I've seen many users requesting a declarative approach to make GUI in Godot.

So, in my humble opinion, this must be implemented to satisfy those needs, regardless of particular implementation.

[reduz]

Closing, superseded by #50420