godotengine / godot

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

Vulkan: VoxelGI does not take custom shaders into account for emission if the mesh's GI mode is Static #64124

Open Calinou opened 2 years ago

Calinou commented 2 years ago

Godot version

4.0.alpha13

System information

Fedora 36, GeForce GTX 1080 (NVIDIA 515.57)

Issue description

VoxelGI does not take custom shaders into account for emission if the mesh's GI mode is Static. This may be a limitation, in which case this should be documented.

2022-08-08_21 44 41

For reference, it works with LightmapGI (dynamic objects are not included in the bake as expected):

image

Steps to reproduce

shader_type spatial;

void fragment() {
    EMISSION = vec3(0.0, 1.0, 0.6);
}

Minimal reproduction project

test_voxelgi_custom_shader_emission.zip

RandomCatDude commented 2 years ago

https://github.com/godotengine/godot/blob/9b7e16a6b8b80fe61881e8f4df28550e18050dd2/scene/3d/voxelizer.cpp#L325-L329

This is the code seemingly used by VoxelGI to bake static meshes.

The problem is pretty clear -- it's hardcoded to use the mesh's StandardMaterial3D if it has one, and then crudely sample its albedo and emission textures. This is why it has issues such as being unaffected by UV scaling options. And of course, this is why it completely fails on meshes using ORMMaterial3D or ShaderMaterial.

I believe the only effective solution would be to move voxelization to be done on GPU, which would enable using the actual material shader to bake the albedo and emission.

Calinou commented 2 years ago

And of course, this is why it completely fails on meshes using ORMMaterial3D

To support both StandardMaterial3D and ORMMaterial3D, it should be easy to use BaseMaterial3D instead of StandardMaterial3D in that code. Feel free to open a pull request for this :slightly_smiling_face:

Edit: Done in https://github.com/godotengine/godot/pull/64219.

RandomCatDude commented 2 years ago

And of course, this is why it completely fails on meshes using ORMMaterial3D

To support both StandardMaterial3D and ORMMaterial3D, it should be easy to use BaseMaterial3D instead of StandardMaterial3D in that code. Feel free to open a pull request for this slightly_smiling_face

Apologies, opening pull requests/contributing is out of the scope of what I'm confident in doing. I just know enough to have been able to pin down where this issue comes from, as I haven't seen anyone else bring it up. But yes, that sounds like a good idea to make it slightly more tolerable, until the aforementioned proper, but far more complex GPU-based solution is made.