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

Allow users to provide a custom shadow shader #4443

Open
Ansraer opened this issue Apr 24, 2022 · 23 comments
Open

Allow users to provide a custom shadow shader #4443

Ansraer opened this issue Apr 24, 2022 · 23 comments

Comments

@Ansraer
Copy link

Ansraer commented Apr 24, 2022

Describe the project you are working on

3D foliage rendering

Describe the problem or limitation you are having in your project

When rendering a scene godot does 5 passes of it: One pre depth pass, three shadow passes, and finally a color pass.
With Godot's default PBR shader the depth and shadow pass use a pretty bare bones fragment shader since only the depth output is wanted.

However, for custom shaders this is not the case. While Godot still creates a shadow pipeline for the custom shader it uses the exact same fragment code as the color pipeline. This means that the depth and shadow passes of custom shaders calculate a lot of unnecessary output, wasting valuable time.

Some tests with Renderdoc have shown that the depth pass for a very basic noise based material with alpha scissors can take up to 60 microseconds. By manually trimming the shader down to only what is necessary for depth rendering (the alpha scissors) I managed to cut down the execution time to 20 microseconds.

With more complex custom shaders the performance loss would be even greater.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

What I want is the ability to provide a custom shadow shader. As far as I can tell Godot already has code for using a different shader for shadow and depth pass:

https://github.com/godotengine/godot/blob/e80aedbf20a7846a0e21dc8623823eef10b2e676/servers/rendering/renderer_rd/forward_clustered/render_forward_clustered.cpp#L2669-L2683

All that would be needed is a way to expose this to the user.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Similar to the next pass property users would be able to set another shader as a shadow material. With some modifications to the code linked above it should be possible to use that material for shadow rendering.

If this enhancement will not be used often, can it be worked around with a few lines of script?

Not that I am aware of. I suppose you could turn off shadows and create a duplicate that only renders shadows with a different material? But that would come with its own problems.

Is there a reason why this should be core and not an add-on in the asset library?

rendering is core.

@wojtekpil
Copy link

We have currently a section for shadow meshes in inspector. Those meshes have a material slot which could be used for this purpose. Currently it is a little bit counter intuitive, because overriding this does nothing.

@Ansraer
Copy link
Author

Ansraer commented Apr 25, 2022

@wojtekpil Maybe I am blind, but where are shadow meshes exposed to the editor in godot 4.0? Afaik Godot creates shadow meshes automatically on import. Also, based on my understanding of the code, godot uses a shared material for those shadow meshes.

My idea was to add a field below next pass like in the mockup below:

AwesomeMockup

image

@wojtekpil
Copy link

Under the mesh menu
image
I did not read the current code so I can't tell how does it work under the hood. It's just the place where I expected to replace a material for a shadow mesh.

@Ansraer
Copy link
Author

Ansraer commented Apr 25, 2022

The problem with this shadow mesh is that it is only used as long as you don't use a custom shader and edit anything vertex, position or alpha related. If you look at the code I linked in my initial post you can see how it isn't used once you edit one of those.
Based on what I can see this is mostly intended to be used by importers that auto-generate simplified meshes.
I wrote a quick PoC PR, here is how the UI currently looks:

PoC This uses a shadow shader where the left part of the mesh isn't rendered and this doesn't cast any shadows.

image

@clayjohn
Copy link
Member

We discussed this at the proposals meeting today, the consensus was that we should instead introduce an AT_SHADOW_PASS constant in the spatial shader that is true during the shadow pass and false otherwise. This would allow users to specify a different code path for shadows. We can rely on SSA to compile out unused code since it will be constant for the entire shader (either using a macro define or a specialization constant).

From a user perspective it would allow users to write things like:

void fragment() {
    if (!AT_SHADOW_PASS) {
         // do expensive transparency calculations here
    } else {
         // do cheap approximation with discard here
    }
}

@Ansraer
Copy link
Author

Ansraer commented Apr 29, 2022

We discussed this at the proposals meeting today, the consensus was that we should instead introduce an AT_SHADOW_PASS constant in the spatial shader that is true during the shadow pass and false otherwise. This would allow users to specify a different code path for shadows. We can rely on SSA to compile out unused code since it will be constant for the entire shader (either using a macro define or a specialization constant).

From a user perspective it would allow users to write things like:

void fragment() {
    if (!AT_SHADOW_PASS) {
         // do expensive transparency calculations here
    } else {
         // do cheap approximation with discard here
    }
}

