godotengine / godot

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

EditorNode3DGizmoPlugin does not always redraw #71979

Open winston-yallow opened 1 year ago

winston-yallow commented 1 year ago

Godot version

v4.0.beta14.official [28a24639c]

System information

Windows 10 (10.0.19045 Build 19045) | Forward+ | Vulkan API 1.3.206 | AMD Radeon RX 6800 XT

Issue description

It seems like EditorNode3DGizmoPlugin._redraw(gizmo) isn't always triggered when dragging a handle. There is noticable stutter when dragging a handle. When moving the mouse too fast then _redraw stops completely until the mouse hovers the handle again. The value is continuously updated, so everything except the drawing seems to be working.

https://user-images.githubusercontent.com/44872771/214330540-98df8830-0f83-4899-894d-d1927c792417.mp4

Steps to reproduce

Select the root node in the minimal reproduction. Then drag the handle and notice the stutter/freezes.

Minimal reproduction project

gizmo-bug.zip

winston-yallow commented 1 year ago

The redraw is also skipped when changing the value in the inspector.

TheDuckCow commented 1 year ago

+1 to say I'm still seeing this issue in Godot 4.0.2 with the above demo project, in my mind would be a blocking for any addons attempting to use gizmos until fixed.

edit: v4.0.2.stable.official on a M1 Mac (os 12.4), issue was present in both Forward+ and Compatibility mode.

MonaMayrhofer commented 1 year ago

Is there any reason why one shouldn't call self._redraw(gizmo) within the _set_handle function themselves?

arnemileswinter commented 1 year ago

Im in 4.1

Using gizmo.get_node_3d().update_gizmos() within the _set_handle function seems effective in this context. To keep the gizmo synchronized with inspector changes too, as pointed out by @winston-yallow, you can also achieve this by invoking update_gizmos() through a setter on the relevant property. only on tool scripts, however.

While @MonaMayrhofer's suggestion also worked, it appears less streamlined, as there is what looks to be a delay in clearing previous gizmo surfaces.

I have a working demo here for everyone else running into this!

donn-xx commented 1 year ago

4.1.1 on Linux Noticed this. Also, I had the script editor "floating" and the handles (the red circles) simply would not draw. Closing the script window immediately restored the handles.

Using gizmo.get_node_3d().update_gizmos() within the _set_handle <-- proper magic. Thanks!

voidexp commented 1 year ago

Adding to @arnemileswinter 's suggestion:

Undo/Redo In order for the gizmo to properly update on Undo/Redo actions, in the _commit_handle() method when constructing the actions, add a callback to edited node's update_gizmos() method via the <UndoRedo instance>.add_[un]do_method(node, 'update_gizmos').

Reacting to property changes made in inspector This one is a little trickier. In the end, I created a custom EditorNode3DGizmo class, an instance of which is returned by the overridden EditorNode3DGizmoPlugin._create_gizmo() method, and inside its _init() I subscribe to EditorInspector.property_edited signal, which in turn triggers the edited node's update_gizmos().

Both of these solutions are probably quite dirty hacks, which show the limitations of current editor plugins API. But the advantage is that they don't require the node class for which the gizmos are provided to know anything about the plugin itself and not have code specifically targeted at supported a plugin for itself.

ultrasuperpingu commented 7 months ago

@arnemileswinter's solution worked for me. I was calling _Redraw() from _SetHandle and it was working fine but indeed, I think it's better to do an UpdateGizmos(); instead.

The redraw is also skipped when changing the value in the inspector.

You can add an UpdateGizmos(); in the node class to maje it work. Personally, I call it only when we're in the editor:

#if TOOLS
UpdateGizmos();
#endif
Okxa commented 4 months ago

Another hack to update gizmos after editing the value in inspector. Similiarly to @voidexp solution, you can connect the EditorInspector.property_edited signal in _redraw():

var inspector = EditorInterface.get_inspector()

func _redraw(gizmo):
    # NOTE: other required redraw code omitted
    var node = gizmo.get_node_3d()

    if !inspector.property_edited.is_connected(node.update_gizmos):
        inspector.property_edited.connect(node.update_gizmos.unbind(1))

However, I am not sure if there are any major drawbacks regarding adding many connections. Maybe the signals could be disconnect in some way if it is an issue. :shrug: