rename App to Gui, "optimize" some stuff, ExecutionResult

flake.nix

@@ -71,6 +71,8 @@
               [
                 xe
                 xz
+
+                roboto
               ]
               ++ lib.optionals pkgs.stdenv.isLinux (
                 with pkgs;

src/execute_command.rs

@@ -1,12 +1,13 @@
-use log::{debug, error, info, trace, warn};
+use crate::execute_command::ExecutionResult::{Failure, Shutdown, Success};
-use servicepoint::{
-    Bitmap, BrightnessGrid, CharGrid, Command, Cp437Grid, Grid, Origin, Tiles,
-    PIXEL_COUNT, PIXEL_WIDTH, TILE_SIZE,
-};
-use std::sync::RwLock;
-
 use crate::font::Cp437Font;
 use crate::font_renderer::FontRenderer8x8;
+use log::{debug, error, info, trace, warn};
+use servicepoint::{
+    BitVec, Bitmap, BrightnessGrid, CharGrid, Command, Cp437Grid, Grid, Offset,
+    Origin, Tiles, PIXEL_COUNT, PIXEL_WIDTH, TILE_SIZE,
+};
+use std::ops::{BitAnd, BitOr, BitXor};
+use std::sync::RwLock;
 
 pub struct CommandExecutor<'t> {
     display: &'t RwLock<Bitmap>,
@@ -15,6 +16,13 @@ pub struct CommandExecutor<'t> {
     utf8_font: FontRenderer8x8,
 }
 
