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

Changing editor font or theme, cause stack overflow #91553

Closed
Chaosus opened this issue May 4, 2024 · 4 comments · Fixed by #91595
Closed

Changing editor font or theme, cause stack overflow #91553

Chaosus opened this issue May 4, 2024 · 4 comments · Fixed by #91595

Comments

@Chaosus
Copy link
Member

Chaosus commented May 4, 2024

Tested versions

4.3 dev (03e6fbb)

System information

Windows 11

Issue description

Changing Main Font or Code Font resulting a crash with the following call stack:

>	godot.windows.editor.dev.x86_64.exe!Variant::recursive_hash(int recursion_count) Line 2953	C++
 	godot.windows.editor.dev.x86_64.exe!Variant::hash() Line 2928	C++
 	godot.windows.editor.dev.x86_64.exe!VariantHasher::hash(const Variant & p_variant) Line 826	C++
 	godot.windows.editor.dev.x86_64.exe!HashMap<Vector2i,TextServerAdvanced::FontForSizeAdvanced *,VariantHasher,VariantComparator,DefaultTypedAllocator<HashMapElement<Vector2i,TextServerAdvanced::FontForSizeAdvanced *>>>::_hash(const Vector2i & p_key) Line 85	C++
 	godot.windows.editor.dev.x86_64.exe!HashMap<Vector2i,TextServerAdvanced::FontForSizeAdvanced *,VariantHasher,VariantComparator,DefaultTypedAllocator<HashMapElement<Vector2i,TextServerAdvanced::FontForSizeAdvanced *>>>::_lookup_pos(const Vector2i & p_key, unsigned int & r_pos) Line 106	C++
 	godot.windows.editor.dev.x86_64.exe!HashMap<Vector2i,TextServerAdvanced::FontForSizeAdvanced *,VariantHasher,VariantComparator,DefaultTypedAllocator<HashMapElement<Vector2i,TextServerAdvanced::FontForSizeAdvanced *>>>::has(const Vector2i & p_key) Line 311	C++
 	godot.windows.editor.dev.x86_64.exe!TextServerAdvanced::_ensure_cache_for_size(TextServerAdvanced::FontAdvanced * p_font_data, const Vector2i & p_size) Line 1352	C++
 	godot.windows.editor.dev.x86_64.exe!TextServerAdvanced::_font_get_descent(const RID & p_font_rid, __int64 p_size) Line 2645	C++
 	godot.windows.editor.dev.x86_64.exe!TextServerAdvanced::font_get_descent(const RID & arg1, __int64 arg2) Line 817	C++
 	godot.windows.editor.dev.x86_64.exe!Font::get_height(int p_font_size) Line 197	C++
 	godot.windows.editor.dev.x86_64.exe!EditorProperty::get_minimum_size() Line 72	C++
 	godot.windows.editor.dev.x86_64.exe!Control::_update_minimum_size_cache() Line 1652	C++
 	godot.windows.editor.dev.x86_64.exe!Control::get_combined_minimum_size() Line 1663	C++
 	godot.windows.editor.dev.x86_64.exe!Control::_size_changed() Line 1681	C++
 	godot.windows.editor.dev.x86_64.exe!Control::_notification(int p_notification) Line 3299	C++
 	godot.windows.editor.dev.x86_64.exe!Control::_notificationv(int p_notification, bool p_reversed) Line 48	C++
 	godot.windows.editor.dev.x86_64.exe!Container::_notificationv(int p_notification, bool p_reversed) Line 37	C++

Steps to reproduce

Change Code Font or Main Font in the editor settings.

Minimal reproduction project (MRP)

No need, requires latest 4.3 build of Godot

@KoBeWi
Copy link
Member

KoBeWi commented May 4, 2024

Better stack trace:

HashMap<Vector2i,TextServerAdvanced::FontForSizeAdvanced *,VariantHasher,VariantComparator,DefaultTypedAllocator<HashMapElement<Vector2i,TextServerAdvanced::FontForSizeAdvanced *>>>::_lookup_pos(const Vector2i & p_key, unsigned int & r_pos) Line 106 (c:\godot_source\core\templates\hash_map.h:106)
HashMap<Vector2i,TextServerAdvanced::FontForSizeAdvanced *,VariantHasher,VariantComparator,DefaultTypedAllocator<HashMapElement<Vector2i,TextServerAdvanced::FontForSizeAdvanced *>>>::has(const Vector2i & p_key) Line 311 (c:\godot_source\core\templates\hash_map.h:311)
TextServerAdvanced::_ensure_cache_for_size(TextServerAdvanced::FontAdvanced * p_font_data, const Vector2i & p_size) Line 1352 (c:\godot_source\modules\text_server_adv\text_server_adv.cpp:1352)
TextServerAdvanced::_font_get_descent(const RID & p_font_rid, __int64 p_size) Line 2645 (c:\godot_source\modules\text_server_adv\text_server_adv.cpp:2645)
TextServerAdvanced::font_get_descent(const RID & arg1, __int64 arg2) Line 817 (c:\godot_source\modules\text_server_adv\text_server_adv.h:817)
Font::get_height(int p_font_size) Line 197 (c:\godot_source\scene\resources\font.cpp:197)
EditorProperty::get_minimum_size() Line 72 (c:\godot_source\editor\editor_inspector.cpp:72)
Control::_update_minimum_size_cache() Line 1652 (c:\godot_source\scene\gui\control.cpp:1652)
Control::get_combined_minimum_size() Line 1663 (c:\godot_source\scene\gui\control.cpp:1663)
Control::_size_changed() Line 1681 (c:\godot_source\scene\gui\control.cpp:1681)
Control::_notification(int p_notification) Line 3299 (c:\godot_source\scene\gui\control.cpp:3299)
Control::_notificationv(int p_notification, bool p_reversed) Line 48 (c:\godot_source\scene\gui\control.h:48)
Container::_notificationv(int p_notification, bool p_reversed) Line 37 (c:\godot_source\scene\gui\container.h:37)
EditorProperty::_notificationv(int p_notification, bool p_reversed) Line 59 (c:\godot_source\editor\editor_inspector.h:59)
EditorPropertyResource::_notificationv(int p_notification, bool p_reversed) Line 695 (c:\godot_source\editor\editor_properties.h:695)
Object::notification(int p_notification, bool p_reversed) Line 907 (c:\godot_source\core\object\object.cpp:907)
Control::_notify_theme_override_changed() Line 2438 (c:\godot_source\scene\gui\control.cpp:2438)
Control::end_bulk_theme_override() Line 2974 (c:\godot_source\scene\gui\control.cpp:2974)
EditorProperty::_update_property_bg() Line 532 (c:\godot_source\editor\editor_inspector.cpp:532)
EditorPropertyResource::_notification(int p_what) Line 3428 (c:\godot_source\editor\editor_properties.cpp:3428)
EditorPropertyResource::_notificationv(int p_notification, bool p_reversed) Line 695 (c:\godot_source\editor\editor_properties.h:695)
Object::notification(int p_notification, bool p_reversed) Line 907 (c:\godot_source\core\object\object.cpp:907)
Control::_notify_theme_override_changed() Line 2438 (c:\godot_source\scene\gui\control.cpp:2438)
Control::end_bulk_theme_override() Line 2974 (c:\godot_source\scene\gui\control.cpp:2974)
EditorProperty::_update_property_bg() Line 532 (c:\godot_source\editor\editor_inspector.cpp:532)
EditorPropertyResource::_notification(int p_what) Line 3428 (c:\godot_source\editor\editor_properties.cpp:3428)
EditorPropertyResource::_notificationv(int p_notification, bool p_reversed) Line 695 (c:\godot_source\editor\editor_properties.h:695)
Object::notification(int p_notification, bool p_reversed) Line 907 (c:\godot_source\core\object\object.cpp:907)
Control::_notify_theme_override_changed() Line 2438 (c:\godot_source\scene\gui\control.cpp:2438)
Control::end_bulk_theme_override() Line 2974 (c:\godot_source\scene\gui\control.cpp:2974)
...

It's a stack overflow.

@billuo
Copy link

billuo commented May 5, 2024

Just ran into the same crash. The call loop is:

EditorProperty::_update_property_bg(); // <-- begin
Control::end_bulk_theme_override();
Control::_notify_theme_override_changed();
Object::notification(NOTIFICATION_THEME_CHANGED);
EditorPropertyResource::_notification(NOTIFICATION_THEME_CHANGED);
EditorProperty::_update_property_bg(); // now we are back

A recent commit cba9606 could be related, which refactored many things including EditorPropertyResource::_notification():

 void EditorPropertyResource::_notification(int p_what) {
 	switch (p_what) {
 		case NOTIFICATION_THEME_CHANGED: {
-			if (!updating_theme) {
+			if (EditorThemeManager::is_generated_theme_outdated()) {
 				_update_property_bg();
 			}
 		} break;

@rsubtil
Copy link
Contributor

rsubtil commented May 5, 2024

Also occurs when switching themes or changing any theme property, so probably any configuration that triggers a theme reload.

@Chaosus Chaosus changed the title Changing editor font, cause editor crash Changing editor font or theme, cause stack overflow May 5, 2024
@Chaosus
Copy link
Member Author

Chaosus commented May 5, 2024

cc @ajreckof (indeed its caused by cba9606)

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

Successfully merging a pull request may close this issue.

4 participants