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

Add ability to include built-in include files #94193

Merged
merged 1 commit into from
Dec 5, 2024

Conversation

BastiaanOlij
Copy link
Contributor

@BastiaanOlij BastiaanOlij commented Jul 11, 2024

This PR is a prelude to functionality we need for custom shader templates, but also has broader use in compositor effects.

This adds extra code so our include files are compiled along side the full template shader files. We can then include those in source and register them which enables us to use them in user shaders.

As part of this PR only 3 include files are actually exposed as a larger reorganisation will be needed before custom shader templates can be a thing.
For compositor effects however these can be useful includes. Right now we only have access to the scene data uniform but we'll be exposing more in the near future.

Note, this PR in godot demo projects has an example of how to use this in compositor effects:
godotengine/godot-demo-projects#1086

Here you can see that we're including our scene data UBO by including the scene data include file:

#include "godot/scene_data_inc.glsl"

layout(set = 0, binding = 0, std140) uniform SceneDataBlock {
	SceneData data;
	SceneData prev_data;
}
scene_data_block;

(the SceneDataBlock is not part of the include file so you can use your own set and binding as this will be likely different from the built in values).

Then in the compositor effect code you simply assign the existing UBO:

var scene_data := p_render_data.get_render_scene_data()
var scene_data_buffers: RID = scene_data.get_uniform_buffer()

var uniform:= RDUniform.new()
uniform.uniform_type = RenderingDevice.UNIFORM_TYPE_UNIFORM_BUFFER
uniform.binding = 0
uniform.add_id(scene_data_buffers)
var uniform_set := UniformSetCacheRD.get_cache(shader, 0, [uniform])

var compute_list := rd.compute_list_begin()
rd.compute_list_bind_uniform_set(compute_list, uniform_set, 0)

Now you have access to all the global scene data information such as projection matrices, view matrices, etc. in your compositor effect.

@BastiaanOlij
Copy link
Contributor Author

BastiaanOlij commented Jul 11, 2024

Added a really crude demonstration of the compositor effects use case of this functionality here: godotengine/godot-demo-projects#1086

This should both work if you build the shader manually like in the post_proces_shader.gd example, as well as doing the include in a .glsl file. I'm assuming for real files they need a res::// prefix, else this means that our build in glsl include files have become reserved names.

As we lack some of the build in defines, it all does come with a bit of a manual (yet to be written), but for instance to include the scene data we get:

#version 450

#define MAX_VIEWS 2

#include "godot/scene_data_inc.glsl"

// Invocations in the (x, y, z) dimension.
layout(local_size_x = 8, local_size_y = 8, local_size_z = 1) in;

layout(set = 0, binding = 0, std140) uniform SceneDataBlock {
	SceneData data;
	SceneData prev_data;
}
scene_data_block;
...

Then in code we get:

var scene_data := p_render_data.get_render_scene_data()
var scene_data_buffers: RID = scene_data.get_uniform_buffer()

To retrieve our RID, and then obviously our uniform set:

var scene_data_uniform := RDUniform.new()
scene_data_uniform.uniform_type = RenderingDevice.UNIFORM_TYPE_UNIFORM_BUFFER
scene_data_uniform.binding = 0
scene_data_uniform.add_id(scene_data_buffers)
var uniform_set := UniformSetCacheRD.get_cache(shader, 0, [scene_data_uniform])

...

rd.compute_list_bind_uniform_set(compute_list, uniform_set, 0)

Now in the shader we can access the build in scene data uniform buffer (view in this case coming from our push constant):

mat4 inv_projection_matrix = scene_data_block.data.inv_projection_matrix_view[int(params.view)];

@BastiaanOlij BastiaanOlij marked this pull request as ready for review July 11, 2024 06:01
@BastiaanOlij BastiaanOlij requested a review from a team as a code owner July 11, 2024 06:01
@AThousandShips AThousandShips changed the title Adding ability to include build-in include files Add ability to include build-in include files Jul 11, 2024
@AThousandShips AThousandShips changed the title Add ability to include build-in include files Add ability to include built-in include files Jul 11, 2024
@BastiaanOlij BastiaanOlij requested a review from clayjohn July 15, 2024 03:53
@BastiaanOlij BastiaanOlij mentioned this pull request Jul 22, 2024
14 tasks
@BastiaanOlij
Copy link
Contributor Author

