Vulkan Mobile backend: LightmapGI baking does not take environment lighting into account

[Calinou]

Godot version

4.0.dev (2a9dd65)

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)}

[Lasuch69]

I can take a look into it. At a first glance the issue appears to be caused by copy_cubemap_to_panorama function, which is not working in Vulkan Mobile backend. Maybe the fix is to create a fallback to CPU.

[Calinou]

Don't we have a fragment version of that shader for use with reflection probes and omni shadows?

cc @BastiaanOlij

[Lasuch69]

I started working on it.

[clayjohn]

Personally, I would just remove the error check here:

godot/servers/rendering/renderer_rd/effects/copy_effects.cpp

Line 389 in 5945768

ERR_FAIL_COND_MSG(prefer_raster_effects, "Can't use the compute version of the copy_cubemap_to_panorama shader with the mobile renderer.");

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.

[Lasuch69]

godot/servers/rendering/renderer_rd/effects/copy_effects.cpp

Line 414 in 5945768

ERR_FAIL_COND(shader.is_null());

Doesn't work after removing error check.

Still works fine on Forward+.

[clayjohn]

godot/servers/rendering/renderer_rd/effects/copy_effects.cpp

Line 414 in 5945768

ERR_FAIL_COND(shader.is_null());

Doesn't work after removing error check.

Still works fine on Forward+.

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:

godot/servers/rendering/renderer_rd/effects/copy_effects.cpp

Line 50 in 5945768

if (prefer_raster_effects) {

[Lasuch69]

I've deleted the correct error check. I should've made it more clear what I meant. :(

[BastiaanOlij]

Don't we have a fragment version of that shader for use with reflection probes and omni shadows?

cc @BastiaanOlij

This deserves further discussion imho. The main rendering pipeline of the mobile renderer avoids using compute shaders because on most mobile phones compute is very limited, though I have often wondered if it was the right course of action to ignore them fully. In fact, I believe various systems still rely on compute such as particles (which is likely not a problem).

Lightmapping however is a whole other story. The phone is the destination, hence using the mobile renderer, the phone is hampered in performance, hence using lightmapping to minimize overhead, but the host machine on which development happens, very likely can happily run compute shaders to create the lightmaps. We're only running the IDE using the mobile renderer to ensure accurate representation of the output of the game (unless the IDE is running on mobile hardware, which is now possible).

There is thus an argument where we allow the compute variants of features in the IDE.

[Lasuch69]

I think that ordinary phone isn't a viable development platform especially for 3D, so I would prefer compute shader approach. Whatever path is least complex is the best in this case IMO. That said I can try to make a raster version.

[BastiaanOlij]

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_effects to 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.

[Lasuch69]

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.

[clayjohn]

My suggestion is to just use is_editor_hint() and just initialize compute shader in editor, while keeping the rest as is.

I have no problem using a compute shader for this. But if we do, we should not limit it to the editor. The function can be called at run time, even though it will be slow if used at runtime (on a phone without good compute support), it should still succeed.

[Lasuch69]

I have no problem using a compute shader for this. But if we do, we should not limit it to the editor. The function can be called at run time, even though it will be slow if used at runtime (on a phone without good compute support), it should still succeed.

Lightmaps can't be baked in exported project, so if there are no other use cases (currently) for it then there is no way to use it without running the editor anyway.

[clayjohn]

@Lasuch69 For better or worse it is exposed directly through the RenderingServer API. Which means that it might be called at run time.

[BastiaanOlij]

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]

I opened a draft PR.