Merge pull request #1127 from pravdomil/claude/exciting-mccarthy-56ae16

crates/ui-egui/src/panels/effect_controls.rs

@@ -1437,7 +1437,11 @@ pub fn viewer_hook(
                 painter.rect_stroke(sw, 2.0, Stroke::new(1.0, Color32::WHITE), egui::StrokeKind::Outside);
             }
         }
+        // No frame to sample yet (it is still rendering, or its readback is pending): the click
+        // leaves the eyedropper armed instead of using it up without a colour.
+        let unsampled = pick.kind == "color" && keyer.is_none() && app.viewer_image.is_none();
         if resp.clicked()
+            && !unsampled
             && let Some(pos) = resp.interact_pointer_pos()
         {
             let c = map.to_comp(pos);

crates/ui-egui/tests/ui_effect_controls.rs

@@ -189,6 +189,43 @@ fn the_eyedropper_samples_under_the_pointer_in_a_region_of_interest() {
     assert!(c[0] > 0.98 && c[1] < 0.02 && c[2] < 0.02, "{c:?}");
 }
 
+/// A click before the viewer has a frame to sample (it is still rendering) leaves the eyedropper
+/// armed: it used the pick up without setting a colour.
+#[test]
+fn the_eyedropper_stays_armed_until_the_viewer_has_a_frame() {
+    // A large blurred frame, so the first render outlasts the click.
+    let mut s = Session::default();
+    s.execute("comp.new", json!({"name": "Slow", "width": 7680, "height": 4320, "frameRate": 30, "duration": 1})).unwrap();
+    let back = s.execute("layer.newSolid", json!({"name": "Back", "color": "#20c040"})).unwrap()["layer"].as_u64().unwrap();
+    s.execute("effect.apply", json!({"layers": [back], "effect": "Gaussian Blur"})).unwrap();
+    let top = s.execute("layer.newSolid", json!({"name": "Top", "color": "#ff0000", "width": 200, "height": 100})).unwrap()["layer"].as_u64().unwrap();
+    s.execute("effect.apply", json!({"layers": [top], "effect": "Color Control"})).unwrap();
+    let prop = layer(&s, top).effects().unwrap().groups().find(|g| g.match_id == "ec.control.color").and_then(|g| g.get("color")).map(|p| p.uid).unwrap();
+    s.execute("prop.set", json!({"layer": top, "prop": prop, "value": [0.0, 0.0, 1.0, 1.0]})).unwrap();
+    let colour = |h: &Harness<'_, EffectcraftApp>| {
+        let v = layer(&h.state().session, top).effects().unwrap().find(prop).unwrap().value.clone();
+        let KV::Color(c) = v else { panic!("{v:?}") };
+        c
+    };
+    let mut h = Harness::builder().with_size(egui::vec2(1700.0, 1100.0)).build_eframe(|_| EffectcraftApp::new(s));
+    h.state_mut().ui.fx_pick = Some(FxPick { kind: "color".into(), layer: top, prop, name: "Color".into() });
+    h.step();
+    let pos = effectcraft_ui_egui::panels::viewer::comp_to_screen(&h.ctx, [100.0, 100.0]).unwrap();
+    click(&mut h, pos);
+    // (A frame that made it in time was sampled: the colour below is checked either way.)
+    if h.state().ui.fx_pick.is_some() {
+        assert_eq!(colour(&h), [0.0, 0.0, 1.0, 1.0], "nothing sampled yet");
+        settle(&mut h);
+        click(&mut h, pos);
+        assert!(h.state().ui.fx_pick.is_none(), "the pick is used up");
+    }
+    let c = colour(&h);
+    let want = [0x20 as f64 / 255.0, 0xc0 as f64 / 255.0, 0x40 as f64 / 255.0];
+    for k in 0..3 {
+        assert!((c[k] - want[k]).abs() < 0.02, "{c:?} vs {want:?}");
+    }
+}
+
 /// Key Light's Screen Colour eyedropper picks from the effect's input: the shown frame is
 /// already keyed (the default screen colour takes most of a green), so sampling it would miss.
 #[test]

crates/ui-egui/tests/ui_timeline_shapes.rs

@@ -28,6 +28,18 @@ fn center(h: &Harness<'_, EffectcraftApp>, id: &str) -> Pos2 {
     pos2(e.rect[0] + e.rect[2] / 2.0, e.rect[1] + e.rect[3] / 2.0)
 }
 
+/// Step until the viewer's frame has rendered (frames render on background threads).
+fn settle(h: &mut Harness<'_, EffectcraftApp>) {
+    for _ in 0..600 {
+        h.step();
+        if h.state().frames.inflight() == 0 && h.state().frames.last_ms.lock().map(|v| *v > 0.0).unwrap_or(false) {
+            break;
+        }
+        std::thread::sleep(std::time::Duration::from_millis(10));
+    }
+    h.run_steps(2);
+}
+
 fn click_at(h: &mut Harness<'_, EffectcraftApp>, p: Pos2) {
     h.input_mut().events.push(Event::PointerMoved(p));
     h.step();
@@ -224,7 +236,7 @@ fn timeline_colour_eyedropper_samples_the_comp() {
     let mut h = harness(s);
     h.state_mut().ui.timeline.open_groups.extend(open);
     open_layer(&mut h, l);
-    h.run_steps(2);
+    settle(&mut h);
     click(&mut h, &format!("timeline.prop.{fill}.eyedropper"));
     assert_eq!(h.state().ui.fx_pick.as_ref().map(|p| (p.prop, p.kind.as_str())), Some((fill, "color")), "the eyedropper waits for a viewer click");
     let pos = effectcraft_ui_egui::panels::viewer::comp_to_screen(&h.ctx, [6.0, 6.0]).unwrap();