Skip to content

Fix specialize_shadows system ordering #18412

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
Merged
merged 1 commit into from
Mar 19, 2025
Merged

Fix specialize_shadows system ordering #18412

merged 1 commit into from
Mar 19, 2025

Conversation

@JMS55
Copy link
Contributor

@JMS55 JMS55 commented Mar 19, 2025

Objective

Solution

  • Move specialize_shadows to ManageViews so that it can run after prepare_lights, so that shadow views exist for specialization.
  • Unfortunately this means that specialize_shadows is no longer in PrepareMeshes like the rest of the specialization systems.

Testing

  • Ran anti_aliasing example, switched between the different AA options, observed no glitches.

@JMS55 JMS55 added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen labels Mar 19, 2025
@JMS55 JMS55 added the S-Needs-Review Needs reviewer attention (from anyone!) to move forward label Mar 19, 2025
@JMS55 JMS55 added this to the 0.16 milestone Mar 19, 2025
@JMS55 JMS55 requested review from pcwalton and tychedelia March 19, 2025 03:28
@JMS55
Copy link
Contributor Author

JMS55 commented Mar 19, 2025

I'm still not really certain why toggling the anti aliasing methods deletes the render world shadow views, but regardless the code is more correct after this PR (and fixes the bug).

tychedelia
tychedelia approved these changes Mar 19, 2025
View reviewed changes
Copy link
Member

@tychedelia tychedelia left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Yup, totally makes sense. Thanks for tracking this down. These kinds of system set orderings can be really sneaky with retained causing these kinds of one frame weirdness.

@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 Mar 19, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Mar 19, 2025
Merged via the queue into bevyengine:main with commit f353cc3 Mar 19, 2025
37 checks passed
mockersf pushed a commit that referenced this pull request Mar 19, 2025
@JMS55 @mockersf
Fix specialize_shadows system ordering (#18412)
62f7f2d 
# Objective
- Fixes #18332

## Solution

- Move specialize_shadows to ManageViews so that it can run after
prepare_lights, so that shadow views exist for specialization.
- Unfortunately this means that specialize_shadows is no longer in
PrepareMeshes like the rest of the specialization systems.

## Testing
- Ran anti_aliasing example, switched between the different AA options,
observed no glitches.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

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

@pcwalton pcwalton Awaiting requested review from pcwalton

+1 more reviewer

@tychedelia tychedelia tychedelia approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior 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: Done

Milestone

0.16

Development

Successfully merging this pull request may close these issues.

1-frame rendering flashes when changing the scene

3 participants

@JMS55 @alice-i-cecile @tychedelia