Adding to default Theme results in slow editor load

Kehom commented 9 months ago

Godot version

4.2

godot-cpp version

4.2

System information

Windows 10 and Linux Mint

Issue description

When creating custom UI elements we have the possibility to use ThemeDB::get_singleton()->get_default_theme() to define the styling of the Control. However doing so significantly slows down the editor loading. Note that this is the loading time of the editor that gets way longer. Running the game/app works without any noticeable longer loading time.

That said, currently I'm calling a function from the constructor which creates the default theme styles/colors/constants... then assign them into the default theme object. I have a static flag that is set whenever this setup is completed for the first time so further instantiations of the Control don't create everything again. Just after that I call a function to cache those things into an internal struct (similarly to the core controls).

I originally wanted to build the styles and assign into the default theme from the _bind_methods() function, however at that moment get_default_theme() returns an invalid object.

I have attached a minimal custom Control to help test the slow down. In the code, however, I have added a third function to first initialize the theme cache. Indeed it leads to multiple creations of the default styles when multiple instances are created. However this is done so the function to assign things into the default theme can be commented out in order to compare the difference in the editor load time. I also added several prints telling that something is being assigned into the default theme object. This is just to show that it does take some time between each theme->set_*() call is finished.

I have noticed that the very first loading of the extension was OK, however subsequent attempts to load the project were as described, slow.

Steps to reproduce

Compile the attached theme tester Control and create a project that uses it
As described, the very first time the extension is loaded might not show any problem, however subsequent loading should be slower
Comment out a line that is indicated within the constructor then build and test again. Commenting that line will not assign things into the default theme object, which should result in faster loading time of the editor

Minimal reproduction project

theme_tester.zip

YuriSizov commented 9 months ago

This is not a supported workflow for styling custom controls. There is, unfortunately, no supported workflow for this in Godot 4.2. Some work has been done to make it possible in the future, you'll know it's available from blogposts and when https://github.com/godotengine/godot-proposals/issues/4486 is closed.

The reason why it slows down is because every time you modify the theme resource, you trigger the changed signal which propagates through the entire editor UI, since the editor is "built with Godot" and uses the same controls and same theming capabilities.

Calinou commented 9 months ago

Please check if this occurs in GDScript as well to make sure this isn't specific to GDExtension.

YuriSizov commented 9 months ago

@Calinou You can make this happen with a GDScript plugin as well.

cmontesano commented 5 months ago

@Kehom You can mitigate the slowness by initializing a new theme with your custom elements and then merging that into the default theme. This works because themes freeze propagation during the merge, so any signals are only emitted once after the merge completes.

    auto custom_theme = memnew(Theme());
    custom_theme->set_font("font", "CustomLabel", font);
    custom_theme->set_font_size("font_size", "CustomLabel", font_size);
    custom_theme->set_color("font_color", "CustomLabel", font_color);

    ThemeDB::get_singleton()->get_default_theme()->merge_with(custom_theme);

Kehom commented 5 months ago

Thanks for the suggestion. I actually went with a completely different route, in which I manually "expose" custom theme elements (using _get_property_list()) so they appear in the inspector. The way I'm exposing those themes means that I don't have to worry with the _get() and _set() because the base Control already does that. I also created a system in which I can register my custom theme elements using macros very similar to the core BIND_THEME_ITEM. With that I can also take advantage of caching those elements within an internal struct.

The major advantage of this work around is that I can perform such registration directly within the _bind_methods() function. The disadvantage here is that the ThemeEditor does not know of the custom elements, meaning that adding those require prior knowledge of what can be set. Yet, selecting a custom control and expanding the "override x" category within the inspector should provide the "list" of available items for such Control.

That said, I did try something similar to your suggestion however I wasn't entirely happy with how that initialization had to occur. More specifically, where it had to be done. In many cases trying to access ThemeDB and several other singletons result in invalid pointers because those aren't fully initialized yet.

godotengine / godot-cpp