From a9279af35e4da04518613c33835ac517e983a2c0 Mon Sep 17 00:00:00 2001 From: Jay Oster Date: Wed, 6 Sep 2023 16:14:21 -0700 Subject: [PATCH 1/3] Add API to access `Bitmap` pixel and mask data # Breaking changes - Adds a lifetime to `BitmapData`, so it is no longer easy to store in persistent structs. `BitmapData` is not allowed to outlive the `Bitmap` that it came from. - Makes all `BitmapData` fields private so they cannot be changed by callers. This is a safety invariant for accessing the underlying pixel and mask buffers. # New features - `BitmapData::pixels()` and `BitmapData::mask()` provides access to the underlying pixel and mask data as byte slices. - `BitmapData` has getter methods for its fields. - Adds `Bitmap::get_data_mut()` to gain mutable access to the pixel and mask data through `BitmapDataMut`. - `BitmapData::to_view()` and `BitmapDataMut::to_view()` are available to create a view of the `BitmapData` or `BitmapDataMut` that is not tied to the lifetime of the `Bitmap` that owns the data. `BitmapDataView` relinquishes access to the pixel and mask data so that it can be easily persisted and outlive the `Bitmap`. # Oddities - `BitmapInner` has a number of public methods, but this type is never made accessible publically. These methods should either be made private or `pub(crate)`. Also, `BitmapInner` supports more methods than the public `Bitmap` type. - `Bitmap::get_data_mut()` technically does not need an exclusive reference to `Self`, since we can rely on `borrow_mut()` panicking at runtime when attempting to acquire two `BitmapDataMut`s from the same `Bitmap`. But it is a better user experience to enforce the invariant at compile time. - `BitmapDataMut` cannot exist more than once for the same `Bitmap`, as mentioned above. This would cause UB by allowing mutable aliasing by e.g. holding two references from `a.pixels_mut()` and `b.pixels_mut()`. These references will point to the same location in memory. --- examples/sprite_game.rs | 24 ++--- src/graphics.rs | 197 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 203 insertions(+), 18 deletions(-) diff --git a/examples/sprite_game.rs b/examples/sprite_game.rs index 576408f..86d8e53 100644 --- a/examples/sprite_game.rs +++ b/examples/sprite_game.rs @@ -7,7 +7,7 @@ use { anyhow::Error, crankstart::{ crankstart_game, - graphics::{rect_make, Bitmap, BitmapData, Graphics, LCDBitmapFlip, PDRect}, + graphics::{rect_make, Bitmap, BitmapDataView, Graphics, LCDBitmapFlip, PDRect}, log_to_console, sprite::{Sprite, SpriteCollider, SpriteManager}, system::{PDButtons, System}, @@ -142,7 +142,7 @@ impl PlayerHandler { } struct BulletHandler { - bullet_image_data: BitmapData, + bullet_image_data: BitmapDataView, } impl BulletHandler { @@ -181,7 +181,7 @@ impl BulletHandler { } struct EnemyPlaneHandler { - enemy_image_data: BitmapData, + enemy_image_data: BitmapDataView, } impl EnemyPlaneHandler { @@ -202,7 +202,7 @@ impl EnemyPlaneHandler { } struct BackgroundPlaneHandler { - background_plane_image_data: BitmapData, + background_plane_image_data: BitmapDataView, } impl BackgroundPlaneHandler { @@ -304,7 +304,7 @@ impl SpriteGame { let sprite_manager = SpriteManager::get_mut(); let mut background = sprite_manager.new_sprite()?; let background_image = graphics.load_bitmap("sprite_game_images/background")?; - let background_image_data = background_image.get_data()?; + let background_image_data = background_image.get_data()?.to_view(); let bounds = rect_make(0.0, 0.0, 400.0, 240.0); background.set_bounds(&bounds)?; background.set_z_index(0)?; @@ -320,7 +320,7 @@ impl SpriteGame { // setup player let mut player = sprite_manager.new_sprite()?; let player_image = graphics.load_bitmap("sprite_game_images/player")?; - let player_image_data = player_image.get_data()?; + let player_image_data = player_image.get_data()?.to_view(); player.set_image(player_image, LCDBitmapFlip::kBitmapUnflipped)?; let center_x: f32 = 200.0 - player_image_data.width as f32 / 2.0; let center_y: f32 = 180.0 - player_image_data.height as f32 / 2.0; @@ -338,14 +338,14 @@ impl SpriteGame { player.move_to(center_x, center_y)?; let bullet_image = graphics.load_bitmap("sprite_game_images/doubleBullet")?; - let bullet_image_data = bullet_image.get_data()?; + let bullet_image_data = bullet_image.get_data()?.to_view(); let enemy_plane_image = graphics.load_bitmap("sprite_game_images/plane1")?; - let enemy_image_data = enemy_plane_image.get_data()?; + let enemy_image_data = enemy_plane_image.get_data()?.to_view(); let enemy_plane_handler = EnemyPlaneHandler { enemy_image_data }; let background_plane_image = graphics.load_bitmap("sprite_game_images/plane2")?; - let background_plane_image_data = background_plane_image.get_data()?; + let background_plane_image_data = background_plane_image.get_data()?.to_view(); let background_plane_handler = BackgroundPlaneHandler { background_plane_image_data, }; @@ -392,7 +392,7 @@ impl SpriteGame { fn player_fire(&mut self) -> Result<(), Error> { let sprite_manager = SpriteManager::get_mut(); let player_bounds = self.player.get_bounds()?; - let bullet_image_data = self.bullet_image.get_data()?; + let bullet_image_data = self.bullet_image.get_data()?.to_view(); let x = player_bounds.x + player_bounds.width / 2.0 - bullet_image_data.width as f32 / 2.0; let y = player_bounds.y; @@ -444,7 +444,7 @@ impl SpriteGame { let sprite_manager = SpriteManager::get_mut(); let mut plane = sprite_manager.new_sprite()?; plane.set_collision_response_type(Some(Box::new(OverlapCollider {})))?; - let plane_image_data = self.enemy_plane_image.get_data()?; + let plane_image_data = self.enemy_plane_image.get_data()?.to_view(); plane.set_image( self.enemy_plane_image.clone(), LCDBitmapFlip::kBitmapUnflipped, @@ -480,7 +480,7 @@ impl SpriteGame { fn create_background_plane(&mut self) -> Result<(), Error> { let sprite_manager = SpriteManager::get_mut(); let mut plane = sprite_manager.new_sprite()?; - let plane_image_data = self.background_plane_image.get_data()?; + let plane_image_data = self.background_plane_image.get_data()?.to_view(); plane.set_image( self.background_plane_image.clone(), LCDBitmapFlip::kBitmapUnflipped, diff --git a/src/graphics.rs b/src/graphics.rs index 0bd2733..b9ecc3b 100644 --- a/src/graphics.rs +++ b/src/graphics.rs @@ -6,7 +6,7 @@ use { }, alloc::{format, rc::Rc}, anyhow::{anyhow, ensure, Error}, - core::{cell::RefCell, ops::RangeInclusive, ptr, slice}, + core::{cell::RefCell, convert::TryFrom, marker::PhantomData, ops::RangeInclusive, ptr, slice}, crankstart_sys::{ctypes::c_int, LCDBitmapTable, LCDPattern}, cstr_core::{CStr, CString}, euclid::default::{Point2D, Vector2D}, @@ -46,24 +46,184 @@ impl From for usize { } #[derive(Debug)] -pub struct BitmapData { +pub struct BitmapDataView { pub width: c_int, pub height: c_int, pub rowbytes: c_int, pub hasmask: bool, } +#[derive(Debug)] +pub struct BitmapData<'bitmap> { + width: c_int, + height: c_int, + rowbytes: c_int, + mask: Option<*const u8>, + data: *const u8, + _phantom: PhantomData<&'bitmap [u8]>, +} + +impl<'bitmap> BitmapData<'bitmap> { + /// Create a view of the bitmap data that can be stored without the `'bitmap` lifetime. + pub fn to_view(&self) -> BitmapDataView { + BitmapDataView { + width: self.width, + height: self.height, + rowbytes: self.rowbytes, + hasmask: self.hasmask(), + } + } + + /// Getter method for the bitmap width. + pub fn width(&self) -> i32 { + self.width + } + + /// Getter method for the bitmap height. + pub fn height(&self) -> i32 { + self.height + } + + /// Getter method for the bitmap bytes-per-row count. + pub fn rowbytes(&self) -> i32 { + self.rowbytes + } + + /// Check if the bitmap has a mask. + pub fn hasmask(&self) -> bool { + self.mask.is_some() + } + + /// Get access to the bitmap pixels. + pub fn pixels(&self) -> &'bitmap [u8] { + // Carefully construct the buffer length, ensuring no integer overflow is possible. + let length = usize::try_from(self.rowbytes.checked_mul(self.height).unwrap()).unwrap(); + + // SAFETY: The length of the slice does not overflow the buffer size. This invariant is + // enforced by making `self.rowbytes` and `self.height` read-only, and trusting that the + // values returned by the Playdate SDK are valid. The slice lives at least at long as the + // owning `Bitmap`, enforced by the 'bitmap lifetime. + unsafe { &*ptr::slice_from_raw_parts(self.data, length) } + } + + /// Get access to the bitmap mask data (if any). + pub fn mask(&self) -> Option<&'bitmap [u8]> { + // Carefully construct the buffer length, ensuring no integer overflow is possible. + let length = usize::try_from(self.rowbytes.checked_mul(self.height)?).ok()?; + + // SAFETY: The length of the slice does not overflow the buffer size. This invariant is + // enforced by making `self.rowbytes` and `self.height` read-only, and trusting that the + // values returned by the Playdate SDK are valid. The slice lives at least at long as the + // owning `Bitmap`, enforced by the 'bitmap lifetime. + self.mask + .map(|mask| unsafe { &*ptr::slice_from_raw_parts(mask, length) }) + } +} + +#[derive(Debug)] +pub struct BitmapDataMut<'bitmap> { + width: c_int, + height: c_int, + rowbytes: c_int, + mask: Option<*mut u8>, + data: *mut u8, + _phantom: PhantomData<&'bitmap mut [u8]>, +} + +impl<'bitmap> BitmapDataMut<'bitmap> { + /// Create a view of the bitmap data that can be stored without the `'bitmap` lifetime. + pub fn to_view(&self) -> BitmapDataView { + BitmapDataView { + width: self.width, + height: self.height, + rowbytes: self.rowbytes, + hasmask: self.hasmask(), + } + } + + /// Getter method for the bitmap width. + pub fn width(&self) -> i32 { + self.width + } + + /// Getter method for the bitmap height. + pub fn height(&self) -> i32 { + self.height + } + + /// Getter method for the bitmap bytes-per-row count. + pub fn rowbytes(&self) -> i32 { + self.rowbytes + } + + /// Check if the bitmap has a mask. + pub fn hasmask(&self) -> bool { + self.mask.is_some() + } + + /// Get access to the bitmap pixels. + pub fn pixels(&self) -> &'bitmap [u8] { + // Carefully construct the buffer length, ensuring no integer overflow is possible. + let length = usize::try_from(self.rowbytes.checked_mul(self.height).unwrap()).unwrap(); + + // SAFETY: The length of the slice does not overflow the buffer size. This invariant is + // enforced by making `self.rowbytes` and `self.height` read-only, and trusting that the + // values returned by the Playdate SDK are valid. The slice lives at least at long as the + // owning `Bitmap`, enforced by the 'bitmap lifetime. + unsafe { &*ptr::slice_from_raw_parts(self.data, length) } + } + + /// Get mutable access to the bitmap pixels. + pub fn pixels_mut(&mut self) -> &'bitmap mut [u8] { + // Carefully construct the buffer length, ensuring no integer overflow is possible. + let length = usize::try_from(self.rowbytes.checked_mul(self.height).unwrap()).unwrap(); + + // SAFETY: The length of the slice does not overflow the buffer size. This invariant is + // enforced by making `self.rowbytes` and `self.height` read-only, and trusting that the + // values returned by the Playdate SDK are valid. The slice lives at least at long as the + // owning `Bitmap`, enforced by the 'bitmap lifetime. + unsafe { &mut *ptr::slice_from_raw_parts_mut(self.data, length) } + } + + /// Get access to the bitmap mask data (if any). + pub fn mask(&self) -> Option<&'bitmap [u8]> { + // Carefully construct the buffer length, ensuring no integer overflow is possible. + let length = usize::try_from(self.rowbytes.checked_mul(self.height)?).ok()?; + + // SAFETY: The length of the slice does not overflow the buffer size. This invariant is + // enforced by making `self.rowbytes` and `self.height` read-only, and trusting that the + // values returned by the Playdate SDK are valid. The slice lives at least at long as the + // owning `Bitmap`, enforced by the 'bitmap lifetime. + self.mask + .map(|mask| unsafe { &*ptr::slice_from_raw_parts(mask, length) }) + } + + /// Get mutable access to the bitmap mask data (if any). + pub fn mask_mut(&mut self) -> Option<&'bitmap mut [u8]> { + // Carefully construct the buffer length, ensuring no integer overflow is possible. + let length = usize::try_from(self.rowbytes.checked_mul(self.height)?).ok()?; + + // SAFETY: The length of the slice does not overflow the buffer size. This invariant is + // enforced by making `self.rowbytes` and `self.height` read-only, and trusting that the + // values returned by the Playdate SDK are valid. The slice lives at least at long as the + // owning `Bitmap`, enforced by the 'bitmap lifetime. + self.mask + .map(|mask| unsafe { &mut *ptr::slice_from_raw_parts_mut(mask, length) }) + } +} + #[derive(Debug)] pub struct BitmapInner { pub(crate) raw_bitmap: *mut crankstart_sys::LCDBitmap, } impl BitmapInner { - pub fn get_data(&self) -> Result { + fn get_data_inner(&self) -> Result<(c_int, c_int, c_int, *mut u8, *mut u8), Error> { let mut width = 0; let mut height = 0; let mut rowbytes = 0; let mut mask_ptr = ptr::null_mut(); + let mut data_ptr = ptr::null_mut(); pd_func_caller!( (*Graphics::get_ptr()).getBitmapData, self.raw_bitmap, @@ -71,13 +231,34 @@ impl BitmapInner { &mut height, &mut rowbytes, &mut mask_ptr, - ptr::null_mut(), + &mut data_ptr, )?; + Ok((width, height, rowbytes, mask_ptr, data_ptr)) + } + + pub fn get_data<'bitmap>(&self) -> Result, Error> { + let (width, height, rowbytes, mask, data) = self.get_data_inner()?; + Ok(BitmapData { width, height, rowbytes, - hasmask: mask_ptr != ptr::null_mut(), + mask: (!mask.is_null()).then_some(mask), + data, + _phantom: PhantomData, + }) + } + + pub fn get_data_mut<'bitmap>(&mut self) -> Result, Error> { + let (width, height, rowbytes, mask, data) = self.get_data_inner()?; + + Ok(BitmapDataMut { + width, + height, + rowbytes, + mask: (!mask.is_null()).then_some(mask), + data, + _phantom: PhantomData, }) } @@ -262,10 +443,14 @@ impl Bitmap { } } - pub fn get_data(&self) -> Result { + pub fn get_data(&self) -> Result, Error> { self.inner.borrow().get_data() } + pub fn get_data_mut(&mut self) -> Result, Error> { + self.inner.borrow_mut().get_data_mut() + } + pub fn draw(&self, location: ScreenPoint, flip: LCDBitmapFlip) -> Result<(), Error> { self.inner.borrow().draw(location, flip) } From 5b00e9c025d0d3cc488fd1188633566c0ee0dc88 Mon Sep 17 00:00:00 2001 From: Jay Oster Date: Thu, 7 Sep 2023 18:08:13 -0700 Subject: [PATCH 2/3] Fix UB caused by Bitmap methods with interior mutability A full analysis is provided in a PR comment. The TLDR is: - Methods with interior mutability have been changed to take exclusive borrows. - `BitmapData` and `BitmapDataMut` now keep the `RefCell` borrows alive so that mutating cloned bitmaps will panic at runtime if the borrow is still held. --- src/graphics.rs | 81 ++++++++++++++++++++++++++++--------------------- 1 file changed, 47 insertions(+), 34 deletions(-) diff --git a/src/graphics.rs b/src/graphics.rs index b9ecc3b..eb93375 100644 --- a/src/graphics.rs +++ b/src/graphics.rs @@ -6,7 +6,12 @@ use { }, alloc::{format, rc::Rc}, anyhow::{anyhow, ensure, Error}, - core::{cell::RefCell, convert::TryFrom, marker::PhantomData, ops::RangeInclusive, ptr, slice}, + core::{ + cell::{Ref, RefCell, RefMut}, + convert::TryFrom, + ops::RangeInclusive, + ptr, slice, + }, crankstart_sys::{ctypes::c_int, LCDBitmapTable, LCDPattern}, cstr_core::{CStr, CString}, euclid::default::{Point2D, Vector2D}, @@ -60,10 +65,10 @@ pub struct BitmapData<'bitmap> { rowbytes: c_int, mask: Option<*const u8>, data: *const u8, - _phantom: PhantomData<&'bitmap [u8]>, + inner: Ref<'bitmap, BitmapInner>, } -impl<'bitmap> BitmapData<'bitmap> { +impl BitmapData<'_> { /// Create a view of the bitmap data that can be stored without the `'bitmap` lifetime. pub fn to_view(&self) -> BitmapDataView { BitmapDataView { @@ -95,7 +100,7 @@ impl<'bitmap> BitmapData<'bitmap> { } /// Get access to the bitmap pixels. - pub fn pixels(&self) -> &'bitmap [u8] { + pub fn pixels(&self) -> &[u8] { // Carefully construct the buffer length, ensuring no integer overflow is possible. let length = usize::try_from(self.rowbytes.checked_mul(self.height).unwrap()).unwrap(); @@ -107,7 +112,7 @@ impl<'bitmap> BitmapData<'bitmap> { } /// Get access to the bitmap mask data (if any). - pub fn mask(&self) -> Option<&'bitmap [u8]> { + pub fn mask(&self) -> Option<&[u8]> { // Carefully construct the buffer length, ensuring no integer overflow is possible. let length = usize::try_from(self.rowbytes.checked_mul(self.height)?).ok()?; @@ -127,10 +132,10 @@ pub struct BitmapDataMut<'bitmap> { rowbytes: c_int, mask: Option<*mut u8>, data: *mut u8, - _phantom: PhantomData<&'bitmap mut [u8]>, + inner: RefMut<'bitmap, BitmapInner>, } -impl<'bitmap> BitmapDataMut<'bitmap> { +impl BitmapDataMut<'_> { /// Create a view of the bitmap data that can be stored without the `'bitmap` lifetime. pub fn to_view(&self) -> BitmapDataView { BitmapDataView { @@ -162,7 +167,7 @@ impl<'bitmap> BitmapDataMut<'bitmap> { } /// Get access to the bitmap pixels. - pub fn pixels(&self) -> &'bitmap [u8] { + pub fn pixels(&self) -> &[u8] { // Carefully construct the buffer length, ensuring no integer overflow is possible. let length = usize::try_from(self.rowbytes.checked_mul(self.height).unwrap()).unwrap(); @@ -174,7 +179,7 @@ impl<'bitmap> BitmapDataMut<'bitmap> { } /// Get mutable access to the bitmap pixels. - pub fn pixels_mut(&mut self) -> &'bitmap mut [u8] { + pub fn pixels_mut(&mut self) -> &mut [u8] { // Carefully construct the buffer length, ensuring no integer overflow is possible. let length = usize::try_from(self.rowbytes.checked_mul(self.height).unwrap()).unwrap(); @@ -186,7 +191,7 @@ impl<'bitmap> BitmapDataMut<'bitmap> { } /// Get access to the bitmap mask data (if any). - pub fn mask(&self) -> Option<&'bitmap [u8]> { + pub fn mask(&self) -> Option<&[u8]> { // Carefully construct the buffer length, ensuring no integer overflow is possible. let length = usize::try_from(self.rowbytes.checked_mul(self.height)?).ok()?; @@ -199,7 +204,7 @@ impl<'bitmap> BitmapDataMut<'bitmap> { } /// Get mutable access to the bitmap mask data (if any). - pub fn mask_mut(&mut self) -> Option<&'bitmap mut [u8]> { + pub fn mask_mut(&mut self) -> Option<&mut [u8]> { // Carefully construct the buffer length, ensuring no integer overflow is possible. let length = usize::try_from(self.rowbytes.checked_mul(self.height)?).ok()?; @@ -218,7 +223,7 @@ pub struct BitmapInner { } impl BitmapInner { - fn get_data_inner(&self) -> Result<(c_int, c_int, c_int, *mut u8, *mut u8), Error> { + fn get_data(inner: Ref<'_, Self>) -> Result, Error> { let mut width = 0; let mut height = 0; let mut rowbytes = 0; @@ -226,39 +231,47 @@ impl BitmapInner { let mut data_ptr = ptr::null_mut(); pd_func_caller!( (*Graphics::get_ptr()).getBitmapData, - self.raw_bitmap, + inner.raw_bitmap, &mut width, &mut height, &mut rowbytes, &mut mask_ptr, &mut data_ptr, )?; - Ok((width, height, rowbytes, mask_ptr, data_ptr)) - } - - pub fn get_data<'bitmap>(&self) -> Result, Error> { - let (width, height, rowbytes, mask, data) = self.get_data_inner()?; Ok(BitmapData { width, height, rowbytes, - mask: (!mask.is_null()).then_some(mask), - data, - _phantom: PhantomData, + mask: (!mask_ptr.is_null()).then_some(mask_ptr), + data: data_ptr, + inner, }) } - pub fn get_data_mut<'bitmap>(&mut self) -> Result, Error> { - let (width, height, rowbytes, mask, data) = self.get_data_inner()?; + fn get_data_mut(inner: RefMut<'_, Self>) -> Result, Error> { + let mut width = 0; + let mut height = 0; + let mut rowbytes = 0; + let mut mask_ptr = ptr::null_mut(); + let mut data_ptr = ptr::null_mut(); + pd_func_caller!( + (*Graphics::get_ptr()).getBitmapData, + inner.raw_bitmap, + &mut width, + &mut height, + &mut rowbytes, + &mut mask_ptr, + &mut data_ptr, + )?; Ok(BitmapDataMut { width, height, rowbytes, - mask: (!mask.is_null()).then_some(mask), - data, - _phantom: PhantomData, + mask: (!mask_ptr.is_null()).then_some(mask_ptr), + data: data_ptr, + inner, }) } @@ -335,7 +348,7 @@ impl BitmapInner { Ok(()) } - pub fn clear(&self, color: LCDColor) -> Result<(), Error> { + pub fn clear(&mut self, color: LCDColor) -> Result<(), Error> { pd_func_caller!( (*Graphics::get_ptr()).clearBitmap, self.raw_bitmap, @@ -377,7 +390,7 @@ impl BitmapInner { Ok(LCDColor::Pattern(pattern)) } - pub fn load(&self, path: &str) -> Result<(), Error> { + pub fn load(&mut self, path: &str) -> Result<(), Error> { let c_path = CString::new(path).map_err(Error::msg)?; let mut out_err: *const crankstart_sys::ctypes::c_char = ptr::null_mut(); let graphics = Graphics::get(); @@ -444,11 +457,11 @@ impl Bitmap { } pub fn get_data(&self) -> Result, Error> { - self.inner.borrow().get_data() + BitmapInner::get_data(self.inner.borrow()) } pub fn get_data_mut(&mut self) -> Result, Error> { - self.inner.borrow_mut().get_data_mut() + BitmapInner::get_data_mut(self.inner.borrow_mut()) } pub fn draw(&self, location: ScreenPoint, flip: LCDBitmapFlip) -> Result<(), Error> { @@ -491,8 +504,8 @@ impl Bitmap { self.inner.borrow().tile(location, size, flip) } - pub fn clear(&self, color: LCDColor) -> Result<(), Error> { - self.inner.borrow().clear(color) + pub fn clear(&mut self, color: LCDColor) -> Result<(), Error> { + self.inner.borrow_mut().clear(color) } pub fn transform(&self, rotation: f32, scale: Vector2D) -> Result { @@ -506,8 +519,8 @@ impl Bitmap { self.inner.borrow().into_color(bitmap, top_left) } - pub fn load(&self, path: &str) -> Result<(), Error> { - self.inner.borrow().load(path) + pub fn load(&mut self, path: &str) -> Result<(), Error> { + self.inner.borrow_mut().load(path) } pub fn check_mask_collision( From 6064d9f97986529a0af728e7e6fb71e35ef35d59 Mon Sep 17 00:00:00 2001 From: Jay Oster Date: Sat, 9 Sep 2023 14:10:38 -0700 Subject: [PATCH 3/3] Make Bitmap inner borrow errors non-panicking --- src/graphics.rs | 65 ++++++++++++++++++++++++++++++++++--------------- 1 file changed, 46 insertions(+), 19 deletions(-) diff --git a/src/graphics.rs b/src/graphics.rs index eb93375..b11860e 100644 --- a/src/graphics.rs +++ b/src/graphics.rs @@ -457,19 +457,25 @@ impl Bitmap { } pub fn get_data(&self) -> Result, Error> { - BitmapInner::get_data(self.inner.borrow()) + BitmapInner::get_data(self.inner.try_borrow().map_err(Error::msg)?) } pub fn get_data_mut(&mut self) -> Result, Error> { - BitmapInner::get_data_mut(self.inner.borrow_mut()) + BitmapInner::get_data_mut(self.inner.try_borrow_mut().map_err(Error::msg)?) } pub fn draw(&self, location: ScreenPoint, flip: LCDBitmapFlip) -> Result<(), Error> { - self.inner.borrow().draw(location, flip) + self.inner + .try_borrow() + .map_err(Error::msg)? + .draw(location, flip) } pub fn draw_scaled(&self, location: ScreenPoint, scale: Vector2D) -> Result<(), Error> { - self.inner.borrow().draw_scaled(location, scale) + self.inner + .try_borrow() + .map_err(Error::msg)? + .draw_scaled(location, scale) } /// Draw the `Bitmap` to the given `location`, rotated `degrees` about the `center` point, @@ -483,13 +489,18 @@ impl Bitmap { scale: Vector2D, ) -> Result<(), Error> { self.inner - .borrow() + .try_borrow() + .map_err(Error::msg)? .draw_rotated(location, degrees, center, scale) } /// Return a copy of self, rotated by `degrees` and scaled up or down in size by `scale`. pub fn rotated(&self, degrees: f32, scale: Vector2D) -> Result { - let raw_bitmap = self.inner.borrow().rotated(degrees, scale)?; + let raw_bitmap = self + .inner + .try_borrow() + .map_err(Error::msg)? + .rotated(degrees, scale)?; Ok(Self { inner: Rc::new(RefCell::new(raw_bitmap)), }) @@ -501,26 +512,39 @@ impl Bitmap { size: ScreenSize, flip: LCDBitmapFlip, ) -> Result<(), Error> { - self.inner.borrow().tile(location, size, flip) + self.inner + .try_borrow() + .map_err(Error::msg)? + .tile(location, size, flip) } pub fn clear(&mut self, color: LCDColor) -> Result<(), Error> { - self.inner.borrow_mut().clear(color) + self.inner + .try_borrow_mut() + .map_err(Error::msg)? + .clear(color) } pub fn transform(&self, rotation: f32, scale: Vector2D) -> Result { - let inner = self.inner.borrow().transform(rotation, scale)?; + let inner = self + .inner + .try_borrow() + .map_err(Error::msg)? + .transform(rotation, scale)?; Ok(Self { inner: Rc::new(RefCell::new(inner)), }) } pub fn into_color(&self, bitmap: Bitmap, top_left: Point2D) -> Result { - self.inner.borrow().into_color(bitmap, top_left) + self.inner + .try_borrow() + .map_err(Error::msg)? + .into_color(bitmap, top_left) } pub fn load(&mut self, path: &str) -> Result<(), Error> { - self.inner.borrow_mut().load(path) + self.inner.try_borrow_mut().map_err(Error::msg)?.load(path) } pub fn check_mask_collision( @@ -532,14 +556,17 @@ impl Bitmap { other_flip: LCDBitmapFlip, rect: ScreenRect, ) -> Result { - self.inner.borrow().check_mask_collision( - my_location, - my_flip, - other, - other_location, - other_flip, - rect, - ) + self.inner + .try_borrow() + .map_err(Error::msg)? + .check_mask_collision( + my_location, + my_flip, + other, + other_location, + other_flip, + rect, + ) } }