godotengine / godot

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

Performance troubles in editor with 3D gizmos. #94648

Open piiertho opened 3 months ago

piiertho commented 3 months ago

Tested versions

System information

Windows 11; Editor; 13th Gen Intel(R) Core(TM) i5-13600K 3.50 GHz; nvidia 4070

Issue description

I'm working on a 2.5D isometric map editor with my team.
This is done by using 2D nodes and a topological graph.
We also add 3D collision so we can manage physics in 3D: image image

We experience slow map opening and closing when it's big (we have an example with more than 10000 tiles), but only in editor mode. We have no troubles when running game.
Here are screenshots of a map example being slow on opening and closing in editor:
image Tiles in this maps have StaticBody3D with CollisionShape3D:
image

I ran a profiler (windows performance recorder) when closing the map to understand the reason of those slow opening and closing in editor.
It seems remove_child method is taking too much time:
image When zooming in this method it appears that destroying the EditorNode3DGizmo is taking a lot of time:
image

I find this a bit a shame as godot does not have any problem to instantiate and destroying more than 10k nodes (all works perfectly in game mode), but it has troubles with editor gizmos, which as far as I understand should be really simpler than complex nodes with physics.

Steps to reproduce

Create a scene with 10k nodes using cubic collision shapes, then open and close in editor.

Minimal reproduction project (MRP)

You can checkout dojobigmap.tscn from our module demo project (with engine compiled with this module): https://github.com/utopia-rise/IsometricMapModule/tree/master/demo/maps

aaronp64 commented 3 months ago

Started looking into this a bit. Haven't built with the linked module, but just a scene with a bunch of duplicated StaticBody3D/CollisionShape3Ds shows similar slowdown. Seems like the opening and closing might be two separate issues.

When opening, a lot of time is spent in TreeItem::create_child, adding nodes for the scene tree editor. Each added node is basically iterating through a linked list to find where to insert it, and since they're added to the end in this case, each one has to iterate through all the other nodes with the same parent. Could you run the same profiling you did on closing on opening as well, just to confirm the slowdown with the module/demo project is the same here?

Closing is mostly due to ~EditorNode3DGizmo removing itself from EditorNode3DGizmoPlugin::current_gizmos, which is a List, so similar issue of having to iterate through a linked list to find itself to remove. Changing this to a HashSet seems to fix the closing slowdown, along with some changes in Joint3DGizmoPlugin::incremental_update_gizmos to work with a HashSet. That function might have its own performance issues at some point, as it gets by index - whether using a List or HashSet, it'll have to iterate through to the right position.

piiertho commented 3 months ago

It seems the major part of time is spent in DynamicBVH::insert when opening in editor:
image

aaronp64 commented 3 months ago

Hmm, I'm not really familiar with the rendering/bvh code, so I probably won't be able to help much with that side of it. I did notice DynamicBVH::insert takes more time when all of the boxes have the same position/size - when they all have different positions, DynamicBVH::insert barely shows up for me when opening the scene. Might be something to do with how balanced the bvh tree is. Any chance all of the boxes (or maybe just the collision shape parts) start out in one place, and get moved to the correct location after loading?

The scene closing slowness at least seems more straightforward, and separate from the opening slowness, I can make a PR for that change.

piiertho commented 3 months ago

Hmm, I'm not really familiar with the rendering/bvh code, so I probably won't be able to help much with that side of it. I did notice DynamicBVH::insert takes more time when all of the boxes have the same position/size - when they all have different positions, DynamicBVH::insert barely shows up for me when opening the scene. Might be something to do with how balanced the bvh tree is. Any chance all of the boxes (or maybe just the collision shape parts) start out in one place, and get moved to the correct location after loading?

The scene closing slowness at least seems more straightforward, and separate from the opening slowness, I can make a PR for that change.

I don't think it all stars with same position. Here is how we add elements to a map: This is called on enter_tree of IsometricMap:

    const Vector<int>& id_vector {grid_3d.get_internal_array()};
    const Vector<uint32_t>& layers_vector {layers_grid_3d.get_internal_array()};
    for (int i = 0; i < id_vector.size(); ++i) {
        add_positionable_as_child(
                id_vector[i],
                grid_3d.get_position_3d_from_index(i),
                layers_vector[i]
        );
    }

This calls within the loop this code:

void IsometricMap::add_positionable_as_child(int positionable_id, const Vector3& p_position, uint32_t layer_id) {
    if (positionable_id == resource::PositionableSet::NONE_POSITIONABLE_ID) { return; }
    if (auto* positionable {
          Object::cast_to<IsometricPositionable>(positionable_set->get_positionable_scene_for_id(positionable_id)->instantiate())}) {
        positionable->set_local_position_3d(p_position);
        positionable->add_to_group(layers_groups[layer_id]);
        add_child(positionable);

Which calls this, in IsometricPositionable:

void IsometricPositionable::set_local_position_3d(Vector3 p_local) {
    local_position = p_local;
    if (self.is_valid() && is_dynamic) {
        IsometricServer::get_instance()->isometric_element_set_position(self, get_global_position_3d());
    }
    update_position();
    _rebind_collision_object_position();

#ifdef TOOLS_ENABLED
    _rebind_debug_mesh_instance();
#endif
}

This method first update the position of the IsometricPositionable, and then rebind the collision object and the mesh. So if I'm not wrong all meshes are at different positions when being added as child.