Refactor editor theme generation and add spacing presets #87085

YuriSizov · 2024-01-11T17:49:27Z

I'm going to invalidate so many PRs with this one...

This is a complete rework of the editor theme generation logic. For the longest time our theme generator has been an unmaintainable spaghetti mess with a lot of cross-borrowing between various definitions without a clear structure.

This naturally made maintaining generated editor themes much harder than it needs to be. We also didn't have a good picture of which parameters can be exposed to users and what effect such exposure would have. So in this mega rework I try to bring some order into this.

This PR contains 3 commits. 2 of them are about organization and cleanup, and the last one introduces a long awaited feature (but in its first iteration, with more work expected in future).

Reorganize code related to editor theming

This change introduces a new EditorThemeManager class to abstract theme generation and its subroutines.
Logic related to EditorTheme, EditorColorMap, and editor icons has been extracted into their respective files with includes cleaned up.
All related files have been moved to a separate folder to better scope them in the project. This includes relevant generated files as well.

This commit acts as a starting point for the changes that follow it. It's pretty extensive in the way it touches across the codebase, and it introduces new files and build system boilerplate changes. Pretty much any PR that is currently touching the old editor_themes.h/cpp needs to be rebased because of this. But such is the price of a better code structure.

Some of the changes here are made with further plans to optimize theme generation and make it more granular and avoid regenerating themes in their entirety if possible. I also plan to base editor theme caching on this, which should speed up editor startup by about half a second (on my laptop, at least).

Split theme generation logic into several subroutines

This change splits the theme generation process into more manageable subroutines, encapsulating one part of the logic.
To better control how parameters and shared definitions are reused across the theme, a new theme configuration struct has been added.
Everything not passed and not explicitly shared — is scoped. This is done to prevent it from being automatically accessible throughout the process. This should ensure that the decision to share styles is a conscious one.

In the future we should try to reduce the number of unique definitions and share most of what we have. Simplicity should make it easier for us to define guidelines for existing and future editor UI/UX features. This PR is a stepping stone on this path.

This change also includes some effort separating redefinitions of default theme items and custom types introduced only by the editor. In a few cases where editor-specific definitions need to reference default definitions we simply fetch them from the theme. It's not ideal and hides the dependency a bit, but hopefully these cases will be abstracted properly in due time.

This is another change that invalidates existing PRs targeting editor_themes.h/cpp.

Introduce editor theme spacing presets

This is the eye candy of the PR, although, to be completely honest, the work only begins here.

The third commit adds new editor settings to give users more control over compactness/spacing of the editor theme. It comes with 2 numerical properties and 3 presets (Compact, Default, Spacious). Numerical properties can be completely customized to achieve a more desirable look if presets aren't satisfactory.

Limited effort was applied to make sure both Compact and Spacious presets work and look fine, but further tuning
and adjustments are totally expected. Some controls will require layout changes or more granular fixes to their constants to make the most of it.

This change supersedes #86363.
Bugsquad edit: Implements godotengine/godot-proposals#8505.

Note that adjusting spacing alone may not be enough, and changing fonts and font sizes is another thing you want to do for a better compact/spacious experience.

For the rough comparison, here's how these presets look compared to the current master:

Current master

godot windows editor dev x86_64_2024-01-11_18-49-08

This PR — Default

godot windows editor dev x86_64_2024-01-11_18-12-18

This PR — Compact

godot windows editor dev x86_64_2024-01-11_18-11-48

This PR — Spacious

godot windows editor dev x86_64_2024-01-11_18-12-49

YuriSizov · 2024-01-11T17:53:11Z

Notes for testing:

The first and the second commit should not result in any discernible visual changes. I would suggest you check the new code as if it was new, because comparing to the old version would be quite a waste of time. You'd have to trust me that nothing was lost in the translation.
Currently when you change theme settings and the theme is regenerated, there are errors in the output log related to benchmarks. This is a problem in master, and I will fix it in a follow up.
I think there are some issues with the SplitContainer and the grab area doesn't actually extend beyond the separation/grabber icon.

tokengamedev · 2024-01-12T13:03:06Z

Awesome Work ❤️
It is exactly what I was asking for too.

Couple of things, that can be looked into.

The font sizes can be played around in spacious and compact as in spacious the font looks small
Default and compact looks the same. I assumed Default should be as the editor currently is (may be it is the screenshot)
Icon sizes may be looked into.

For me @passivestar image above looks almost ideal to have,

YuriSizov · 2024-01-12T13:11:05Z

@tokengamedev

The font sizes can be played around in spacious and compact as in spacious the font looks small

Because font sizes are configured on another settings page, I didn't want to automatically adjust them because it may not be expected by users. But as mentioned in the OP, I agree with the idea that for the best experience you might want to adjust them as well.

