Merge pull request #3 from kaesaecracker/fix-pixels

Replace pixels with softbuffer

Cargo.toml

@@ -10,19 +10,17 @@ license = "GPL-3.0-or-later"
 log = "0.4"
 env_logger = "0.11"
 clap = { version = "4.5", features = ["derive"] }
-# for drawing pixels onto the surface of the window
-pixels = "0.15"
-# I should not need this as a direct dependency, but then I cannot spell the types needed to use font-kit...
-pathfinder_geometry = "0.5.1"
-font-kit = "0.14.2"
 thiserror = "2.0"
 
-[dependencies.servicepoint]
-version = "0.13.0"
-features = ["all_compressions"]
+# package parsing
+servicepoint = { version = "0.13.0", features = ["all_compressions"] }
 
-[dependencies.winit]
-version = "0.30"
-features = ["rwh_05"]
-default-features = true
+# font rendering
+font-kit = "0.14.2"
+# I should not need this as a direct dependency, but then I cannot spell the types needed to use font-kit...
+pathfinder_geometry = "0.5.1"
 
+# for opening a window
+winit = "0.30.8"
+# for drawing pixels onto the surface of the window
+softbuffer = "0.4.6"

src/command_executor.rs

@@ -1,5 +1,5 @@
+use crate::command_executor::ExecutionResult::{Failure, Shutdown, Success};
 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::{
@@ -9,6 +9,7 @@ use servicepoint::{
 use std::ops::{BitAnd, BitOr, BitXor};
 use std::sync::RwLock;
 
+#[derive(Debug)]
 pub struct CommandExecutor<'t> {
     display: &'t RwLock<Bitmap>,
     luma: &'t RwLock<BrightnessGrid>,

src/cp437_font.rs

@@ -3,6 +3,7 @@ use std::ops::Index;
 
 const CHAR_COUNT: usize = u8::MAX as usize + 1;
 
+#[derive(Debug)]
 pub struct Cp437Font {
     bitmaps: [Bitmap; CHAR_COUNT],
 }

src/font_renderer.rs

@@ -15,6 +15,7 @@ use pathfinder_geometry::{
 use servicepoint::{Bitmap, Grid, Origin, Pixels, TILE_SIZE};
 use std::sync::{Mutex, MutexGuard};
 
+#[derive(Debug)]
 struct SendFont(Font);
 
 // struct is only using primitives and pointers - lets try if it is only missing the declaration
@@ -26,6 +27,7 @@ impl AsRef<Font> for SendFont {
     }
 }
 
+#[derive(Debug)]
 pub struct FontRenderer8x8 {
     font: SendFont,
     canvas: Mutex<Canvas>,
@@ -50,12 +52,11 @@ impl FontRenderer8x8 {
         assert_eq!(canvas.pixels.len(), TILE_SIZE * TILE_SIZE);
         assert_eq!(canvas.stride, TILE_SIZE);
         let fallback_char = font.glyph_for_char(Self::FALLBACK_CHAR);
-        let result = Self {
+        Self {
             font: SendFont(font),
             fallback_char,
             canvas: Mutex::new(canvas),
-        };
-        result
+        }
     }
 
     pub fn from_name(family_name: String) -> Self {
@@ -116,7 +117,7 @@ impl FontRenderer8x8 {
             .as_ref()
             .glyph_for_char(char)
             .or(self.fallback_char)
-            .ok_or_else(|| GlyphNotFound(char))
+            .ok_or(GlyphNotFound(char))
     }
 }
 

src/gui.rs

@@ -1,26 +1,22 @@
-use std::slice::ChunksExactMut;
-use std::sync::mpsc::Sender;
-use std::sync::RwLock;
+use std::{sync::mpsc::Sender, sync::RwLock};
 
 use log::{info, warn};
-use pixels::{Pixels, SurfaceTexture};
 use servicepoint::*;
-use winit::application::ApplicationHandler;
-use winit::dpi::LogicalSize;
-use winit::event::WindowEvent;
-use winit::event_loop::ActiveEventLoop;
-use winit::keyboard::KeyCode::KeyC;
-use winit::window::{Window, WindowId};
+use winit::{
+    application::ApplicationHandler, dpi::LogicalSize, event::WindowEvent,
+    event_loop::ActiveEventLoop, keyboard::KeyCode::KeyC, window::WindowId,
+};
 
 use crate::cli::GuiOptions;
+use crate::gui_window::GuiWindow;
 
 pub struct Gui<'t> {
     display: &'t RwLock<Bitmap>,
     luma: &'t RwLock<BrightnessGrid>,
-    window: Option<Window>,
     stop_udp_tx: Sender<()>,
     options: GuiOptions,
     logical_size: LogicalSize<u16>,
+    window: Option<GuiWindow>,
 }
 
 const SPACER_HEIGHT: usize = 4;
@@ -28,7 +24,7 @@ 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];
+const OFF_COLOR: u32 = u32::from_ne_bytes([0u8, 0, 0, 0]);
 
 #[derive(Debug)]
 pub enum AppEvents {
@@ -43,7 +39,7 @@ impl<'t> Gui<'t> {
         stop_udp_tx: Sender<()>,
         options: GuiOptions,
     ) -> Self {
-        Gui {
+        Self {
             window: None,
             logical_size: Self::get_logical_size(options.spacers),
             display,
@@ -54,31 +50,14 @@ impl<'t> Gui<'t> {
     }
 
     fn draw(&mut self) {
-        let window = self.window.as_ref().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();
         let brightness_scale =
             (u8::MAX as f32) / (u8::from(Brightness::MAX) as f32);
 
+        let mut buffer = self.window.as_mut().unwrap().get_buffer();
+        let mut frame = buffer.iter_mut();
+
         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
@@ -93,7 +72,8 @@ impl<'t> Gui<'t> {
                     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 on_color =
+                        Self::get_on_color(&self.options, brightness);
                     let start_x = tile_x * TILE_SIZE;
                     for x in start_x..start_x + TILE_SIZE {
                         let color = if display.get(x, y) {
@@ -101,21 +81,22 @@ impl<'t> Gui<'t> {
                         } else {
                             OFF_COLOR
                         };
-                        let pixel = frame.next().unwrap();
-                        pixel.copy_from_slice(&color);
+                        *frame.next().unwrap() = color;
                     }
                 }
             }
         }
+
+        buffer.present().unwrap();
     }
 
-    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_on_color(options: &GuiOptions, brightness: u8) -> u32 {
+        u32::from_ne_bytes([
+            if options.blue { brightness } else { 0u8 },
+            if options.green { brightness } else { 0u8 },
+            if options.red { brightness } else { 0u8 },
+            0,
+        ])
     }
 
     fn get_logical_size(spacers: bool) -> LogicalSize<u16> {
@@ -128,15 +109,9 @@ impl<'t> Gui<'t> {
     }
 }
 
-impl ApplicationHandler<AppEvents> for Gui<'_> {
+impl<'t> ApplicationHandler<AppEvents> for Gui<'t> {
     fn resumed(&mut self, event_loop: &ActiveEventLoop) {
-        let attributes = Window::default_attributes()
-            .with_title("servicepoint-simulator")
-            .with_inner_size(self.logical_size)
-            .with_transparent(false);
-
-        let window = event_loop.create_window(attributes).unwrap();
-        self.window = Some(window);
+        self.window = Some(GuiWindow::new(event_loop, self.logical_size));
     }
 
     fn user_event(&mut self, event_loop: &ActiveEventLoop, event: AppEvents) {
@@ -162,7 +137,6 @@ impl ApplicationHandler<AppEvents> for Gui<'_> {
         match event {
             WindowEvent::CloseRequested => {
                 warn!("window event close requested");
-                self.window = None;
                 let _ = self.stop_udp_tx.send(()); // try to stop udp thread
                 event_loop.exit();
             }
@@ -179,4 +153,8 @@ impl ApplicationHandler<AppEvents> for Gui<'_> {
             _ => {}
         }
     }
+
+    fn suspended(&mut self, _: &ActiveEventLoop) {
+        self.window = None;
+    }
 }

src/gui_window.rs

@@ -0,0 +1,46 @@
+use softbuffer::Buffer;
+use std::{num::NonZero, rc::Rc};
+use winit::{dpi::LogicalSize, event_loop::ActiveEventLoop, window::Window};
+
+type Context = softbuffer::Context<Rc<Window>>;
+type Surface = softbuffer::Surface<Rc<Window>, Rc<Window>>;
+
+pub struct GuiWindow {
+    winit_window: Rc<Window>,
+    surface: Surface,
+}
+
+impl GuiWindow {
+    pub fn new(
+        event_loop: &ActiveEventLoop,
+        logical_size: LogicalSize<u16>,
+    ) -> GuiWindow {
+        let attributes = Window::default_attributes()
+            .with_title("servicepoint-simulator")
+            .with_min_inner_size(logical_size)
+            .with_inner_size(logical_size)
+            .with_transparent(false);
+        let winit_window =
+            Rc::new(event_loop.create_window(attributes).unwrap());
+        let context = Context::new(winit_window.clone()).unwrap();
+        let mut surface = Surface::new(&context, winit_window.clone()).unwrap();
+        surface
+            .resize(
+                NonZero::new(logical_size.width as u32).unwrap(),
+                NonZero::new(logical_size.height as u32).unwrap(),
+            )
+            .unwrap();
+
+        Self {
+            winit_window,
+            surface,
+        }
+    }
+
+    pub fn get_buffer(&mut self) -> Buffer<Rc<Window>, Rc<Window>> {
+        self.surface.buffer_mut().unwrap()
+    }
+    pub(crate) fn request_redraw(&self) {
+        self.winit_window.request_redraw();
+    }
+}

src/main.rs

@@ -1,27 +1,22 @@
 #![deny(clippy::all)]
 
 use crate::font_renderer::FontRenderer8x8;
-use crate::{
-    execute_command::{CommandExecutor, ExecutionResult},
-    gui::{AppEvents, Gui},
-};
+use crate::udp_server::UdpServer;
+use crate::{command_executor::CommandExecutor, gui::Gui};
 use clap::Parser;
 use cli::Cli;
-use log::{error, info, warn, LevelFilter};
+use log::{info, 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 winit::event_loop::{ControlFlow, EventLoop};
 
 mod cli;
+mod command_executor;
 mod cp437_font;
-mod execute_command;
 mod font_renderer;
 mod gui;
-
-const BUF_SIZE: usize = 8985;
+mod gui_window;
+mod udp_server;
 
 fn main() {
     let mut cli = Cli::parse();
@@ -32,68 +27,35 @@ fn main() {
     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 (stop_udp_tx, stop_udp_rx) = mpsc::channel();
-    let mut gui = Gui::new(&display, &luma, stop_udp_tx, cli.gui);
-
     let event_loop = EventLoop::with_user_event()
         .build()
         .expect("could not create event loop");
     event_loop.set_control_flow(ControlFlow::Wait);
 
-    let event_proxy = event_loop.create_proxy();
+    let display = RwLock::new(Bitmap::new(PIXEL_WIDTH, PIXEL_HEIGHT));
+    let luma = RwLock::new(BrightnessGrid::new(TILE_WIDTH, TILE_HEIGHT));
+    let (stop_udp_tx, stop_udp_rx) = mpsc::channel();
     let font_renderer = cli
         .font
-        .map(move |font| FontRenderer8x8::from_name(font))
-        .unwrap_or_else(move || FontRenderer8x8::default());
+        .map(FontRenderer8x8::from_name)
+        .unwrap_or_else(FontRenderer8x8::default);
     let command_executor = CommandExecutor::new(&display, &luma, font_renderer);
+    let mut udp_server = UdpServer::new(
+        cli.bind,
+        stop_udp_rx,
+        command_executor,
+        event_loop.create_proxy(),
+    );
+    let mut gui = Gui::new(&display, &luma, stop_udp_tx, cli.gui);
 
     std::thread::scope(move |scope| {
-        scope.spawn(move || {
-            let mut buf = [0; BUF_SIZE];
-            while stop_udp_rx.try_recv().is_err() {
-                receive_into_buf(&socket, &mut buf)
-                    .and_then(move |amount| command_from_slice(&buf[..amount]))
-                    .map(|cmd| {
-                        handle_command(&event_proxy, &command_executor, cmd)
-                    });
-            }
-        });
+        scope.spawn(move || udp_server.run());
         event_loop
             .run_app(&mut gui)
             .expect("could not run event loop");
     });
 }
 
-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
@@ -105,38 +67,3 @@ fn init_logging(debug: bool) {
         .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)
-}

src/udp_server.rs

@@ -0,0 +1,99 @@
+use crate::command_executor::{CommandExecutor, ExecutionResult};
+use crate::gui::AppEvents;
+use log::{error, warn};
+use servicepoint::Command;
+use std::io::ErrorKind;
+use std::net::UdpSocket;
+use std::sync::mpsc::Receiver;
+use std::time::Duration;
+use winit::event_loop::EventLoopProxy;
+
+const BUF_SIZE: usize = 8985;
+
+#[derive(Debug)]
+pub struct UdpServer<'t> {
+    socket: UdpSocket,
+    stop_rx: Receiver<()>,
+    command_executor: CommandExecutor<'t>,
+    app_events: EventLoopProxy<AppEvents>,
+    buf: [u8; BUF_SIZE],
+}
+
+impl<'t> UdpServer<'t> {
+    pub fn new(
+        bind: String,
+        stop_rx: Receiver<()>,
+        command_executor: CommandExecutor<'t>,
+        app_events: EventLoopProxy<AppEvents>,
+    ) -> Self {
+        let socket = UdpSocket::bind(bind).expect("could not bind socket");
+        socket
+            .set_nonblocking(true)
+            .expect("could not enter non blocking mode");
+
+        Self {
+            socket,
+            stop_rx,
+            command_executor,
+            app_events,
+            buf: [0; BUF_SIZE],
+        }
+    }
+
+    pub(crate) fn run(&mut self) {
+        while self.stop_rx.try_recv().is_err() {
+            if let Some(cmd) = self.receive_into_buf().and_then(|amount| {
+                Self::command_from_slice(&self.buf[..amount])
+            }) {
+                match self.command_executor.execute(cmd) {
+                    ExecutionResult::Success => {
+                        self.app_events
+                            .send_event(AppEvents::UdpPacketHandled)
+                            .expect("could not send packet handled event");
+                    }
+                    ExecutionResult::Failure => {
+                        error!("failed to execute command");
+                    }
+                    ExecutionResult::Shutdown => {
+                        self.app_events
+                            .send_event(AppEvents::UdpThreadClosed)
+                            .expect("could not send close event");
+                        break;
+                    }
+                }
+            }
+        }
+    }
+
+    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(&mut self) -> Option<usize> {
+        let (amount, _) = match self.socket.recv_from(&mut self.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 == self.buf.len() {
+            warn!(
+                "the received package may have been truncated to a length of {}",
+                amount
+            );
+        }
+        Some(amount)
+    }
+}