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

Expose light storage to render data #100710

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

BadMachine
Copy link

@BadMachine BadMachine commented Dec 21, 2024

This pull request adds functionality that allows access to the LightStorage object via the RenderData object. This enhances the ability to work with lighting data during rendering, providing new methods to retrieve light buffers.

  1. omni_light_buffer – a buffer for processing omni-lights (lights emitting in all directions).
  2. directional_light_buffer – a buffer for processing directional lights (e.g., sunlight), where light is emitted in one direction.

Now, through the RenderData object, you can access the LightStorage object, and then use methods to extract the necessary light buffers for further processing in shaders or other parts of rendering.

Example usage:

func _render_callback(p_effect_callback_type, p_render_data: RenderData):
    var light_storage: LightStorage = p_render_data.get_light_storage()

    var directional_light_buffer: RID = light_storage.get_directional_light_buffer()
    var omni_light_buffer: RID = light_storage.get_omni_light_buffer()

This enhancement improves the flexibility of working with lighting data, allowing manipulation of light source data in compute shaders, which opens up additional opportunities for optimizing and processing light at a low level within compute shaders.

get_direcitonal_light_buffer and get_omni_light_buffer accessible via gdscript
[b]Note:[/b] This is an internal rendering server object, do not instantiate this from script.
</brief_description>
<description>
[b]Note:[/b] This is an internal rendering server object only exposed for GDExtension plugins.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also add a remark here that instances of this class (and any of its subclasses), should only ever be interacted with from the rendering thread.

GDVIRTUAL_BIND(_get_environment)
GDVIRTUAL_BIND(_get_camera_attributes)
}

RendererLightStorage *RenderDataExtension::get_light_storage() const {
RendererLightStorage *ret = nullptr;
Copy link
Contributor

@BastiaanOlij BastiaanOlij Dec 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So light storage, as with the other storage classes, are part of the core rendering system. You can just return this through https://github.com/godotengine/godot/blob/master/servers/rendering/rendering_server_globals.cpp.
It can then be cast to (RendererRD::)LightStorage in GDScript.

This also means this function does not need to be virtual, and we don't need the overridden function in RenderDataRD.

@@ -48,6 +48,11 @@ class RenderDataRD;
namespace RendererRD {

class LightStorage : public RendererLightStorage {
GDCLASS(LightStorage, RendererLightStorage);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for others, this is the only question mark in this solution.

The classes in the rendering engine are namespaced (RendererRD:: and GLES3::) and thus do not have unique names. Now that we're exposing them through ClassDB we have a problem if we want to expose both the compatibility and render device storage classes because ClassDB doesn't support namespacing (yet).

It is likely that we'll only ever expose the rendering device storage classes. Otherwise we may need to rename the classes after all or talk to the script team about what it will take to add namespace support to ClassDB.

Copy link
Contributor

@BastiaanOlij BastiaanOlij left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, this is indeed the approach that I envisioned. This will also pave the way to turn the base storage classes (like RendererLightStorage into proper virtual classes that can be implemented in GDExtension, which is the first step to building an alternative renderer as a plugin.

We need to add at least support for TextureStorage to this as well to give access to the Decal/Projector atlas. But in the long run we should expose all storage classes in this manner.

There will be other methods, especially on MeshStorage that will enable more advanced compositor effects or even be useful for compute shaders.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants