Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vulkan: LightmapGI does not take MultiMeshes into account for baking #56027

Open
Tracked by #56033
Calinou opened this issue Dec 17, 2021 · 3 comments · May be fixed by #97755
Open
Tracked by #56033

Vulkan: LightmapGI does not take MultiMeshes into account for baking #56027

Calinou opened this issue Dec 17, 2021 · 3 comments · May be fixed by #97755

Comments

@Calinou
Copy link
Member

Calinou commented Dec 17, 2021

Related to #56024 which is for VoxelGI.

Godot version

4.0.dev (1cbf394)

System information

Fedora 34, GeForce GTX 1080 (NVIDIA 470.74)

Issue description

Similar to #48909, but with the new lightmap GI implementation in master.

LightmapGI does not take MultiMeshes into account for baking. On the left, you can clearly notice the indirect lighting, but it's completely missing on the right:

Left: Individual MeshInstances, right: MultiMeshInstance:

2021-12-17_18 09 28

Steps to reproduce

Note: An imported 3D scene with UV2 generated must be used. You cannot use primitive meshes for testing this, as they do not contain a UV2 layer and it can't be generated using the MeshInstance tools. The minimal reproduction project includes a ready-to-use setup for testing this.

  • Import a 3D scene with Static Lightmaps bake mode. Click Advanced in the import dock, go to the Meshes tab and enable Save to File. Specify a path to a .tres file to save.
  • Add a MeshInstance3D node with the mesh resource loaded to act as a floor. Scale it so it can fit several objects on top. Set the global illumination mode to Baked in the inspector.
  • Add a MeshInstance3D node with the mesh resource loaded to act as a source mesh for the MultiMesh. Set the global illumination mode to Baked in the inspector.
  • Add a MultiMeshInstance3D node. Use the menu at the top of the 3D editor viewport to populate the MultiMeshInstance with BoxMeshes.
  • Add a LightmapGI, select it and click Bake at the top of the 3D editor viewport.

Minimal reproduction project

test_lightmapgi_multimesh.zip

Old MRPs

test_lightmapgi_multimesh.zip

With .godot/ included to workaround importing issue: test_lightmapgi_multimesh_1.zip

@Calinou
Copy link
Member Author

Calinou commented Jun 25, 2024

I can still reproduce this as of 4.3.beta2. I updated the MRP in OP.

@clayjohn
Copy link
Member

Probably needs the same solution as implemented in #81616

@Calinou
Copy link
Member Author

Calinou commented Jun 27, 2024

Probably needs the same solution as implemented in #81616

I was looking around and noticed that LightmapGI relies on get_bake_meshes() and not get_meshes(). This is done to support GridMap, and I noticed MultiMeshInstance has no equivalent (it only has get_meshes()).

I've tried replacing

Array bmeshes = p_at_node->call("get_bake_meshes");
with the following:

Array bmeshes;
MultiMeshInstance3D *multi_mesh = Object::cast_to<MultiMeshInstance3D>(p_at_node);
if (multi_mesh) {
	bmeshes = multi_mesh->get_meshes();
} else {
	bmeshes = p_at_node->call("get_bake_meshes");
}

However, this doesn't result in any visual difference after baking, even though no errors are printed.

Do we need to reimplement get_bake_meshes() for MultiMeshInstance3D?

cc @bitsawer

@tdaven tdaven linked a pull request Oct 3, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Up for grabs
Development

Successfully merging a pull request may close this issue.

2 participants