Merge pull request #2 from kaesaecracker/refactor

Refactor

.github/workflows/rust.yml

@@ -16,7 +16,9 @@ jobs:
 
     steps:
     - uses: actions/checkout@v4
-    - name: Install liblzma-dev
-      run: sudo apt-get install -y liblzma-dev
+
+    - name: Install system dependencies
+      run: sudo apt-get install -y liblzma-dev libfontconfig1-dev
+
     - name: Build
       run: cargo build --verbose

README.md

@@ -23,7 +23,19 @@ Without nix: check out this repository and use `cargo run --release`.
 
 ## Command line arguments
 
-The application binds to `0.0.0.0:2342` by default (`./servicepoint-simulator --bind host:port` to change this).
+```
+Usage: servicepoint-simulator [OPTIONS]
+
+Options:
+      --bind <BIND>  address and port to bind to [default: 0.0.0.0:2342]
+  -f, --font <FONT>  The name of the font family to use. This defaults to the system monospace font.
+  -s, --spacers      add spacers between tile rows to simulate gaps in real display
+  -r, --red          Use the red color channel
+  -g, --green        Use the green color channel
+  -b, --blue         Use the blue color channel
+  -v, --verbose      Set default log level lower. You can also change this via the RUST_LOG environment variable.
+  -h, --help         Print help
+```
 
 See [env_logger](https://docs.rs/env_logger/latest/env_logger/) to configure logging.
 

flake.nix

@@ -25,97 +25,24 @@
         "aarch64-darwin"
         "x86_64-darwin"
       ];
-      forAllSystems = lib.genAttrs supported-systems;
-      make-rust-toolchain-core =
-        pkgs:
-        pkgs.symlinkJoin {
-          name = "rust-toolchain-core";
-          paths = with pkgs; [
-            rustc
-            cargo
-            rustPlatform.rustcSrc
-          ];
-        };
+      forAllSystems =
+        f:
+        lib.genAttrs supported-systems (
+          system:
+          f rec {
+            pkgs = nixpkgs.legacyPackages.${system};
+            inherit system;
+          }
+        );
     in
     rec {
       packages = forAllSystems (
-        system:
-        let
-          pkgs = nixpkgs.legacyPackages."${system}";
-          rust-toolchain-core = make-rust-toolchain-core pkgs;
-          naersk' = pkgs.callPackage naersk {
-            cargo = rust-toolchain-core;
-            rustc = rust-toolchain-core;
-          };
-        in
+        { pkgs, ... }:
         rec {
-          servicepoint-simulator = naersk'.buildPackage rec {
-            src = nix-filter.lib.filter {
-              root = ./.;
-              include = [
-                ./Cargo.toml
-                ./Cargo.lock
-                ./src
-                ./Web437_IBM_BIOS.woff
-                ./README.md
-                ./LICENSE
-              ];
-            };
-            nativeBuildInputs = with pkgs; [
-              pkg-config
-              makeWrapper
-            ];
-            strictDeps = true;
-            buildInputs =
-              with pkgs;
-              [
-                xe
-                xz
-              ]
-              ++ lib.optionals pkgs.stdenv.isLinux (
-                with pkgs;
-                [
-                  # gpu
-                  libGL
-                  vulkan-headers
-                  vulkan-loader
-                  vulkan-tools vulkan-tools-lunarg
-                  vulkan-extension-layer
-                  vulkan-validation-layers
-
-                  # keyboard
-                  libxkbcommon
-
-                   # font loading
-                  fontconfig
-                  freetype
-
-                  # WINIT_UNIX_BACKEND=wayland
-                  wayland
-
-                  # WINIT_UNIX_BACKEND=x11
-                  xorg.libXcursor
-                  xorg.libXrandr
-                  xorg.libXi
-                  xorg.libX11
-                  xorg.libX11.dev
-                ]
-              )
-              ++ lib.optionals pkgs.stdenv.isDarwin (
-                with pkgs.darwin.apple_sdk.frameworks;
-                [
-                  Carbon
-                  QuartzCore
-                  AppKit
-                ]
-              );
-
-            postInstall = ''
-              wrapProgram $out/bin/servicepoint-simulator \
-                --suffix LD_LIBRARY_PATH : ${lib.makeLibraryPath buildInputs}
-            '';
+          servicepoint-simulator = import ./servicepoint-simulator.nix {
+            inherit nix-filter pkgs;
+            naersk' = pkgs.callPackage naersk { };
           };
-
           default = servicepoint-simulator;
         }
       );
