From a84d2045f22678bf1a386c7b540ed242d73dcb53 Mon Sep 17 00:00:00 2001 From: Andrea Ciliberti Date: Sun, 24 Sep 2023 13:40:39 +0200 Subject: [PATCH] Fixed Screen unsafety on VRAM --- ctru-rs/src/console.rs | 7 ++- ctru-rs/src/services/gfx.rs | 81 ++++++++++++++++++++++++++++++----- ctru-rs/src/services/hid.rs | 22 +++++----- ctru-rs/src/services/romfs.rs | 2 +- 4 files changed, 88 insertions(+), 24 deletions(-) diff --git a/ctru-rs/src/console.rs b/ctru-rs/src/console.rs index ba85222..697b088 100644 --- a/ctru-rs/src/console.rs +++ b/ctru-rs/src/console.rs @@ -75,6 +75,11 @@ impl<'screen> Console<'screen> { /// /// [`Console`] automatically takes care of flushing and swapping buffers for its screen when printing. /// + /// # Panics + /// + /// If the [`Gfx`](crate::services::gfx::Gfx) service was initialised via [`Gfx::with_formats_vram()`](crate::services::gfx::Gfx::with_formats_vram) + /// this function will crash the program with an ARM exception. + /// /// # Example /// /// ```no_run @@ -84,7 +89,7 @@ impl<'screen> Console<'screen> { /// use ctru::services::gfx::Gfx; /// use ctru::console::Console; /// - /// // Initialize graphics. + /// // Initialize graphics (using framebuffers allocated on the HEAP). /// let gfx = Gfx::new()?; /// /// // Create a `Console` that takes control of the upper LCD screen. diff --git a/ctru-rs/src/services/gfx.rs b/ctru-rs/src/services/gfx.rs index c32a603..dc1dca6 100644 --- a/ctru-rs/src/services/gfx.rs +++ b/ctru-rs/src/services/gfx.rs @@ -37,10 +37,16 @@ pub trait Screen: private::Sealed { /// Returns the Screen side (left or right). fn side(&self) -> Side; - /// Returns a [`RawFrameBuffer`] for the screen. + /// Returns a [`RawFrameBuffer`] for the screen (if the framebuffer was allocated on the HEAP). /// - /// Note that the pointer of the framebuffer returned by this function can - /// change after each call to this function if double buffering is enabled. + /// # Notes + /// + /// The pointer of the framebuffer returned by this function can change after each call + /// to this function if double buffering is enabled, so it's suggested to NOT save it for later use. + /// + /// # Panics + /// + /// If the [`Gfx`] service was initialised via [`Gfx::with_formats_vram()`] this function will crash the program with an ARM exception. #[doc(alias = "gfxGetFramebuffer")] fn raw_framebuffer(&mut self) -> RawFrameBuffer { let mut width: u16 = 0; @@ -251,7 +257,8 @@ impl Gfx { /// /// # Notes /// - /// It's the same as calling: + /// The new `Gfx` instance will allocate the needed framebuffers in the CPU-GPU shared memory region (to ensure compatibiltiy with all possible uses of the `Gfx` service). + /// As such, it's the same as calling: /// /// ```no_run /// # use std::error::Error; @@ -260,12 +267,14 @@ impl Gfx { /// # use ctru::services::gfx::Gfx; /// # use ctru::services::gspgpu::FramebufferFormat; /// # - /// Gfx::with_formats(FramebufferFormat::Bgr8, FramebufferFormat::Bgr8, false)?; + /// Gfx::with_formats_shared(FramebufferFormat::Bgr8, FramebufferFormat::Bgr8)?; /// # /// # Ok(()) /// # } /// ``` /// + /// Have a look at [`Gfx::with_formats_vram()`] if you aren't interested in manipulating the framebuffers using the CPU. + /// /// # Example /// /// ```no_run @@ -281,10 +290,10 @@ impl Gfx { /// ``` #[doc(alias = "gfxInit")] pub fn new() -> Result { - Gfx::with_formats(FramebufferFormat::Bgr8, FramebufferFormat::Bgr8, false) + Gfx::with_formats_shared(FramebufferFormat::Bgr8, FramebufferFormat::Bgr8) } - /// Initialize a new service handle with the chosen framebuffer formats for the top and bottom screens. + /// Initialize a new service handle with the chosen framebuffer formats on the HEAP for the top and bottom screens. /// /// Use [`Gfx::new()`] instead of this function to initialize the module with default parameters /// @@ -298,21 +307,71 @@ impl Gfx { /// /// // Top screen uses RGBA8, bottom screen uses RGB565. /// // The screen buffers are allocated in the standard HEAP memory, and not in VRAM. - /// let gfx = Gfx::with_formats(FramebufferFormat::Rgba8, FramebufferFormat::Rgb565, false)?; + /// let gfx = Gfx::with_formats_shared(FramebufferFormat::Rgba8, FramebufferFormat::Rgb565)?; + /// # + /// # Ok(()) + /// # } + /// ``` + #[doc(alias = "gfxInit")] + pub fn with_formats_shared( + top_fb_fmt: FramebufferFormat, + bottom_fb_fmt: FramebufferFormat, + ) -> Result { + let handler = ServiceReference::new( + &GFX_ACTIVE, + || unsafe { + ctru_sys::gfxInit(top_fb_fmt.into(), bottom_fb_fmt.into(), false); + + Ok(()) + }, + || unsafe { ctru_sys::gfxExit() }, + )?; + + Ok(Self { + top_screen: RefCell::new(TopScreen::new()), + bottom_screen: RefCell::new(BottomScreen), + _service_handler: handler, + }) + } + + /// Initialize a new service handle with the chosen framebuffer formats on the VRAM for the top and bottom screens. + /// + /// # Notes + /// + /// Though unsafe to do so, it's suggested to use VRAM buffers when working exclusively with the GPU, + /// since they result in faster performance and less memory waste. + /// + /// # Safety + /// + /// By initializing the [`Gfx`] service as such, all functionality that relies on CPU manipulation of the framebuffers will + /// be completely unavailable (usually resulting in an ARM panic if wrongly used). + /// + /// Things such as [`Console`](crate::console::Console) and [`Screen::raw_framebuffer()`] will result in ARM exceptions. + /// + /// # Example + /// + /// ```no_run + /// # use std::error::Error; + /// # fn main() -> Result<(), Box> { + /// # + /// use ctru::services::{gfx::Gfx, gspgpu::FramebufferFormat}; + /// + /// // Top screen uses RGBA8, bottom screen uses RGB565. + /// // The screen buffers are allocated in the in VRAM, so they will NOT be accessible from the CPU. + /// let gfx = unsafe { Gfx::with_formats_vram(FramebufferFormat::Rgba8, FramebufferFormat::Rgb565)? }; /// # /// # Ok(()) /// # } /// ``` #[doc(alias = "gfxInit")] - pub fn with_formats( + pub unsafe fn with_formats_vram( top_fb_fmt: FramebufferFormat, bottom_fb_fmt: FramebufferFormat, - use_vram_buffers: bool, ) -> Result { let handler = ServiceReference::new( &GFX_ACTIVE, || unsafe { - ctru_sys::gfxInit(top_fb_fmt.into(), bottom_fb_fmt.into(), use_vram_buffers); + ctru_sys::gfxInit(top_fb_fmt.into(), bottom_fb_fmt.into(), true); Ok(()) }, diff --git a/ctru-rs/src/services/hid.rs b/ctru-rs/src/services/hid.rs index 0634091..5daabb1 100644 --- a/ctru-rs/src/services/hid.rs +++ b/ctru-rs/src/services/hid.rs @@ -400,13 +400,13 @@ impl Hid { pub fn set_accelerometer(&mut self, enabled: bool) { if enabled { let _ = unsafe { ctru_sys::HIDUSER_EnableAccelerometer() }; - } else { + } else { let _ = unsafe { ctru_sys::HIDUSER_DisableAccelerometer() }; } self.active_accelerometer = enabled; } - + /// Activate/deactivate the console's gyroscopic sensor. /// /// # Example @@ -429,7 +429,7 @@ impl Hid { pub fn set_gyroscope(&mut self, enabled: bool) { if enabled { let _ = unsafe { ctru_sys::HIDUSER_EnableGyroscope() }; - } else { + } else { let _ = unsafe { ctru_sys::HIDUSER_DisableGyroscope() }; } @@ -437,9 +437,9 @@ impl Hid { } /// Returns the acceleration vector (x,y,z) registered by the accelerometer. - /// + /// /// # Errors - /// + /// /// This function will return an error if the accelerometer was not previously enabled. /// Have a look at [`Hid::set_accelerometer()`] to enable the accelerometer. /// @@ -451,10 +451,10 @@ impl Hid { /// # /// use ctru::services::hid::Hid; /// let mut hid = Hid::new()?; - /// + /// /// // The accelerometer will start to register movements. /// hid.set_accelerometer(true); - /// + /// /// // It's necessary to run `scan_input()` to update the accelerometer's readings. /// hid.scan_input(); /// @@ -480,9 +480,9 @@ impl Hid { } /// Returns the angular rate (x,y,z) registered by the gyroscope. - /// + /// /// # Errors - /// + /// /// This function returns an error if the gyroscope was not previously enabled. /// Have a look at [`Hid::set_gyroscope()`]. /// @@ -494,10 +494,10 @@ impl Hid { /// # /// use ctru::services::hid::Hid; /// let mut hid = Hid::new()?; - /// + /// /// // The gyroscope will start to register positions. /// hid.set_gyroscope(true); - /// + /// /// // It's necessary to run `scan_input()` to update the gyroscope's readings. /// hid.scan_input(); /// diff --git a/ctru-rs/src/services/romfs.rs b/ctru-rs/src/services/romfs.rs index fd6064b..572aea6 100644 --- a/ctru-rs/src/services/romfs.rs +++ b/ctru-rs/src/services/romfs.rs @@ -87,7 +87,7 @@ mod tests { fn romfs_lock() { let romfs = RomFS::new().unwrap(); - let _value = *ROMFS_ACTIVE.lock().unwrap(); + let _value = *ROMFS_ACTIVE.try_lock().unwrap(); drop(romfs); }