Default and compact looks the same. I assumed Default should be as the editor currently is (may be it is the screenshot)

They are definitely not the same. Some elements have similar spacing, but that's because they are already very compact. But a lot of extra spacing is removed.

Compare Default to Compact. And then to the Current master. Note that Default is not exactly the same as the current master because we've discussed reducing the spacing even for the default layout, which this does. But Compact is still more... compact.

Icon sizes may be looked into.

Yeah, that's a good point. Currently all icons are scale solely based on the editor scale factor, but we may want to introduce a separate icon scale and use it here, especially for the Spacious mode.

Calinou · 2024-01-12T15:59:04Z

The UI looks great on the screenshots, although Spacious feels a little excessive. It probably needs testing on a tablet to see if it really needs to be this spacious, or whether a middleground would be more suited.

adamscott

Few nitpicks, but it's great overall! Great job @YuriSizov !

editor/themes/editor_theme_builders.py

editor/themes/editor_theme_manager.cpp

editor/themes/editor_theme.cpp

Listwon · 2024-01-14T10:07:42Z

Removing margins/paddings in labels here (Import As, Compress) hurts readability, it shouldn't stick to the edge of the window/screen. If we can afford large indentations for properties, we can afford subtle margins for labels too.

YuriSizov · 2024-01-15T10:26:39Z

@Listwon Thanks, this is known. This is why this is only the first draft of the compact mode and why further tuning is expected. I may look into some tweaks before we merge this, but it's not really blocking — for us to get the compact mode right with this PR, and follow-ups will address these issues.

YuriSizov · 2024-01-15T12:43:35Z

I addressed the feedback by @adamscott (the new method is renamed, formatting in some affected python files improved).

I also adjusted Trees in the compact mode, which should now be close to what @passivestar outlined in his proposal. I also made the defaults for the spacious mode a little less spacious. Here's quick comparison:

Default preset

godot windows editor dev x86_64_2024-01-15_13-38-38

Compact preset

godot windows editor dev x86_64_2024-01-15_13-38-51

Spacious preset

godot windows editor dev x86_64_2024-01-15_13-39-07

Also ~~fixed this gem~~ addressed this confusing code, present in master:

YuriSizov · 2024-01-15T12:44:40Z

With these updates I'd like for this to be considered for merging. This way others will be able to contribute to the compact mode math, and we can also start a tracker with the findings from early testers.

akien-mga

Looks good to me!

This change introduces a new EditorThemeManager class to abstract theme generatio and its subroutines. Logic related to EditorTheme, EditorColorMap, and editor icons has been extracted into their respective files with includes cleaned up. All related files have been moved to a separate folder to better scope them in the project. This includes relevant generated files as well.

This change introduces a new theme configuration struct to be passed to the aforementioned routines to better control reuse of styles and definitions in the generator. Everything not passed and not explicitly shared is scoped so it is not automatically accessible throughout the routine. This should ensure that the decision to share styles is a conscious one. In the future we will try to reduce the number of unique definitions and share most of it. This PR is a stepping stone on this path. This also puts the effort into separating redefinitions of default theme items vs custom types introduced only by the editor. In a few cases where editor-specific definitions need to reference default definitions we simply fetch them from the theme. It's not ideal and hides the dependency a bit, but hopefully these cases will be abstracted properly in due time.

This change adds a new editor setting related to theming which controls base and additional spacing used in the generated editor theme. These values can also be changed manually by the user to customize their experience. Limited effort was applied to make sure both Compact and Spacious presets work and look fine, but further tuning and adjustments are totally expected. Some controls will require layout changes or additional fixes to their constants.

YuriSizov · 2024-01-16T11:06:15Z

Rebased on master after #84193.

akien-mga · 2024-01-16T11:55:05Z

Thanks!

KoBeWi · 2024-01-16T23:01:51Z

editor/themes/editor_theme_manager.cpp

+		p_theme->set_constant("buttons_separation", "AcceptDialog", 8 * EDSCALE);
+
+		// FileDialog.
+		p_theme->set_icon("folder", "FileDialog", p_theme->get_icon(SNAME("Folder"), EditorStringName(EditorIcons)));


Inconsistent use of SNAME. This line has 4 StringNames, but only one is SNAME; 2 use raw strings. Though we have such cases everywhere in the codebase :/

None of this should be SNAME btw.

Yeah, we can do a cleanup here. I also wanted to add get_editor_icon to EditorTheme to avoid all the repetition.

YuriSizov added enhancement topic:editor usability topic:gui topic:codestyle labels Jan 11, 2024

YuriSizov added this to the 4.3 milestone Jan 11, 2024

YuriSizov requested review from a team as code owners January 11, 2024 17:49