@@ -123,29 +50,35 @@
       legacyPackages = packages;
 
       devShells = forAllSystems (
-        system:
-        let
-          pkgs = nixpkgs.legacyPackages."${system}";
-          rust-toolchain = pkgs.symlinkJoin {
-            name = "rust-toolchain";
-            paths = with pkgs; [
-              (make-rust-toolchain-core pkgs)
-              rustfmt
-              clippy
-              cargo-expand
-            ];
-          };
-        in
+        {
+          pkgs,
+          system,
+        }:
         {
           default = pkgs.mkShell rec {
             inputsFrom = [ self.packages.${system}.default ];
-            packages = [ rust-toolchain pkgs.gdb ];
+            packages = [
+              pkgs.gdb
+              (pkgs.symlinkJoin {
+                name = "rust-toolchain";
+                paths = with pkgs; [
+                  rustc
+                  cargo
+                  rustPlatform.rustcSrc
+                  rustfmt
+                  clippy
+                  cargo-expand
+                ];
+              })
+            ];
             LD_LIBRARY_PATH = "${pkgs.lib.makeLibraryPath (builtins.concatMap (d: d.buildInputs) inputsFrom)}";
+            NIX_LD_LIBRARY_PATH = LD_LIBRARY_PATH;
+            NIX_LD = pkgs.stdenv.cc.bintools.dynamicLinker;
             RUST_SRC_PATH = "${pkgs.rust.packages.stable.rustPlatform.rustLibSrc}";
           };
         }
       );
 
-      formatter = forAllSystems (system: nixpkgs.legacyPackages."${system}".nixfmt-rfc-style);
+      formatter = forAllSystems ({ pkgs, ... }: pkgs.nixfmt-rfc-style);
     };
 }

servicepoint-simulator.nix

@@ -0,0 +1,74 @@
+{
+  naersk',
+  pkgs,
+  nix-filter,
+}:
+naersk'.buildPackage rec {
+  src = nix-filter.lib.filter {
+    root = ./.;
+    include = [
+      ./Cargo.toml
+      ./Cargo.lock
+      ./src
+      ./Web437_IBM_BIOS.woff
+      ./README.md
+      ./LICENSE
+    ];
+  };
+  nativeBuildInputs = with pkgs; [
+    pkg-config
+    makeWrapper
+  ];
+  strictDeps = true;
+  buildInputs =
+    with pkgs;
+    [
+      xe
+      xz
+
+      roboto
+    ]
+    ++ lib.optionals pkgs.stdenv.isLinux (
+      with pkgs;
+      [
+        # gpu
+        libGL
+        vulkan-headers
+        vulkan-loader
+        vulkan-tools
+        vulkan-tools-lunarg
+        vulkan-extension-layer
+        vulkan-validation-layers
+
+        # keyboard
+        libxkbcommon
+
+        # font loading
+        fontconfig
+        freetype
+
+        # WINIT_UNIX_BACKEND=wayland
+        wayland
+
+        # WINIT_UNIX_BACKEND=x11
+        xorg.libXcursor
+        xorg.libXrandr
+        xorg.libXi
+        xorg.libX11
+        xorg.libX11.dev
+      ]
+    )
+    ++ lib.optionals pkgs.stdenv.isDarwin (
+      with pkgs.darwin.apple_sdk.frameworks;
+      [
+        Carbon
+        QuartzCore
+        AppKit
+      ]
+    );
+
+  postInstall = ''
+    wrapProgram $out/bin/servicepoint-simulator \
+      --suffix LD_LIBRARY_PATH : ${pkgs.lib.makeLibraryPath buildInputs}
+  '';
+}

src/cli.rs

@@ -0,0 +1,57 @@
+use clap::Parser;
+
+#[derive(Parser, Debug)]
+pub struct Cli {
+    #[arg(
+        long,
+        default_value = "0.0.0.0:2342",
+        help = "address and port to bind to"
+    )]
+    pub bind: String,
+    #[arg(
+        short,
+        long,
+        help = "The name of the font family to use. This defaults to the system monospace font."
+    )]
+    pub font: Option<String>,
+    #[clap(flatten)]
+    pub gui: GuiOptions,
+    #[arg(
+        short,
+        long,
+        help = "Set default log level lower. You can also change this via the RUST_LOG environment variable."
+    )]
+    pub verbose: bool,
+}
+
+#[derive(Parser, Debug)]
+pub struct GuiOptions {
+    #[arg(
+        short,
+        long,
+        default_value_t = false,
+        help = "add spacers between tile rows to simulate gaps in real display"
+    )]
+    pub spacers: bool,
+    #[arg(
+        short,
+        long,
+        default_value_t = false,
+        help = "Use the red color channel"
+    )]
+    pub red: bool,
+    #[arg(
+        short,
+        long,
+        default_value_t = false,
+        help = "Use the green color channel"
+    )]
+    pub green: bool,
+    #[arg(
+        short,
+        long,
+        default_value_t = false,
+        help = "Use the blue color channel"
+    )]
+    pub blue: bool,
+}

