Merge #332: M0.9: every hidden window answers control requests, not only the main one (#269) (@patlux)

crates/ui-egui/src/control.rs

@@ -139,27 +139,45 @@ pub fn drain(app: &mut FigApp, ctx: &Context) {
 /// Methods that need no frame drawn (#269): a hidden window answers them too.
 const NO_FRAME: &[&str] = &["engine.execute", "engine.commands", "document.inspect", "node.get", "window.list", "ui.render"];
 
-/// While the main window is hidden (minimised, or covered by other windows) it draws no frames,
-/// but the desktop app's `logic` still runs: it answers the requests for that window that need no
-/// frame, so an attached agent can edit, inspect and save. The rest (screenshots, input, the
-/// interface's own state) wait for the window to show.
+/// Whether window `id` draws no frames now: minimised, or covered by other windows (on macOS also
+/// on another Space).
+fn hidden(ctx: &Context, id: u64) -> bool {
+    ctx.input(|i| i.raw.viewports.get(&crate::windows::viewport(id)).and_then(egui::ViewportInfo::visible)) == Some(false)
+}
+
+/// A hidden window (minimised, or covered by other windows) draws no frames, but the desktop app's
+/// `logic` still runs, for the main window even while every window is hidden: it answers the
+/// requests for any hidden window that need no frame, in that window's file, so an attached agent
+/// can edit, inspect and save. The rest (screenshots, input, the interface's own state) wait for the
+/// window to show; another window on screen is asked to draw, which the main window's frame would
+/// do but doesn't while it is hidden. A request for a window that doesn't exist is an error at once.
 pub fn drain_hidden(app: &mut FigApp, ctx: &Context) {
-    if ctx.input(|i| i.viewport().visible()) != Some(false) {
-        return;
-    }
     for r in std::mem::take(&mut app.control_queue) {
         if r.is_cancelled() {
             continue;
         }
-        let main = r.params.get("window").and_then(Value::as_u64).unwrap_or(app.windows.focused) == 0;
-        if !main || !NO_FRAME.contains(&r.method.as_str()) {
+        let target = r.params.get("window").and_then(Value::as_u64).unwrap_or(app.windows.focused);
+        if !app.windows.exists(target) {
+            let _ = r.reply.send(ControlReply { id: r.id, result: Err(format!("no window {target}")) });
+            continue;
+        }
+        if !hidden(ctx, target) || !NO_FRAME.contains(&r.method.as_str()) {
+            // Another window on screen draws only when asked: while the main window is hidden no
+            // frame of it does that (`drain`), so ask here.
+            if target != app.windows.current && !hidden(ctx, target) {
+                ctx.request_repaint_of(crate::windows::viewport(target));
+            }
             app.control_queue.push(r);
             continue;
         }
-        // Outside a frame, so outside the frame's guard: a panic still ends in recovery copies.
-        let mut res = Err("internal error".to_string());
-        crate::windows::guard(app, ctx, |app| res = handle(app, ctx, &r.method, &r.params));
-        let _ = r.reply.send(ControlReply { id: r.id, result: res });
+        // Outside a frame, so outside the frame's guard: within the window's swap, a panic still
+        // ends in recovery copies and leaves every window its own state.
+        let res = crate::windows::as_window(app, target, |app| {
+            let mut res = Err("internal error".to_string());
+            crate::windows::guard(app, ctx, |app| res = handle(app, ctx, &r.method, &r.params));
+            res
+        });
+        let _ = r.reply.send(ControlReply { id: r.id, result: res.unwrap_or_else(|| Err(format!("no window {target}"))) });
     }
 }
 

crates/ui-egui/src/windows.rs

@@ -433,21 +433,74 @@ pub(crate) fn show(app: &FigApp, ctx: &Context) {
     }
 }
 
-/// One frame of window `id`, with its state swapped in.
-fn draw(app: &mut FigApp, id: u64, ui: &mut egui::Ui) {
-    let Some(k) = app.windows.list.iter().position(|w| w.id == id) else { return };
-    let ctx = ui.ctx().clone();
+/// A window's state swapped into the app ([`enter`]) until [`leave`] swaps it back out.
+struct Entered {
+    id: u64,
+    pane: Pane,
+    /// The newest open file before: files opened after it are the window's own.
+    newest: u64,
+}
+
+/// Swap window `id`'s state into the app and show its file; `None` when there is no such window.
+fn enter(app: &mut FigApp, id: u64) -> Option<Entered> {
+    let k = app.windows.list.iter().position(|w| w.id == id)?;
     let main_active = app.session.ds().ok().map(|d| d.uid);
     let newest = app.session.docs.iter().map(|d| d.uid).max().unwrap_or(0);
-    let files_before = app.session.docs.len();
-    let prefs = app.session.prefs.clone();
-    let Some(w) = app.windows.list.get_mut(k) else { return };
+    let w = app.windows.list.get_mut(k)?;
     let want = w.active;
     let mut pane = std::mem::take(&mut w.pane);
     exchange(app, &mut pane);
     app.windows.current = id;
     app.windows.main_active = main_active;
     show_own(app, want);
+    Some(Entered { id, pane, newest })
+}
+
+/// Files the entered window opened are its own.
+fn adopt(app: &mut FigApp, e: &Entered) {
+    let new: Vec<u64> = app.session.docs.iter().map(|d| d.uid).filter(|u| *u > e.newest && app.windows.of(*u) == 0).collect();
+    if let Some(w) = app.windows.list.iter_mut().find(|w| w.id == e.id) {
+        w.docs.extend(new);
+    }
+}
+
+/// Swap the entered window's state back out (with where it is on screen, when known) and the
+/// main window's back in.
+fn leave(app: &mut FigApp, mut e: Entered, outer: Option<egui::Rect>, inner: Option<egui::Rect>) {
+    let shown = app.session.ds().ok().map(|d| d.uid).filter(|u| app.windows.of(*u) == e.id);
+    exchange(app, &mut e.pane);
+    if let Some(w) = app.windows.list.iter_mut().find(|w| w.id == e.id) {
+        w.active = shown.or(w.active);
+        w.pos = outer.map(|r| r.min).or(w.pos);
+        w.size = inner.map_or(w.size, |r| r.size());
+        w.pane = e.pane;
+    }
+    app.windows.current = 0;
+    let main_active = app.windows.main_active.take();
+    show_own(app, main_active);
+}
+
+/// Run `f` as window `id` (0: the main window) outside its frame, with its state swapped in as
+/// while it draws; `None` when there is no such window. For a window that draws nothing now
+/// (minimized, or occluded: behind other windows or on another Space).
+pub(crate) fn as_window<R>(app: &mut FigApp, id: u64, f: impl FnOnce(&mut FigApp) -> R) -> Option<R> {
+    if id == 0 {
+        // Between frames the app holds the main window's state.
+        return Some(f(app));
+    }
+    let e = enter(app, id)?;
+    let r = f(app);
+    adopt(app, &e);
+    leave(app, e, None, None);
+    Some(r)
+}
+
+/// One frame of window `id`, with its state swapped in.
+fn draw(app: &mut FigApp, id: u64, ui: &mut egui::Ui) {
+    let ctx = ui.ctx().clone();
+    let files_before = app.session.docs.len();
+    let prefs = app.session.prefs.clone();
+    let Some(e) = enter(app, id) else { return };
     if ctx.input(|i| i.viewport().focused == Some(true)) {
         app.windows.focused = id;
     }
@@ -455,25 +508,11 @@ fn draw(app: &mut FigApp, id: u64, ui: &mut egui::Ui) {
     app.logic(&ctx);
     // Within the swap, so a panic leaves every window its own state.
     guard(app, &ctx, |app| app.frame(ui));
-    // Files it opened are its own.
-    let new: Vec<u64> = app.session.docs.iter().map(|d| d.uid).filter(|u| *u > newest && app.windows.of(*u) == 0).collect();
-    if let Some(w) = app.windows.list.iter_mut().find(|w| w.id == id) {
-        w.docs.extend(new);
-    }
+    adopt(app, &e);
     settle(app, &ctx, files_before);
     if app.session.prefs != prefs {
         repaint_others(app, &ctx);
     }
-    let shown = app.session.ds().ok().map(|d| d.uid).filter(|u| app.windows.of(*u) == id);
     let (outer, inner) = ctx.input(|i| (i.viewport().outer_rect, i.viewport().inner_rect));
-    exchange(app, &mut pane);
-    if let Some(w) = app.windows.list.iter_mut().find(|w| w.id == id) {
-        w.active = shown.or(w.active);
-        w.pos = outer.map(|r| r.min).or(w.pos);
-        w.size = inner.map_or(w.size, |r| r.size());
-        w.pane = pane;
-    }
-    app.windows.current = 0;
-    let main_active = app.windows.main_active.take();
-    show_own(app, main_active);
+    leave(app, e, outer, inner);
 }

crates/ui-egui/src/windows_tests.rs

@@ -1,10 +1,13 @@
 //! Several windows: a window holds its own tabs; closing it closes its files, asking first when
 //! one has unsaved changes, and the main window takes over another window instead of quitting.
 
+use std::sync::mpsc::{Receiver, channel};
+
 use egui::{RawInput, ViewportEvent, ViewportId, ViewportInfo};
 use figcraft_engine::Session;
-use serde_json::json;
+use serde_json::{Value, json};
 
+use crate::control::{ControlReply, ControlRequest};
 use crate::{FigApp, NoServices};
 
 /// One main-window frame, as the desktop app runs it; `close` asks the window to close.
@@ -21,6 +24,23 @@ fn frame(ctx: &egui::Context, app: &mut FigApp, close: bool) {
     .clear();
 }
 
+/// A control request queued for the UI, and where its reply arrives.
+fn request(app: &mut FigApp, method: &str, params: Value) -> Receiver<ControlReply> {
+    let (reply, rx) = channel();
+    app.control_queue.push(ControlRequest { id: json!(1), method: method.into(), params, reply, cancelled: Default::default() });
+    rx
+}
+
+/// The desktop app's logic as eframe runs it for the main window, with each window's visibility
+/// (`hidden`: minimised or covered). While the main window is hidden too this is all that runs.
+fn logic(ctx: &egui::Context, app: &mut FigApp, hidden: &[ViewportId]) {
+    let mut input = RawInput::default();
+    for id in std::iter::once(ViewportId::ROOT).chain(app.windows.list.iter().map(|w| crate::windows::viewport(w.id))) {
+        input.viewports.insert(id, ViewportInfo { minimized: Some(false), occluded: Some(hidden.contains(&id)), ..Default::default() });
+    }
+    let _ = ctx.run_logic(&input, |ctx| crate::control::drain_hidden(app, ctx));
+}
+
 fn names(app: &FigApp) -> Vec<String> {
     app.session.docs.iter().map(|d| d.doc.name.clone()).collect()
 }
@@ -64,3 +84,69 @@ fn windows_hold_tabs_and_close_with_their_files() {
     assert!(!app.ui.quit, "the app keeps running");
     assert_eq!(app.session.ds().unwrap().doc.name, "A");
 }
+
+#[test]
+fn another_hidden_window_answers_requests_that_need_no_frame_in_its_own_file() {
+    let mut s = Session::empty();
+    for n in ["A", "B"] {
+        s.execute("file.new", &json!({"name": n})).unwrap();
+    }
+    let ctx = egui::Context::default();
+    ctx.set_embed_viewports(false);
+    let mut app = FigApp::new(s, Box::new(NoServices));
+    frame(&ctx, &mut app, false);
+    let w = crate::control::handle(&mut app, &ctx, "window.new", &json!({"index": 0})).unwrap()["window"].as_u64().unwrap();
+    frame(&ctx, &mut app, false);
+    let other = crate::windows::viewport(w);
+    let rect = |size: u32| json!({"window": w, "command": "shape.rectangle", "params": {"x": 0, "y": 0, "width": size, "height": size}});
+    let dirty = |app: &FigApp, name: &str| app.session.docs.iter().find(|d| d.doc.name == name).unwrap().is_dirty();
+
+    // The main window is on screen, the other one covered: the request to it is answered in its
+    // file (A), and the main window keeps showing its own (B).
+    let edit = request(&mut app, "engine.execute", rect(10));
+    logic(&ctx, &mut app, &[other]);
+    assert!(edit.try_recv().expect("answered without the other window's frame").result.is_ok());
+    assert!(dirty(&app, "A") && !dirty(&app, "B"), "in the other window's file");
+    assert_eq!(app.session.ds().unwrap().doc.name, "B");
+    assert_eq!(app.windows.current, 0);
+
+    // Both hidden: still answered, and inspection sees the other window's file.
+    let inspect = request(&mut app, "document.inspect", json!({"window": w, "depth": 0}));
+    logic(&ctx, &mut app, &[ViewportId::ROOT, other]);
+    assert_eq!(inspect.try_recv().expect("answered").result.unwrap()["name"], json!("A"));
+    assert_eq!(app.session.ds().unwrap().doc.name, "B");
+
+    // What needs a frame keeps waiting for the other window; an unknown window is an error at once.
+    let ui = request(&mut app, "ui.inspect", json!({"window": w}));
+    let unknown = request(&mut app, "window.list", json!({"window": 99}));
+    logic(&ctx, &mut app, &[ViewportId::ROOT, other]);
+    assert!(ui.try_recv().is_err(), "waits for the window to show");
+    assert!(unknown.try_recv().expect("answered").result.unwrap_err().contains("no window 99"));
+    assert_eq!(app.control_queue.len(), 1);
+}
+
+#[test]
+fn a_request_for_a_window_on_screen_wakes_it_while_the_main_window_is_hidden() {
+    let mut s = Session::empty();
+    for n in ["A", "B"] {
+        s.execute("file.new", &json!({"name": n})).unwrap();
+    }
+    let ctx = egui::Context::default();
+    ctx.set_embed_viewports(false);
+    let mut app = FigApp::new(s, Box::new(NoServices));
+    frame(&ctx, &mut app, false);
+    let w = crate::control::handle(&mut app, &ctx, "window.new", &json!({"index": 0})).unwrap()["window"].as_u64().unwrap();
+    frame(&ctx, &mut app, false);
+    let other = crate::windows::viewport(w);
+    // Which windows are asked to draw from now on (an idle window draws no frames until asked).
+    let woken = std::sync::Arc::new(std::sync::Mutex::new(Vec::new()));
+    let log = woken.clone();
+    ctx.set_request_repaint_callback(move |info| log.lock().unwrap().push(info.viewport_id));
+
+    // Only the main window's logic runs while it is hidden: the request for the other window,
+    // which is on screen, waits for that window's frame, and that window is asked to draw it.
+    let ui = request(&mut app, "ui.inspect", json!({"window": w}));
+    logic(&ctx, &mut app, &[ViewportId::ROOT]);
+    assert!(ui.try_recv().is_err(), "answered in the other window's frame");
+    assert!(woken.lock().unwrap().contains(&other), "the other window is asked to draw: {:?}", woken.lock().unwrap());
+}

docs/control-protocol.md

@@ -35,9 +35,10 @@ instead of dispatching the new request. These limits apply to the opt-in loopbac
 requests still queued after the UI timeout are discarded, including requests deferred to a hidden
 window. A command already running is not interrupted.
 
-A minimised or covered main window draws no frames, but still answers `engine.execute`,
-`engine.commands`, `document.inspect`, `node.get`, `window.list` and `ui.render`, so an attached
-agent can edit, inspect and save while the app is out of sight. Screenshots, input (`ui.input`,
+A minimised or covered window (the main one or another, see Windows below) draws no frames, but
+still answers `engine.execute`, `engine.commands`, `document.inspect`, `node.get`, `window.list`
+and `ui.render`, acting on the file that window shows, so an attached agent can edit, inspect and
+save while the app is out of sight. Screenshots, input (`ui.input`,
 `ui.pointer`, `ui.key`, `ui.type`) and the interface's own state wait for the window to show, and
 time out after 60 s if it doesn't.