feat!: Support counting pixels (behind count-pixels feature)

CHANGELOG.md

@@ -4,11 +4,25 @@ All notable changes to this project will be documented in this file.
 
 ## [Unreleased]
 
+### Added
+
+- Added support for counting the number of pixels each IP has set and expose it as Prometheus metric
+  `breakwater_pixels`. Pixels are only counted when the `count-pixels` feature is explicitly
+  enabled, as it has a big performance impact! The feature `count-pixels` has been added as well,
+  which does a very crude approximation of pixels set ([#62])
+
+### Changed
+
+- BREAKING: The Prometheus metric `breakwater_frame` has been renamed to `breakwater_vnc_frame` and
+  is only exported when the `vnc` feature is enabled ([#62])
+
+[#62]: https://github.com/sbernauer/breakwater/pull/62
+
 ## [0.18.1] - 2025-05-02
 
 ### Fixed
 
-- Fix wrong `PXMULTI` command handling introduced in [#55] ([#60]).
+- Fix wrong `PXMULTI` command handling introduced in [#55] ([#60])
 
 [#60]: https://github.com/sbernauer/breakwater/pull/60
 

README.md

@@ -124,6 +124,10 @@ As of writing the following features are supported:
 * `alpha` (disabled by default): Respect alpha values during `PX` commands. Disabled by default as this can cause performance degradation.
 * `binary-set-pixel` (disabled by default): Allows use of the `PB` command.
 * `binary-sync-pixels`(disabled by default): Allows use of the `PXMULTI` command.
+* `count-pixels` (disabled by default): Count the number of pixels each IP has set and expose it as Prometheus metric `breakwater_pixels`.
+  Turning this feature on has a big performance impact!
+* `count-pixels-approx` (disabled by default): Like `count-pixels`, but instead of actually counting the pixels, we just
+  divide the number of bytes by the average bytes for a pixel SET command to get a very rough estimate.
 
 To e.g. turn the VNC server off, build with
 

breakwater-parser/Cargo.toml

@@ -27,5 +27,6 @@ rstest.workspace = true
 alpha = []
 binary-set-pixel = []
 binary-sync-pixels = []
+count-pixels = []
 
 default = []

breakwater-parser/benches/parsing.rs

@@ -110,11 +110,31 @@ fn invoke_benchmark(
     for parse_name in parser_names {
         c_group.bench_with_input(parse_name, &commands, |b, input| {
             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()),
+                "original" => OriginalParser::new(fb.clone()).parse(
+                    input,
+                    &mut Vec::new(),
+                    #[cfg(feature = "count-pixels")]
+                    &breakwater_parser::NoopSetPixelsCallback,
+                ),
+                "refactored" => RefactoredParser::new(fb.clone()).parse(
+                    input,
+                    &mut Vec::new(),
+                    #[cfg(feature = "count-pixels")]
+                    &breakwater_parser::NoopSetPixelsCallback,
+                ),
+                "memchr" => MemchrParser::new(fb.clone()).parse(
+                    input,
+                    &mut Vec::new(),
+                    #[cfg(feature = "count-pixels")]
+                    &breakwater_parser::NoopSetPixelsCallback,
+                ),
                 #[cfg(target_arch = "x86_64")]
-                "assembler" => AssemblerParser::new(fb.clone()).parse(input, &mut Vec::new()),
+                "assembler" => AssemblerParser::new(fb.clone()).parse(
+                    input,
+                    &mut Vec::new(),
+                    #[cfg(feature = "count-pixels")]
+                    &breakwater_parser::NoopSetPixelsCallback,
+                ),
                 _ => panic!("Parser implementation {parse_name} not known"),
             });
         });

breakwater-parser/src/assembler.rs

@@ -15,7 +15,12 @@ impl<FB: FrameBuffer> AssemblerParser<FB> {
 }
 
 impl<FB: FrameBuffer> Parser for AssemblerParser<FB> {
-    fn parse(&mut self, buffer: &[u8], _response: &mut Vec<u8>) -> usize {
+    fn parse(
+        &mut self,
+        buffer: &[u8],
+        _response: &mut Vec<u8>,
+        #[cfg(feature = "count-pixels")] _set_pixels_callback: &impl crate::SetPixelsCallback,
+    ) -> usize {
         let mut last_byte_parsed = 0;
 
         // This loop does nothing and should be seen as a placeholder

breakwater-parser/src/framebuffer/mod.rs

@@ -31,17 +31,34 @@ pub trait FrameBuffer {
     /// 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 set(
+        &self,
+        x: usize,
+        y: usize,
+        rgba: u32,
+        #[cfg(feature = "count-pixels")] set_pixels_callback: &impl crate::SetPixelsCallback,
+    );
 
     /// 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)]
-    fn set_multi(&self, start_x: usize, start_y: usize, pixels: &[u8]) -> (usize, usize) {
+    fn set_multi(
+        &self,
+        start_x: usize,
+        start_y: usize,
+        pixels: &[u8],
+        #[cfg(feature = "count-pixels")] set_pixels_callback: &impl crate::SetPixelsCallback,
+    ) -> (usize, usize) {
         let starting_index = start_x + start_y * self.get_width();
-        let pixels_copied = self.set_multi_from_start_index(starting_index, pixels);
+        let pixels_copied = self.set_multi_from_start_index(
+            starting_index,
+            pixels,
+            #[cfg(feature = "count-pixels")]
+            set_pixels_callback,
+        );
 
         let new_x = (start_x + pixels_copied) % self.get_width();
         let new_y = start_y + (pixels_copied / self.get_width());
@@ -50,7 +67,12 @@ pub trait FrameBuffer {
     }
 
     /// Returns the number of pixels copied
-    fn set_multi_from_start_index(&self, starting_index: usize, pixels: &[u8]) -> usize;
+    fn set_multi_from_start_index(
+        &self,
+        starting_index: usize,
+        pixels: &[u8],
+        #[cfg(feature = "count-pixels")] set_pixels_callback: &impl crate::SetPixelsCallback,
+    ) -> usize;
 
     /// As the pixel memory doesn't necessarily need to be aligned (think of using shared memory for
     /// that), we can only return it as a list of bytes, not a list of pixels.

breakwater-parser/src/framebuffer/shared_memory.rs

@@ -175,19 +175,33 @@ impl FrameBuffer for SharedMemoryFrameBuffer {
     }
 
     #[inline(always)]
-    fn set(&self, x: usize, y: usize, rgba: u32) {
+    fn set(
+        &self,
+        x: usize,
+        y: usize,
+        rgba: u32,
+        #[cfg(feature = "count-pixels")] set_pixels_callback: &impl crate::SetPixelsCallback,
+    ) {
         // See 'SimpleFrameBuffer::set' for performance consideration
         if x < self.width && y < self.height {
             let offset = (x + y * self.width) * FB_BYTES_PER_PIXEL;
             let pixel_ptr = unsafe { self.buffer.get_unchecked(offset).get() } as *mut u32;
 
             // The buffer coming from the shared memory might be unaligned!
             unsafe { pixel_ptr.write_unaligned(rgba) }
+
+            #[cfg(feature = "count-pixels")]
+            set_pixels_callback.pixels_set(1);
         }
     }
 
     #[inline(always)]
-    fn set_multi_from_start_index(&self, starting_index: usize, pixels: &[u8]) -> usize {
+    fn set_multi_from_start_index(
+        &self,
+        starting_index: usize,
+        pixels: &[u8],
+        #[cfg(feature = "count-pixels")] set_pixels_callback: &impl crate::SetPixelsCallback,
+    ) -> usize {
         let num_pixels = pixels.len() / FB_BYTES_PER_PIXEL;
 
         if starting_index + num_pixels > self.get_size() {
@@ -209,6 +223,13 @@ impl FrameBuffer for SharedMemoryFrameBuffer {
         let target_slice = unsafe { slice::from_raw_parts_mut(starting_ptr, pixels.len()) };
         target_slice.copy_from_slice(pixels);
 
+        #[cfg(feature = "count-pixels")]
+        set_pixels_callback.pixels_set(
+            num_pixels
+                .try_into()
+                .expect("More than u64::MAX pixels colored!"),
+        );
+
         num_pixels
     }
 

breakwater-parser/src/framebuffer/simple.rs

@@ -39,7 +39,13 @@ impl FrameBuffer for SimpleFrameBuffer {
     }
 
     #[inline(always)]
-    fn set(&self, x: usize, y: usize, rgba: u32) {
+    fn set(
+        &self,
+        x: usize,
+        y: usize,
+        rgba: u32,
+        #[cfg(feature = "count-pixels")] set_pixels_callback: &impl crate::SetPixelsCallback,
+    ) {
         // 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
@@ -50,11 +56,19 @@ impl FrameBuffer for SimpleFrameBuffer {
                 let ptr = self.buffer.as_ptr().add(x + y * self.width) as *mut u32;
                 *ptr = rgba;
             }
+
+            #[cfg(feature = "count-pixels")]
+            set_pixels_callback.pixels_set(1);
         }
     }
 
     #[inline(always)]
-    fn set_multi_from_start_index(&self, starting_index: usize, pixels: &[u8]) -> usize {
+    fn set_multi_from_start_index(
+        &self,
+        starting_index: usize,
+        pixels: &[u8],
+        #[cfg(feature = "count-pixels")] set_pixels_callback: &impl crate::SetPixelsCallback,
+    ) -> usize {
         let num_pixels = pixels.len() / FB_BYTES_PER_PIXEL;
 
         if starting_index + num_pixels > self.buffer.len() {
@@ -73,6 +87,13 @@ impl FrameBuffer for SimpleFrameBuffer {
             unsafe { slice::from_raw_parts_mut(starting_ptr as *mut u8, pixels.len()) };
         target_slice.copy_from_slice(pixels);
 
+        #[cfg(feature = "count-pixels")]
+        set_pixels_callback.pixels_set(
+            num_pixels
+                .try_into()
+                .expect("More than u64::MAX pixels colored!"),
+        );
+
         num_pixels
     }
 
@@ -107,7 +128,13 @@ mod tests {
         #[case] y: usize,
         #[case] rgba: u32,
     ) {
-        fb.set(x, y, rgba);
+        fb.set(
+            x,
+            y,
+            rgba,
+            #[cfg(feature = "count-pixels")]
+            &crate::NoopSetPixelsCallback,
+        );
         assert_eq!(fb.get(x, y), Some(rgba));
     }
 
@@ -122,7 +149,13 @@ mod tests {
         let pixels = (0..10_u32).collect::<Vec<_>>();
         let pixel_bytes: Vec<u8> = pixels.iter().flat_map(|p| p.to_le_bytes()).collect();
 
-        let (current_x, current_y) = fb.set_multi(0, 0, &pixel_bytes);
+        let (current_x, current_y) = fb.set_multi(
+            0,
+            0,
+            &pixel_bytes,
+            #[cfg(feature = "count-pixels")]
+            &crate::NoopSetPixelsCallback,
+        );
 
         assert_eq!(current_x, 10);
         assert_eq!(current_y, 0);
@@ -143,7 +176,13 @@ mod tests {
         // Let's color exactly 3 lines and 42 pixels
         let pixels = (0..3 * fb.width as u32 + 42).collect::<Vec<_>>();
         let pixel_bytes: Vec<u8> = pixels.iter().flat_map(|p| p.to_le_bytes()).collect();
-        let (current_x, current_y) = fb.set_multi(x, y, &pixel_bytes);
+        let (current_x, current_y) = fb.set_multi(
+            x,
+            y,
+            &pixel_bytes,
+            #[cfg(feature = "count-pixels")]
+            &crate::NoopSetPixelsCallback,
+        );
 
         assert_eq!(current_x, 52);
         assert_eq!(current_y, 103);
@@ -175,7 +214,13 @@ mod tests {
     pub fn test_set_multi_does_nothing_when_too_long(fb: SimpleFrameBuffer) {
         let mut too_long = Vec::with_capacity(fb.width * fb.height * FB_BYTES_PER_PIXEL);
         too_long.fill_with(|| 42_u8);
-        let (current_x, current_y) = fb.set_multi(1, 0, &too_long);
+        let (current_x, current_y) = fb.set_multi(
+            1,
+            0,
+            &too_long,
+            #[cfg(feature = "count-pixels")]
+            &crate::NoopSetPixelsCallback,
+        );
 
         // Should be unchanged
         assert_eq!(current_x, 1);

breakwater-parser/src/lib.rs

@@ -52,8 +52,30 @@ pub const ALT_HELP_TEXT: &[u8] = b"Stop spamming HELP!\n";
 
 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<u8>) -> usize;
+    fn parse(
+        &mut self,
+        buffer: &[u8],
+        response: &mut Vec<u8>,
+        #[cfg(feature = "count-pixels")] set_pixels_callback: &impl SetPixelsCallback,
+    ) -> usize;
 
     // 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;
 }
+#[cfg(feature = "count-pixels")]
+pub trait SetPixelsCallback {
+    /// This function is called for every pixel or set of pixels that are set.
+    ///
+    /// It get's one parameter `pixels`: The number of pixels set.
+    ///
+    /// For performance reasons it's neither `async` nor fallible.
+    fn pixels_set(&self, pixels: u64);
+}
+
+#[cfg(feature = "count-pixels")]
+pub struct NoopSetPixelsCallback;
+
+#[cfg(feature = "count-pixels")]
+impl SetPixelsCallback for NoopSetPixelsCallback {
+    fn pixels_set(&self, _pixels: u64) {}
+}

breakwater-parser/src/memchr.rs

@@ -13,7 +13,12 @@ impl<FB: FrameBuffer> MemchrParser<FB> {
 }
 
 impl<FB: FrameBuffer> Parser for MemchrParser<FB> {
-    fn parse(&mut self, buffer: &[u8], _response: &mut Vec<u8>) -> usize {
+    fn parse(
+        &mut self,
+        buffer: &[u8],
+        _response: &mut Vec<u8>,
+        #[cfg(feature = "count-pixels")] set_pixels_callback: &impl crate::SetPixelsCallback,
+    ) -> usize {
         let mut last_char_after_newline = 0;
         for newline in memchr::memchr_iter(b'\n', buffer) {
             // TODO Use get_unchecked everywhere
@@ -53,7 +58,13 @@ impl<FB: FrameBuffer> Parser for MemchrParser<FB> {
                         .parse()
                         .expect("rgba was not a number");
 
-                    self.fb.set(x as usize, y as usize, rgba);
+                    self.fb.set(
+                        x as usize,
+                        y as usize,
+                        rgba,
+                        #[cfg(feature = "count-pixels")]
+                        set_pixels_callback,
+                    );
                 }
                 _ => {
                     continue;