godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.14k stars 93 forks source link

Expose `Theme.freeze_change_propagation()` and `Theme.unfreeze_and_propagate_changes()` #10873

Open sockeye-d opened 1 week ago

sockeye-d commented 1 week ago

Describe the project you are working on

A text editor that supports multiple themes

Describe the problem or limitation you are having in your project

I have a function that changes the theme according to a .tet file, so it makes a lot of calls to Theme.set_* (over 120). Since each call to those set methods then calls _emit_theme_changed() (which propagates NOTIFICATION_THEME_CHANGED to all nodes), it results in basically doing 120x the work it should, and so the entire project freezes for 12 seconds.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Exposing these methods would let the Theme not notify all nodes after every change. Alternatively, the Controls can only update once on idle frames instead of immediately after every notification.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Instead of

theme.set_color(&"font_color", &"Tree", Color.RED)
# x100

it would be

theme.freeze_change_propagation()

theme.set_color(&"font_color", &"Tree", Color.RED)
# x100

theme.unfreeze_and_propagate_changes()

Alternatively, it should follow Control's begin/end_bulk_theme_override:

theme.begin_bulk_change()

theme.set_color(&"font_color", &"Tree", Color.RED)
# x100

theme.end_bulk_change()

If this enhancement will not be used often, can it be worked around with a few lines of script?

Technically yes, by unparenting all nodes, setting all the theme items, then parenting them back. This is less than ideal, especially since subwindows will be closed temporarily, things will disappear for two frames, etc.

Is there a reason why this should be core and not an add-on in the asset library?

This is not possible to do with an addon

dalexeev commented 1 week ago

Alternatively, you can duplicate a theme, apply the changes to the duplicate, and replace the original theme with the duplicate. An example:

@tool
extends EditorScript

func _run() -> void:
    var image23 := preload("res://23.png")
    var bc := get_editor_interface().get_base_control()
    var theme := bc.theme
    var new_theme := theme.duplicate()
    for icon_name in theme.get_theme_item_list(Theme.DATA_TYPE_ICON, &"EditorIcons"):
        if ClassDB.class_exists(icon_name) and ClassDB.is_parent_class(icon_name, &"Node"):
            var texture: ImageTexture = theme.get_icon(icon_name, &"EditorIcons")
            var image := texture.get_image().duplicate() as Image
            var new_texture := ImageTexture.new()
            image.crop(22, 16)
            if ClassDB.is_parent_class(icon_name, &"Node2D"):
                image.blend_rect(image23, Rect2i(0, 0, 6, 8), Vector2i(16, 0))
            elif ClassDB.is_parent_class(icon_name, &"Node3D"):
                image.blend_rect(image23, Rect2i(6, 0, 6, 8), Vector2i(16, 0))
            new_texture.set_image(image)
            new_theme.set_icon(icon_name, &"EditorIcons", new_texture)
    bc.theme = new_theme
    print("Done.")
KoBeWi commented 1 week ago

You can suppress the changed signal with set_block_signals().

sockeye-d commented 1 week ago

You can suppress the changed signal with set_block_signals().

That helped but not as much as I thought it would until I waited a single frame after blocking signals:

# my final solution
theme.set_block_signals(true)
await get_tree().process_frame

theme.set_color(&"font_color", &"Tree", Color.RED)
# x100

await get_tree().process_frame
theme.set_block_signals(false)

Not 100% sure why. Maybe something in the docs can mention that blocking signals can increase performance?