mirror of
https://github.com/storytold/lightcraft.git
synced 2026-10-24 10:09:01 -06:00
Merge slideshow-playback into slideshow-composer (origin/main: batches 32–33)
This commit is contained in:
@@ -303,16 +303,20 @@ fn mcp(args: &[String]) -> Result<(), String> {
|
||||
while i < args.len() {
|
||||
match args[i].as_str() {
|
||||
"--connect" => {
|
||||
let (addr, took) = connect_value(args.get(i + 1).map(String::as_str));
|
||||
connect = Some(addr);
|
||||
if took {
|
||||
i += 1;
|
||||
// Optional address argument. A bare port (all digits) is taken as a port on the
|
||||
// loopback default — matches the help's `--connect [ADDR]` hint alongside the
|
||||
// `--control 7980` example. A non-numeric, non-`--`-prefixed token (e.g. a
|
||||
// command id like `ui.inspect`) leaves the address at its default so the
|
||||
// command is still consumed next. (#1178)
|
||||
match args.get(i + 1).filter(|a| a.contains(':') || a.chars().all(|c| c.is_ascii_digit())) {
|
||||
Some(a) => {
|
||||
connect = Some(if a.contains(':') { a.clone() } else { format!("127.0.0.1:{a}") });
|
||||
i += 1;
|
||||
}
|
||||
None => connect = Some(DEFAULT_ADDR.to_string()),
|
||||
}
|
||||
}
|
||||
a if a.starts_with("--connect=") => {
|
||||
let (addr, _) = connect_value(Some(&a["--connect=".len()..]));
|
||||
connect = Some(addr);
|
||||
}
|
||||
a if a.starts_with("--connect=") => connect = Some(a["--connect=".len()..].to_string()),
|
||||
"--headless" => connect = None,
|
||||
"--demo" => demo = true,
|
||||
"--compact" => compact = true,
|
||||
@@ -525,17 +529,14 @@ fn run(args: &[String]) -> Result<(), String> {
|
||||
i += 1;
|
||||
break;
|
||||
}
|
||||
"--connect" => {
|
||||
let (addr, took) = connect_value(args.get(i + 1).map(String::as_str));
|
||||
connect = Some(addr);
|
||||
if took {
|
||||
"--connect" => match args.get(i + 1).filter(|a| !a.contains('=') && (a.contains(':') || a.chars().all(|c| c.is_ascii_digit()))) {
|
||||
Some(a) => {
|
||||
connect = Some(if a.contains(':') { a.clone() } else { format!("127.0.0.1:{a}") });
|
||||
i += 1;
|
||||
}
|
||||
}
|
||||
a if a.starts_with("--connect=") => {
|
||||
let (addr, _) = connect_value(Some(&a["--connect=".len()..]));
|
||||
connect = Some(addr);
|
||||
}
|
||||
None => connect = Some(DEFAULT_ADDR.to_string()),
|
||||
},
|
||||
a if a.starts_with("--connect=") => connect = Some(a["--connect=".len()..].to_string()),
|
||||
"--demo" => demo = true,
|
||||
"--library" => library = Some(take_value(args, &mut i, "--library")?.to_string()),
|
||||
"--import" => imports.push(take_value(args, &mut i, "--import")?.to_string()),
|
||||
@@ -630,18 +631,6 @@ fn take_value<'a>(args: &'a [String], i: &mut usize, flag: &str) -> Result<&'a s
|
||||
|
||||
/// The port an `--connect` address names, for the recovery hint. The whole address when it has
|
||||
/// no port, so a hint is never built from a guess.
|
||||
/// `--connect` takes an address, or a bare port (`7980` → `127.0.0.1:7980`). A following
|
||||
/// command id is left alone (#1178).
|
||||
fn connect_value(next: Option<&str>) -> (String, bool) {
|
||||
match next {
|
||||
Some(a) if !a.starts_with('-') && a.contains(':') && !a.contains('=') => (a.to_string(), true),
|
||||
Some(a) if !a.is_empty() && a.bytes().all(|c| c.is_ascii_digit()) && a.parse::<u16>().ok().is_some_and(|p| p > 0) => {
|
||||
(format!("127.0.0.1:{a}"), true)
|
||||
}
|
||||
_ => (DEFAULT_ADDR.to_string(), false),
|
||||
}
|
||||
}
|
||||
|
||||
fn connect_port(addr: &str) -> &str {
|
||||
addr.rsplit_once(':').map_or(addr, |(_, port)| port)
|
||||
}
|
||||
@@ -980,20 +969,6 @@ fn denoise_serve(args: &[String]) -> Result<(), String> {
|
||||
serve(listener, s.model, s.tile, s.device, s.run, opts, std::sync::Arc::new(|m: &str| eprintln!("lightcraft-cli denoise-serve: {m}")))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod connect_tests {
|
||||
use super::connect_value;
|
||||
|
||||
#[test]
|
||||
fn a_bare_port_is_loopback_and_a_command_is_not_consumed() {
|
||||
assert_eq!(connect_value(Some("7980")), ("127.0.0.1:7980".into(), true));
|
||||
assert_eq!(connect_value(Some("127.0.0.1:7980")), ("127.0.0.1:7980".into(), true));
|
||||
assert_eq!(connect_value(Some("photo.rate")), ("127.0.0.1:7980".into(), false));
|
||||
assert_eq!(connect_value(Some("0")), ("127.0.0.1:7980".into(), false));
|
||||
assert_eq!(connect_value(None), ("127.0.0.1:7980".into(), false));
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(not(feature = "denoise"))]
|
||||
fn denoise_serve(_: &[String]) -> Result<(), String> {
|
||||
Err("denoise-serve: this build has no denoise support (build with the `denoise` feature)".into())
|
||||
|
||||
@@ -476,6 +476,28 @@ fn connect_failure_hint_names_the_attempted_port() {
|
||||
assert!(!err.contains("--control 7980"), "{err}");
|
||||
}
|
||||
|
||||
/// Issue #1178: `--connect 7980 ui.inspect` (after `lightcraft --control 7980`) was leaving
|
||||
/// `7980` unconsumed so it parsed as a command id and produced "unknown command `7980`".
|
||||
/// A bare port is now taken as `127.0.0.1:<port>`; the same address used with `:port` still
|
||||
/// works; non-numeric, non-`ADDR`-shaped tokens (e.g. a real command id) keep falling through.
|
||||
#[test]
|
||||
fn connect_bare_port_is_treated_as_loopback_port() {
|
||||
// Bare port: the failure hint names that port.
|
||||
let (ok, _lines, err) = run_cli(&["--connect", "18437", "ui.inspect"], None);
|
||||
assert!(!ok);
|
||||
assert!(err.contains("--control 18437"), "bare port must reach the hint: {err}");
|
||||
assert!(!err.contains("--control 7980"), "{err}");
|
||||
// `<host>:<port>` still works (issue #167 case).
|
||||
let (ok, _lines, err) = run_cli(&["--connect", "127.0.0.1:18437", "ui.inspect"], None);
|
||||
assert!(!ok);
|
||||
assert!(err.contains("--control 18437"), "{err}");
|
||||
// `--connect=<bare-port>` already worked (the `a.starts_with("--connect=")` arm); spot-check
|
||||
// the hint matches its port too.
|
||||
let (ok, _lines, err) = run_cli(&["--connect=18437", "ui.inspect"], None);
|
||||
assert!(!ok);
|
||||
assert!(err.contains("--control 18437"), "{err}");
|
||||
}
|
||||
|
||||
/// The CLI logs warnings on stderr (#168): with `RUST_LOG=warn` an unknown GPU backend name is
|
||||
/// reported, and `RUST_LOG=off` silences it. Before, no logger was installed and nothing appeared.
|
||||
#[test]
|
||||
|
||||
@@ -228,6 +228,7 @@ fn services(originals: Originals, backend: Option<Backend>, files: Files, frozen
|
||||
reveal: None,
|
||||
log_file: None,
|
||||
open_with: None,
|
||||
open_all_with: None,
|
||||
check_editor: None,
|
||||
open_url: Some(Box::new(|url: &str| {
|
||||
let w = web_sys::window().ok_or("no window")?;
|
||||
|
||||
@@ -64,7 +64,7 @@ impl WireJob {
|
||||
/// `view`: the slot name, for jobs that keep a stage cache.
|
||||
pub fn from_job(job: &RenderJob, thumb_cached: bool, view: &str) -> WireJob {
|
||||
let max_edge = match &job.source {
|
||||
SourceRef::Demo { max_edge, .. } | SourceRef::File { max_edge, .. } => *max_edge,
|
||||
SourceRef::Demo { max_edge, .. } | SourceRef::File { max_edge, .. } | SourceRef::Fit { max_edge, .. } => *max_edge,
|
||||
SourceRef::Loaded(img) => img.image.width.max(img.image.height),
|
||||
SourceRef::Smart { .. } => 2560,
|
||||
};
|
||||
|
||||
@@ -0,0 +1,125 @@
|
||||
//! The ICC profile the desktop assigns to the display (Linux, through colord).
|
||||
//!
|
||||
//! GNOME, KDE and most other desktops keep each monitor's profile in colord
|
||||
//! (`org.freedesktop.ColorManager` on the system bus): Settings ▸ Color writes it there, and a
|
||||
//! calibrator (DisplayCAL, gnome-color-manager) installs its profile there. While no monitor profile
|
||||
//! is chosen in Settings ▸ Display, previews are shown through that profile
|
||||
//! (`Services::screen_profile`). A background thread asks colord every few seconds, so the UI never
|
||||
//! waits on D-Bus; without colord, or a display with no profile, the previews stay sRGB.
|
||||
//!
|
||||
//! With several displays, the window's own one isn't known here (eframe doesn't say which monitor a
|
||||
//! window is on), so the profile is used only when every display that has one has the same: a
|
||||
//! mixed set stays sRGB rather than guess.
|
||||
|
||||
use std::sync::{Arc, Mutex, PoisonError};
|
||||
use std::time::Duration;
|
||||
|
||||
/// How often colord is asked again (a profile change in the desktop's settings shows within this).
|
||||
const POLL: Duration = Duration::from_secs(3);
|
||||
/// A wait before trying again when colord isn't there (not installed, no system bus).
|
||||
const RETRY: Duration = Duration::from_secs(30);
|
||||
/// The largest profile file read (real display profiles are a few KiB to a few MiB).
|
||||
const MAX_PROFILE: u64 = 16 << 20;
|
||||
|
||||
const SERVICE: &str = "org.freedesktop.ColorManager";
|
||||
const DEVICE: &str = "org.freedesktop.ColorManager.Device";
|
||||
const PROFILE: &str = "org.freedesktop.ColorManager.Profile";
|
||||
|
||||
/// Start the background reader and return the service the UI polls (`Services::screen_profile`).
|
||||
pub fn service() -> Box<dyn FnMut() -> Option<Vec<u8>>> {
|
||||
let shared: Arc<Mutex<Option<Vec<u8>>>> = Arc::new(Mutex::new(None));
|
||||
let writer = Arc::clone(&shared);
|
||||
let spawned = std::thread::Builder::new().name("colord".into()).spawn(move || {
|
||||
loop {
|
||||
let wait = match connect() {
|
||||
Some(conn) => {
|
||||
let icc = read(&conn);
|
||||
*writer.lock().unwrap_or_else(PoisonError::into_inner) = icc;
|
||||
POLL
|
||||
}
|
||||
None => RETRY,
|
||||
};
|
||||
std::thread::sleep(wait);
|
||||
// the app is gone: nobody reads the profile any more
|
||||
if Arc::strong_count(&writer) == 1 {
|
||||
return;
|
||||
}
|
||||
}
|
||||
});
|
||||
if let Err(e) = spawned {
|
||||
log::warn!("screen profile: no colord reader: {e}");
|
||||
}
|
||||
Box::new(move || shared.lock().unwrap_or_else(PoisonError::into_inner).clone())
|
||||
}
|
||||
|
||||
fn connect() -> Option<zbus::blocking::Connection> {
|
||||
zbus::blocking::connection::Builder::system().ok()?.method_timeout(Duration::from_secs(2)).build().ok()
|
||||
}
|
||||
|
||||
/// The profile colord assigns to the displays, as file bytes (see the module notes for several displays).
|
||||
fn read(conn: &zbus::blocking::Connection) -> Option<Vec<u8>> {
|
||||
let manager = zbus::blocking::Proxy::new(conn, SERVICE, "/org/freedesktop/ColorManager", SERVICE).ok()?;
|
||||
let devices: Vec<zbus::zvariant::OwnedObjectPath> = manager.call("GetDevicesByKind", &("display",)).ok()?;
|
||||
let mut files = Vec::new();
|
||||
for device in devices.iter().take(64) {
|
||||
let Some(proxy) = zbus::blocking::Proxy::new(conn, SERVICE, device.as_str(), DEVICE).ok() else { continue };
|
||||
let profiles: Vec<zbus::zvariant::OwnedObjectPath> = proxy.get_property("Profiles").unwrap_or_default();
|
||||
// colord lists a device's profiles default first
|
||||
let Some(first) = profiles.first() else { continue };
|
||||
let Some(profile) = zbus::blocking::Proxy::new(conn, SERVICE, first.as_str(), PROFILE).ok() else { continue };
|
||||
if let Ok(file) = profile.get_property::<String>("Filename") {
|
||||
files.push(file);
|
||||
}
|
||||
}
|
||||
let file = pick(&files)?;
|
||||
load(std::path::Path::new(file))
|
||||
}
|
||||
|
||||
/// The profile to use for these displays' profile files: the one they share, `None` with none or
|
||||
/// with different ones (empty names are ignored).
|
||||
fn pick(files: &[String]) -> Option<&str> {
|
||||
let mut named = files.iter().map(String::as_str).filter(|f| !f.is_empty());
|
||||
let first = named.next()?;
|
||||
named.all(|f| f == first).then_some(first)
|
||||
}
|
||||
|
||||
/// The bytes of profile file `path`, `None` when it can't be read or is implausibly large.
|
||||
fn load(path: &std::path::Path) -> Option<Vec<u8>> {
|
||||
use std::io::Read as _;
|
||||
let file = std::fs::File::open(path).ok()?;
|
||||
let mut bytes = Vec::new();
|
||||
file.take(MAX_PROFILE + 1).read_to_end(&mut bytes).ok()?;
|
||||
(!bytes.is_empty() && bytes.len() as u64 <= MAX_PROFILE).then_some(bytes)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn one_shared_profile_is_used_and_a_mixed_set_is_not() {
|
||||
let s = |v: &[&str]| v.iter().map(|f| f.to_string()).collect::<Vec<_>>();
|
||||
assert_eq!(pick(&s(&[])), None);
|
||||
assert_eq!(pick(&s(&["/a.icc"])), Some("/a.icc"));
|
||||
assert_eq!(pick(&s(&["/a.icc", "", "/a.icc"])), Some("/a.icc"));
|
||||
assert_eq!(pick(&s(&["/a.icc", "/b.icc"])), None, "which one the window is on isn't known");
|
||||
assert_eq!(pick(&s(&[""])), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn profile_files_are_read_with_a_size_cap() {
|
||||
let dir = std::env::temp_dir().join(format!("colord-test-{}", std::process::id()));
|
||||
std::fs::create_dir_all(&dir).unwrap();
|
||||
let small = dir.join("small.icc");
|
||||
std::fs::write(&small, b"acsp").unwrap();
|
||||
assert_eq!(load(&small).as_deref(), Some(&b"acsp"[..]));
|
||||
let empty = dir.join("empty.icc");
|
||||
std::fs::write(&empty, b"").unwrap();
|
||||
assert_eq!(load(&empty), None);
|
||||
assert_eq!(load(&dir.join("missing.icc")), None);
|
||||
let big = dir.join("big.icc");
|
||||
std::fs::File::create(&big).unwrap().set_len(MAX_PROFILE + 1).unwrap();
|
||||
assert_eq!(load(&big), None);
|
||||
std::fs::remove_dir_all(&dir).unwrap();
|
||||
}
|
||||
}
|
||||
@@ -55,16 +55,65 @@ fn accept_loop(listener: TcpListener, tx: Sender<ControlRequest>, ctx: egui::Con
|
||||
}
|
||||
}
|
||||
|
||||
/// How long a request waits for its reply.
|
||||
const REPLY_TIMEOUT: Duration = Duration::from_secs(60);
|
||||
/// A request the app has not picked up while it drew no frame for this long is given up: the
|
||||
/// window is not being redrawn (on Wayland, a window on a hidden workspace or minimized gets no
|
||||
/// frame callbacks, so the app's loop is paused: issue #718).
|
||||
const NO_FRAMES_AFTER: Duration = Duration::from_secs(5);
|
||||
|
||||
/// The reply for a request given up because the app drew no frames.
|
||||
pub const HIDDEN_WINDOW_ERROR: &str = "the app drew no frame for 5 s, so the request was not run: its window is probably not visible \
|
||||
(on Wayland a window on a hidden workspace or minimized is not redrawn, which pauses the app). Bring the window on screen and send it again";
|
||||
|
||||
fn serve(stream: TcpStream, tx: &Sender<ControlRequest>, ctx: &egui::Context) {
|
||||
let Ok(read) = stream.try_clone() else { return };
|
||||
serve_lines(read, stream, |method, params| {
|
||||
let (req, rrx) = ControlRequest::new(method, params);
|
||||
let state = Arc::clone(&req.state);
|
||||
tx.send(req).ok()?;
|
||||
ctx.request_repaint();
|
||||
Some(rrx.recv_timeout(Duration::from_secs(60)).unwrap_or_else(|_| json!({"ok": false, "error": "timeout"})))
|
||||
Some(await_reply(&rrx, &state, || ctx.cumulative_pass_nr(), || ctx.request_repaint(), NO_FRAMES_AFTER, REPLY_TIMEOUT))
|
||||
});
|
||||
}
|
||||
|
||||
/// Wait for a request's reply. While the app has not picked the request up and `frames` (its
|
||||
/// frame counter) stands still for `no_frames`, the request is abandoned (it will not run late)
|
||||
/// and the reply says why; a request the app is running gets the whole `timeout`.
|
||||
fn await_reply(
|
||||
rrx: &Receiver<Value>,
|
||||
state: &std::sync::atomic::AtomicU8,
|
||||
frames: impl Fn() -> u64,
|
||||
wake: impl Fn(),
|
||||
no_frames: Duration,
|
||||
timeout: Duration,
|
||||
) -> Value {
|
||||
let start = std::time::Instant::now();
|
||||
let slice = (no_frames / 10).max(Duration::from_millis(1));
|
||||
let (mut last, mut since) = (frames(), std::time::Instant::now());
|
||||
loop {
|
||||
match rrx.recv_timeout(slice) {
|
||||
Ok(v) => return v,
|
||||
Err(std::sync::mpsc::RecvTimeoutError::Disconnected) => return json!({"ok": false, "error": "the app dropped the request"}),
|
||||
Err(std::sync::mpsc::RecvTimeoutError::Timeout) => {}
|
||||
}
|
||||
let now = frames();
|
||||
if now != last {
|
||||
(last, since) = (now, std::time::Instant::now());
|
||||
}
|
||||
let taken = state.load(Ordering::SeqCst) == lightcraft_ui_egui::control::TAKEN;
|
||||
if !taken && since.elapsed() >= no_frames && ControlRequest::abandon(state) {
|
||||
return json!({"ok": false, "error": HIDDEN_WINDOW_ERROR});
|
||||
}
|
||||
if start.elapsed() >= timeout {
|
||||
// not run yet: make sure it never is; already running: it may still finish
|
||||
ControlRequest::abandon(state);
|
||||
return json!({"ok": false, "error": "timeout"});
|
||||
}
|
||||
wake();
|
||||
}
|
||||
}
|
||||
|
||||
/// One parsed line.
|
||||
enum Line {
|
||||
Request {
|
||||
@@ -160,6 +209,51 @@ fn serve_lines<R: Read, W: Write>(read: R, mut out: W, mut handle: impl FnMut(St
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
/// Issue #718: an app that draws no frames (a hidden Wayland window) gets the request given
|
||||
/// up with a reason, quickly, and the app then skips it instead of running it late.
|
||||
#[test]
|
||||
fn a_request_to_an_app_drawing_no_frames_is_given_up_with_a_reason() {
|
||||
let (req, rrx) = ControlRequest::new("ui.inspect", json!({}));
|
||||
let t0 = std::time::Instant::now();
|
||||
let r = await_reply(&rrx, &req.state, || 7, || {}, Duration::from_millis(50), Duration::from_secs(10));
|
||||
assert!(t0.elapsed() < Duration::from_secs(2));
|
||||
assert_eq!(r["ok"], false);
|
||||
assert!(r["error"].as_str().unwrap().contains("not visible"), "{r}");
|
||||
assert!(!req.take(), "the app does not run it once it shows up");
|
||||
}
|
||||
|
||||
/// A request the app is running is not given up because frames stopped (a long command
|
||||
/// blocks the frame loop), and its reply arrives.
|
||||
#[test]
|
||||
fn a_request_being_run_waits_for_its_reply() {
|
||||
let (req, rrx) = ControlRequest::new("export.run", json!({}));
|
||||
assert!(req.take());
|
||||
let reply = req.reply.clone();
|
||||
let worker = std::thread::spawn(move || {
|
||||
std::thread::sleep(Duration::from_millis(200));
|
||||
let _ = reply.send(json!({"ok": true, "result": 1}));
|
||||
});
|
||||
let r = await_reply(&rrx, &req.state, || 7, || {}, Duration::from_millis(20), Duration::from_secs(10));
|
||||
assert_eq!(r["ok"], true, "{r}");
|
||||
worker.join().unwrap();
|
||||
}
|
||||
|
||||
/// Frames still coming: the request just waits, then is answered.
|
||||
#[test]
|
||||
fn a_drawing_app_is_waited_for() {
|
||||
let (req, rrx) = ControlRequest::new("ui.inspect", json!({}));
|
||||
let n = std::sync::atomic::AtomicU64::new(0);
|
||||
let reply = req.reply.clone();
|
||||
let state = Arc::clone(&req.state);
|
||||
let worker = std::thread::spawn(move || {
|
||||
std::thread::sleep(Duration::from_millis(150));
|
||||
let _ = reply.send(json!({"ok": true, "result": 2}));
|
||||
});
|
||||
let r = await_reply(&rrx, &state, || n.fetch_add(1, Ordering::SeqCst), || {}, Duration::from_millis(30), Duration::from_secs(10));
|
||||
assert_eq!(r["ok"], true, "{r}");
|
||||
worker.join().unwrap();
|
||||
}
|
||||
|
||||
fn run(input: &[u8]) -> (Vec<Value>, Vec<String>) {
|
||||
let mut out = Vec::new();
|
||||
let mut calls = Vec::new();
|
||||
|
||||
@@ -61,6 +61,39 @@ pub fn open_with(path: &str, app: &str) -> Result<(), String> {
|
||||
c.spawn().map(|_| ()).map_err(|e| e.to_string())
|
||||
}
|
||||
|
||||
/// Open every file in `paths` with one start of `app`, as a file manager hands over a selection:
|
||||
/// an editor that keeps a single window (DxO PhotoLab) loses the files that arrive while it is
|
||||
/// still starting when it is started once per file. The system default opens them one by one
|
||||
/// (Explorer and `xdg-open` take one file).
|
||||
pub fn open_all_with(paths: &[String], app: &str) -> Result<(), String> {
|
||||
match all_command(launcher(app)?, paths) {
|
||||
Some(mut c) => c.spawn().map(|_| ()).map_err(|e| e.to_string()),
|
||||
None => paths.iter().try_for_each(|p| open_with(p, app)),
|
||||
}
|
||||
}
|
||||
|
||||
/// The one command that opens all of `paths` in `launcher` (none for the system default).
|
||||
fn all_command(launcher: Launcher, paths: &[String]) -> Option<Command> {
|
||||
match launcher {
|
||||
Launcher::Default => None,
|
||||
Launcher::MacApp(app) => {
|
||||
let mut c = Command::new("open");
|
||||
c.args(["-a", &app]).args(paths);
|
||||
Some(c)
|
||||
}
|
||||
Launcher::Program(p) => {
|
||||
let mut c = Command::new(p);
|
||||
c.args(paths);
|
||||
Some(c)
|
||||
}
|
||||
Launcher::Flatpak => {
|
||||
let mut c = Command::new("flatpak");
|
||||
c.args(["run", "--file-forwarding", PHOTOCRAFT_ID, "@@"]).args(paths).arg("@@");
|
||||
Some(c)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Windows: open `path` in its default application through Explorer, given the path as its one
|
||||
/// argument. Not `cmd /C start`: cmd.exe would read `&`, `^`, `%` or `|` in a file name as its own
|
||||
/// syntax (running what follows an `&` as a command).
|
||||
@@ -172,8 +205,10 @@ fn desktop_exec(entry: &str) -> Option<PathBuf> {
|
||||
Some(rest) => rest.split('"').next()?,
|
||||
None => exec.split_whitespace().next()?,
|
||||
};
|
||||
// a bare name (`photocraft %F`) was already looked for on the PATH
|
||||
Some(PathBuf::from(program)).filter(|p| p.is_absolute())
|
||||
// a bare name (`photocraft %F`) was already looked for on the PATH. Desktop entries are a
|
||||
// freedesktop (POSIX) format: absolute means a leading `/` on every platform the parser runs on
|
||||
// (`Path::is_absolute` wants a drive letter on Windows, where the tests run too)
|
||||
program.starts_with('/').then(|| PathBuf::from(program))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -191,6 +226,25 @@ mod tests {
|
||||
assert_eq!(args, [r"C:\Edits\a & b ^c %PATH% | d.psd"]);
|
||||
}
|
||||
|
||||
/// Open Original with several photos: one start of the editor with every file, which a
|
||||
/// single-window editor (DxO PhotoLab) needs to get them all.
|
||||
#[test]
|
||||
fn several_files_go_to_one_start_of_the_editor() {
|
||||
let paths = [r"D:\Pictures\a.dng".to_string(), r"D:\Pictures\b c.dng".to_string()];
|
||||
let args = |c: &Command| c.get_args().map(|a| a.to_string_lossy().into_owned()).collect::<Vec<_>>();
|
||||
let c = all_command(Launcher::Program(PathBuf::from(r"C:\DxO\DxO.PhotoLab.exe")), &paths).unwrap();
|
||||
assert_eq!(c.get_program(), r"C:\DxO\DxO.PhotoLab.exe");
|
||||
assert_eq!(args(&c), paths);
|
||||
let c = all_command(Launcher::MacApp("DxO PhotoLab 8".into()), &paths).unwrap();
|
||||
assert_eq!(
|
||||
(c.get_program().to_string_lossy().into_owned(), args(&c)),
|
||||
("open".into(), vec!["-a".into(), "DxO PhotoLab 8".into(), paths[0].clone(), paths[1].clone()])
|
||||
);
|
||||
let c = all_command(Launcher::Flatpak, &paths).unwrap();
|
||||
assert_eq!(args(&c), ["run", "--file-forwarding", PHOTOCRAFT_ID, "@@", &paths[0], &paths[1], "@@"]);
|
||||
assert!(all_command(Launcher::Default, &paths).is_none(), "the system default opens them one by one");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn windows_lookup_prefers_the_recorded_install_folder() {
|
||||
let env = |k: &str| match k {
|
||||
|
||||
@@ -0,0 +1,174 @@
|
||||
//! HDR display output (LR-VIEW-HDR-DISPLAY): whether the window can show the range above SDR
|
||||
//! white, and with which surface format. Pure decisions, unit-tested here; the window always
|
||||
//! falls back to SDR when any condition is missing, so an HDR edit is never shown wrong.
|
||||
//!
|
||||
//! HDR needs three things: a platform path (macOS EDR via an extended-range float Metal layer,
|
||||
//! Windows HDR via scRGB float or HDR10 swap chains, Wayland via the colour-management protocol),
|
||||
//! a surface format that carries values above 1.0 (`Rgba16Float`) or a 10-bit PQ signal
|
||||
//! (`Rgb10a2Unorm`), and a renderer that actually configured that format. eframe 0.36 picks the
|
||||
//! surface format itself (8-bit only, `egui_wgpu::preferred_framebuffer_format`), so today the
|
||||
//! choice reports SDR with the reason; the moment the renderer's target is an HDR format the same
|
||||
//! decision turns HDR on.
|
||||
|
||||
use eframe::wgpu::TextureFormat;
|
||||
|
||||
/// Windowing platform, as far as HDR output is concerned.
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
pub enum Platform {
|
||||
MacOs,
|
||||
Windows,
|
||||
Wayland,
|
||||
X11,
|
||||
Web,
|
||||
Other,
|
||||
}
|
||||
|
||||
impl Platform {
|
||||
/// The platform this build runs on (`wayland` from the session environment on Linux / BSD).
|
||||
pub fn current() -> Self {
|
||||
if cfg!(target_os = "macos") {
|
||||
Self::MacOs
|
||||
} else if cfg!(target_os = "windows") {
|
||||
Self::Windows
|
||||
} else if cfg!(target_arch = "wasm32") {
|
||||
Self::Web
|
||||
} else if std::env::var_os("WAYLAND_DISPLAY").is_some() {
|
||||
Self::Wayland
|
||||
} else if std::env::var_os("DISPLAY").is_some() {
|
||||
Self::X11
|
||||
} else {
|
||||
Self::Other
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether the platform has an HDR compositing path wgpu can reach at all.
|
||||
fn has_hdr_path(self) -> bool {
|
||||
matches!(self, Self::MacOs | Self::Windows | Self::Wayland)
|
||||
}
|
||||
}
|
||||
|
||||
/// How HDR values reach the display.
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
pub enum HdrEncoding {
|
||||
/// Linear float, SDR white at 1.0 and the headroom above it (macOS EDR, Windows scRGB,
|
||||
/// Wayland extended linear).
|
||||
ExtendedLinear,
|
||||
/// 10-bit BT.2100 PQ (Windows HDR10, Wayland PQ).
|
||||
Pq10,
|
||||
}
|
||||
|
||||
/// The display output decision.
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
pub enum DisplayOutput {
|
||||
Sdr { reason: &'static str },
|
||||
Hdr { format: TextureFormat, encoding: HdrEncoding },
|
||||
}
|
||||
|
||||
impl DisplayOutput {
|
||||
#[cfg_attr(not(test), allow(dead_code))] // read by the HDR view once the surface can be HDR
|
||||
pub fn is_hdr(self) -> bool {
|
||||
matches!(self, Self::Hdr { .. })
|
||||
}
|
||||
}
|
||||
|
||||
/// The encoding a surface format carries on `platform`, if it is an HDR one.
|
||||
fn encoding_of(format: TextureFormat, platform: Platform) -> Option<HdrEncoding> {
|
||||
match format {
|
||||
TextureFormat::Rgba16Float => Some(HdrEncoding::ExtendedLinear),
|
||||
// macOS takes HDR through EDR floats only
|
||||
TextureFormat::Rgb10a2Unorm if platform != Platform::MacOs => Some(HdrEncoding::Pq10),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
/// The best HDR surface format among those a surface offers (`available`), for `platform`:
|
||||
/// float first (keeps SDR white at 1.0 and needs no PQ encode), then 10-bit PQ. `None` = SDR only.
|
||||
/// (For configuring the surface once eframe lets the app pick its format.)
|
||||
#[cfg_attr(not(test), allow(dead_code))]
|
||||
pub fn preferred_hdr_format(available: &[TextureFormat], platform: Platform) -> Option<TextureFormat> {
|
||||
if !platform.has_hdr_path() {
|
||||
return None;
|
||||
}
|
||||
[TextureFormat::Rgba16Float, TextureFormat::Rgb10a2Unorm].into_iter().find(|f| available.contains(f) && encoding_of(*f, platform).is_some())
|
||||
}
|
||||
|
||||
/// The decision for the window: `target` is the format the renderer configured (`None`: the glow
|
||||
/// renderer or no wgpu state), `display_headroom` the display's current peak over SDR white as the
|
||||
/// OS reports it (`None`: unknown, `Some(1.0)`: an SDR display or HDR switched off in the OS),
|
||||
/// `disabled` the user's opt-out (`LIGHTCRAFT_HDR_DISPLAY=0`).
|
||||
pub fn choose(target: Option<TextureFormat>, platform: Platform, display_headroom: Option<f32>, disabled: bool) -> DisplayOutput {
|
||||
if disabled {
|
||||
return DisplayOutput::Sdr { reason: "HDR display is switched off" };
|
||||
}
|
||||
if !platform.has_hdr_path() {
|
||||
return DisplayOutput::Sdr { reason: "this windowing system has no HDR output" };
|
||||
}
|
||||
let Some(format) = target else {
|
||||
return DisplayOutput::Sdr { reason: "the window renderer is not wgpu" };
|
||||
};
|
||||
let Some(encoding) = encoding_of(format, platform) else {
|
||||
return DisplayOutput::Sdr { reason: "the window surface is 8-bit" };
|
||||
};
|
||||
match display_headroom {
|
||||
Some(h) if h.is_finite() && h > 1.0 => DisplayOutput::Hdr { format, encoding },
|
||||
Some(_) => DisplayOutput::Sdr { reason: "the display is in SDR mode" },
|
||||
None => DisplayOutput::Sdr { reason: "the display's HDR headroom is unknown" },
|
||||
}
|
||||
}
|
||||
|
||||
/// The decision for this start, logged once (the window renderer's `target` format).
|
||||
pub fn report(target: Option<TextureFormat>) -> DisplayOutput {
|
||||
let disabled = std::env::var("LIGHTCRAFT_HDR_DISPLAY").ok().is_some_and(|v| matches!(v.trim(), "0" | "off" | "false" | "no"));
|
||||
// no OS headroom query is wired yet: unknown, so SDR unless everything else is HDR-ready
|
||||
let out = choose(target, Platform::current(), None, disabled);
|
||||
match out {
|
||||
DisplayOutput::Sdr { reason } => log::info!("display: SDR ({reason})"),
|
||||
DisplayOutput::Hdr { format, encoding } => log::info!("display: HDR {encoding:?} ({format:?})"),
|
||||
}
|
||||
out
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use TextureFormat as F;
|
||||
|
||||
#[test]
|
||||
fn eight_bit_surfaces_are_sdr() {
|
||||
for p in [Platform::MacOs, Platform::Windows, Platform::Wayland] {
|
||||
for f in [F::Bgra8Unorm, F::Bgra8UnormSrgb, F::Rgba8Unorm] {
|
||||
assert!(!choose(Some(f), p, Some(4.0), false).is_hdr(), "{p:?} {f:?}");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hdr_needs_platform_format_and_headroom() {
|
||||
let hdr = DisplayOutput::Hdr { format: F::Rgba16Float, encoding: HdrEncoding::ExtendedLinear };
|
||||
assert_eq!(choose(Some(F::Rgba16Float), Platform::MacOs, Some(2.0), false), hdr);
|
||||
assert_eq!(
|
||||
choose(Some(F::Rgb10a2Unorm), Platform::Windows, Some(4.0), false),
|
||||
DisplayOutput::Hdr { format: F::Rgb10a2Unorm, encoding: HdrEncoding::Pq10 }
|
||||
);
|
||||
// any missing condition falls back to SDR
|
||||
assert!(!choose(Some(F::Rgba16Float), Platform::X11, Some(4.0), false).is_hdr());
|
||||
assert!(!choose(Some(F::Rgba16Float), Platform::Web, Some(4.0), false).is_hdr());
|
||||
assert!(!choose(Some(F::Rgba16Float), Platform::MacOs, Some(1.0), false).is_hdr());
|
||||
assert!(!choose(Some(F::Rgba16Float), Platform::MacOs, None, false).is_hdr());
|
||||
assert!(!choose(Some(F::Rgba16Float), Platform::MacOs, Some(f32::NAN), false).is_hdr());
|
||||
assert!(!choose(Some(F::Rgba16Float), Platform::MacOs, Some(2.0), true).is_hdr());
|
||||
assert!(!choose(None, Platform::Windows, Some(4.0), false).is_hdr());
|
||||
// macOS has no PQ swap chain path
|
||||
assert!(!choose(Some(F::Rgb10a2Unorm), Platform::MacOs, Some(4.0), false).is_hdr());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn preferred_format_picks_float_then_pq() {
|
||||
let all = [F::Bgra8Unorm, F::Rgb10a2Unorm, F::Rgba16Float];
|
||||
assert_eq!(preferred_hdr_format(&all, Platform::Windows), Some(F::Rgba16Float));
|
||||
assert_eq!(preferred_hdr_format(&[F::Bgra8Unorm, F::Rgb10a2Unorm], Platform::Wayland), Some(F::Rgb10a2Unorm));
|
||||
assert_eq!(preferred_hdr_format(&[F::Bgra8Unorm, F::Rgb10a2Unorm], Platform::MacOs), None);
|
||||
assert_eq!(preferred_hdr_format(&all, Platform::X11), None);
|
||||
assert_eq!(preferred_hdr_format(&[], Platform::MacOs), None);
|
||||
}
|
||||
}
|
||||
@@ -24,9 +24,12 @@
|
||||
|
||||
mod alloc_release;
|
||||
mod appearance;
|
||||
#[cfg(target_os = "linux")]
|
||||
mod colord;
|
||||
mod control_server;
|
||||
mod dialog_filter;
|
||||
mod editors;
|
||||
mod hdr_display;
|
||||
mod logging;
|
||||
#[cfg(target_os = "macos")]
|
||||
mod native_menu;
|
||||
@@ -87,6 +90,16 @@ impl eframe::App for App {
|
||||
if !self.0.shutdown(EXIT_WAIT) {
|
||||
log::warn!("quit: background work was still running after {EXIT_WAIT:?}");
|
||||
}
|
||||
// macOS (#1331): end the process here, before eframe tears the window down, as Quit from
|
||||
// the menu bar already does (PR #444). AccessKit's macOS adapter subclasses the window's
|
||||
// content view and restores its class on teardown; AppKit's Touch Bar support observes
|
||||
// that view (KVO), so in the next display cycle `-[_NSTouchBarFinderObservation
|
||||
// invalidate]` throws "not registered" and closing the window ends in a crash report.
|
||||
// Nothing after this is needed: settings and library are saved, background work has
|
||||
// stopped and the log file is unbuffered. `on_exit` only runs once the app exists, so a
|
||||
// renderer that fails to start still relaunches on the next one (`window_gpu`).
|
||||
#[cfg(target_os = "macos")]
|
||||
std::process::exit(0);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -450,6 +463,7 @@ fn services(cc: &eframe::CreationContext<'_>, log_file: Option<&std::path::Path>
|
||||
file_dialog().set_title(lightcraft_ui_egui::i18n::tr("Open Library")).pick_folder().map(|p| p.to_string_lossy().to_string())
|
||||
})),
|
||||
open_with: Some(Box::new(editors::open_with)),
|
||||
open_all_with: Some(Box::new(editors::open_all_with)),
|
||||
check_editor: Some(Box::new(editors::check)),
|
||||
open_url: Some(Box::new(|url: &str| {
|
||||
if !url.starts_with("https://") {
|
||||
@@ -525,7 +539,10 @@ fn services(cc: &eframe::CreationContext<'_>, log_file: Option<&std::path::Path>
|
||||
// macOS doesn't colour-manage the window: previews follow the screen's profile (#787)
|
||||
#[cfg(target_os = "macos")]
|
||||
screen_profile: Some(Box::new(screen_profile::icc)),
|
||||
#[cfg(not(target_os = "macos"))]
|
||||
// Linux: the profile the desktop assigns in colord, read off the UI thread
|
||||
#[cfg(target_os = "linux")]
|
||||
screen_profile: Some(colord::service()),
|
||||
#[cfg(not(any(target_os = "macos", target_os = "linux")))]
|
||||
screen_profile: None,
|
||||
pick_display_profile: Some(Box::new(|| {
|
||||
let mut d = file_dialog()
|
||||
@@ -833,6 +850,8 @@ fn main() -> eframe::Result {
|
||||
// (never required: without it, AI masks offer to download it; see docs/ai-masks.md)
|
||||
// and the user's own download locations, one base URL per line (LIGHTCRAFT_SAM3_MIRRORS too)
|
||||
session = session.with_default_sam3_model();
|
||||
// HDR display: SDR unless the surface, platform and display all allow HDR
|
||||
hdr_display::report(cc.wgpu_render_state.as_ref().map(|r| r.target_format));
|
||||
let mut app = LightcraftApp::new(session, services(cc, app_log_file.as_deref()));
|
||||
if let Some(ui) = prefs {
|
||||
app.ui = ui;
|
||||
@@ -842,6 +861,7 @@ fn main() -> eframe::Result {
|
||||
app.notices.extend(prefs_warning);
|
||||
app.notices.extend(launch_notices);
|
||||
app.notices.extend(window_notice);
|
||||
app.gpu_notice = true;
|
||||
// what's on disk now: only changes are written
|
||||
let writer = PrefsWriter {
|
||||
path: if in_memory { None } else { prefs_path() },
|
||||
@@ -1207,6 +1227,35 @@ mod tests {
|
||||
assert!(loaded.is_some() && warning.is_none());
|
||||
let _ = std::fs::remove_dir_all(&d);
|
||||
}
|
||||
|
||||
/// #1331: on macOS the process ends inside `on_exit`, before eframe would tear the window down.
|
||||
/// The check runs in a child (this test binary again), since on macOS it ends the process.
|
||||
#[test]
|
||||
fn on_exit_ends_the_process_on_macos() {
|
||||
if std::env::var_os("LIGHTCRAFT_TEST_ON_EXIT").is_some() {
|
||||
let app = LightcraftApp::new(lightcraft_engine::Session::new(), Services::default());
|
||||
let mut app = App(
|
||||
app,
|
||||
PrefsWriter::default(),
|
||||
#[cfg(target_os = "macos")]
|
||||
None,
|
||||
);
|
||||
println!("calling on_exit");
|
||||
eframe::App::on_exit(&mut app, None);
|
||||
println!("on_exit returned");
|
||||
return;
|
||||
}
|
||||
let out = std::process::Command::new(std::env::current_exe().unwrap())
|
||||
.args(["--exact", "tests::on_exit_ends_the_process_on_macos", "--nocapture"])
|
||||
.env("LIGHTCRAFT_TEST_ON_EXIT", "1")
|
||||
.env("LIGHTCRAFT_NO_PREFS", "1")
|
||||
.output()
|
||||
.unwrap();
|
||||
let stdout = String::from_utf8_lossy(&out.stdout);
|
||||
assert!(out.status.success(), "{stdout}");
|
||||
assert!(stdout.contains("calling on_exit"), "the child didn't run the test: {stdout}");
|
||||
assert_eq!(stdout.contains("on_exit returned"), !cfg!(target_os = "macos"), "{stdout}");
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
Reference in New Issue
Block a user