extract methods, CommandExecutor

src/execute_command.rs

@@ -3,35 +3,47 @@ use servicepoint::{
     Bitmap, BrightnessGrid, CharGrid, Command, Cp437Grid, Grid, Origin, Tiles,
     PIXEL_COUNT, PIXEL_WIDTH, TILE_SIZE,
 };
-use std::sync::{RwLock, RwLockWriteGuard};
+use std::sync::RwLock;
 
 use crate::font::Cp437Font;
 use crate::font_renderer::FontRenderer8x8;
 
-pub(crate) fn execute_command(
+pub struct CommandExecutor<'t> {
-    command: Command,
+    display: &'t RwLock<Bitmap>,
-    cp436_font: &Cp437Font,
+    luma: &'t RwLock<BrightnessGrid>,
-    utf8_font: &FontRenderer8x8,
+    cp437_font: Cp437Font,
-    display_ref: &RwLock<Bitmap>,
+    utf8_font: FontRenderer8x8,
-    luma_ref: &RwLock<BrightnessGrid>,
+}
-) -> bool {
+
+impl<'t> CommandExecutor<'t> {
+    pub fn new(
+        display: &'t RwLock<Bitmap>,
+        luma: &'t RwLock<BrightnessGrid>,
+    ) -> Self {
+        CommandExecutor {
+            display,
+            luma,
+            utf8_font: FontRenderer8x8::default(),
+            cp437_font: Cp437Font::default(),
+        }
+    }
+
+    pub(crate) fn execute(&self, command: Command) -> bool {
         debug!("received {command:?}");
         match command {
             Command::Clear => {
                 info!("clearing display");
-            display_ref.write().unwrap().fill(false);
+                self.display.write().unwrap().fill(false);
             }
             Command::HardReset => {
                 warn!("display shutting down");
                 return false;
             }
             Command::BitmapLinearWin(Origin { x, y, .. }, pixels, _) => {
-            let mut display = display_ref.write().unwrap();
+                self.print_pixel_grid(x, y, &pixels);
-            print_pixel_grid(x, y, &pixels, &mut display);
             }
             Command::Cp437Data(origin, grid) => {
-            let mut display = display_ref.write().unwrap();
+                self.print_cp437_data(origin, &grid);
-            print_cp437_data(origin, &grid, cp436_font, &mut display);
             }
             #[allow(deprecated)]
             Command::BitmapLegacy => {
@@ -39,50 +51,54 @@ pub(crate) fn execute_command(
             }
             // TODO: how to deduplicate this code in a rusty way?
             Command::BitmapLinear(offset, vec, _) => {
-            if !check_bitmap_valid(offset as u16, vec.len()) {
+                if !Self::check_bitmap_valid(offset as u16, vec.len()) {
                     return true;
                 }
-            let mut display = display_ref.write().unwrap();
+                let mut display = self.display.write().unwrap();
                 for bitmap_index in 0..vec.len() {
-                let (x, y) = get_coordinates_for_index(offset, bitmap_index);
+                    let (x, y) =
+                        Self::get_coordinates_for_index(offset, bitmap_index);
                     display.set(x, y, vec[bitmap_index]);
                 }
             }
             Command::BitmapLinearAnd(offset, vec, _) => {
-            if !check_bitmap_valid(offset as u16, vec.len()) {
+                if !Self::check_bitmap_valid(offset as u16, vec.len()) {
                     return true;
                 }
-            let mut display = display_ref.write().unwrap();
+                let mut display = self.display.write().unwrap();
                 for bitmap_index in 0..vec.len() {
-                let (x, y) = get_coordinates_for_index(offset, bitmap_index);
+                    let (x, y) =
+                        Self::get_coordinates_for_index(offset, bitmap_index);
                     let old_value = display.get(x, y);
                     display.set(x, y, old_value && vec[bitmap_index]);
                 }
             }
             Command::BitmapLinearOr(offset, vec, _) => {
-            if !check_bitmap_valid(offset as u16, vec.len()) {
+                if !Self::check_bitmap_valid(offset as u16, vec.len()) {
                     return true;
                 }
-            let mut display = display_ref.write().unwrap();
+                let mut display = self.display.write().unwrap();
                 for bitmap_index in 0..vec.len() {
-                let (x, y) = get_coordinates_for_index(offset, bitmap_index);
+                    let (x, y) =
+                        Self::get_coordinates_for_index(offset, bitmap_index);
                     let old_value = display.get(x, y);
                     display.set(x, y, old_value || vec[bitmap_index]);
                 }
             }
             Command::BitmapLinearXor(offset, vec, _) => {
-            if !check_bitmap_valid(offset as u16, vec.len()) {
+                if !Self::check_bitmap_valid(offset as u16, vec.len()) {
                     return true;
                 }
-            let mut display = display_ref.write().unwrap();
+                let mut display = self.display.write().unwrap();
                 for bitmap_index in 0..vec.len() {
-                let (x, y) = get_coordinates_for_index(offset, bitmap_index);
+                    let (x, y) =
+                        Self::get_coordinates_for_index(offset, bitmap_index);
                     let old_value = display.get(x, y);
                     display.set(x, y, old_value ^ vec[bitmap_index]);
                 }
             }
             Command::CharBrightness(origin, grid) => {
-            let mut luma = luma_ref.write().unwrap();
+                let mut luma = self.luma.write().unwrap();
                 for inner_y in 0..grid.height() {
                     for inner_x in 0..grid.width() {
                         let brightness = grid.get(inner_x, inner_y);
@@ -95,14 +111,13 @@ pub(crate) fn execute_command(
                 }
             }
             Command::Brightness(brightness) => {
-            luma_ref.write().unwrap().fill(brightness);
+                self.luma.write().unwrap().fill(brightness);
             }
             Command::FadeOut => {
                 error!("command not implemented: {command:?}")
             }
             Command::Utf8Data(origin, grid) => {
-            let mut display = display_ref.write().unwrap();
+                self.print_utf8_data(origin, &grid);
-            print_utf8_data(origin, &grid, utf8_font, &mut display);
             }
         };
 
@@ -111,19 +126,17 @@ pub(crate) fn execute_command(
 
     fn check_bitmap_valid(offset: u16, payload_len: usize) -> bool {
         if offset as usize + payload_len > PIXEL_COUNT {
-        error!("bitmap with offset {offset} is too big ({payload_len} bytes)");
+            error!(
+                "bitmap with offset {offset} is too big ({payload_len} bytes)"
+            );
             return false;
         }
 
         true
     }
 
-fn print_cp437_data(
+    fn print_cp437_data(&self, origin: Origin<Tiles>, grid: &Cp437Grid) {
-    origin: Origin<Tiles>,
+        let font = &self.cp437_font;
-    grid: &Cp437Grid,
-    font: &Cp437Font,
-    display: &mut RwLockWriteGuard<Bitmap>,
-) {
         let Origin { x, y, .. } = origin;
         for char_y in 0usize..grid.height() {
             for char_x in 0usize..grid.width() {
@@ -137,11 +150,10 @@ fn print_cp437_data(
                 let tile_y = char_y + y;
 
                 let bitmap = font.get_bitmap(char_code);
-            if !print_pixel_grid(
+                if !self.print_pixel_grid(
                     tile_x * TILE_SIZE,
                     tile_y * TILE_SIZE,
                     bitmap,
-                display,
                 ) {
                     error!("stopping drawing text because char draw failed");
                     return;
@@ -150,12 +162,9 @@ fn print_cp437_data(
         }
     }
 
-fn print_utf8_data(
+    fn print_utf8_data(&self, origin: Origin<Tiles>, grid: &CharGrid) {
-    origin: Origin<Tiles>,
+        let mut display = self.display.write().unwrap();
-    grid: &CharGrid,
+
-    font: &FontRenderer8x8,
-    display: &mut RwLockWriteGuard<Bitmap>,
-) {
         let Origin { x, y, .. } = origin;
         for char_y in 0usize..grid.height() {
             for char_x in 0usize..grid.width() {
@@ -165,12 +174,14 @@ fn print_utf8_data(
                 let tile_x = char_x + x;
                 let tile_y = char_y + y;
 
-            if let Err(e) = font.render(
+                if let Err(e) = self.utf8_font.render(
                     char,
-                display,
+                    &mut display,
                     Origin::new(tile_x * TILE_SIZE, tile_y * TILE_SIZE),
                 ) {
-                error!("stopping drawing text because char draw failed: {e}");
+                    error!(
+                        "stopping drawing text because char draw failed: {e}"
+                    );
                     return;
                 }
             }
@@ -178,16 +189,17 @@ fn print_utf8_data(
     }
 
     fn print_pixel_grid(
+        &self,
         offset_x: usize,
         offset_y: usize,
         pixels: &Bitmap,
-    display: &mut RwLockWriteGuard<Bitmap>,
     ) -> bool {
         debug!(
             "printing {}x{} grid at {offset_x} {offset_y}",
             pixels.width(),
             pixels.height()
         );
+        let mut display = self.display.write().unwrap();
         for inner_y in 0..pixels.height() {
             for inner_x in 0..pixels.width() {
                 let is_set = pixels.get(inner_x, inner_y);
@@ -206,7 +218,11 @@ fn print_pixel_grid(
         true
     }
 
-fn get_coordinates_for_index(offset: usize, index: usize) -> (usize, usize) {
+    fn get_coordinates_for_index(
+        offset: usize,
+        index: usize,
+    ) -> (usize, usize) {
         let pixel_index = offset + index;
         (pixel_index % PIXEL_WIDTH, pixel_index / PIXEL_WIDTH)
     }
+}

src/font_renderer.rs

@@ -1,18 +1,33 @@
 use crate::font_renderer::RenderError::{GlyphNotFound, OutOfBounds};
-use font_kit::canvas::{Canvas, Format, RasterizationOptions};
+use font_kit::{
-use font_kit::error::GlyphLoadingError;
+    canvas::{Canvas, Format, RasterizationOptions},
-use font_kit::family_name::FamilyName;
+    error::GlyphLoadingError,
-use font_kit::font::Font;
+    family_name::FamilyName,
-use font_kit::hinting::HintingOptions;
+    font::Font,
-use font_kit::properties::Properties;
+    hinting::HintingOptions,
-use font_kit::source::SystemSource;
+    properties::Properties,
-use pathfinder_geometry::transform2d::Transform2F;
+    source::SystemSource,
-use pathfinder_geometry::vector::{vec2f, vec2i};
+};
+use pathfinder_geometry::{
+    transform2d::Transform2F,
+    vector::{vec2f, vec2i},
+};
 use servicepoint::{Bitmap, Grid, Origin, Pixels, TILE_SIZE};
 use std::sync::Mutex;
 
+struct SendFont(Font);
+
+// struct is only using primitives and pointers - lets try if it is only missing the declaration
+unsafe impl Send for SendFont {}
+
+impl AsRef<Font> for SendFont {
+    fn as_ref(&self) -> &Font {
+        &self.0
+    }
+}
+
 pub struct FontRenderer8x8 {
-    font: Font,
+    font: SendFont,
     canvas: Mutex<Canvas>,
     fallback_char: Option<u32>,
 }
@@ -35,7 +50,7 @@ impl FontRenderer8x8 {
         assert_eq!(canvas.stride, TILE_SIZE);
         let fallback_char = fallback_char.and_then(|c| font.glyph_for_char(c));
         let result = Self {
-            font,
+            font: SendFont(font),
             fallback_char,
             canvas: Mutex::new(canvas),
         };
@@ -49,14 +64,18 @@ impl FontRenderer8x8 {
         offset: Origin<Pixels>,
     ) -> Result<(), RenderError> {
         let mut canvas = self.canvas.lock().unwrap();
-        let glyph_id = self.font.glyph_for_char(char).or(self.fallback_char);
+        let glyph_id = self
+            .font
+            .as_ref()
+            .glyph_for_char(char)
+            .or(self.fallback_char);
         let glyph_id = match glyph_id {
             None => return Err(GlyphNotFound(char)),
             Some(val) => val,
         };
 
         canvas.pixels.fill(0);
-        self.font.rasterize_glyph(
+        self.font.as_ref().rasterize_glyph(
             &mut canvas,
             glyph_id,
             TILE_SIZE as f32,
@@ -85,7 +104,14 @@ impl FontRenderer8x8 {
 impl Default for FontRenderer8x8 {
     fn default() -> Self {
         let utf8_font = SystemSource::new()
-            .select_best_match(&[FamilyName::Monospace], &Properties::new())
+            .select_best_match(
+                &[
+                    FamilyName::Title("Roboto Mono".to_string()),
+                    FamilyName::Title("Fira Mono".to_string()),
+                    FamilyName::Monospace,
+                ],
+                &Properties::new(),
+            )
             .unwrap()
             .load()
             .unwrap();

src/gui.rs

@@ -1,3 +1,4 @@
+use std::slice::ChunksExactMut;
 use std::sync::mpsc::Sender;
 use std::sync::RwLock;
 
@@ -62,24 +63,25 @@ impl<'t> App<'t> {
 
     fn draw(&mut self) {
         let window = self.window.as_ref().unwrap();
-        let mut pixels = {
         let window_size = window.inner_size();
-            let surface_texture = SurfaceTexture::new(
+        let surface_texture =
-                window_size.width,
+            SurfaceTexture::new(window_size.width, window_size.height, &window);
-                window_size.height,
+        let mut pixels = Pixels::new(
-                &window,
-            );
-            Pixels::new(
             self.logical_size.width as u32,
             self.logical_size.height as u32,
             surface_texture,
         )
-            .unwrap()
+        .unwrap();
-        };
 
         let mut frame = pixels.frame_mut().chunks_exact_mut(4);
+        self.draw_frame(&mut frame);
+        pixels.render().expect("could not render");
+    }
+
+    fn draw_frame(&self, frame: &mut ChunksExactMut<u8>) {
         let display = self.display.read().unwrap();
         let luma = self.luma.read().unwrap();
+
         for y in 0..PIXEL_HEIGHT {
             if self.cli.spacers && y != 0 && y % TILE_SIZE == 0 {
                 // cannot just frame.skip(PIXEL_WIDTH as usize * SPACER_HEIGHT as usize) because of typing
@@ -90,14 +92,20 @@ impl<'t> App<'t> {
 
             for x in 0..PIXEL_WIDTH {
                 let is_set = display.get(x, y);
-                let brightness: u8 =
+                let brightness =
-                    luma.get(x / TILE_SIZE, y / TILE_SIZE).into();
+                    u8::from(luma.get(x / TILE_SIZE, y / TILE_SIZE));
-                let max_brightness: u8 = Brightness::MAX.into();
+                let scale =
-                let scale: f32 = (u8::MAX as f32) / (max_brightness as f32);
+                    (u8::MAX as f32) / (u8::from(Brightness::MAX) as f32);
-
                 let brightness = (scale * brightness as f32) as u8;
+                let color = self.get_color(is_set, brightness);
+                let pixel = frame.next().unwrap();
+                pixel.copy_from_slice(&color);
+            }
+        }
+    }
 
-                let color = if is_set {
+    fn get_color(&self, is_set: bool, brightness: u8) -> [u8; 4] {
+        if is_set {
             [
                 if self.cli.red { brightness } else { 0u8 },
                 if self.cli.green { brightness } else { 0u8 },
@@ -106,15 +114,8 @@ impl<'t> App<'t> {
             ]
         } else {
             [0u8, 0, 0, 255]
-                };
-
-                let pixel = frame.next().unwrap();
-                pixel.copy_from_slice(&color);
         }
     }
-
-        pixels.render().expect("could not render");
-    }
 }
 
 impl ApplicationHandler<AppEvents> for App<'_> {

src/main.rs

@@ -1,20 +1,16 @@
 #![deny(clippy::all)]
 
+use crate::execute_command::CommandExecutor;
+use crate::gui::{App, AppEvents};
+use clap::Parser;
+use log::{info, warn, LevelFilter};
+use servicepoint::*;
 use std::io::ErrorKind;
 use std::net::UdpSocket;
 use std::sync::{mpsc, RwLock};
 use std::time::Duration;
-
-use clap::Parser;
-use log::{info, warn, LevelFilter};
-use servicepoint::*;
 use winit::event_loop::{ControlFlow, EventLoop};
 
-use crate::execute_command::execute_command;
-use crate::font::Cp437Font;
-use crate::font_renderer::FontRenderer8x8;
-use crate::gui::{App, AppEvents};
-
 mod execute_command;
 mod font;
 mod font_renderer;
@@ -34,6 +30,8 @@ struct Cli {
     blue: bool,
 }
 
+const BUF_SIZE: usize = 8985;
+
 fn main() {
     env_logger::builder()
         .filter_level(LevelFilter::Info)
@@ -57,19 +55,8 @@ fn main() {
     luma.fill(Brightness::MAX);
     let luma = RwLock::new(luma);
 
-    run(&display, &luma, socket, Cp437Font::default(), &cli);
-}
-
-fn run(
-    display_ref: &RwLock<Bitmap>,
-    luma_ref: &RwLock<BrightnessGrid>,
-    socket: UdpSocket,
-    cp437_font: Cp437Font,
-    cli: &Cli,
-) {
     let (stop_udp_tx, stop_udp_rx) = mpsc::channel();
-
+    let mut app = App::new(&display, &luma, stop_udp_tx, &cli);
-    let mut app = App::new(display_ref, luma_ref, stop_udp_tx, cli);
 
     let event_loop = EventLoop::with_user_event()
         .build()
@@ -77,46 +64,23 @@ fn run(
     event_loop.set_control_flow(ControlFlow::Wait);
 
     let event_proxy = event_loop.create_proxy();
+    let command_executor = CommandExecutor::new(&display, &luma);
 
     std::thread::scope(move |scope| {
-        let udp_thread = scope.spawn(move || {
+        scope.spawn(move || {
-            let mut buf = [0; 8985];
+            let mut buf = [0; BUF_SIZE];
-            let utf8_font = FontRenderer8x8::default();
-
             while stop_udp_rx.try_recv().is_err() {
-                let (amount, _) = match socket.recv_from(&mut buf) {
+                let amount = match receive_into_buf(&socket, &mut buf) {
-                    Err(err) if err.kind() == ErrorKind::WouldBlock => {
+                    Some(value) => value,
-                        std::thread::sleep(Duration::from_millis(1));
+                    None => continue,
-                        continue;
-                    }
-                    Ok(result) => result,
-                    other => other.unwrap(),
                 };
 
-                if amount == buf.len() {
+                let command = match command_from_slice(&buf[..amount]) {
-                    warn!(
+                    Some(value) => value,
-                        "the received package may have been truncated to a length of {}",
+                    None => continue,
-                        amount
-                    );
-                }
-
-                let package = match servicepoint::Packet::try_from(&buf[..amount]) {
-                    Err(_) => {
-                        warn!("could not load packet with length {amount} into header");
-                        continue;
-                    }
-                    Ok(package) => package,
                 };
 
-                let command = match Command::try_from(package) {
+                if !command_executor.execute(command) {
-                    Err(err) => {
-                        warn!("could not read command for packet: {:?}", err);
-                        continue;
-                    }
-                    Ok(val) => val,
-                };
-
-                if !execute_command(command, &cp437_font, &utf8_font, display_ref, luma_ref) {
                     // hard reset
                     event_proxy
                         .send_event(AppEvents::UdpThreadClosed)
@@ -129,11 +93,49 @@ fn run(
                     .expect("could not send packet handled event");
             }
         });
-
         event_loop
             .run_app(&mut app)
             .expect("could not run event loop");
-
-        udp_thread.join().expect("could not join udp thread");
     });
 }
+
+fn command_from_slice(slice: &[u8]) -> Option<Command> {
+    let package = match servicepoint::Packet::try_from(slice) {
+        Err(_) => {
+            warn!("could not load packet with length {}", slice.len());
+            return None;
+        }
+        Ok(package) => package,
+    };
+
+    let command = match Command::try_from(package) {
+        Err(err) => {
+            warn!("could not read command for packet: {:?}", err);
+            return None;
+        }
+        Ok(val) => val,
+    };
+    Some(command)
+}
+
+fn receive_into_buf(
+    socket: &UdpSocket,
+    buf: &mut [u8; BUF_SIZE],
+) -> Option<usize> {
+    let (amount, _) = match socket.recv_from(buf) {
+        Err(err) if err.kind() == ErrorKind::WouldBlock => {
+            std::thread::sleep(Duration::from_millis(1));
+            return None;
+        }
+        Ok(result) => result,
+        other => other.unwrap(),
+    };
+
+    if amount == buf.len() {
+        warn!(
+            "the received package may have been truncated to a length of {}",
+            amount
+        );
+    }
+    Some(amount)
+}