-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Use dynamic uniform buffer in post processing example #13540
Merged
mockersf
merged 1 commit into
bevyengine:main
from
jgayfer:post-processing-dynamic-uniform
Jun 14, 2024
Merged
Use dynamic uniform buffer in post processing example #13540
mockersf
merged 1 commit into
bevyengine:main
from
jgayfer:post-processing-dynamic-uniform
Jun 14, 2024
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
While learning about shaders and pipelines, I found this example to be misleading; it wasn't clear to me how the node knew what the correct "instance" of `PostProcessSettings` we should send to the GPU (as the combination of `ExtractComponentPlugin` and `UniformComponentPlugin` extracts + sends _all_ of our `PostProcessSetting` components). While the example in its current state is _correct_, I believe that fact that it's intended to showcase a per camera post processing effect warrants a dynamic uniform buffer (even though in the context of this example we have only one camera, and therefore no adverse behaviour).
Welcome, new contributor! Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨ |
jgayfer
changed the title
Use dynamic uniform buffer for post process settings
Use dynamic uniform buffer in post processing example
May 27, 2024
IceSentry
approved these changes
May 28, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Would be nice to actually take advantage of it in an example, but it's not required
alice-i-cecile
added
A-Rendering
Drawing game state to the screen
C-Examples
An addition or correction to our examples
S-Needs-Review
Needs reviewer attention (from anyone!) to move forward
labels
Jun 1, 2024
alice-i-cecile
approved these changes
Jun 13, 2024
alice-i-cecile
added
S-Ready-For-Final-Review
This PR has been approved by the community. It's ready for a maintainer to consider merging it
and removed
S-Needs-Review
Needs reviewer attention (from anyone!) to move forward
labels
Jun 13, 2024
mockersf
approved these changes
Jun 14, 2024
mockersf
pushed a commit
that referenced
this pull request
Jun 14, 2024
# Objective While learning about shaders and pipelines, I found this example to be misleading; it wasn't clear to me how the node knew what the correct "instance" of `PostProcessSettings` we should send to the shader (as the combination of `ExtractComponentPlugin` and `UniformComponentPlugin` extracts + sends _all_ of our `PostProcessSetting` components to the GPU). The goal of this PR is to clarify how to target the view specific `PostProcessSettings` in the shader when there are multiple cameras. ## Solution To accomplish this, we can use a dynamic uniform buffer for `PostProcessSettings`, querying for the relevant `DynamicUniformIndex` in the `PostProcessNode` to get the relevant index to use with the bind group. While the example in its current state is _correct_, I believe that fact that it's intended to showcase a per camera post processing effect warrants a dynamic uniform buffer (even though in the context of this example we have only one camera, and therefore no adverse behaviour). ## Testing - Run the `post_processing` example before and after this change, verifying they behave the same. ## Reviewer notes This is my first PR to Bevy, and I'm by no means an expert in the world of rendering (though I'm trying to learn all I can). If there's a better way to do this / a reason not to take this route, I'd love to hear it! Thanks in advance.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
A-Rendering
Drawing game state to the screen
C-Examples
An addition or correction to our examples
S-Ready-For-Final-Review
This PR has been approved by the community. It's ready for a maintainer to consider merging it
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
While learning about shaders and pipelines, I found this example to be misleading; it wasn't clear to me how the node knew what the correct "instance" of
PostProcessSettings
we should send to the shader (as the combination ofExtractComponentPlugin
andUniformComponentPlugin
extracts + sends all of ourPostProcessSetting
components to the GPU).The goal of this PR is to clarify how to target the view specific
PostProcessSettings
in the shader when there are multiple cameras.Solution
To accomplish this, we can use a dynamic uniform buffer for
PostProcessSettings
, querying for the relevantDynamicUniformIndex
in thePostProcessNode
to get the relevant index to use with the bind group.While the example in its current state is correct, I believe that fact that it's intended to showcase a per camera post processing effect warrants a dynamic uniform buffer (even though in the context of this example we have only one camera, and therefore no adverse behaviour).
Testing
post_processing
example before and after this change, verifying they behave the same.Reviewer notes
This is my first PR to Bevy, and I'm by no means an expert in the world of rendering (though I'm trying to learn all I can). If there's a better way to do this / a reason not to take this route, I'd love to hear it!
Thanks in advance.