resolve all internal ambiguities (bevyengine#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 bevyengine#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 <[email protected]> Co-authored-by: Alice Cecile <[email protected]> Co-authored-by: François <[email protected]>
robtfm · Jan 9, 2024 · a657478 · a657478
1 parent 13d3de8
commit a657478
Show file tree

Hide file tree

Showing 19 changed files with 199 additions and 28 deletions.
diff --git a/crates/bevy_a11y/src/lib.rs b/crates/bevy_a11y/src/lib.rs
@@ -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>();
     }
 }
diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs
@@ -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) {

diff --git a/crates/bevy_audio/src/audio_output.rs b/crates/bevy_audio/src/audio_output.rs
@@ -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() {

diff --git a/crates/bevy_core_pipeline/src/blit/mod.rs b/crates/bevy_core_pipeline/src/blit/mod.rs
@@ -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) {

diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs
@@ -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 {

diff --git a/crates/bevy_gizmos/src/lib.rs b/crates/bevy_gizmos/src/lib.rs
@@ -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,

diff --git a/crates/bevy_gizmos/src/pipeline_2d.rs b/crates/bevy_gizmos/src/pipeline_2d.rs
@@ -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>),
             );
     }

diff --git a/crates/bevy_gizmos/src/pipeline_3d.rs b/crates/bevy_gizmos/src/pipeline_3d.rs
@@ -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>),
             );
     }

diff --git a/crates/bevy_internal/src/default_plugins.rs b/crates/bevy_internal/src/default_plugins.rs
@@ -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)

diff --git a/crates/bevy_pbr/src/lib.rs b/crates/bevy_pbr/src/lib.rs
@@ -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) {

diff --git a/crates/bevy_pbr/src/material.rs b/crates/bevy_pbr/src/material.rs
@@ -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>),
                     ),
                 );
         }

diff --git a/crates/bevy_pbr/src/prepass/mod.rs b/crates/bevy_pbr/src/prepass/mod.rs
@@ -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>),
             );
     }
 }

diff --git a/crates/bevy_pbr/src/render/mesh.rs b/crates/bevy_pbr/src/render/mesh.rs
@@ -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),

diff --git a/crates/bevy_render/src/view/mod.rs b/crates/bevy_render/src/view/mod.rs
@@ -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),
                 ),
             );

diff --git a/crates/bevy_text/src/lib.rs b/crates/bevy_text/src/lib.rs
@@ -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`

diff --git a/crates/bevy_ui/src/accessibility.rs b/crates/bevy_ui/src/accessibility.rs
@@ -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,