Skip to content

Split LatePrepassNode query in a few groups #19840

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Split LatePrepassNode query in a few groups #19840

wants to merge 1 commit into from
+29 −27

Conversation

IceSentry
Copy link
Contributor

Objective

  • That node has a bunch of query items that are mostly ignored in a few places
  • Previously this lead to having a long chain of ignored params that was replaced with ..,. This works, but this seems a bit more likely to break in a subtle way if new parameters are added

Solution

  • Split the query in a few groups based on how it was already structured (Mandatory, Optional, Has)

Testing

  • None, it's just code style changes

@IceSentry IceSentry added the A-Rendering Drawing game state to the screen label Jun 27, 2025
@IceSentry IceSentry added D-Trivial Nice and easy! A great choice to get started with Bevy C-Code-Quality A section of code that is hard to understand or change S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jun 27, 2025
atlv24
atlv24 approved these changes Jun 27, 2025
View reviewed changes
Copy link
Contributor

@atlv24 atlv24 left a comment

Choose a reason for hiding this comment

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

this pattern can be used a lot more in the codebase

JMS55
JMS55 reviewed Jun 27, 2025
View reviewed changes
crates/bevy_core_pipeline/src/prepass/node.rs
@@ -79,7 +85,7 @@ impl ViewNode for LatePrepassNode {
) -> Result<(), NodeRunError> {
// We only need a late prepass if we have occlusion culling and indirect
// drawing.
let (.., occlusion_culling, no_indirect_drawing, _) = query;
Copy link
Contributor

Choose a reason for hiding this comment

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

Iirc this should be done for the deferred node as well.

@alice-i-cecile 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 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JMS55 JMS55 JMS55 left review comments

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

@atlv24 atlv24 atlv24 approved these changes

Assignees
No one assigned
Labels
A-Rendering Drawing game state to the screen C-Code-Quality A section of code that is hard to understand or change D-Trivial Nice and easy! A great choice to get started with Bevy S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
Rendering
Status: No status
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@IceSentry @alice-i-cecile @atlv24 @JMS55