src/cp437_font.rs

@@ -1,4 +1,5 @@
 use servicepoint::{Bitmap, DataRef, TILE_SIZE};
+use std::ops::Index;
 
 const CHAR_COUNT: usize = u8::MAX as usize + 1;
 
@@ -10,15 +11,29 @@ impl Cp437Font {
     pub fn new(bitmaps: [Bitmap; CHAR_COUNT]) -> Self {
         Self { bitmaps }
     }
-
-    pub fn get_bitmap(&self, char_code: u8) -> &Bitmap {
-        &self.bitmaps[char_code as usize]
-    }
 }
 
 impl Default for Cp437Font {
     fn default() -> Self {
-        load_static()
+        let mut bitmaps =
+            core::array::from_fn(|_| Bitmap::new(TILE_SIZE, TILE_SIZE));
+
+        for (char_code, bitmap) in bitmaps.iter_mut().enumerate() {
+            let bits = CP437_FONT_LINEAR[char_code];
+            let mut bytes = bits.to_be_bytes();
+            bytes.reverse();
+            bitmap.data_ref_mut().copy_from_slice(bytes.as_slice());
+        }
+
+        Self::new(bitmaps)
+    }
+}
+
+impl Index<u8> for Cp437Font {
+    type Output = Bitmap;
+
+    fn index(&self, char_code: u8) -> &Self::Output {
+        &self.bitmaps[char_code as usize]
     }
 }
 
@@ -281,17 +296,3 @@ pub(crate) const CP437_FONT_LINEAR: [u64; 256] = [
     0x00007c7c7c7c7c00, // 0xfe
     0x0000000000000000, // 0xff
 ];
-
-fn load_static() -> Cp437Font {
-    let mut bitmaps =
-        core::array::from_fn(|_| Bitmap::new(TILE_SIZE, TILE_SIZE));
-
-    for (char_code, bitmap) in bitmaps.iter_mut().enumerate() {
-        let bits = CP437_FONT_LINEAR[char_code];
-        let mut bytes = bits.to_be_bytes();
-        bytes.reverse();
-        bitmap.data_ref_mut().copy_from_slice(bytes.as_slice());
-    }
-
-    Cp437Font::new(bitmaps)
-}

src/execute_command.rs

@@ -1,212 +1,246 @@
+use crate::cp437_font::Cp437Font;
+use crate::execute_command::ExecutionResult::{Failure, Shutdown, Success};
+use crate::font_renderer::FontRenderer8x8;
 use log::{debug, error, info, trace, warn};
 use servicepoint::{
-    Bitmap, BrightnessGrid, CharGrid, Command, Cp437Grid, Grid, Origin, Tiles,
-    PIXEL_COUNT, PIXEL_WIDTH, TILE_SIZE,
+    BitVec, Bitmap, BrightnessGrid, CharGrid, Command, Cp437Grid, Grid, Offset,
+    Origin, Tiles, PIXEL_COUNT, PIXEL_WIDTH, TILE_SIZE,
 };
-use std::sync::{RwLock, RwLockWriteGuard};
+use std::ops::{BitAnd, BitOr, BitXor};
+use std::sync::RwLock;
 
