godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
89.11k stars 20.2k forks source link

Theme scale behaves incorrectly and does not apply #83547

Open Swarkin opened 11 months ago

Swarkin commented 11 months ago

Godot version

v4.2.beta1.official [b1371806a]

System information

Godot v4.2.beta1 - Windows 10.0.22621 - GLES3 (Compatibility) - Intel(R) UHD Graphics 615 (Intel Corporation; 31.0.101.2115) - Intel(R) Core(TM) i3-10100Y CPU @ 1.30GHz (4 Threads)

Issue description

The theme scaling property does not get applied in the majority of cases and behaves weirdly.

  1. Project Settings gui/theme/default_theme_scale overrides any other scaling setting set in theme resources.
  2. Project Settings gui/theme/custom scaling gets ignored completely during runtime but displays as expected in the editor (See MRP).
  3. Adding a theme resource directly onto a Control node ignores theme scaling as well.

Steps to reproduce

See MRP

Minimal reproduction project

ThemeScaleMRP.zip

Goutte commented 7 months ago

I can confirm all three points on 4.2.1-stable and master.

I tried a little bug hunting with prints and saw that the Editor makes calls to ThemeOwner::get_theme_default_base_scale() when opening the scene, but the game never does.

AdriaandeJongh commented 6 months ago

Is there a workaround for this?

tiniuclx commented 3 months ago

Also affected by this - looks like Project Settings gui/theme/default_theme_scale cannot be overridden by runtime changes either: I'm setting the default_base_scale property on a Theme, and also ThemeDB.fallback_base_scale and none of them take priority over gui/theme/default_theme_scale

Calinou commented 3 months ago

looks like Project Settings gui/theme/default_theme_scale cannot be overridden by runtime changes either

With a handful of exceptions, project settings are only read when the project starts. The Theme properties ought to be effective though.

tiniuclx commented 3 months ago

Here's a minimal project showing that the default_base_scale Theme property is not taking effect. The property is set to 8, but the Project theme size setting of 1.5 takes priority

Theme default_base_scale MRP.zip

tiniuclx commented 3 months ago

Having a look at this... get_theme_default_base_scale seems to be returning the right value when called from GDScript: it returns the Default Base Scale of the nearest parent, if the property is set. Otherwise Default Theme Scale is returned. Just like @Goutte said, the problem might be that get_theme_default_base_scale is (not) called at the correct time.

tiniuclx commented 3 months ago

I think the problem is that ThemeOwner::get_theme_default_base_scale() is simply not used in a lot of places where it would be expected. For example, in a LineEdit it correctly scales the thickness of the cursor based on either the theme default base scale, or the global/fallback scale if it is not found. But things like font size are seemingly not scaled in this manner.

tiniuclx commented 3 months ago

The Problem

I got to the bottom of this. The problem is that the scaling behaviour of the default theme is very different compared to scaling of custom themes. When the default theme is initialised using default_theme.cpp:fill_default_theme(), most values are manually scaled using default_theme_scale. For example:

void fill_default_theme(Ref<Theme> &theme, const Ref<Font> &default_font, const Ref<Font> &bold_font, const Ref<Font> &bold_italics_font, const Ref<Font> &italics_font, Ref<Texture2D> &default_icon, Ref<StyleBox> &default_style, float p_scale) {
    scale = p_scale;

...
    theme->set_default_font_size(Math::round(default_font_size * scale));
    theme->set_default_base_scale(scale);
...
    theme->set_constant("h_separation", "Button", Math::round(4 * scale));
...
    theme->set_constant("underline_spacing", "LinkButton", Math::round(2 * scale));
... // many other things are scaled

Likewise, the styleboxes of the default theme are manually scaled as well upon initialization:

static Ref<StyleBox> make_empty_stylebox(float p_margin_left = -1, float p_margin_top = -1, float p_margin_right = -1, float p_margin_bottom = -1) {
    Ref<StyleBox> style(memnew(StyleBoxEmpty));
    style->set_content_margin_individual(Math::round(p_margin_left * scale), Math::round(p_margin_top * scale), Math::round(p_margin_right * scale), Math::round(p_margin_bottom * scale));
    return style;
}

However, only a very limited number of things are scaled by the default_base_scale, which is what is checked at runtime. Things that are scaled by it include LineEdit cursor thickness, underline thickness in some places, some spacings in the gradient editor & curve editor... Minor details essentially. Throughout Godot, get_theme_default_base_scale is used ~25 times.

What is notably not scaled by the default_base_scale are things like the font size, label and text edit line spacing, style box margains, style box corner radius, focus expand margin, horizontal separation, content margins, and more. In the default theme, the scale is used ~130 times!

When you create a custom theme, it is empty by default, and its theme constants and properties all come from the fallback values. _These are all scaled by fill_default_theme_, and therefore respond to gui/theme/default_theme_scale. They respond to the Theme default_base_scale property too, but the latter controls essentially only minor details.

If you create a custom theme, and import items from the default theme, then the imported items will not be affected by gui/theme/default_theme_scale, only by default_base_scale. This will make it obvious how few things are actually affected by default_base_scale

The Workaround

I searched through Discord for people also having this issue, and I found @derkork's script which scales the various theme properties, which is GPL licensed. @AdriaandeJongh you can use that if you are happy with the license terms, or implement something like that yourself.

The Solution

Before this can be fixed, we need to agree what the problem that needs fixing actually is.

@Calinou Should a deep clone of the default theme with default_base_scale set to 2 look identical to the Default Theme with gui/theme/default_theme_scale set to 2? I see you've implemented the current default theme scaling in #51159 - in my opinion Theme::default_base_scale should act like that as well.

If the answer is yes, I think we need to discuss approches for fixing it. IMO the most transparent fix would be to modify the low-level drawing code for each Control to scale its constants by default_base_scale at draw-time. However this is very labour-intensive, and risks breaking in the future as people add new things that would need to be scaled without scaling them first. I'm also not entirely sure how this would work - looking at style_box.cpp, the details of the drawing don't seem to be handled three directly.

I could modify the getters of properties that need to be scaled, to scale them with Theme::default_base_scale, but this risks being quite confusing: the value you get from something you've just set would be different than what you set.

For the record, my usecase for Theme::default_base_scale is to scale the GUI of a mobile app based on the resolution of the device. I want to calculate a scale factor based on the resolution within my project, and apply that throughout the theme using Theme::default_base_scale so that the GUI looks good on high-DPI displays that are common nowadays, as well as low-resolution displays of older / cheaper devices. I think fixing this would make the Godot editor much better menu-heavy mobile apps & strategy games.

Calinou commented 3 months ago

For the record, my usecase for Theme::default_base_scale is to scale the GUI of a mobile app based on the resolution of the device. I want to calculate a scale factor based on the resolution within my project, and apply that throughout the theme using Theme::default_base_scale so that the GUI looks good on high-DPI displays that are common nowadays, as well as low-resolution displays of older / cheaper devices. I think fixing this would make the Godot editor much better menu-heavy mobile apps & strategy games.

Let Godot's multiple resolutions support do the work for you, as described in the common use case scenarios. You don't need to scale the theme resource at all to achieve this. See also the associated demo project.

Scaling the default theme is only really needed during the prototyping stage when working with a project with a high base window size (so UI elements aren't tiny relative to this base window size).

tiniuclx commented 3 months ago

Thanks a lot for the heads-up - I found some scaling settings that work really well for my app!