Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resolve all internal ambiguities #10411

Merged
merged 21 commits into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
d15ea7d
relax unnecessary &mut GlobalTransform to &GlobalTransform
jakobhellermann Nov 6, 2023
510ee3c
remove after relation to different schedule
jakobhellermann Nov 6, 2023
f468a76
refactor: merge add_systems
jakobhellermann Nov 6, 2023
6849743
run winit accessibility window_closed before poll_receivers and updat…
jakobhellermann Nov 6, 2023
bc59251
run bevy_ui::calc_bounds after CameraUpdateSystem
jakobhellermann Nov 6, 2023
efed113
ignore render schedule ambiguities
jakobhellermann Nov 6, 2023
b086945
ignore accessibility ambiguities
jakobhellermann Nov 6, 2023
8f72022
ignore ui/text ambiguities
jakobhellermann Nov 6, 2023
ef7b2a1
ignore ui ambiguities
jakobhellermann Nov 6, 2023
a6d3eb3
add standalone ambiguous_with function for acknowledging an ambiguity…
jakobhellermann Nov 6, 2023
c77396f
ignore core_pipeline/bevy_pbr ambiguity
jakobhellermann Nov 6, 2023
6e1975a
ignore bevy_animation/bevy_ui transform ambiguity
jakobhellermann Nov 6, 2023
64c4845
rename ambiguous_with -> ignore_ambiguities
jakobhellermann Nov 6, 2023
0643193
allow sets in ignore_ambiguities
jakobhellermann Nov 6, 2023
ce4bdb2
label gizmo queue sets and ignore ambiguity
jakobhellermann Nov 6, 2023
90698b2
rename ignore_ambiguities -> ignore_ambiguity
jakobhellermann Nov 6, 2023
87cc414
fix doc comment mentioning private type
jakobhellermann Nov 6, 2023
22a972d
Merge branch 'main' into resolve-ambiguities
alice-i-cecile Jan 1, 2024
f5bc124
cargo fmt
Jan 8, 2024
43e7d05
Only access the render app wtih bevy_render
alice-i-cecile Jan 9, 2024
be2c42d
Allow unused variables
Jan 9, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion crates/bevy_a11y/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>();
}
}
32 changes: 32 additions & 0 deletions crates/bevy_app/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -998,6 +998,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) {
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_audio/src/audio_output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
4 changes: 4 additions & 0 deletions crates/bevy_core_pipeline/src/blit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
31 changes: 31 additions & 0 deletions crates/bevy_ecs/src/schedule/schedule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
13 changes: 12 additions & 1 deletion crates/bevy_gizmos/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand Down
10 changes: 7 additions & 3 deletions crates/bevy_gizmos/src/pipeline_2d.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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},
};
Expand All @@ -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>),
);
}
Expand Down
10 changes: 7 additions & 3 deletions crates/bevy_gizmos/src/pipeline_3d.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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},
};
Expand All @@ -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>),
);
}
Expand Down
44 changes: 43 additions & 1 deletion crates/bevy_internal/src/default_plugins.rs
Original file line number Diff line number Diff line change
@@ -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)
Expand Down Expand Up @@ -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) {
alice-i-cecile marked this conversation as resolved.
Show resolved Hide resolved
#[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)
Expand Down
9 changes: 9 additions & 0 deletions crates/bevy_pbr/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,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) {
Expand Down
4 changes: 3 additions & 1 deletion crates/bevy_pbr/src/material.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>),
),
);
}
Expand Down
5 changes: 4 additions & 1 deletion crates/bevy_pbr/src/prepass/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ where
)
.init_resource::<PrepassViewBindGroup>()
.init_resource::<SpecializedMeshPipelines<PrepassPipeline<M>>>()
.allow_ambiguous_resource::<SpecializedMeshPipelines<PrepassPipeline<M>>>()
.init_resource::<PreviousViewProjectionUniforms>();
}

Expand Down Expand Up @@ -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>),
);
}
}
Expand Down
1 change: 1 addition & 0 deletions crates/bevy_pbr/src/render/mesh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,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),
Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_render/src/view/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
),
);
Expand Down
1 change: 1 addition & 0 deletions crates/bevy_text/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
9 changes: 7 additions & 2 deletions crates/bevy_ui/src/accessibility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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,
Expand Down
Loading