From 2c6cf9a597ef3d4b732e23534eec0cca8661a9f1 Mon Sep 17 00:00:00 2001 From: andriyDev Date: Sun, 6 Jul 2025 18:31:40 -0700 Subject: [PATCH] Run `RenderStartup` in/before extract instead of after it. (#19926) # Objective - Fixes #19910. ## Solution - First, allow extraction function to be FnMut instead of Fn. FnMut is a superset of Fn anyway, and we only ever call this function once at a time (we would never call this in parallel for different pairs of worlds or something). - Run the `RenderStartup` schedule in the extract function with a flag to only do it once. - Remove all the `MainRender` stuff. One sad part here is that now the `RenderStartup` blocks extraction. So for pipelined rendering, our simulation will be blocked on the first frame while we set up all the rendering resources. I don't see this as a big loss though since A) that is fundamentally what we want here - extraction **has to** run after `RenderStartup`, and the only way to do better is to somehow run `RenderStartup` in parallel with the first simulation frame, and B) without `RenderStartup` the **entire** app was blocked on initializing render resources during Plugin construction - so we're not really losing anything here. ## Testing - I ran the `custom_post_processing` example (which was ported to use `RenderStartup` in #19886) and it still works. --- crates/bevy_app/src/sub_app.rs | 8 ++--- crates/bevy_render/src/lib.rs | 33 ++++++++----------- .../migration-guides/extract_fn_is_mut.md | 10 ++++++ 3 files changed, 28 insertions(+), 23 deletions(-) create mode 100644 release-content/migration-guides/extract_fn_is_mut.md diff --git a/crates/bevy_app/src/sub_app.rs b/crates/bevy_app/src/sub_app.rs index 56d6b43d38..56a496f2b5 100644 --- a/crates/bevy_app/src/sub_app.rs +++ b/crates/bevy_app/src/sub_app.rs @@ -12,7 +12,7 @@ use core::fmt::Debug; #[cfg(feature = "trace")] use tracing::info_span; -type ExtractFn = Box; +type ExtractFn = Box; /// A secondary application with its own [`World`]. These can run independently of each other. /// @@ -160,7 +160,7 @@ impl SubApp { /// The first argument is the `World` to extract data from, the second argument is the app `World`. pub fn set_extract(&mut self, extract: F) -> &mut Self where - F: Fn(&mut World, &mut World) + Send + 'static, + F: FnMut(&mut World, &mut World) + Send + 'static, { self.extract = Some(Box::new(extract)); self @@ -177,13 +177,13 @@ impl SubApp { /// ``` /// # use bevy_app::SubApp; /// # let mut app = SubApp::new(); - /// let default_fn = app.take_extract(); + /// let mut default_fn = app.take_extract(); /// app.set_extract(move |main, render| { /// // Do pre-extract custom logic /// // [...] /// /// // Call Bevy's default, which executes the Extract phase - /// if let Some(f) = default_fn.as_ref() { + /// if let Some(f) = default_fn.as_mut() { /// f(main, render); /// } /// diff --git a/crates/bevy_render/src/lib.rs b/crates/bevy_render/src/lib.rs index 471a672540..9b8ef6fb37 100644 --- a/crates/bevy_render/src/lib.rs +++ b/crates/bevy_render/src/lib.rs @@ -221,22 +221,6 @@ pub enum RenderSystems { PostCleanup, } -/// The schedule that contains the app logic that is evaluated each tick -/// -/// This is highly inspired by [`bevy_app::Main`] -#[derive(ScheduleLabel, Clone, Debug, PartialEq, Eq, Hash, Default)] -pub struct MainRender; -impl MainRender { - pub fn run(world: &mut World, mut run_at_least_once: Local) { - if !*run_at_least_once { - let _ = world.try_run_schedule(RenderStartup); - *run_at_least_once = true; - } - - let _ = world.try_run_schedule(Render); - } -} - /// Deprecated alias for [`RenderSystems`]. #[deprecated(since = "0.17.0", note = "Renamed to `RenderSystems`.")] pub type RenderSet = RenderSystems; @@ -561,7 +545,7 @@ unsafe fn initialize_render_app(app: &mut App) { app.init_resource::(); let mut render_app = SubApp::new(); - render_app.update_schedule = Some(MainRender.intern()); + render_app.update_schedule = Some(Render.intern()); let mut extract_schedule = Schedule::new(ExtractSchedule); // We skip applying any commands during the ExtractSchedule @@ -576,7 +560,6 @@ unsafe fn initialize_render_app(app: &mut App) { .add_schedule(extract_schedule) .add_schedule(Render::base_schedule()) .init_resource::() - .add_systems(MainRender, MainRender::run) .insert_resource(app.world().resource::().clone()) .add_systems(ExtractSchedule, PipelineCache::extract_shaders) .add_systems( @@ -592,7 +575,19 @@ unsafe fn initialize_render_app(app: &mut App) { ), ); - render_app.set_extract(|main_world, render_world| { + // We want the closure to have a flag to only run the RenderStartup schedule once, but the only + // way to have the closure store this flag is by capturing it. This variable is otherwise + // unused. + let mut should_run_startup = true; + render_app.set_extract(move |main_world, render_world| { + if should_run_startup { + // Run the `RenderStartup` if it hasn't run yet. This does mean `RenderStartup` blocks + // the rest of the app extraction, but this is necessary since extraction itself can + // depend on resources initialized in `RenderStartup`. + render_world.run_schedule(RenderStartup); + should_run_startup = false; + } + { #[cfg(feature = "trace")] let _stage_span = tracing::info_span!("entity_sync").entered(); diff --git a/release-content/migration-guides/extract_fn_is_mut.md b/release-content/migration-guides/extract_fn_is_mut.md new file mode 100644 index 0000000000..a27db69fc0 --- /dev/null +++ b/release-content/migration-guides/extract_fn_is_mut.md @@ -0,0 +1,10 @@ +--- +title: `take_extract` now returns `dyn FnMut` instead of `dyn Fn`. +pull_requests: [19926] +--- + +Previously, `set_extract` accepted any `Fn`. Now we accept any `FnMut`. For callers of +`set_extract`, there is no difference since `Fn: FnMut`. + +However, callers of `take_extract` will now be returned +`Option>` instead.