Improve documentation relating to Frustum and HalfSpace (#9136)

# Objective

This PR's first aim is to fix a mistake in `HalfSpace`'s documentation.

When defining a `Frustum` myself in bevy_basic_portals, I realised that
the "distance" of the `HalfSpace` is not, as the current doc defines,
the "distance from the origin along the normal", but actually the
opposite of that.

See the example I gave in this PR.

This means one of two things:
1. The documentation about `HalfSpace` is wrong (it is either way
because of the `n.p + d > 0` formula given later anyway, which is how it
behaves, but in that formula `d` is indeed the opposite of the "distance
from the origin along the normal", otherwise it should be `n.p > d`)
2. The distance is supposed to be the "distance from the origin along
the normal" but when used in a Frustum it's used as the opposite, and it
is a mistake
3. Same as 2, but it is somehow intended

Since I think `HalfSpace` is only used for `Frustum`, and it's easier to
fix documentation than code, I assumed for this PR we're in case number
1. If we're in case number 3, the documentation of `Frustum` needs to
change, and in case number 2, the code needs to be fixed.

While I was at it, I also :
- Tried to improve the documentation for `Frustum`, `Aabb`, and
`VisibilitySystems`, among others, since they're all related to
`Frustum`.
- Fixed documentation about frustum culling not applying to 2d objects,
which is not true since #7885

## Remarks and questions

- What about a `HalfSpace` with an infinite distance, is it allowed and
does it represents the whole space? If so it should probably be
mentioned.
- I referenced the `update_frusta` system in
`bevy_render::view::visibility` directly instead of referencing its
system set, should I reference the system set instead? It's a bit
annoying since it's in 3 sets.
- `visibility_propagate` is not public for some reason, I think it
probably should be, but for now I only documented its system set, should
I make it public? I don't think that would count as a breaking change?
- Why is `Aabb` inserted by a system, with `NoFrustumCulling` as an
opt-out, instead of having it inserted by default in `PbrBundle` for
example and then the system calculating it when it's added? Is it
because there is still no way to have an optional component inside a
bundle?

---------

Co-authored-by: SpecificProtagonist <[email protected]>
Co-authored-by: Alice Cecile <[email protected]>

Author: 3 people

crates/bevy_render/src/primitives/mod.rs

@@ -3,7 +3,32 @@ use bevy_math::{Affine3A, Mat3A, Mat4, Vec3, Vec3A, Vec4, Vec4Swizzles};
 use bevy_reflect::Reflect;
 use bevy_utils::HashMap;
 
-/// An axis-aligned bounding box.
+/// An axis-aligned bounding box, defined by:
+/// - a center,
+/// - the distances from the center to each faces along the axis,
+/// the faces are orthogonal to the axis.
+///
+/// It is typically used as a component on an entity to represent the local space
+/// occupied by this entity, with faces orthogonal to its local axis.
+///
+/// This component is notably used during "frustum culling", a process to determine
+/// if an entity should be rendered by a [`Camera`] if its bounding box intersects
+/// with the camera's [`Frustum`].
+///
+/// It will be added automatically by the systems in [`CalculateBounds`] to entities that:
+/// - could be subject to frustum culling, for example with a [`Handle<Mesh>`]
+/// or `Sprite` component,
+/// - don't have the [`NoFrustumCulling`] component.
+///
+/// It won't be updated automatically if the space occupied by the entity changes,
+/// for example if the vertex positions of a [`Mesh`] inside a `Handle<Mesh>` are
+/// updated.
+///
+/// [`Camera`]: crate::camera::Camera
+/// [`NoFrustumCulling`]: crate::view::visibility::NoFrustumCulling
+/// [`CalculateBounds`]: crate::view::visibility::VisibilitySystems::CalculateBounds
+/// [`Mesh`]: crate::mesh::Mesh
+/// [`Handle<Mesh>`]: crate::mesh::Mesh
 #[derive(Component, Clone, Copy, Debug, Default, Reflect)]
 #[reflect(Component)]
 pub struct Aabb {
@@ -76,20 +101,37 @@ impl Sphere {
     }
 }
 
-/// A bisecting plane that partitions 3D space into two regions.
+/// A region of 3D space, specifically an open set whose border is a bisecting 2D plane.
+/// This bisecting plane partitions 3D space into two infinite regions,
+/// the half-space is one of those regions and excludes the bisecting plane.
+///
+/// Each instance of this type is characterized by:
+/// - the bisecting plane's unit normal, normalized and pointing "inside" the half-space,
+/// - the signed distance along the normal from the bisecting plane to the origin of 3D space.
+///
+/// The distance can also be seen as:
+/// - the distance along the inverse of the normal from the origin of 3D space to the bisecting plane,
+/// - the opposite of the distance along the normal from the origin of 3D space to the bisecting plane.
+///
+/// Any point `p` is considered to be within the `HalfSpace` when the length of the projection
+/// of p on the normal is greater or equal than the opposite of the distance,
+/// meaning: if the equation `normal.dot(p) + distance > 0.` is satisfied.
 ///
-/// Each instance of this type is characterized by the bisecting plane's unit normal and distance from the origin along the normal.
-/// Any point `p` is considered to be within the `HalfSpace` when the distance is positive,
-/// meaning: if the equation `n.p + d > 0` is satisfied.
+/// For example, the half-space containing all the points with a z-coordinate lesser
+/// or equal than `8.0` would be defined by: `HalfSpace::new(Vec3::NEG_Z.extend(-8.0))`.
+/// It includes all the points from the bisecting plane towards `NEG_Z`, and the distance
+/// from the plane to the origin is `-8.0` along `NEG_Z`.
+///
+/// It is used to define a [`Frustum`], but is also a useful mathematical primitive for rendering tasks such as  light computation.
 #[derive(Clone, Copy, Debug, Default)]
 pub struct HalfSpace {
     normal_d: Vec4,
 }
 
 impl HalfSpace {
     /// Constructs a `HalfSpace` from a 4D vector whose first 3 components
-    /// represent the bisecting plane's unit normal, and the last component signifies
-    /// the distance from the origin to the plane along the normal.
+    /// represent the bisecting plane's unit normal, and the last component is
+    /// the signed distance along the normal from the plane to the origin.
     /// The constructor ensures the normal vector is normalized and the distance is appropriately scaled.
     #[inline]
     pub fn new(normal_d: Vec4) -> Self {
@@ -104,23 +146,46 @@ impl HalfSpace {
         Vec3A::from(self.normal_d)
     }
 
-    /// Returns the distance from the origin to the bisecting plane along the plane's unit normal vector.
-    /// This distance helps determine the position of a point `p` on the bisecting plane, as per the equation `n.p + d = 0`.
+    /// Returns the signed distance from the bisecting plane to the origin along
+    /// the plane's unit normal vector.
     #[inline]
     pub fn d(&self) -> f32 {
         self.normal_d.w
     }
 
-    /// Returns the bisecting plane's unit normal vector and the distance from the origin to the plane.
+    /// Returns the bisecting plane's unit normal vector and the signed distance
+    /// from the plane to the origin.
     #[inline]
     pub fn normal_d(&self) -> Vec4 {
         self.normal_d
     }
 }
 
-/// A frustum made up of the 6 defining half spaces.
-/// Half spaces are ordered left, right, top, bottom, near, far.
-/// The normal vectors of the half spaces point towards the interior of the frustum.
+/// A region of 3D space defined by the intersection of 6 [`HalfSpace`]s.
+///
+/// Frustums are typically an apex-truncated square pyramid (a pyramid without the top) or a cuboid.
+///
+/// Half spaces are ordered left, right, top, bottom, near, far. The normal vectors
+/// of the half-spaces point towards the interior of the frustum.
+///
+/// A frustum component is used on an entity with a [`Camera`] component to
+/// determine which entities will be considered for rendering by this camera.
+/// All entities with an [`Aabb`] component that are not contained by (or crossing
+/// the boundary of) the frustum will not be rendered, and not be used in rendering computations.
+///
+/// This process is called frustum culling, and entities can opt out of it using
+/// the [`NoFrustumCulling`] component.
+///
+/// The frustum component is typically added from a bundle, either the `Camera2dBundle`
+/// or the `Camera3dBundle`.
+/// It is usually updated automatically by [`update_frusta`] from the
+/// [`CameraProjection`] component and [`GlobalTransform`] of the camera entity.
+///
+/// [`Camera`]: crate::camera::Camera
+/// [`NoFrustumCulling`]: crate::view::visibility::NoFrustumCulling
+/// [`update_frusta`]: crate::view::visibility::update_frusta
+/// [`CameraProjection`]: crate::camera::CameraProjection
+/// [`GlobalTransform`]: bevy_transform::components::GlobalTransform
 #[derive(Component, Clone, Copy, Debug, Default, Reflect)]
 #[reflect(Component)]
 pub struct Frustum {

crates/bevy_render/src/view/visibility/mod.rs

@@ -153,23 +153,26 @@ pub struct VisibilityBundle {
     pub computed: ComputedVisibility,
 }
 
-/// Use this component to opt-out of built-in frustum culling for Mesh entities
+/// Use this component to opt-out of built-in frustum culling for entities, see
+/// [`Frustum`].
+///
+/// It can be used for example:
+/// - when a [`Mesh`] is updated but its [`Aabb`] is not, which might happen with animations,
+/// - when using some light effects, like wanting a [`Mesh`] out of the [`Frustum`]
+/// to appear in the reflection of a [`Mesh`] within.
 #[derive(Component, Default, Reflect)]
 #[reflect(Component, Default)]
 pub struct NoFrustumCulling;
 
 /// Collection of entities visible from the current view.
 ///
 /// This component contains all entities which are visible from the currently
-/// rendered view. The collection is updated automatically by the [`check_visibility()`]
-/// system, and renderers can use it to optimize rendering of a particular view, to
+/// rendered view. The collection is updated automatically by the [`VisibilitySystems::CheckVisibility`]
+/// system set, and renderers can use it to optimize rendering of a particular view, to
 /// prevent drawing items not visible from that view.
 ///
 /// This component is intended to be attached to the same entity as the [`Camera`] and
 /// the [`Frustum`] defining the view.
-///
-/// Currently this component is ignored by the sprite renderer, so sprite rendering
-/// is not optimized per view.
 #[derive(Clone, Component, Default, Debug, Reflect)]
 #[reflect(Component)]
 pub struct VisibleEntities {
@@ -193,13 +196,21 @@ impl VisibleEntities {
 
 #[derive(Debug, Hash, PartialEq, Eq, Clone, SystemSet)]
 pub enum VisibilitySystems {
+    /// Label for the [`calculate_bounds`] and `calculate_bounds_2d` systems,
+    /// calculating and inserting an [`Aabb`] to relevant entities.
     CalculateBounds,
+    /// Label for the [`apply_deferred`] call after [`VisibilitySystems::CalculateBounds`]
     CalculateBoundsFlush,
+    /// Label for the [`update_frusta<OrthographicProjection>`] system.
     UpdateOrthographicFrusta,
+    /// Label for the [`update_frusta<PerspectiveProjection>`] system.
     UpdatePerspectiveFrusta,
+    /// Label for the [`update_frusta<Projection>`] system.
     UpdateProjectionFrusta,
+    /// Label for the system propagating the [`ComputedVisibility`] in a
+    /// [`hierarchy`](bevy_hierarchy).
     VisibilityPropagate,
-    /// Label for the [`check_visibility()`] system updating each frame the [`ComputedVisibility`]
+    /// Label for the [`check_visibility`] system updating [`ComputedVisibility`]
     /// of each entity and the [`VisibleEntities`] of each view.
     CheckVisibility,
 }
@@ -253,6 +264,10 @@ impl Plugin for VisibilityPlugin {
     }
 }
 
+/// Computes and adds an [`Aabb`] component to entities with a
+/// [`Handle<Mesh>`](Mesh) component and without a [`NoFrustumCulling`] component.
+///
+/// This system is used in system set [`VisibilitySystems::CalculateBounds`].
 pub fn calculate_bounds(
     mut commands: Commands,
     meshes: Res<Assets<Mesh>>,
@@ -267,6 +282,11 @@ pub fn calculate_bounds(
     }
 }
 
+/// Updates [`Frustum`].
+///
+/// This system is used in system sets [`VisibilitySystems::UpdateProjectionFrusta`],
+/// [`VisibilitySystems::UpdatePerspectiveFrusta`], and
+/// [`VisibilitySystems::UpdateOrthographicFrusta`].
 pub fn update_frusta<T: Component + CameraProjection + Send + Sync + 'static>(
     mut views: Query<
         (&GlobalTransform, &T, &mut Frustum),
@@ -345,9 +365,9 @@ fn propagate_recursive(
     Ok(())
 }
 
-/// System updating the visibility of entities each frame.
+/// Updates the visibility of entities each frame.
 ///
-/// The system is part of the [`VisibilitySystems::CheckVisibility`] set. Each frame, it updates the
+/// This system is part of the [`VisibilitySystems::CheckVisibility`] set. Each frame, it updates the
 /// [`ComputedVisibility`] of all entities, and for each view also compute the [`VisibleEntities`]
 /// for that view.
 pub fn check_visibility(

crates/bevy_sprite/src/lib.rs

@@ -108,6 +108,13 @@ impl Plugin for SpritePlugin {
     }
 }
 
+/// System calculating and inserting an [`Aabb`] component to entities with either:
+/// - a `Mesh2dHandle` component,
+/// - a `Sprite` and `Handle<Image>` components,
+/// - a `TextureAtlasSprite` and `Handle<TextureAtlas>` components,
+/// and without a [`NoFrustumCulling`] component.
+///
+/// Used in system set [`VisibilitySystems::CalculateBounds`].
 pub fn calculate_bounds_2d(
     mut commands: Commands,
     meshes: Res<Assets<Mesh>>,