From 8ba6bdad66cb70f1f3d6ce99d8981134b387bab7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tudor=20Lechin=C8=9Ban?= Date: Sat, 7 Sep 2024 15:20:47 +0300 Subject: [PATCH] refactor: clean up component sparse set code --- examples/components.rs | 35 ------- src/component/component_set.rs | 16 +-- src/component/component_sparse_set.rs | 136 ++++++++++++-------------- 3 files changed, 75 insertions(+), 112 deletions(-) delete mode 100644 examples/components.rs diff --git a/examples/components.rs b/examples/components.rs deleted file mode 100644 index 6705ff3..0000000 --- a/examples/components.rs +++ /dev/null @@ -1,35 +0,0 @@ -use sparsey::component::GroupLayout; -use sparsey::World; - -pub struct Handler<'a>(Vec<(&'a mut Position,)>); - -impl Drop for Handler<'_> { - fn drop(&mut self) { - println!("{:#?}", self.0); - } -} - -#[derive(Clone, Copy, Debug)] -pub struct Position { - pub x: i32, - pub y: i32, -} - -#[derive(Clone, Copy, Debug)] -pub struct Speed { - pub x: i32, - pub y: i32, -} - -fn main() { - let layout = GroupLayout::builder() - .add_group::<(Position, Speed)>() - .build_layout(); - - let mut world = World::new(&layout); - world.create((Position { x: -1, y: -1 },)); - world.create((Position { x: 0, y: 0 }, Speed { x: 0, y: 0 })); - world.create((Position { x: 1, y: 1 }, Speed { x: 1, y: 1 })); - - dbg!(world.query_all::<&mut Position>().iter().is_dense()); -} diff --git a/src/component/component_set.rs b/src/component/component_set.rs index 67693f6..8691baa 100644 --- a/src/component/component_set.rs +++ b/src/component/component_set.rs @@ -4,6 +4,11 @@ use crate::World; use core::any::TypeId; /// Handles insert and remove operations for components stored in a [`World`]. +/// +/// # Safety +/// +/// This trait is considered an implementation detail and cannot be safely +/// implemented outside the crate. pub unsafe trait ComponentSet { /// The components returned by [`remove`](Self::remove) operations. type Remove; @@ -146,7 +151,7 @@ macro_rules! impl_component_set { },)*); unsafe { - if group_mask.0 != 0 { + if group_mask != GroupMask::EMPTY { ungroup( &mut world.components.components, &mut world.components.groups, @@ -183,7 +188,7 @@ macro_rules! impl_component_set { },)*); unsafe { - if group_mask.0 != 0 { + if group_mask != GroupMask::EMPTY { ungroup( &mut world.components.components, &mut world.components.groups, @@ -201,12 +206,11 @@ macro_rules! impl_component_set { }; } -#[allow(unused_variables)] unsafe impl ComponentSet for () { type Remove = (); #[inline(always)] - fn insert(world: &mut World, entity: Entity, components: Self) { + fn insert(_world: &mut World, _entity: Entity, _components: Self) { // Empty } @@ -224,12 +228,12 @@ unsafe impl ComponentSet for () { } #[inline(always)] - fn remove(world: &mut World, entity: Entity) -> Self::Remove { + fn remove(_world: &mut World, _entity: Entity) -> Self::Remove { // Empty } #[inline(always)] - fn delete(world: &mut World, entity: Entity) { + fn delete(_world: &mut World, _entity: Entity) { // Empty } } diff --git a/src/component/component_sparse_set.rs b/src/component/component_sparse_set.rs index 769376c..516e6f4 100644 --- a/src/component/component_sparse_set.rs +++ b/src/component/component_sparse_set.rs @@ -2,7 +2,7 @@ use crate::component::Component; use crate::entity::{DenseEntity, Entity, SparseVec}; use alloc::{alloc, Layout, LayoutError}; use core::ptr::NonNull; -use core::{fmt, mem, ptr, slice}; +use core::{fmt, mem, slice}; pub(crate) struct ComponentSparseSet { sparse: SparseVec, @@ -39,15 +39,9 @@ impl ComponentSparseSet { Some(dense_entity) => { let index = dense_entity.dense(); - self.entities.add(index).as_ptr().write(entity); - - Some( - self.components - .cast::() - .add(index) - .as_ptr() - .replace(component), - ) + // Replace existing entity and component. + *self.entities.add(index).as_mut() = entity; + Some(self.components.cast::().add(index).replace(component)) } None => { *dense_entity = Some(DenseEntity { @@ -59,13 +53,9 @@ impl ComponentSparseSet { self.grow(); } - self.entities.add(self.len).as_ptr().write(entity); - - self.components - .cast::() - .add(self.len) - .as_ptr() - .write(component); + // Write entity and component to uninitialized memory. + self.entities.add(self.len).write(entity); + self.components.cast::().add(self.len).write(component); self.len += 1; None @@ -90,11 +80,12 @@ impl ComponentSparseSet { }); } - let removed_ptr = self.components.cast::().add(index).as_ptr(); - let last_ptr = self.components.cast::().add(self.len).as_ptr(); + let removed_ptr = self.components.cast::().add(index); + let last_ptr = self.components.cast::().add(self.len); + // Replace removed component with last component. let component = removed_ptr.read(); - ptr::copy(last_ptr, removed_ptr, 1); + last_ptr.copy_to(removed_ptr, 1); Some(component) } @@ -119,11 +110,11 @@ impl ComponentSparseSet { }); } - let dropped_ptr = self.components.cast::().add(index).as_ptr(); + let dropped_ptr = self.components.cast::().add(index); dropped_ptr.drop_in_place(); - let last_ptr = self.components.cast::().add(self.len).as_ptr(); - ptr::copy(last_ptr, dropped_ptr, 1); + let last_ptr = self.components.cast::().add(self.len); + last_ptr.copy_to(dropped_ptr, 1); } #[inline] @@ -231,76 +222,72 @@ impl ComponentSparseSet { } } + #[cold] + #[inline(never)] unsafe fn grow_typed(&mut self) where T: Component, { + // Allocate new storage for entities and components. let (new_entities, new_components, new_cap) = { let new_cap = match self.cap { 0 => 4, - cap => cap.saturating_add(cap), + cap => { + let new_cap = cap.saturating_add(cap); + assert_ne!(new_cap, self.cap, "Cannot grow sparse set"); + new_cap + } }; - assert_ne!( - new_cap, self.cap, - "Cannot allocate space for more components", - ); - - let (new_layout, new_components_offset) = - Self::compute_layout::(new_cap).expect("Failed to compute new component layout"); + let (new_layout, new_components_offset) = Self::compute_layout::(new_cap); - let new_data = alloc::alloc(new_layout); - - if new_data.is_null() { + let Some(new_data) = NonNull::new(alloc::alloc(new_layout)) else { alloc::handle_alloc_error(new_layout); - } + }; - #[allow(clippy::cast_ptr_alignment)] ( - NonNull::new_unchecked(new_data.cast::()), - NonNull::new_unchecked(new_data.byte_add(new_components_offset)), + new_data.cast::(), + new_data.byte_add(new_components_offset).cast::(), new_cap, ) }; - ptr::copy_nonoverlapping(self.entities.as_ptr(), new_entities.as_ptr(), self.len); + // Copy old entities and components to new location. + self.entities.copy_to_nonoverlapping(new_entities, self.len); - ptr::copy_nonoverlapping::( - self.components.cast().as_ptr(), - new_components.cast().as_ptr(), - self.len, - ); + self.components + .cast::() + .copy_to_nonoverlapping(new_components, self.len); + // Deallocate old storage, if any. if self.cap != 0 { - let (layout, _) = Self::compute_layout::(self.cap).unwrap(); + let (layout, _) = Self::compute_layout::(self.cap); alloc::dealloc(self.entities.cast().as_ptr(), layout); } + // Update pointers and capacity. self.entities = new_entities; - self.components = new_components; + self.components = new_components.cast(); self.cap = new_cap; } - unsafe fn swap_typed(&mut self, a: usize, b: usize) + unsafe fn swap_typed(&mut self, dense_a: usize, dense_b: usize) where T: Component, { - debug_assert!(a < self.len); - debug_assert!(b < self.len); - debug_assert_ne!(a, b); - - let (sparse_a, sparse_b) = { - let entity_a = self.entities.add(a).as_mut(); - let entity_b = self.entities.add(b).as_mut(); - mem::swap(entity_a, entity_b); - - (entity_a.sparse(), entity_b.sparse()) - }; - - self.sparse.swap(sparse_a, sparse_b); - - let component_a = self.components.cast::().add(a).as_mut(); - let component_b = self.components.cast::().add(b).as_mut(); + debug_assert!(dense_a < self.len); + debug_assert!(dense_b < self.len); + debug_assert_ne!(dense_a, dense_b); + + // Swap entities. + let entity_a = self.entities.add(dense_a).as_mut(); + let entity_b = self.entities.add(dense_b).as_mut(); + self.sparse.swap(entity_a.sparse(), entity_b.sparse()); + mem::swap(entity_a, entity_b); + + // Swap components. + let component_a = self.components.cast::().add(dense_a).as_mut(); + let component_b = self.components.cast::().add(dense_b).as_mut(); mem::swap(component_a, component_b); } @@ -313,7 +300,7 @@ impl ComponentSparseSet { if mem::needs_drop::() { for i in 0..self.len { unsafe { - self.components.cast::().add(i).as_ptr().drop_in_place(); + self.components.cast::().add(i).drop_in_place(); } } } @@ -328,21 +315,28 @@ impl ComponentSparseSet { if mem::needs_drop::() { for i in 0..self.len { unsafe { - self.components.cast::().add(i).as_ptr().drop_in_place(); + self.components.cast::().add(i).drop_in_place(); } } } if self.cap != 0 { - let (layout, _) = Self::compute_layout::(self.cap).unwrap(); + let (layout, _) = Self::compute_layout::(self.cap); alloc::dealloc(self.entities.cast::().as_ptr(), layout); } } - fn compute_layout(cap: usize) -> Result<(Layout, usize), LayoutError> { - let entities_layout = Layout::array::(cap)?; - let components_layout = Layout::array::(cap)?; - entities_layout.extend(components_layout) + fn compute_layout(cap: usize) -> (Layout, usize) { + fn compute_layout_impl(cap: usize) -> Result<(Layout, usize), LayoutError> { + let entities_layout = Layout::array::(cap)?; + let components_layout = Layout::array::(cap)?; + entities_layout.extend(components_layout) + } + + match compute_layout_impl::(cap) { + Ok(result) => result, + Err(e) => panic!("Cannot compute sparse set data layout: {e}"), + } } } @@ -371,7 +365,7 @@ impl fmt::Debug for ComponentSparseSet { } } -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy)] struct ComponentSparseSetVtable { grow: unsafe fn(&mut ComponentSparseSet), swap: unsafe fn(&mut ComponentSparseSet, usize, usize),