From 8b11aba68ac3e2cd4e3523aa2a2bb5d93dd49c0e Mon Sep 17 00:00:00 2001 From: cyrgani Date: Fri, 10 Jan 2025 00:26:13 +0100 Subject: [PATCH 1/2] add new methods to avoid unsound accesses to image fields --- src/texture.rs | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/src/texture.rs b/src/texture.rs index 39fc2b93..93bda709 100644 --- a/src/texture.rs +++ b/src/texture.rs @@ -64,13 +64,28 @@ impl TexturesContext { /// Image, data stored in CPU memory #[derive(Clone)] pub struct Image { + // FIXME(0.5): remove all the deprecation notes once the `Image` fields are private + #[deprecated( + since = "0.4.14", + note = "this will be made private, use `Image::bytes`, `Image::bytes_mut` or `Image::bytes_vec_mut` for reading and writing instead" + )] pub bytes: Vec, + #[deprecated( + since = "0.4.14", + note = "this will be made private, use `Image::width` or `Image::width_mut` for reading and writing instead" + )] pub width: u16, + #[deprecated( + since = "0.4.14", + note = "this will be made private, use `Image::height` or `Image::height_mut` for reading and writing instead" + )] pub height: u16, } impl std::fmt::Debug for Image { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + // FIXME(0.5): remove this once the `Image` fields are private + #[allow(deprecated)] f.debug_struct("Image") .field("width", &self.width) .field("height", &self.height) @@ -79,6 +94,8 @@ impl std::fmt::Debug for Image { } } +// FIXME(0.5): remove this once the `Image` fields are private +#[allow(deprecated)] impl Image { /// Creates an empty Image. /// @@ -94,6 +111,20 @@ impl Image { } } + /// Creates an image from the provided parts. + /// + /// # Panics + /// Panics if the width and height do not match the amount of bytes given. + pub fn from_parts(width: u16, height: u16, bytes: Vec) -> Image { + assert_eq!(width as usize * height as usize * 4, bytes.len()); + + Image { + width, + height, + bytes, + } + } + /// Creates an Image from a slice of bytes that contains an encoded image. /// /// If `format` is None, it will make an educated guess on the @@ -157,15 +188,54 @@ impl Image { } /// Returns the width of this image. + // FIXME(0.5): change the argument type to u16 pub const fn width(&self) -> usize { self.width as usize } /// Returns the height of this image. + // FIXME(0.5): change the argument type to u16 pub const fn height(&self) -> usize { self.height as usize } + /// Returns the bytes of this image as an immutable slice. + pub fn bytes(&self) -> &[u8] { + &self.bytes + } + + /// Returns the bytes of this image as a mutable slice. + pub fn bytes_mut(&mut self) -> &mut [u8] { + &mut self.bytes + } + + /// Allows changing the width of this image unsafely. + /// + /// # Safety + /// Increasing the width without properly filling the new pixels can cause Undefined Behaviour. + pub unsafe fn width_mut(&mut self) -> &mut u16 { + &mut self.width + } + + /// Allows changing the height of this image unsafely. + /// + /// # Safety + /// Increasing the height without properly filling the new pixels can cause Undefined Behaviour. + pub unsafe fn height_mut(&mut self) -> &mut u16 { + &mut self.height + } + + /// Allows changing the bytes of this image unsafely. + /// + /// If you do not intend to change the amount of the bytes, + /// use `Image::bytes_mut` instead, which is safe. + /// + /// # Safety + /// Removing bytes and not changing width and height accordingly can cause Undefined Behaviour. + pub unsafe fn bytes_vec_mut(&mut self) -> &mut Vec { + &mut self.bytes + } + /// Returns this image's data as a slice of 4-byte arrays. pub fn get_image_data(&self) -> &[[u8; 4]] { use std::slice; From 1019ade9e572e3f1c748736252f9954c845452e9 Mon Sep 17 00:00:00 2001 From: cyrgani Date: Fri, 10 Jan 2025 00:36:33 +0100 Subject: [PATCH 2/2] use the new methods throughout macroquad --- src/text.rs | 10 +++++----- src/text/atlas.rs | 31 ++++++++++++++++++------------- src/texture.rs | 19 ++++++++----------- src/ui/style.rs | 20 ++++++++++---------- 4 files changed, 41 insertions(+), 39 deletions(-) diff --git a/src/text.rs b/src/text.rs index 0107c468..4cc0087b 100644 --- a/src/text.rs +++ b/src/text.rs @@ -111,14 +111,14 @@ impl Font { let sprite = self.atlas.lock().unwrap().new_unique_id(); self.atlas.lock().unwrap().cache_sprite( sprite, - Image { - bytes: bitmap + Image::from_parts( + width, + height, + bitmap .iter() .flat_map(|coverage| vec![255, 255, 255, *coverage]) .collect(), - width, - height, - }, + ), ); let advance = metrics.advance_width; diff --git a/src/text/atlas.rs b/src/text/atlas.rs index 01fbc10c..152ade39 100644 --- a/src/text/atlas.rs +++ b/src/text/atlas.rs @@ -42,7 +42,8 @@ impl Atlas { pub fn new(ctx: &mut dyn miniquad::RenderingBackend, filter: miniquad::FilterMode) -> Atlas { let image = Image::gen_image_color(512, 512, Color::new(0.0, 0.0, 0.0, 0.0)); - let texture = ctx.new_texture_from_rgba8(image.width, image.height, &image.bytes); + let texture = + ctx.new_texture_from_rgba8(image.width() as u16, image.height() as u16, image.bytes()); ctx.texture_set_filter( texture, miniquad::FilterMode::Nearest, @@ -79,11 +80,11 @@ impl Atlas { } pub const fn width(&self) -> u16 { - self.image.width + self.image.width() as u16 } pub const fn height(&self) -> u16 { - self.image.height + self.image.height() as u16 } pub fn texture(&mut self) -> miniquad::TextureId { @@ -91,18 +92,20 @@ impl Atlas { if self.dirty { self.dirty = false; let (texture_width, texture_height) = ctx.texture_size(self.texture); - if texture_width != self.image.width as _ || texture_height != self.image.height as _ { + if texture_width != self.image.width() as _ + || texture_height != self.image.height() as _ + { ctx.delete_texture(self.texture); self.texture = ctx.new_texture_from_rgba8( - self.image.width, - self.image.height, - &self.image.bytes[..], + self.image.width() as u16, + self.image.height() as u16, + self.image.bytes(), ); ctx.texture_set_filter(self.texture, self.filter, miniquad::MipmapFilterMode::None); } - ctx.texture_update(self.texture, &self.image.bytes); + ctx.texture_update(self.texture, self.image.bytes()); } self.texture @@ -123,9 +126,9 @@ impl Atlas { } pub fn cache_sprite(&mut self, key: SpriteKey, sprite: Image) { - let (width, height) = (sprite.width as usize, sprite.height as usize); + let (width, height) = (sprite.width(), sprite.height()); - let x = if self.cursor_x + (width as u16) < self.image.width { + let x = if self.cursor_x + (width as u16) < self.image.width() as u16 { if height as u16 > self.max_line_height { self.max_line_height = height as u16; } @@ -141,7 +144,9 @@ impl Atlas { let y = self.cursor_y; // texture bounds exceeded - if y + sprite.height > self.image.height || x + sprite.width > self.image.width { + if y + sprite.height() as u16 > self.image.height() as u16 + || x + sprite.width() as u16 > self.image.width() as u16 + { // reset glyph cache state let sprites = self.sprites.drain().collect::>(); self.cursor_x = 0; @@ -154,8 +159,8 @@ impl Atlas { // note: if we tried to fit gigantic texture into a small atlas, // new_width will still be not enough. But its fine, it will // be regenerated on the recursion call. - let new_width = self.image.width * 2; - let new_height = self.image.height * 2; + let new_width = self.image.width() as u16 * 2; + let new_height = self.image.height() as u16 * 2; self.image = Image::gen_image_color(new_width, new_height, Color::new(0.0, 0.0, 0.0, 0.0)); diff --git a/src/texture.rs b/src/texture.rs index 93bda709..bd228dce 100644 --- a/src/texture.rs +++ b/src/texture.rs @@ -5,6 +5,7 @@ use crate::{ text::atlas::SpriteKey, Error, }; +use crate::color::BLANK; pub use crate::quad_gl::FilterMode; use crate::quad_gl::{DrawMode, Vertex}; use glam::{vec2, Vec2}; @@ -774,7 +775,7 @@ impl Texture2D { /// Creates a Texture2D from an [Image]. pub fn from_image(image: &Image) -> Texture2D { - Texture2D::from_rgba8(image.width, image.height, &image.bytes) + Texture2D::from_rgba8(image.width() as u16, image.height() as u16, image.bytes()) } /// Creates a Texture2D from a miniquad @@ -816,10 +817,10 @@ impl Texture2D { let ctx = get_quad_context(); let (width, height) = ctx.texture_size(self.raw_miniquad_id()); - assert_eq!(width, image.width as u32); - assert_eq!(height, image.height as u32); + assert_eq!(width, image.width() as u32); + assert_eq!(height, image.height() as u32); - ctx.texture_update(self.raw_miniquad_id(), &image.bytes); + ctx.texture_update(self.raw_miniquad_id(), image.bytes()); } // Updates the texture from an array of bytes. @@ -850,7 +851,7 @@ impl Texture2D { y_offset, width, height, - &image.bytes, + image.bytes(), ); } @@ -950,12 +951,8 @@ impl Texture2D { pub fn get_texture_data(&self) -> Image { let ctx = get_quad_context(); let (width, height) = ctx.texture_size(self.raw_miniquad_id()); - let mut image = Image { - width: width as _, - height: height as _, - bytes: vec![0; width as usize * height as usize * 4], - }; - ctx.texture_read_pixels(self.raw_miniquad_id(), &mut image.bytes); + let mut image = Image::gen_image_color(width as u16, height as u16, BLANK); + ctx.texture_read_pixels(self.raw_miniquad_id(), image.bytes_mut()); image } } diff --git a/src/ui/style.rs b/src/ui/style.rs index e345f32b..91275429 100644 --- a/src/ui/style.rs +++ b/src/ui/style.rs @@ -409,11 +409,11 @@ impl Skin { .color_inactive(Color::from_rgba(238, 238, 238, 128)) .text_color(Color::from_rgba(0, 0, 0, 255)) .color(Color::from_rgba(220, 220, 220, 255)) - .background(Image { - width: 16, - height: 30, - bytes: include_bytes!("combobox.img").to_vec(), - }) + .background(Image::from_parts( + 16, + 30, + include_bytes!("combobox.img").to_vec(), + )) .build(), tabbar_style: Style { margin: Some(RectOffset::new(2., 2., 2., 2.)), @@ -429,15 +429,15 @@ impl Skin { .background_margin(RectOffset::new(1., 1., 1., 1.)) .color_inactive(Color::from_rgba(238, 238, 238, 128)) .text_color(Color::from_rgba(0, 0, 0, 255)) - .background(Image { - width: 3, - height: 3, - bytes: vec![ + .background(Image::from_parts( + 3, + 3, + vec![ 68, 68, 68, 255, 68, 68, 68, 255, 68, 68, 68, 255, 68, 68, 68, 255, 238, 238, 238, 255, 68, 68, 68, 255, 68, 68, 68, 255, 68, 68, 68, 255, 68, 68, 68, 255, ], - }) + )) .build(), window_titlebar_style: Style { color: Color::from_rgba(68, 68, 68, 255),