Support custom AABBs within MultiMesh resources

[puchik]

• Supporting custom AABBs on MultiMesh resources themselves allows us to prevent costly runtime AABB recalculations.
• May also help improve CPUParticle performance.

Fixes #79573

[clayjohn]

Looks great! I think this also needs to be exposed to CPUParticles so that they can benefit from the change as well.

GPUParticles3D have a visibility_aabb property that maps onto particles_custom_aabb. I think we can implement visibility aabb for CPUParticles as well so the API matches (we could also update the "convert to CPUParticles" code to keep the custom_aabb as well

[mohsenph69]

That is great, I can not believe you already did something to solve this problem
So fast
My grass system depend on this, As it change multi-mesh buffer in each grass update

[puchik]

Thanks for the starting point! I added the visibility_aabb property to CPUParticles3D which just passes it forward to the MultiMesh. The converter is also updated and I added the same AABB generator. Seems complete.

[puchik]

Not sure why the godot-cpp test is failing with an insufficient storage error. Is it a problem with the workflow? Does it need to try running again?

[akien-mga]

Yes, you need to rebase to get workflow changes that fixed that.

[puchik]

Okay thanks, just rebased and pushed 👍

[clayjohn]

Just one final little change. We need to ensure that we aren't notifying dependencies that the AABB has changed when a custom AABB is being used:

diff --git a/drivers/gles3/storage/mesh_storage.cpp b/drivers/gles3/storage/mesh_storage.cpp
index a276b73c44..9ed1b29225 100644
--- a/drivers/gles3/storage/mesh_storage.cpp
+++ b/drivers/gles3/storage/mesh_storage.cpp
@@ -1961,9 +1961,10 @@ void MeshStorage::multimesh_set_buffer(RID p_multimesh, const Vector<float> &p_b
        } else if (multimesh->mesh.is_valid()) {
                //if we have a mesh set, we need to re-generate the AABB from the new data
                const float *data = p_buffer.ptr();
-
-               _multimesh_re_create_aabb(multimesh, data, multimesh->instances);
-               multimesh->dependency.changed_notify(Dependency::DEPENDENCY_CHANGED_AABB);
+               if (multimesh->custom_aabb != AABB()) {
+                       _multimesh_re_create_aabb(multimesh, data, multimesh->instances);
+                       multimesh->dependency.changed_notify(Dependency::DEPENDENCY_CHANGED_AABB);
+               }
        }
 }
 
@@ -2110,9 +2111,11 @@ void MeshStorage::_update_dirty_multimeshes() {
                        }
 
                        if (multimesh->aabb_dirty && multimesh->mesh.is_valid()) {
-                               _multimesh_re_create_aabb(multimesh, data, visible_instances);
                                multimesh->aabb_dirty = false;
-                               multimesh->dependency.changed_notify(Dependency::DEPENDENCY_CHANGED_AABB);
+                               if (multimesh->custom_aabb != AABB()) {
+                                       _multimesh_re_create_aabb(multimesh, data, visible_instances);
+                                       multimesh->dependency.changed_notify(Dependency::DEPENDENCY_CHANGED_AABB);
+                               }
                        }
                }
 
diff --git a/servers/rendering/renderer_rd/storage_rd/mesh_storage.cpp b/servers/rendering/renderer_rd/storage_rd/mesh_storage.cpp
index 0451a66066..2f348838b0 100644
--- a/servers/rendering/renderer_rd/storage_rd/mesh_storage.cpp
+++ b/servers/rendering/renderer_rd/storage_rd/mesh_storage.cpp
@@ -1962,9 +1962,10 @@ void MeshStorage::multimesh_set_buffer(RID p_multimesh, const Vector<float> &p_b
        } else if (multimesh->mesh.is_valid()) {
                //if we have a mesh set, we need to re-generate the AABB from the new data
                const float *data = p_buffer.ptr();
-
-               _multimesh_re_create_aabb(multimesh, data, multimesh->instances);
-               multimesh->dependency.changed_notify(Dependency::DEPENDENCY_CHANGED_AABB);
+               if (multimesh->custom_aabb != AABB()) {
+                       _multimesh_re_create_aabb(multimesh, data, multimesh->instances);
+                       multimesh->dependency.changed_notify(Dependency::DEPENDENCY_CHANGED_AABB);
+               }
        }
 }
 
@@ -2083,10 +2084,11 @@ void MeshStorage::_update_dirty_multimeshes() {
}
 
                        if (multimesh->aabb_dirty) {
-                               //aabb is dirty..
-                               _multimesh_re_create_aabb(multimesh, data, visible_instances);
                                multimesh->aabb_dirty = false;
-                               multimesh->dependency.changed_notify(Dependency::DEPENDENCY_CHANGED_AABB);
+                               if (multimesh->custom_aabb != AABB()) {
+                                       _multimesh_re_create_aabb(multimesh, data, visible_instances);
+                                       multimesh->dependency.changed_notify(Dependency::DEPENDENCY_CHANGED_AABB);
+                               }
                        }
                }

This saves us having to do a bunch of updates in the RenderingServer

[puchik]

Updated!

[clayjohn]

Looks great! Thanks for responding quickly to my feedback

[akien-mga]

Thanks!

[Bimbam360]

Assume I should comment here, I just tried applying this in 4.3dev4 and get the following odd behaviour:

Base and when set to (-1000, -1000, -1000, 2000, 2000, 2000) manually in inspector

When set in GDScript prior to (or after) adding to the scene with MMInst.multimesh.custom_aabb = AABB(Vector3(-1000, -1000, -1000), Vector3(2000, 2000, 2000))

[clayjohn]

Assume I should comment here, I just tried applying this in 4.3dev4 and get the following odd behaviour:

Base and when set to (-1000, -1000, -1000, 2000, 2000, 2000) manually in inspector

When set in GDScript prior to (or after) adding to the scene with MMInst.multimesh.custom_aabb = AABB(Vector3(-1000, -1000, -1000), Vector3(2000, 2000, 2000))

Could you make a new issue with an MRP? That looks super strange and I can't figure out what the issue is just from looking at it. The fact that the cubes are cut in half doesn't make any sense haha

[Bimbam360]

Could you make a new issue with an MRP? That looks super strange and I can't figure out what the issue is just from looking at it. The fact that the cubes are cut in half doesn't make any sense haha

Done, see #89028

Also note this seems to be specific to mobile renderer with an imported mesh (which has me worried I've somehow made a borked cube and it's my fault lol).