Try to rework buffer info to be less error-prone

bindgen-citro3d/Cargo.toml

@@ -3,6 +3,7 @@ name = "bindgen-citro3d"
 version = "0.1.0"
 edition = "2021"
 description = "Helper tool to generate citro3d-sys bindings."
+publish = false
 
 [dependencies]
 bindgen = "0.62.0"

citro3d/examples/triangle.rs

@@ -84,7 +84,9 @@ fn main() {
 
     let mut vbo_data = Vec::with_capacity_in(VERTICES.len(), ctru::linear::LinearAllocator);
     vbo_data.extend_from_slice(VERTICES);
-    let vbo_idx = prepare_vbos(&vbo_data);
+
+    let mut buf_info = buffer::Info::new();
+    let (attr_info, vbo_idx) = prepare_vbos(&mut buf_info, &vbo_data);
 
     let (uloc_projection, projection) = scene_init(&mut program);
 
@@ -113,6 +115,8 @@ fn main() {
                     );
                 }
 
+                instance.set_attr_info(&attr_info);
+
                 instance.draw_arrays(buffer::Primitive::Triangles, vbo_idx);
             });
         };
@@ -122,9 +126,17 @@ fn main() {
     }
 }
 
-fn prepare_vbos(vbo_data: &[Vertex]) -> buffer::Index {
+// sheeeesh, this sucks to type:
+fn prepare_vbos<'buf, 'info, 'vbo>(
+    buf_info: &'info mut buffer::Info,
+    vbo_data: &'vbo [Vertex],
+) -> (attrib::Info, buffer::Index<'buf>)
+where
+    'info: 'buf,
+    'vbo: 'buf,
+{
     // Configure attributes for use with the vertex shader
-    let mut attr_info = attrib::Info::get_mut().expect("failed to get global attr info");
+    let mut attr_info = attrib::Info::new();
 
     let reg0 = attrib::Register::new(0).unwrap();
     let reg1 = attrib::Register::new(1).unwrap();
@@ -145,11 +157,9 @@ fn prepare_vbos(vbo_data: &[Vertex]) -> buffer::Index {
         .set_permutation(&[position_attr, color_attr])
         .unwrap();
 
-    // Configure buffers
-    let mut buf_info = buffer::Info::get_mut().unwrap();
     let buf_idx = buf_info.add(vbo_data, &attr_info).unwrap();
 
-    buf_idx
+    (attr_info, buf_idx)
 }
 
 fn scene_init(program: &mut shader::Program) -> (i8, C3D_Mtx) {

citro3d/src/attrib.rs

@@ -1,22 +1,9 @@
-use std::ops::{Deref, DerefMut};
-use std::sync::{LazyLock, RwLock};
+use std::mem::MaybeUninit;
 
-static INFO: LazyLock<RwLock<Info>> = LazyLock::new(|| {
-    let raw = unsafe {
-        // TODO: should we check is_null() here?
-        let info = citro3d_sys::C3D_GetAttrInfo();
-        citro3d_sys::AttrInfo_Init(info);
-        info
-    };
-
-    RwLock::new(Info { raw })
-});
+#[derive(Debug)]
+pub struct Info(pub(crate) citro3d_sys::C3D_AttrInfo);
 
-pub struct Info {
-    raw: *mut citro3d_sys::C3D_AttrInfo,
-}
-
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug)]
 pub struct Register(libc::c_int);
 
 impl Register {
@@ -46,14 +33,23 @@ unsafe impl Sync for Info {}
 unsafe impl Send for Info {}
 
 impl Info {
-    /// Get a reference to the global attribute info.
-    pub fn get() -> crate::Result<impl Deref<Target = Self>> {
-        Ok(INFO.try_read()?)
+    pub fn new() -> Self {
+        let mut raw = MaybeUninit::zeroed();
+        let raw = unsafe {
+            citro3d_sys::AttrInfo_Init(raw.as_mut_ptr());
+            raw.assume_init()
+        };
+        Self(raw)
     }
 
-    /// Get a mutable reference to the global attribute info.
-    pub fn get_mut() -> crate::Result<impl DerefMut<Target = Self>> {
-        Ok(INFO.try_write()?)
+    pub(crate) fn copy_from(raw: *const citro3d_sys::C3D_AttrInfo) -> Option<Self> {
+        if raw.is_null() {
+            None
+        } else {
+            // This is less efficient than returning a pointer or something, but it's
+            // safer since we don't know the lifetime of the pointee
+            Some(Self(unsafe { *raw }))
+        }
     }
 
     /// Add an attribute loader to the attribute info. By default, the resulting
@@ -66,8 +62,11 @@ impl Info {
     ) -> crate::Result<Index> {
         let count = count.try_into()?;
 
-        let ret =
-            unsafe { citro3d_sys::AttrInfo_AddLoader(self.raw, register.0, format as u32, count) };
+        // 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)
+        };
 
         let Ok(idx) = ret.try_into() else {
             return Err(crate::Error::FailedToInitialize)
@@ -100,10 +99,9 @@ impl Info {
 
         let permutation = bytemuck::cast(<[u8; 8]>::try_from(bytes).unwrap());
 
-        unsafe {
-            (*self.raw).permutation = permutation;
-            (*self.raw).attrCount = attr_count;
-        }
+        self.0.permutation = permutation;
+        self.0.attrCount = attr_count;
+
         Ok(())
     }
 
@@ -113,11 +111,11 @@ impl Info {
     ///
     /// [GPU/Internal Registers]: https://3dbrew.org/wiki/GPU/Internal_Registers#GPUREG_SH_ATTRIBUTES_PERMUTATION_LOW
     pub fn permutation(&self) -> u64 {
-        unsafe { (*self.raw).permutation }
+        self.0.permutation
     }
 
     /// Get the number of attributes in the current permutation.
     pub fn count(&self) -> libc::c_int {
-        unsafe { (*self.raw).attrCount }
+        self.0.attrCount
     }
 }

citro3d/src/buffer.rs

@@ -1,36 +1,18 @@
 use std::marker::PhantomData;
 use std::mem::MaybeUninit;
-use std::ops::{Deref, DerefMut};
-use std::sync::{LazyLock, RwLock};
 
 use crate::attrib;
 
-static BUF_INFO: LazyLock<RwLock<Info>> = LazyLock::new(|| {
-    let raw = unsafe {
-        let info = citro3d_sys::C3D_GetBufInfo();
-        citro3d_sys::BufInfo_Init(info);
-        info
-    };
-
-    RwLock::new(Info { raw })
-});
-
-/// Vertex attribute info. This struct can be used to
-pub struct Info {
-    raw: *mut citro3d_sys::C3D_BufInfo,
-}
-
-// SAFETY: the RWLock ensures unique access when mutating the global struct, and
-// we trust citro3d to Do The Right Thing™ and not mutate it otherwise.
-unsafe impl Sync for Info {}
-unsafe impl Send for Info {}
+#[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<'vbo> {
+pub struct Index<'buf> {
     index: libc::c_int,
     size: libc::c_int,
-    _data: PhantomData<&'vbo ()>,
+    _vbo_data: PhantomData<&'buf ()>,
+    buf_info: &'buf Info,
 }
 
 impl Index<'_> {
@@ -41,6 +23,10 @@ impl Index<'_> {
     pub fn size(&self) -> libc::c_int {
         self.size
     }
+
+    pub fn info(&self) -> &Info {
+        self.buf_info
+    }
 }
 
 #[repr(u32)]
@@ -53,28 +39,44 @@ pub enum Primitive {
 }
 
 impl Info {
-    /// Get a reference to the global buffer info.
-    pub fn get() -> crate::Result<impl Deref<Target = Self>> {
-        Ok(BUF_INFO.try_read()?)
+    pub fn new() -> Self {
+        let mut info = MaybeUninit::zeroed();
+        let info = unsafe {
+            citro3d_sys::BufInfo_Init(info.as_mut_ptr());
+            info.assume_init()
+        };
+        Self(info)
     }
 
-    /// Get a mutable reference to the global buffer info.
-    pub fn get_mut() -> crate::Result<impl DerefMut<Target = Self>> {
-        Ok(BUF_INFO.try_write()?)
+    pub(crate) fn copy_from(raw: *const citro3d_sys::C3D_BufInfo) -> Option<Self> {
+        if raw.is_null() {
+            None
+        } else {
+            // This is less efficient than returning a pointer or something, but it's
+            // safer since we don't know the lifetime of the pointee
+            Some(Self(unsafe { *raw }))
+        }
     }
 
-    pub fn add<'vbo, T>(
-        &mut self,
+    pub fn add<'this, 'vbo, 'idx, T>(
+        &'this mut self,
         vbo_data: &'vbo [T],
         attrib_info: &attrib::Info,
-    ) -> crate::Result<Index<'vbo>> {
+    ) -> crate::Result<Index<'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
+        // in the BufInfo, not copied to be used later.
         let res = unsafe {
             citro3d_sys::BufInfo_Add(
-                self.raw,
+                &mut self.0,
                 vbo_data.as_ptr().cast(),
                 stride,
                 attrib_count,
@@ -94,7 +96,8 @@ impl Info {
             Ok(Index {
                 index: res,
                 size: vbo_data.len().try_into()?,
-                _data: PhantomData,
+                _vbo_data: PhantomData,
+                buf_info: self,
             })
         }
     }

citro3d/src/lib.rs

@@ -3,7 +3,7 @@
 //! Safe Rust bindings to `citro3d`.
 
 pub mod attrib;
-pub mod buffers;
+pub mod buffer;
 pub mod error;
 pub mod render;
 pub mod shader;
@@ -15,6 +15,7 @@ pub use error::{Error, Result};
 /// The single instance for using `citro3d`. This is the base type that an application
 /// should instantiate to use this library.
 #[non_exhaustive]
+#[must_use]
 #[derive(Debug)]
 pub struct Instance;
 
@@ -74,7 +75,40 @@ impl Instance {
         }
     }
 
-    pub fn draw_arrays(&mut self, primitive: buffers::Primitive, index: buffers::Index) {
+    /// Get the buffer info being used, if it exists. Note that the resulting
+    /// [`buffer::Info`] is copied from the one currently in use.
+    pub fn buffer_info(&self) -> Option<buffer::Info> {
+        let raw = unsafe { citro3d_sys::C3D_GetBufInfo() };
+        buffer::Info::copy_from(raw)
+    }
+
+    /// Set the buffer info to use for any following draw calls.
+    pub fn set_buffer_info(&mut self, buffer_info: &buffer::Info) {
+        let raw: *const _ = &buffer_info.0;
+        // SAFETY: C3D_SetBufInfo actually copies the pointee instead of mutating it.
+        unsafe { citro3d_sys::C3D_SetBufInfo(raw.cast_mut()) };
+    }
+
+    /// Get the attribute info being used, if it exists. Note that the resulting
+    /// [`attrib::Info`] is copied from the one currently in use.
+    pub fn attr_info(&self) -> Option<attrib::Info> {
+        let raw = unsafe { citro3d_sys::C3D_GetAttrInfo() };
+        attrib::Info::copy_from(raw)
+    }
+
+    /// Set the attribute info to use for any following draw calls.
+    pub fn set_attr_info(&mut self, attr_info: &attrib::Info) {
+        let raw: *const _ = &attr_info.0;
+        // SAFETY: C3D_SetAttrInfo actually copies the pointee instead of mutating it.
+        unsafe { citro3d_sys::C3D_SetAttrInfo(raw.cast_mut()) };
+    }
+
+    /// Draw the specified primitivearrays. The
+    pub fn draw_arrays(&mut self, primitive: buffer::Primitive, index: buffer::Index) {
+        self.set_buffer_info(index.info());
+
+        // TODO: should we also require the attrib info directly here?
+
         unsafe {
             citro3d_sys::C3D_DrawArrays(
                 primitive as ctru_sys::GPU_Primitive_t,