Use a checked index API and do a bit of cleanup

citro3d/examples/cube.rs

@@ -8,8 +8,7 @@ use citro3d::math::{
     AspectRatio, ClipPlanes, CoordinateOrientation, FVec3, Matrix4, Projection, StereoDisplacement,
 };
 use citro3d::render::ClearFlags;
-use citro3d::{attrib, buffer, render, shader};
-use citro3d::{texenv, IndexType};
+use citro3d::{attrib, buffer, render, shader, texenv};
 use ctru::prelude::*;
 use ctru::services::gfx::{RawFrameBuffer, Screen, TopScreen3D};
 
@@ -86,17 +85,20 @@ fn main() {
     let (mut top_left, mut top_right) = top_screen.split_mut();
 
     let RawFrameBuffer { width, height, .. } = top_left.raw_framebuffer();
-    let mut top_left_target =
-        render::Target::new(width, height, top_left, None).expect("failed to create render target");
+    let mut top_left_target = instance
+        .render_target(width, height, top_left, None)
+        .expect("failed to create render target");
 
     let RawFrameBuffer { width, height, .. } = top_right.raw_framebuffer();
-    let mut top_right_target = render::Target::new(width, height, top_right, None)
+    let mut top_right_target = instance
+        .render_target(width, height, top_right, None)
         .expect("failed to create render target");
 
     let mut bottom_screen = gfx.bottom_screen.borrow_mut();
     let RawFrameBuffer { width, height, .. } = bottom_screen.raw_framebuffer();
 
-    let mut bottom_target = render::Target::new(width, height, bottom_screen, None)
+    let mut bottom_target = instance
+        .render_target(width, height, bottom_screen, None)
         .expect("failed to create bottom screen render target");
 
     let shader = shader::Library::from_bytes(SHADER_BYTES).unwrap();
@@ -111,13 +113,17 @@ fn main() {
             y: v[1],
             z: v[2],
         },
-        color: Vec3::new(1.0, 0.7, 0.5),
+        color: {
+            // Give each vertex a slightly different color just to highlight edges/corners
+            let value = i as f32 / VERTS.len() as f32;
+            Vec3::new(1.0, 0.7 * value, 0.5)
+        },
     }) {
         vbo_data.push(vert);
     }
 
     let mut buf_info = buffer::Info::new();
-    let (attr_info, vbo_data) = prepare_vbos(&mut buf_info, &vbo_data);
+    let (attr_info, vbo_slice) = prepare_vbos(&mut buf_info, &vbo_data);
 
     // Configure the first fragment shading substage to just pass through the vertex color
     // See https://www.opengl.org/sdk/docs/man2/xhtml/glTexEnv.xml for more insight
@@ -134,16 +140,15 @@ fn main() {
         FVec3::new(0.0, 1.0, 0.0),
         CoordinateOrientation::RightHanded,
     );
-    let indecies_a = [
+    let indices: &[u8] = &[
         0, 3, 1, 1, 3, 2, // triangles making up the top (+y) facing side.
         4, 5, 7, 5, 6, 7, // bottom (-y)
         8, 11, 9, 9, 11, 10, // right (+x)
         12, 13, 15, 13, 14, 15, // left (-x)
         16, 19, 17, 17, 19, 18, // back (+z)
         20, 21, 23, 21, 22, 23, // forward (-z)
     ];
-    let mut indecies = Vec::with_capacity_in(indecies_a.len(), ctru::linear::LinearAllocator);
-    indecies.extend_from_slice(&indecies_a);
+    let indices = vbo_slice.index_buffer(indices).unwrap();
 
     while apt.main_loop() {
         hid.scan_input();
@@ -160,21 +165,12 @@ fn main() {
                     .select_render_target(target)
                     .expect("failed to set render target");
 
-                instance.bind_vertex_uniform(
-                    projection_uniform_idx,
-                    &(projection * camera_transform.clone()),
-                );
+                instance.bind_vertex_uniform(projection_uniform_idx, projection * camera_transform);
 
                 instance.set_attr_info(&attr_info);
                 unsafe {
-                    instance.draw_elements(
-                        buffer::Primitive::Triangles,
-                        &buf_info,
-                        IndexType::U16(&indecies),
-                    );
+                    instance.draw_elements(buffer::Primitive::Triangles, &buf_info, &indices);
                 }
-
-                //instance.draw_arrays(buffer::Primitive::Triangles, vbo_data);
             };
 
             let Projections {

citro3d/src/buffer.rs

@@ -5,7 +5,10 @@
 
 use std::mem::MaybeUninit;
 
+use ctru::linear::LinearAllocator;
+
 use crate::attrib;
+use crate::Error;
 
 /// Vertex buffer info. This struct is used to describe the shape of the buffer
 /// data to be sent to the GPU for rendering.
@@ -46,6 +49,62 @@ impl Slice<'_> {
     pub fn info(&self) -> &Info {
         self.buf_info
     }
+
+    /// Get an index buffer for this slice using the given indices.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if:
+    /// - any of the given indices are out of bounds.
+    /// - the given slice is too long for its length to fit in a `libc::c_int`.
+    pub fn index_buffer<I>(&self, indices: &[I]) -> Result<Vec<I, LinearAllocator>, Error>
+    where
+        I: Index + Copy + Into<libc::c_int>,
+    {
+        if libc::c_int::try_from(indices.len()).is_err() {
+            return Err(Error::InvalidSize);
+        }
+
+        for &idx in indices {
+            let idx = idx.into();
+            let len = self.len();
+            if idx >= len {
+                return Err(Error::IndexOutOfBounds { idx, len });
+            }
+        }
+
+        Ok(unsafe { self.index_buffer_unchecked(indices) })
+    }
+
+    /// Get an index buffer for this slice using the given indices without
+    /// bounds checking.
+    ///
+    /// # Safety
+    ///
+    /// If any indices are outside this buffer it can cause an invalid access by the GPU
+    /// (this crashes citra).
+    pub unsafe fn index_buffer_unchecked<I: Index + Clone>(
+        &self,
+        indices: &[I],
+    ) -> Vec<I, LinearAllocator> {
+        let mut buf = Vec::with_capacity_in(indices.len(), LinearAllocator);
+        buf.extend_from_slice(indices);
+        buf
+    }
+}
+
+/// A type that can be used as an index for indexed drawing.
+pub trait Index: crate::private::Sealed {
+    /// The data type of the index, as used by [`citro3d_sys::C3D_DrawElements`]'s `type_` parameter.
+    const TYPE: libc::c_int;
+}
+
+impl Index for u8 {
+    const TYPE: libc::c_int = citro3d_sys::C3D_UNSIGNED_BYTE as _;
+}
+
+impl Index for u16 {
+    const TYPE: libc::c_int = citro3d_sys::C3D_UNSIGNED_SHORT as _;
 }
 
 /// The geometric primitive to draw (i.e. what shapes the buffer data describes).

citro3d/src/error.rs

@@ -1,5 +1,6 @@
 //! General-purpose error and result types returned by public APIs of this crate.
 
+use core::fmt;
 use std::ffi::NulError;
 use std::num::TryFromIntError;
 use std::sync::TryLockError;
@@ -36,6 +37,13 @@ pub enum Error {
     InvalidName,
     /// The requested resource could not be found.
     NotFound,
+    /// Attempted to use an index that was out of bounds.
+    IndexOutOfBounds {
+        /// The index used.
+        idx: libc::c_int,
+        /// The length of the collection.
+        len: libc::c_int,
+    },
 }
 
 impl From<TryFromIntError> for Error {

citro3d/src/lib.rs

@@ -1,5 +1,6 @@
 #![feature(custom_test_frameworks)]
 #![test_runner(test_runner::run_gdb)]
+#![feature(allocator_api)]
 #![feature(doc_cfg)]
 #![feature(doc_auto_cfg)]
 #![doc(html_root_url = "https://rust3ds.github.io/citro3d-rs/crates")]
@@ -24,15 +25,14 @@ pub mod render;
 pub mod shader;
 pub mod texenv;
 pub mod uniform;
-mod util;
 
 use std::cell::{OnceCell, RefMut};
 use std::fmt;
 use std::rc::Rc;
 
+use ctru::linear::LinearAllocation;
 use ctru::services::gfx::Screen;
 pub use error::{Error, Result};
-use util::is_linear_ptr;
 
 use self::texenv::TexEnv;
 use self::uniform::Uniform;
@@ -42,6 +42,12 @@ pub mod macros {
     pub use citro3d_macros::*;
 }
 
+mod private {
+    pub trait Sealed {}
+    impl Sealed for u8 {}
+    impl Sealed for u16 {}
+}
+
 /// The single instance for using `citro3d`. This is the base type that an application
 /// should instantiate to use this library.
 #[non_exhaustive]
@@ -205,36 +211,33 @@ impl Instance {
     /// Draws the vertices in `buf` indexed by `indices`. `indices` must be linearly allocated
     ///
     /// # Safety
-    /// If `indices` goes out of scope before the current frame ends it will cause a use-after-free (possibly by the GPU)
-    /// If `buf` does not contain all the vertices references by `indices` it will cause an invalid access by the GPU (this crashes citra)
+    // TODO: #41 might be able to solve this:
+    /// If `indices` goes out of scope before the current frame ends it will cause a
+    /// use-after-free (possibly by the GPU).
     ///
     /// # Panics
-    /// If `indices` is not allocated in linear memory
+    ///
+    /// If the given index buffer is too long to have its length converted to `i32`.
     #[doc(alias = "C3D_DrawElements")]
-    pub unsafe fn draw_elements<'a>(
+    pub unsafe fn draw_elements<I, Indices>(
         &mut self,
         primitive: buffer::Primitive,
         buf: &buffer::Info,
-        indices: impl Into<IndexType<'a>>,
-    ) {
+        indices: &Indices,
+    ) where
+        I: buffer::Index,
+        Indices: AsRef<[I]> + LinearAllocation,
+    {
         self.set_buffer_info(buf);
-        let indices: IndexType<'a> = indices.into();
-        let elements = match indices {
-            IndexType::U16(v) => v.as_ptr() as *const _,
-            IndexType::U8(v) => v.as_ptr() as *const _,
-        };
-        assert!(
-            is_linear_ptr(elements),
-            "draw_elements requires linear allocated indices buffer"
-        );
+
+        let indices = indices.as_ref();
+        let elements = indices.as_ptr().cast();
+
         citro3d_sys::C3D_DrawElements(
             primitive as ctru_sys::GPU_Primitive_t,
-            indices.len() as i32,
+            indices.len().try_into().unwrap(),
             // flag bit for short or byte
-            match indices {
-                IndexType::U16(_) => citro3d_sys::C3D_UNSIGNED_SHORT,
-                IndexType::U8(_) => citro3d_sys::C3D_UNSIGNED_BYTE,
-            } as i32,
+            I::TYPE,
             elements,
         );
     }
@@ -321,31 +324,6 @@ impl Drop for RenderQueue {
     }
 }
 
-pub enum IndexType<'a> {
-    U16(&'a [u16]),
-    U8(&'a [u8]),
-}
-impl IndexType<'_> {
-    fn len(&self) -> usize {
-        match self {
-            IndexType::U16(a) => a.len(),
-            IndexType::U8(a) => a.len(),
-        }
-    }
-}
-
-impl<'a> From<&'a [u8]> for IndexType<'a> {
-    fn from(v: &'a [u8]) -> Self {
-        Self::U8(v)
-    }
-}
-
-impl<'a> From<&'a [u16]> for IndexType<'a> {
-    fn from(v: &'a [u16]) -> Self {
-        Self::U16(v)
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use ctru::services::gfx::Gfx;