Crash when loading old theme from earlier Godot versions

[Dunkhan]

Godot version

4.0 Beta 3 and 4.0 beta 6

System information

Linux Mint 20.3 Cinnamon - Intel© Core™ i7-8700K CPU @ 3.70GHz × 6 - NVIDIA Corporation TU106 [GeForce RTX 2070]

Issue description

If I try to change the theme at runtime using this.mainContainer.Theme = ServiceManager.Current.ThemeService.CurrentTheme;

It simply crashes to desktop with the following trace:

================================================================
handle_crash: Program crashed with signal 8
Engine version: Godot Engine v4.0.beta6.mono.official (7f8ecffa56834dce3ccbd736738b613d51133dea)
Dumping the backtrace. Please include this when reporting the bug on: https://github.com/godotengine/godot/issues
[1] /usr/share/dotnet/shared/Microsoft.NETCore.App/6.0.11/libcoreclr.so(+0x4dce2e) [0x7f898b5a3e2e] (??:0)
[2] /lib/x86_64-linux-gnu/libpthread.so.0(+0x14420) [0x7f89c9b41420] (??:0)
[3] /media/me/Storage/Godot/Godot_v4.0-beta6_mono_linux_x86_64/Godot_v4.0-beta6_mono_linux.x86_64() [0x2ce6875] (??:0)
[4] /media/me/Storage/Godot/Godot_v4.0-beta6_mono_linux_x86_64/Godot_v4.0-beta6_mono_linux.x86_64() [0x2cc00ad] (??:0)
[5] /media/me/Storage/Godot/Godot_v4.0-beta6_mono_linux_x86_64/Godot_v4.0-beta6_mono_linux.x86_64() [0x432ae04] (??:0)
[6] /media/me/Storage/Godot/Godot_v4.0-beta6_mono_linux_x86_64/Godot_v4.0-beta6_mono_linux.x86_64() [0x2b2fdef] (??:0)
[7] /media/me/Storage/Godot/Godot_v4.0-beta6_mono_linux_x86_64/Godot_v4.0-beta6_mono_linux.x86_64() [0x2b31af8] (??:0)
[8] /media/me/Storage/Godot/Godot_v4.0-beta6_mono_linux_x86_64/Godot_v4.0-beta6_mono_linux.x86_64() [0x2ac4091] (??:0)
[9] /media/me/Storage/Godot/Godot_v4.0-beta6_mono_linux_x86_64/Godot_v4.0-beta6_mono_linux.x86_64() [0x432ae04] (??:0)
[10] /media/me/Storage/Godot/Godot_v4.0-beta6_mono_linux_x86_64/Godot_v4.0-beta6_mono_linux.x86_64() [0x2b1967c] (??:0)
[11] /media/me/Storage/Godot/Godot_v4.0-beta6_mono_linux_x86_64/Godot_v4.0-beta6_mono_linux.x86_64() [0x4910385] (??:0)
[12] /media/me/Storage/Godot/Godot_v4.0-beta6_mono_linux_x86_64/Godot_v4.0-beta6_mono_linux.x86_64() [0x43c3307] (??:0)
[13] /media/me/Storage/Godot/Godot_v4.0-beta6_mono_linux_x86_64/Godot_v4.0-beta6_mono_linux.x86_64() [0x2a27b95] (??:0)
[14] /media/me/Storage/Godot/Godot_v4.0-beta6_mono_linux_x86_64/Godot_v4.0-beta6_mono_linux.x86_64() [0xe66d55] (??:0)
[15] /media/me/Storage/Godot/Godot_v4.0-beta6_mono_linux_x86_64/Godot_v4.0-beta6_mono_linux.x86_64() [0xda7193] (??:0)
[16] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf3) [0x7f89c980a083] (??:0)
[17] /media/me/Storage/Godot/Godot_v4.0-beta6_mono_linux_x86_64/Godot_v4.0-beta6_mono_linux.x86_64() [0xdc8bfe] (??:0)
-- END OF BACKTRACE --
================================================================

Steps to reproduce

Load this branch of my project https://gitlab.com/gametheatre/poriferareader/Porifera/-/tree/ThemeSwitch In the top bar, the rightmost element should be an options button Select anything other than default in that menu Observe crash

Minimal reproduction project

https://gitlab.com/gametheatre/poriferareader/Porifera/-/tree/ThemeSwitch Please note this is not minimal, I tried making a minimal reproduction and it did not crash. There is something about the more complex UI of my project which I have not identified.