Ah damn it, I keep missing the meetings.
Yeah, this will probably be a far better solution than what I currently use in my fork.
Since the shadow shader is also used for the pre depth pass it might be a better idea to use something like DEPTH_ONLY (to better convey that this will be true for the depth and shadow pass), and have a second, non-constant, boolean to differentiate between shadow and pre depth pass.

@clayjohn
Copy link
Member

Ah damn it, I keep missing the meetings. Yeah, this will probably be a far better solution than what I currently use in my fork. Since the shadow shader is also used for the pre depth pass it might be a better idea to use something like DEPTH_ONLY (to better convey that this will be true for the depth and shadow pass), and have a second, non-constant, boolean to differentiate between shadow and pre depth pass.

I wouldn't worry about exposing DEPTH_ONLY unless someone points out a use-case for a custom depth-prepass/shadow shader. It should be fine to just expose AT_SHADOW_PASS. It is important to do this with a #define or with a specialization constant so that the SSA optimizer can remove the code inside the branch. If you use a uniform or push constant there is no guarantee at compile time that the code won't run.

In the GLES3 backend, this can be done with a shader varient (which is roughly equivalent to a specialization constant)

@Ansraer
Copy link
Author

Ansraer commented Apr 29, 2022

Oh, I think you misunderstood what I meant @clayjohn.
I want DEPTH_ONLY to be the name of the define (or constant) that is only true for the depth pre-pass and shadowmap pipeline (internally this is MaterialData::shadow_pass afaik).
imo telling the user what the result of the shader is (in this case writing only to depth) makes more sense than calling it AT_SHADOW_PASS, when it is used for passes other than shadows as well.
And I get why it needs to be either a define or a specialization constant, after all the entire point of my proposal is to make sure that as much code as possible is removed before the shader is executed. Just wanted to propose a different name.

The reason why I proposed the second (not const) bool was so that people could manipulate only the shadows without affecting the depth pass. While not useful for my particular usecase I could totally see how someone working on a horror game might want to modify only the shadows without changing the depth or color pass.

EDIT: Now I really want to make a horror game with shadows that try to grasp the player somehow. Just imagine seeing a completely unassuming tree, and then you look down and notice how the shadows of its branches slowly creep towards you...
I should write that down for the next game jam I participate in.

@EMBYRDEV
Copy link

The reason why I proposed the second (not const) bool was so that people could manipulate only the shadows without affecting the depth pass. While not useful for my particular usecase I could totally see how someone working on a horror game might want to modify only the shadows without changing the depth or color pass.

+1 that would be a super useful feature and just increases engine flexibility.

@clayjohn
Copy link
Member

The reason why I proposed the second (not const) bool was so that people could manipulate only the shadows without affecting the depth pass. While not useful for my particular usecase I could totally see how someone working on a horror game might want to modify only the shadows without changing the depth or color pass.

+1 that would be a super useful feature and just increases engine flexibility.

That's a plus one for being able to manipulate the shadow pass apart from the depth pre-pass? Or a plus one to the proposal in general? If the former, can you please elaborate?

@EMBYRDEV
Copy link

That's a plus one for being able to manipulate the shadow pass apart from the depth pre-pass? Or a plus one to the proposal in general? If the former, can you please elaborate?

My thinking at the time was the ability to affect how shadows would be drawn vs in the depth pass as I thought that having them tied could lead to issues. Upon rereading, I think you're right that AT_SHADOW_PASS would differentiate enough for that case.

Naming this DEPTH_ONLY would be misleading if I understand how this would be implemented.

My appologies, I was up rather late.

@QbieShay
Copy link

QbieShay commented Jun 3, 2022

Nit on the name: IN_SHADOW_PASS is probably clearer.

@EvilNickolas
Copy link

+1 for being able to customize shadows.

I currently have a need overide the shadows influence on color, and id like to be able so apply my own smoothing/blur
to appear more like cell shading found in a toon shader

@Calinou
Copy link
Member

Calinou commented Jun 10, 2022

I currently have a need overide the shadows influence on color, and id like to be able so apply my own smoothing/blur to appear more like cell shading found in a toon shader

This may already be feasible; see the comments in godotengine/godot#60360.

@viktor-ferenczi
Copy link

viktor-ferenczi commented Sep 16, 2022

We have currently a section for shadow meshes in inspector. Those meshes have a material slot which could be used for this purpose. Currently it is a little bit counter intuitive, because overriding this does nothing.

Just tested with alpha16, setting the shadow_mesh still does nothing. Also, it is available only in ArrayMesh. It is also confusing, since an entire chain of embedded shadow meshes can be set up, which is meaningless.

