From 700646e2174d979262c03bcd32e8985851cf93e9 Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Sat, 6 Jan 2024 21:06:54 -0500 Subject: [PATCH] Use an Rc to keep render queue alive --- citro3d/examples/triangle.rs | 11 ++++--- citro3d/src/lib.rs | 59 +++++++++++++++++++++++++++++++++++- citro3d/src/render.rs | 19 ++++++------ 3 files changed, 74 insertions(+), 15 deletions(-) diff --git a/citro3d/examples/triangle.rs b/citro3d/examples/triangle.rs index 305a9f8..6559f9c 100644 --- a/citro3d/examples/triangle.rs +++ b/citro3d/examples/triangle.rs @@ -65,17 +65,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(); diff --git a/citro3d/src/lib.rs b/citro3d/src/lib.rs index c18e6fc..ea1927a 100644 --- a/citro3d/src/lib.rs +++ b/citro3d/src/lib.rs @@ -18,9 +18,11 @@ pub mod shader; pub mod texenv; pub mod uniform; -use std::cell::OnceCell; +use std::cell::{OnceCell, RefMut}; use std::fmt; +use std::rc::Rc; +use ctru::services::gfx::Screen; pub use error::{Error, Result}; use self::texenv::TexEnv; @@ -37,8 +39,15 @@ pub mod macros { #[must_use] pub struct Instance { texenvs: [OnceCell; texenv::TEXENV_COUNT], + queue: Rc, } +/// Representation of `citro3d`'s internal render queue. This is something that +/// lives in the global context, but it keeps references to resources that are +/// used for rendering, so it's useful for us to have something to represent its +/// lifetime. +struct RenderQueue; + impl fmt::Debug for Instance { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("Instance").finish_non_exhaustive() @@ -73,12 +82,31 @@ impl Instance { OnceCell::new(), OnceCell::new(), ], + queue: Rc::new(RenderQueue), }) } else { Err(Error::FailedToInitialize) } } + /// Create a new render target with the specified size, color format, + /// and depth format. + /// + /// # Errors + /// + /// Fails if the target could not be created with the given parameters. + #[doc(alias = "C3D_RenderTargetCreate")] + #[doc(alias = "C3D_RenderTargetSetOutput")] + pub fn render_target<'screen>( + &self, + width: usize, + height: usize, + screen: RefMut<'screen, dyn Screen>, + depth_format: Option, + ) -> Result> { + render::Target::new(width, height, screen, depth_format, Rc::clone(&self.queue)) + } + /// Select the given render target for drawing the frame. /// /// # Errors @@ -229,11 +257,40 @@ impl Instance { } } +// This only exists to be an alias, which admittedly is kinda silly. The default +// impl should be equivalent though, since RenderQueue has a drop impl too. impl Drop for Instance { #[doc(alias = "C3D_Fini")] + fn drop(&mut self) {} +} + +impl Drop for RenderQueue { fn drop(&mut self) { unsafe { citro3d_sys::C3D_Fini(); } } } + +#[cfg(test)] +mod tests { + use ctru::services::gfx::Gfx; + + use super::*; + + #[test] + fn select_render_target() { + let gfx = Gfx::new().unwrap(); + let screen = gfx.top_screen.borrow_mut(); + + let mut instance = Instance::new().unwrap(); + let target = instance.render_target(10, 10, screen, None).unwrap(); + + instance.select_render_target(&target).unwrap(); + + // Check that we don't get a double-free or use-after-free by dropping + // the global instance before dropping the target. + drop(instance); + drop(target); + } +} diff --git a/citro3d/src/render.rs b/citro3d/src/render.rs index 03d001d..4960447 100644 --- a/citro3d/src/render.rs +++ b/citro3d/src/render.rs @@ -2,6 +2,7 @@ //! of data to the GPU, including the format of color and depth data to be rendered. use std::cell::RefMut; +use std::rc::Rc; use citro3d_sys::{ C3D_RenderTarget, C3D_RenderTargetCreate, C3D_RenderTargetDelete, C3D_DEPTHTYPE, @@ -10,7 +11,7 @@ use ctru::services::gfx::Screen; use ctru::services::gspgpu::FramebufferFormat; use ctru_sys::{GPU_COLORBUF, GPU_DEPTHBUF}; -use crate::{Error, Result}; +use crate::{Error, RenderQueue, Result}; mod transfer; @@ -22,6 +23,7 @@ pub struct Target<'screen> { // This is unused after construction, but ensures unique access to the // screen this target writes to during rendering _screen: RefMut<'screen, dyn Screen>, + _queue: Rc, } impl Drop for Target<'_> { @@ -34,19 +36,15 @@ impl Drop for Target<'_> { } impl<'screen> Target<'screen> { - /// Create a new render target with the specified size, color format, - /// and depth format. - /// - /// # Errors - /// - /// Fails if the target could not be created. - #[doc(alias = "C3D_RenderTargetCreate")] - #[doc(alias = "C3D_RenderTargetSetOutput")] - pub fn new( + /// Create a new render target with the given parameters. This takes a + /// [`RenderQueue`] parameter to make sure this [`Target`] doesn't outlive + /// the render queue. + pub(crate) fn new( width: usize, height: usize, screen: RefMut<'screen, dyn Screen>, depth_format: Option, + queue: Rc, ) -> Result { let color_format: ColorFormat = screen.framebuffer_format().into(); @@ -80,6 +78,7 @@ impl<'screen> Target<'screen> { Ok(Self { raw, _screen: screen, + _queue: queue, }) }