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.

Author: andriyDev

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<dyn Fn(&mut World, &mut World) + Send>;
+type ExtractFn = Box<dyn FnMut(&mut World, &mut World) + Send>;
 
 /// 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<F>(&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);
     ///     }
     ///

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<bool>) {
-        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::<ScratchMainWorld>();
 
     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::<render_graph::RenderGraph>()
-        .add_systems(MainRender, MainRender::run)
         .insert_resource(app.world().resource::<AssetServer>().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();

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<Box<dyn FnMut(&mut World, &mut World) + Send>>` instead.