Shrink ComputedVisibility (#6305)

# Objective
`ComputedVisibility` could afford to be smaller/faster. Optimizing the size and performance of operations on the component will positively benefit almost all extraction systems.

This was listed as one of the potential pieces of future work for #5310.

## Solution
Merge both internal booleans into a single `u8` bitflag field. Rely on bitmasks to evaluate local, hierarchical, and general visibility.

Pros:

 - `ComputedVisibility::is_visible` should be a single bitmask test instead of two.
 - `ComputedVisibility` is now only 1 byte. Should be able to fit 100% more per cache line when using dense iteration.

Cons:

 - Harder to read.
 - Setting individual values inside `ComputedVisiblity` require bitmask mutations. 

This should be a non-breaking change. No public API was changed. The only publicly visible effect is that `ComputedVisibility` is now 1 byte instead of 2.
This commit is contained in:
James Liu 2022-11-14 23:34:52 +00:00
parent 8ebd4d909c
commit 342f69e304

View File

@ -54,12 +54,19 @@ impl Visibility {
} }
} }
bitflags::bitflags! {
#[derive(Reflect)]
struct ComputedVisibilityFlags: u8 {
const VISIBLE_IN_VIEW = 1 << 0;
const VISIBLE_IN_HIERARCHY = 1 << 1;
}
}
/// Algorithmically-computed indication of whether an entity is visible and should be extracted for rendering /// Algorithmically-computed indication of whether an entity is visible and should be extracted for rendering
#[derive(Component, Clone, Reflect, Debug, Eq, PartialEq)] #[derive(Component, Clone, Reflect, Debug, Eq, PartialEq)]
#[reflect(Component, Default)] #[reflect(Component, Default)]
pub struct ComputedVisibility { pub struct ComputedVisibility {
is_visible_in_hierarchy: bool, flags: ComputedVisibilityFlags,
is_visible_in_view: bool,
} }
impl Default for ComputedVisibility { impl Default for ComputedVisibility {
@ -71,8 +78,7 @@ impl Default for ComputedVisibility {
impl ComputedVisibility { impl ComputedVisibility {
/// A [`ComputedVisibility`], set as invisible. /// A [`ComputedVisibility`], set as invisible.
pub const INVISIBLE: Self = ComputedVisibility { pub const INVISIBLE: Self = ComputedVisibility {
is_visible_in_hierarchy: false, flags: ComputedVisibilityFlags::empty(),
is_visible_in_view: false,
}; };
/// Whether this entity is visible to something this frame. This is true if and only if [`Self::is_visible_in_hierarchy`] and [`Self::is_visible_in_view`] /// Whether this entity is visible to something this frame. This is true if and only if [`Self::is_visible_in_hierarchy`] and [`Self::is_visible_in_view`]
@ -81,7 +87,7 @@ impl ComputedVisibility {
/// [`CoreStage::Update`] stage will yield the value from the previous frame. /// [`CoreStage::Update`] stage will yield the value from the previous frame.
#[inline] #[inline]
pub fn is_visible(&self) -> bool { pub fn is_visible(&self) -> bool {
self.is_visible_in_hierarchy && self.is_visible_in_view self.flags.bits == ComputedVisibilityFlags::all().bits
} }
/// Whether this entity is visible in the entity hierarchy, which is determined by the [`Visibility`] component. /// Whether this entity is visible in the entity hierarchy, which is determined by the [`Visibility`] component.
@ -90,7 +96,8 @@ impl ComputedVisibility {
/// [`VisibilitySystems::VisibilityPropagate`] system label. /// [`VisibilitySystems::VisibilityPropagate`] system label.
#[inline] #[inline]
pub fn is_visible_in_hierarchy(&self) -> bool { pub fn is_visible_in_hierarchy(&self) -> bool {
self.is_visible_in_hierarchy self.flags
.contains(ComputedVisibilityFlags::VISIBLE_IN_HIERARCHY)
} }
/// Whether this entity is visible in _any_ view (Cameras, Lights, etc). Each entity type (and view type) should choose how to set this /// Whether this entity is visible in _any_ view (Cameras, Lights, etc). Each entity type (and view type) should choose how to set this
@ -102,7 +109,8 @@ impl ComputedVisibility {
/// Other entities might just set this to `true` every frame. /// Other entities might just set this to `true` every frame.
#[inline] #[inline]
pub fn is_visible_in_view(&self) -> bool { pub fn is_visible_in_view(&self) -> bool {
self.is_visible_in_view self.flags
.contains(ComputedVisibilityFlags::VISIBLE_IN_VIEW)
} }
/// Sets `is_visible_in_view` to `true`. This is not reversible for a given frame, as it encodes whether or not this is visible in /// Sets `is_visible_in_view` to `true`. This is not reversible for a given frame, as it encodes whether or not this is visible in
@ -111,7 +119,16 @@ impl ComputedVisibility {
/// label. Don't call this unless you are defining a custom visibility system. For normal user-defined entity visibility, see [`Visibility`]. /// label. Don't call this unless you are defining a custom visibility system. For normal user-defined entity visibility, see [`Visibility`].
#[inline] #[inline]
pub fn set_visible_in_view(&mut self) { pub fn set_visible_in_view(&mut self) {
self.is_visible_in_view = true; self.flags.insert(ComputedVisibilityFlags::VISIBLE_IN_VIEW);
}
#[inline]
fn reset(&mut self, visible_in_hierarchy: bool) {
self.flags = if visible_in_hierarchy {
ComputedVisibilityFlags::VISIBLE_IN_HIERARCHY
} else {
ComputedVisibilityFlags::empty()
};
} }
} }
@ -280,13 +297,12 @@ fn visibility_propagate_system(
children_query: Query<&Children, (With<Parent>, With<Visibility>, With<ComputedVisibility>)>, children_query: Query<&Children, (With<Parent>, With<Visibility>, With<ComputedVisibility>)>,
) { ) {
for (children, visibility, mut computed_visibility, entity) in root_query.iter_mut() { for (children, visibility, mut computed_visibility, entity) in root_query.iter_mut() {
computed_visibility.is_visible_in_hierarchy = visibility.is_visible;
// reset "view" visibility here ... if this entity should be drawn a future system should set this to true // reset "view" visibility here ... if this entity should be drawn a future system should set this to true
computed_visibility.is_visible_in_view = false; computed_visibility.reset(visibility.is_visible);
if let Some(children) = children { if let Some(children) = children {
for child in children.iter() { for child in children.iter() {
let _ = propagate_recursive( let _ = propagate_recursive(
computed_visibility.is_visible_in_hierarchy, computed_visibility.is_visible_in_hierarchy(),
&mut visibility_query, &mut visibility_query,
&children_query, &children_query,
*child, *child,
@ -313,10 +329,10 @@ fn propagate_recursive(
child_parent.get(), expected_parent, child_parent.get(), expected_parent,
"Malformed hierarchy. This probably means that your hierarchy has been improperly maintained, or contains a cycle" "Malformed hierarchy. This probably means that your hierarchy has been improperly maintained, or contains a cycle"
); );
computed_visibility.is_visible_in_hierarchy = visibility.is_visible && parent_visible; let visible_in_hierarchy = visibility.is_visible && parent_visible;
// reset "view" visibility here ... if this entity should be drawn a future system should set this to true // reset "view" visibility here ... if this entity should be drawn a future system should set this to true
computed_visibility.is_visible_in_view = false; computed_visibility.reset(visible_in_hierarchy);
computed_visibility.is_visible_in_hierarchy visible_in_hierarchy
}; };
for child in children_query.get(entity).map_err(drop)?.iter() { for child in children_query.get(entity).map_err(drop)?.iter() {
@ -390,7 +406,7 @@ pub fn check_visibility(
} }
} }
computed_visibility.is_visible_in_view = true; computed_visibility.set_visible_in_view();
let cell = thread_queues.get_or_default(); let cell = thread_queues.get_or_default();
let mut queue = cell.take(); let mut queue = cell.take();
queue.push(entity); queue.push(entity);
@ -412,7 +428,7 @@ pub fn check_visibility(
return; return;
} }
computed_visibility.is_visible_in_view = true; computed_visibility.set_visible_in_view();
let cell = thread_queues.get_or_default(); let cell = thread_queues.get_or_default();
let mut queue = cell.take(); let mut queue = cell.take();
queue.push(entity); queue.push(entity);
@ -518,7 +534,7 @@ mod test {
.entity(e) .entity(e)
.get::<ComputedVisibility>() .get::<ComputedVisibility>()
.unwrap() .unwrap()
.is_visible_in_hierarchy .is_visible_in_hierarchy()
}; };
assert!( assert!(
!is_visible(root1), !is_visible(root1),