resolve all internal ambiguities (#10411)

- ignore all ambiguities that are not a problem
- remove `.before(Assets::<Image>::track_assets),` that points into a
different schedule (-> should this be caught?)
- add some explicit orderings:
- run `poll_receivers` and `update_accessibility_nodes` after
`window_closed` in `bevy_winit::accessibility`
  - run `bevy_ui::accessibility::calc_bounds` after `CameraUpdateSystem`
- run ` bevy_text::update_text2d_layout` and `bevy_ui::text_system`
after `font_atlas_set::remove_dropped_font_atlas_sets`
- add `app.ignore_ambiguity(a, b)` function for cases where you want to
ignore an ambiguity between two independent plugins `A` and `B`
- add `IgnoreAmbiguitiesPlugin` in `DefaultPlugins` that allows
cross-crate ambiguities like `bevy_animation`/`bevy_ui`
- Fixes https://github.com/bevyengine/bevy/issues/9511

## Before
**Render**
![render_schedule_Render
dot](https://github.com/bevyengine/bevy/assets/22177966/1c677968-7873-40cc-848c-91fca4c8e383)

**PostUpdate**
![schedule_PostUpdate
dot](https://github.com/bevyengine/bevy/assets/22177966/8fc61304-08d4-4533-8110-c04113a7367a)

## After
**Render**
![render_schedule_Render
dot](https://github.com/bevyengine/bevy/assets/22177966/462f3b28-cef7-4833-8619-1f5175983485)
**PostUpdate**
![schedule_PostUpdate
dot](https://github.com/bevyengine/bevy/assets/22177966/8cfb3d83-7842-4a84-9082-46177e1a6c70)

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Alice Cecile <alice.i.cecil@gmail.com>
Co-authored-by: François <mockersf@gmail.com>
This commit is contained in:
Jakob Hellermann 2024-01-09 20:08:15 +01:00 committed by GitHub
parent 13d3de8ee1
commit a657478675
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
19 changed files with 199 additions and 28 deletions

View file

@ -105,6 +105,7 @@ impl Plugin for AccessibilityPlugin {
fn build(&self, app: &mut bevy_app::App) {
app.init_resource::<AccessibilityRequested>()
.init_resource::<ManageAccessibilityUpdates>()
.init_resource::<Focus>();
.init_resource::<Focus>()
.allow_ambiguous_component::<AccessibilityNode>();
}
}

View file

@ -1000,6 +1000,38 @@ impl App {
self.world.allow_ambiguous_resource::<T>();
self
}
/// Suppress warnings and errors that would result from systems in these sets having ambiguities
/// (conflicting access but indeterminate order) with systems in `set`.
///
/// When possible, do this directly in the `.add_systems(Update, a.ambiguous_with(b))` call.
/// However, sometimes two independant plugins `A` and `B` are reported as ambiguous, which you
/// can only supress as the consumer of both.
#[track_caller]
pub fn ignore_ambiguity<M1, M2, S1, S2>(
&mut self,
schedule: impl ScheduleLabel,
a: S1,
b: S2,
) -> &mut Self
where
S1: IntoSystemSet<M1>,
S2: IntoSystemSet<M2>,
{
let schedule = schedule.intern();
let mut schedules = self.world.resource_mut::<Schedules>();
if let Some(schedule) = schedules.get_mut(schedule) {
let schedule: &mut Schedule = schedule;
schedule.ignore_ambiguity(a, b);
} else {
let mut new_schedule = Schedule::new(schedule);
new_schedule.ignore_ambiguity(a, b);
schedules.insert(new_schedule);
}
self
}
}
fn run_once(mut app: App) {

View file

@ -291,7 +291,7 @@ pub(crate) fn audio_output_available(audio_output: Res<AudioOutput>) -> bool {
/// Updates spatial audio sinks when emitter positions change.
pub(crate) fn update_emitter_positions(
mut emitters: Query<(&mut GlobalTransform, &SpatialAudioSink), Changed<GlobalTransform>>,
mut emitters: Query<(&GlobalTransform, &SpatialAudioSink), Changed<GlobalTransform>>,
spatial_scale: Res<SpatialScale>,
) {
for (transform, sink) in emitters.iter_mut() {

View file

@ -20,6 +20,10 @@ pub struct BlitPlugin;
impl Plugin for BlitPlugin {
fn build(&self, app: &mut App) {
load_internal_asset!(app, BLIT_SHADER_HANDLE, "blit.wgsl", Shader::from_wgsl);
if let Ok(render_app) = app.get_sub_app_mut(RenderApp) {
render_app.allow_ambiguous_resource::<SpecializedRenderPipelines<BlitPipeline>>();
}
}
fn finish(&self, app: &mut App) {

View file

@ -244,6 +244,37 @@ impl Schedule {
self
}
/// Suppress warnings and errors that would result from systems in these sets having ambiguities
/// (conflicting access but indeterminate order) with systems in `set`.
#[track_caller]
pub fn ignore_ambiguity<M1, M2, S1, S2>(&mut self, a: S1, b: S2) -> &mut Self
where
S1: IntoSystemSet<M1>,
S2: IntoSystemSet<M2>,
{
let a = a.into_system_set();
let b = b.into_system_set();
let Some(&a_id) = self.graph.system_set_ids.get(&a.intern()) else {
panic!(
"Could not mark system as ambiguous, `{:?}` was not found in the schedule.
Did you try to call `ambiguous_with` before adding the system to the world?",
a
);
};
let Some(&b_id) = self.graph.system_set_ids.get(&b.intern()) else {
panic!(
"Could not mark system as ambiguous, `{:?}` was not found in the schedule.
Did you try to call `ambiguous_with` before adding the system to the world?",
b
);
};
self.graph.ambiguous_with.add_edge(a_id, b_id, ());
self
}
/// Configures a collection of system sets in this schedule, adding them if they does not exist.
#[track_caller]
pub fn configure_sets(&mut self, sets: impl IntoSystemSetConfigs) -> &mut Self {

View file

@ -15,6 +15,17 @@
//!
//! See the documentation on [`Gizmos`] for more examples.
/// Label for the the render systems handling the
#[derive(SystemSet, Clone, Debug, Hash, PartialEq, Eq)]
pub enum GizmoRenderSystem {
/// Adds gizmos to the [`Transparent2d`](bevy_core_pipeline::core_2d::Transparent2d) render phase
#[cfg(feature = "bevy_sprite")]
QueueLineGizmos2d,
/// Adds gizmos to the [`Transparent3d`](bevy_core_pipeline::core_3d::Transparent3d) render phase
#[cfg(feature = "bevy_pbr")]
QueueLineGizmos3d,
}
pub mod arcs;
pub mod arrows;
pub mod circles;
@ -40,7 +51,7 @@ use bevy_ecs::{
entity::Entity,
query::{ROQueryItem, Without},
reflect::{ReflectComponent, ReflectResource},
schedule::IntoSystemConfigs,
schedule::{IntoSystemConfigs, SystemSet},
system::{
lifetimeless::{Read, SRes},
Commands, Query, Res, ResMut, Resource, SystemParamItem,

View file

@ -1,5 +1,5 @@
use crate::{
line_gizmo_vertex_buffer_layouts, DrawLineGizmo, GizmoConfig, LineGizmo,
line_gizmo_vertex_buffer_layouts, DrawLineGizmo, GizmoConfig, GizmoRenderSystem, LineGizmo,
LineGizmoUniformBindgroupLayout, SetLineGizmoBindGroup, LINE_SHADER_HANDLE,
};
use bevy_app::{App, Plugin};
@ -8,7 +8,7 @@ use bevy_core_pipeline::core_2d::Transparent2d;
use bevy_ecs::{
prelude::Entity,
schedule::IntoSystemConfigs,
schedule::{IntoSystemConfigs, IntoSystemSetConfigs},
system::{Query, Res, ResMut, Resource},
world::{FromWorld, World},
};
@ -34,10 +34,14 @@ impl Plugin for LineGizmo2dPlugin {
render_app
.add_render_command::<Transparent2d, DrawLineGizmo2d>()
.init_resource::<SpecializedRenderPipelines<LineGizmoPipeline>>()
.configure_sets(
Render,
GizmoRenderSystem::QueueLineGizmos2d.in_set(RenderSet::Queue),
)
.add_systems(
Render,
queue_line_gizmos_2d
.in_set(RenderSet::Queue)
.in_set(GizmoRenderSystem::QueueLineGizmos2d)
.after(prepare_assets::<LineGizmo>),
);
}

View file

@ -1,5 +1,5 @@
use crate::{
line_gizmo_vertex_buffer_layouts, DrawLineGizmo, GizmoConfig, LineGizmo,
line_gizmo_vertex_buffer_layouts, DrawLineGizmo, GizmoConfig, GizmoRenderSystem, LineGizmo,
LineGizmoUniformBindgroupLayout, SetLineGizmoBindGroup, LINE_SHADER_HANDLE,
};
use bevy_app::{App, Plugin};
@ -12,7 +12,7 @@ use bevy_core_pipeline::{
use bevy_ecs::{
prelude::Entity,
query::Has,
schedule::IntoSystemConfigs,
schedule::{IntoSystemConfigs, IntoSystemSetConfigs},
system::{Query, Res, ResMut, Resource},
world::{FromWorld, World},
};
@ -36,10 +36,14 @@ impl Plugin for LineGizmo3dPlugin {
render_app
.add_render_command::<Transparent3d, DrawLineGizmo3d>()
.init_resource::<SpecializedRenderPipelines<LineGizmoPipeline>>()
.configure_sets(
Render,
GizmoRenderSystem::QueueLineGizmos3d.in_set(RenderSet::Queue),
)
.add_systems(
Render,
queue_line_gizmos_3d
.in_set(RenderSet::Queue)
.in_set(GizmoRenderSystem::QueueLineGizmos3d)
.after(prepare_assets::<LineGizmo>),
);
}

View file

@ -1,4 +1,4 @@
use bevy_app::{PluginGroup, PluginGroupBuilder};
use bevy_app::{Plugin, PluginGroup, PluginGroupBuilder};
/// This plugin group will add all the default plugins for a *Bevy* application:
/// * [`LogPlugin`](crate::log::LogPlugin)
@ -133,10 +133,52 @@ impl PluginGroup for DefaultPlugins {
group = group.add(bevy_gizmos::GizmoPlugin);
}
group = group.add(IgnoreAmbiguitiesPlugin);
group
}
}
struct IgnoreAmbiguitiesPlugin;
impl Plugin for IgnoreAmbiguitiesPlugin {
#[allow(unused_variables)] // Variables are used depending on enabled features
fn build(&self, app: &mut bevy_app::App) {
// bevy_ui owns the Transform and cannot be animated
#[cfg(all(feature = "bevy_animation", feature = "bevy_ui"))]
app.ignore_ambiguity(
bevy_app::PostUpdate,
bevy_animation::animation_player,
bevy_ui::ui_layout_system,
);
#[cfg(feature = "bevy_render")]
if let Ok(render_app) = app.get_sub_app_mut(bevy_render::RenderApp) {
#[cfg(all(feature = "bevy_gizmos", feature = "bevy_sprite"))]
{
render_app.ignore_ambiguity(
bevy_render::Render,
bevy_gizmos::GizmoRenderSystem::QueueLineGizmos2d,
bevy_sprite::queue_sprites,
);
render_app.ignore_ambiguity(
bevy_render::Render,
bevy_gizmos::GizmoRenderSystem::QueueLineGizmos2d,
bevy_sprite::queue_material2d_meshes::<bevy_sprite::ColorMaterial>,
);
}
#[cfg(all(feature = "bevy_gizmos", feature = "bevy_pbr"))]
{
render_app.ignore_ambiguity(
bevy_render::Render,
bevy_gizmos::GizmoRenderSystem::QueueLineGizmos3d,
bevy_pbr::queue_material_meshes::<bevy_pbr::StandardMaterial>,
);
}
}
}
}
/// This plugin group will add the minimal plugins for a *Bevy* application:
/// * [`TaskPoolPlugin`](crate::core::TaskPoolPlugin)
/// * [`TypeRegistrationPlugin`](crate::core::TypeRegistrationPlugin)

View file

@ -361,6 +361,15 @@ impl Plugin for PbrPlugin {
draw_3d_graph::node::SHADOW_PASS,
bevy_core_pipeline::core_3d::graph::node::START_MAIN_PASS,
);
render_app.ignore_ambiguity(
bevy_render::Render,
bevy_core_pipeline::core_3d::prepare_core_3d_transmission_textures,
bevy_render::batching::batch_and_prepare_render_phase::<
bevy_core_pipeline::core_3d::Transmissive3d,
MeshPipeline,
>,
);
}
fn finish(&self, app: &mut App) {

View file

@ -234,7 +234,9 @@ where
.after(prepare_materials::<M>),
queue_material_meshes::<M>
.in_set(RenderSet::QueueMeshes)
.after(prepare_materials::<M>),
.after(prepare_materials::<M>)
// queue_material_meshes only writes to `material_bind_group_id`, which `queue_shadows` doesn't read
.ambiguous_with(render::queue_shadows::<M>),
),
);
}

View file

@ -98,6 +98,7 @@ where
)
.init_resource::<PrepassViewBindGroup>()
.init_resource::<SpecializedMeshPipelines<PrepassPipeline<M>>>()
.allow_ambiguous_resource::<SpecializedMeshPipelines<PrepassPipeline<M>>>()
.init_resource::<PreviousViewProjectionUniforms>();
}
@ -168,7 +169,9 @@ where
Render,
queue_prepass_material_meshes::<M>
.in_set(RenderSet::QueueMeshes)
.after(prepare_materials::<M>),
.after(prepare_materials::<M>)
// queue_material_meshes only writes to `material_bind_group_id`, which `queue_prepass_material_meshes` doesn't read
.ambiguous_with(queue_material_meshes::<StandardMaterial>),
);
}
}

View file

@ -130,6 +130,7 @@ impl Plugin for MeshRenderPlugin {
.init_resource::<SkinIndices>()
.init_resource::<MorphUniform>()
.init_resource::<MorphIndices>()
.allow_ambiguous_resource::<GpuArrayBuffer<MeshUniform>>()
.add_systems(
ExtractSchedule,
(extract_meshes, extract_skins, extract_morphs),

View file

@ -61,7 +61,8 @@ impl Plugin for ViewPlugin {
prepare_view_targets
.in_set(RenderSet::ManageViews)
.after(prepare_windows)
.after(crate::render_asset::prepare_assets::<Image>),
.after(crate::render_asset::prepare_assets::<Image>)
.ambiguous_with(crate::camera::sort_cameras), // doesn't use `sorted_camera_index_for_target`
prepare_view_uniforms.in_set(RenderSet::PrepareResources),
),
);

View file

@ -94,6 +94,7 @@ impl Plugin for TextPlugin {
PostUpdate,
(
update_text2d_layout
.after(font_atlas_set::remove_dropped_font_atlas_sets)
// Potential conflict: `Assets<Image>`
// In practice, they run independently since `bevy_render::camera_update_system`
// will only ever observe its own render target, and `update_text2d_layout`

View file

@ -15,7 +15,7 @@ use bevy_ecs::{
world::Ref,
};
use bevy_hierarchy::Children;
use bevy_render::prelude::Camera;
use bevy_render::{camera::CameraUpdateSystem, prelude::Camera};
use bevy_text::Text;
use bevy_transform::prelude::GlobalTransform;
@ -150,7 +150,12 @@ impl Plugin for AccessibilityPlugin {
app.add_systems(
PostUpdate,
(
calc_bounds.after(bevy_transform::TransformSystem::TransformPropagate),
calc_bounds
.after(bevy_transform::TransformSystem::TransformPropagate)
.after(CameraUpdateSystem)
// the listed systems do not affect calculated size
.ambiguous_with(crate::resolve_outlines_system)
.ambiguous_with(crate::ui_stack_system),
button_changed,
image_changed,
label_changed,

View file

@ -45,10 +45,9 @@ use crate::prelude::UiCameraConfig;
#[cfg(feature = "bevy_text")]
use crate::widget::TextFlags;
use bevy_app::prelude::*;
use bevy_asset::Assets;
use bevy_ecs::prelude::*;
use bevy_input::InputSystem;
use bevy_render::{extract_component::ExtractComponentPlugin, texture::Image, RenderApp};
use bevy_render::{extract_component::ExtractComponentPlugin, RenderApp};
use bevy_transform::TransformSystem;
use stack::ui_stack_system;
pub use stack::UiStack;
@ -152,16 +151,26 @@ impl Plugin for UiPlugin {
// Potential conflict: `Assets<Image>`
// Since both systems will only ever insert new [`Image`] assets,
// they will never observe each other's effects.
.ambiguous_with(bevy_text::update_text2d_layout),
.ambiguous_with(bevy_text::update_text2d_layout)
// We assume Text is on disjoint UI entities to UiImage and UiTextureAtlasImage
// FIXME: Add an archetype invariant for this https://github.com/bevyengine/bevy/issues/1481.
.ambiguous_with(widget::update_image_content_size_system)
.ambiguous_with(widget::update_atlas_content_size_system),
widget::text_system
.after(UiSystem::Layout)
.before(Assets::<Image>::track_assets),
.after(bevy_text::remove_dropped_font_atlas_sets)
// Text2d and bevy_ui text are entirely on separate entities
.ambiguous_with(bevy_text::update_text2d_layout),
),
);
#[cfg(feature = "bevy_text")]
app.add_plugins(accessibility::AccessibilityPlugin);
app.add_systems(PostUpdate, {
let system = widget::update_image_content_size_system.before(UiSystem::Layout);
let system = widget::update_image_content_size_system
.before(UiSystem::Layout)
// We assume UiImage, UiTextureAtlasImage are disjoint UI entities
// FIXME: Add an archetype invariant for this https://github.com/bevyengine/bevy/issues/1481.
.ambiguous_with(widget::update_atlas_content_size_system);
// Potential conflicts: `Assets<Image>`
// They run independently since `widget::image_node_system` will only ever observe
// its own UiImage, and `widget::text_system` & `bevy_text::update_text2d_layout`
@ -185,8 +194,17 @@ impl Plugin for UiPlugin {
.before(TransformSystem::TransformPropagate),
resolve_outlines_system
.in_set(UiSystem::Outlines)
.after(UiSystem::Layout),
ui_stack_system.in_set(UiSystem::Stack),
.after(UiSystem::Layout)
// clipping doesn't care about outlines
.ambiguous_with(update_clipping_system)
.ambiguous_with(widget::text_system),
ui_stack_system
.in_set(UiSystem::Stack)
// the systems don't care about stack index
.ambiguous_with(update_clipping_system)
.ambiguous_with(resolve_outlines_system)
.ambiguous_with(ui_layout_system)
.ambiguous_with(widget::text_system),
update_clipping_system.after(TransformSystem::TransformPropagate),
),
);

View file

@ -74,6 +74,7 @@ pub fn build_ui_render(app: &mut App) {
.init_resource::<UiImageBindGroups>()
.init_resource::<UiMeta>()
.init_resource::<ExtractedUiNodes>()
.allow_ambiguous_resource::<ExtractedUiNodes>()
.init_resource::<DrawFunctions<TransparentUi>>()
.add_render_command::<TransparentUi, DrawUi>()
.add_systems(

View file

@ -187,12 +187,13 @@ impl Plugin for AccessKitPlugin {
.add_event::<ActionRequestWrapper>()
.add_systems(
PostUpdate,
(window_closed, poll_receivers).in_set(AccessibilitySystem::Update),
)
.add_systems(
PostUpdate,
update_accessibility_nodes
.run_if(should_update_accessibility_nodes)
(
poll_receivers,
update_accessibility_nodes.run_if(should_update_accessibility_nodes),
window_closed
.before(poll_receivers)
.before(update_accessibility_nodes),
)
.in_set(AccessibilitySystem::Update),
);
}