Skip to content

Use RenderStartup for all basic cases in bevy_core_pipeline. #20002

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 2 commits into from
Jul 15, 2025
Merged

Use RenderStartup for all basic cases in bevy_core_pipeline. #20002

merged 2 commits into from
Jul 15, 2025

Conversation

andriyDev
Copy link
Contributor

@andriyDev andriyDev commented Jul 7, 2025 •
edited
Loading

Objective

Solution

  • Convert FromWorld impls into systems.
  • Run those systems in RenderStartup.

Testing

  • Ran bloom_3d, auto_exposure, depth_of_field, motion_blur, skybox, post_processing, and tonemapping examples and they all work :)

@andriyDev andriyDev added A-Rendering Drawing game state to the screen C-Code-Quality A section of code that is hard to understand or change D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jul 7, 2025
@andriyDev andriyDev requested a review from atlv24 July 7, 2025 18:59
atlv24
atlv24 approved these changes Jul 9, 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.

fantastic work thanks

crates/bevy_core_pipeline/src/blit/mod.rs
render_app
.init_resource::<BlitPipeline>()
.init_resource::<SpecializedRenderPipelines<BlitPipeline>>();
.allow_ambiguous_resource::<SpecializedRenderPipelines<BlitPipeline>>()
Copy link
Contributor

Choose a reason for hiding this comment

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

i feel like a lot of "just data" resources like this and configs etc should be allow_ambiguous_resource. Immutable resources pleaaaaaasseeeeeeeeeeeeee @alice-i-cecile

@andriyDev andriyDev 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 Jul 9, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jul 14, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 14, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jul 14, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Jul 14, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jul 15, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 15, 2025
@andriyDev
Copy link
Contributor Author

andriyDev commented Jul 15, 2025 •
edited
Loading

@alice-i-cecile I have discovered that the culprit is CopyDeferredLightingIdPipeline. I don't yet know why it's special, but it's the only one that consistently causes the segfault. I've removed the offending portion and will follow up with another PR once I've figured out what's going on there.

So I think we can move forward merging this PR.

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jul 15, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Jul 15, 2025
@alice-i-cecile alice-i-cecile enabled auto-merge July 15, 2025 06:54
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jul 15, 2025
Merged via the queue into bevyengine:main with commit 2bddbdf Jul 15, 2025
32 checks passed
@andriyDev andriyDev deleted the startup-core branch July 15, 2025 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@atlv24 atlv24 atlv24 approved these changes

@IceSentry IceSentry IceSentry 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-Straightforward Simple bug fixes and API improvements, docs, test and examples 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
No milestone
Development

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