Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding to default Theme results in slow editor load #1332

Open
Kehom opened this issue Dec 12, 2023 · 5 comments
Open

Adding to default Theme results in slow editor load #1332

Kehom opened this issue Dec 12, 2023 · 5 comments

Comments

@Kehom
Copy link
Contributor

Kehom commented Dec 12, 2023

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
Copy link
Contributor

YuriSizov commented Dec 12, 2023

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 godotengine/godot-proposals#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
Copy link
Member

Calinou commented Dec 12, 2023

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

@YuriSizov
Copy link
Contributor

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

@cmontesano
Copy link

@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
Copy link
Contributor Author

Kehom commented Apr 6, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants