fix: one frame-callback chain per animated surface; hex never panics

src/backdrop.rs

@@ -21,8 +21,8 @@ use wayland_client::{
 use wayland_protocols::wp::viewporter::client::wp_viewport::WpViewport;
 
 use crate::lock::Lock;
-use crate::theme::Palette;
 use crate::theme::gl::{Frame, Paint};
+use crate::theme::{Pacer, Palette, Step};
 use crate::wayland::{State, Wayland};
 
 pub const NAMESPACE: &str = "valw-backdrop";
@@ -75,8 +75,7 @@ struct Surface {
     changed: Instant,
     /// A wallpaper palette being worked out on a thread.
     incoming: Option<mpsc::Receiver<Option<Palette>>>,
-    /// Frame callbacks since the last draw.
-    skipped: u32,
+    pacer: Pacer,
 }
 
 #[derive(Default)]
@@ -143,7 +142,7 @@ pub fn add(state: &mut State, qh: &QueueHandle<State>, output: &WlOutput) {
             to: palette,
             changed: now,
             incoming: None,
-            skipped: 0,
+            pacer: Pacer::new(DRAW_EVERY),
         });
     }
 }
@@ -193,27 +192,24 @@ impl Backdrop {
                 }
             },
         }
-        s.draw(qh);
+        let step = s.pacer.configure();
+        s.step(step, qh);
     }
 
     pub fn frame_done(&mut self, surface: &WlSurface, qh: &QueueHandle<State>) {
         if let Some(i) = self.index(surface) {
-            self.surfaces[i].draw(qh);
+            let s = &mut self.surfaces[i];
+            let step = s.pacer.frame();
+            s.step(step, qh);
         }
     }
 }
 
 impl Surface {
-    fn draw(&mut self, qh: &QueueHandle<State>) {
-        // Between draws, only ask for the next frame (an empty commit).
-        self.skipped += 1;
-        if self.skipped < DRAW_EVERY && self.last_frame.is_some() {
-            let surface = self.layer.wl_surface();
-            surface.frame(qh, FrameCallbackData(surface.clone()));
-            surface.commit();
-            return;
-        }
-        self.skipped = 0;
+    /// Every configure and frame callback: notice a pause (the overview was
+    /// closed) and pick up a new wallpaper palette, then draw or wait as
+    /// the pacer says.
+    fn step(&mut self, step: Step, qh: &QueueHandle<State>) {
         let now = Instant::now();
         if should_refresh(self.last_frame, now) && self.incoming.is_none() {
             let (tx, rx) = mpsc::channel();
@@ -237,11 +233,19 @@ impl Surface {
             }
             self.incoming = None;
         }
+        // Every callback counts, drawn or not: skipped frames are not a pause.
         self.last_frame = Some(now);
+        let surface = self.layer.wl_surface();
+        match step {
+            Step::Idle => {}
+            Step::Wait => {
+                surface.frame(qh, FrameCallbackData(surface.clone()));
+                surface.commit();
+            }
+            Step::Draw => {
                 let Some(paint) = &self.paint else {
                     return;
                 };
-        let surface = self.layer.wl_surface();
                 surface.frame(qh, FrameCallbackData(surface.clone()));
                 let frame = Frame {
                     time: (now - self.start).as_secs_f32(),
@@ -254,6 +258,8 @@ impl Surface {
                     tracing::warn!("backdrop on {}: {e:#}", self.name);
                 }
             }
+        }
+    }
 }
 
 #[cfg(test)]

src/picker/mod.rs

@@ -32,8 +32,8 @@ use wayland_protocols::wp::viewporter::client::wp_viewport::WpViewport;
 use self::layout::Rect;
 use crate::editor::text;
 use crate::error::{Cancelled, HintExt};
-use crate::theme::Palette;
 use crate::theme::gl::{Frame, Paint};
+use crate::theme::{Pacer, Palette, Step};
 use crate::wayland::{Output, State, Wayland};
 
 pub const NAMESPACE: &str = "valw-picker";
@@ -65,6 +65,8 @@ pub struct Picker {
     from: Palette,
     changed: Instant,
     overlay_dirty: bool,
+    /// One frame-callback chain, however often the surface is configured.
+    pacer: Pacer,
     shift: bool,
     outcome: Option<Option<usize>>,
     error: Option<anyhow::Error>,
@@ -136,6 +138,7 @@ pub fn run(wl: &mut Wayland, output: &Output) -> Result<Window> {
         from: first,
         changed: now,
         overlay_dirty: true,
+        pacer: Pacer::new(1),
         shift: false,
         outcome: None,
         error: None,
@@ -223,8 +226,10 @@ impl Picker {
             },
         }
         self.overlay_dirty = true;
+        if self.pacer.configure() == Step::Draw {
             self.draw(qh);
         }
+    }
 
     pub fn closed(&mut self, layer: &LayerSurface) {
         if self.is(layer.wl_surface()) {
@@ -234,7 +239,7 @@ impl Picker {
     }
 
     pub fn frame_done(&mut self, surface: &WlSurface, qh: &QueueHandle<State>) {
-        if self.is(surface) {
+        if self.is(surface) && self.pacer.frame() == Step::Draw {
             self.draw(qh);
         }
     }

src/theme/mod.rs

@@ -9,7 +9,7 @@ use image::RgbaImage;
 
 pub mod gl;
 
-/// Three colours, dark to light, as linear 0–1 RGB.
+/// Three colours, dark to light, as 0–1 sRGB.
 #[derive(Debug, Clone, Copy, PartialEq)]
 pub struct Palette {
     pub base: [f32; 3],
@@ -37,7 +37,7 @@ impl Palette {
 
 pub fn hex(s: &str) -> Option<[f32; 3]> {
     let s = s.trim().trim_start_matches('#');
-    if s.len() != 6 {
+    if s.len() != 6 || !s.is_ascii() {
         return None;
     }
     let v = |i: usize| {
@@ -48,6 +48,59 @@ pub fn hex(s: &str) -> Option<[f32; 3]> {
     Some([v(0)?, v(2)?, v(4)?])
 }
 
+/// What an animated surface does on a configure or a frame callback.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum Step {
+    /// Draw (and ask for the next frame).
+    Draw,
+    /// Only ask for the next frame (an empty commit).
+    Wait,
+    /// Nothing: a frame is already on its way.
+    Idle,
+}
+
+/// Frame-callback pacing for an animated surface: exactly one chain of
+/// callbacks, however often the surface is configured, drawing on every
+/// `every`-th callback.
+#[derive(Debug, Clone)]
+pub struct Pacer {
+    every: u32,
+    count: u32,
+    running: bool,
+}
+
+impl Pacer {
+    pub fn new(every: u32) -> Pacer {
+        Pacer {
+            every: every.max(1),
+            count: 0,
+            running: false,
+        }
+    }
+
+    /// A configure: draw now, unless the chain already runs.
+    pub fn configure(&mut self) -> Step {
+        if self.running {
+            Step::Idle
+        } else {
+            self.running = true;
+            self.count = 0;
+            Step::Draw
+        }
+    }
+
+    /// A frame callback arrived.
+    pub fn frame(&mut self) -> Step {
+        self.count += 1;
+        if self.count >= self.every {
+            self.count = 0;
+            Step::Draw
+        } else {
+            Step::Wait
+        }
+    }
+}
+
 /// A Noctalia community palette file at `mode`.
 pub fn from_noctalia_file(text: &str, mode: &str) -> Option<Palette> {
     let json: serde_json::Value = serde_json::from_str(text).ok()?;
@@ -273,6 +326,34 @@ mod tests {
         assert_eq!(theme_choice("not toml ["), None);
     }
 
+    #[test]
+    fn hex_rejects_non_ascii_without_panicking() {
+        assert_eq!(hex("aé€"), None);
+        assert_eq!(hex("#ééé"), None);
+    }
+
+    #[test]
+    fn one_frame_chain_per_surface() {
+        let mut p = Pacer::new(2);
+        assert_eq!(
+            p.configure(),
+            Step::Draw,
+            "first configure starts the chain"
+        );
+        assert_eq!(
+            p.configure(),
+            Step::Idle,
+            "a reconfigure doesn't start a second one"
+        );
+        assert_eq!(p.frame(), Step::Wait);
+        assert_eq!(p.frame(), Step::Draw);
+        assert_eq!(p.frame(), Step::Wait);
+        let mut every = Pacer::new(1);
+        every.configure();
+        assert_eq!(every.frame(), Step::Draw);
+        assert_eq!(every.frame(), Step::Draw);
+    }
+
     #[test]
     fn lerp_eases() {
         let a = Palette::DEFAULT;