-use crate::font::Cp437Font;
-use crate::font_renderer::FontRenderer8x8;
-
-pub(crate) fn execute_command(
-    command: Command,
-    cp436_font: &Cp437Font,
-    utf8_font: &FontRenderer8x8,
-    display_ref: &RwLock<Bitmap>,
-    luma_ref: &RwLock<BrightnessGrid>,
-) -> bool {
-    debug!("received {command:?}");
-    match command {
-        Command::Clear => {
-            info!("clearing display");
-            display_ref.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();
-            print_pixel_grid(x, y, &pixels, &mut display);
-        }
-        Command::Cp437Data(origin, grid) => {
-            let mut display = display_ref.write().unwrap();
-            print_cp437_data(origin, &grid, cp436_font, &mut display);
-        }
-        #[allow(deprecated)]
-        Command::BitmapLegacy => {
-            warn!("ignoring deprecated command {:?}", command);
-        }
-        // TODO: how to deduplicate this code in a rusty way?
-        Command::BitmapLinear(offset, vec, _) => {
-            if !check_bitmap_valid(offset as u16, vec.len()) {
-                return true;
-            }
-            let mut display = display_ref.write().unwrap();
-            for bitmap_index in 0..vec.len() {
-                let (x, y) = 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()) {
-                return true;
-            }
-            let mut display = display_ref.write().unwrap();
-            for bitmap_index in 0..vec.len() {
-                let (x, y) = 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()) {
-                return true;
-            }
-            let mut display = display_ref.write().unwrap();
-            for bitmap_index in 0..vec.len() {
-                let (x, y) = 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()) {
-                return true;
-            }
-            let mut display = display_ref.write().unwrap();
-            for bitmap_index in 0..vec.len() {
-                let (x, y) = 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();
-            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) => {
-            luma_ref.write().unwrap().fill(brightness);
-        }
-        Command::FadeOut => {
-            error!("command not implemented: {command:?}")
-        }
-        Command::Utf8Data(origin, grid) => {
-            let mut display = display_ref.write().unwrap();
-            print_utf8_data(origin, &grid, utf8_font, &mut display);
-        }
-    };
-
-    true
+pub struct CommandExecutor<'t> {
+    display: &'t RwLock<Bitmap>,
+    luma: &'t RwLock<BrightnessGrid>,
+    cp437_font: Cp437Font,
+    font_renderer: FontRenderer8x8,
 }
 
-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)");
-        return false;
+#[must_use]
+pub enum ExecutionResult {
+    Success,
+    Failure,
+    Shutdown,
+}
+
+impl<'t> CommandExecutor<'t> {
+    pub fn new(
+        display: &'t RwLock<Bitmap>,
+        luma: &'t RwLock<BrightnessGrid>,
+        font_renderer: FontRenderer8x8,
+    ) -> Self {
+        CommandExecutor {
+            display,
+            luma,
+            font_renderer,
+            cp437_font: Cp437Font::default(),
+        }
     }
 
-    true
-}
+    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");
+                Shutdown
+            }
+            Command::BitmapLinearWin(Origin { x, y, .. }, pixels, _) => {
+                self.print_pixel_grid(x, y, &pixels)
+            }
+            Command::Cp437Data(origin, grid) => {
+                self.print_cp437_data(origin, &grid)
+            }
+            #[allow(deprecated)]
+            Command::BitmapLegacy => {
+                warn!("ignoring deprecated command {:?}", command);
+                Failure
+            }
+            Command::BitmapLinearAnd(offset, vec, _) => {
+                self.execute_bitmap_linear(offset, vec, BitAnd::bitand)
+            }
+            Command::BitmapLinearOr(offset, vec, _) => {
+                self.execute_bitmap_linear(offset, vec, BitOr::bitor)
+            }
+            Command::BitmapLinearXor(offset, vec, _) => {
+                self.execute_bitmap_linear(offset, vec, BitXor::bitxor)
+            }
+            Command::BitmapLinear(offset, vec, _) => {
+                self.execute_bitmap_linear(offset, vec, move |_, new| new)
+            }
+            Command::CharBrightness(origin, grid) => {
+                self.execute_char_brightness(origin, grid)
+            }
+            Command::Brightness(brightness) => {
+                self.luma.write().unwrap().fill(brightness);
+                Success
+            }
+            Command::FadeOut => {
+                error!("command not implemented: {command:?}");
+                Success
+            }
+            Command::Utf8Data(origin, grid) => {
+                self.print_utf8_data(origin, &grid)
+            }
+        }
+    }
 
