-
-
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
[Merged by Bors] - Pipelined Rendering #6503
Closed
Closed
Changes from all commits
Commits
Show all changes
59 commits
Select commit
Hold shift + click to select a range
3a648bc
split sub app runner into 2 separate closures
hymm 115db67
bad pipelining
hymm 833e47c
fix issue with systems not running sometimes
hymm 1bd4908
create a global main thread executor
hymm 33f8207
replace task scope executor with main thread executor
hymm 1abddb8
reenable NonSendMarker on prepare_windows
hymm ade5b77
make a safer abstraction for the main thread executor
hymm 1e0cecc
try to fix ci
hymm 5498fe5
change scope to take a thread executor
hymm d971288
add a test and update docs
hymm bbd8626
fix wasm compilation
hymm 6842f4e
move extract after scope to make input processing closer to main app run
hymm b6598fb
switch to sending render world back and forth with channels
hymm 0892f5b
add ability to disable pipelined rendering
hymm dd9163c
cleanup
hymm 5d70b8d
remove executor.run from scope
hymm dcd2d83
wrap scope so most uses don't need to pass None
hymm 6e149e4
make a setup function on app to run the setup_rendering function
hymm a53cb13
change pipelined rendering into a plugin
hymm 3cd2122
fix pipelined rendering
hymm 799511f
fix wasm again
hymm d306874
move setup to plugin
hymm f2507fa
move cloning the MainThreadExecutor to setup
hymm 8f5f259
remove runner and just run the schedule
hymm 1bf771c
fix headless example
hymm c537afb
cleanup
hymm 98e9c04
add render app span
hymm 24241a4
move inserting MainThreadExecutor to PipelinedRenderingPlugin
hymm a3b1929
remove unnecessary sync bound
hymm 32270e0
change scope to not tick global executor when running parallel executor
hymm 6e3837e
fix wasm
hymm 27d095c
move extract commands to render world
hymm 41b0cce
clean up
hymm a4aa001
use resource scope instead of removing resource
hymm 45d8f00
remove incorrect comment
hymm c57257b
fix wasm builds
hymm 883fe29
fix rebase issues
hymm f506f74
tick the task pool executor if there are no threads allocated
hymm 357549c
call clear trackers on sub apps
hymm 128de37
remove unnecessary system and rename another
hymm cee9c53
remove unnecessary dependency on bevy_tasks
hymm e326796
run executors forever even with panics
hymm 0c993a9
Merge remote-tracking branch 'upstream/main' into maybe-pipelining
hymm c9f088e
Merge remote-tracking branch 'bevyengine/main' into maybe-pipelining
hymm d8425d6
Merge remote-tracking branch 'upstream/main' into maybe-pipelining
hymm 0fe7234
fix some merge issues
hymm f3d5a0c
fix wasm build
hymm 8cf5e65
Apply suggestions from code review
hymm 47c5364
resolve some review comments
hymm e4af50a
add example to subapp docs
hymm a78ae52
fix ci
hymm 39b636c
refactor scope executors into separate functions
hymm 2f1c317
add tracing spans for all sub apps
hymm b856cd7
Merge remote-tracking branch 'upstream/main' into maybe-pipelining
hymm 92d123c
change world.get_resource to resource
hymm 1d6a5b9
add doc for tick_task_pool_executor
hymm 2faeb9b
fix spelling
hymm 04440d5
Apply suggestions from code review
hymm 0ebbde7
fix transmute
hymm File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ mod extract_param; | |
pub mod extract_resource; | ||
pub mod globals; | ||
pub mod mesh; | ||
pub mod pipelined_rendering; | ||
pub mod primitives; | ||
pub mod render_asset; | ||
pub mod render_graph; | ||
|
@@ -71,6 +72,9 @@ pub enum RenderStage { | |
/// running the next frame while rendering the current frame. | ||
Extract, | ||
|
||
/// A stage for applying the commands from the [`Extract`] stage | ||
ExtractCommands, | ||
|
||
/// Prepare render resources from the extracted data for the GPU. | ||
Prepare, | ||
|
||
|
@@ -190,8 +194,14 @@ impl Plugin for RenderPlugin { | |
// after access to the main world is removed | ||
// See also https://github.com/bevyengine/bevy/issues/5082 | ||
extract_stage.set_apply_buffers(false); | ||
|
||
// This stage applies the commands from the extract stage while the render schedule | ||
// is running in parallel with the main app. | ||
let mut extract_commands_stage = SystemStage::parallel(); | ||
extract_commands_stage.add_system(apply_extract_commands.at_start()); | ||
Comment on lines
+198
to
+201
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice touch! :) |
||
render_app | ||
.add_stage(RenderStage::Extract, extract_stage) | ||
.add_stage(RenderStage::ExtractCommands, extract_commands_stage) | ||
.add_stage(RenderStage::Prepare, SystemStage::parallel()) | ||
.add_stage(RenderStage::Queue, SystemStage::parallel()) | ||
.add_stage(RenderStage::PhaseSort, SystemStage::parallel()) | ||
|
@@ -222,7 +232,7 @@ impl Plugin for RenderPlugin { | |
|
||
app.add_sub_app(RenderApp, render_app, move |app_world, render_app| { | ||
#[cfg(feature = "trace")] | ||
let _render_span = bevy_utils::tracing::info_span!("renderer subapp").entered(); | ||
let _render_span = bevy_utils::tracing::info_span!("extract main app to render subapp").entered(); | ||
{ | ||
#[cfg(feature = "trace")] | ||
let _stage_span = | ||
|
@@ -308,10 +318,12 @@ fn extract(app_world: &mut World, render_app: &mut App) { | |
let inserted_world = render_world.remove_resource::<MainWorld>().unwrap(); | ||
let scratch_world = std::mem::replace(app_world, inserted_world.0); | ||
app_world.insert_resource(ScratchMainWorld(scratch_world)); | ||
|
||
// Note: We apply buffers (read, Commands) after the `MainWorld` has been removed from the render app's world | ||
// so that in future, pipelining will be able to do this too without any code relying on it. | ||
// see <https://github.com/bevyengine/bevy/issues/5082> | ||
extract_stage.0.apply_buffers(render_world); | ||
}); | ||
} | ||
|
||
// system for render app to apply the extract commands | ||
fn apply_extract_commands(world: &mut World) { | ||
world.resource_scope(|world, mut extract_stage: Mut<ExtractStage>| { | ||
cart marked this conversation as resolved.
Show resolved
Hide resolved
|
||
extract_stage.0.apply_buffers(world); | ||
}); | ||
} |
Oops, something went wrong.
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.
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.
Maybe we should just remove this in favor of
insert_sub_app
. Theinsert
semantics are "correct" anyway whereasadd
is generally used for repeatable / non-overwriting actions. This function is slightly more ergonomic, but SubApps aren't exactly a common pattern anyway.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.
I'd be ok with removing this. I'll make an issue for this, so it doesn't get lost.