godot: Vulkan Mobile backend: LightmapGI baking does not take environment lighting into account
Godot version
4.0.dev (2a9dd654b)
System information
Fedora 34, GeForce GTX 1080 (NVIDIA 470.74)
Issue description
Despite lightmap rendering not working correctly when using the Vulkan Mobile backend, LightmapGI baking does work. However, it ignores environment lighting from the sky when it’s enabled in the node properties.
Note: You need to switch back to the Vulkan Clustered backend and restart the editor to see the lightmap baked in the Vulkan Mobile backend in the editor/project.
Lightmap baked in Vulkan Clustered
Lightmap baked in Vulkan Mobile (then displayed in Vulkan Clustered)
Steps to reproduce
- Import a glTF scene. Set the light bake mode to Static Lightmaps in the Import dock and click Reimport.
- Add a LightmapGI node.
- Bake lightmaps. Open the resulting EXR file in an image editor.
- Switch to Vulkan Mobile backend in the project settings (search for
back end).- Note: The MRP linked below already uses the Vulkan Mobile backend by default.
- Restart the editor and bake lightmaps again. Open the resulting EXR file in an image editor for comparison. Notice the environment lighting is missing.
Minimal reproduction project
test_vulkan_clustered_vs_mobile_lightmap_sampling.zip
With .godot/ included to workaround importing issue: https://0x0.st/orQD.zip (link expires in September 2022)
About this issue
- Original URL
- State: closed
- Created 3 years ago
- Comments: 16 (16 by maintainers)
Owh flagship phones can do 3D just fine, but it was interesting that I was told the other day that the vast majority of successful phone games are all 2D games (not just Godot, but just in general).
That all said, these decisions were made before we decided to use OpenGL (compatibility renderer) for lower end phones, so there is an argument for being less restrictive around compute for the mobile renderer.
I definitely think there is no problem for the code that creates the lightmap data to use compute. We could do something like change the init code in
copy_effectsto compile both raster and compute versions of all copy effects IF we’re on mobile and are in the editor. Then in code we can decide to only use the compute versions for features available in the IDE.Works after deleting 2 checks.
I would just add some checks and use the compute version as it is the simplest way to fix the issue. Considering that the particles are using compute shaders and are going to be used as often as the Lightmap in mobile projects I think that it shouldn’t really be a problem to not make a raster version.
My suggestion is to just use
is_editor_hint()and just initialize compute shader in editor, while keeping the rest as is.I think if it’s exposed like that, might as well make compute versions available. In the end it is up to us to make sure that the core of the rendering engine scarcely use the compute versions.
@Lasuch69 For better or worse it is exposed directly through the RenderingServer API. Which means that it might be called at run time.
That looks like the wrong error check. See the only I linked in my comment
Edit: At any rate, just removing the error check isn’t enough. Looks like you have to create a raster version anyway: https://github.com/godotengine/godot/blob/59457685c18e2d729eea50c751c11f049a7186f0/servers/rendering/renderer_rd/effects/copy_effects.cpp#L50
Personally, I would just remove the error check here: https://github.com/godotengine/godot/blob/59457685c18e2d729eea50c751c11f049a7186f0/servers/rendering/renderer_rd/effects/copy_effects.cpp#L389
There isn’t much point in creating a raster version of this function as its not something that should be called at run time anyway.