[Dunkhan]

It just occurred to me that discussion in https://github.com/godotengine/godot/issues/68814 implied there might be an issue using themes from older versions of Godot. It might be sensible to let this bug mature until that one is worked out. I don't know what the situation is with supporting older themes.

[Dunkhan]

Alright old themes are not supported at all, these are ancient. I will report a new issue if and when I get around to building my own themes if there are still issues.

[akien-mga]

If you can make a reproduction project with just one such old theme that triggers a crash, it would still be good for us to fix this. It's fine that we don't support old themes but we shouldn't crash on them.

[Dunkhan]

I created a branch with only one theme to switch to, switching to it at runtime causes the crash: https://gitlab.com/gametheatre/poriferareader/Porifera/-/commits/godot_bug_69209

Notes:

I have no idea how old this theme is or what version it was made with, I found it on the asset store.

I have been getting an identical crash (same backtrace) in my project when the UI is updated (specifically child controls created and destroyed) from a UI signal. I was able to reduce the frequency of crashes by ensuring that all calls that caused updates are routed through the _Process function of a control to make sure they were on the main thread, and also hiding the parent control before each update and showing it again after the update was complete. Neither of these methods prevented all crashes, but I believe I am dealing with multiple separate bugs so it is hard to track down. I applied both of these methods to the code where I change themes (Main.cs line 205 OnThemeChanged()) and was able to load some of the themes afterward. The one left in the repro branch linked above is one of the ones that still crashed even after these measures.

[YuriSizov]

Do you still experience this issue in RC2? Was the theme resource converted before using it? If you create a simple project and try to apply this theme in a similar manner, does it still crash?

I have been getting an identical crash (same backtrace)

How do you know it's the same backtrace? Your backtrace is obfuscated. If you can compile Godot with debug symbols, please do so we can have a readable backtrace.

[Dunkhan]

I will test that for you in the next couple of days and let you know. How do I convert the theme resource?

[YuriSizov]

Well, you open a 3.x project and run a project convertor on it. It converts everything in the project that it can. I don't think we have a way to convert individual resources.

[Dunkhan]

I can confirm that the issue still persists, and have updated the project branch in the reproduction steps to RC2. The themes here are very old and might be outside the scope of what we want to support. I found them online.

I have not been able to successfully build the engine/editor with export templates to this point and therefore can not easily test with debug symbols. I might attempt this again in the next while if I have time.

[YuriSizov]

So when I follow the provided steps, I do get a crash, and it crashes in TextEdit trying to do this:

int TextEdit::get_visible_line_count() const {
	return _get_control_height() / get_line_height();
}

The error here is that we get division by zero because get_line_height() returns 0, probably because that older theme has some no longer valid fonts. We could probably patch this with ad-hoc checks here and there, though I'm not sure it would be worth it. Another option can be to have some minimal default value in this case, but I don't know what value could be reasonable to have in this case.

cc @bruvzg

[Dunkhan]

I am sorry, in an earlier beta it was crashing on every theme. This time when setting up the version I tested and a lot of themes were fixed, but a couple still broke. I should have made sure it wasn't my code first.

Given that, and the definite improvement in loading old themes, I am satisfied with the fix. Feel free to close this if you agree.

[YuriSizov]

This is not your code, this is engine code.

[jinyangcruise]

UPDATE: I finally figured out I should wait for some time to call theses methods, so I add await get_tree().create_timer(1.0).timeout and results seem correct:

Sorry for bothering.

-----------------------↓↓↓ origin comment-------------------------

@YuriSizov

might find a related bug of get_visible_line_count and get_last_full_visible_line

in my case, the line 116: get_last_full_visible_line should return 4 (the index of 56777923fdfjhlksdfj)

the line 122: get_visible_line_count should return 6 as line 123: get_visible_line_count_in_range returned

System info:
Godot Engine v4.0.stable.official.92bee43ad - https://godotengine.org
Vulkan API 1.3.205 - Forward+ - Using Vulkan Device #0: NVIDIA - NVIDIA GeForce RTX 3060 Ti

btw, I set the theme_override_styles/normal for my TextEdit:

[Rindbee]

https://github.com/godotengine/godot/blob/9f12e7b52d944281a39b7d3a33de6700c76cc23a/scene/gui/text_edit.cpp#L3478-L3480

text.get_line_height() depends on font_size, so it is mainly caused by using a negative line_spacing.