[Merged by Bors] - bevy_pbr2: Add support for not casting/receiving shadows #2726
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
superdump
force-pushed
the
pipelined-dont-cast-dont-receive
branch
from
4f8292d
to
a1a5b01
Compare
Having to add a zero-sized type component to every entity to mark it as a shadow caster or shadow receiver by default feels quite unnecessary. As such, I have added NotShadowCaster and NotShadowReceiver components. This introduces double-negatives like Without<NotShadowReceiver>, etc but it means that only the exceptions have to be specified in user code. I also added an example to illustrate the effect.
superdump
force-pushed
the
pipelined-dont-cast-dont-receive
branch
from
a1a5b01
to
1de89e7
Compare
64 tasks
|
This looks good to me / I think its worth merging as-is for now. My biggest questions are around "ideal user facing apis". There are lots of options here. Some engines make "shadow receiving" a material property, which might make sense here. I also want to consider something like: struct ShadowCaster {
enabled: bool,
}To avoid the need for adding removing components / make the settings more discoverable. But yeah this impl looks solid and I'd rather integrate it sooner than later. I removed the camera controls from the new example because they make it bigger / more complicated and I dont think the example benefits from freedom of movement. |
|
bors r+ |
bors bot
pushed a commit
that referenced
this pull request
Aug 25, 2021
# Objective Allow marking meshes as not casting / receiving shadows. ## Solution - Added `NotShadowCaster` and `NotShadowReceiver` zero-sized type components. - Extract these components into `bool`s in `ExtractedMesh` - Only generate `DrawShadowMesh` `Drawable`s for meshes _without_ `NotShadowCaster` - Add a `u32` bit `flags` member to `MeshUniform` with one flag indicating whether the mesh is a shadow receiver - If a mesh does _not_ have the `NotShadowReceiver` component, then it is a shadow receiver, and so the bit in the `MeshUniform` is set, otherwise it is not set. - Added an example illustrating the functionality. NOTE: I wanted to have the default state of a mesh as being a shadow caster and shadow receiver, hence the `Not*` components. However, I am on the fence about this. I don't want to have a negative performance impact, nor have people wondering why their custom meshes don't have shadows because they forgot to add `ShadowCaster` and `ShadowReceiver` components, but I also really don't like the double negatives the `Not*` approach incurs. What do you think? Co-authored-by: Carter Anderson <mcanders1@gmail.com>
|
Pull request successfully merged into pipelined-rendering. Build succeeded: |
bors
bot
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objective
Allow marking meshes as not casting / receiving shadows.
Solution
NotShadowCasterandNotShadowReceiverzero-sized type components.bools inExtractedMeshDrawShadowMeshDrawables for meshes withoutNotShadowCasteru32bitflagsmember toMeshUniformwith one flag indicating whether the mesh is a shadow receiverNotShadowReceivercomponent, then it is a shadow receiver, and so the bit in theMeshUniformis set, otherwise it is not set.NOTE: I wanted to have the default state of a mesh as being a shadow caster and shadow receiver, hence the
Not*components. However, I am on the fence about this. I don't want to have a negative performance impact, nor have people wondering why their custom meshes don't have shadows because they forgot to addShadowCasterandShadowReceivercomponents, but I also really don't like the double negatives theNot*approach incurs. What do you think?