The idea of having a shadow mess with its own material (which can be a custom shader) is good. But the interface needs to support using the same mesh, but with different material as well, in case the mesh does not need to be simplified for shadow rendering.

I suggest adding two new properties to MeshInstance3D:

  • shadow_mesh_override
    Provides a mesh and material for shadow rendering.
  • shadow_material_override
    Overrides only the material for shadow rendering. Can be used without providing an override mesh, but it takes precedence if the override mesh also comes with a material. It would work similarly to surface material override.

I also suggest removing ArrayMesh.shadow_mesh if that's not used for anything. If it is used, then must be documented. It has no documentation, currently. It is pretty confusing to have it there.

@cdoise-vbg
Copy link

I believe the IN_SHADOW_PASS flag would be very helpful. I'm coming from Unreal which has the "Shadow Pass Switch" node in the visual shader editor, and it makes it very easy to do things like have an x-ray effect into a building where shadows from walls and ceilings are still applied to the inside.

@cdoise-vbg
Copy link

I found a somewhat hacky workaround to detect being in the shadow pass which allowed me to test my x-ray method (and things do work out as desired). I say it's hacky because it won't work properly if the camera is ever in same position as one of the lights.

In the shadow passes, Godot sets the "camera_position_world" built-in to the position of the the light, so I did the following in my project:

  • Added script that sets the active camera.global_position to a global_shader_parameter ("camera_world_pos").
  • Then, in my shader, I compare the global uniform "camera_world_pos" to the Input "camera_position_world" and if those are different, I assume I'm in a shadow pass (in my case, I always return 1 for opacity in the shadow pass case, and potentially return something less than 1 if I have the x-ray vision toggled on my node). I'm able to see through the wall, but the light doesn't get through.

Having IN_SHADOW_PASS flag (plus visual shader node switch equivalent) would be ideal though since I wouldn't have to worry about the camera and light positions overlapping (and presumably the shader compiler could possibly optimize things away in the shadow vs non-shadow case).

@cdoise-vbg
Copy link

cdoise-vbg commented Nov 27, 2023

Well, I just discovered another work around that relies on the fact that near_z is set to 0 for the shadow passes. This means that PROJECTION_MATRIX[3][2] will be zero in shadow passes, so you can do the following in the shader code to detect shadow passes:

bool is_shadow_pass = PROJECTION_MATRIX[3][2] == 0.0;

UPDATE:
Upon further experimentation, just having this doesn't necessarily seem to solve my issue. This blocks the light without issues for my directional light. For omni lights, however, it seems to work at first, but every now and then the light acts as though the wall isn't there but then if I move my camera around, and it goes back to behaving the way I want (which is like the Shadow Pass Switch in Unreal).

@aldebaranzbradaradjan
Copy link

The ability to know if we are in shadow pass should be nice.

bool is_shadow_pass = PROJECTION_MATRIX[3][2] == 0.0;
This workaroud was working just fine in 4.2 but no more in 4.3 beta for me.

@Calinou
Copy link
Member

Calinou commented Jun 24, 2024

This workaroud was working just fine in 4.2 but no more in 4.3 beta for me.

This is surely due to reverse Z being implemented in 4.3. You should be able to adjust the check to account for this.

@aldebaranzbradaradjan
Copy link

aldebaranzbradaradjan commented Jun 24, 2024

Thanks Calinou, i have read the new at time but don't realize it was important for me !
The trick work again with this modification : bool is_shadow_pass = PROJECTION_MATRIX[2][3] == 0.0;
I don't understand very well all this matrix stuf anyway, i hope the IN_SHADOW_PASS flag will be here someday (but i cannot help with that with my actual knowledge) !

@KiSetsuFu-PuLiN
Copy link

The trick work again with this modification : bool is_shadow_pass = PROJECTION_MATRIX[2][3] == 0.0;

This trick does work in 4.3 Shader. But in VisualShader, it seems that there is no way to access PROJECTION_MATRIX[2][3].
Maybe it could be optimized.

@RonYanDaik
Copy link

RonYanDaik commented Sep 27, 2024

The trick work again with this modification : bool is_shadow_pass = PROJECTION_MATRIX[2][3] == 0.0;

Doesn't work for me. Should it work for all types of light? I'm trying to disable distance fade for shadows.
Upd: works when shadow is Dual Paraboloid but doesn't work when shadow is CubeMap.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Needs consensus
Development

No branches or pull requests