Add shadow_caster_mask to Light3D.#85338
Conversation
514383e to
0e7577e
Compare
|
One point of discussion on this might be do we want this mask to be ANDed with the existing light cull mask (what we do in this PR) or if it should be entirely standalone to allow people setting certian meshes to not be lit by this light but will still cast shadows (I don't see many use cases for this). |
0e7577e to
f7a2e2d
Compare
shadow_caster_mask to Light3D.
|
There seems to be a reasonable demand for this feature, any word on the reviews? Would love to make it into 4.3. |
There was a problem hiding this comment.
Tested locally, it works as expected.
Testing project: test_shadow_caster_mask.zip
Example with the green object not casting shadows for the directional light only, and the purple OmniLight having a projector but not casting shadows for any objects:
Forward+
Mobile
Compatibility
Light projectors aren't supported when using Compatibility.
One point of discussion on this might be do we want this mask to be ANDed with the existing light cull mask (what we do in this PR) or if it should be entirely standalone to allow people setting certian meshes to not be lit by this light but will still cast shadows (I don't see many use cases for this).
I think the current behavior in this PR is fine; I can't think of any use cases for having an object not be lit by a light but still cast a shadow. We can revisit this later if there's enough demand.
| FUNC5(light_set_distance_fade, RID, bool, float, float, float) | ||
| FUNC2(light_set_reverse_cull_face_mode, RID, bool) | ||
| FUNC2(light_set_shadow_caster_mask, RID, uint32_t) | ||
| FUNC1RC(uint32_t, light_get_shadow_caster_mask, RID) |
There was a problem hiding this comment.
Does this need to be exposed? Other light storage methods don't have a getter exposed, presumably to avoid pipeline stalls when the method is called.
cc @clayjohn
There was a problem hiding this comment.
As @Calinou says for getters it is usually preferable to cache scene side and return that rather than query the servers, which can cause a sync and stall the pipeline.
| ClassDB::bind_method(D_METHOD("set_shadow_reverse_cull_face", "enable"), &Light3D::set_shadow_reverse_cull_face); | ||
| ClassDB::bind_method(D_METHOD("get_shadow_reverse_cull_face"), &Light3D::get_shadow_reverse_cull_face); | ||
|
|
||
| ClassDB::bind_method(D_METHOD("set_shadow_caster_mask", "enable"), &Light3D::set_shadow_caster_mask); |
There was a problem hiding this comment.
| ClassDB::bind_method(D_METHOD("set_shadow_caster_mask", "enable"), &Light3D::set_shadow_caster_mask); | |
| ClassDB::bind_method(D_METHOD("set_shadow_caster_mask", "caster_mask"), &Light3D::set_shadow_caster_mask); |
|
NOTE: This isn't abandoned, I've just been sidetracked with other work in the meantime. I will revisit and incorporate feedback soon. |
Glad to hear that someone is working on this feature! I was just having problems with unwanted shadows cast from billboarded and shaded Sprite3Ds (that also appear to be self-shadowing...) in my 3D project, and I believe that this feature would likely resolve it. |
|
Any chance we could get something like this for 2D lights? |
|
Very important feature I've been hoping for, really hope this makes it into 4.3! |
Maybe, but this should be requested in a proposal first as it's entirely separate functionality.
4.3 is in feature freeze, so any new features may only be merged in 4.4 at the earliest. If you need this feature right now, you can checkout the PR's branch and compile the editor and any export templates you might need for your project and specify them as custom export templates in the Export dialog. |
|
Any chance this will make it in for 4.4? Seems like everything has been addressed. |
|
@EMBYRDEV I fully intend to merge this for 4.4! Sorry for my delay in reviewing |
|
Confirmed working including rebased. |
clayjohn
left a comment
There was a problem hiding this comment.
Reviewed with the rendering team at today's rendering meeting and confirmed that the implementation is correct!
Sorry for the delay in reviewing
|
Thanks! |
Implements godotengine/godot-proposals#3606 & godotengine/godot-proposals#8509
Tested on Forward+, Mobile & Compatibility renderers.
I feel this is a very useful change for optimisation when using dynamic shadows in projects and is required to provide a cheaper way of preventing lights bleeding into other rooms, even when they dont need detailed shadows otherwise.
This is more fine grained than just disabling shadows on the MeshInstances as you may want the ability for some lights to cast shadows for that object but not others.
See below, the SpotLight3D is set to only cast shadows for Layer 2. It defaults to existing behaviour (all layers that the light affects).
