MultiMesh: setting visible_instance_count is ignored after the first frame

[vfig]

Godot version

v4.0.alpha11.official [afdae67]

System information

Windows 10; NVidia GTX 1060; Vulkan.

Issue description

I am trying to dynamically change how many instances of my multimesh get drawn, by setting visible_instance_count each frame, according to what my game needs. But this only works on the first frame; on subsequent frames, visible_instance_count is updated (as reading the property back confirms), but the number of instances drawn remains the same as on the first frame, neither more nor less.

Steps to reproduce

1. Run the MRP.
2. Wait two seconds for the visible_instance_count to start being incremented each frame (and printed to the console).
3. Note that the number of visible cubes does not change accordingly.

Minimal reproduction project

bug_multimesh_visible_instances.zip

[rburing]

Maybe something should happen here:

godot/servers/rendering/renderer_scene_cull.h

Lines 494 to 497 in 8ec6c07

	case Dependency::DEPENDENCY_CHANGED_MULTIMESH_VISIBLE_INSTANCES:
	case Dependency::DEPENDENCY_CHANGED_SKELETON_BONES: {
	//ignored
	} break;

or maybe somewhere else.

At one point (back in March) I tried doing

case Dependency::DEPENDENCY_CHANGED_MULTIMESH_VISIBLE_INSTANCES: {
    singleton->_instance_queue_update(instance, false, true);
} break;

and it seemed to fix the issue, but @reduz said that's probably too costly in general, and now I'm seeing that it doesn't even fully fix this issue. In the minimal reproduction project the number of actually visible instances (out of 10000) gets stuck at a maximum of 512 if visible_instance_count <= 512 on the first frame, gets stuck at a maximum of 1024 if 513 <= visible_instance_count <= 1024 on the first frame, etc.

Edit: Combining the above with changing the following line in MeshStorage::multimesh_set_visible_instances

godot/servers/rendering/renderer_rd/storage_rd/mesh_storage.cpp

Line 1644 in 95ddc8c

_multimesh_mark_all_dirty(multimesh, false, true);

to

_multimesh_mark_all_dirty(multimesh, true, true);

seems to fully fix the issue (probably again not efficiently, but I hope it can help to find the proper solution).

[vfig]

I think #45862 is probably due to the same cause. Maybe #56357 as well?

Further background: I encountered this problem because I expected that setting visible_instance_count would be essentially free to do every frame, only changing the number of instances passed in to the relevant draw calls. So I was surprised to find that there was some kind of caching behaviour going on with it. I take it that visible_instance_count is cached in order to rebuild the AABB when it changes? The discussion of the unexpected performance implications of automatic AABB rebuilding (instead of requiring a suitable custom AABB to be provided) over in godotengine/godot-proposals#957 seems relevant.

[Syntaxxor]

I'm able to replicate this in 4.0.beta1, though there's a (rather inefficient) workaround of just setting instance_count instead.

[JonqsGames]

Just faced this issue on the current master branch. The fix explained by @rburing is working well. I'm not sure to fully understand the reason why it could not be implemented as is. If there is a performance issue i think we should do it anyway since this feature is avaible with other renderer (gles3 and mobile handles the change on the fly from what i see in the code) and open a new ticket for further optimization.

[phire]

Also just encountered this on current master.

I tested and it's not just a vulkan bug, it's appears to be a regression that effects the gl_compatibility backend too. I suspect it dates back to #44838 when the dependency changed notifications were split up.

@rburing's fix appears to be the correct approach. Discussions about performance are red herrings, the bug needs to be fixed.
Though, it could be optimised slightly by modifying _multimesh_mark_all_dirty to take a range so that only the extra elements between the old and new counts are marked as dirty.