YuriSizov force-pushed the editor-improved-theme-flexibility branch 2 times, most recently from a6fdb2a to f0cd5cc Compare January 11, 2024 18:54

YuriSizov requested a review from a team as a code owner January 11, 2024 18:54

adamscott approved these changes Jan 12, 2024

View reviewed changes

YuriSizov force-pushed the editor-improved-theme-flexibility branch from f0cd5cc to 7357f55 Compare January 15, 2024 12:40

akien-mga approved these changes Jan 15, 2024

View reviewed changes

YuriSizov added 3 commits January 16, 2024 11:57

YuriSizov force-pushed the editor-improved-theme-flexibility branch from 7357f55 to dc3b07e Compare January 16, 2024 11:04

akien-mga merged commit 4b55c81 into godotengine:master Jan 16, 2024
15 checks passed

This was referenced Jan 16, 2024

Make editor's dock gaps and trees line heights configurable in editor settings godotengine/godot-proposals#8505

Closed

Add interface/theme/{window_border_margin,top_bar_separation,default_margin_size} editor settings #86363

Closed

YuriSizov deleted the editor-improved-theme-flexibility branch January 16, 2024 11:57

KoBeWi reviewed Jan 16, 2024

View reviewed changes

YuriSizov mentioned this pull request Jan 17, 2024

Improve editor theme generation after the refactor #87293

Merged

Calinou mentioned this pull request Jan 22, 2024

Change the default editor font to look better on small displays godotengine/godot-proposals#3764

Open

Rubonnek mentioned this pull request Feb 15, 2024

Editor crashes upon adding arguments to BTCallMethod limbonaut/limboai#40

Closed

kitbdev mentioned this pull request Feb 20, 2024

Allow reducing SplitContainer drag bar thickness using the default themes #74691

Open

Jowan-Spooner mentioned this pull request Feb 21, 2024

Editor theme color renames break compatibility for plugins #88626

Closed

Mickeon mentioned this pull request Mar 8, 2024

Add pre-4.3 Editor theme color names for compatibility #89302

Merged

Calinou mentioned this pull request Mar 19, 2024

PopupMenu horizontal margins in the editor theme became too low since 4.3.dev3 #89697

Open

kleonc mentioned this pull request Mar 20, 2024

Add minimum width/height to dialog buttons #89693

Merged

emmanuel-ferdman mentioned this pull request Apr 6, 2024

Update the location of editor theme #90307

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor editor theme generation and add spacing presets #87085

Refactor editor theme generation and add spacing presets #87085

YuriSizov commented Jan 11, 2024 •

edited by akien-mga

Loading

YuriSizov commented Jan 11, 2024 •

edited

Loading

tokengamedev commented Jan 12, 2024

YuriSizov commented Jan 12, 2024

Calinou commented Jan 12, 2024

adamscott left a comment

Listwon commented Jan 14, 2024

YuriSizov commented Jan 15, 2024

YuriSizov commented Jan 15, 2024 •

edited

Loading

YuriSizov commented Jan 15, 2024

akien-mga left a comment

YuriSizov commented Jan 16, 2024

akien-mga commented Jan 16, 2024

KoBeWi Jan 16, 2024 •

edited

Loading

YuriSizov Jan 16, 2024

Refactor editor theme generation and add spacing presets #87085

Refactor editor theme generation and add spacing presets #87085

Conversation

YuriSizov commented Jan 11, 2024 • edited by akien-mga Loading

Reorganize code related to editor theming

Split theme generation logic into several subroutines

Introduce editor theme spacing presets

Current master

This PR — Default

This PR — Compact

This PR — Spacious

YuriSizov commented Jan 11, 2024 • edited Loading

tokengamedev commented Jan 12, 2024

YuriSizov commented Jan 12, 2024

Calinou commented Jan 12, 2024

adamscott left a comment

Choose a reason for hiding this comment

Listwon commented Jan 14, 2024

YuriSizov commented Jan 15, 2024

YuriSizov commented Jan 15, 2024 • edited Loading

Default preset

Compact preset

Spacious preset

YuriSizov commented Jan 15, 2024

akien-mga left a comment

Choose a reason for hiding this comment

YuriSizov commented Jan 16, 2024

akien-mga commented Jan 16, 2024

KoBeWi Jan 16, 2024 • edited Loading

Choose a reason for hiding this comment

YuriSizov Jan 16, 2024

Choose a reason for hiding this comment

YuriSizov commented Jan 11, 2024 •

edited by akien-mga

Loading

YuriSizov commented Jan 11, 2024 •

edited

Loading

YuriSizov commented Jan 15, 2024 •

edited

Loading

KoBeWi Jan 16, 2024 •

edited

Loading