From d571b1aa72895daa0e3e9dcec4807aaea6487a39 Mon Sep 17 00:00:00 2001 From: bits0rcerer <25325997+bits0rcerer@users.noreply.github.com> Date: Sat, 3 Aug 2024 22:38:36 +0200 Subject: [PATCH 1/3] Original parser fixes (#36) * remove 2 unnecessary loop iterations for HELP and SIZE commands * fix binary set pixel last_byte_parsed --- breakwater-parser/src/original.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/breakwater-parser/src/original.rs b/breakwater-parser/src/original.rs index 801149b..a9c8171 100644 --- a/breakwater-parser/src/original.rs +++ b/breakwater-parser/src/original.rs @@ -154,7 +154,8 @@ impl Parser for OriginalParser { // TODO: Support alpha channel (behind alpha feature flag) self.fb.set(x as usize, y as usize, rgba & 0x00ff_ffff); - + // P B XX YY RGBA + last_byte_parsed = i + 1 + 2 + 2 + 4; i += 10; continue; } else if current_command & 0x00ff_ffff_ffff_ffff == OFFSET_PATTERN { @@ -171,7 +172,7 @@ impl Parser for OriginalParser { } } else if current_command & 0xffff_ffff == SIZE_PATTERN { i += 4; - last_byte_parsed = i - 1; + last_byte_parsed = i + 1; response.extend_from_slice( format!("SIZE {} {}\n", self.fb.get_width(), self.fb.get_height()).as_bytes(), @@ -179,7 +180,7 @@ impl Parser for OriginalParser { continue; } else if current_command & 0xffff_ffff == HELP_PATTERN { i += 4; - last_byte_parsed = i - 1; + last_byte_parsed = i + 1; match help_count { 0..=2 => { From c8b875701cd1d6b39e9c8c89e6dfb0f7176ac4cf Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Sat, 3 Aug 2024 22:42:47 +0200 Subject: [PATCH 2/3] chore: Add missing changelog entry for #36 --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index db7f5bc..3510d37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,13 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Fixed + +- Performance improvement due to addition of missing set of `last_byte_parsed` when sending binary pixel commands ([#36]) +- Minor performance improvement when parsing `HELP` and `SIZE` requests ([#36]) + +[#36]: https://github.com/sbernauer/breakwater/pull/36 + ## [0.15.0] - 2024-06-12 ### Added From 1db11a7e1cd341a5a816aa0eaed38a5cdbe417b0 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Sun, 4 Aug 2024 10:51:26 +0200 Subject: [PATCH 3/3] feat!: Implement binary pixel sync command (#34) * Rename feature binary-commands to binary-set-single-pixel * Add skeleton * Add pixe sync, but missing wrapping around buffers * Add test-case for buffer wrapping * WIP, not working! * More testing and debugging * Finally all tests are passing :) * Turn feature off by default * remove leftover comment * fix: Put import behind [cfg()] * Remove println!() statements * changelog * Rename binary-set-single-pixel to binary-set-pixel * cargo update * changelog * Correctly propagate features * fix: Wrong calculation of index after px muli * Add command to readme --- CHANGELOG.md | 9 + Cargo.lock | 219 +++++------ README.md | 4 + breakwater-core/Cargo.toml | 6 +- breakwater-core/src/framebuffer.rs | 140 ++++++++ breakwater-core/src/lib.rs | 11 +- .../src/test_helpers/mock_tcp_stream.rs | 9 +- breakwater-parser/Cargo.toml | 5 +- breakwater-parser/src/original.rs | 118 +++++- breakwater-parser/src/refactored.rs | 2 +- breakwater/Cargo.toml | 6 +- breakwater/src/server.rs | 9 + breakwater/src/tests.rs | 339 +++++++++++++----- 13 files changed, 655 insertions(+), 222 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3510d37..5cd40ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,11 +4,20 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Added + +- Added support for binary sync protocol behind the `binary-sync-pixels` feature ([#34]) + +### Changed + +- BREAKING: Feature `binary-commands` has been renamed to `binary-set-pixel` ([#34]) + ### Fixed - Performance improvement due to addition of missing set of `last_byte_parsed` when sending binary pixel commands ([#36]) - Minor performance improvement when parsing `HELP` and `SIZE` requests ([#36]) +[#34]: https://github.com/sbernauer/breakwater/pull/34 [#36]: https://github.com/sbernauer/breakwater/pull/36 ## [0.15.0] - 2024-06-12 diff --git a/Cargo.lock b/Cargo.lock index 78db8ae..78b0164 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -174,9 +174,9 @@ dependencies = [ [[package]] name = "backtrace" -version = "0.3.72" +version = "0.3.73" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "17c6a35df3749d2e8bb1b7b21a976d82b15548788d2735b9d82f329268f71a11" +checksum = "5cc23269a4f8976d0a4d2e7109211a419fe30e8d88d677cd60b6bc79c5732e0a" dependencies = [ "addr2line", "cc", @@ -193,10 +193,10 @@ version = "0.69.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a00dc851838a2120612785d195287475a3ac45514741da670b735818822129a0" dependencies = [ - "bitflags 2.5.0", + "bitflags 2.6.0", "cexpr", "clang-sys", - "itertools 0.10.5", + "itertools 0.12.1", "lazy_static", "lazycell", "log", @@ -224,15 +224,15 @@ checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" [[package]] name = "bitflags" -version = "2.5.0" +version = "2.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cf4b9d6a944f767f8e5e0db018570623c85f3d925ac718db4e06d0187adb21c1" +checksum = "b048fb63fd8b5923fc5aa7b340d8e156aec7ec02f0c78fa8a6ddc2613f6f71de" [[package]] name = "bitstream-io" -version = "2.3.0" +version = "2.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7c12d1856e42f0d817a835fe55853957c85c8c8a470114029143d3f12671446e" +checksum = "3dcde5f311c85b8ca30c2e4198d4326bc342c76541590106f5fa4a50946ea499" [[package]] name = "breakwater" @@ -265,6 +265,7 @@ name = "breakwater-core" version = "0.15.0" dependencies = [ "const_format", + "rstest", "tokio", ] @@ -287,9 +288,9 @@ checksum = "40e38929add23cdf8a366df9b0e088953150724bcbe5fc330b0d8eb3b328eec8" [[package]] name = "built" -version = "0.7.3" +version = "0.7.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c6a6c0b39c38fd754ac338b00a88066436389c0f029da5d37d1e01091d9b7c17" +checksum = "236e6289eda5a812bc6b53c3b024039382a2895fbbeef2d748b2931546d392c4" [[package]] name = "bumpalo" @@ -299,9 +300,9 @@ checksum = "79296716171880943b8470b5f8d03aa55eb2e645a4874bdbb28adb49162e012c" [[package]] name = "bytemuck" -version = "1.16.0" +version = "1.16.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "78834c15cb5d5efe3452d58b1e8ba890dd62d21907f867f383358198e56ebca5" +checksum = "b236fc92302c97ed75b38da1f4917b5cdda4984745740f153a5d3059e48d725e" [[package]] name = "byteorder" @@ -317,9 +318,9 @@ checksum = "8f1fe948ff07f4bd06c30984e69f5b4899c516a3ef74f34df92a2df2ab535495" [[package]] name = "bytes" -version = "1.6.0" +version = "1.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "514de17de45fdb8dc022b1a7975556c53c86f9f0aa5f534b98977b171857c2c9" +checksum = "a12916984aab3fa6e39d655a33e09c0071eb36d6ab3aea5c2d78551f1df6d952" [[package]] name = "cast" @@ -329,9 +330,9 @@ checksum = "37b2a672a2cb129a2e41c10b1224bb368f9f37a2b16b612598138befd7b37eb5" [[package]] name = "cc" -version = "1.0.99" +version = "1.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "96c51067fd44124faa7f870b4b1c969379ad32b2ba805aa959430ceaa384f695" +checksum = "47de7e88bbbd467951ae7f5a6f34f70d1b4d9cfce53d5fd70f74ebe118b3db56" dependencies = [ "jobserver", "libc", @@ -380,7 +381,7 @@ dependencies = [ "js-sys", "num-traits", "wasm-bindgen", - "windows-targets 0.52.5", + "windows-targets 0.52.6", ] [[package]] @@ -429,9 +430,9 @@ dependencies = [ [[package]] name = "clap" -version = "4.5.6" +version = "4.5.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a9689a29b593160de5bc4aacab7b5d54fb52231de70122626c178e6a368994c7" +checksum = "64acc1846d54c1fe936a78dc189c34e28d3f5afc348403f28ecf53660b9b8462" dependencies = [ "clap_builder", "clap_derive", @@ -439,9 +440,9 @@ dependencies = [ [[package]] name = "clap_builder" -version = "4.5.6" +version = "4.5.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2e5387378c84f6faa26890ebf9f0a92989f8873d4d380467bcd0d8d8620424df" +checksum = "6fb8393d67ba2e7bfaf28a23458e4e2b543cc73a99595511eb207fdb8aede942" dependencies = [ "anstream", "anstyle", @@ -451,9 +452,9 @@ dependencies = [ [[package]] name = "clap_derive" -version = "4.5.5" +version = "4.5.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c780290ccf4fb26629baa7a1081e68ced113f1d3ec302fa5948f1c381ebf06c6" +checksum = "2bac35c6dafb060fd4d275d9a4ffae97917c13a6327903a8be2153cd964f7085" dependencies = [ "heck", "proc-macro2", @@ -594,9 +595,9 @@ dependencies = [ [[package]] name = "either" -version = "1.12.0" +version = "1.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3dca9240753cf90908d7e4aac30f630662b02aebaa1b58a3cadabdb23385b58b" +checksum = "60b1af1c220855b6ceac025d3f6ecdd2b7c4894bfe9cd9bda4fbb4bc7c0d4cf0" [[package]] name = "enum_dispatch" @@ -1049,9 +1050,9 @@ dependencies = [ [[package]] name = "lazy_static" -version = "1.4.0" +version = "1.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646" +checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe" [[package]] name = "lazycell" @@ -1084,12 +1085,12 @@ dependencies = [ [[package]] name = "libloading" -version = "0.8.3" +version = "0.8.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0c2a198fb6b0eada2a8df47933734e6d35d350665a33a3593d7164fa52c75c19" +checksum = "e310b3a6b5907f99202fcdb4960ff45b93735d7c7d96b760fcff8db2dc0e103d" dependencies = [ "cfg-if 1.0.0", - "windows-targets 0.48.5", + "windows-targets 0.52.6", ] [[package]] @@ -1110,9 +1111,9 @@ dependencies = [ [[package]] name = "log" -version = "0.4.21" +version = "0.4.22" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "90ed8c1e510134f979dbc4f070f87d4313098b704861a105fe34231c70a3901c" +checksum = "a7a70ba024b9dc04c27ea2f0c0548feb474ec5c54bba33a7f72f873a39d07b24" [[package]] name = "loop9" @@ -1147,9 +1148,9 @@ dependencies = [ [[package]] name = "memchr" -version = "2.7.2" +version = "2.7.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6c8640c5d730cb13ebd907d8d04b52f55ac9a2eec55b440c8892f40d56c76c1d" +checksum = "78ca9ab1a0babb1e7d5695e3530886289c18cf2f87ec19a575a0abdce112e3a3" [[package]] name = "minimal-lexical" @@ -1159,9 +1160,9 @@ checksum = "68354c5c6bd36d73ff3feceb05efa59b6acb7626617f4962be322a825e61f79a" [[package]] name = "miniz_oxide" -version = "0.7.3" +version = "0.7.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "87dfd01fe195c66b572b37921ad8803d010623c0aca821bea2302239d155cdae" +checksum = "b8a240ddb74feaf34a79a7add65a741f3167852fba007066dcac1ca548d89c08" dependencies = [ "adler", "simd-adler32", @@ -1222,9 +1223,9 @@ dependencies = [ [[package]] name = "num-bigint" -version = "0.4.5" +version = "0.4.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c165a9ab64cf766f73521c0dd2cfdff64f488b8f0b3e621face3462d3db536d7" +checksum = "a5e44f723f1133c9deac646763579fdb3ac745e418f2a7af9cd0c431da1f20b9" dependencies = [ "num-integer", "num-traits", @@ -1294,9 +1295,9 @@ checksum = "830b246a0e5f20af87141b25c173cd1b609bd7779a4617d6ec582abaf90870f3" [[package]] name = "object" -version = "0.35.0" +version = "0.36.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b8ec7ab813848ba4522158d5517a6093db1ded27575b070f4177b8d12b41db5e" +checksum = "081b846d1d56ddfc18fdf1a922e4f6e07a11768ea1b92dec44e42b72712ccfce" dependencies = [ "memchr", ] @@ -1309,9 +1310,9 @@ checksum = "3fdb12b2476b595f9358c5161aa467c2438859caa136dec86c26fdd2efe17b92" [[package]] name = "oorandom" -version = "11.1.3" +version = "11.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0ab1bc2a289d34bd04a330323ac98a1b4bc82c9d9fcb1e66b63caa84da26b575" +checksum = "b410bbe7e14ab526a0e86877eb47c6996a2bd7746f027ba551028c925390e4e9" [[package]] name = "owned_ttf_parser" @@ -1362,7 +1363,7 @@ dependencies = [ "libc", "redox_syscall", "smallvec", - "windows-targets 0.52.5", + "windows-targets 0.52.6", ] [[package]] @@ -1483,9 +1484,9 @@ dependencies = [ [[package]] name = "proc-macro2" -version = "1.0.85" +version = "1.0.86" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "22244ce15aa966053a896d1accb3a6e68469b97c7f33f284b99f0d576879fc23" +checksum = "5e719e8df665df0d1c8fbfd238015744736151d4445ec0836b8e628aae103b77" dependencies = [ "unicode-ident", ] @@ -1628,9 +1629,9 @@ dependencies = [ [[package]] name = "ravif" -version = "0.11.5" +version = "0.11.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bc13288f5ab39e6d7c9d501759712e6969fcc9734220846fc9ed26cae2cc4234" +checksum = "c6ba61c28ba24c0cf8406e025cb29a742637e3f70776e61c27a8a8b72a042d12" dependencies = [ "avif-serialize", "imgref", @@ -1663,11 +1664,11 @@ dependencies = [ [[package]] name = "redox_syscall" -version = "0.5.1" +version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "469052894dcb553421e483e4209ee581a45100d31b4018de03e5a7ad86374a7e" +checksum = "c82cf8cff14456045f55ec4241383baeff27af886adb72ffb2162f99911de0fd" dependencies = [ - "bitflags 2.5.0", + "bitflags 2.6.0", ] [[package]] @@ -1707,9 +1708,9 @@ checksum = "ba39f3699c378cd8970968dcbff9c43159ea4cfbd88d43c00b22f2ef10a435d2" [[package]] name = "rgb" -version = "0.8.37" +version = "0.8.44" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "05aaa8004b64fd573fc9d002f4e632d51ad4f026c2b5ba95fcb6c2f32c2c47d8" +checksum = "1aee83dc281d5a3200d37b299acd13b81066ea126a7f16f0eae70fc9aed241d9" dependencies = [ "bytemuck", ] @@ -1771,7 +1772,7 @@ version = "0.38.34" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "70dc5ec042f7a43c4a73241207cecc9873a06d45debb38b329f8541d85c2730f" dependencies = [ - "bitflags 2.5.0", + "bitflags 2.6.0", "errno", "libc", "linux-raw-sys", @@ -1823,18 +1824,18 @@ checksum = "61697e0a1c7e512e84a621326239844a24d8207b4669b41bc18b32ea5cbf988b" [[package]] name = "serde" -version = "1.0.203" +version = "1.0.204" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7253ab4de971e72fb7be983802300c30b5a7f0c2e56fab8abfc6a214307c0094" +checksum = "bc76f558e0cbb2a839d37354c575f1dc3fdc6546b5be373ba43d95f231bf7c12" dependencies = [ "serde_derive", ] [[package]] name = "serde_derive" -version = "1.0.203" +version = "1.0.204" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "500cbc0ebeb6f46627f50f3f5811ccf6bf00643be300b4c3eabc0ef55dc5b5ba" +checksum = "e0cd7e117be63d3c3678776753929474f3b04a43a080c744d6b0ae2a8c28e222" dependencies = [ "proc-macro2", "quote", @@ -1843,9 +1844,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.117" +version = "1.0.120" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "455182ea6142b14f93f4bc5320a2b31c1f266b66a4a5c858b013302a5d8cbfc3" +checksum = "4e0d21c9a8cae1235ad58a00c11cb40d4b1e5c784f1ef2c537876ed6ffd8b7c5" dependencies = [ "itoa", "ryu", @@ -1917,18 +1918,18 @@ checksum = "3c5e1a9a646d36c3599cd173a41282daf47c44583ad367b8e6837255952e5c67" [[package]] name = "snafu" -version = "0.8.3" +version = "0.8.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "418b8136fec49956eba89be7da2847ec1909df92a9ae4178b5ff0ff092c8d95e" +checksum = "2b835cb902660db3415a672d862905e791e54d306c6e8189168c7f3d9ae1c79d" dependencies = [ "snafu-derive", ] [[package]] name = "snafu-derive" -version = "0.8.3" +version = "0.8.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1a4812a669da00d17d8266a0439eddcacbc88b17f732f927e52eeb9d196f7fb5" +checksum = "38d1e02fca405f6280643174a50c942219f0bbf4dbf7d480f1dd864d6f211ae5" dependencies = [ "heck", "proc-macro2", @@ -1963,9 +1964,9 @@ checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f" [[package]] name = "syn" -version = "2.0.66" +version = "2.0.71" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c42f3f41a2de00b01c0aaad383c5a45241efc8b2d1eda5661812fda5f3cdcff5" +checksum = "b146dcf730474b4bcd16c311627b31ede9ab149045db4d6088b3becaea046462" dependencies = [ "proc-macro2", "quote", @@ -1974,9 +1975,9 @@ dependencies = [ [[package]] name = "sysinfo" -version = "0.30.12" +version = "0.30.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "732ffa00f53e6b2af46208fba5718d9662a421049204e156328b66791ffa15ae" +checksum = "0a5b4ddaee55fb2bea2bf0e5000747e5f5c0de765e5a5ff87f4cd106439f4bb3" dependencies = [ "cfg-if 1.0.0", "core-foundation-sys", @@ -2002,24 +2003,24 @@ dependencies = [ [[package]] name = "target-lexicon" -version = "0.12.14" +version = "0.12.15" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e1fc403891a21bcfb7c37834ba66a547a8f402146eba7265b5a6d88059c9ff2f" +checksum = "4873307b7c257eddcb50c9bedf158eb669578359fb28428bef438fec8e6ba7c2" [[package]] name = "thiserror" -version = "1.0.61" +version = "1.0.62" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c546c80d6be4bc6a00c0f01730c08df82eaa7a7a61f11d656526506112cc1709" +checksum = "f2675633b1499176c2dff06b0856a27976a8f9d436737b4cf4f312d4d91d8bbb" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "1.0.61" +version = "1.0.62" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "46c3384250002a6d5af4d114f2845d37b57521033f30d5c3f46c4d70e1197533" +checksum = "d20468752b09f49e909e55a5d338caa8bedf615594e9d80bc4c565d30faf798c" dependencies = [ "proc-macro2", "quote", @@ -2032,7 +2033,7 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0d3b04d33c9633b8662b167b847c7ab521f83d1ae20f2321b65b5b925e532e36" dependencies = [ - "bitflags 2.5.0", + "bitflags 2.6.0", "cfg-if 1.0.0", "libc", "log", @@ -2107,9 +2108,9 @@ dependencies = [ [[package]] name = "tinyvec" -version = "1.6.0" +version = "1.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "87cc5ceb3875bb20c2890005a4e226a4651264a5c75edb2421b52861a0a0cb50" +checksum = "445e881f4f6d382d5f27c034e25eb92edd7c784ceab92a0937db7f2e9471b938" dependencies = [ "tinyvec_macros", ] @@ -2158,7 +2159,7 @@ dependencies = [ "serde", "serde_spanned", "toml_datetime", - "toml_edit 0.22.14", + "toml_edit 0.22.15", ] [[package]] @@ -2183,9 +2184,9 @@ dependencies = [ [[package]] name = "toml_edit" -version = "0.22.14" +version = "0.22.15" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f21c7aaf97f1bd9ca9d4f9e73b0a6c74bd5afef56f2bc931943a6e1c37e04e38" +checksum = "d59a3a72298453f564e2b111fa896f8d07fabb36f51f06d7e875fc5e0b5a3ef1" dependencies = [ "indexmap", "serde", @@ -2229,9 +2230,9 @@ checksum = "f962df74c8c05a667b5ee8bcf162993134c104e96440b663c8daa176dc772d8c" [[package]] name = "url" -version = "2.5.0" +version = "2.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "31e6302e3bb753d46e83516cae55ae196fc0c309407cf11ab35cc51a4c2a4633" +checksum = "22784dbdf76fdde8af1aeda5622b546b422b6fc585325248a2bf9f5e41e94d6c" dependencies = [ "form_urlencoded", "idna", @@ -2419,7 +2420,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e48a53791691ab099e5e2ad123536d0fff50652600abaf43bbf952894110d0be" dependencies = [ "windows-core", - "windows-targets 0.52.5", + "windows-targets 0.52.6", ] [[package]] @@ -2428,7 +2429,7 @@ version = "0.52.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "33ab640c8d7e35bf8ba19b884ba838ceb4fba93a4e8c65a9059d08afcfc683d9" dependencies = [ - "windows-targets 0.52.5", + "windows-targets 0.52.6", ] [[package]] @@ -2446,7 +2447,7 @@ version = "0.52.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "282be5f36a8ce781fad8c8ae18fa3f9beff57ec1b52cb3de0789201425d9a33d" dependencies = [ - "windows-targets 0.52.5", + "windows-targets 0.52.6", ] [[package]] @@ -2466,18 +2467,18 @@ dependencies = [ [[package]] name = "windows-targets" -version = "0.52.5" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6f0713a46559409d202e70e28227288446bf7841d3211583a4b53e3f6d96e7eb" +checksum = "9b724f72796e036ab90c1021d4780d4d3d648aca59e491e6b98e725b84e99973" dependencies = [ - "windows_aarch64_gnullvm 0.52.5", - "windows_aarch64_msvc 0.52.5", - "windows_i686_gnu 0.52.5", + "windows_aarch64_gnullvm 0.52.6", + "windows_aarch64_msvc 0.52.6", + "windows_i686_gnu 0.52.6", "windows_i686_gnullvm", - "windows_i686_msvc 0.52.5", - "windows_x86_64_gnu 0.52.5", - "windows_x86_64_gnullvm 0.52.5", - "windows_x86_64_msvc 0.52.5", + "windows_i686_msvc 0.52.6", + "windows_x86_64_gnu 0.52.6", + "windows_x86_64_gnullvm 0.52.6", + "windows_x86_64_msvc 0.52.6", ] [[package]] @@ -2488,9 +2489,9 @@ checksum = "2b38e32f0abccf9987a4e3079dfb67dcd799fb61361e53e2882c3cbaf0d905d8" [[package]] name = "windows_aarch64_gnullvm" -version = "0.52.5" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7088eed71e8b8dda258ecc8bac5fb1153c5cffaf2578fc8ff5d61e23578d3263" +checksum = "32a4622180e7a0ec044bb555404c800bc9fd9ec262ec147edd5989ccd0c02cd3" [[package]] name = "windows_aarch64_msvc" @@ -2500,9 +2501,9 @@ checksum = "dc35310971f3b2dbbf3f0690a219f40e2d9afcf64f9ab7cc1be722937c26b4bc" [[package]] name = "windows_aarch64_msvc" -version = "0.52.5" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9985fd1504e250c615ca5f281c3f7a6da76213ebd5ccc9561496568a2752afb6" +checksum = "09ec2a7bb152e2252b53fa7803150007879548bc709c039df7627cabbd05d469" [[package]] name = "windows_i686_gnu" @@ -2512,15 +2513,15 @@ checksum = "a75915e7def60c94dcef72200b9a8e58e5091744960da64ec734a6c6e9b3743e" [[package]] name = "windows_i686_gnu" -version = "0.52.5" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "88ba073cf16d5372720ec942a8ccbf61626074c6d4dd2e745299726ce8b89670" +checksum = "8e9b5ad5ab802e97eb8e295ac6720e509ee4c243f69d781394014ebfe8bbfa0b" [[package]] name = "windows_i686_gnullvm" -version = "0.52.5" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "87f4261229030a858f36b459e748ae97545d6f1ec60e5e0d6a3d32e0dc232ee9" +checksum = "0eee52d38c090b3caa76c563b86c3a4bd71ef1a819287c19d586d7334ae8ed66" [[package]] name = "windows_i686_msvc" @@ -2530,9 +2531,9 @@ checksum = "8f55c233f70c4b27f66c523580f78f1004e8b5a8b659e05a4eb49d4166cca406" [[package]] name = "windows_i686_msvc" -version = "0.52.5" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "db3c2bf3d13d5b658be73463284eaf12830ac9a26a90c717b7f771dfe97487bf" +checksum = "240948bc05c5e7c6dabba28bf89d89ffce3e303022809e73deaefe4f6ec56c66" [[package]] name = "windows_x86_64_gnu" @@ -2542,9 +2543,9 @@ checksum = "53d40abd2583d23e4718fddf1ebec84dbff8381c07cae67ff7768bbf19c6718e" [[package]] name = "windows_x86_64_gnu" -version = "0.52.5" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4e4246f76bdeff09eb48875a0fd3e2af6aada79d409d33011886d3e1581517d9" +checksum = "147a5c80aabfbf0c7d901cb5895d1de30ef2907eb21fbbab29ca94c5b08b1a78" [[package]] name = "windows_x86_64_gnullvm" @@ -2554,9 +2555,9 @@ checksum = "0b7b52767868a23d5bab768e390dc5f5c55825b6d30b86c844ff2dc7414044cc" [[package]] name = "windows_x86_64_gnullvm" -version = "0.52.5" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "852298e482cd67c356ddd9570386e2862b5673c85bd5f88df9ab6802b334c596" +checksum = "24d5b23dc417412679681396f2b49f3de8c1473deb516bd34410872eff51ed0d" [[package]] name = "windows_x86_64_msvc" @@ -2566,9 +2567,9 @@ checksum = "ed94fce61571a4006852b7389a063ab983c02eb1bb37b47f8272ce92d06d9538" [[package]] name = "windows_x86_64_msvc" -version = "0.52.5" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bec47e5bfd1bff0eeaf6d8b485cc1074891a197ab4225d504cb7a1ab88b02bf0" +checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" [[package]] name = "winnow" diff --git a/README.md b/README.md index cf3fc03..0a484df 100644 --- a/README.md +++ b/README.md @@ -20,6 +20,8 @@ Commands must be sent newline-separated, for more details see [Pixelflut](https: * `PX x y`: Get the color value of the pixel (x,y), e.g. `PX 10 10` * `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. Tipp: For most use-cases this is the most efficient format with 10 bytes per pixel ;) +* `PXMULTI...`: EXPERIMENTAL binary syncing of whole pixel areas. Please note that for performance reasons this will be copied 1:1 to the servers framebuffer. The server will just take the following bytes and memcpy it into the framebuffer, so the alpha channel doesn't matter and you might mess up the screen. This is intended for export-use, especially when syncing or combining multiple Pixelflut screens across multiple servers. +Note: This command needs to be enabled using the `binary-sync-pixels` feature * `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, e.g. `OFFSET 100 100` @@ -100,6 +102,8 @@ As of writing the following features are supported * `vnc` (enabled by default): Starts a VNC server, where users can connect to. Needs `libvncserver-dev` to be installed. Please note that the VNC server offers basically no latency, but consumes quite some CPU. * `alpha` (disabled by default): Respect alpha values during `PX` commands. Disabled by default as this can cause performance degradation. +* `binary-set-pixel` (enabled by default): Allows use of the `PB` command. +* `binary-sync-pixels`(disabled by default): Allows use of the `PXMULTI` command. To e.g. turn the VNC server off, build with diff --git a/breakwater-core/Cargo.toml b/breakwater-core/Cargo.toml index 665e770..7f5a0ab 100644 --- a/breakwater-core/Cargo.toml +++ b/breakwater-core/Cargo.toml @@ -11,6 +11,10 @@ repository.workspace = true const_format.workspace = true tokio.workspace = true +[dev-dependencies] +rstest.workspace = true + [features] alpha = [] -binary-commands = [] +binary-set-pixel = [] +binary-sync-pixels = [] diff --git a/breakwater-core/src/framebuffer.rs b/breakwater-core/src/framebuffer.rs index 61f7eda..db5c1f8 100644 --- a/breakwater-core/src/framebuffer.rs +++ b/breakwater-core/src/framebuffer.rs @@ -58,6 +58,46 @@ impl FrameBuffer { } } + /// We can *not* take an `&[u32]` for the pixel here, as `std::slice::from_raw_parts` requires the data to be + /// aligned. As the data already is stored in a buffer we can not guarantee it's correctly aligned, so let's just + /// treat the pixels as raw bytes. + /// + /// Returns the coordinates where we landed after filling + #[inline(always)] + pub fn set_multi(&self, start_x: usize, start_y: usize, pixels: &[u8]) -> (usize, usize) { + let starting_index = start_x + start_y * self.width; + let pixels_copied = self.set_multi_from_start_index(starting_index, pixels); + + let new_x = (start_x + pixels_copied) % self.width; + let new_y = start_y + (pixels_copied / self.width); + + (new_x, new_y) + } + + /// Returns the number of pixels copied + #[inline(always)] + pub fn set_multi_from_start_index(&self, starting_index: usize, pixels: &[u8]) -> usize { + let num_pixels = pixels.len() / 4; + + if starting_index + num_pixels > self.buffer.len() { + dbg!( + "Ignoring invalid set_multi call, which would exceed the screen", + starting_index, + num_pixels, + self.buffer.len() + ); + // We did not move + return 0; + } + + let starting_ptr = unsafe { self.buffer.as_ptr().add(starting_index) }; + let target_slice = + unsafe { slice::from_raw_parts_mut(starting_ptr as *mut u8, pixels.len()) }; + target_slice.copy_from_slice(pixels); + + num_pixels + } + pub fn get_buffer(&self) -> &[u32] { &self.buffer } @@ -67,3 +107,103 @@ impl FrameBuffer { unsafe { slice::from_raw_parts(self.buffer.as_ptr() as *const u8, len_in_bytes) } } } + +#[cfg(test)] +mod tests { + use super::*; + + use rstest::{fixture, rstest}; + + #[fixture] + fn fb() -> FrameBuffer { + // We keep the framebuffer so small, so that we can easily test all pixels in a test run + FrameBuffer::new(640, 480) + } + + #[rstest] + #[case(0, 0, 0)] + #[case(0, 0, 0xff0000)] + #[case(0, 0, 0x0000ff)] + #[case(0, 0, 0x12345678)] + pub fn test_roundtrip(fb: FrameBuffer, #[case] x: usize, #[case] y: usize, #[case] rgba: u32) { + fb.set(x, y, rgba); + assert_eq!(fb.get(x, y), Some(rgba)); + } + + #[rstest] + pub fn test_out_of_bounds(fb: FrameBuffer) { + assert_eq!(fb.get(usize::MAX, usize::MAX), None); + assert_eq!(fb.get(usize::MAX, usize::MAX), None); + } + + #[rstest] + pub fn test_set_multi_from_beginning(fb: FrameBuffer) { + let pixels = (0..10_u32).collect::>(); + let pixel_bytes: Vec = pixels.iter().flat_map(|p| p.to_le_bytes()).collect(); + + let (current_x, current_y) = fb.set_multi(0, 0, &pixel_bytes); + + assert_eq!(current_x, 10); + assert_eq!(current_y, 0); + + for x in 0..10 { + assert_eq!(fb.get(x as usize, 0), Some(x), "Checking pixel {x}"); + } + + // The next pixel must not have been colored + assert_eq!(fb.get(11, 0), Some(0)); + } + + #[rstest] + pub fn test_set_multi_in_the_middle(fb: FrameBuffer) { + let mut x = 10; + let mut y = 100; + + // Let's color exactly 3 lines and 42 pixels + let pixels = (0..3 * fb.width as u32 + 42).collect::>(); + let pixel_bytes: Vec = pixels.iter().flat_map(|p| p.to_le_bytes()).collect(); + let (current_x, current_y) = fb.set_multi(x, y, &pixel_bytes); + + assert_eq!(current_x, 52); + assert_eq!(current_y, 103); + + // Let's check everything has been colored + for rgba in 0..3 * fb.width as u32 + 42 { + assert_eq!(fb.get(x, y), Some(rgba)); + + x += 1; + if x >= fb.width { + x = 0; + y += 1; + } + } + + // Everything afterwards must have not been touched (let's check the next 10 lines) + for _ in 0..10 * fb.width as u32 { + assert_eq!(fb.get(x, y), Some(0)); + + x += 1; + if x >= fb.width { + x = 0; + y += 1; + } + } + } + + #[rstest] + pub fn test_set_multi_does_nothing_when_too_long(fb: FrameBuffer) { + let mut too_long = Vec::with_capacity(fb.width * fb.height * 4 /* pixels per byte */); + too_long.fill_with(|| 42_u8); + let (current_x, current_y) = fb.set_multi(1, 0, &too_long); + + // Should be unchanged + assert_eq!(current_x, 1); + assert_eq!(current_y, 0); + + for x in 0..fb.width { + for y in 0..fb.height { + assert_eq!(fb.get(x, y), Some(0)); + } + } + } +} diff --git a/breakwater-core/src/lib.rs b/breakwater-core/src/lib.rs index e3828e8..645513a 100644 --- a/breakwater-core/src/lib.rs +++ b/breakwater-core/src/lib.rs @@ -11,7 +11,7 @@ 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` +{}{}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") { @@ -19,11 +19,16 @@ if cfg!(feature = "alpha") { } 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") { +if cfg!(feature = "binary-set-pixel") { "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 { "" -} +}, +if cfg!(feature = "binary-sync-pixels") { + "PXMULTI...: EXPERIMENTAL binary syncing of whole pixel areas. Please note that for performance reasons this will be copied 1:1 to the servers framebuffer. The server will just take the following bytes and memcpy it into the framebuffer, so the alpha channel doesn't matter and you might mess up the screen. This is intended for export-use, especially when syncing or combining multiple Pixelflut screens across multiple servers\n" +} else { + "" +}, ).as_bytes(); pub const ALT_HELP_TEXT: &[u8] = b"Stop spamming HELP!\n"; diff --git a/breakwater-core/src/test_helpers/mock_tcp_stream.rs b/breakwater-core/src/test_helpers/mock_tcp_stream.rs index cbf06ee9..0764d4a 100644 --- a/breakwater-core/src/test_helpers/mock_tcp_stream.rs +++ b/breakwater-core/src/test_helpers/mock_tcp_stream.rs @@ -13,13 +13,20 @@ pub struct MockTcpStream { } impl MockTcpStream { - pub fn from_input(input: &str) -> Self { + pub fn from_string(input: &str) -> Self { MockTcpStream { read_data: input.as_bytes().to_vec(), write_data: Vec::new(), } } + pub fn from_bytes(input: Vec) -> Self { + MockTcpStream { + read_data: input, + write_data: Vec::new(), + } + } + pub fn get_output(self) -> String { String::from_utf8(self.write_data).unwrap() } diff --git a/breakwater-parser/Cargo.toml b/breakwater-parser/Cargo.toml index 35ba5d6..2116414 100644 --- a/breakwater-parser/Cargo.toml +++ b/breakwater-parser/Cargo.toml @@ -23,4 +23,7 @@ pixelbomber.workspace = true [features] alpha = [] -binary-commands = [] +binary-set-pixel = [] +binary-sync-pixels = [] + +default = ["binary-set-pixel"] diff --git a/breakwater-parser/src/original.rs b/breakwater-parser/src/original.rs index a9c8171..5f76b70 100644 --- a/breakwater-parser/src/original.rs +++ b/breakwater-parser/src/original.rs @@ -1,3 +1,5 @@ +#[cfg(feature = "binary-sync-pixels")] +use core::slice; use std::{ simd::{num::SimdUint, u32x8, Simd}, sync::Arc, @@ -14,11 +16,22 @@ pub(crate) const PB_PATTERN: u64 = string_to_number(b"PB\0\0\0\0\0\0"); pub(crate) const OFFSET_PATTERN: u64 = string_to_number(b"OFFSET \0\0"); pub(crate) const SIZE_PATTERN: u64 = string_to_number(b"SIZE\0\0\0\0"); pub(crate) const HELP_PATTERN: u64 = string_to_number(b"HELP\0\0\0\0"); +#[cfg(feature = "binary-sync-pixels")] +pub(crate) const PXMULTI_PATTERN: u64 = string_to_number(b"PXMULTI\0"); pub struct OriginalParser { connection_x_offset: usize, connection_y_offset: usize, fb: Arc, + #[cfg(feature = "binary-sync-pixels")] + remaining_pixel_sync: Option, +} + +#[cfg(feature = "binary-sync-pixels")] +#[derive(Debug)] +pub struct RemainingPixelSync { + current_index: usize, + bytes_remaining: usize, } impl OriginalParser { @@ -27,6 +40,8 @@ impl OriginalParser { connection_x_offset: 0, connection_y_offset: 0, fb, + #[cfg(feature = "binary-sync-pixels")] + remaining_pixel_sync: None, } } } @@ -39,6 +54,45 @@ impl Parser for OriginalParser { let mut i = 0; // We can't use a for loop here because Rust don't lets use skip characters by incrementing i let loop_end = buffer.len().saturating_sub(PARSER_LOOKAHEAD); // Let's extract the .len() call and the subtraction into it's own variable so we only compute it once + #[cfg(feature = "binary-sync-pixels")] + if let Some(remaining) = &self.remaining_pixel_sync { + let buffer = &buffer[0..loop_end]; + + if remaining.bytes_remaining <= buffer.len() { + // Easy going here + self.fb + .set_multi_from_start_index(remaining.current_index, unsafe { + slice::from_raw_parts(buffer.as_ptr(), remaining.bytes_remaining) + }); + i += remaining.bytes_remaining; + last_byte_parsed = i; + self.remaining_pixel_sync = None; + } else { + // The client requested to write more bytes that are currently in the buffer, we need to remember + // what the client is doing. + + // We need to round down to the 4 bytes of a pixel alignment + let pixel_bytes = buffer.len() / 4 * 4; + + let mut index = remaining.current_index; + index += self + .fb + .set_multi_from_start_index(remaining.current_index, unsafe { + slice::from_raw_parts(buffer.as_ptr(), pixel_bytes) + }); + + self.remaining_pixel_sync = Some(RemainingPixelSync { + current_index: index, + bytes_remaining: remaining.bytes_remaining.saturating_sub(pixel_bytes), + }); + + // Nothing to do left, we can early return + // I have absolutely no idea why we need to subtract 1 here, but it is what it is. At least we have + // tests for this madness :) + return i + pixel_bytes.saturating_sub(1); + } + } + while i < loop_end { let current_command = unsafe { (buffer.as_ptr().add(i) as *const u64).read_unaligned() }; @@ -141,10 +195,9 @@ impl Parser for OriginalParser { continue; } } - // In case the feature is disabled this if should be optimized away, as "cfg!" should be a constant expression. - } else if cfg!(feature = "binary-commands") - && current_command & 0x0000_ffff == PB_PATTERN - { + } + #[cfg(feature = "binary-set-pixel")] + if current_command & 0x0000_ffff == PB_PATTERN { let command_bytes = unsafe { (buffer.as_ptr().add(i + 2) as *const u64).read_unaligned() }; @@ -158,7 +211,53 @@ impl Parser for OriginalParser { last_byte_parsed = i + 1 + 2 + 2 + 4; i += 10; continue; - } else if current_command & 0x00ff_ffff_ffff_ffff == OFFSET_PATTERN { + } + #[cfg(feature = "binary-sync-pixels")] + if current_command & 0x00ff_ffff_ffff_ffff == PXMULTI_PATTERN { + i += "PXMULTI".len(); + let header = unsafe { (buffer.as_ptr().add(i) as *const u64).read_unaligned() }; + i += 8; + + let start_x = u16::from_le((header) as u16); + let start_y = u16::from_le((header >> 16) as u16); + let len = u32::from_le((header >> 32) as u32); + let len_in_bytes = len as usize * 4; + let bytes_left_in_buffer = loop_end.saturating_sub(i); + + if len_in_bytes <= bytes_left_in_buffer { + // Easy going here + self.fb + .set_multi(start_x as usize, start_y as usize, unsafe { + slice::from_raw_parts(buffer.as_ptr().add(i), len_in_bytes) + }); + + i += len_in_bytes; + last_byte_parsed = i; + continue; + } else { + // We need to round down to the 4 bytes of a pixel alignment + let pixel_bytes: usize = bytes_left_in_buffer / 4 * 4; + + // The client requested to write more bytes that are currently in the buffer, we need to remember + // what the client is doing. + let mut current_index = + start_x as usize + start_y as usize * self.fb.get_width(); + current_index += self.fb.set_multi_from_start_index(current_index, unsafe { + slice::from_raw_parts(buffer.as_ptr().add(i), pixel_bytes) + }); + + self.remaining_pixel_sync = Some(RemainingPixelSync { + current_index, + bytes_remaining: len_in_bytes - pixel_bytes, + }); + + // Nothing to do left, we can early return + // I have absolutely no idea why we need to subtract 1 here, but it is what it is. At least we have + // tests for this madness :) + return i + pixel_bytes.saturating_sub(1); + } + } + if current_command & 0x00ff_ffff_ffff_ffff == OFFSET_PATTERN { i += 7; let (x, y, present) = parse_pixel_coordinates(buffer.as_ptr(), &mut i); @@ -170,7 +269,8 @@ impl Parser for OriginalParser { self.connection_y_offset = y; continue; } - } else if current_command & 0xffff_ffff == SIZE_PATTERN { + } + if current_command & 0xffff_ffff == SIZE_PATTERN { i += 4; last_byte_parsed = i + 1; @@ -178,7 +278,8 @@ impl Parser for OriginalParser { format!("SIZE {} {}\n", self.fb.get_width(), self.fb.get_height()).as_bytes(), ); continue; - } else if current_command & 0xffff_ffff == HELP_PATTERN { + } + if current_command & 0xffff_ffff == HELP_PATTERN { i += 4; last_byte_parsed = i + 1; @@ -201,7 +302,8 @@ impl Parser for OriginalParser { i += 1; } - last_byte_parsed.wrapping_sub(1) + last_byte_parsed + // last_byte_parsed.saturating_sub(1) } fn parser_lookahead(&self) -> usize { diff --git a/breakwater-parser/src/refactored.rs b/breakwater-parser/src/refactored.rs index 4bdf336..2a4182d 100644 --- a/breakwater-parser/src/refactored.rs +++ b/breakwater-parser/src/refactored.rs @@ -204,7 +204,7 @@ impl Parser for RefactoredParser { unsafe { (buffer.as_ptr().add(i) as *const u64).read_unaligned() }; if current_command & 0x00ff_ffff == PX_PATTERN { (i, last_byte_parsed) = self.handle_pixel(buffer, i, response); - } else if cfg!(feature = "binary-commands") + } else if cfg!(feature = "binary-set-pixel") && current_command & 0x0000_ffff == PB_PATTERN { (i, last_byte_parsed) = self.handle_binary_pixel(buffer, i); diff --git a/breakwater/Cargo.toml b/breakwater/Cargo.toml index 810ea92..00ec4ee 100644 --- a/breakwater/Cargo.toml +++ b/breakwater/Cargo.toml @@ -37,8 +37,10 @@ vncserver = { workspace = true, optional = true } rstest.workspace = true [features] -default = ["vnc", "binary-commands"] +# We don't enable binary-sync-pixels by default to make it a bit harder for clients ;) +default = ["vnc", "binary-set-pixel"] vnc = ["dep:vncserver"] alpha = ["breakwater-core/alpha", "breakwater-parser/alpha"] -binary-commands = ["breakwater-core/binary-commands", "breakwater-parser/binary-commands"] +binary-set-pixel = ["breakwater-core/binary-set-pixel", "breakwater-parser/binary-set-pixel"] +binary-sync-pixels = ["breakwater-core/binary-sync-pixels", "breakwater-parser/binary-sync-pixels"] diff --git a/breakwater/src/server.rs b/breakwater/src/server.rs index d378228..7933d44 100644 --- a/breakwater/src/server.rs +++ b/breakwater/src/server.rs @@ -248,6 +248,15 @@ pub async fn handle_connection( // This happens, because last_byte_parsed is an index starting at 0, so index 6 is from an array of length 7 leftover_bytes_in_buffer = data_end.saturating_sub(last_byte_parsed).saturating_sub(1); + // dbg!( + // buffer.len(), + // last_byte_parsed, + // leftover_bytes_in_buffer, + // &buffer[..25], + // &buffer[last_byte_parsed.saturating_sub(5)..last_byte_parsed], + // &buffer[buffer.len().saturating_sub(5)..] + // ); + // There is no need to leave anything longer than a command can take // This prevents malicious clients from sending gibberish and the buffer not getting drained leftover_bytes_in_buffer = min(leftover_bytes_in_buffer, parser_lookahead); diff --git a/breakwater/src/tests.rs b/breakwater/src/tests.rs index abfcb1a..fabbabe 100644 --- a/breakwater/src/tests.rs +++ b/breakwater/src/tests.rs @@ -46,30 +46,8 @@ 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( - #[case] input: &str, - #[case] expected: &str, - ip: IpAddr, - fb: Arc, - statistics_channel: ( - mpsc::Sender, - mpsc::Receiver, - ), -) { - let mut stream = MockTcpStream::from_input(input); - handle_connection( - &mut stream, - ip, - fb, - statistics_channel.0, - page_size::get(), - DEFAULT_NETWORK_BUFFER_SIZE, - None, - ) - .await - .unwrap(); - - assert_eq!(expected, stream.get_output()); +async fn test_correct_responses_to_general_commands(#[case] input: &str, #[case] expected: &str) { + assert_returns(input.as_bytes(), expected).await; } #[rstest] @@ -113,73 +91,8 @@ 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( - #[case] input: &str, - #[case] expected: &str, - ip: IpAddr, - fb: Arc, - statistics_channel: ( - mpsc::Sender, - mpsc::Receiver, - ), -) { - let mut stream = MockTcpStream::from_input(input); - handle_connection( - &mut stream, - ip, - fb, - statistics_channel.0, - DEFAULT_NETWORK_BUFFER_SIZE, - page_size::get(), - None, - ) - .await - .unwrap(); - - assert_eq!(expected, stream.get_output()); -} - -#[cfg(feature = "binary-commands")] -#[rstest] -// No newline in between needed -#[case("PB\0\0\0\0\0\0\0\0PX 0 0\n", "PX 0 0 000000\n")] -#[case("PB\0\0\0\01234PX 0 0\n", "PX 0 0 313233\n")] -#[case("PB\0\0\0\0\0\0\0\0PB\0\0\0\01234PX 0 0\n", "PX 0 0 313233\n")] -#[case( - "PB\0\0\0\0\0\0\0\0PX 0 0\nPB\0\0\0\01234PX 0 0\n", - "PX 0 0 000000\nPX 0 0 313233\n" -)] -#[case("PB \0*\0____PX 32 42\n", "PX 32 42 5f5f5f\n")] -// Also test that there can be newlines in between -#[case( - "PB\0\0\0\0\0\0\0\0\nPX 0 0\nPB\0\0\0\01234\n\n\nPX 0 0\n", - "PX 0 0 000000\nPX 0 0 313233\n" -)] -#[tokio::test] -async fn test_binary_commands( - #[case] input: &str, - #[case] expected: &str, - ip: IpAddr, - fb: Arc, - statistics_channel: ( - mpsc::Sender, - mpsc::Receiver, - ), -) { - let mut stream = MockTcpStream::from_input(input); - handle_connection( - &mut stream, - ip, - fb, - statistics_channel.0, - DEFAULT_NETWORK_BUFFER_SIZE, - page_size::get(), - None, - ) - .await - .unwrap(); - - assert_eq!(expected, stream.get_output()); +async fn test_setting_pixel(#[case] input: &str, #[case] expected: &str) { + assert_returns(input.as_bytes(), expected).await; } #[rstest] @@ -195,7 +108,7 @@ async fn test_safe( mpsc::Receiver, ), ) { - let mut stream = MockTcpStream::from_input(input); + let mut stream = MockTcpStream::from_string(input); handle_connection( &mut stream, ip, @@ -270,7 +183,7 @@ async fn test_drawing_rect( } // Color the pixels - let mut stream = MockTcpStream::from_input(&fill_commands); + let mut stream = MockTcpStream::from_string(&fill_commands); handle_connection( &mut stream, ip, @@ -285,7 +198,7 @@ async fn test_drawing_rect( assert_eq!("", stream.get_output()); // Read the pixels again - let mut stream = MockTcpStream::from_input(&read_commands); + let mut stream = MockTcpStream::from_string(&read_commands); handle_connection( &mut stream, ip, @@ -300,7 +213,7 @@ async fn test_drawing_rect( assert_eq!(fill_commands, stream.get_output()); // We can also do coloring and reading in a single connection - let mut stream = MockTcpStream::from_input(&combined_commands); + let mut stream = MockTcpStream::from_string(&combined_commands); handle_connection( &mut stream, ip, @@ -315,7 +228,7 @@ async fn test_drawing_rect( assert_eq!(combined_commands_expected, stream.get_output()); // Check that nothing else was colored - let mut stream = MockTcpStream::from_input(&read_other_pixels_commands); + let mut stream = MockTcpStream::from_string(&read_other_pixels_commands); handle_connection( &mut stream, ip, @@ -329,3 +242,237 @@ async fn test_drawing_rect( .unwrap(); assert_eq!(read_other_pixels_commands_expected, stream.get_output()); } + +#[cfg(feature = "binary-set-pixel")] +#[rstest] +// No newline in between needed +#[case("PB\0\0\0\0\0\0\0\0PX 0 0\n", "PX 0 0 000000\n")] +#[case("PB\0\0\0\01234PX 0 0\n", "PX 0 0 313233\n")] +#[case("PB\0\0\0\0\0\0\0\0PB\0\0\0\01234PX 0 0\n", "PX 0 0 313233\n")] +#[case( + "PB\0\0\0\0\0\0\0\0PX 0 0\nPB\0\0\0\01234PX 0 0\n", + "PX 0 0 000000\nPX 0 0 313233\n" +)] +#[case("PB \0*\0____PX 32 42\n", "PX 32 42 5f5f5f\n")] +// Also test that there can be newlines in between +#[case( + "PB\0\0\0\0\0\0\0\0\nPX 0 0\nPB\0\0\0\01234\n\n\nPX 0 0\n", + "PX 0 0 000000\nPX 0 0 313233\n" +)] +#[tokio::test] +async fn test_binary_commands( + #[case] input: &str, + #[case] expected: &str, + ip: IpAddr, + fb: Arc, + statistics_channel: ( + mpsc::Sender, + mpsc::Receiver, + ), +) { + let mut stream = MockTcpStream::from_string(input); + handle_connection( + &mut stream, + ip, + fb, + statistics_channel.0, + DEFAULT_NETWORK_BUFFER_SIZE, + page_size::get(), + None, + ) + .await + .unwrap(); + + assert_eq!(expected, stream.get_output()); +} + +#[cfg(feature = "binary-sync-pixels")] +#[tokio::test] +async fn test_binary_sync_pixels() { + // Test byte conversion works + assert_returns("PX 0 0 42\nPX 0 0\n".as_bytes(), "PX 0 0 424242\n").await; + + // Don't set any pixels + let mut input = Vec::new(); + input.extend("PXMULTI".as_bytes()); + input.extend([ + 0, 0, /* startX */ + 0, 0, /* startY */ + 0, 0, 0, 0, /* length */ + ]); + input.extend("PX 0 0\n".as_bytes()); + assert_returns(&input, "PX 0 0 000000\n").await; + + // Set first 10 pixels + let mut input = Vec::new(); + input.extend("PXMULTI".as_bytes()); + input.extend(0_u16.to_le_bytes()); // x + input.extend(0_u16.to_le_bytes()); // y + input.extend(10_u32.to_le_bytes()); // length + for pixel in 0..10_u32 { + // Some alpha stuff going on (which I don't fully understand) + input.extend((pixel << 8).to_be_bytes()); + } + input.extend( + "PX 0 0\nPX 1 0\nPX 2 0\nPX 3 0\nPX 4 0\nPX 5 0\nPX 6 0\nPX 7 0\nPX 8 0\nPX 9 0\n" + .as_bytes(), + ); + assert_returns(&input, "PX 0 0 000000\nPX 1 0 000001\nPX 2 0 000002\nPX 3 0 000003\nPX 4 0 000004\nPX 5 0 000005\nPX 6 0 000006\nPX 7 0 000007\nPX 8 0 000008\nPX 9 0 000009\n").await; +} + +#[cfg(feature = "binary-sync-pixels")] +#[rstest] +#[tokio::test] +/// Try painting the very last pixel of the screen. There is only space for a single pixel left. +async fn test_binary_sync_pixels_last_pixel(fb: Arc) { + let mut input = Vec::new(); + let x = fb.get_width() as u16 - 1; + let y = fb.get_height() as u16 - 1; + input.extend("PXMULTI".as_bytes()); + input.extend(x.to_le_bytes()); // x + input.extend(y.to_le_bytes()); // y + input.extend(1_u32.to_le_bytes()); // length + input.extend(0x12345678_u32.to_be_bytes()); + + input.extend(format!("PX 0 0\nPX {} {y}\nPX {x} {y}\n", x - 1).as_bytes()); + assert_returns( + &input, + &format!( + "PX 0 0 000000\nPX {} {y} 000000\nPX {x} {y} 123456\n", + x - 1 + ), + ) + .await; +} + +#[cfg(feature = "binary-sync-pixels")] +#[rstest] +#[tokio::test] +/// Try painting some pixels in the middle of the screen +async fn test_binary_sync_pixels_in_the_middle(fb: Arc) { + let mut input = Vec::new(); + let mut expected = String::new(); + + let x = 42_u16; + let y = 13_u16; + let num_pixels = fb.get_width() as u32 + 10; + input.extend("PXMULTI".as_bytes()); + input.extend(x.to_le_bytes()); // x + input.extend(y.to_le_bytes()); // y + input.extend(num_pixels.to_le_bytes()); // length + + for rgba in 0..num_pixels { + input.extend((rgba << 8).to_be_bytes()); + } + + let mut rgba = 0_u32; + for x in 42..fb.get_width() { + input.extend(format!("PX {x} 13\n").as_bytes()); + expected += &format!("PX {x} 13 {rgba:06x}\n"); + rgba += 1; + } + + for x in 0..52 { + input.extend(format!("PX {x} 14\n").as_bytes()); + expected += &format!("PX {x} 14 {rgba:06x}\n"); + rgba += 1; + } + + input.extend("PX 52 14\n".as_bytes()); + expected += "PX 52 14 000000\n"; + + assert_returns(&input, &expected).await; +} + +#[cfg(feature = "binary-sync-pixels")] +#[rstest] +#[tokio::test] +/// Try painting too much pixels, so it overflows the framebuffer. +async fn test_binary_sync_pixels_exceeding_screen(fb: Arc) { + let mut input = Vec::new(); + let x = fb.get_width() as u16 - 1; + let y = fb.get_height() as u16 - 1; + input.extend("PXMULTI".as_bytes()); + input.extend(x.to_le_bytes()); // x + input.extend(y.to_le_bytes()); // y + input.extend(2_u32.to_le_bytes()); // length + input.extend(0x12345678_u32.to_be_bytes()); + input.extend(0x87654321_u32.to_be_bytes()); + + input.extend(format!("PX {x} {y}\n").as_bytes()); + // As we exceeded the screen nothing should have been set + assert_returns(&input, &format!("PX {x} {y} 000000\n")).await; +} + +#[cfg(feature = "binary-sync-pixels")] +#[rstest] +#[tokio::test] +/// Try painting more pixels that fit in the buffer. This checks if the parse correctly keeps track of the command +/// across multiple parse calls as the pixel screen send is bigger than the buffer. +async fn test_binary_sync_pixels_larger_than_buffer(fb: Arc) { + // let fb = Arc::new(FrameBuffer::new(50, 30)); // For testing + + let num_pixels = (fb.get_width() * fb.get_height()) as u32; + let pixel_bytes = num_pixels * 4 /* bytes per pixel */; + assert!( + pixel_bytes > DEFAULT_NETWORK_BUFFER_SIZE as u32 * 3, + "The number of bytes we send must be bigger than the network buffer size so that we test the wrapping. \ + We actually pick a bit more, just to be safe and do a few cycles. Additionally, in tests the number of bytes \ + read into the socket is actually around 2074 (and differs each run), so we should be really good here" + ); + + let mut input = Vec::new(); + let mut expected = String::new(); + input.extend("PXMULTI".as_bytes()); + input.extend(0_u16.to_le_bytes()); // x + input.extend(0_u16.to_le_bytes()); // y + input.extend(num_pixels.to_le_bytes()); // length + + for rgba in 0..num_pixels { + input.extend((rgba << 8).to_be_bytes()); + // input.extend((0xdeadbeef_u32).to_be_bytes()); // For testing + } + + let mut rgba = 0_u32; + // Watch out, we first iterate over y, than x + for y in 0..fb.get_height() { + for x in 0..fb.get_width() { + // rgba = 0xdeadbe_u32; // For testing + input.extend(format!("PX {x} {y}\n").as_bytes()); + expected += &format!("PX {x} {y} {rgba:06x}\n"); + rgba += 1; + } + } + + let mut stream = MockTcpStream::from_bytes(input.to_owned()); + handle_connection( + &mut stream, + ip(), + fb, + statistics_channel().0, + DEFAULT_NETWORK_BUFFER_SIZE, + page_size::get(), + None, + ) + .await + .unwrap(); + + assert_eq!(expected, stream.get_output()); +} + +async fn assert_returns(input: &[u8], expected: &str) { + let mut stream = MockTcpStream::from_bytes(input.to_owned()); + handle_connection( + &mut stream, + ip(), + fb(), + statistics_channel().0, + DEFAULT_NETWORK_BUFFER_SIZE, + page_size::get(), + None, + ) + .await + .unwrap(); + + assert_eq!(expected, stream.get_output()); +}