Rebased and added some more code that is needed for custom shader templates.

@BastiaanOlij
Copy link
Contributor Author

@clayjohn Would you mind reviewing this, I'd like to get this merged as the functionality added here is already useful even before we add custom shader templates.

@BastiaanOlij
Copy link
Contributor Author

Forgot to put some extra info here after our rendering meeting a month ago.
Decision was made to change this PR so the include files are held within their own ShaderIncludeDB class.
Just haven't had time to do this yet.

@BastiaanOlij
Copy link
Contributor Author

Rofl, edited the OP to include an example only to realise I had previously provided an example. A well, makes more sense in the OP. :)

This is ready for review again, with changes applied as discussed in rendering meeting from some time ago.

Copy link
Contributor

@BlueCube3310 BlueCube3310 left a comment

Choose a reason for hiding this comment

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

I've tested this locally and it works as expected. Looks good to me aside from some minor nitpicks

servers/rendering/renderer_rd/shader_rd.cpp Outdated Show resolved Hide resolved
servers/rendering/rendering_device.cpp Outdated Show resolved Hide resolved
doc/classes/ShaderIncludeDB.xml Outdated Show resolved Hide resolved
doc/classes/ShaderIncludeDB.xml Outdated Show resolved Hide resolved
doc/classes/ShaderIncludeDB.xml Outdated Show resolved Hide resolved
@BastiaanOlij BastiaanOlij force-pushed the buildin_includes branch 2 times, most recently from feafccd to 3d9698a Compare November 22, 2024 00:41
servers/rendering/rendering_device_binds.cpp Show resolved Hide resolved
servers/rendering/rendering_device_binds.cpp Outdated Show resolved Hide resolved
servers/rendering/shader_include_db.h Outdated Show resolved Hide resolved
servers/rendering/shader_include_db.h Outdated Show resolved Hide resolved
servers/rendering/renderer_rd/shader_rd.cpp Outdated Show resolved Hide resolved
servers/rendering/renderer_rd/shader_rd.cpp Outdated Show resolved Hide resolved
servers/rendering/shader_include_db.cpp Outdated Show resolved Hide resolved
servers/rendering/shader_include_db.cpp Outdated Show resolved Hide resolved
servers/rendering/shader_include_db.cpp Outdated Show resolved Hide resolved
Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

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

:) Reviewed in the rendering meeting

<?xml version="1.0" encoding="UTF-8" ?>
<class name="ShaderIncludeDB" inherits="Object" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="../class.xsd">
<brief_description>
Internal database of built in shader include files.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Internal database of built in shader include files.
Internal database of built-in shader include files.

Internal database of built in shader include files.
</brief_description>
<description>
This object contains shader fragments from Godot's internal shaders. These can be used when access to internal uniform buffers and/or internal functions is required for instance when composing compositor effects or compute shaders. Only fragments for the current rendering device are loaded.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This object contains shader fragments from Godot's internal shaders. These can be used when access to internal uniform buffers and/or internal functions is required for instance when composing compositor effects or compute shaders. Only fragments for the current rendering device are loaded.
This object contains shader fragments from Godot's internal shaders. These can be used when it is necessary to access to internal uniform buffers and/or internal functions, for example when composing compositor effects or compute shaders. Only fragments for the current rendering device are loaded.

<return type="bool" />
<param index="0" name="filename" type="String" />
<description>
Returns [code]true[/code] if an include file with this name exists.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Returns [code]true[/code] if an include file with this name exists.
Returns [code]true[/code] if an include file with the given [param filename] exists.

<method name="list_built_in_include_files" qualifiers="static">
<return type="PackedStringArray" />
<description>
Returns a list of built-in include files that are currently registered.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Returns a list of built-in include files that are currently registered.
Returns the list of built-in include files that are currently registered.

@Repiteo Repiteo merged commit 637239e into godotengine:master Dec 5, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Dec 5, 2024

Thanks!

@Mickeon
Copy link
Contributor

Mickeon commented Dec 5, 2024

Merged a bit too soon with the "built in" typo but ah well

@BastiaanOlij
Copy link
Contributor Author

h the "built in" typo but a

Shhhh, you can just fix it separately :P

@BastiaanOlij BastiaanOlij deleted the buildin_includes branch December 9, 2024 01:01
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.

6 participants