Viewer: the eyedropper samples under the pointer in a region of interest (#284)

Effect eyedroppers (other than keyers', which pick from the effect's
input) sample the composite the viewer shows; a test covers that on
main. But they, and the Info panel, read the shown frame as if it
covered the whole comp. With a Region of Interest, or the Extended
Viewer of a 3D view, the frame covers only that area, so they read a
pixel somewhere else. They now map the point through the area the
frame covers (`shown_region`, `frame_pixel`).

Tests: the_eyedropper_samples_under_the_pointer_in_a_region_of_interest
(ui_effect_controls), eyedropper_samples_straight_colour (fx_widgets).

Co-Authored-By: Claude Opus 5.5 <[email protected]>

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

@@ -1167,7 +1167,7 @@ pub fn viewer_hook(
             if pick.kind == "color"
                 && keyer.is_none()
                 && let Some(img) = &app.viewer_image
-                && let Some(s) = fw::sample_frame(img, map.comp, c)
+                && let Some(s) = fw::sample_frame(img, super::viewer_tools::shown_region(&ctx, map.comp), c)
             {
                 let sw = Rect::from_min_size(hp + vec2(14.0, 14.0), vec2(26.0, 18.0));
                 painter.rect_filled(sw, 2.0, Color32::from_rgb((s[0] * 255.0) as u8, (s[1] * 255.0) as u8, (s[2] * 255.0) as u8));
@@ -1192,7 +1192,7 @@ pub fn viewer_hook(
                     ));
                 }
             } else if let Some(img) = &app.viewer_image
-                && let Some(s) = fw::sample_frame(img, map.comp, c)
+                && let Some(s) = fw::sample_frame(img, super::viewer_tools::shown_region(&ctx, map.comp), c)
             {
                 actions.push(("prop.set".into(), json!({"layer": pick.layer, "prop": pick.prop, "value": [s[0], s[1], s[2], 1.0]})));
             }

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

@@ -185,18 +185,23 @@ pub fn layer_to_comp(l2c: &Mat3, p: [f64; 2]) -> [f64; 2] {
     [v.x, v.y]
 }
 
