default RenderingMask is group 0 (mask 1), more tests, fix inner loop…

…, private mask, docs, fn names, added all_groups and no_groups

crates/bevy_render/src/camera/visible_entities.rs

@@ -29,43 +29,72 @@ impl VisibleEntities {
 /// Cameras with this component will only render entities with a matching
 /// mask.
 ///
-/// The [`Default`] instance of `RenderingMask` returns a mask that contains
-/// no groups.
+/// There are 32 groups numbered `0` - `31`. A mask may belong to one or more
+/// groups, or no group at all.
+///
+/// An entity with a mask belonging to no groups is invisible.
+///
+/// The [`Default`] instance of `RenderingMask` returns a mask belonging to
+/// group `0`, the first group.
 #[derive(Copy, Clone, Debug, Reflect, PartialEq, Eq, PartialOrd, Ord)]
 #[reflect(Component)]
-pub struct RenderingMask(pub u32);
+pub struct RenderingMask(u32);
 
+/// Defaults to a mask belonging to group `0`, the first group.
 impl Default for RenderingMask {
     fn default() -> Self {
-        RenderingMask(0)
+        RenderingMask(1)
     }
 }
 
 impl RenderingMask {
-    /// Create new `RenderingMask` with the given rendering group set.
+    /// Create a new `RenderingMask` belonging to the given rendering group.
     pub fn group(n: u8) -> Self {
-        RenderingMask::default().set_group(n)
+        RenderingMask(0).with_group(n)
+    }
+
+    /// Create a new `RenderingMask` that belongs to all rendering groups.
+    pub fn all_groups() -> Self {
+        RenderingMask(u32::MAX)
     }
 
-    /// Set the given group on the mask.
+    /// Create a new `RenderingMask` that belongs to no rendering groups.
+    pub fn no_groups() -> Self {
+        RenderingMask(0)
+    }
+
+    /// Add the given group to the mask.
+    ///
     /// This may be called multiple times to allow an entity to belong
-    /// to multiple rendering groups.
-    pub fn set_group(mut self, group: u8) -> Self {
+    /// to multiple rendering groups. The maximum group is 31.
+    ///
+    /// # Panics
+    /// Panics when called with a group greater than 31.
+    pub fn with_group(mut self, group: u8) -> Self {
+        assert!(group < 32, "RenderingMask only supports groups 0 to 31");
         self.0 |= 1 << group;
         self
     }
 
-    /// Unset the given rendering group from the mask.
-    pub fn unset_group(mut self, group: u8) -> Self {
+    /// Removes the given rendering group from the mask.
+    ///
+    /// # Panics
+    /// Panics when called with a group greater than 31.
+    pub fn without_group(mut self, group: u8) -> Self {
+        assert!(group < 32, "RenderingMask only supports groups 0 to 31");
         self.0 |= 0 << group;
         self
     }
 
     /// Determine if a `RenderingMask` matches another.
+    ///
     /// `RenderingMask`s match if the first mask contains any of the groups
-    /// in the other, or if both masks contain no groups.
+    /// in the other.
+    ///
+    /// A `RenderingMask` belonging to no groups will not match any other
+    /// mask, even another belonging to no groups.
     pub fn matches(&self, other: &RenderingMask) -> bool {
-        ((self.0 & other.0) > 0) || (self.0 == 0 && other.0 == 0)
+        (self.0 & other.0) > 0
     }
 }
 
@@ -75,20 +104,39 @@ mod rendering_mask_tests {
 
     #[test]
     fn rendering_mask_sanity() {
-        // groups match groups
-        assert!(RenderingMask::group(1).matches(&RenderingMask::group(1)));
-        // a group of 0 means the mask is just 1 bit
-        assert!(RenderingMask::group(0).matches(&RenderingMask(1)));
-        // a mask will match another mask containing any similar groups
-        assert!(RenderingMask::group(0)
-            .set_group(3)
-            .matches(&RenderingMask::group(3)));
-        // default masks match each other
-        assert!(RenderingMask::default().matches(&RenderingMask::default()));
-        // masks with differing groups do not match
+        assert_eq!(RenderingMask::group(0).0, 1, "group 0 is mask 1");
+        assert_eq!(RenderingMask::group(1).0, 2, "group 1 is mask 2");
+        assert_eq!(RenderingMask::group(0).with_group(1).0, 3, "group 0 + 1 is mask 3");
+        assert!(
+            RenderingMask::group(1).matches(&RenderingMask::group(1)),
+            "groups match like groups"
+        );
+        assert!(
+            RenderingMask::group(0).matches(&RenderingMask(1)),
+            "a group of 0 means the mask is just 1 bit"
+        );
+
+        assert!(
+            RenderingMask::group(0)
+                .with_group(3)
+                .matches(&RenderingMask::group(3)),
+            "a mask will match another mask containing any similar groups"
+        );
+
+        assert!(
+            RenderingMask::default().matches(&RenderingMask::default()),
+            "default masks match each other"
+        );
+
         assert_eq!(
             RenderingMask::group(0).matches(&RenderingMask::group(1)),
-            false
+            false,
+            "masks with differing groups do not match"
+        );
+        assert_eq!(
+            RenderingMask(0).matches(&RenderingMask(0)),
+            false,
+            "empty masks don't match"
         );
     }
 }
@@ -108,6 +156,7 @@ pub fn visible_entities_system(
     {
         visible_entities.value.clear();
         let camera_position = camera_global_transform.translation;
+        let camera_mask = maybe_camera_mask.copied().unwrap_or_default();
 
         let mut no_transform_order = 0.0;
         let mut transparent_entities = Vec::new();
@@ -116,7 +165,6 @@ pub fn visible_entities_system(
                 continue;
             }
 
-            let camera_mask = maybe_camera_mask.copied().unwrap_or_default();
             let entity_mask = maybe_entity_mask.copied().unwrap_or_default();
             if !camera_mask.matches(&entity_mask) {
                 continue;