+#[must_use]
+pub enum ExecutionResult {
+    Success,
+    Failure,
+    Shutdown,
+}
+
 impl<'t> CommandExecutor<'t> {
     pub fn new(
         display: &'t RwLock<Bitmap>,
@@ -28,100 +36,92 @@ impl<'t> CommandExecutor<'t> {
         }
     }
 
-    pub(crate) fn execute(&self, command: Command) -> bool {
+    pub(crate) fn execute(&self, command: Command) -> ExecutionResult {
         debug!("received {command:?}");
         match command {
             Command::Clear => {
                 info!("clearing display");
                 self.display.write().unwrap().fill(false);
+                Success
             }
             Command::HardReset => {
                 warn!("display shutting down");
-                return false;
+                Shutdown
             }
             Command::BitmapLinearWin(Origin { x, y, .. }, pixels, _) => {
-                self.print_pixel_grid(x, y, &pixels);
+                self.print_pixel_grid(x, y, &pixels)
             }
             Command::Cp437Data(origin, grid) => {
-                self.print_cp437_data(origin, &grid);
+                self.print_cp437_data(origin, &grid)
             }
             #[allow(deprecated)]
             Command::BitmapLegacy => {
                 warn!("ignoring deprecated command {:?}", command);
-            }
+                Failure
-            // TODO: how to deduplicate this code in a rusty way?
-            Command::BitmapLinear(offset, vec, _) => {
-                if !Self::check_bitmap_valid(offset as u16, vec.len()) {
-                    return true;
-                }
-                let mut display = self.display.write().unwrap();
-                for bitmap_index in 0..vec.len() {
-                    let (x, y) =
-                        Self::get_coordinates_for_index(offset, bitmap_index);
-                    display.set(x, y, vec[bitmap_index]);
-                }
             }
             Command::BitmapLinearAnd(offset, vec, _) => {
-                if !Self::check_bitmap_valid(offset as u16, vec.len()) {
+                self.execute_bitmap_linear(offset, vec, BitAnd::bitand)
-                    return true;
-                }
-                let mut display = self.display.write().unwrap();
-                for bitmap_index in 0..vec.len() {
-                    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 !Self::check_bitmap_valid(offset as u16, vec.len()) {
+                self.execute_bitmap_linear(offset, vec, BitOr::bitor)
-                    return true;
-                }
-                let mut display = self.display.write().unwrap();
-                for bitmap_index in 0..vec.len() {
-                    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 !Self::check_bitmap_valid(offset as u16, vec.len()) {
+                self.execute_bitmap_linear(offset, vec, BitXor::bitxor)
-                    return true;
+            }
-                }
+            Command::BitmapLinear(offset, vec, _) => {
-                let mut display = self.display.write().unwrap();
+                self.execute_bitmap_linear(offset, vec, move |_, new| new)
-                for bitmap_index in 0..vec.len() {
-                    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 = self.luma.write().unwrap();
+                self.execute_char_brightness(origin, grid)
-                for inner_y in 0..grid.height() {
-                    for inner_x in 0..grid.width() {
-                        let brightness = grid.get(inner_x, inner_y);
-                        luma.set(
-                            origin.x + inner_x,
-                            origin.y + inner_y,
-                            brightness,
-                        );
-                    }
-                }
             }
             Command::Brightness(brightness) => {
                 self.luma.write().unwrap().fill(brightness);
+                Success
             }
             Command::FadeOut => {
-                error!("command not implemented: {command:?}")
+                error!("command not implemented: {command:?}");
+                Success
             }
             Command::Utf8Data(origin, grid) => {
-                self.print_utf8_data(origin, &grid);
+                self.print_utf8_data(origin, &grid)
             }
-        };
+        }
+    }
 
-        true
+    fn execute_char_brightness(
+        &self,
+        origin: Origin<Tiles>,
+        grid: BrightnessGrid,
+    ) -> ExecutionResult {
+        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);
+                luma.set(origin.x + inner_x, origin.y + inner_y, brightness);
+            }
+        }
+        Success
+    }
+
+    fn execute_bitmap_linear<Op>(
+        &self,
+        offset: Offset,
+        vec: BitVec,
+        op: Op,
+    ) -> ExecutionResult
+    where
+        Op: Fn(bool, bool) -> bool,
+    {
+        if !Self::check_bitmap_valid(offset as u16, vec.len()) {
+            return Failure;
+        }
+        let mut display = self.display.write().unwrap();
+        for bitmap_index in 0..vec.len() {
+            let (x, y) = Self::get_coordinates_for_index(offset, bitmap_index);
+            let old_value = display.get(x, y);
+            display.set(x, y, op(old_value, vec[bitmap_index]));
+        }
+        Success
     }
 
     fn check_bitmap_valid(offset: u16, payload_len: usize) -> bool {
@@ -135,7 +135,11 @@ impl<'t> CommandExecutor<'t> {
         true
     }
 
-    fn print_cp437_data(&self, origin: Origin<Tiles>, grid: &Cp437Grid) {
+    fn print_cp437_data(
+        &self,
+        origin: Origin<Tiles>,
+        grid: &Cp437Grid,
+    ) -> ExecutionResult {
         let font = &self.cp437_font;
         let Origin { x, y, .. } = origin;
         for char_y in 0usize..grid.height() {
@@ -150,19 +154,31 @@ impl<'t> CommandExecutor<'t> {
                 let tile_y = char_y + y;
 
                 let bitmap = font.get_bitmap(char_code);
-                if !self.print_pixel_grid(
+                match self.print_pixel_grid(
                     tile_x * TILE_SIZE,
                     tile_y * TILE_SIZE,
                     bitmap,
                 ) {
-                    error!("stopping drawing text because char draw failed");
+                    Success => {}
-                    return;
+                    Failure => {
+                        error!(
+                            "stopping drawing text because char draw failed"
+                        );
+                        return Failure;
+                    }
+                    Shutdown => return Shutdown,
                 }
             }
         }
+
+        Success
     }
 
-    fn print_utf8_data(&self, origin: Origin<Tiles>, grid: &CharGrid) {
+    fn print_utf8_data(
+        &self,
+        origin: Origin<Tiles>,
+        grid: &CharGrid,
+    ) -> ExecutionResult {
         let mut display = self.display.write().unwrap();
 
         let Origin { x, y, .. } = origin;
@@ -182,10 +198,12 @@ impl<'t> CommandExecutor<'t> {
                     error!(
                         "stopping drawing text because char draw failed: {e}"
                     );
-                    return;
+                    return Failure;
                 }
             }
         }
+
+        Success
     }
 
     fn print_pixel_grid(
@@ -193,7 +211,7 @@ impl<'t> CommandExecutor<'t> {
         offset_x: usize,
         offset_y: usize,
         pixels: &Bitmap,
-    ) -> bool {
+    ) -> ExecutionResult {
         debug!(
             "printing {}x{} grid at {offset_x} {offset_y}",
             pixels.width(),
@@ -208,14 +226,14 @@ impl<'t> CommandExecutor<'t> {
 
                 if x >= display.width() || y >= display.height() {
                     error!("stopping pixel grid draw because coordinate {x} {y} is out of bounds");
-                    return false;
+                    return Failure;
                 }
 
                 display.set(x, y, is_set);
             }
         }
 
-        true
+        Success
     }
 
     fn get_coordinates_for_index(

src/gui.rs

@@ -4,10 +4,7 @@ use std::sync::RwLock;
 
 use log::{info, warn};
 use pixels::{Pixels, SurfaceTexture};
-use servicepoint::{
+use servicepoint::*;
-    Bitmap, Brightness, BrightnessGrid, Grid, PIXEL_HEIGHT, PIXEL_WIDTH,
-    TILE_SIZE,
-};
 use winit::application::ApplicationHandler;
 use winit::dpi::LogicalSize;
 use winit::event::WindowEvent;
@@ -17,7 +14,7 @@ use winit::window::{Window, WindowId};
 
 use crate::Cli;
 
-pub struct App<'t> {
+pub struct Gui<'t> {
     display: &'t RwLock<Bitmap>,
     luma: &'t RwLock<BrightnessGrid>,
     window: Option<Window>,
@@ -27,6 +24,11 @@ pub struct App<'t> {
 }
 
 const SPACER_HEIGHT: usize = 4;
+const NUM_SPACERS: usize = (PIXEL_HEIGHT / TILE_SIZE) - 1;
+const PIXEL_HEIGHT_WITH_SPACERS: usize =
+    PIXEL_HEIGHT + NUM_SPACERS * SPACER_HEIGHT;
+
+const OFF_COLOR: [u8; 4] = [0u8, 0, 0, 255];
 
 #[derive(Debug)]
 pub enum AppEvents {
@@ -34,30 +36,20 @@ pub enum AppEvents {
     UdpThreadClosed,
 }
 
-impl<'t> App<'t> {
+impl<'t> Gui<'t> {
     pub fn new(
         display: &'t RwLock<Bitmap>,
         luma: &'t RwLock<BrightnessGrid>,
         stop_udp_tx: Sender<()>,
         cli: &'t Cli,
     ) -> Self {
-        let logical_size = {
+        Gui {
-            let height = if cli.spacers {
-                let num_spacers = (PIXEL_HEIGHT / TILE_SIZE) - 1;
-                PIXEL_HEIGHT + num_spacers * SPACER_HEIGHT
-            } else {
-                PIXEL_HEIGHT
-            };
-            LogicalSize::new(PIXEL_WIDTH as u16, height as u16)
-        };
-
-        App {
             display,
             luma,
             stop_udp_tx,
-            window: None,
             cli,
-            logical_size,
+            window: None,
+            logical_size: Self::get_logical_size(cli.spacers),
         }
     }
 
@@ -66,6 +58,9 @@ impl<'t> App<'t> {
         let window_size = window.inner_size();
         let surface_texture =
             SurfaceTexture::new(window_size.width, window_size.height, &window);
+
+        // TODO: fix pixels: creating a new instance per draw crashes after some time on macOS,
+        // but keeping one instance for the lifetime of the Gui SIGSEGVs on Wayland when entering a background state.
         let mut pixels = Pixels::new(
             self.logical_size.width as u32,
             self.logical_size.height as u32,
@@ -81,44 +76,53 @@ impl<'t> App<'t> {
     fn draw_frame(&self, frame: &mut ChunksExactMut<u8>) {
         let display = self.display.read().unwrap();
         let luma = self.luma.read().unwrap();
+        let brightness_scale = (u8::MAX as f32) / (u8::from(Brightness::MAX) as f32);
 
-        for y in 0..PIXEL_HEIGHT {
+        for tile_y in 0..TILE_HEIGHT {
-            if self.cli.spacers && y != 0 && y % TILE_SIZE == 0 {
+            if self.cli.spacers && tile_y != 0 {
                 // cannot just frame.skip(PIXEL_WIDTH as usize * SPACER_HEIGHT as usize) because of typing
                 for _ in 0..PIXEL_WIDTH * SPACER_HEIGHT {
                     frame.next().unwrap();
                 }
             }
 
-            for x in 0..PIXEL_WIDTH {
+            let start_y = tile_y * TILE_SIZE;
-                let is_set = display.get(x, y);
+            for y in start_y..start_y + TILE_SIZE {
-                let brightness =
+                for tile_x in 0..TILE_WIDTH {
-                    u8::from(luma.get(x / TILE_SIZE, y / TILE_SIZE));
+                    let brightness = u8::from(luma.get(tile_x, tile_y));
-                let scale =
+                    let brightness = (brightness_scale * brightness as f32) as u8;
-                    (u8::MAX as f32) / (u8::from(Brightness::MAX) as f32);
+                    let on_color = self.get_on_color(brightness);
-                let brightness = (scale * brightness as f32) as u8;
+                    let start_x = tile_x * TILE_SIZE;
-                let color = self.get_color(is_set, brightness);
+                    for x in start_x..start_x + TILE_SIZE {
-                let pixel = frame.next().unwrap();
+                        let color = if display.get(x, y) { on_color } else { OFF_COLOR };
-                pixel.copy_from_slice(&color);
+                        let pixel = frame.next().unwrap();
+                        pixel.copy_from_slice(&color);
+                    }
+                }
             }
         }
     }
 
-    fn get_color(&self, is_set: bool, brightness: u8) -> [u8; 4] {
+    fn get_on_color(&self, brightness: u8) -> [u8; 4] {
-        if is_set {
+        [
-            [
+            if self.cli.red { brightness } else { 0u8 },
-                if self.cli.red { brightness } else { 0u8 },
+            if self.cli.green { brightness } else { 0u8 },
-                if self.cli.green { brightness } else { 0u8 },
+            if self.cli.blue { brightness } else { 0u8 },
-                if self.cli.blue { brightness } else { 0u8 },
+            255,
-                255,
+        ]
-            ]
+    }
+
+    fn get_logical_size(spacers: bool) -> LogicalSize<u16> {
+        let height = if spacers {
+            PIXEL_HEIGHT_WITH_SPACERS
         } else {
-            [0u8, 0, 0, 255]
+            PIXEL_HEIGHT
-        }
+        };
+        LogicalSize::new(PIXEL_WIDTH as u16, height as u16)
     }
 }
 
-impl ApplicationHandler<AppEvents> for App<'_> {
+impl ApplicationHandler<AppEvents> for Gui<'_> {
     fn resumed(&mut self, event_loop: &ActiveEventLoop) {
         let attributes = Window::default_attributes()
             .with_title("servicepoint-simulator")

src/main.rs

@@ -1,9 +1,11 @@
 #![deny(clippy::all)]
 
-use crate::execute_command::CommandExecutor;
+use crate::{
-use crate::gui::{App, AppEvents};
+    execute_command::{CommandExecutor, ExecutionResult},
+    gui::{AppEvents, Gui},
+};
 use clap::Parser;
-use log::{info, warn, LevelFilter};
+use log::{error, info, warn, LevelFilter};
 use servicepoint::*;
 use std::io::ErrorKind;
 use std::net::UdpSocket;
@@ -18,27 +20,62 @@ mod gui;
 
 #[derive(Parser, Debug)]
 struct Cli {
-    #[arg(long, default_value = "0.0.0.0:2342")]
+    #[arg(
+        long,
+        default_value = "0.0.0.0:2342",
+        help = "address and port to bind to"
+    )]
     bind: String,
-    #[arg(short, long, default_value_t = false)]
+    #[arg(
+        short,
+        long,
+        default_value_t = false,
+        help = "add spacers between tile rows to simulate gaps in real display"
+    )]
     spacers: bool,
-    #[arg(short, long, default_value_t = false)]
+    #[arg(
+        short,
+        long,
+        help = "Set default log level lower. You can also change this via the RUST_LOG environment variable."
+    )]
+    debug: bool,
+    #[arg(
+        short,
+        long,
+        default_value_t = false,
+        help = "Use the red color channel"
+    )]
     red: bool,
-    #[arg(short, long, default_value_t = false)]
+    #[arg(
+        short,
+        long,
+        default_value_t = false,
+        help = "Use the green color channel"
+    )]
     green: bool,
-    #[arg(short, long, default_value_t = false)]
+    #[arg(
+        short,
+        long,
+        default_value_t = false,
+        help = "Use the blue color channel"
+    )]
     blue: bool,
 }
 
 const BUF_SIZE: usize = 8985;
 
 fn main() {
+    let mut cli = Cli::parse();
+
     env_logger::builder()
-        .filter_level(LevelFilter::Info)
+        .filter_level(if cli.debug {
+            LevelFilter::Debug
+        } else {
+            LevelFilter::Info
+        })
         .parse_default_env()
         .init();
 
-    let mut cli = Cli::parse();
     if !(cli.red || cli.blue || cli.green) {
         cli.green = true;
     }
@@ -56,7 +93,7 @@ fn main() {
     let luma = RwLock::new(luma);
 
     let (stop_udp_tx, stop_udp_rx) = mpsc::channel();
-    let mut app = App::new(&display, &luma, stop_udp_tx, &cli);
+    let mut app = Gui::new(&display, &luma, stop_udp_tx, &cli);
 
     let event_loop = EventLoop::with_user_event()
         .build()
@@ -80,17 +117,21 @@ fn main() {
                     None => continue,
                 };
 
-                if !command_executor.execute(command) {
+                match command_executor.execute(command) {
-                    // hard reset
+                    ExecutionResult::Success => {
-                    event_proxy
+                        event_proxy
-                        .send_event(AppEvents::UdpThreadClosed)
+                            .send_event(AppEvents::UdpPacketHandled)
-                        .expect("could not send close event");
+                            .expect("could not send packet handled event");
-                    break;
+                    }
+                    ExecutionResult::Failure => {
+                        error!("failed to execute command");
+                    }
+                    ExecutionResult::Shutdown => {
+                        event_proxy
+                            .send_event(AppEvents::UdpThreadClosed)
+                            .expect("could not send close event");
+                    }
                 }
-
-                event_proxy
-                    .send_event(AppEvents::UdpPacketHandled)
-                    .expect("could not send packet handled event");
             }
         });
         event_loop