Viewer: a locked viewer keeps its comp when the Project panel opens another (#156)

The Project panel (like other panels) runs comp.open while it draws, so the
new comp became the active comp mid-frame. Later in the same frame the
active, locked viewer drew and recorded the active comp as its own. On the
next frame on_open_comp compared that record with the opened comp, found
them equal and skipped the lock, so the locked viewer switched comps.

The active viewer no longer records its comp while an OpenComp event is
pending (it asks for a repaint instead); on_open_comp then sees the comp
the locked viewer really shows and routes the new one to an unlocked or
new viewer.

The viewers test's click helper now releases a frame after the press, as
a hand does: a press in a tab strip focuses the shown panel first, and a
same-frame press and release refocused it after the tab's own click.

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

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

@@ -73,8 +73,14 @@ pub fn comp_of(app: &EffectcraftApp, id: u32) -> Option<ItemId> {
 /// becomes the active viewer when clicked.
 pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, id: u32, rect: Rect) {
     if id == app.ui.active_viewer {
-        let c = app.session.active_comp_id().map(|c| c.0);
-        app.ui.viewers.insert(id, c);
+        // A comp opened earlier this frame (the Project panel) is routed next frame by
+        // `on_open_comp`, which needs the comp this viewer showed (to keep it when locked).
+        if app.session.events.iter().any(|e| matches!(e, effectcraft_engine::Event::OpenComp(_))) {
+            ui.ctx().request_repaint();
+        } else {
+            let c = app.session.active_comp_id().map(|c| c.0);
+            app.ui.viewers.insert(id, c);
+        }
         super::viewer::show(app, ui, rect);
         return;
     }

crates/ui-egui/tests/ui_viewers.rs

@@ -17,13 +17,35 @@ fn invoke(h: &mut Harness<'_, EffectcraftApp>, id: &str, p: Value) {
     h.run_steps(3);
 }
 
-fn click(h: &mut Harness<'_, EffectcraftApp>, id: &str) {
+/// Hover element `id`; returns its centre.
+fn hover(h: &mut Harness<'_, EffectcraftApp>, id: &str) -> egui::Pos2 {
     let e = h.state().auto.find(id).unwrap_or_else(|| panic!("no {id}")).clone();
     let p = pos2(e.rect[0] + e.rect[2] / 2.0, e.rect[1] + e.rect[3] / 2.0);
     h.input_mut().events.push(Event::PointerMoved(p));
     h.step();
-    h.input_mut().events.push(Event::PointerButton { pos: p, button: egui::PointerButton::Primary, pressed: true, modifiers: Default::default() });
-    h.input_mut().events.push(Event::PointerButton { pos: p, button: egui::PointerButton::Primary, pressed: false, modifiers: Default::default() });
+    p
+}
+
+fn button(h: &mut Harness<'_, EffectcraftApp>, pos: egui::Pos2, pressed: bool) {
+    h.input_mut().events.push(Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() });
+}
+
+/// Click element `id`, the release a frame after the press (as a hand does: a press in a tab
+/// strip focuses the shown panel first, the click then the tab).
+fn click(h: &mut Harness<'_, EffectcraftApp>, id: &str) {
+    let p = hover(h, id);
+    button(h, p, true);
+    h.step();
+    button(h, p, false);
+    h.run_steps(3);
+}
+
+/// Double-click element `id` (in one frame: the harness' frame time is longer than a double-click's).
+fn double_click(h: &mut Harness<'_, EffectcraftApp>, id: &str) {
+    let p = hover(h, id);
+    for pressed in [true, false, true, false] {
+        button(h, p, pressed);
+    }
     h.run_steps(3);
 }
 
@@ -31,14 +53,16 @@ fn shows(h: &Harness<'_, EffectcraftApp>, viewer: u32) -> Option<u64> {
     viewers::comp_of(h.state(), viewer).map(|c| c.0)
 }
 
+fn new_comp(s: &mut Session, name: &str) -> u64 {
+    s.execute("comp.new", json!({"name": name, "width": 64, "height": 36, "duration": 1})).unwrap()["comp"].as_u64().unwrap()
+}
+
 #[test]
 fn new_viewers_lock_route_comps_and_activate_on_click() {
     let mut s = Session::default();
-    let new =
-        |s: &mut Session, name: &str| s.execute("comp.new", json!({"name": name, "width": 64, "height": 36, "duration": 1})).unwrap()["comp"].as_u64().unwrap();
-    let c = new(&mut s, "C");
-    let b = new(&mut s, "B");
-    let a = new(&mut s, "A");
+    let c = new_comp(&mut s, "C");
+    let b = new_comp(&mut s, "B");
+    let a = new_comp(&mut s, "A");
     let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).build_eframe(|_| EffectcraftApp::new(s));
     h.run_steps(3);
     // View ▸ New Viewer: viewer 1 shows A too, viewer 0 (the Composition panel) is locked.
@@ -87,6 +111,29 @@ fn new_viewers_lock_route_comps_and_activate_on_click() {
     assert!(titles.iter().any(|t| t == "Composition A") && titles.iter().any(|t| t == "Composition C"), "{titles:?}");
 }
 
+/// Issue #156: a locked viewer keeps its comp when the Project panel opens another one. The
+/// Project panel opens it mid-frame, before the viewers draw; the active (locked) viewer used to
+/// record the new comp as its own then, so the lock no longer applied.
+#[test]
+fn a_locked_viewer_keeps_its_comp_when_the_project_panel_opens_another() {
+    let mut s = Session::default();
+    let b = new_comp(&mut s, "B");
+    let a = new_comp(&mut s, "A");
+    let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).build_eframe(|_| EffectcraftApp::new(s));
+    h.run_steps(3);
+    // Viewer 0 locked to A, and B in viewer 1 (a tab next to it).
+    invoke(&mut h, "view.newViewer", json!({}));
+    invoke(&mut h, "comp.open", json!({"comp": b}));
+    // Back to the locked viewer by its tab.
+    click(&mut h, "panel.tab.Composition");
+    assert!(viewers::locked(h.state(), 0));
+    assert_eq!((h.state().ui.active_viewer, shows(&h, 0), shows(&h, 1)), (0, Some(a), Some(b)));
+    // Double-click B in the Project panel: it shows in its viewer, the locked one keeps A.
+    double_click(&mut h, &format!("project.item.{b}.name"));
+    assert_eq!(h.state().session.active_comp_id(), Some(ItemId(b)));
+    assert_eq!((h.state().ui.active_viewer, shows(&h, 0), shows(&h, 1)), (1, Some(a), Some(b)));
+}
+
 /// `cargo test -p effectcraft-ui-egui --test ui_viewers -- --ignored`: two viewers side by side
 /// (`target/test-out/viewers.png`).
 #[test]