only redraw when needed, send proper close event to gui

Cargo.lock

@@ -2151,7 +2151,7 @@ dependencies = [
 [[package]]
 name = "servicepoint2"
 version = "0.1.0"
-source = "git+https://github.com/kaesaecracker/servicepoint.git#27f891cd921821400b4ae69e02986bf5ed2687b8"
+source = "git+https://github.com/kaesaecracker/servicepoint.git#35ae1f20ce52e36c6fbb94c74d183dda0cc1a0af"
 dependencies = [
  "log",
  "num",

src/gui.rs

@@ -1,8 +1,8 @@
-use log::warn;
+use log::{info, warn};
 use pixels::wgpu::TextureFormat;
 use pixels::{Pixels, PixelsBuilder, SurfaceTexture};
 use servicepoint2::{ByteGrid, PixelGrid, PIXEL_HEIGHT, PIXEL_WIDTH, TILE_SIZE};
-use std::sync::mpsc::{Receiver, Sender};
+use std::sync::mpsc::Sender;
 use std::sync::RwLock;
 use winit::application::ApplicationHandler;
 use winit::dpi::{LogicalSize, Size};
@@ -15,21 +15,24 @@ pub struct App<'t> {
     luma: &'t RwLock<ByteGrid>,
     window: Option<Window>,
     pixels: Option<Pixels>,
-    stop_ui_rx: Receiver<()>,
     stop_udp_tx: Sender<()>,
 }
 
+#[derive(Debug)]
+pub enum AppEvents {
+    UdpPacketHandled,
+    UdpThreadClosed,
+}
+
 impl<'t> App<'t> {
     pub fn new(
         display: &'t RwLock<PixelGrid>,
         luma: &'t RwLock<ByteGrid>,
-        stop_ui_rx: Receiver<()>,
         stop_udp_tx: Sender<()>,
     ) -> Self {
         App {
             display,
             luma,
-            stop_ui_rx,
             stop_udp_tx,
             pixels: None,
             window: None,
@@ -37,7 +40,7 @@ impl<'t> App<'t> {
     }
 }
 
-impl ApplicationHandler for App<'_> {
+impl ApplicationHandler<AppEvents> for App<'_> {
     fn resumed(&mut self, event_loop: &ActiveEventLoop) {
         let size = Size::from(LogicalSize::new(PIXEL_WIDTH as f64, PIXEL_HEIGHT as f64));
         let attributes = Window::default_attributes()
@@ -61,22 +64,32 @@ impl ApplicationHandler for App<'_> {
         };
     }
 
+    fn user_event(&mut self, event_loop: &ActiveEventLoop, event: AppEvents) {
+        match event {
+            AppEvents::UdpPacketHandled => {
+                if let Some(window) = &self.window {
+                    window.request_redraw();
+                }
+            }
+            AppEvents::UdpThreadClosed => {
+                info!("stopping ui thread after udp thread stopped");
+                event_loop.exit();
+            }
+        }
+    }
+
     fn window_event(&mut self, event_loop: &ActiveEventLoop, _: WindowId, event: WindowEvent) {
-        if self.stop_ui_rx.try_recv().is_ok() {
+        if event == WindowEvent::CloseRequested {
-            warn!("ui thread stop requested");
+            warn!("window event cloe requested");
+            self.window = None;
+            let _ = self.stop_udp_tx.send(()); // try to stop udp thread
             event_loop.exit();
         }
 
-        match event {
+        if event != WindowEvent::RedrawRequested {
-            WindowEvent::CloseRequested => {
+            return;
-                warn!("The close button was pressed; stopping");
-                self.stop_udp_tx
-                    .send(())
-                    .expect("could not stop udp thread");
-                event_loop.exit();
         }
-            WindowEvent::RedrawRequested => {
+
-                let window = self.window.as_ref().unwrap();
         let pixels = self.pixels.as_mut().unwrap();
 
         let mut frame = pixels.frame_mut().chunks_exact_mut(4);
@@ -101,9 +114,5 @@ impl ApplicationHandler for App<'_> {
         }
 
         pixels.render().expect("could not render");
-                window.request_redraw();
-            }
-            _ => (),
-        }
     }
 }

src/main.rs

@@ -4,12 +4,12 @@ mod font;
 mod gui;
 
 use crate::font::BitmapFont;
-use crate::gui::App;
+use crate::gui::{App, AppEvents};
 use clap::Parser;
 use log::{debug, error, info, warn};
 use servicepoint2::{
-    ByteGrid, Command, Origin, Packet, PixelGrid, PIXEL_HEIGHT, PIXEL_WIDTH, TILE_HEIGHT,
+    ByteGrid, Command, Origin, Packet, PixelGrid, PIXEL_COUNT, PIXEL_HEIGHT, PIXEL_WIDTH,
-    TILE_SIZE, TILE_WIDTH,
+    TILE_HEIGHT, TILE_SIZE, TILE_WIDTH,
 };
 use std::io::ErrorKind;
 use std::net::UdpSocket;
@@ -45,12 +45,15 @@ fn main() {
     let luma_ref = &luma;
 
     let (stop_udp_tx, stop_udp_rx) = mpsc::channel();
-    let (stop_ui_tx, stop_ui_rx) = mpsc::channel();
 
-    let mut app = App::new(display_ref, luma_ref, stop_ui_rx, stop_udp_tx);
+    let mut app = App::new(display_ref, luma_ref, stop_udp_tx);
 
-    let event_loop = EventLoop::new().expect("could not create event loop");
+    let event_loop = EventLoop::with_user_event()
-    event_loop.set_control_flow(ControlFlow::Poll);
+        .build()
+        .expect("could not create event loop");
+    event_loop.set_control_flow(ControlFlow::Wait);
+
+    let event_proxy = event_loop.create_proxy();
 
     std::thread::scope(move |scope| {
         let udp_thread = scope.spawn(move || {
@@ -77,11 +80,18 @@ fn main() {
                 let package = servicepoint2::Packet::from(vec);
 
                 if !handle_package(package, &font, display_ref, luma_ref) {
-                    break; // hard reset
+                    // hard reset
-                }
+                    event_proxy
+                        .send_event(AppEvents::UdpThreadClosed)
+                        .expect("could not send close event");
+                    break;
                 }
 
-            stop_ui_tx.send(()).expect("could not stop ui thread");
+                event_proxy
+                    .send_event(AppEvents::UdpPacketHandled)
+                    .expect("could not send packet handled event");
+                std::thread::sleep(Duration::from_millis(1)); // give ui a change to get lock
+            }
         });
 
         event_loop
@@ -130,40 +140,44 @@ fn handle_package(
         }
         // TODO: how to deduplicate this code in a rusty way?
         Command::BitmapLinear(offset, vec) => {
+            if !check_bitmap_valid(offset, vec.len()) {
+                return true;
+            }
             let mut display = display_ref.write().unwrap();
             for bitmap_index in 0..vec.len() {
-                let pixel_index = offset as usize + bitmap_index;
+                let (x, y) = get_coordinates_for_index(offset as usize, bitmap_index);
-                let y = pixel_index / PIXEL_WIDTH as usize;
-                let x = pixel_index % PIXEL_WIDTH as usize;
                 display.set(x, y, vec.get(bitmap_index));
             }
         }
         Command::BitmapLinearAnd(offset, vec) => {
+            if !check_bitmap_valid(offset, vec.len()) {
+                return true;
+            }
             let mut display = display_ref.write().unwrap();
             for bitmap_index in 0..vec.len() {
-                let pixel_index = offset as usize + bitmap_index;
+                let (x, y) = get_coordinates_for_index(offset as usize, bitmap_index);
-                let y = pixel_index / PIXEL_WIDTH as usize;
-                let x = pixel_index % PIXEL_WIDTH as usize;
                 let old_value = display.get(x, y);
                 display.set(x, y, old_value && vec.get(bitmap_index));
             }
         }
         Command::BitmapLinearOr(offset, vec) => {
+            if !check_bitmap_valid(offset, vec.len()) {
+                return true;
+            }
             let mut display = display_ref.write().unwrap();
             for bitmap_index in 0..vec.len() {
-                let pixel_index = offset as usize + bitmap_index;
+                let (x, y) = get_coordinates_for_index(offset as usize, bitmap_index);
-                let y = pixel_index / PIXEL_WIDTH as usize;
-                let x = pixel_index % PIXEL_WIDTH as usize;
                 let old_value = display.get(x, y);
                 display.set(x, y, old_value || vec.get(bitmap_index));
             }
         }
         Command::BitmapLinearXor(offset, vec) => {
+            if !check_bitmap_valid(offset, vec.len()) {
+                return true;
+            }
             let mut display = display_ref.write().unwrap();
             for bitmap_index in 0..vec.len() {
-                let pixel_index = offset as usize + bitmap_index;
+                let (x, y) = get_coordinates_for_index(offset as usize, bitmap_index);
-                let y = pixel_index / PIXEL_WIDTH as usize;
-                let x = pixel_index % PIXEL_WIDTH as usize;
                 let old_value = display.get(x, y);
                 display.set(x, y, old_value ^ vec.get(bitmap_index));
             }
@@ -192,6 +206,18 @@ fn handle_package(
     true
 }
 
+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 ({} bytes)",
+            payload_len
+        );
+        return false;
+    }
+
+    true
+}
+
 fn print_cp437_data(
     origin: Origin,
     grid: &ByteGrid,
@@ -234,3 +260,11 @@ fn print_pixel_grid(
         }
     }
 }
+
+fn get_coordinates_for_index(offset: usize, index: usize) -> (usize, usize) {
+    let pixel_index = offset + index;
+    (
+        pixel_index % PIXEL_WIDTH as usize,
+        pixel_index / PIXEL_WIDTH as usize,
+    )
+}