From 8c6e8de7c4c9f318ed6600ecacd6d820594d5d7d Mon Sep 17 00:00:00 2001 From: thomas725 <68635351+thomas725@users.noreply.github.com> Date: Thu, 31 Aug 2023 16:58:46 +0200 Subject: [PATCH 01/11] add get_pixel & toggle_pixel & extract shared code --- src/mode/buffered_graphics.rs | 46 +++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/src/mode/buffered_graphics.rs b/src/mode/buffered_graphics.rs index 6a9cfb66..c1dd29e9 100644 --- a/src/mode/buffered_graphics.rs +++ b/src/mode/buffered_graphics.rs @@ -156,13 +156,8 @@ where } } - /// Turn a pixel on or off. A non-zero `value` is treated as on, `0` as off. If the X and Y - /// coordinates are out of the bounds of the display, this method call is a noop. - pub fn set_pixel(&mut self, x: u32, y: u32, value: bool) { - let value = value as u8; - let rotation = self.rotation; - - let (idx, bit) = match rotation { + fn pixel_location(&self, x: u32, y: u32) -> (usize, u32) { + match self.rotation { DisplayRotation::Rotate0 | DisplayRotation::Rotate180 => { let idx = ((y as usize) / 8 * SIZE::WIDTH as usize) + (x as usize); let bit = y % 8; @@ -175,19 +170,38 @@ where (idx, bit) } - }; + } + } - if let Some(byte) = self.mode.buffer.as_mut().get_mut(idx) { - // Keep track of max and min values - self.mode.min_x = self.mode.min_x.min(x as u8); - self.mode.max_x = self.mode.max_x.max(x as u8); + fn track_change(&mut self, x: u32, y: u32) { + self.mode.min_x = self.mode.min_x.min(x as u8); + self.mode.max_x = self.mode.max_x.max(x as u8); + self.mode.min_y = self.mode.min_y.min(y as u8); + self.mode.max_y = self.mode.max_y.max(y as u8); + } - self.mode.min_y = self.mode.min_y.min(y as u8); - self.mode.max_y = self.mode.max_y.max(y as u8); + pub fn get_pixel(&mut self, x: u32, y: u32) -> Option { + let (idx, bit) = self.pixel_location(x,y); + &self.mode.buffer.as_mut().get(idx).map(|byte| byte & !(1 << bit) != 0) + } - // Set pixel value in byte + /// Turn a pixel on or off. If the Y coordinate is out of bounds for the display, this method call is a noop. + /// If X is out of bounds, it will overflow into a different Y coordinate. + pub fn set_pixel(&mut self, x: u32, y: u32, value: bool) { + let value = value as u8; + let (idx, bit) = self.pixel_location(x,y); + if let Some(byte) = self.mode.buffer.as_mut().get_mut(idx) { + self.track_change(x,y); + *byte = *byte & !(1 << bit) | (value << bit) // Set pixel value in byte // Ref this comment https://stackoverflow.com/questions/47981/how-do-you-set-clear-and-toggle-a-single-bit#comment46654671_47990 - *byte = *byte & !(1 << bit) | (value << bit) + } + } + + pub fn toggle_pixel(&mut self, x: u32, y: u32) { + let (idx, bit) = self.pixel_location(x,y); + if let Some(byte) = self.mode.buffer.as_mut().get_mut(idx) { + self.track_change(x,y); + *byte = *byte ^ !(1 << bit) // toggle pixel in byte } } } From 15840aaa16e1d3003edb8b8d7e4715f3ea172775 Mon Sep 17 00:00:00 2001 From: thomas725 <68635351+thomas725@users.noreply.github.com> Date: Thu, 31 Aug 2023 17:02:27 +0200 Subject: [PATCH 02/11] bugfix --- src/mode/buffered_graphics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mode/buffered_graphics.rs b/src/mode/buffered_graphics.rs index c1dd29e9..3e138dde 100644 --- a/src/mode/buffered_graphics.rs +++ b/src/mode/buffered_graphics.rs @@ -182,7 +182,7 @@ where pub fn get_pixel(&mut self, x: u32, y: u32) -> Option { let (idx, bit) = self.pixel_location(x,y); - &self.mode.buffer.as_mut().get(idx).map(|byte| byte & !(1 << bit) != 0) + self.mode.buffer.as_mut().get(idx).map(|byte| byte & !(1 << bit) != 0) } /// Turn a pixel on or off. If the Y coordinate is out of bounds for the display, this method call is a noop. From a6cfe678bc95a55aa8a58367ea5338cd147aac9b Mon Sep 17 00:00:00 2001 From: thomas725 <68635351+thomas725@users.noreply.github.com> Date: Thu, 31 Aug 2023 17:09:25 +0200 Subject: [PATCH 03/11] bugfix, not optimal had to move track_changes call out of if block, therefore it will also run for invalid Y values (invalid X values where never caught anyway, they overflow into later Y values) --- src/mode/buffered_graphics.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mode/buffered_graphics.rs b/src/mode/buffered_graphics.rs index 3e138dde..70ea65bf 100644 --- a/src/mode/buffered_graphics.rs +++ b/src/mode/buffered_graphics.rs @@ -190,8 +190,8 @@ where pub fn set_pixel(&mut self, x: u32, y: u32, value: bool) { let value = value as u8; let (idx, bit) = self.pixel_location(x,y); + self.track_change(x,y); if let Some(byte) = self.mode.buffer.as_mut().get_mut(idx) { - self.track_change(x,y); *byte = *byte & !(1 << bit) | (value << bit) // Set pixel value in byte // Ref this comment https://stackoverflow.com/questions/47981/how-do-you-set-clear-and-toggle-a-single-bit#comment46654671_47990 } @@ -199,8 +199,8 @@ where pub fn toggle_pixel(&mut self, x: u32, y: u32) { let (idx, bit) = self.pixel_location(x,y); + self.track_change(x,y); if let Some(byte) = self.mode.buffer.as_mut().get_mut(idx) { - self.track_change(x,y); *byte = *byte ^ !(1 << bit) // toggle pixel in byte } } From ebada2b3d0039b2d8405e009031f5b8b01d6878d Mon Sep 17 00:00:00 2001 From: thomas725 <68635351+thomas725@users.noreply.github.com> Date: Thu, 31 Aug 2023 22:53:33 +0200 Subject: [PATCH 04/11] bugfix: toggle the chosen bit, not the other 7 of it's byte --- src/mode/buffered_graphics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mode/buffered_graphics.rs b/src/mode/buffered_graphics.rs index 70ea65bf..e44ae26c 100644 --- a/src/mode/buffered_graphics.rs +++ b/src/mode/buffered_graphics.rs @@ -201,7 +201,7 @@ where let (idx, bit) = self.pixel_location(x,y); self.track_change(x,y); if let Some(byte) = self.mode.buffer.as_mut().get_mut(idx) { - *byte = *byte ^ !(1 << bit) // toggle pixel in byte + *byte = *byte ^ (1 << bit) // toggle pixel in byte } } } From 65e13fc7fe6702e790e3733879c3c6c9f41add44 Mon Sep 17 00:00:00 2001 From: thomas725 <68635351+thomas725@users.noreply.github.com> Date: Fri, 1 Sep 2023 08:15:46 +0200 Subject: [PATCH 05/11] bugfix get_pixel apply same fix we did for toggle_pixel to get_pixel, i.e. check the one bit and not all others instead: `(1 << bit)` gives a 1 for the bit we want to read, and 0 for all others. by applying a bit-wise AND operator between the current byte and this mask, we will only get something unequal to zero, if the bit we want to read is not zero: `byte & (1 << bit)` --- src/mode/buffered_graphics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mode/buffered_graphics.rs b/src/mode/buffered_graphics.rs index e44ae26c..983ee81a 100644 --- a/src/mode/buffered_graphics.rs +++ b/src/mode/buffered_graphics.rs @@ -182,7 +182,7 @@ where pub fn get_pixel(&mut self, x: u32, y: u32) -> Option { let (idx, bit) = self.pixel_location(x,y); - self.mode.buffer.as_mut().get(idx).map(|byte| byte & !(1 << bit) != 0) + self.mode.buffer.as_mut().get(idx).map(|byte| byte & (1 << bit) != 0) } /// Turn a pixel on or off. If the Y coordinate is out of bounds for the display, this method call is a noop. From 4a324a31437e4312862baf50c4d982d6f89cc3ec Mon Sep 17 00:00:00 2001 From: thomas725 Date: Sat, 2 Sep 2023 08:36:26 +0200 Subject: [PATCH 06/11] don't require a mutable reference for reading pixel state --- src/mode/buffered_graphics.rs | 4 ++-- src/size.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/mode/buffered_graphics.rs b/src/mode/buffered_graphics.rs index 983ee81a..455bbe89 100644 --- a/src/mode/buffered_graphics.rs +++ b/src/mode/buffered_graphics.rs @@ -180,9 +180,9 @@ where self.mode.max_y = self.mode.max_y.max(y as u8); } - pub fn get_pixel(&mut self, x: u32, y: u32) -> Option { + pub fn get_pixel(&self, x: u32, y: u32) -> Option { let (idx, bit) = self.pixel_location(x,y); - self.mode.buffer.as_mut().get(idx).map(|byte| byte & (1 << bit) != 0) + self.mode.buffer.as_ref().get(idx).map(|byte| byte & (1 << bit) != 0) } /// Turn a pixel on or off. If the Y coordinate is out of bounds for the display, this method call is a noop. diff --git a/src/size.rs b/src/size.rs index b177300e..a503042a 100644 --- a/src/size.rs +++ b/src/size.rs @@ -40,7 +40,7 @@ pub trait DisplaySize { /// Size of framebuffer. Because the display is monochrome, this is /// width * height / 8 - type Buffer: AsMut<[u8]> + NewZeroed; + type Buffer: AsRef<[u8]> + AsMut<[u8]> + NewZeroed; /// Send resolution and model-dependent configuration to the display /// From 423e1180e81a859ac636caf57f3f4e7a46ae55c2 Mon Sep 17 00:00:00 2001 From: thomas725 Date: Sat, 2 Sep 2023 12:05:57 +0200 Subject: [PATCH 07/11] prevent pixel overflow to next line --- src/mode/buffered_graphics.rs | 63 ++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 30 deletions(-) diff --git a/src/mode/buffered_graphics.rs b/src/mode/buffered_graphics.rs index 455bbe89..591a0b88 100644 --- a/src/mode/buffered_graphics.rs +++ b/src/mode/buffered_graphics.rs @@ -156,20 +156,17 @@ where } } - fn pixel_location(&self, x: u32, y: u32) -> (usize, u32) { - match self.rotation { - DisplayRotation::Rotate0 | DisplayRotation::Rotate180 => { - let idx = ((y as usize) / 8 * SIZE::WIDTH as usize) + (x as usize); - let bit = y % 8; - - (idx, bit) - } - DisplayRotation::Rotate90 | DisplayRotation::Rotate270 => { - let idx = ((x as usize) / 8 * SIZE::WIDTH as usize) + (y as usize); - let bit = x % 8; - - (idx, bit) - } + fn pixel_location(&self, x: u32, y: u32) -> Option<(usize, u32)> { + let (x_rotated, y_rotated) = match self.rotation { + DisplayRotation::Rotate0 | DisplayRotation::Rotate180 => (x, y), + DisplayRotation::Rotate90 | DisplayRotation::Rotate270 => (y, x), + }; + if x_rotated >= SIZE::WIDTH || y_rotated >= SIZE::HEIGHT { + None + } else { + let idx = ((y_rotated as usize) / 8 * SIZE::WIDTH as usize) + (x_rotated as usize); + let bit = y_rotated % 8; + Some((idx, bit)) } } @@ -181,28 +178,34 @@ where } pub fn get_pixel(&self, x: u32, y: u32) -> Option { - let (idx, bit) = self.pixel_location(x,y); - self.mode.buffer.as_ref().get(idx).map(|byte| byte & (1 << bit) != 0) + self.pixel_location(x, y).map(|idx, bit| { + self.mode + .buffer + .as_ref() + .get(idx) + .map(|byte| byte & (1 << bit) != 0) + }) } - /// Turn a pixel on or off. If the Y coordinate is out of bounds for the display, this method call is a noop. - /// If X is out of bounds, it will overflow into a different Y coordinate. + /// Turn a pixel on or off. If the coordinates are out of bounds for the display, this method call is a noop pub fn set_pixel(&mut self, x: u32, y: u32, value: bool) { - let value = value as u8; - let (idx, bit) = self.pixel_location(x,y); - self.track_change(x,y); - if let Some(byte) = self.mode.buffer.as_mut().get_mut(idx) { - *byte = *byte & !(1 << bit) | (value << bit) // Set pixel value in byte - // Ref this comment https://stackoverflow.com/questions/47981/how-do-you-set-clear-and-toggle-a-single-bit#comment46654671_47990 - } + self.pixel_location(x, y).map(|idx, bit| { + self.track_change(x, y); + if let Some(byte) = self.mode.buffer.as_mut().get_mut(idx) { + // Ref this comment https://stackoverflow.com/questions/47981/how-do-you-set-clear-and-toggle-a-single-bit#comment46654671_47990 + *byte = *byte & !(1 << bit) | ((value as u8) << bit) // Set pixel value in byte + } + }) } + /// Toggle a pixel to the opposite of it's current state. If the coordinates are out of bounds for the display, this method call is a noop pub fn toggle_pixel(&mut self, x: u32, y: u32) { - let (idx, bit) = self.pixel_location(x,y); - self.track_change(x,y); - if let Some(byte) = self.mode.buffer.as_mut().get_mut(idx) { - *byte = *byte ^ (1 << bit) // toggle pixel in byte - } + self.pixel_location(x, y).map(|idx, bit| { + self.track_change(x, y); + if let Some(byte) = self.mode.buffer.as_mut().get_mut(idx) { + *byte = *byte ^ (1 << bit) // toggle pixel in byte + } + }) } } From e91bdfd66af4b83ebe89dae2941d2fd9e953b3f7 Mon Sep 17 00:00:00 2001 From: thomas725 Date: Sat, 2 Sep 2023 12:08:20 +0200 Subject: [PATCH 08/11] bugfix: tuples need brackets --- src/mode/buffered_graphics.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/mode/buffered_graphics.rs b/src/mode/buffered_graphics.rs index 591a0b88..411e24f0 100644 --- a/src/mode/buffered_graphics.rs +++ b/src/mode/buffered_graphics.rs @@ -178,7 +178,7 @@ where } pub fn get_pixel(&self, x: u32, y: u32) -> Option { - self.pixel_location(x, y).map(|idx, bit| { + self.pixel_location(x, y).map(|(idx, bit)| { self.mode .buffer .as_ref() @@ -189,7 +189,7 @@ where /// Turn a pixel on or off. If the coordinates are out of bounds for the display, this method call is a noop pub fn set_pixel(&mut self, x: u32, y: u32, value: bool) { - self.pixel_location(x, y).map(|idx, bit| { + self.pixel_location(x, y).map(|(idx, bit)| { self.track_change(x, y); if let Some(byte) = self.mode.buffer.as_mut().get_mut(idx) { // Ref this comment https://stackoverflow.com/questions/47981/how-do-you-set-clear-and-toggle-a-single-bit#comment46654671_47990 @@ -200,7 +200,7 @@ where /// Toggle a pixel to the opposite of it's current state. If the coordinates are out of bounds for the display, this method call is a noop pub fn toggle_pixel(&mut self, x: u32, y: u32) { - self.pixel_location(x, y).map(|idx, bit| { + self.pixel_location(x, y).map(|(idx, bit)| { self.track_change(x, y); if let Some(byte) = self.mode.buffer.as_mut().get_mut(idx) { *byte = *byte ^ (1 << bit) // toggle pixel in byte From 7808b4f8992aa77087e48a1903c0caeeae498e1b Mon Sep 17 00:00:00 2001 From: thomas725 Date: Sat, 2 Sep 2023 12:09:55 +0200 Subject: [PATCH 09/11] bugfix: statements that don't return need semicolon --- src/mode/buffered_graphics.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mode/buffered_graphics.rs b/src/mode/buffered_graphics.rs index 411e24f0..adcee715 100644 --- a/src/mode/buffered_graphics.rs +++ b/src/mode/buffered_graphics.rs @@ -195,7 +195,7 @@ where // Ref this comment https://stackoverflow.com/questions/47981/how-do-you-set-clear-and-toggle-a-single-bit#comment46654671_47990 *byte = *byte & !(1 << bit) | ((value as u8) << bit) // Set pixel value in byte } - }) + }); } /// Toggle a pixel to the opposite of it's current state. If the coordinates are out of bounds for the display, this method call is a noop @@ -205,7 +205,7 @@ where if let Some(byte) = self.mode.buffer.as_mut().get_mut(idx) { *byte = *byte ^ (1 << bit) // toggle pixel in byte } - }) + }); } } From 548d5d3644e10072ebc62f8643eb7412a508594b Mon Sep 17 00:00:00 2001 From: thomas725 Date: Sat, 2 Sep 2023 12:14:28 +0200 Subject: [PATCH 10/11] bugfix: use and_then instead of map to get Option instead of Option> --- src/mode/buffered_graphics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mode/buffered_graphics.rs b/src/mode/buffered_graphics.rs index adcee715..d6504e48 100644 --- a/src/mode/buffered_graphics.rs +++ b/src/mode/buffered_graphics.rs @@ -178,7 +178,7 @@ where } pub fn get_pixel(&self, x: u32, y: u32) -> Option { - self.pixel_location(x, y).map(|(idx, bit)| { + self.pixel_location(x, y).and_then(|(idx, bit)| { self.mode .buffer .as_ref() From bd3ecc7eb6bf9b6ee8e8ee9144c1f3476fc63fd8 Mon Sep 17 00:00:00 2001 From: thomas725 Date: Sat, 2 Sep 2023 12:16:57 +0200 Subject: [PATCH 11/11] bugfix: can't compare u32 to u8 without casting --- src/mode/buffered_graphics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mode/buffered_graphics.rs b/src/mode/buffered_graphics.rs index d6504e48..f7d70bce 100644 --- a/src/mode/buffered_graphics.rs +++ b/src/mode/buffered_graphics.rs @@ -161,7 +161,7 @@ where DisplayRotation::Rotate0 | DisplayRotation::Rotate180 => (x, y), DisplayRotation::Rotate90 | DisplayRotation::Rotate270 => (y, x), }; - if x_rotated >= SIZE::WIDTH || y_rotated >= SIZE::HEIGHT { + if x_rotated >= SIZE::WIDTH as u32 || y_rotated >= SIZE::HEIGHT as u32 { None } else { let idx = ((y_rotated as usize) / 8 * SIZE::WIDTH as usize) + (x_rotated as usize);