diff --git a/CHANGELOG.md b/CHANGELOG.md index db7f5bc..7a82735 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,13 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Changed + +- BREAKING: Remove the `breakwater-core` crate ([#35]) +- Add a `FrameBuffer` trait, rename the existing implementation one to `SimpleFrameBuffer` ([#35]) + +[#35]: https://github.com/sbernauer/breakwater/pull/35 + ## [0.15.0] - 2024-06-12 ### Added diff --git a/Cargo.lock b/Cargo.lock index de6d0f8..5abbb9c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -238,7 +238,6 @@ checksum = "7c12d1856e42f0d817a835fe55853957c85c8c8a470114029143d3f12671446e" name = "breakwater" version = "0.15.0" dependencies = [ - "breakwater-core", "breakwater-parser", "chrono", "clap", @@ -260,21 +259,12 @@ dependencies = [ "vncserver", ] -[[package]] -name = "breakwater-core" -version = "0.15.0" -dependencies = [ - "breakwater-parser", - "const_format", - "tokio", -] - [[package]] name = "breakwater-parser" version = "0.15.0" dependencies = [ + "const_format", "criterion", - "enum_dispatch", "memchr", "pixelbomber", ] @@ -598,18 +588,6 @@ version = "1.12.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3dca9240753cf90908d7e4aac30f630662b02aebaa1b58a3cadabdb23385b58b" -[[package]] -name = "enum_dispatch" -version = "0.3.13" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "aa18ce2bc66555b3218614519ac839ddb759a7d6720732f979ef8d13be147ecd" -dependencies = [ - "once_cell", - "proc-macro2", - "quote", - "syn", -] - [[package]] name = "env_filter" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index bc0691e..9606c3b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,5 +1,5 @@ [workspace] -members = ["breakwater-core", "breakwater-parser", "breakwater"] +members = ["breakwater-parser", "breakwater"] resolver = "2" [workspace.package] @@ -14,7 +14,6 @@ chrono = "0.4" clap = { version = "4.5", features = ["derive"] } const_format = "0.2" criterion = {version = "0.5", features = ["async_tokio"]} -enum_dispatch = "0.3" env_logger = "0.11" log = "0.4" memadvise = "0.1" diff --git a/breakwater-core/Cargo.toml b/breakwater-core/Cargo.toml deleted file mode 100644 index a62c0cc..0000000 --- a/breakwater-core/Cargo.toml +++ /dev/null @@ -1,17 +0,0 @@ -[package] -name = "breakwater-core" -description = "Core structs" -version.workspace = true -authors.workspace = true -license.workspace = true -edition.workspace = true -repository.workspace = true - -[dependencies] -breakwater-parser.workspace = true -const_format.workspace = true -tokio.workspace = true - -[features] -alpha = [] -binary-commands = [] diff --git a/breakwater-core/src/lib.rs b/breakwater-core/src/lib.rs deleted file mode 100644 index e3828e8..0000000 --- a/breakwater-core/src/lib.rs +++ /dev/null @@ -1,30 +0,0 @@ -use const_format::formatcp; - -pub mod framebuffer; -pub mod test_helpers; - -pub const HELP_TEXT: &[u8] = formatcp!("\ -Pixelflut server powered by breakwater https://github.com/sbernauer/breakwater -Available commands: -HELP: Show this help -PX x y rrggbb: Color the pixel (x,y) with the given hexadecimal color rrggbb -{} -PX x y gg: Color the pixel (x,y) with the hexadecimal color gggggg. Basically this is the same as the other commands, but is a more efficient way of filling white, black or gray areas -PX x y: Get the color value of the pixel (x,y) -{}SIZE: Get the size of the drawing surface, e.g. `SIZE 1920 1080` -OFFSET x y: Apply offset (x,y) to all further pixel draws on this connection. This can e.g. be used to pre-calculate an image/animation and simply use the OFFSET command to move it around the screen without the need to re-calculate it -", -if cfg!(feature = "alpha") { - "PX x y rrggbbaa: Color the pixel (x,y) with the given hexadecimal color rrggbb and a transparency of aa, where ff means draw normally on top of the existing pixel and 00 means fully transparent (no change at all)" -} else { - "PX x y rrggbbaa: Color the pixel (x,y) with the given hexadecimal color rrggbb. The alpha part is discarded for performance reasons, as breakwater was compiled without the alpha feature" -}, -if cfg!(feature = "binary-commands") { - "PBxxyyrgba: Binary version of the PX command. x and y are little-endian 16 bit coordinates, r, g, b and a are a byte each. There is *no* newline after the command.\n" -} else { - "" -} -).as_bytes(); - -pub const ALT_HELP_TEXT: &[u8] = b"Stop spamming HELP!\n"; -pub const CONNECTION_DENIED_TEXT: &[u8] = b"Connection denied as connection limit is reached"; diff --git a/breakwater-core/src/test_helpers/mod.rs b/breakwater-core/src/test_helpers/mod.rs deleted file mode 100644 index 78968bc..0000000 --- a/breakwater-core/src/test_helpers/mod.rs +++ /dev/null @@ -1,5 +0,0 @@ -pub use dev_null_tcp_stream::*; -pub use mock_tcp_stream::*; - -mod dev_null_tcp_stream; -mod mock_tcp_stream; diff --git a/breakwater-parser/Cargo.toml b/breakwater-parser/Cargo.toml index f571c76..339bc6c 100644 --- a/breakwater-parser/Cargo.toml +++ b/breakwater-parser/Cargo.toml @@ -12,7 +12,7 @@ name = "parsing" harness = false [dependencies] -enum_dispatch.workspace = true +const_format.workspace = true memchr.workspace = true [dev-dependencies] diff --git a/breakwater-parser/benches/parsing.rs b/breakwater-parser/benches/parsing.rs index 3324c5f..91f757f 100644 --- a/breakwater-parser/benches/parsing.rs +++ b/breakwater-parser/benches/parsing.rs @@ -1,11 +1,9 @@ use std::{sync::Arc, time::Duration}; -use breakwater_core::framebuffer::FrameBuffer; #[cfg(target_arch = "x86_64")] -use breakwater_parser::assembler::AssemblerParser; +use breakwater_parser::AssemblerParser; use breakwater_parser::{ - memchr::MemchrParser, original::OriginalParser, refactored::RefactoredParser, Parser, - ParserImplementation, + MemchrParser, OriginalParser, Parser, RefactoredParser, SimpleFrameBuffer, }; use criterion::{criterion_group, criterion_main, Criterion}; use pixelbomber::image_handler::{self, ImageConfigBuilder}; @@ -99,7 +97,10 @@ fn invoke_benchmark( let mut c_group = c.benchmark_group(bench_name); - let fb = Arc::new(FrameBuffer::new(FRAMEBUFFER_WIDTH, FRAMEBUFFER_HEIGHT)); + let fb = Arc::new(SimpleFrameBuffer::new( + FRAMEBUFFER_WIDTH, + FRAMEBUFFER_HEIGHT, + )); let parser_names = vec!["original", "refactored" /*"memchr"*/]; @@ -108,19 +109,13 @@ fn invoke_benchmark( for parse_name in parser_names { c_group.bench_with_input(parse_name, &commands, |b, input| { - b.iter(|| { - let mut parser = match parse_name { - "original" => ParserImplementation::Original(OriginalParser::new(fb.clone())), - "refactored" => { - ParserImplementation::Refactored(RefactoredParser::new(fb.clone())) - } - "memchr" => ParserImplementation::Naive(MemchrParser::new(fb.clone())), - #[cfg(target_arch = "x86_64")] - "assembler" => ParserImplementation::Assembler(AssemblerParser::default()), - _ => panic!("Parser implementation {parse_name} not known"), - }; - - parser.parse(input, &mut Vec::new()); + b.iter(|| match parse_name { + "original" => OriginalParser::new(fb.clone()).parse(input, &mut Vec::new()), + "refactored" => RefactoredParser::new(fb.clone()).parse(input, &mut Vec::new()), + "memchr" => MemchrParser::new(fb.clone()).parse(input, &mut Vec::new()), + #[cfg(target_arch = "x86_64")] + "assembler" => AssemblerParser::new(fb.clone()).parse(input, &mut Vec::new()), + _ => panic!("Parser implementation {parse_name} not known"), }); }); } diff --git a/breakwater-parser/src/assembler.rs b/breakwater-parser/src/assembler.rs index 6c85530..ed947b8 100644 --- a/breakwater-parser/src/assembler.rs +++ b/breakwater-parser/src/assembler.rs @@ -4,12 +4,14 @@ use crate::{FrameBuffer, Parser}; const PARSER_LOOKAHEAD: usize = "PX 1234 1234 rrggbbaa\n".len(); // Longest possible command -#[derive(Default)] -#[allow(dead_code)] pub struct AssemblerParser { - help_text: &'static [u8], - alt_help_text: &'static [u8], - fb: Arc, + _fb: Arc, +} + +impl AssemblerParser { + pub fn new(_fb: Arc) -> Self { + Self { _fb } + } } impl Parser for AssemblerParser { diff --git a/breakwater-parser/src/framebuffer/mod.rs b/breakwater-parser/src/framebuffer/mod.rs new file mode 100644 index 0000000..a7f95a4 --- /dev/null +++ b/breakwater-parser/src/framebuffer/mod.rs @@ -0,0 +1,28 @@ +pub mod simple; + +pub trait FrameBuffer { + fn get_width(&self) -> usize; + + fn get_height(&self) -> usize; + + fn get_size(&self) -> usize { + self.get_width() * self.get_height() + } + + #[inline] + fn get(&self, x: usize, y: usize) -> Option { + if x < self.get_width() && y < self.get_height() { + Some(unsafe { self.get_unchecked(x, y) }) + } else { + None + } + } + + /// # Safety + /// make sure x and y are in bounds + unsafe fn get_unchecked(&self, x: usize, y: usize) -> u32; + + fn set(&self, x: usize, y: usize, rgba: u32); + + fn as_bytes(&self) -> &[u8]; +} diff --git a/breakwater-core/src/framebuffer.rs b/breakwater-parser/src/framebuffer/simple.rs similarity index 52% rename from breakwater-core/src/framebuffer.rs rename to breakwater-parser/src/framebuffer/simple.rs index 6cb400c..46771be 100644 --- a/breakwater-core/src/framebuffer.rs +++ b/breakwater-parser/src/framebuffer/simple.rs @@ -1,50 +1,41 @@ -use std::slice; +use super::FrameBuffer; -pub struct FrameBuffer { +pub struct SimpleFrameBuffer { width: usize, height: usize, buffer: Vec, } -impl FrameBuffer { +impl SimpleFrameBuffer { pub fn new(width: usize, height: usize) -> Self { let mut buffer = Vec::with_capacity(width * height); buffer.resize_with(width * height, || 0); - FrameBuffer { + Self { width, height, buffer, } } +} - pub fn get_width(&self) -> usize { +impl FrameBuffer for SimpleFrameBuffer { + #[inline(always)] + fn get_width(&self) -> usize { self.width } - pub fn get_height(&self) -> usize { - self.height - } - - pub fn get_size(&self) -> usize { - self.width * self.height - } - #[inline(always)] - pub fn get(&self, x: usize, y: usize) -> Option { - if x < self.width && y < self.height { - Some(*unsafe { self.buffer.get_unchecked(x + y * self.width) }) - } else { - None - } + fn get_height(&self) -> usize { + self.height } #[inline(always)] - pub fn get_unchecked(&self, x: usize, y: usize) -> u32 { - *unsafe { self.buffer.get_unchecked(x + y * self.width) } + unsafe fn get_unchecked(&self, x: usize, y: usize) -> u32 { + *self.buffer.get_unchecked(x + y * self.width) } #[inline(always)] - pub fn set(&self, x: usize, y: usize, rgba: u32) { + fn set(&self, x: usize, y: usize, rgba: u32) { // https://github.com/sbernauer/breakwater/pull/11 // If we make the FrameBuffer large enough (e.g. 10_000 x 10_000) we don't need to check the bounds here // (x and y are max 4 digit numbers). Flamegraph has shown 5.21% of runtime in this bound check. On the other @@ -58,34 +49,9 @@ impl FrameBuffer { } } - pub fn get_buffer(&self) -> &[u32] { - &self.buffer - } - - pub fn as_bytes(&self) -> &[u8] { - let len_in_bytes = self.buffer.len() * 4; - unsafe { slice::from_raw_parts(self.buffer.as_ptr() as *const u8, len_in_bytes) } - } -} - -impl breakwater_parser::FrameBuffer for FrameBuffer { - #[inline] - fn get_width(&self) -> usize { - self.get_width() - } - - #[inline] - fn get_height(&self) -> usize { - self.get_height() - } - - #[inline] - unsafe fn get_unchecked(&self, x: usize, y: usize) -> u32 { - self.get_unchecked(x, y) - } - - #[inline] - fn set(&self, x: usize, y: usize, rgba: u32) { - self.set(x, y, rgba) + fn as_bytes(&self) -> &[u8] { + let len = 4 * self.buffer.len(); + let ptr = self.buffer.as_ptr() as *const u8; + unsafe { std::slice::from_raw_parts(ptr, len) } } } diff --git a/breakwater-parser/src/lib.rs b/breakwater-parser/src/lib.rs index 381a79c..336e05b 100644 --- a/breakwater-parser/src/lib.rs +++ b/breakwater-parser/src/lib.rs @@ -1,15 +1,46 @@ // Needed for simple implementation #![feature(portable_simd)] -use enum_dispatch::enum_dispatch; +use const_format::formatcp; + +mod assembler; +mod framebuffer; +mod memchr; +mod original; +mod refactored; #[cfg(target_arch = "x86_64")] -pub mod assembler; -pub mod memchr; -pub mod original; -pub mod refactored; +pub use assembler::AssemblerParser; +pub use framebuffer::{simple::SimpleFrameBuffer, FrameBuffer}; +pub use memchr::MemchrParser; +pub use original::OriginalParser; +pub use refactored::RefactoredParser; + +pub const HELP_TEXT: &[u8] = formatcp!("\ +Pixelflut server powered by breakwater https://github.com/sbernauer/breakwater +Available commands: +HELP: Show this help +PX x y rrggbb: Color the pixel (x,y) with the given hexadecimal color rrggbb +{} +PX x y gg: Color the pixel (x,y) with the hexadecimal color gggggg. Basically this is the same as the other commands, but is a more efficient way of filling white, black or gray areas +PX x y: Get the color value of the pixel (x,y) +{}SIZE: Get the size of the drawing surface, e.g. `SIZE 1920 1080` +OFFSET x y: Apply offset (x,y) to all further pixel draws on this connection. This can e.g. be used to pre-calculate an image/animation and simply use the OFFSET command to move it around the screen without the need to re-calculate it +", +if cfg!(feature = "alpha") { + "PX x y rrggbbaa: Color the pixel (x,y) with the given hexadecimal color rrggbb and a transparency of aa, where ff means draw normally on top of the existing pixel and 00 means fully transparent (no change at all)" +} else { + "PX x y rrggbbaa: Color the pixel (x,y) with the given hexadecimal color rrggbb. The alpha part is discarded for performance reasons, as breakwater was compiled without the alpha feature" +}, +if cfg!(feature = "binary-commands") { + "PBxxyyrgba: Binary version of the PX command. x and y are little-endian 16 bit coordinates, r, g, b and a are a byte each. There is *no* newline after the command.\n" +} else { + "" +} +).as_bytes(); + +pub const ALT_HELP_TEXT: &[u8] = b"Stop spamming HELP!\n"; -#[enum_dispatch(ParserImplementation)] pub trait Parser { /// Returns the last byte parsed. The next parsing loop will again contain all data that was not parsed. fn parse(&mut self, buffer: &[u8], response: &mut Vec) -> usize; @@ -17,37 +48,3 @@ pub trait Parser { // Sadly this cant be const (yet?) (https://github.com/rust-lang/rust/issues/71971 and https://github.com/rust-lang/rfcs/pull/2632) fn parser_lookahead(&self) -> usize; } - -#[enum_dispatch] -pub enum ParserImplementation { - Original(original::OriginalParser), - Refactored(refactored::RefactoredParser), - Naive(memchr::MemchrParser), - #[cfg(target_arch = "x86_64")] - Assembler(assembler::AssemblerParser), -} - -pub trait FrameBuffer { - fn get_width(&self) -> usize; - - fn get_height(&self) -> usize; - - fn get_size(&self) -> usize { - self.get_width() * self.get_height() - } - - #[inline] - fn get(&self, x: usize, y: usize) -> Option { - if x < self.get_width() && y < self.get_height() { - Some(unsafe { self.get_unchecked(x, y) }) - } else { - None - } - } - - /// # Safety - /// make sure x and y are in bounds - unsafe fn get_unchecked(&self, x: usize, y: usize) -> u32; - - fn set(&self, x: usize, y: usize, rgba: u32); -} diff --git a/breakwater-parser/src/memchr.rs b/breakwater-parser/src/memchr.rs index 3d6c276..d91327b 100644 --- a/breakwater-parser/src/memchr.rs +++ b/breakwater-parser/src/memchr.rs @@ -2,20 +2,13 @@ use std::sync::Arc; use crate::{FrameBuffer, Parser}; -#[allow(dead_code)] pub struct MemchrParser { - help_text: &'static [u8], - alt_help_text: &'static [u8], fb: Arc, } impl MemchrParser { - pub fn new(fb: Arc, help_text: &'static [u8], alt_help_text: &'static [u8]) -> Self { - Self { - fb, - help_text, - alt_help_text, - } + pub fn new(fb: Arc) -> Self { + Self { fb } } } diff --git a/breakwater-parser/src/original.rs b/breakwater-parser/src/original.rs index a0f587d..b3c685d 100644 --- a/breakwater-parser/src/original.rs +++ b/breakwater-parser/src/original.rs @@ -3,7 +3,7 @@ use std::{ sync::Arc, }; -use crate::{FrameBuffer, Parser}; +use crate::{FrameBuffer, Parser, ALT_HELP_TEXT, HELP_TEXT}; pub const PARSER_LOOKAHEAD: usize = "PX 1234 1234 rrggbbaa\n".len(); // Longest possible command @@ -16,19 +16,15 @@ pub(crate) const HELP_PATTERN: u64 = string_to_number(b"HELP\0\0\0\0"); pub struct OriginalParser { connection_x_offset: usize, connection_y_offset: usize, - help_text: &'static [u8], - alt_help_text: &'static [u8], fb: Arc, } impl OriginalParser { - pub fn new(fb: Arc, help_text: &'static [u8], alt_help_text: &'static [u8]) -> Self { + pub fn new(fb: Arc) -> Self { Self { connection_x_offset: 0, connection_y_offset: 0, fb, - help_text, - alt_help_text, } } } @@ -185,11 +181,11 @@ impl Parser for OriginalParser { match help_count { 0..=2 => { - response.extend_from_slice(self.help_text); + response.extend_from_slice(HELP_TEXT); help_count += 1; } 3 => { - response.extend_from_slice(self.alt_help_text); + response.extend_from_slice(ALT_HELP_TEXT); help_count += 1; } _ => { diff --git a/breakwater-parser/src/refactored.rs b/breakwater-parser/src/refactored.rs index a87b338..8fdccd1 100644 --- a/breakwater-parser/src/refactored.rs +++ b/breakwater-parser/src/refactored.rs @@ -5,28 +5,23 @@ use crate::{ parse_pixel_coordinates, simd_unhex, HELP_PATTERN, OFFSET_PATTERN, PB_PATTERN, PX_PATTERN, SIZE_PATTERN, }, - FrameBuffer, Parser, + FrameBuffer, Parser, HELP_TEXT, }; const PARSER_LOOKAHEAD: usize = "PX 1234 1234 rrggbbaa\n".len(); // Longest possible command -#[allow(dead_code)] pub struct RefactoredParser { connection_x_offset: usize, connection_y_offset: usize, - help_text: &'static [u8], - alt_help_text: &'static [u8], fb: Arc, } impl RefactoredParser { - pub fn new(fb: Arc, help_text: &'static [u8], alt_help_text: &'static [u8]) -> Self { + pub fn new(fb: Arc) -> Self { Self { connection_x_offset: 0, connection_y_offset: 0, fb, - help_text, - alt_help_text, } } @@ -125,7 +120,7 @@ impl RefactoredParser { #[inline(always)] fn handle_help(&self, response: &mut Vec) { - response.extend_from_slice(self.help_text); + response.extend_from_slice(HELP_TEXT); } #[inline(always)] diff --git a/breakwater/Cargo.toml b/breakwater/Cargo.toml index 810ea92..1248c7c 100644 --- a/breakwater/Cargo.toml +++ b/breakwater/Cargo.toml @@ -12,7 +12,6 @@ name = "breakwater" path = "src/main.rs" [dependencies] -breakwater-core.workspace = true breakwater-parser.workspace = true chrono.workspace = true @@ -40,5 +39,5 @@ rstest.workspace = true default = ["vnc", "binary-commands"] vnc = ["dep:vncserver"] -alpha = ["breakwater-core/alpha", "breakwater-parser/alpha"] -binary-commands = ["breakwater-core/binary-commands", "breakwater-parser/binary-commands"] +alpha = ["breakwater-parser/alpha"] +binary-commands = ["breakwater-parser/binary-commands"] diff --git a/breakwater/src/main.rs b/breakwater/src/main.rs index 5402c11..01effae 100644 --- a/breakwater/src/main.rs +++ b/breakwater/src/main.rs @@ -1,6 +1,6 @@ use std::{env, num::TryFromIntError, sync::Arc}; -use breakwater_core::framebuffer::FrameBuffer; +use breakwater_parser::SimpleFrameBuffer; use clap::Parser; use log::{info, trace}; use prometheus_exporter::PrometheusExporter; @@ -26,6 +26,8 @@ mod prometheus_exporter; mod server; mod sinks; mod statistics; +#[cfg(test)] +mod test_helpers; #[cfg(test)] mod tests; @@ -87,7 +89,8 @@ async fn main() -> Result<(), Error> { let args = CliArgs::parse(); - let fb = Arc::new(FrameBuffer::new(args.width, args.height)); + // Not using dynamic dispatch here for performance reasons + let fb = Arc::new(SimpleFrameBuffer::new(args.width, args.height)); // If we make the channel to big, stats will start to lag behind // TODO: Check performance impact in real-world scenario. Maybe the statistics thread blocks the other threads diff --git a/breakwater/src/server.rs b/breakwater/src/server.rs index 80d71f0..d0d0972 100644 --- a/breakwater/src/server.rs +++ b/breakwater/src/server.rs @@ -3,9 +3,7 @@ use std::collections::hash_map::Entry; use std::collections::HashMap; use std::{cmp::min, net::IpAddr, sync::Arc, time::Duration}; -use breakwater_core::{framebuffer::FrameBuffer, CONNECTION_DENIED_TEXT}; -use breakwater_core::{ALT_HELP_TEXT, HELP_TEXT}; -use breakwater_parser::{original::OriginalParser, Parser}; +use breakwater_parser::{FrameBuffer, OriginalParser, Parser}; use log::{debug, info, warn}; use memadvise::{Advice, MemAdviseError}; use snafu::{ResultExt, Snafu}; @@ -18,6 +16,8 @@ use tokio::{ use crate::statistics::StatisticsEvent; +const CONNECTION_DENIED_TEXT: &[u8] = b"Connection denied as connection limit is reached"; + // Every client connection spawns a new thread, so we need to limit the number of stat events we send const STATISTICS_REPORT_INTERVAL: Duration = Duration::from_millis(250); @@ -41,20 +41,20 @@ pub enum Error { }, } -pub struct Server { +pub struct Server { // listen_address: String, listener: TcpListener, - fb: Arc, + fb: Arc, statistics_tx: mpsc::Sender, network_buffer_size: usize, connections_per_ip: HashMap, max_connections_per_ip: Option, } -impl Server { +impl Server { pub async fn new( listen_address: &str, - fb: Arc, + fb: Arc, statistics_tx: mpsc::Sender, network_buffer_size: usize, max_connections_per_ip: Option, @@ -142,10 +142,10 @@ impl Server { } } -pub async fn handle_connection( +pub async fn handle_connection( mut stream: impl AsyncReadExt + AsyncWriteExt + Send + Unpin, ip: IpAddr, - fb: Arc, + fb: Arc, statistics_tx: mpsc::Sender, page_size: usize, network_buffer_size: usize, @@ -179,7 +179,7 @@ pub async fn handle_connection( // Not using `ParserImplementation` to avoid the dynamic dispatch. // let mut parser = ParserImplementation::Simple(SimpleParser::new(fb)); - let mut parser = OriginalParser::new(fb, HELP_TEXT, ALT_HELP_TEXT); + let mut parser = OriginalParser::new(fb); let parser_lookahead = parser.parser_lookahead(); // If we send e.g. an StatisticsEvent::BytesRead for every time we read something from the socket the statistics thread would go crazy. diff --git a/breakwater/src/sinks/ffmpeg.rs b/breakwater/src/sinks/ffmpeg.rs index 26bf6df..ae6e00e 100644 --- a/breakwater/src/sinks/ffmpeg.rs +++ b/breakwater/src/sinks/ffmpeg.rs @@ -1,6 +1,6 @@ use std::{process::Stdio, sync::Arc, time::Duration}; -use breakwater_core::framebuffer::FrameBuffer; +use breakwater_parser::FrameBuffer; use chrono::Local; use log::debug; use snafu::{ResultExt, Snafu}; @@ -25,15 +25,15 @@ pub enum Error { WriteDataToFfmeg { source: std::io::Error }, } -pub struct FfmpegSink { - fb: Arc, +pub struct FfmpegSink { + fb: Arc, rtmp_address: Option, video_save_folder: Option, fps: u32, } -impl FfmpegSink { - pub fn new(args: &CliArgs, fb: Arc) -> Option { +impl FfmpegSink { + pub fn new(args: &CliArgs, fb: Arc) -> Option { if args.rtmp_address.is_some() || args.video_save_folder.is_some() { Some(FfmpegSink { fb, diff --git a/breakwater/src/sinks/vnc.rs b/breakwater/src/sinks/vnc.rs index 496e6b7..83e1da6 100644 --- a/breakwater/src/sinks/vnc.rs +++ b/breakwater/src/sinks/vnc.rs @@ -1,6 +1,6 @@ use std::{sync::Arc, time::Duration}; -use breakwater_core::framebuffer::FrameBuffer; +use breakwater_parser::FrameBuffer; use core::slice; use number_prefix::NumberPrefix; use rusttype::{point, Font, Scale}; @@ -42,9 +42,10 @@ pub enum Error { } // Sorry! Help needed :) -unsafe impl<'a> Send for VncServer<'a> {} -pub struct VncServer<'a> { - fb: Arc, +unsafe impl<'a, FB: FrameBuffer> Send for VncServer<'a, FB> {} + +pub struct VncServer<'a, FB: FrameBuffer> { + fb: Arc, screen: RfbScreenInfoPtr, target_fps: u32, @@ -56,10 +57,10 @@ pub struct VncServer<'a> { font: Font<'a>, } -impl<'a> VncServer<'a> { +impl<'a, FB: FrameBuffer> VncServer<'a, FB> { #[allow(clippy::too_many_arguments)] pub fn new( - fb: Arc, + fb: Arc, port: u16, target_fps: u32, statistics_tx: Sender, @@ -119,8 +120,11 @@ impl<'a> VncServer<'a> { pub fn run(&mut self) -> Result<(), Error> { let target_loop_duration = Duration::from_micros(1_000_000 / self.target_fps as u64); - let vnc_fb_slice: &mut [u32] = unsafe { - slice::from_raw_parts_mut((*self.screen).frameBuffer as *mut u32, self.fb.get_size()) + let vnc_fb_slice: &mut [u8] = unsafe { + slice::from_raw_parts_mut( + (*self.screen).frameBuffer as *mut u8, + self.fb.get_size() * 4, + ) }; // A line less because the (height - STATS_SURFACE_HEIGHT) belongs to the stats and gets refreshed by them let height_up_to_stats_text = self.fb.get_height() - STATS_HEIGHT - 1; @@ -132,8 +136,9 @@ impl<'a> VncServer<'a> { } let start = std::time::Instant::now(); - vnc_fb_slice[0..fb_size_up_to_stats_text] - .copy_from_slice(&self.fb.get_buffer()[0..fb_size_up_to_stats_text]); + + vnc_fb_slice[0..fb_size_up_to_stats_text * 4] + .copy_from_slice(&self.fb.as_bytes()[0..fb_size_up_to_stats_text * 4]); // Only refresh the drawing surface, not the stats surface rfb_mark_rect_as_modified( diff --git a/breakwater-core/src/test_helpers/dev_null_tcp_stream.rs b/breakwater/src/test_helpers/dev_null_tcp_stream.rs similarity index 100% rename from breakwater-core/src/test_helpers/dev_null_tcp_stream.rs rename to breakwater/src/test_helpers/dev_null_tcp_stream.rs diff --git a/breakwater-core/src/test_helpers/mock_tcp_stream.rs b/breakwater/src/test_helpers/mock_tcp_stream.rs similarity index 100% rename from breakwater-core/src/test_helpers/mock_tcp_stream.rs rename to breakwater/src/test_helpers/mock_tcp_stream.rs diff --git a/breakwater/src/test_helpers/mod.rs b/breakwater/src/test_helpers/mod.rs new file mode 100644 index 0000000..d388a74 --- /dev/null +++ b/breakwater/src/test_helpers/mod.rs @@ -0,0 +1,2 @@ +pub mod dev_null_tcp_stream; +pub mod mock_tcp_stream; diff --git a/breakwater/src/tests.rs b/breakwater/src/tests.rs index abfcb1a..154cb3f 100644 --- a/breakwater/src/tests.rs +++ b/breakwater/src/tests.rs @@ -5,12 +5,13 @@ use std::{ sync::Arc, }; -use breakwater_core::{framebuffer::FrameBuffer, test_helpers::MockTcpStream, HELP_TEXT}; +use breakwater_parser::{FrameBuffer, SimpleFrameBuffer, HELP_TEXT}; use rstest::{fixture, rstest}; use tokio::sync::mpsc; use crate::{ cli_args::DEFAULT_NETWORK_BUFFER_SIZE, server::handle_connection, statistics::StatisticsEvent, + test_helpers::mock_tcp_stream::MockTcpStream, }; #[fixture] @@ -19,9 +20,9 @@ fn ip() -> IpAddr { } #[fixture] -fn fb() -> Arc { +fn fb() -> Arc { // We keep the framebuffer so small, so that we can easily test all pixels in a test run - Arc::new(FrameBuffer::new(640, 480)) + Arc::new(SimpleFrameBuffer::new(640, 480)) } #[fixture] @@ -46,11 +47,11 @@ fn statistics_channel() -> ( #[case("HELP\n", std::str::from_utf8(HELP_TEXT).unwrap())] #[case("bla bla bla\nSIZE\nblub\nbla", "SIZE 640 480\n")] #[tokio::test] -async fn test_correct_responses_to_general_commands( +async fn test_correct_responses_to_general_commands( #[case] input: &str, #[case] expected: &str, ip: IpAddr, - fb: Arc, + fb: Arc, statistics_channel: ( mpsc::Sender, mpsc::Receiver, @@ -113,11 +114,11 @@ async fn test_correct_responses_to_general_commands( )] // The get pixel result is also offseted #[case("OFFSET 0 0\nPX 0 42 abcdef\nPX 0 42\n", "PX 0 42 abcdef\n")] #[tokio::test] -async fn test_setting_pixel( +async fn test_setting_pixel( #[case] input: &str, #[case] expected: &str, ip: IpAddr, - fb: Arc, + fb: Arc, statistics_channel: ( mpsc::Sender, mpsc::Receiver, @@ -156,11 +157,11 @@ async fn test_setting_pixel( "PX 0 0 000000\nPX 0 0 313233\n" )] #[tokio::test] -async fn test_binary_commands( +async fn test_binary_commands( #[case] input: &str, #[case] expected: &str, ip: IpAddr, - fb: Arc, + fb: Arc, statistics_channel: ( mpsc::Sender, mpsc::Receiver, @@ -186,10 +187,10 @@ async fn test_binary_commands( #[case("PX 0 0 aaaaaa\n")] #[case("PX 0 0 aa\n")] #[tokio::test] -async fn test_safe( +async fn test_safe( #[case] input: &str, ip: IpAddr, - fb: Arc, + fb: Arc, statistics_channel: ( mpsc::Sender, mpsc::Receiver, @@ -229,13 +230,13 @@ async fn test_safe( // Yes, this exceeds the framebuffer size #[case(10, 10, fb().get_width() - 5, fb().get_height() - 5)] #[tokio::test] -async fn test_drawing_rect( +async fn test_drawing_rect( #[case] width: usize, #[case] height: usize, #[case] offset_x: usize, #[case] offset_y: usize, ip: IpAddr, - fb: Arc, + fb: Arc, statistics_channel: ( mpsc::Sender, mpsc::Receiver,