Skip to content

Use RenderStartup for all basic cases in bevy_core_pipeline.#20002

Merged
alice-i-cecile merged 2 commits into
bevyengine:mainfrom
andriyDev:startup-core
Jul 15, 2025
Merged

Use RenderStartup for all basic cases in bevy_core_pipeline.#20002
alice-i-cecile merged 2 commits into
bevyengine:mainfrom
andriyDev:startup-core

Conversation

@andriyDev

@andriyDev andriyDev commented Jul 7, 2025

Copy link
Copy Markdown
Contributor

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 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

fantastic work thanks

Comment thread crates/bevy_core_pipeline/src/lib.rs
render_app
.init_resource::<BlitPipeline>()
.init_resource::<SpecializedRenderPipelines<BlitPipeline>>();
.allow_ambiguous_resource::<SpecializedRenderPipelines<BlitPipeline>>()

Copy link
Copy Markdown
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

andriyDev commented Jul 15, 2025

Copy link
Copy Markdown
Contributor Author

@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
This was referenced Jul 16, 2025
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-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

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants