Merge remote-tracking branch 'pr/1328' into HEAD

crates/ui-egui/src/app/frame.rs

@@ -378,6 +378,7 @@ impl EffectcraftApp {
         // Video Preview is a second viewport: only a real pass can update it.
         prefs_live::frame(self, &ctx);
         self.auto.begin_frame();
+        panels::viewer_tools::clear_sample_source(&ctx);
         if let Some(rect) = crate::color_picker::take_automation(&ctx) {
             self.auto.add("colorPicker.eyedropper", rect, "Eyedropper");
         }
@@ -409,6 +410,7 @@ impl EffectcraftApp {
         let header = egui::Rect::from_min_size(egui::pos2(full.min.x, top), egui::vec2(full.width(), header_h));
         header::show(self, ui, header);
         let body = egui::Rect::from_min_max(egui::pos2(full.min.x + 4.0, header.max.y + 2.0), egui::pos2(full.max.x - 4.0, full.max.y - 4.0));
+        crate::color_picker::note_dialog(&ctx, self.dialog);
         self.dock_area(ui, body);
         #[cfg(not(target_arch = "wasm32"))]
         prefs_live::dropped_files(self, ui, full);

crates/ui-egui/src/color_picker.rs

@@ -122,12 +122,27 @@ struct Armed {
     popup: egui::Id,
     pass: u64,
     sample: Option<[f32; 3]>,
+    dialog: Option<crate::Dialog>,
 }
 
 fn armed_key() -> egui::Id {
     egui::Id::unique("color-picker-eyedropper")
 }
 
+fn dialog_key() -> egui::Id {
+    egui::Id::unique("color-picker-dialog")
+}
+
+/// Record the modal before drawing the viewer. A picker may sample behind its own dialog, but
+/// a new, unrelated modal must block it from its very first pass, before its rect is known.
+pub(crate) fn note_dialog(ctx: &egui::Context, dialog: Option<crate::Dialog>) {
+    ctx.data_mut(|data| data.insert_temp(dialog_key(), dialog));
+}
+
+fn current_dialog(ctx: &egui::Context) -> Option<crate::Dialog> {
+    ctx.data(|data| data.get_temp::<Option<crate::Dialog>>(dialog_key())).flatten()
+}
+
 /// The armed eyedropper, if its picker was drawn in this pass or the last: one whose picker stops
 /// being drawn (its dialog gone) lapses.
 fn armed_record(ctx: &egui::Context) -> Option<Armed> {
@@ -147,7 +162,8 @@ pub fn eyedropper_armed(ctx: &egui::Context) -> bool {
 
 fn arm(ctx: &egui::Context, popup: egui::Id) {
     let pass = ctx.cumulative_pass_nr();
-    ctx.data_mut(|d| d.insert_temp(armed_key(), Armed { popup, pass, sample: None }));
+    let dialog = current_dialog(ctx);
+    ctx.data_mut(|d| d.insert_temp(armed_key(), Armed { popup, pass, sample: None, dialog }));
 }
 
 fn disarm(ctx: &egui::Context, popup: egui::Id) {
@@ -218,6 +234,9 @@ fn overlaid(ctx: &egui::Context, pos: egui::Pos2) -> bool {
 /// dialog. The pointer is read rather than the viewer's responses, since a modal dialog over the
 /// viewer takes its clicks.
 pub fn eyedropper_point(ctx: &egui::Context) -> Option<egui::Pos2> {
+    if armed_record(ctx).is_some_and(|armed| armed.dialog != current_dialog(ctx)) {
+        return None;
+    }
     let viewer: Option<(u64, Rect)> = ctx.data(|d| d.get_temp(viewer_key()));
     let viewer = viewer.filter(|(p, _)| p.saturating_add(1) >= ctx.cumulative_pass_nr())?.1;
     let pos = ctx.input(|i| i.pointer.interact_pos())?;
@@ -646,6 +665,101 @@ pub fn hsv_to_rgb([h, s, v]: [f32; 3]) -> [f32; 3] {
 mod tests {
     use super::*;
 
+    fn sampling_viewer() -> egui_kittest::Harness<'static, crate::EffectcraftApp> {
+        let mut session = effectcraft_engine::Session::default();
+        session.execute("comp.new", serde_json::json!({"width": 320, "height": 180, "duration": 2})).unwrap();
+        session.execute("layer.newSolid", serde_json::json!({"color": "#204060"})).unwrap();
+        let mut harness = egui_kittest::Harness::builder().with_size(vec2(1200.0, 800.0)).build_eframe(|_| crate::EffectcraftApp::new(session));
+        settle_sampling_viewer(&mut harness);
+        harness
+    }
+
+    fn settle_sampling_viewer(harness: &mut egui_kittest::Harness<'_, crate::EffectcraftApp>) {
+        for _ in 0..200 {
+            harness.step();
+            if harness.state().frames.inflight() == 0 && harness.state().viewer_image.is_some() {
+                harness.run_steps(3);
+                return;
+            }
+            std::thread::sleep(std::time::Duration::from_millis(5));
+        }
+        panic!("the synthetic viewer frame did not arrive");
+    }
+
+    fn press_sampling_viewer(harness: &mut egui_kittest::Harness<'_, crate::EffectcraftApp>) -> Option<[f32; 3]> {
+        let ctx = harness.ctx.clone();
+        arm(&ctx, egui::Id::unique("test-popup"));
+        let point = crate::panels::viewer::comp_to_screen(&ctx, [20.0, 20.0]).unwrap();
+        harness.event(egui::Event::PointerMoved(point));
+        harness.event(egui::Event::PointerButton { pos: point, button: egui::PointerButton::Primary, pressed: true, modifiers: egui::Modifiers::NONE });
+        harness.step();
+        let sample = ctx.data(|data| data.get_temp::<Armed>(armed_key())).and_then(|armed| armed.sample);
+        harness.event(egui::Event::PointerButton { pos: point, button: egui::PointerButton::Primary, pressed: false, modifiers: egui::Modifiers::NONE });
+        harness.step();
+        sample
+    }
+
+    #[test]
+    fn popup_eyedropper_samples_the_displayed_channel_and_exposure() {
+        let mut harness = sampling_viewer();
+        harness.state_mut().session.state.viewer.channel = effectcraft_engine::viewer::Channel::Green;
+        let green = press_sampling_viewer(&mut harness).unwrap();
+        assert!(green.iter().all(|value| (*value - 64.0 / 255.0).abs() < 0.01), "{green:?}");
+        let options = &mut harness.state_mut().session.state.viewer;
+        options.channel = effectcraft_engine::viewer::Channel::Rgb;
+        options.exposure = 1.0;
+        let exposed = press_sampling_viewer(&mut harness).unwrap();
+        let mut expected = [[32, 64, 96, 255]];
+        effectcraft_engine::viewer::display_transform(&mut expected, effectcraft_engine::viewer::Channel::Rgb, false, 1.0);
+        for (actual, expected) in exposed.iter().zip(expected[0]) {
+            assert!((*actual - expected as f32 / 255.0).abs() < 0.01, "{exposed:?}");
+        }
+    }
+
+    #[test]
+    fn popup_eyedropper_samples_snapshot_and_refuses_wireframe() {
+        let mut harness = sampling_viewer();
+        harness.state_mut().session.execute("view.takeSnapshot", serde_json::json!({})).unwrap();
+        harness.state_mut().session.execute("layer.newSolid", serde_json::json!({"color": "#a0b0c0"})).unwrap();
+        settle_sampling_viewer(&mut harness);
+        harness.event(egui::Event::Key { key: egui::Key::F5, physical_key: None, pressed: true, repeat: false, modifiers: egui::Modifiers::NONE });
+        // Let the snapshot texture's discard passes finish before arming this synthetic picker.
+        harness.step();
+        let snapshot = press_sampling_viewer(&mut harness).unwrap();
+        for (actual, expected) in snapshot.iter().zip([32, 64, 96]) {
+            assert!((*actual - expected as f32 / 255.0).abs() < 0.01, "{snapshot:?}");
+        }
+        let options = &mut harness.state_mut().session.state.viewer;
+        options.show_snapshot = false;
+        options.fast_previews = effectcraft_engine::commands::viewer_cmds::FastPreviews::Wireframe;
+        harness.event(egui::Event::Key { key: egui::Key::F5, physical_key: None, pressed: false, repeat: false, modifiers: egui::Modifiers::NONE });
+        assert_eq!(press_sampling_viewer(&mut harness), None, "wireframe must not sample an old pixel frame");
+    }
+
+    #[test]
+    fn a_new_modal_backdrop_cannot_deliver_a_pending_sample() {
+        let mut harness = sampling_viewer();
+        harness.state_mut().dialog = Some(crate::Dialog::About);
+        assert_eq!(press_sampling_viewer(&mut harness), None, "the first modal pass must already exclude its backdrop");
+    }
+
+    #[test]
+    fn a_picker_can_sample_behind_its_own_dialog_but_not_a_new_modal() {
+        let ctx = egui::Context::default();
+        let point = pos2(20.0, 20.0);
+        let mut output = ctx.run_ui(egui::RawInput { events: vec![egui::Event::PointerMoved(point)], ..Default::default() }, |ui| {
+            let ctx = ui.ctx();
+            note_dialog(ctx, Some(crate::Dialog::SolidSettings));
+            note_viewer(ctx, Rect::from_min_size(pos2(0.0, 0.0), vec2(320.0, 180.0)));
+            note_overlay(ctx, Rect::from_min_size(pos2(100.0, 100.0), vec2(120.0, 80.0)));
+            arm(ctx, egui::Id::unique("test-picker"));
+            assert_eq!(eyedropper_point(ctx), Some(point));
+            note_dialog(ctx, Some(crate::Dialog::About));
+            assert_eq!(eyedropper_point(ctx), None);
+        });
+        output.textures_delta.clear();
+    }
+
     #[test]
     fn armed_eyedropper_takes_one_sample_then_disarms() {
         let ctx = egui::Context::default();

crates/ui-egui/src/frames.rs

@@ -539,18 +539,16 @@ impl Default for Frames {
 /// Premultiplied f32 → egui premultiplied Color32 (alpha kept, so the viewer can show the
 /// transparency grid or the comp background colour underneath).
 pub fn to_color_image(img: &effectcraft_engine::render::Image) -> egui::ColorImage {
-    let px: Vec<egui::Color32> = img
-        .data
-        .par_iter()
-        .map(|p| {
-            let a = p[3].clamp(0.0, 1.0);
-            let c = |v: f32| (v.clamp(0.0, a) * 255.0 + 0.5) as u8;
-            egui::Color32::from_rgba_premultiplied(c(p[0]), c(p[1]), c(p[2]), (a * 255.0 + 0.5) as u8)
-        })
-        .collect();
+    let px: Vec<egui::Color32> = img.data.par_iter().map(|p| display_pixel(*p)).collect();
     egui::ColorImage::new([img.width as usize, img.height as usize], px)
 }
 
+pub(crate) fn display_pixel(pixel: [f32; 4]) -> egui::Color32 {
+    let alpha = if pixel[3].is_nan() { 0.0 } else { pixel[3].clamp(0.0, 1.0) };
+    let component = |value: f32| (value.clamp(0.0, alpha) * 255.0 + 0.5) as u8;
+    egui::Color32::from_rgba_premultiplied(component(pixel[0]), component(pixel[1]), component(pixel[2]), (alpha * 255.0 + 0.5) as u8)
+}
+
 /// The whole factor an image of `size` is averaged down by for a texture: at least `min_k`, and
 /// the smallest that fits its sides in `max_side` (frames of comps wider than the GPU's texture
 /// limit, #201).

crates/ui-egui/src/frames/tests.rs

@@ -1,5 +1,11 @@
 use super::*;
 
+#[test]
+fn converting_a_frame_with_nan_alpha_keeps_the_viewer_alive() {
+    let image = effectcraft_engine::render::Image::filled(1, 1, [0.5, 0.25, 0.1, f32::NAN]);
+    assert_eq!(to_color_image(&image).pixels, [egui::Color32::TRANSPARENT]);
+}
+
 struct ProbeDisk {
     keys: Mutex<HashSet<u128>>,
     probes: std::sync::atomic::AtomicUsize,

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

@@ -125,8 +125,7 @@ fn color_pick_preview(app: &EffectcraftApp, painter: &egui::Painter, map: &crate
     painter.rect_filled(chip, 11.0, Color32::from_black_alpha(180));
     painter.text(chip.center(), Align2::CENTER_CENTER, hint, Tokens::ui(11.5), app.tokens.text);
     if let Some(pos) = pointer
-        && let Some(img) = &app.viewer_image
-        && let Some(sample) = fw::sample_frame(img, crate::panels::viewer_tools::shown_region(painter.ctx(), map.comp), map.to_comp(pos))
+        && let Some(sample) = crate::panels::viewer_tools::sample_displayed(app, painter.ctx(), map.to_comp(pos))
     {
         let swatch = Rect::from_min_size(pos + vec2(14.0, 14.0), vec2(26.0, 18.0));
         painter.rect_filled(swatch, 2.0, Color32::from_rgb((sample[0] * 255.0) as u8, (sample[1] * 255.0) as u8, (sample[2] * 255.0) as u8));
@@ -162,8 +161,7 @@ fn text_color_pick(
         }
         if resp.clicked()
             && let Some(pos) = resp.interact_pointer_pos()
-            && let Some(img) = &app.viewer_image
-            && let Some(s) = fw::sample_frame(img, crate::panels::viewer_tools::shown_region(&ctx, map.comp), map.to_comp(pos))
+            && let Some(s) = crate::panels::viewer_tools::sample_displayed(app, &ctx, map.to_comp(pos))
         {
             let mut params = json!({"layer": pick.layer});
             params[&pick.field] = json!([s[0], s[1], s[2]]);
@@ -196,8 +194,7 @@ fn popup_eyedropper(app: &mut EffectcraftApp, ui: &mut egui::Ui, painter: &egui:
     app.viewer_pixels();
     color_pick_preview(app, painter, map, crate::i18n::tr("Click to sample color"), crate::color_picker::eyedropper_point(&ctx));
     if let Some(pos) = crate::color_picker::sample_press(&ctx)
-        && let Some(img) = &app.viewer_image
-        && let Some(s) = fw::sample_frame(img, crate::panels::viewer_tools::shown_region(&ctx, map.comp), map.to_comp(pos))
+        && let Some(s) = crate::panels::viewer_tools::sample_displayed(app, &ctx, map.to_comp(pos))
     {
         crate::color_picker::deliver_sample(&ctx, [s[0], s[1], s[2]]);
     }

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

@@ -196,14 +196,21 @@ pub fn layer_to_comp(l2c: &Mat3, p: [f64; 2]) -> [f64; 2] {
 /// `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> {
+    img.pixels.get(frame_pixel_index(img.size, region, pt)?).copied()
+}
+
+pub(crate) fn frame_pixel_index(size: [usize; 2], region: [f64; 4], pt: [f64; 2]) -> Option<usize> {
+    if size.contains(&0) {
+        return None;
+    }
     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 = ((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()
+    let sx = ((fx * size[0] as f64).floor() as usize).min(size[0].saturating_sub(1));
+    let sy = ((fy * size[1] as f64).floor() as usize).min(size[1].saturating_sub(1));
+    sy.checked_mul(size[0])?.checked_add(sx)
 }
 
 /// Straight RGBA (0..1) of the displayed frame at comp point `pt` ([`frame_pixel`]).

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

@@ -366,6 +366,7 @@ pub(crate) fn display_key(app: &EffectcraftApp, has_conversion: bool) -> String
 /// is set), or the snapshot while Show Snapshot is on / F5 is held, through Show Channel and the
 /// exposure. Fast Previews ▸ Wireframe draws layer outlines instead.
 pub(crate) fn draw_frame(app: &mut EffectcraftApp, ctx: &egui::Context, painter: &egui::Painter, comp_rect: Rect, cid: ItemId, ectx: &EvalCtx) {
+    clear_sample_source(ctx);
     let opts = app.session.state.viewer.clone();
     let snap = showing_snapshot(app, ctx);
     // Settings ▸ Video ▸ Mirror on Computer Monitor off: playback goes to Video Preview only.
@@ -393,6 +394,8 @@ pub(crate) fn draw_frame(app: &mut EffectcraftApp, ctx: &egui::Context, painter:
     let dc = vw::DisplayColor::of(&app.session);
     let dc_key = display_key(app, dc.is_some());
     let plain = opts.channel == Channel::Rgb && opts.exposure == 0.0 && dc.is_none();
+    let display = SampleDisplay { channel: opts.channel, colorized: opts.colorized, exposure: opts.exposure, conversion: dc.clone() };
+    let region = shown_region(ctx, [ectx.comp.width as f32, ectx.comp.height as f32]);
     let uv = Rect::from_min_max(pos2(0.0, 0.0), pos2(1.0, 1.0));
     if snap {
         let Some(s) = app.session.snapshot.clone() else { return };
@@ -415,6 +418,12 @@ pub(crate) fn draw_frame(app: &mut EffectcraftApp, ctx: &egui::Context, painter:
             }
         };
         painter.image(tex.id(), comp_rect, uv, Color32::WHITE);
+        note_sample_source(
+            ctx,
+            SampleSource::Snapshot(s.image.clone()),
+            [0.0, 0.0, ectx.comp.width as f64, ectx.comp.height as f64],
+            (!plain).then_some(display),
+        );
         painter.text(comp_rect.left_top() + vec2(6.0, 6.0), Align2::LEFT_TOP, crate::i18n::tr("Snapshot"), Tokens::ui(11.0), SNAP_COLOR);
         return;
     }
@@ -428,6 +437,9 @@ pub(crate) fn draw_frame(app: &mut EffectcraftApp, ctx: &egui::Context, painter:
                 ctx.request_repaint();
             }
             painter.image(id, comp_rect, uv, Color32::WHITE);
+            if let Some((_, key)) = app.viewer_shown {
+                note_sample_source(ctx, SampleSource::Gpu(key), region, None);
+            }
             return;
         }
         comp_rect
@@ -444,6 +456,9 @@ pub(crate) fn draw_frame(app: &mut EffectcraftApp, ctx: &egui::Context, painter:
         if plain {
             let uv = app.viewer_image.as_ref().map_or(uv, |src| fitted_uv(ctx, src.size, minify));
             painter.image(tex.id(), rect, uv, Color32::WHITE);
+            if let Some(image) = app.viewer_image.clone() {
+                note_sample_source(ctx, SampleSource::Cpu(image), region, None);
+            }
             return;
         }
         rect
@@ -470,6 +485,71 @@ pub(crate) fn draw_frame(app: &mut EffectcraftApp, ctx: &egui::Context, painter:
         }
     };
     painter.image(tex.id(), rect, fitted_uv(ctx, src.size, minify), Color32::WHITE);
+    note_sample_source(ctx, SampleSource::Cpu(src), region, Some(display));
+}
+
+#[derive(Clone)]
+enum SampleSource {
+    Cpu(std::sync::Arc<egui::ColorImage>),
+    Gpu(crate::frames::FrameKey),
+    Snapshot(std::sync::Arc<effectcraft_engine::render::Image>),
+}
+
+#[derive(Clone)]
+struct SampleDisplay {
+    channel: Channel,
+    colorized: bool,
+    exposure: f32,
+    conversion: Option<vw::DisplayColor>,
+}
+
+#[derive(Clone)]
+struct DisplayedSample {
+    pass: u64,
+    source: SampleSource,
+    region: [f64; 4],
+    display: Option<SampleDisplay>,
+}
+
+fn sample_source_id() -> egui::Id {
+    egui::Id::unique("viewer-displayed-sample-source")
+}
+
+pub(crate) fn clear_sample_source(ctx: &egui::Context) {
+    ctx.data_mut(|data| data.remove::<DisplayedSample>(sample_source_id()));
+}
+
+fn note_sample_source(ctx: &egui::Context, source: SampleSource, region: [f64; 4], display: Option<SampleDisplay>) {
+    let sample = DisplayedSample { pass: ctx.cumulative_pass_nr(), source, region, display };
+    ctx.data_mut(|data| data.insert_temp(sample_source_id(), sample));
+}
+
+/// Sample only a frame actually drawn this pass, with that draw's channel, exposure and
+/// display conversion. Hidden and wireframe views deliberately have no sample source.
+pub(crate) fn sample_displayed(app: &EffectcraftApp, ctx: &egui::Context, point: [f64; 2]) -> Option<[f32; 4]> {
+    let sample = ctx.data(|data| data.get_temp::<DisplayedSample>(sample_source_id()))?;
+    if sample.pass != ctx.cumulative_pass_nr() {
+        return None;
+    }
+    let pixel = match sample.source {
+        SampleSource::Cpu(image) => super::fx_widgets::frame_pixel(&image, sample.region, point)?,
+        SampleSource::Gpu(key) => {
+            if app.viewer_shown.map(|(_, shown)| shown) != Some(key) {
+                return None;
+            }
+            super::fx_widgets::frame_pixel(app.viewer_image.as_ref()?, sample.region, point)?
+        }
+        SampleSource::Snapshot(image) => {
+            let index = super::fx_widgets::frame_pixel_index([image.width as usize, image.height as usize], sample.region, point)?;
+            crate::frames::display_pixel(*image.data.get(index)?)
+        }
+    };
+    let image = egui::ColorImage::new([1, 1], vec![pixel]);
+    let image = match sample.display {
+        Some(display) => transformed(&image, display.channel, display.colorized, display.exposure, display.conversion),
+        None => image,
+    };
+    super::fx_widgets::sample_frame(&image, [0.0, 0.0, 1.0, 1.0], [0.5, 0.5])
 }
 
 /// The texture coordinates of a frame of `size` in its texture, fitted with at least `minify`