Skip to content
Closed
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
b411da6
Flatten EntityPhaseItem into PhaseItem
james7132 Dec 7, 2022
702b2c4
Add WorldQuery associated types to RenderCommand
james7132 Dec 7, 2022
2c099b1
Pass WorldQuery into RenderCommand
james7132 Dec 8, 2022
8e4f9ff
Flatten EntityRenderCommand out
james7132 Dec 8, 2022
45f227a
Cleanup
james7132 Dec 8, 2022
03d6ca3
Fix CI
james7132 Dec 8, 2022
22fd3c7
Use QueryState::get_manual over get
james7132 Dec 8, 2022
4a5638f
Unwrap instead of return
james7132 Dec 8, 2022
1a4083a
Merge branch 'main' into flatten-render-commands
james7132 Dec 12, 2022
5efcff3
Formatting
james7132 Dec 12, 2022
54cd232
Merge branch 'main' into flatten-render-commands
james7132 Dec 14, 2022
1756709
Remove spurious files
james7132 Dec 14, 2022
aa146b3
Fix CI
james7132 Dec 14, 2022
68f4a36
Merge branch 'main' into flatten-render-commands
james7132 Dec 25, 2022
4d4d9b4
Update Draw::prepare documentation
james7132 Dec 25, 2022
c09c358
WorldQuery -> ItemWorldQUery
james7132 Dec 25, 2022
f63f670
Fix CI
james7132 Dec 26, 2022
d034b09
Merge branch 'main' into flatten-render-commands
james7132 Jan 3, 2023
0a84aaa
Fix CI
james7132 Jan 3, 2023
592dd8e
Add SystemState::get_manual(_mut)
james7132 Jan 3, 2023
ef78c5b
Merge branch 'main' into less-atomic-render-commands
james7132 Jan 4, 2023
0a79eec
Add doc comment for `validate_world`
james7132 Jan 9, 2023
266583c
Merge branch 'main' into less-atomic-render-commands
james7132 Jan 17, 2023
84a2d7f
Merge branch 'less-atomic-render-commands' of github.com:james7132/be…
james7132 Jan 17, 2023
b90341e
Improve wording on get_manual variant's docs
james7132 Jan 17, 2023
ce70df9
Document update_archetypes
james7132 Jan 17, 2023
15b8527
Formatting
james7132 Jan 17, 2023
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
62 changes: 59 additions & 3 deletions crates/bevy_ecs/src/system/function_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,15 +170,17 @@ impl<Param: SystemParam> SystemState<Param> {
where
Param: ReadOnlySystemParam,
{
self.validate_world_and_update_archetypes(world);
self.validate_world(world);
self.update_archetypes(world);
// SAFETY: Param is read-only and doesn't allow mutable access to World. It also matches the World this SystemState was created with.
unsafe { self.get_unchecked_manual(world) }
}

/// Retrieve the mutable [`SystemParam`] values.
#[inline]
pub fn get_mut<'w, 's>(&'s mut self, world: &'w mut World) -> SystemParamItem<'w, 's, Param> {
self.validate_world_and_update_archetypes(world);
self.validate_world(world);
self.update_archetypes(world);
// SAFETY: World is uniquely borrowed and matches the World this SystemState was created with.
unsafe { self.get_unchecked_manual(world) }
}
Expand All @@ -196,8 +198,13 @@ impl<Param: SystemParam> SystemState<Param> {
self.world_id == world.id()
}

fn validate_world_and_update_archetypes(&mut self, world: &World) {
#[inline]
fn validate_world(&mut self, world: &World) {
assert!(self.matches_world(world), "Encountered a mismatched World. A SystemState cannot be used with Worlds other than the one it was created with.");
}

#[inline]
pub fn update_archetypes(&mut self, world: &World) {
let archetypes = world.archetypes();
let new_generation = archetypes.generation();
let old_generation = std::mem::replace(&mut self.archetype_generation, new_generation);
Expand All @@ -211,6 +218,42 @@ impl<Param: SystemParam> SystemState<Param> {
}
}

/// Retrieve the [`SystemParam`] values. This can only be called when all parameters are read-only.
/// This will not update archetypes automatically nor increment the world's change tick.
///
/// For this to return accurate results, ensure [`SystemState::update_archetypes`] is called before this
/// function.
Comment on lines +233 to +234
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In what way will the results be inaccurate? Is it just that any newly-added archetypes will be skipped? Not suggesting a change necessarily, just asking why this function is safe.

Copy link
Member Author

@james7132 james7132 Jan 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New archetypes are ignored. Queries, for example, will just not return the target entity in those new archetypes. The prior archetypes are still accessed correctly, so this shouldn't cause any UB if not updated.

///
/// Users should strongly prefer to use [`SystemState::get`] over this function.
#[inline]
pub fn get_manual<'w, 's>(&'s mut self, world: &'w World) -> SystemParamItem<'w, 's, Param>
where
Param: ReadOnlySystemParam,
{
self.validate_world(world);
let change_tick = world.read_change_tick();
// SAFETY: Param is read-only and doesn't allow mutable access to World. It also matches the World this SystemState was created with.
unsafe { self.fetch(world, change_tick) }
}

/// Retrieve the mutable [`SystemParam`] values. This will not update archetypes automatically nor increment
/// the world's change tick.
///
/// For this to return accurate results, ensure [`SystemState::update_archetypes`] is called before this
/// function.
///
/// Users should strongly prefer to use [`SystemState::get_mut`] over this function.
#[inline]
pub fn get_manual_mut<'w, 's>(
&'s mut self,
world: &'w mut World,
) -> SystemParamItem<'w, 's, Param> {
self.validate_world(world);
let change_tick = world.change_tick();
// SAFETY: World is uniquely borrowed and matches the World this SystemState was created with.
unsafe { self.fetch(world, change_tick) }
}

/// Retrieve the [`SystemParam`] values. This will not update archetypes automatically.
///
/// # Safety
Expand All @@ -223,6 +266,19 @@ impl<Param: SystemParam> SystemState<Param> {
world: &'w World,
) -> SystemParamItem<'w, 's, Param> {
let change_tick = world.increment_change_tick();
self.fetch(world, change_tick)
}

/// # Safety
/// This call might access any of the input parameters in a way that violates Rust's mutability rules. Make sure the data
/// access is safe in the context of global [`World`] access. The passed-in [`World`] _must_ be the [`World`] the [`SystemState`] was
/// created with.
#[inline]
unsafe fn fetch<'w, 's>(
&'s mut self,
world: &'w World,
change_tick: u32,
) -> SystemParamItem<'w, 's, Param> {
let param = <Param::State as SystemParamState>::get_param(
&mut self.param_state,
&self.meta,
Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_render/src/render_phase/draw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,7 @@ where
C::Param: ReadOnlySystemParam,
{
fn prepare(&mut self, world: &'_ World) {
self.state.update_archetypes(world);
self.view.update_archetypes(world);
self.entity.update_archetypes(world);
}
Expand All @@ -337,7 +338,7 @@ where
view: Entity,
item: &P,
) {
let param = self.state.get(world);
let param = self.state.get_manual(world);
let view = self.view.get_manual(world, view).unwrap();
let entity = self.entity.get_manual(world, item.entity()).unwrap();
C::render(item, view, entity, param, pass);
Expand Down