-fn print_cp437_data(
-    origin: Origin<Tiles>,
-    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() {
-            let char_code = grid.get(char_x, char_y);
-            trace!(
+    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 {
+        if offset as usize + payload_len > PIXEL_COUNT {
+            error!(
+                "bitmap with offset {offset} is too big ({payload_len} bytes)"
+            );
+            return false;
+        }
+
+        true
+    }
+
+    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() {
+            for char_x in 0usize..grid.width() {
+                let char_code = grid.get(char_x, char_y);
+                trace!(
                 "drawing char_code {char_code:#04x} (if this was UTF-8, it would be {})",
                 char::from(char_code)
             );
 
-            let tile_x = char_x + x;
-            let tile_y = char_y + y;
+                let tile_x = char_x + x;
+                let tile_y = char_y + y;
 
-            let bitmap = font.get_bitmap(char_code);
-            if !print_pixel_grid(
-                tile_x * TILE_SIZE,
-                tile_y * TILE_SIZE,
-                bitmap,
-                display,
-            ) {
-                error!("stopping drawing text because char draw failed");
-                return;
+                match self.print_pixel_grid(
+                    tile_x * TILE_SIZE,
+                    tile_y * TILE_SIZE,
+                    &font[char_code],
+                ) {
+                    Success => {}
+                    Failure => {
+                        error!(
+                            "stopping drawing text because char draw failed"
+                        );
+                        return Failure;
+                    }
+                    Shutdown => return Shutdown,
+                }
             }
         }
+
+        Success
     }
-}
 
-fn print_utf8_data(
-    origin: Origin<Tiles>,
-    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() {
-            let char = grid.get(char_x, char_y);
-            trace!("drawing {char}");
+    fn print_utf8_data(
+        &self,
+        origin: Origin<Tiles>,
+        grid: &CharGrid,
+    ) -> ExecutionResult {
+        let mut display = self.display.write().unwrap();
 
-            let tile_x = char_x + x;
-            let tile_y = char_y + y;
+        let Origin { x, y, .. } = origin;
+        for char_y in 0usize..grid.height() {
+            for char_x in 0usize..grid.width() {
+                let char = grid.get(char_x, char_y);
+                trace!("drawing {char}");
 
-            if let Err(e) = font.render(
-                char,
-                display,
-                Origin::new(tile_x * TILE_SIZE, tile_y * TILE_SIZE),
-            ) {
-                error!("stopping drawing text because char draw failed: {e}");
-                return;
+                let tile_x = char_x + x;
+                let tile_y = char_y + y;
+
+                if let Err(e) = self.font_renderer.render(
+                    char,
+                    &mut display,
+                    Origin::new(tile_x * TILE_SIZE, tile_y * TILE_SIZE),
+                ) {
+                    error!(
+                        "stopping drawing text because char draw failed: {e}"
+                    );
+                    return Failure;
+                }
             }
         }
+
+        Success
     }
-}
 
-fn print_pixel_grid(
-    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()
-    );
-    for inner_y in 0..pixels.height() {
-        for inner_x in 0..pixels.width() {
-            let is_set = pixels.get(inner_x, inner_y);
-            let x = offset_x + inner_x;
-            let y = offset_y + inner_y;
+    fn print_pixel_grid(
+        &self,
+        offset_x: usize,
+        offset_y: usize,
+        pixels: &Bitmap,
+    ) -> ExecutionResult {
+        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);
+                let x = offset_x + inner_x;
+                let y = offset_y + inner_y;
 
-            if x >= display.width() || y >= display.height() {
-                error!("stopping pixel grid draw because coordinate {x} {y} is out of bounds");
-                return false;
+                if x >= display.width() || y >= display.height() {
+                    error!("stopping pixel grid draw because coordinate {x} {y} is out of bounds");
+                    return Failure;
+                }
+
+                display.set(x, y, is_set);
             }
-
-            display.set(x, y, is_set);
         }
+
+        Success
     }
 
-    true
-}
-
-fn get_coordinates_for_index(offset: usize, index: usize) -> (usize, usize) {
-    let pixel_index = offset + index;
-    (pixel_index % PIXEL_WIDTH, pixel_index / PIXEL_WIDTH)
+    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::error::GlyphLoadingError;
-use font_kit::family_name::FamilyName;
-use font_kit::font::Font;
-use font_kit::hinting::HintingOptions;
-use font_kit::properties::Properties;
-use font_kit::source::SystemSource;
-use pathfinder_geometry::transform2d::Transform2F;
-use pathfinder_geometry::vector::{vec2f, vec2i};
+use font_kit::{
+    canvas::{Canvas, Format, RasterizationOptions},
+    error::GlyphLoadingError,
+    family_name::FamilyName,
+    font::Font,
+    hinting::HintingOptions,
+    properties::Properties,
+    source::SystemSource,
+};
+use pathfinder_geometry::{
+    transform2d::Transform2F,
+    vector::{vec2f, vec2i},
+};
 use servicepoint::{Bitmap, Grid, Origin, Pixels, TILE_SIZE};
-use std::sync::Mutex;
+use std::sync::{Mutex, MutexGuard};
+
+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>,
 }
@@ -28,35 +43,44 @@ pub enum RenderError {
 }
 
 impl FontRenderer8x8 {
-    pub fn new(font: Font, fallback_char: Option<char>) -> Self {
+    const FALLBACK_CHAR: char = '?';
+    pub fn new(font: Font) -> Self {
         let canvas =
             Canvas::new(vec2i(TILE_SIZE as i32, TILE_SIZE as i32), Format::A8);
         assert_eq!(canvas.pixels.len(), TILE_SIZE * TILE_SIZE);
         assert_eq!(canvas.stride, TILE_SIZE);
-        let fallback_char = fallback_char.and_then(|c| font.glyph_for_char(c));
+        let fallback_char = font.glyph_for_char(Self::FALLBACK_CHAR);
         let result = Self {
-            font,
+            font: SendFont(font),
             fallback_char,
             canvas: Mutex::new(canvas),
         };
         result
     }
 
+    pub fn from_name(family_name: String) -> Self {
+        let font = SystemSource::new()
+            .select_best_match(
+                &[FamilyName::Title(family_name)],
+                &Properties::new(),
+            )
+            .unwrap()
+            .load()
+            .unwrap();
+        Self::new(font)
+    }
+
     pub fn render(
         &self,
         char: char,
         bitmap: &mut Bitmap,
         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 = match glyph_id {
-            None => return Err(GlyphNotFound(char)),
-            Some(val) => val,
-        };
+        let glyph_id = self.get_glyph(char)?;
 
+        let mut canvas = self.canvas.lock().unwrap();
         canvas.pixels.fill(0);
-        self.font.rasterize_glyph(
+        self.font.as_ref().rasterize_glyph(
             &mut canvas,
             glyph_id,
             TILE_SIZE as f32,
@@ -66,10 +90,17 @@ impl FontRenderer8x8 {
             RasterizationOptions::Bilevel,
         )?;
 
+        Self::copy_to_bitmap(canvas, bitmap, offset)
+    }
+
+    fn copy_to_bitmap(
+        canvas: MutexGuard<Canvas>,
+        bitmap: &mut Bitmap,
+        offset: Origin<Pixels>,
+    ) -> Result<(), RenderError> {
         for y in 0..TILE_SIZE {
             for x in 0..TILE_SIZE {
-                let index = x + y * TILE_SIZE;
-                let canvas_val = canvas.pixels[index] != 0;
+                let canvas_val = canvas.pixels[x + y * TILE_SIZE] != 0;
                 let bitmap_x = (offset.x + x) as isize;
                 let bitmap_y = (offset.y + y) as isize;
                 if !bitmap.set_optional(bitmap_x, bitmap_y, canvas_val) {
@@ -77,9 +108,16 @@ impl FontRenderer8x8 {
                 }
             }
         }
-
         Ok(())
     }
+
+    fn get_glyph(&self, char: char) -> Result<u32, RenderError> {
+        self.font
+            .as_ref()
+            .glyph_for_char(char)
+            .or(self.fallback_char)
+            .ok_or_else(|| GlyphNotFound(char))
+    }
 }
 
 impl Default for FontRenderer8x8 {
@@ -89,6 +127,6 @@ impl Default for FontRenderer8x8 {
             .unwrap()
             .load()
             .unwrap();
-        FontRenderer8x8::new(utf8_font, Some('?'))
+        FontRenderer8x8::new(utf8_font)
     }
 }

src/gui.rs

@@ -1,12 +1,10 @@
+use std::slice::ChunksExactMut;
 use std::sync::mpsc::Sender;
 use std::sync::RwLock;
 
 use log::{info, warn};
 use pixels::{Pixels, SurfaceTexture};
-use servicepoint::{
-    Bitmap, Brightness, BrightnessGrid, Grid, PIXEL_HEIGHT, PIXEL_WIDTH,
-    TILE_SIZE,
-};
+use servicepoint::*;
 use winit::application::ApplicationHandler;
 use winit::dpi::LogicalSize;
 use winit::event::WindowEvent;
@@ -14,18 +12,23 @@ use winit::event_loop::ActiveEventLoop;
 use winit::keyboard::KeyCode::KeyC;
 use winit::window::{Window, WindowId};
 
-use crate::Cli;
+use crate::cli::GuiOptions;
 
-pub struct App<'t> {
+pub struct Gui<'t> {
     display: &'t RwLock<Bitmap>,
     luma: &'t RwLock<BrightnessGrid>,
     window: Option<Window>,
     stop_udp_tx: Sender<()>,
-    cli: &'t Cli,
+    options: GuiOptions,
     logical_size: LogicalSize<u16>,
 }
 
 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 {
@@ -33,91 +36,99 @@ 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,
+        options: GuiOptions,
     ) -> Self {
-        let logical_size = {
-            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 {
+        Gui {
+            window: None,
+            logical_size: Self::get_logical_size(options.spacers),
             display,
             luma,
             stop_udp_tx,
-            window: None,
-            cli,
-            logical_size,
+            options,
         }
     }
 
     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(
-                window_size.width,
-                window_size.height,
-                &window,
-            );
-            Pixels::new(
-                self.logical_size.width as u32,
-                self.logical_size.height as u32,
-                surface_texture,
-            )
-            .unwrap()
-        };
+        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,
+            surface_texture,
+        )
+        .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 {
+        let brightness_scale =
+            (u8::MAX as f32) / (u8::from(Brightness::MAX) as f32);
+
+        for tile_y in 0..TILE_HEIGHT {
+            if self.options.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 is_set = display.get(x, y);
-                let brightness: u8 =
-                    luma.get(x / TILE_SIZE, y / TILE_SIZE).into();
-                let max_brightness: u8 = Brightness::MAX.into();
-                let scale: f32 = (u8::MAX as f32) / (max_brightness as f32);
-
-                let brightness = (scale * brightness as f32) as u8;
-
-                let color = if is_set {
-                    [
-                        if self.cli.red { brightness } else { 0u8 },
-                        if self.cli.green { brightness } else { 0u8 },
-                        if self.cli.blue { brightness } else { 0u8 },
-                        255,
-                    ]
-                } else {
-                    [0u8, 0, 0, 255]
-                };
-
-                let pixel = frame.next().unwrap();
-                pixel.copy_from_slice(&color);
+            let start_y = tile_y * TILE_SIZE;
+            for y in start_y..start_y + TILE_SIZE {
+                for tile_x in 0..TILE_WIDTH {
+                    let brightness = u8::from(luma.get(tile_x, tile_y));
+                    let brightness =
+                        (brightness_scale * brightness as f32) as u8;
+                    let on_color = self.get_on_color(brightness);
+                    let start_x = tile_x * TILE_SIZE;
+                    for x in start_x..start_x + TILE_SIZE {
+                        let color = if display.get(x, y) {
+                            on_color
+                        } else {
+                            OFF_COLOR
+                        };
+                        let pixel = frame.next().unwrap();
+                        pixel.copy_from_slice(&color);
+                    }
+                }
             }
         }
+    }
 
-        pixels.render().expect("could not render");
+    fn get_on_color(&self, brightness: u8) -> [u8; 4] {
+        [
+            if self.options.red { brightness } else { 0u8 },
+            if self.options.green { brightness } else { 0u8 },
+            if self.options.blue { brightness } else { 0u8 },
+            255,
+        ]
+    }
+
+    fn get_logical_size(spacers: bool) -> LogicalSize<u16> {
+        let height = if spacers {
+            PIXEL_HEIGHT_WITH_SPACERS
+        } else {
+            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,75 +1,47 @@
 #![deny(clippy::all)]
 
+use crate::font_renderer::FontRenderer8x8;
+use crate::{
+    execute_command::{CommandExecutor, ExecutionResult},
+    gui::{AppEvents, Gui},
+};
+use clap::Parser;
+use cli::Cli;
+use log::{error, info, warn, LevelFilter};
+use servicepoint::*;
 use std::io::ErrorKind;
 use std::net::UdpSocket;
 use std::sync::{mpsc, RwLock};
 use std::time::Duration;
+use winit::event_loop::{ControlFlow, EventLoop, EventLoopProxy};
 
-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 cli;
+mod cp437_font;
 mod execute_command;
-mod font;
 mod font_renderer;
 mod gui;
 
-#[derive(Parser, Debug)]
-struct Cli {
-    #[arg(long, default_value = "0.0.0.0:2342")]
-    bind: String,
-    #[arg(short, long, default_value_t = false)]
-    spacers: bool,
-    #[arg(short, long, default_value_t = false)]
-    red: bool,
-    #[arg(short, long, default_value_t = false)]
-    green: bool,
-    #[arg(short, long, default_value_t = false)]
-    blue: bool,
-}
+const BUF_SIZE: usize = 8985;
 
 fn main() {
-    env_logger::builder()
-        .filter_level(LevelFilter::Info)
-        .parse_default_env()
-        .init();
-
     let mut cli = Cli::parse();
-    if !(cli.red || cli.blue || cli.green) {
-        cli.green = true;
+    if !(cli.gui.red || cli.gui.blue || cli.gui.green) {
+        cli.gui.green = true;
     }
 
+    init_logging(cli.verbose);
     info!("starting with args: {:?}", &cli);
+
     let socket = UdpSocket::bind(&cli.bind).expect("could not bind socket");
     socket
         .set_nonblocking(true)
         .expect("could not enter non blocking mode");
 
     let display = RwLock::new(Bitmap::new(PIXEL_WIDTH, PIXEL_HEIGHT));
+    let luma = RwLock::new(BrightnessGrid::new(TILE_WIDTH, TILE_HEIGHT));
 
-    let mut luma = BrightnessGrid::new(TILE_WIDTH, TILE_HEIGHT);
-    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_ref, luma_ref, stop_udp_tx, cli);
+    let mut gui = Gui::new(&display, &luma, stop_udp_tx, cli.gui);
 
     let event_loop = EventLoop::with_user_event()
         .build()
@@ -77,63 +49,94 @@ fn run(
     event_loop.set_control_flow(ControlFlow::Wait);
 
     let event_proxy = event_loop.create_proxy();
+    let font_renderer = cli
+        .font
+        .map(move |font| FontRenderer8x8::from_name(font))
+        .unwrap_or_else(move || FontRenderer8x8::default());
+    let command_executor = CommandExecutor::new(&display, &luma, font_renderer);
 
     std::thread::scope(move |scope| {
-        let udp_thread = scope.spawn(move || {
-            let mut buf = [0; 8985];
-            let utf8_font = FontRenderer8x8::default();
-
+        scope.spawn(move || {
+            let mut buf = [0; BUF_SIZE];
             while stop_udp_rx.try_recv().is_err() {
-                let (amount, _) = match socket.recv_from(&mut buf) {
-                    Err(err) if err.kind() == ErrorKind::WouldBlock => {
-                        std::thread::sleep(Duration::from_millis(1));
-                        continue;
-                    }
-                    Ok(result) => result,
-                    other => other.unwrap(),
-                };
-
-                if amount == buf.len() {
-                    warn!(
-                        "the received package may have been truncated to a length of {}",
-                        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) {
-                    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)
-                        .expect("could not send close event");
-                    break;
-                }
-
-                event_proxy
-                    .send_event(AppEvents::UdpPacketHandled)
-                    .expect("could not send packet handled event");
+                receive_into_buf(&socket, &mut buf)
+                    .and_then(move |amount| command_from_slice(&buf[..amount]))
+                    .map(|cmd| {
+                        handle_command(&event_proxy, &command_executor, cmd)
+                    });
             }
         });
-
         event_loop
-            .run_app(&mut app)
+            .run_app(&mut gui)
             .expect("could not run event loop");
-
-        udp_thread.join().expect("could not join udp thread");
     });
 }
+
+fn handle_command(
+    event_proxy: &EventLoopProxy<AppEvents>,
+    command_executor: &CommandExecutor,
+    command: Command,
+) {
+    match command_executor.execute(command) {
+        ExecutionResult::Success => {
+            event_proxy
+                .send_event(AppEvents::UdpPacketHandled)
+                .expect("could not send packet handled event");
+        }
+        ExecutionResult::Failure => {
+            error!("failed to execute command");
+        }
+        ExecutionResult::Shutdown => {
+            event_proxy
+                .send_event(AppEvents::UdpThreadClosed)
+                .expect("could not send close event");
+        }
+    }
+}
+
+fn init_logging(debug: bool) {
+    let filter = if debug {
+        LevelFilter::Debug
+    } else {
+        LevelFilter::Info
+    };
+    env_logger::builder()
+        .filter_level(filter)
+        .parse_default_env()
+        .init();
+}
+
+fn command_from_slice(slice: &[u8]) -> Option<Command> {
+    let packet = servicepoint::Packet::try_from(slice)
+        .inspect_err(|_| {
+            warn!("could not load packet with length {}", slice.len())
+        })
+        .ok()?;
+    Command::try_from(packet)
+        .inspect_err(move |err| {
+            warn!("could not read command for packet: {:?}", err)
+        })
+        .ok()
+}
+
+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)
+}