Fix LightmapGI not working with MultiMeshes

[tdaven]

Resolves https://github.com/godotengine/godot/issues/56027

This commit adds support for calling get_meshes on the multimesh_instance_3d. The order of the transform and mesh differs from get_bake_meshes from gridmap so swap the other so they match. This appears to fix the issue.

This fixes uses the attempt from https://github.com/godotengine/godot/issues/56027#issuecomment-2192837710 but fixes the ordering in the return so the bake works correctly.

[tdaven]

An easy alternative way to fix this would be to add a get_bake_mesh that returns the result in the correct order. I'm not sure what is best. It would duplicate a lot of get_meshes, which was the primary reason I just added an arg with a default value that preserves the old behavior but allows swapping for this use.

[tdaven]

It wasn't clear to me if we want to be doing more of the work related to UV2 that is done for normal meshes or not.

[tdaven]

There is still something wrong. The multi mesh seems to be getting dynamic shadows where the mesh instances are not. Will investigate further.

[clayjohn]

There is still something wrong. The multi mesh seems to be getting dynamic shadows where the mesh instances are not. Will investigate further.

There is code to register the Lightmap with a MeshInstance when the Lightmap is loaded. That is probably missing MultiMeshInstances. I would start by looking here and making sure the lightmap is registered properly: https://github.com/godotengine/godot/blob/5ccbf6e4c794a4e47456edd9434b75fcd6096a2f/scene/3d/lightmap_gi.cpp#L1386

[tdaven]

There is still something wrong. The multi mesh seems to be getting dynamic shadows where the mesh instances are not. Will investigate further.

There is code to register the Lightmap with a MeshInstance when the Lightmap is loaded. That is probably missing MultiMeshInstances. I would start by looking here and making sure the lightmap is registered properly:

https://github.com/godotengine/godot/blob/5ccbf6e4c794a4e47456edd9434b75fcd6096a2f/scene/3d/lightmap_gi.cpp#L1386

Yeah, doing this has stopped the dynamic shadows but still not working entirely. I think is a problem where its not using the lightmap possibly because something is missing to tell it to use UV2. Still investigating what is going wrong. I tried looking at gridmap but it seems entirely broken for lightmap_gi as well for similar reasons.

[tdaven]

This message was pasted to the rocket chat thread. Pasting here in-case its helpful for others.

I did a bit of investigation with gridmaps and I don't think this can be implemented like gridmaps. Gridmaps do have some uses of multimesh in the code, but it hardly used at all, especially for lightmaps. Everytime i've followed how grid map does it, you just get duplicate geometry. This is because gridmap spawns meshs when you bake the meshes.

I have a branch which does not use multimesh at all here (gridmap-no-multimesh). This branch won't display anything when editted, until you bake lightmaps. When it makes the baked meshes, it spawns new meshes. You can see the place this mostly happens here(gridmap mesh instancing). It also does some of this elsewhere to update. Its unclear to me where the multimesh stops drawing but it does. This was further confirmed by instrumenting the code and using the debugger. In the state mentioned above where the grid doesn't have baked meshes(and it doesn't show things properly), the multimesh is used and you can see it being drawed(assuming you're not on my no multimesh branch), but as soon as you bake meshes, it stops and instead you are only drawing mesh(s) made during baking linked above.

Here is an image from my no multi-mesh branch showing that it still renders and with working lightmaps:

[tdaven]

This my latest push I have basic support implemented for lightmapgi and multimesh. Its not complete though and there are number of things I plan to revisit. But it does work across all three renderers and produces identical results.

Small test scene. One side is made with individual mesh instances, the other is multi-mesh. They look identical as desired. Multimesh is on the left in case you are wondering.

[tdaven]

I think this is more or less ready for more wide spread review. Lightmap support for mulit-meshes is implemented for compatibility, mobile and forward+.

In order to implement this support, I've added a way to have per instance lightmap uv scale and slice index. For the most part, this has just been appended to the existing instances buffer. This isn't the only way to implement it. At one point I had mobile/forward+ using their own buffer for only this data. Its mostly tradeoff between some complexity in dealing with one buffer vs the complexity of adding a new buffer to a uniform set.

