Schedule build pass

[PixelDust22]

Objective

This is a follow up to #9822, which automatically adds sync points during the Schedule build process.

However, the implementation in #9822 feels very "special case" to me. As the number of things we want to do with the Schedule grows, we need a modularized way to manage those behaviors. For example, in one of my current experiments I want to automatically add systems to apply GPU pipeline barriers between systems accessing GPU resources.

For dynamic modifications of the schedule, we mostly need these capabilities:

• Storing custom data on schedule edges
• Storing custom data on schedule nodes
• Modify the schedule graph whenever it builds

These should be enough to allows us to add "hooks" to the schedule build process for various reasons.

cc @hymm

Solution

This PR abstracts the process of schedule modification and created a new trait, ScheduleBuildPass. Most of the logics in #9822 were moved to an implementation of ScheduleBuildPass, AutoInsertApplyDeferredPass.

Whether a dependency edge should "ignore deferred" is now indicated by the presence of a marker struct, IgnoreDeferred.

This PR has no externally visible effects. However, in a future PR I propose to change the before_ignore_deferred and after_ignore_deferred API into a more general form, before_with_options and after_with_options.

schedule.add_systems(
    system.before_with_options(another_system, IgnoreDeferred)
);

schedule.add_systems(
    system.before_with_options(another_system, (
        IgnoreDeferred,
        AnyOtherOption {
            key: value
        }
    ))
);

schedule.add_systems(
    system.before_with_options(another_system, ())
);

[alice-i-cecile]

Broadly in favor of generalizing this into a series of passes to modularize the API and add more expressive tools.

I'm quite reluctant about the proposed future API using tuples for config: I think it's much more complex to reason about both internally and externally. I'd much rather have a simple config struct if possible. Perhaps it will become clearer to me as more documentation is added?

[PixelDust22]

@alice-i-cecile
The proposed future API uses a tuple of config structs because the passes added to the schedule are not known ahead of time. For example, as of right now the only config we may have on the dependency edges is "ignore deferred". So your API will look like this.

schedule.add_systems(
    system.before_with_options(another_system, DependencyOptions {
        ignore_deferred: true,
    })
);

However, an external plugin adds a different ScheduleBuildPass: AutoAddPipelineBarrierPass. Then you're left with no way to add those additional options to DependencyOptions.

schedule.add_systems(
    system.before_with_options(another_system, DependencyOptions {
        ignore_deferred: true,
        // Can't add more options here! Not extensible. Sucks!
    })
);

The natural solution is providing those options as a tuple when adding dependency edges

schedule.add_systems(
    system.before_with_options(another_system, (
        IgnoreDeferred, // Provided by AutoInsertApplyDeferredPass in bevy core
        AutoAddPipelineBarrierOptions {
            prev_stages: vk::PipelineStages::Compute,
            next_stages: vk::PipelineStages::Transfer,
        } // Provided by `AutoAddPipelineBarrierPass` from a third party plugin. Extensible. Good!
    ))
);

[alice-i-cecile]

This should have a doc test or link to a type that has a doc test :)

[PixelDust22]

We only add this inside after_ignore_deferred and before_ignore_deferred. I intend to remove these two APIs and replace them with the API I described above. I can certainly add a doc test in that PR.

[alice-i-cecile]

Can we use a plain BTreeMap here? The order of the schedule passes surely matters once we have more complex passes. While the iteration order of a BTree is stable, I don't think it's able to be controlled in the way that we might need.

[PixelDust22]

Not sure what it means? We're already using a BTreeMap here.

We'd certainly want to control the ordering of those passes but I would defer that to a future PR when such a need arises.

For now, the reason we use a BTreeMap instead of a Vec here is that a BTreeMap makes it easier to remove passes and it guarantees unique instantiation of passes based on type, ensuring that the same type won't be accidentally added twice.

[alice-i-cecile]

I meant "is it correct to use a BTreeMap". I'm definitely fine to leave that until a later PR though.

[alice-i-cecile]

The documentation was extremely helpful, thank you! I suspected that we couldn't use a simple config struct due to extensibility, but I was having trouble putting together the pieces.

[PixelDust22]

A bit delayed because I wanted to make sure that the PR is suitable for my use case. But now it should be ready to merge. Resolved all merge conflicts. No major changes required from the initial implementation.

[hymm]

why does this now need &mut World access?

[PixelDust22]

Because custom schedule build passes will frequently need &mut World access to be useful. This allows us to pass the &mut World all the way down. The custom schedule build pass may want to insert resources into the world for these systems.

[hymm]

I'm a little worried about the change to &mut World needed for building schedules. Since schedule building is a bit expensive, I was hoping to be able to move it into async tasks. But if that becomes an issue we can maybe switch to passing a command queue in the future.