Review follow-ups: per-strip lock-key hold, idle poll, fewer allocations

src-tauri/src/capture.rs

@@ -360,9 +360,11 @@ impl DisplayCapture {
     pub fn colors(&mut self, settings: &Settings) -> Result<Vec<Rgb>, String> {
         #[cfg(target_os = "windows")]
         {
+            // Once GDI has taken over (DXGI lost for good), retry far less often.
+            let retry_ms = if self.gdi_fallback { 2000 } else { 250 };
             if self.desktop.is_none()
                 && matches!(self.region, crate::capture_source::Source::Display {})
-                && self.retry.elapsed() >= std::time::Duration::from_millis(250)
+                && self.retry.elapsed() >= std::time::Duration::from_millis(retry_ms)
             {
                 self.retry = std::time::Instant::now();
                 self.desktop =

src-tauri/src/device.rs

@@ -21,6 +21,8 @@ pub struct Controller {
     pub detected: Option<(u8, u8)>,
     /// When the last frame was sent (`now_ms`).
     last_frame_ms: u64,
+    /// Rate this strip's engine runs at (see `set_output_fps`).
+    output_fps: u8,
 }
 static CLAIMED: std::sync::LazyLock<std::sync::Mutex<std::collections::HashSet<String>>> =
     std::sync::LazyLock::new(Default::default);
@@ -39,22 +41,26 @@ static HOLD_UNTIL_MS: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU6
 fn now_ms() -> u64 {
     EPOCH.elapsed().as_millis() as u64
 }
-/// Output rate the engine is running at. At 30 FPS or less a frame (33 ms apart)
-/// never overlaps the controller's ~27 ms indicator update, so no hold is needed.
-static OUTPUT_FPS: std::sync::atomic::AtomicU8 = std::sync::atomic::AtomicU8::new(60);
+/// At 30 FPS or less a frame (33 ms apart) never overlaps the controller's ~27 ms
+/// indicator update, so no hold is needed.
 const HOLD_ABOVE_FPS: u8 = 30;
 /// During a hold frames keep flowing at ~30 FPS (at least this far apart) instead
 /// of freezing: that cadence never overlaps an indicator update.
 const HELD_FRAME_MS: u64 = 32;
-pub fn set_output_fps(fps: u8) {
-    OUTPUT_FPS.store(fps, std::sync::atomic::Ordering::Relaxed);
-}
+/// Until when some strip runs above HOLD_ABOVE_FPS (`now_ms`); the lock-key poll
+/// idles otherwise. Each strip has its own rate, so this only ever extends.
+static FAST_UNTIL_MS: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0);
 #[cfg_attr(not(target_os = "windows"), allow(dead_code))]
 pub fn hold_output(ms: u64) {
     HOLD_UNTIL_MS.fetch_max(now_ms() + ms, std::sync::atomic::Ordering::Relaxed);
 }
-fn held_for() -> Option<Duration> {
-    if OUTPUT_FPS.load(std::sync::atomic::Ordering::Relaxed) <= HOLD_ABOVE_FPS {
+/// Whether any strip currently needs the lock-key guard.
+#[cfg_attr(not(target_os = "windows"), allow(dead_code))]
+pub fn guard_needed() -> bool {
+    FAST_UNTIL_MS.load(std::sync::atomic::Ordering::Relaxed) > now_ms()
+}
+fn held_for(fps: u8) -> Option<Duration> {
+    if fps <= HOLD_ABOVE_FPS {
         return None;
     }
     let until = HOLD_UNTIL_MS.load(std::sync::atomic::Ordering::Relaxed);
@@ -171,6 +177,13 @@ pub fn lighting_interface(interface: i32, usage_page: u16, path: &str) -> bool {
         && usage_page != 1
         && (interface == 0 || path.to_ascii_lowercase().contains("mi_00") || usage_page >= 0xff00)
 }
+/// (LED count, inches) from a device-info (0x82) reply, None for other reports.
+fn parse_info(reply: &[u8]) -> Option<(u8, u8)> {
+    // Tolerate a leading report-ID byte on some platforms.
+    let o = usize::from(reply.first() == Some(&0));
+    (reply.len() >= o + 24 && reply[o] == 0x52 && reply[o + 1] == 0x42 && reply[o + 4] == 0x82)
+        .then(|| (reply[o + 11], reply[o + 8]))
+}
 fn write_reports(
     packet: &[u8],
     mut write: impl FnMut(&[u8]) -> Result<usize, String>,
@@ -199,12 +212,20 @@ impl Controller {
             sequence: 0,
             detected: None,
             last_frame_ms: 0,
+            output_fps: 60,
             status: Status {
                 connected: false,
                 message: "Ready to connect".into(),
             },
         }
     }
+    /// Output rate of this strip's engine; above 30 FPS lock keys hold frames.
+    pub fn set_output_fps(&mut self, fps: u8) {
+        self.output_fps = fps;
+        if fps > HOLD_ABOVE_FPS {
+            FAST_UNTIL_MS.fetch_max(now_ms() + 1000, std::sync::atomic::Ordering::Relaxed);
+        }
+    }
     fn id(&mut self) -> u8 {
         self.sequence = self.sequence % 254 + 1;
         self.sequence
@@ -300,13 +321,7 @@ impl Controller {
             let mut buf = [0u8; 65];
             for _ in 0..5 {
                 let n = d.read_timeout(&mut buf, 100).ok()?;
-                if n == 0 {
-                    continue;
-                }
-                // Tolerate a leading report-ID byte on some platforms.
-                let o = if buf[0] == 0 { 1 } else { 0 };
-                if n >= o + 24 && buf[o] == 0x52 && buf[o + 1] == 0x42 && buf[o + 4] == 0x82 {
-                    let (leds, inches) = (buf[o + 11], buf[o + 8]);
+                if let Some((leds, inches)) = parse_info(&buf[..n]) {
                     return (1..=127).contains(&leds).then_some((leds, inches));
                 }
             }
@@ -377,7 +392,7 @@ impl Controller {
         // skipping it (re-checked: a held key keeps extending the window), but never
         // stall the engine for more than a second.
         let deadline = std::time::Instant::now() + Duration::from_secs(1);
-        while let Some(wait) = held_for() {
+        while let Some(wait) = held_for(self.output_fps) {
             if std::time::Instant::now() >= deadline {
                 break;
             }
@@ -389,7 +404,9 @@ impl Controller {
     pub fn frame(&mut self, colors: &[Rgb]) -> Result<(), String> {
         // Frames stream continuously; while a lock key is active drop down to ~30 FPS.
         let now = now_ms();
-        if held_for().is_some() && now.saturating_sub(self.last_frame_ms) < HELD_FRAME_MS {
+        if held_for(self.output_fps).is_some()
+            && now.saturating_sub(self.last_frame_ms) < HELD_FRAME_MS
+        {
             return Ok(());
         }
         self.last_frame_ms = now;
@@ -426,6 +443,32 @@ mod tests {
         assert!(!lighting_interface(-1, 1, "x"));
     }
     #[test]
+    fn device_info_reply_with_and_without_report_id() {
+        let mut reply = [0u8; 24];
+        reply[..5].copy_from_slice(&[0x52, 0x42, 24, 1, 0x82]);
+        reply[8] = 27;
+        reply[11] = 94;
+        assert_eq!(parse_info(&reply), Some((94, 27)));
+        let mut prefixed = vec![0];
+        prefixed.extend_from_slice(&reply);
+        assert_eq!(parse_info(&prefixed), Some((94, 27)));
+        assert_eq!(parse_info(&reply[..23]), None);
+        reply[4] = 0x93;
+        assert_eq!(parse_info(&reply), None);
+        assert_eq!(parse_info(&[]), None);
+    }
+    #[test]
+    fn lock_keys_hold_only_fast_strips() {
+        // No global hold here: other tests write frames through test buses.
+        assert!(held_for(30).is_none());
+        let mut slow = Controller::new();
+        slow.set_output_fps(30);
+        let mut fast = Controller::new();
+        fast.set_output_fps(60);
+        assert!(guard_needed());
+        assert_eq!((slow.output_fps, fast.output_fps), (30, 60));
+    }
+    #[test]
     fn incomplete_write_stops_remaining_reports() {
         let packet = protocol::frame(1, &[[0, 0, 0]; 65]);
         let mut calls = 0;

src-tauri/src/engine.rs

@@ -1640,24 +1640,31 @@ impl Engine {
                         self.external_cue = None;
                     }
                     self.routines.overlay(&mut colors);
-                    let mut output_settings = self.state.settings.clone();
-                    output_settings.brightness =
-                        self.ambient.brightness(output_settings.brightness, dt);
-                    output_settings.brightness = (u16::from(output_settings.brightness)
-                        * u16::from(self.state.group_brightness)
+                    let s = &self.state.settings;
+                    let mut brightness = self.ambient.brightness(s.brightness, dt);
+                    brightness = (u16::from(brightness) * u16::from(self.state.group_brightness)
                         / 100) as u8;
-                    output_settings.minimum_glow = (u16::from(output_settings.minimum_glow)
+                    let mut minimum_glow = (u16::from(s.minimum_glow)
                         * u16::from(self.state.group_brightness)
                         / 100) as u8;
                     if let Some((start, length, off)) = self.fade {
                         let fraction =
                             (start.elapsed().as_secs_f32() / length.as_secs_f32()).clamp(0.0, 1.0);
                         let gain = if off { 1.0 - fraction } else { fraction };
-                        output_settings.brightness =
-                            (f32::from(output_settings.brightness) * gain).round() as u8;
-                        output_settings.minimum_glow =
-                            (f32::from(output_settings.minimum_glow) * gain).round() as u8;
+                        brightness = (f32::from(brightness) * gain).round() as u8;
+                        minimum_glow = (f32::from(minimum_glow) * gain).round() as u8;
                     }
+                    // Clone the settings (strings, zones, palettes) only when output differs.
+                    let output_settings =
+                        if (brightness, minimum_glow) == (s.brightness, s.minimum_glow) {
+                            std::borrow::Cow::Borrowed(s)
+                        } else {
+                            std::borrow::Cow::Owned(Settings {
+                                brightness,
+                                minimum_glow,
+                                ..s.clone()
+                            })
+                        };
                     self.previous =
                         lighting::transform(&colors, &self.previous, &output_settings, dt);
                 }
@@ -1812,7 +1819,7 @@ pub fn start(
                     );
                     let began = Instant::now();
                     let fps = crate::device::effective_fps(engine.state.settings.fps);
-                    crate::device::set_output_fps(fps);
+                    engine.controller.set_output_fps(fps);
                     let period = Duration::from_secs_f64(1.0 / fps as f64);
                     let late = began.saturating_duration_since(next);
                     if engine.state.powered

src-tauri/src/lifecycle_windows.rs

@@ -20,7 +20,8 @@ const LOCK_KEYS: [i32; 3] = [0x14, 0x90, 0x91]; // VK_CAPITAL, VK_NUMLOCK, VK_SC
 /// Guards the strip against Windows' keyboard-indicator reports (see device::hold_output).
 /// Two independent detectors: a low-level hook on its own thread, so nothing else can
 /// stall it past the hook timeout (Windows then silently removes it), and a 4 ms poll
-/// of the physical key state as a backstop for windows the hook does not see.
+/// of the physical key state as a backstop for windows the hook does not see. The
+/// poll idles while no strip runs above 30 FPS, where frames never collide.
 pub fn guard_lock_keys() {
     let _ = std::thread::Builder::new()
         .name("lock-key-hook".into())
@@ -36,6 +37,10 @@ pub fn guard_lock_keys() {
     let _ = std::thread::Builder::new()
         .name("lock-key-poll".into())
         .spawn(|| loop {
+            if !crate::device::guard_needed() {
+                std::thread::sleep(std::time::Duration::from_millis(100));
+                continue;
+            }
             if LOCK_KEYS
                 .iter()
                 .any(|&key| unsafe { GetAsyncKeyState(key) } < 0)