-/// Straight RGBA (0..1) of the displayed frame at comp point `pt` (the frame may be rendered
-/// at a reduced resolution: `comp` is the comp size in pixels). `None` outside the frame.
-pub fn sample_frame(img: &egui::ColorImage, comp: [f32; 2], pt: [f64; 2]) -> Option<[f32; 4]> {
-    if comp[0] <= 0.0 || comp[1] <= 0.0 || pt[0] < 0.0 || pt[1] < 0.0 {
+/// The premultiplied pixel of the displayed frame at comp point `pt`. The frame covers
+/// `region` ([x, y, w, h] in comp pixels: the whole comp, or the region of interest or Extended
+/// Viewer area it was rendered for), maybe at a reduced resolution. `None` outside it.
+pub fn frame_pixel(img: &egui::ColorImage, region: [f64; 4], pt: [f64; 2]) -> Option<Color32> {
+    let [x, y, w, h] = region;
+    let (fx, fy) = ((pt[0] - x) / w, (pt[1] - y) / h);
+    if !(0.0..1.0).contains(&fx) || !(0.0..1.0).contains(&fy) {
         return None;
     }
-    let sx = (pt[0] / comp[0] as f64 * img.size[0] as f64).floor() as usize;
-    let sy = (pt[1] / comp[1] as f64 * img.size[1] as f64).floor() as usize;
-    if sx >= img.size[0] || sy >= img.size[1] {
-        return None;
-    }
-    let c = img.pixels[sy * img.size[0] + sx];
+    let sx = ((fx * img.size[0] as f64).floor() as usize).min(img.size[0].saturating_sub(1));
+    let sy = ((fy * img.size[1] as f64).floor() as usize).min(img.size[1].saturating_sub(1));
+    img.pixels.get(sy.checked_mul(img.size[0])?.checked_add(sx)?).copied()
+}
+
+/// Straight RGBA (0..1) of the displayed frame at comp point `pt` ([`frame_pixel`]).
+pub fn sample_frame(img: &egui::ColorImage, region: [f64; 4], pt: [f64; 2]) -> Option<[f32; 4]> {
+    let c = frame_pixel(img, region, pt)?;
     let a = c.a() as f32 / 255.0;
     if a <= 0.0 {
         return Some([0.0, 0.0, 0.0, 0.0]);
@@ -639,12 +644,20 @@ mod tests {
         img.pixels[4 + 3] = Color32::from_rgba_premultiplied(128, 64, 0, 128); // (3, 1), half alpha
         img.pixels[0] = Color32::from_rgb(255, 0, 0);
         // The frame is rendered at half resolution of an 8 × 4 comp.
-        let c = sample_frame(&img, [8.0, 4.0], [7.5, 3.0]).unwrap();
+        let comp = [0.0, 0.0, 8.0, 4.0];
+        let c = sample_frame(&img, comp, [7.5, 3.0]).unwrap();
         assert!((c[0] - 1.0).abs() < 0.01 && (c[1] - 0.5).abs() < 0.01 && c[2] == 0.0, "{c:?}");
         assert!((c[3] - 128.0 / 255.0).abs() < 1e-6);
-        assert_eq!(sample_frame(&img, [8.0, 4.0], [0.5, 0.5]), Some([1.0, 0.0, 0.0, 1.0]));
-        assert_eq!(sample_frame(&img, [8.0, 4.0], [8.0, 1.0]), None, "outside");
-        assert_eq!(sample_frame(&img, [8.0, 4.0], [-1.0, 1.0]), None);
+        assert_eq!(sample_frame(&img, comp, [0.5, 0.5]), Some([1.0, 0.0, 0.0, 1.0]));
+        assert_eq!(sample_frame(&img, comp, [8.0, 1.0]), None, "outside");
+        assert_eq!(sample_frame(&img, comp, [-1.0, 1.0]), None);
+        assert_eq!(sample_frame(&img, [0.0, 0.0, 0.0, 4.0], [0.0, 1.0]), None, "an empty region");
+        // A frame of the region of interest (16, 8) to (24, 12) of a bigger comp (#284: it was
+        // read as if it covered the whole comp).
+        let roi = [16.0, 8.0, 8.0, 4.0];
+        assert_eq!(sample_frame(&img, roi, [16.5, 8.5]), Some([1.0, 0.0, 0.0, 1.0]));
+        assert!(sample_frame(&img, roi, [23.5, 11.0]).is_some_and(|c| c[3] > 0.4 && c[3] < 0.6));
+        assert_eq!(sample_frame(&img, roi, [0.5, 0.5]), None, "outside the region");
     }
 
     #[test]

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

@@ -17,16 +17,9 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) {
     }
     let (rgba, xy) = match (app.pointer_comp, &app.viewer_image) {
         (Some([cx, cy]), Some(img)) => {
-            let comp = app.session.active_comp();
-            let (cw, ch) = comp.map(|c| (c.width as f32, c.height as f32)).unwrap_or((1.0, 1.0));
-            let sx = (cx / cw * img.size[0] as f32) as i64;
-            let sy = (cy / ch * img.size[1] as f32) as i64;
-            let px = if sx >= 0 && sy >= 0 && (sx as usize) < img.size[0] && (sy as usize) < img.size[1] {
-                Some(img.pixels[sy as usize * img.size[0] + sx as usize])
-            } else {
-                None
-            };
-            (px, Some((cx, cy)))
+            let comp = app.session.active_comp().map_or([1.0, 1.0], |c| [c.width as f32, c.height as f32]);
+            let region = super::viewer_tools::shown_region(ui.ctx(), comp);
+            (super::fx_widgets::frame_pixel(img, region, [cx as f64, cy as f64]), Some((cx, cy)))
         }
         _ => (None, None),
     };

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

@@ -325,6 +325,12 @@ fn texture_roi(ctx: &egui::Context) -> Option<[f64; 4]> {
     ctx.data(|d| d.get_temp::<Option<[f64; 4]>>(egui::Id::new("viewer-tex-roi"))).flatten()
 }
 
+/// The comp area ([x, y, w, h], comp pixels) the shown frame covers: its region of interest or
+/// Extended Viewer area, else the whole `comp` (width, height).
+pub(crate) fn shown_region(ctx: &egui::Context, comp: [f32; 2]) -> [f64; 4] {
+    texture_roi(ctx).unwrap_or([0.0, 0.0, comp[0] as f64, comp[1] as f64])
+}
+
 /// Show Snapshot: the stored snapshot is on screen (button or F5 held).
 pub(crate) fn showing_snapshot(app: &EffectcraftApp, ctx: &egui::Context) -> bool {
     let f5 = ctx.input(|i| i.key_down(egui::Key::F5)) && !ctx.egui_wants_keyboard_input();

crates/ui-egui/tests/ui_effect_controls.rs

@@ -167,6 +167,28 @@ fn crosshair_and_eyedropper_pick_from_the_viewer() {
     assert_eq!(c[3], 1.0);
 }
 
+/// #284: with a Region of Interest the shown frame covers only that region; the eyedropper
+/// samples the composite under the pointer there (it read the frame as if it covered the whole
+/// comp, so it sampled somewhere else).
+#[test]
+fn the_eyedropper_samples_under_the_pointer_in_a_region_of_interest() {
+    let (mut app, x) = app();
+    app.session.execute("view.setRegionOfInterest", json!({"rect": [0.0, 0.0, 320.0, 180.0]})).unwrap();
+    // (Not the red it will pick.)
+    app.session.execute("prop.set", json!({"layer": x.small, "prop": x.color, "value": [0.0, 0.0, 1.0, 1.0]})).unwrap();
+    let mut h = Harness::builder().with_size(egui::vec2(1700.0, 1100.0)).build_eframe(|_| app);
+    settle(&mut h);
+    h.state_mut().ui.fx_pick = Some(FxPick { kind: "color".into(), layer: x.small, prop: x.color, name: "Color".into() });
+    h.step();
+    // (200, 150) is on the red layer (it covers 120..520 × 80..280); read as a frame of the
+    // whole comp, the green background at (100, 75).
+    let pos = effectcraft_ui_egui::panels::viewer::comp_to_screen(&h.ctx, [200.0, 150.0]).unwrap();
+    click(&mut h, pos);
+    let v = layer(&h.state().session, x.small).effects().unwrap().find(x.color).unwrap().value.clone();
+    let KV::Color(c) = v else { panic!("{v:?}") };
+    assert!(c[0] > 0.98 && c[1] < 0.02 && c[2] < 0.02, "{c:?}");
+}
+
 /// 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]