Add attrib docstrings and update some names

citro3d/examples/triangle.rs

@@ -143,7 +143,7 @@ fn main() {
 fn prepare_vbos<'buf, 'info, 'vbo>(
     buf_info: &'info mut buffer::Info,
     vbo_data: &'vbo [Vertex],
-) -> (attrib::Info, buffer::Index<'buf>)
+) -> (attrib::Info, buffer::Slice<'buf>)
 where
     'info: 'buf,
     'vbo: 'buf,
@@ -154,20 +154,12 @@ where
     let reg0 = attrib::Register::new(0).unwrap();
     let reg1 = attrib::Register::new(1).unwrap();
 
-    // The default permutation would actually already be what we want if we
-    // inserted position, then color, but just show that it's customizable
-    // by swapping the order then using `set_permutation`.
-
-    let color_attr = attr_info
+    attr_info
         .add_loader(reg0, attrib::Format::Float, 3)
         .unwrap();
 
-    let position_attr = attr_info
-        .add_loader(reg1, attrib::Format::Float, 3)
-        .unwrap();
-
     attr_info
-        .set_permutation(&[position_attr, color_attr])
+        .add_loader(reg1, attrib::Format::Float, 5)
         .unwrap();
 
     let buf_idx = buf_info.add(vbo_data, &attr_info).unwrap();

citro3d/src/attrib.rs

@@ -1,29 +1,49 @@
 use std::mem::MaybeUninit;
 
+/// Vertex attribute info. This struct is used to describe how vertex buffers are
+/// used (i.e. the shape of the vertex data).
 #[derive(Debug)]
 pub struct Info(pub(crate) citro3d_sys::C3D_AttrInfo);
 
-#[derive(Debug)]
+/// A shader input register, usually corresponding to a single vertex attribute
+/// (e.g. position or color). These are called `v0`, `v1`, ... `v15` in the
+/// [picasso](https://github.com/devkitPro/picasso/blob/master/Manual.md)
+/// shader language.
+#[derive(Debug, Clone, Copy)]
 pub struct Register(libc::c_int);
 
 impl Register {
+    /// Get a register corresponding to the given index.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error for `n >= 16`.
     pub fn new(n: u16) -> crate::Result<Self> {
-        // TODO proper validation for attributes? Or maybe just a next() function
-        // that gets atomically increasing indices or something? Or look at
-        // <https://3dbrew.org/wiki/GPU/Internal_Registers> and define some consts
-        // or lookup functions
+        if n < 16 {
             Ok(Self(n.into()))
+        } else {
+            Err(crate::Error::TooManyAttributes)
+        }
     }
 }
 
-#[must_use]
+/// An attribute index. This is the attribute's actual index in the input buffer,
+/// and may correspond to any [`Register`] (or multiple) as input in the shader
+/// program.
+#[derive(Debug, Clone, Copy)]
 pub struct Index(u8);
 
+/// The data format of an attribute.
 #[repr(u32)]
+#[derive(Debug, Clone, Copy)]
 pub enum Format {
+    /// A signed byte, i.e. [`i8`].
     Byte = ctru_sys::GPU_BYTE,
+    /// An unsigned byte, i.e. [`u8`].
     UnsignedByte = ctru_sys::GPU_UNSIGNED_BYTE,
+    /// A float, i.e. [`f32`].
     Float = ctru_sys::GPU_FLOAT,
+    /// A short integer, i.e. [`i16`].
     Short = ctru_sys::GPU_SHORT,
 }
 
@@ -44,6 +64,7 @@ impl Default for Info {
 }
 
 impl Info {
+    /// Construct a new attribute info structure with no attributes.
     pub fn new() -> Self {
         Self::default()
     }
@@ -58,70 +79,49 @@ impl Info {
         }
     }
 
-    /// Add an attribute loader to the attribute info. By default, the resulting
-    /// attribute index will be appended to the permutation
+    /// Add an attribute loader to the attribute info. The resulting attribute index
+    /// indicates the registration order of the attributes.
+    ///
+    /// ## Parameters
+    ///
+    /// * `register`: the shader program input register for this attribute.
+    /// * `format`: the data format of this attribute.
+    /// * `count`: the number of elements in each attribute (up to 4, corresponding
+    ///   to `xyzw` / `rgba` / `stpq`).
+    ///
+    /// ## Errors
+    ///
+    /// * If `count > 4`
+    /// * If this attribute info already has the maximum number of attributes.
     pub fn add_loader(
         &mut self,
         register: Register,
         format: Format,
-        count: usize,
+        count: u8,
     ) -> crate::Result<Index> {
-        let count = count.try_into()?;
+        if count > 4 {
+            return Err(crate::Error::InvalidSize);
+        }
 
         // SAFETY: the &mut self.0 reference is only used to access fields in
         // the attribute info, not stored somewhere for later use
         let ret = unsafe {
-            citro3d_sys::AttrInfo_AddLoader(&mut self.0, register.0, format as u32, count)
+            citro3d_sys::AttrInfo_AddLoader(&mut self.0, register.0, format as u32, count.into())
         };
 
         let Ok(idx) = ret.try_into() else {
-            return Err(crate::Error::FailedToInitialize)
+            return Err(crate::Error::TooManyAttributes)
         };
 
         Ok(Index(idx))
     }
 
-    pub fn set_permutation(&mut self, indices: &[Index]) -> crate::Result<()> {
-        if indices.len() > 16 {
-            return Err(crate::Error::TooManyAttributes);
-        }
-        let attr_count: libc::c_int = indices.len().try_into().unwrap();
-
-        let mut bytes: Vec<u8> = indices
-            .windows(2)
-            .map(|window| {
-                let [lo, hi] = match *window {
-                    [Index(lo), Index(hi)] => [lo, hi],
-                    [Index(lo)] => [lo, 0], // high nibble is just padding
-                    _ => unreachable!(),    // window size of 2 == always 1 or 2 elements
-                };
-                // each value is a nibble, combine them into a byte
-                lo | (hi << 4)
-            })
-            .collect();
-
-        // pad the remainder with zeros
-        bytes.extend(std::iter::repeat(0).take(8 - bytes.len()));
-
-        let permutation = bytemuck::cast(<[u8; 8]>::try_from(bytes).unwrap());
-
-        self.0.permutation = permutation;
-        self.0.attrCount = attr_count;
-
-        Ok(())
-    }
-
-    /// Get the current permutation of input register to vertex attributes mapping.
-    /// See [GPU/Internal Registers] for an explanation of how the bits are laid out
-    /// in the resulting value.
-    ///
-    /// [GPU/Internal Registers]: https://3dbrew.org/wiki/GPU/Internal_Registers#GPUREG_SH_ATTRIBUTES_PERMUTATION_LOW
-    pub fn permutation(&self) -> u64 {
+    pub(crate) fn permutation(&self) -> u64 {
         self.0.permutation
     }
 
-    /// Get the number of attributes in the current permutation.
-    pub fn count(&self) -> libc::c_int {
+    /// Get the number of registered attributes.
+    pub fn attr_count(&self) -> libc::c_int {
         self.0.attrCount
     }
 }

citro3d/src/buffer.rs

@@ -6,16 +6,15 @@ use crate::attrib;
 #[derive(Debug)]
 pub struct Info(pub(crate) citro3d_sys::C3D_BufInfo);
 
-// TODO: is this a good name? It's more like a "handle" to the VBO data, or a slice.
 #[derive(Debug, Clone, Copy)]
-pub struct Index<'buf> {
+pub struct Slice<'buf> {
     index: libc::c_int,
     size: libc::c_int,
     _vbo_data: PhantomData<&'buf ()>,
     buf_info: &'buf Info,
 }
 
-impl Index<'_> {
+impl Slice<'_> {
     pub fn as_raw(&self) -> libc::c_int {
         self.index
     }
@@ -68,14 +67,12 @@ impl Info {
         &'this mut self,
         vbo_data: &'vbo [T],
         attrib_info: &attrib::Info,
-    ) -> crate::Result<Index<'idx>>
+    ) -> crate::Result<Slice<'idx>>
     where
         'this: 'idx,
         'vbo: 'idx,
     {
         let stride = std::mem::size_of::<T>().try_into()?;
-        let attrib_count = attrib_info.count();
-        let permutation = attrib_info.permutation();
 
         // SAFETY: the lifetime of the VBO data is encapsulated in the return value's
         // 'vbo lifetime, and the pointer to &mut self.0 is used to access values
@@ -85,8 +82,8 @@ impl Info {
                 &mut self.0,
                 vbo_data.as_ptr().cast(),
                 stride,
-                attrib_count,
-                permutation,
+                attrib_info.attr_count(),
+                attrib_info.permutation(),
             )
         };
 
@@ -99,7 +96,7 @@ impl Info {
             // <https://github.com/devkitPro/citro3d/blob/master/source/buffers.c#L13-L17>
             Err(crate::Error::System(res))
         } else {
-            Ok(Index {
+            Ok(Slice {
                 index: res,
                 size: vbo_data.len().try_into()?,
                 _vbo_data: PhantomData,

citro3d/src/error.rs

@@ -24,7 +24,7 @@ pub enum Error {
     /// Indicates that a reference could not be obtained because a lock is already
     /// held on the requested object.
     LockHeld,
-    /// Indicates that too many vertex attributes were specified (max 16 supported).
+    /// Indicates that too many vertex attributes were specified (max 12 supported).
     TooManyAttributes,
 }
 

citro3d/src/lib.rs

@@ -102,7 +102,7 @@ impl Instance {
     }
 
     /// Draw the specified primitivearrays. The
-    pub fn draw_arrays(&mut self, primitive: buffer::Primitive, index: buffer::Index) {
+    pub fn draw_arrays(&mut self, primitive: buffer::Primitive, index: buffer::Slice) {
         self.set_buffer_info(index.info());
 
         // TODO: should we also require the attrib info directly here?