There are likely a few debug prints remaining I haven't removed yet. The compatibility support has to use another vertex attribute binding. I just used 2 which was documented as unused in the shader. This was helpful since it kept the max under 16 which is the limit for some systems (I think webgl2 and llvmpipe for example).

One question was can we implement it like gridmap does. I don't think so. Gridmap seems to just spawn normal mesh instances which defeats the benefit multi-mesh(but where there is already batching maybe its not needed much). Presumably, once this merges, gridmap can be fixed to just use multi-mesh more thoroughly and not have to implement its own lightmap support (but I've not investigated what all that would take yet).

[tdaven]

@Calinou, I think I have fixed the issue you hit plus a few others as well (motion vectors where not being handle properly after I had cleaned up code).

I had thought those dark cubes were a problem but I believe they just have basically a negative scale so they are inside out. Non-lightmap lighting is also dark on them.

You'll want to re-import the the MRP project though. The main defect was a buffer copy which still had an offset (from one of the way I tried implement this) which resulted in no the entire buffer being copied which may have resulted in the project not saving the transforms correctly.

I also added menu items to view uv1/uv2 plus unwrap for lightmap since it was frustrating not having them. The code is mostly copy pasted from mesh_instance_3d. Not idea but it definitely improves the work flow. This bit could always be strip out to its own PR or maybe some common class both could use.

[michaelharmonart]

super excited for this! awesome work

[Repiteo]

Needs rebase.

[michaelharmonart]

@Repiteo are there any specific things you're hoping to merge before this makes it in/any foreseeable conflicts or blockers for merging this?

I know it's not good form to ask to expedite changes, but I'm really hoping this makes it into mainline Godot soon, it looks like @tdaven has done some really great work, and it'll be a huge improvement for my project.

[tdaven]

I keep occasionally rebasing or can do as needed if someone pings me but after a few months of waiting I kinda stopped regular updates since it just wasted build resources waiting for people to review.

[tdaven]

I'll try that out and rebase tonight if I get a chance. Thanks for reporting the issue.

[tdaven]

Alright, hopefully both issues are now resolved and rebased on master again.

[dog-on-moon]

Did a little more testing!

• baking a scene with no mesh UV2s, but a MultiMeshInstance3D, will freeze the lightmap baking on "Determining optimal atlas size" -- basically, it seems that the check to popup the "No meshses with lightmapping support to bake." window is ignored whenever a multimesh is involved (even when its mesh has no UV2). this should be fixed

• very unpredictably, one of my lightmaps seem to hard crash on load when there are multiple multimeshes. I was able to capture a traceback of one of these:

unfortunately, I could not capture an MRP -- attempting to port the bugged scene over to a separate project led to the multimesh buffer performing a beautiful waltz, incurring a grand frustration within me:

https://github.com/user-attachments/assets/b7748979-c7d0-423a-a38c-23bd9d756dce

I think this crash is caused by some memory issues with multimeshes (especially since I was also having other tracebacks caused by an AnimationTree in the scene attempting to allocate memory). i cannot find any consistency in this behavior, either

I would take a closer look over all of the memory allocations in this PR. if they all seem fine, then I think this memory issue is unrelated -- the PR is good, and the lightmap multimeshes are stable from my testing (whenever the multimeshes themselves are behaving nicely)

[tdaven]

baking a scene with no mesh UV2s, but a MultiMeshInstance3D, will freeze the lightmap baking on "Determining optimal atlas size" -- basically, it seems that the check to popup the "No meshses with lightmapping support to bake." window is ignored whenever a multimesh is involved (even when its mesh has no UV2). this should be fixed

I've pushed a fix for this issue. Was definitely missing something there.

very unpredictably, one of my lightmaps seem to hard crash on load when there are multiple multimeshes. I was able to capture a traceback of one of these.

Not sure about this one. Its not crashing in anything related to lightmaps but insteads the gizmos. Maybe I can try an asan build and see if anything happens but I've not been able to reproduce this one yet with any of my test projects. I modified on to have multiple multi meshes but haven't gotten a crash yet.

[dog-on-moon]

Good news, it seems that my memory issues with lightmaps are related to #75485 instead (a persistent memory corruption issue with lightmaps). Calling multimesh.buffer = PackedFloat32Array() after modifying instance count appears to be a stable workaround, as mentioned in that issue

this continues to work great after the fixes