make gizmo groups ordering stable when rendering (#12034)

# Objective

- During rendering of different gizmo groups, the ordering is random
between executions

## Solution

- Make the ordering stable
- It's using a `TypeIdMap`, its iteration order depends on the insertion
order. so insert when adding a group instead of when adding a gizmo
- Also changed `extract_gizmo_data` to not be group dependent. there
will be only one of those systems, no matter the number of gizmo groups
added

---------

Co-authored-by: pablo-lua <126117294+pablo-lua@users.noreply.github.com>
This commit is contained in:
François 2024-02-26 16:55:26 +01:00 committed by GitHub
parent 1b1934f4fb
commit ebdda09fc3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

View File

@ -116,6 +116,8 @@ impl Plugin for GizmoPlugin {
prepare_line_gizmo_bind_group.in_set(RenderSet::PrepareBindGroups), prepare_line_gizmo_bind_group.in_set(RenderSet::PrepareBindGroups),
); );
render_app.add_systems(ExtractSchedule, extract_gizmo_data);
#[cfg(feature = "bevy_sprite")] #[cfg(feature = "bevy_sprite")]
app.add_plugins(pipeline_2d::LineGizmo2dPlugin); app.add_plugins(pipeline_2d::LineGizmo2dPlugin);
#[cfg(feature = "bevy_pbr")] #[cfg(feature = "bevy_pbr")]
@ -163,6 +165,12 @@ impl AppGizmoBuilder for App {
return self; return self;
} }
let mut handles = self
.world
.get_resource_or_insert_with::<LineGizmoHandles>(Default::default);
handles.list.insert(TypeId::of::<T>(), None);
handles.strip.insert(TypeId::of::<T>(), None);
self.init_resource::<GizmoStorage<T>>() self.init_resource::<GizmoStorage<T>>()
.add_systems(Last, update_gizmo_meshes::<T>); .add_systems(Last, update_gizmo_meshes::<T>);
@ -170,12 +178,6 @@ impl AppGizmoBuilder for App {
.get_resource_or_insert_with::<GizmoConfigStore>(Default::default) .get_resource_or_insert_with::<GizmoConfigStore>(Default::default)
.register::<T>(); .register::<T>();
let Ok(render_app) = self.get_sub_app_mut(RenderApp) else {
return self;
};
render_app.add_systems(ExtractSchedule, extract_gizmo_data::<T>);
self self
} }
@ -192,23 +194,28 @@ impl AppGizmoBuilder for App {
return self; return self;
} }
let mut handles = self
.world
.get_resource_or_insert_with::<LineGizmoHandles>(Default::default);
handles.list.insert(TypeId::of::<T>(), None);
handles.strip.insert(TypeId::of::<T>(), None);
self.init_resource::<GizmoStorage<T>>() self.init_resource::<GizmoStorage<T>>()
.add_systems(Last, update_gizmo_meshes::<T>); .add_systems(Last, update_gizmo_meshes::<T>);
let Ok(render_app) = self.get_sub_app_mut(RenderApp) else {
return self;
};
render_app.add_systems(ExtractSchedule, extract_gizmo_data::<T>);
self self
} }
} }
/// Holds handles to the line gizmos for each gizmo configuration group
// As `TypeIdMap` iteration order depends on the order of insertions and deletions, this uses
// `Option<Handle>` to be able to reserve the slot when creating the gizmo configuration group.
// That way iteration order is stable accross executions and depends on the order of configuration
// group creation.
#[derive(Resource, Default)] #[derive(Resource, Default)]
struct LineGizmoHandles { struct LineGizmoHandles {
list: TypeIdMap<Handle<LineGizmo>>, list: TypeIdMap<Option<Handle<LineGizmo>>>,
strip: TypeIdMap<Handle<LineGizmo>>, strip: TypeIdMap<Option<Handle<LineGizmo>>>,
} }
fn update_gizmo_meshes<T: GizmoConfigGroup>( fn update_gizmo_meshes<T: GizmoConfigGroup>(
@ -217,63 +224,66 @@ fn update_gizmo_meshes<T: GizmoConfigGroup>(
mut storage: ResMut<GizmoStorage<T>>, mut storage: ResMut<GizmoStorage<T>>,
) { ) {
if storage.list_positions.is_empty() { if storage.list_positions.is_empty() {
handles.list.remove(&TypeId::of::<T>()); handles.list.insert(TypeId::of::<T>(), None);
} else if let Some(handle) = handles.list.get(&TypeId::of::<T>()) { } else if let Some(handle) = handles.list.get_mut(&TypeId::of::<T>()) {
let list = line_gizmos.get_mut(handle).unwrap(); if let Some(handle) = handle {
let list = line_gizmos.get_mut(handle.id()).unwrap();
list.positions = mem::take(&mut storage.list_positions); list.positions = mem::take(&mut storage.list_positions);
list.colors = mem::take(&mut storage.list_colors); list.colors = mem::take(&mut storage.list_colors);
} else { } else {
let mut list = LineGizmo { let mut list = LineGizmo {
strip: false, strip: false,
..Default::default() ..Default::default()
}; };
list.positions = mem::take(&mut storage.list_positions); list.positions = mem::take(&mut storage.list_positions);
list.colors = mem::take(&mut storage.list_colors); list.colors = mem::take(&mut storage.list_colors);
handles *handle = Some(line_gizmos.add(list));
.list }
.insert(TypeId::of::<T>(), line_gizmos.add(list));
} }
if storage.strip_positions.is_empty() { if storage.strip_positions.is_empty() {
handles.strip.remove(&TypeId::of::<T>()); handles.strip.insert(TypeId::of::<T>(), None);
} else if let Some(handle) = handles.strip.get(&TypeId::of::<T>()) { } else if let Some(handle) = handles.strip.get_mut(&TypeId::of::<T>()) {
let strip = line_gizmos.get_mut(handle).unwrap(); if let Some(handle) = handle {
let strip = line_gizmos.get_mut(handle.id()).unwrap();
strip.positions = mem::take(&mut storage.strip_positions); strip.positions = mem::take(&mut storage.strip_positions);
strip.colors = mem::take(&mut storage.strip_colors); strip.colors = mem::take(&mut storage.strip_colors);
} else { } else {
let mut strip = LineGizmo { let mut strip = LineGizmo {
strip: true, strip: true,
..Default::default() ..Default::default()
}; };
strip.positions = mem::take(&mut storage.strip_positions); strip.positions = mem::take(&mut storage.strip_positions);
strip.colors = mem::take(&mut storage.strip_colors); strip.colors = mem::take(&mut storage.strip_colors);
handles *handle = Some(line_gizmos.add(strip));
.strip }
.insert(TypeId::of::<T>(), line_gizmos.add(strip));
} }
} }
fn extract_gizmo_data<T: GizmoConfigGroup>( fn extract_gizmo_data(
mut commands: Commands, mut commands: Commands,
handles: Extract<Res<LineGizmoHandles>>, handles: Extract<Res<LineGizmoHandles>>,
config: Extract<Res<GizmoConfigStore>>, config: Extract<Res<GizmoConfigStore>>,
) { ) {
let (config, _) = config.config::<T>(); for (group_type_id, handle) in handles.list.iter().chain(handles.strip.iter()) {
let Some((config, _)) = config.get_config_dyn(group_type_id) else {
if !config.enabled {
return;
}
for map in [&handles.list, &handles.strip].into_iter() {
let Some(handle) = map.get(&TypeId::of::<T>()) else {
continue; continue;
}; };
if !config.enabled {
continue;
}
let Some(handle) = handle else {
continue;
};
commands.spawn(( commands.spawn((
LineGizmoUniform { LineGizmoUniform {
line_width: config.line_width, line_width: config.line_width,