Merge pull request #659 from storytold/claude/integrate-batch-24

Integrate #607 follow-up, #655, #656
This commit is contained in:
Brandon Thomas
2026-10-10 02:18:46 -04:00
committed by GitHub
19 changed files with 924 additions and 73 deletions
+8
View File
@@ -78,9 +78,17 @@ impl eframe::App for App {
if let Err(e) = self.0.session.close_library() {
log::error!("saving the library failed: {e}");
}
// last, with everything saved: no render may be inside the GPU driver when the process ends
if !self.0.shutdown(EXIT_WAIT) {
log::warn!("quit: background work was still running after {EXIT_WAIT:?}");
}
}
}
/// How long quitting waits for renders that are running (issue #620). A GPU render takes well under
/// a second; anything slower gives way to quitting.
const EXIT_WAIT: std::time::Duration = std::time::Duration::from_secs(2);
/// The window's renderer (egui-wgpu): eframe's defaults, with LightCraft's backend choice — DX12
/// alone on Windows unless `LIGHTCRAFT_GPU_BACKEND` / `WGPU_BACKEND` say otherwise (issue #136:
/// with Vulkan in the set, wgpu loads the Vulkan driver even when it then picks DX12) — and DX12
+51
View File
@@ -1084,6 +1084,57 @@ mod tests {
}
}
/// A band clipped in green whose red rises through its own clip from left to right (sensor `[0.985 + 0.01 t, 1,
/// 0.8]`, t from 0 to 1 across 320 px), `band` rows at the bottom of the picture, below `strip` rows of a pale
/// surface just under the clip (`[0.4, 0.95, 0.8]`: the band's rim, of another colour) and above those `top` rows
/// of a strongly coloured reliable surface (`[0.8, 0.35, 0.3]`). Rebuilt and rendered at Exposure -2: the
/// rendered colour on either side of where red clips on the band's middle row, and the largest step between
/// neighbouring pixels along that row.
fn second_clip(top: usize, strip: usize, band: usize) -> ([[u8; 4]; 2], u8) {
use lightcraft_pipeline::{RenderRequest, render};
let wb = [2.5f32, 1.0, 1.6];
let colour = CameraColourModel { wb, matrix: lightcraft_color::Mat3::IDENTITY, gain: 1.0, tables: None, hue_sat: None };
let (w, h) = (320usize, top + strip + band);
let sensor = Rgb32f::from_fn(w, h, |x, y| {
if y < top {
[0.8, 0.35, 0.3]
} else if y < top + strip {
[0.4, 0.95, 0.8]
} else {
[0.985 + 0.01 * x as f32 / (w - 1) as f32, 1.0, 0.8]
}
});
let y = top + strip + band / 2;
let at = (0..w).find(|&x| sensor.get(x, y)[0] >= HIGHLIGHT_CLIP).unwrap();
let mut img = sensor.clone();
colour.rebuild_highlights(&mut img);
colour.apply(&mut img);
let mut s = lightcraft_develop::DevelopSettings::default();
s.light.exposure = -2.0;
let out = render(&img, &SourceInfo { raw: true, ..Default::default() }, &s, &RenderRequest::fit(w, h)).image;
let step = |x: usize| {
let (a, b) = (out.get(x - 1, y), out.get(x, y));
(0..3).map(|c| a[c].abs_diff(b[c])).max().unwrap_or(0)
};
([out.get(at - 1, y), out.get(at, y)], (1..w).map(step).max().unwrap_or(0))
}
/// Where a second channel clips within reach of the surface's own rim, the rebuilt colour carries on too. The
/// rim's comparison with the pixel used to drop each clipped channel, so a rim far from the pixel in red suddenly
/// matched it once red clipped (80 rows of colour, 16 of rim and 32 of band: [237, 124, 153] then
/// [205, 181, 168]); and a channel about to clip counted as an uncapped floor that was capped once it clipped
/// (without the coloured surface: [198, 195, 178] then [205, 181, 168]).
#[test]
fn rendered_colour_is_continuous_where_a_second_channel_clips() {
for (top, strip, band) in [(80, 16, 32), (0, 16, 32), (80, 16, 4), (200, 4, 8), (16, 64, 16), (8, 32, 24)] {
let ([a, b], worst) = second_clip(top, strip, band);
let case = format!("{top} rows of colour, {strip} of rim, {band} of band");
let step = (0..3).map(|i| a[i].abs_diff(b[i])).max().unwrap_or(0);
assert!(step <= 1, "{case}: {a:?} then {b:?} where red clips");
assert!(worst <= 3, "{case}: largest step between neighbouring pixels {worst} levels");
}
}
/// Issue #523 on a public ILCE-7RM4 raw (raw.pixls.us, CC0; skipped without the corpus): a dusk sky clipped in
/// green behind a poplar. Its rebuilt colour used to come from the tree's foliage and lens fringes, a green halo
/// on one side of the tree and a magenta one on the other (Highlights -100 or the issue's edit show them).
+9 -2
View File
@@ -62,7 +62,7 @@ impl SettingsHashes {
}
/// Bump when the pipeline's output changes, to invalidate cached thumbnails.
pub const RENDER_CACHE_VERSION: u64 = 36;
pub const RENDER_CACHE_VERSION: u64 = 37;
/// Thumbnails render at one of these long edges (so window/cell size changes reuse the cache).
pub const THUMB_SIZES: [usize; 4] = [128, 256, 384, 512];
@@ -773,7 +773,14 @@ impl RenderJob {
};
}
let was_loaded = matches!(self.source, SourceRef::Loaded(_));
match self.source.load_source() {
// The app is closing (issue #620): nobody is left to show this, and quitting waits for the
// jobs that are running. Without this, one that lost the GPU mid-way would render on the CPU.
let closing = || lightcraft_gpu::shutting_down().then(|| "LightCraft is closing".to_string());
let source = match closing() {
Some(e) => Err(e),
None => self.source.load_source().and_then(|s| closing().map_or(Ok(s), Err)),
};
match source {
Ok(source) => {
// the plain picture, or (Denoise amount above 0 and a denoised picture cached) the mix the amount asks for
let src = &source.image_for(self.settings.denoise_amount());
+132
View File
@@ -0,0 +1,132 @@
//! Leaving: no GPU work may be in flight when the process ends (issue #620).
//!
//! The devices are statics that are never dropped, and the threads that render (preview workers,
//! export, denoise, the device warm-up) are not the one that ends the process. Exiting runs the
//! driver's own teardown while those threads keep running: a render that is inside the driver at
//! that moment crashes there (seen as a SIGSEGV in the NVIDIA Vulkan driver), where neither the
//! panic hook nor `catch_unwind` can help.
//!
//! So every entry into a device goes through [`enter`], and the host ends the process with
//! [`crate::begin_shutdown`] (nothing new enters) and [`crate::wait_idle`] (what is in flight
//! finishes, up to a deadline).
use std::sync::{Condvar, Mutex, MutexGuard};
use std::time::{Duration, Instant};
struct State {
closed: bool,
/// [`Work`] in flight, on any thread.
busy: usize,
}
pub(crate) struct Gate {
state: Mutex<State>,
idle: Condvar,
}
/// GPU work in flight (until dropped).
pub(crate) struct Work<'a>(&'a Gate);
static GATE: Gate = Gate::new();
/// Start GPU work, or `None` when the process is ending: the caller then does without the GPU.
/// Work may nest (a render that creates the device).
pub(crate) fn enter() -> Option<Work<'static>> {
GATE.enter()
}
pub(crate) fn begin_shutdown() {
GATE.close();
}
pub(crate) fn shutting_down() -> bool {
GATE.lock().closed
}
pub(crate) fn wait_idle(timeout: Duration) -> bool {
GATE.wait_idle(timeout)
}
impl Gate {
pub const fn new() -> Gate {
Gate { state: Mutex::new(State { closed: false, busy: 0 }), idle: Condvar::new() }
}
fn lock(&self) -> MutexGuard<'_, State> {
self.state.lock().unwrap_or_else(|e| e.into_inner())
}
pub fn enter(&self) -> Option<Work<'_>> {
let mut s = self.lock();
if s.closed {
return None;
}
s.busy = s.busy.saturating_add(1);
Some(Work(self))
}
/// Nothing enters from now on.
pub fn close(&self) {
self.lock().closed = true;
}
/// Wait up to `timeout` until no work is in flight; `false` when some still is.
pub fn wait_idle(&self, timeout: Duration) -> bool {
let t0 = Instant::now();
let mut s = self.lock();
while s.busy > 0 {
let Some(left) = timeout.checked_sub(t0.elapsed()).filter(|d| !d.is_zero()) else { return false };
s = self.idle.wait_timeout(s, left).unwrap_or_else(|e| e.into_inner()).0;
}
true
}
}
impl Drop for Work<'_> {
fn drop(&mut self) {
let mut s = self.0.lock();
s.busy = s.busy.saturating_sub(1);
if s.busy == 0 {
drop(s);
self.0.idle.notify_all();
}
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn closed_gate_lets_nothing_in_and_waits_for_what_is_inside() {
let gate = Gate::new();
assert!(gate.wait_idle(Duration::ZERO), "idle: no wait");
let outer = gate.enter().unwrap();
let inner = gate.enter().unwrap();
gate.close();
assert!(gate.enter().is_none(), "nothing new once closed");
drop(inner);
let t0 = Instant::now();
assert!(!gate.wait_idle(Duration::from_millis(50)), "the outer work is still in flight");
assert!(t0.elapsed() >= Duration::from_millis(50));
std::thread::scope(|s| {
s.spawn(move || {
std::thread::sleep(Duration::from_millis(100));
drop(outer);
});
assert!(gate.wait_idle(Duration::from_secs(30)), "woken when the work ends");
});
assert!(gate.enter().is_none());
}
#[test]
fn work_that_panics_still_leaves() {
let gate = Gate::new();
let r = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
let _work = gate.enter();
panic!("a render panics (expected in this test)");
}));
assert!(r.is_err());
assert!(gate.wait_idle(Duration::ZERO));
}
}
+52 -1
View File
@@ -15,6 +15,8 @@
//! and [`last_fallback`] say why (`ui.inspect` → `perf.gpuReason` / `perf.gpuFallback`).
//! The browser build has no GPU path yet (WebGPU device creation is asynchronous): everything here
//! compiles to the CPU fallback on wasm32.
//!
//! Before the process ends: [`begin_shutdown`], then [`wait_idle`] (issue #620, see `exit.rs`).
#![forbid(unsafe_code)]
#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]
@@ -29,6 +31,8 @@ use lightcraft_raster::Rgb32f;
pub mod backend;
#[cfg(not(target_arch = "wasm32"))]
mod ctx;
#[cfg(not(target_arch = "wasm32"))]
mod exit;
#[cfg(all(feature = "denoise", not(target_arch = "wasm32")))]
pub mod nn;
#[cfg(not(target_arch = "wasm32"))]
@@ -77,6 +81,9 @@ pub fn unavailable_reason() -> Option<String> {
if !ENABLED.load(Ordering::Relaxed) {
return Some("disabled by the GPU rendering preference (app.gpu)".into());
}
if shutting_down() {
return Some("LightCraft is closing".into());
}
if BROKEN.load(Ordering::Relaxed) {
let r = BROKEN_REASON.lock().unwrap_or_else(|e| e.into_inner()).clone().unwrap_or_else(|| "device error".into());
return Some(format!("stopped after a GPU failure: {r}"));
@@ -147,6 +154,41 @@ pub fn enabled() -> bool {
ENABLED.load(Ordering::Relaxed) && !BROKEN.load(Ordering::Relaxed) && !env_disabled()
}
/// The process is about to end: no GPU work starts from now on, on any thread. [`render`] returns
/// `None` (callers render on the CPU), the devices are not created and denoise tiles fail over to
/// the CPU. There is no way back. Follow it with [`wait_idle`].
pub fn begin_shutdown() {
#[cfg(not(target_arch = "wasm32"))]
exit::begin_shutdown();
}
/// Whether [`begin_shutdown`] was called.
pub fn shutting_down() -> bool {
#[cfg(not(target_arch = "wasm32"))]
{
exit::shutting_down()
}
#[cfg(target_arch = "wasm32")]
{
false
}
}
/// Wait up to `timeout` for the GPU work in flight on every thread to end. `true`: none is left,
/// and after [`begin_shutdown`] none can start, so the process may end; `false`: the deadline
/// passed with a render (or a device being created) still inside the driver.
pub fn wait_idle(timeout: std::time::Duration) -> bool {
#[cfg(not(target_arch = "wasm32"))]
{
exit::wait_idle(timeout)
}
#[cfg(target_arch = "wasm32")]
{
let _ = timeout;
true
}
}
/// Why GPU compute that has a device of its own (denoise) is switched off by the user's settings, or `None`: the
/// environment switches and the rendering preference, but not a render-device failure (the devices are separate).
#[cfg(all(feature = "denoise", not(target_arch = "wasm32")))]
@@ -177,6 +219,11 @@ pub(crate) fn device() -> Option<&'static ctx::Gpu> {
if env_disabled() || !ENABLED.load(Ordering::Relaxed) {
return existing_device();
}
if let Some(g) = GPU.get() {
return g.as_ref().ok();
}
// creating the device is a long call into the driver: not once the process is ending
let _work = exit::enter()?;
GPU.get_or_init(|| {
let Some(backends) = backend::compute_backends() else { return Err("disabled by LIGHTCRAFT_GPU_BACKEND=off".into()) };
backend::with_init_marker(backends, || {
@@ -281,7 +328,9 @@ pub fn set_pool_limit(bytes: u64) {
/// Free recycled buffers until at most `keep` bytes stay pooled (e.g. when the app goes idle).
pub fn trim_pool(keep: u64) {
#[cfg(not(target_arch = "wasm32"))]
if let Some(g) = existing_device() {
if let Some(g) = existing_device()
&& let Some(_work) = exit::enter()
{
if keep == 0 {
render::release_shared_source();
}
@@ -346,6 +395,8 @@ pub fn render(src: &Arc<Rgb32f>, info: &SourceInfo, s: &DevelopSettings, req: &R
return None;
}
let s = &*lightcraft_pipeline::settings_for(s, req);
// in flight until this returns; `None` once the process is ending (issue #620)
let _work = exit::enter()?;
let gpu = device()?;
let ext = stages.map(|c| c.extension::<GpuStages>());
let fault = take_fault();
+11
View File
@@ -63,9 +63,17 @@ fn dev() -> Result<&'static Dev, String> {
if let Some(why) = crate::switched_off() {
return Err(why);
}
// creating the device is GPU work too: not once the process is ending (issue #620)
let _work = match DEV.get() {
Some(_) => None,
None => Some(crate::exit::enter().ok_or(CLOSING)?),
};
DEV.get_or_init(create_device).as_ref().map_err(|e| e.clone())
}
/// Why nothing runs on the GPU once [`crate::begin_shutdown`] was called.
const CLOSING: &str = "LightCraft is closing";
fn create_device() -> Result<Dev, String> {
let Some(backends) = crate::backend::compute_backends() else { return Err("disabled by LIGHTCRAFT_GPU_BACKEND=off".into()) };
crate::backend::with_init_marker(backends, || {
@@ -653,6 +661,7 @@ pub struct NetRunner {
/// Put `net` on the GPU for `tile × tile` input cells, or say why it cannot be (no GPU, a layer or a size the kernels
/// do not do, the device refusing the buffers). Creates the device on first use and compiles the kernels.
pub fn runner(net: &Net, tile: usize) -> Result<NetRunner, String> {
let _work = crate::exit::enter().ok_or(CLOSING)?;
let d = dev()?;
let max_binding = d.limits.max_storage_buffer_binding_size.min(d.limits.max_buffer_size).min(MAX_BUFFER);
let (layout, layers) = plan(net, tile, max_binding)?;
@@ -839,6 +848,8 @@ impl TileRunner for NetRunner {
if input.len() != self.shared.layout.in_floats {
return Err(Error::Input("the tile is not the size the model wants".into()));
}
// one tile is one piece of GPU work: a photo's worth would outlast any wait at exit
let Some(_work) = crate::exit::enter() else { return Err(Error::Runtime(CLOSING.into())) };
let w = self.take()?;
let r = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| self.execute(&w, input)));
match r {
+58
View File
@@ -0,0 +1,58 @@
//! Issue #620: a render still inside the GPU driver while the process exits crashes there (a
//! SIGSEGV no panic hook catches). Once `begin_shutdown` is called nothing starts on the GPU, and
//! `wait_idle` returns when the renders in flight on other threads have ended.
//! One test: the shutdown holds for the whole process. Without a GPU adapter only the CPU side of
//! the contract is checked.
use std::sync::Arc;
use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
use std::time::Duration;
use lightcraft_develop::DevelopSettings;
use lightcraft_pipeline::{RenderRequest, SourceInfo};
#[test]
fn nothing_renders_on_the_gpu_once_the_process_is_ending() {
let (w, h) = (600, 400);
let src = Arc::new(lightcraft_scenes::demo_library()[0].render(w, h));
let info = SourceInfo { raw: true, ..Default::default() };
let mut s = DevelopSettings::default();
s.light.exposure = 0.3;
let req = RenderRequest::fit(w, h);
let has_gpu = lightcraft_gpu::available();
if !has_gpu {
eprintln!("no GPU adapter ({:?}): only the CPU side is checked", lightcraft_gpu::unavailable_reason());
}
assert!(!lightcraft_gpu::shutting_down());
assert!(lightcraft_gpu::wait_idle(Duration::ZERO), "nothing is in flight yet");
// a worker that renders until the GPU refuses, as the preview workers do while the app quits
let (rendered, refused) = (Arc::new(AtomicUsize::new(0)), Arc::new(AtomicBool::new(false)));
let worker = {
let (src, info, s) = (src.clone(), info.clone(), s.clone());
let (rendered, refused) = (rendered.clone(), refused.clone());
std::thread::spawn(move || {
while lightcraft_gpu::render(&src, &info, &s, &req, None).is_some() {
rendered.fetch_add(1, Ordering::SeqCst);
}
refused.store(true, Ordering::SeqCst);
})
};
if has_gpu {
while rendered.load(Ordering::SeqCst) == 0 && !refused.load(Ordering::SeqCst) {
std::thread::yield_now();
}
assert!(rendered.load(Ordering::SeqCst) > 0, "the GPU renders before the shutdown ({:?})", lightcraft_gpu::last_fallback());
}
lightcraft_gpu::begin_shutdown();
assert!(lightcraft_gpu::shutting_down());
assert!(lightcraft_gpu::wait_idle(Duration::from_secs(60)), "the render in flight ends");
worker.join().unwrap();
assert!(refused.load(Ordering::SeqCst));
assert!(lightcraft_gpu::render(&src, &info, &s, &req, None).is_none(), "no GPU render once the process is ending");
assert!(lightcraft_gpu::wait_idle(Duration::ZERO));
if has_gpu {
assert_eq!(lightcraft_gpu::unavailable_reason().as_deref(), Some("LightCraft is closing"));
}
}
+25
View File
@@ -0,0 +1,25 @@
//! Issue #620: quitting right after the start must not leave the device being created (a long
//! call into the driver, on the `lc-gpu-init` thread) while the process exits. Once
//! `begin_shutdown` is called the device is not created at all.
//! One test in its own process: the shutdown comes before the first use of the GPU.
use std::sync::Arc;
use std::time::Duration;
use lightcraft_develop::DevelopSettings;
use lightcraft_pipeline::{RenderRequest, SourceInfo};
#[test]
fn the_device_is_not_created_once_the_process_is_ending() {
lightcraft_gpu::begin_shutdown();
lightcraft_gpu::warm_up();
assert!(!lightcraft_gpu::available());
assert_eq!(lightcraft_gpu::adapter_name(), None);
let src = Arc::new(lightcraft_scenes::demo_library()[0].render(300, 200));
let rendered = lightcraft_gpu::render(&src, &SourceInfo::default(), &DevelopSettings::default(), &RenderRequest::fit(300, 200), None);
assert!(rendered.is_none());
assert!(lightcraft_gpu::wait_idle(Duration::from_secs(60)), "the warm-up thread found the gate closed");
// `ready()` is "device creation finished" (or the GPU is switched off by the environment)
let switched_off = lightcraft_gpu::unavailable_reason().is_some_and(|r| r.starts_with("disabled by LIGHTCRAFT_GPU"));
assert!(switched_off || !lightcraft_gpu::ready(), "no device was created");
}
+145 -15
View File
@@ -7,11 +7,16 @@
//!
//! Native: a fixed set of worker threads, started on first use. wasm32 (no threads): the host
//! calls [`JobPool::run_inline`] once per frame.
//!
//! No worker outlives its pool by more than a deadline (issue #620: a render still inside the GPU
//! driver while the process exits crashes there): [`JobPool::shutdown`] and dropping the pool
//! discard the queued jobs and wait, bounded, for the running ones.
use std::hash::Hash;
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::mpsc::{Receiver, Sender, channel};
use std::sync::{Arc, Condvar, Mutex};
use std::time::Duration;
pub type Job<R> = Box<dyn FnOnce() -> R + Send>;
@@ -27,8 +32,16 @@ struct Shared<S, R> {
queue: Mutex<Vec<Entry<S, R>>>,
cv: Condvar,
shutdown: AtomicBool,
/// The worker threads that have not ended yet, and the signal that one did.
#[cfg(not(target_arch = "wasm32"))]
live: Mutex<Vec<std::thread::ThreadId>>,
#[cfg(not(target_arch = "wasm32"))]
ended: Condvar,
}
/// How long dropping a pool waits for the jobs still running (see [`JobPool::shutdown`]).
const DROP_WAIT: Duration = Duration::from_secs(2);
/// A finished job.
pub struct Done<S, R> {
pub slot: S,
@@ -47,6 +60,10 @@ pub struct JobPool<S, R> {
threads: usize,
started: bool,
seq: u64,
#[cfg(not(target_arch = "wasm32"))]
workers: Vec<std::thread::JoinHandle<()>>,
/// [`JobPool::shutdown`] ran: dropping the pool doesn't wait a second time.
stopped: bool,
}
impl<S: Copy + Eq + Hash + Send + 'static, R: Send + 'static> JobPool<S, R> {
@@ -54,12 +71,23 @@ impl<S: Copy + Eq + Hash + Send + 'static, R: Send + 'static> JobPool<S, R> {
pub fn new(threads: usize) -> Self {
let (tx, rx) = channel();
JobPool {
shared: Arc::new(Shared { queue: Mutex::new(Vec::new()), cv: Condvar::new(), shutdown: AtomicBool::new(false) }),
shared: Arc::new(Shared {
queue: Mutex::new(Vec::new()),
cv: Condvar::new(),
shutdown: AtomicBool::new(false),
#[cfg(not(target_arch = "wasm32"))]
live: Mutex::new(Vec::new()),
#[cfg(not(target_arch = "wasm32"))]
ended: Condvar::new(),
}),
tx,
rx,
threads,
started: false,
seq: 0,
#[cfg(not(target_arch = "wasm32"))]
workers: Vec::new(),
stopped: false,
}
}
@@ -81,24 +109,69 @@ impl<S: Copy + Eq + Hash + Send + 'static, R: Send + 'static> JobPool<S, R> {
}
self.started = true;
#[cfg(not(target_arch = "wasm32"))]
for i in 0..self.threads {
let shared = self.shared.clone();
let tx = self.tx.clone();
let _ = std::thread::Builder::new().name(format!("lc-job-{i}")).spawn(move || {
while let Some(e) = take(&shared, true) {
let t0 = std::time::Instant::now();
let result = (e.job)();
let done = Done { slot: e.slot, key: e.key, result, ms: t0.elapsed().as_secs_f64() * 1000.0 };
if tx.send(done).is_err() {
break;
{
// held while spawning: a worker that ends at once still finds its id listed
let mut live = self.shared.live.lock().unwrap_or_else(|e| e.into_inner());
for i in 0..self.threads {
let shared = self.shared.clone();
let tx = self.tx.clone();
let spawned = std::thread::Builder::new().name(format!("lc-job-{i}")).spawn(move || {
let _ended = Ended(&shared);
while let Some(e) = take(&shared, true) {
let t0 = std::time::Instant::now();
let result = (e.job)();
let done = Done { slot: e.slot, key: e.key, result, ms: t0.elapsed().as_secs_f64() * 1000.0 };
if tx.send(done).is_err() {
break;
}
}
});
match spawned {
Ok(h) => {
live.push(h.thread().id());
self.workers.push(h);
}
Err(e) => log::error!("job pool: worker {i} could not start: {e}"),
}
});
}
}
}
/// Stop the pool: queued jobs are dropped, no new one starts, and the call waits up to
/// `timeout` for the jobs that are running. Returns whether every worker has ended; one that
/// hasn't is left to finish on its own. For owners that are about to end the process, where
/// dropping the pool may come too late or never (`std::process::exit`).
pub fn shutdown(&mut self, timeout: Duration) -> bool {
self.stopped = true;
#[cfg(not(target_arch = "wasm32"))]
{
stop(&self.shared, &mut self.workers, timeout)
}
#[cfg(target_arch = "wasm32")]
{
let _ = timeout;
close(&self.shared);
true
}
}
/// Worker threads that have not ended (0 before the first job, and on wasm).
pub fn live_workers(&self) -> usize {
#[cfg(not(target_arch = "wasm32"))]
{
self.shared.live.lock().map(|l| l.len()).unwrap_or(0)
}
#[cfg(target_arch = "wasm32")]
{
0
}
}
/// Queue `job` for `slot`, replacing a queued (not yet running) job for the same slot.
pub fn submit(&mut self, slot: S, key: u64, priority: u32, job: Job<R>) {
if self.stopped {
return; // nothing runs it any more
}
self.start();
self.seq += 1;
let (lock, cv) = (&self.shared.queue, &self.shared.cv);
@@ -172,9 +245,66 @@ fn take<S, R>(shared: &Shared<S, R>, wait: bool) -> Option<Entry<S, R>> {
}
}
impl<S, R> Drop for JobPool<S, R> {
/// Takes a worker off the live list when its thread ends (also when a job panicked).
#[cfg(not(target_arch = "wasm32"))]
struct Ended<'a, S, R>(&'a Shared<S, R>);
#[cfg(not(target_arch = "wasm32"))]
impl<S, R> Drop for Ended<'_, S, R> {
fn drop(&mut self) {
self.shared.shutdown.store(true, Ordering::Relaxed);
self.shared.cv.notify_all();
let me = std::thread::current().id();
self.0.live.lock().unwrap_or_else(|e| e.into_inner()).retain(|id| *id != me);
self.0.ended.notify_all();
}
}
/// Tell the workers to stop and take the queued jobs away from them.
fn close<S, R>(shared: &Shared<S, R>) {
// the flag changes under the queue lock: a worker is either before its check or waiting
let queued = {
let mut q = shared.queue.lock().unwrap_or_else(|e| e.into_inner());
shared.shutdown.store(true, Ordering::Relaxed);
std::mem::take(&mut *q)
};
shared.cv.notify_all();
drop(queued); // outside the lock: a job may own a lot
}
/// [`JobPool::shutdown`]: wait up to `timeout` for the workers to end, without holding the queue
/// lock, and never for the calling thread (a pool dropped by one of its own jobs).
#[cfg(not(target_arch = "wasm32"))]
fn stop<S, R>(shared: &Shared<S, R>, workers: &mut Vec<std::thread::JoinHandle<()>>, timeout: Duration) -> bool {
close(shared);
let me = std::thread::current().id();
let t0 = std::time::Instant::now();
let mut live = shared.live.lock().unwrap_or_else(|e| e.into_inner());
while live.iter().any(|id| *id != me) {
let Some(left) = timeout.checked_sub(t0.elapsed()).filter(|d| !d.is_zero()) else { break };
live = shared.ended.wait_timeout(live, left).unwrap_or_else(|e| e.into_inner()).0;
}
let running: Vec<std::thread::ThreadId> = live.iter().copied().filter(|id| *id != me).collect();
drop(live);
for h in workers.drain(..) {
let id = h.thread().id();
// a worker off the list is past its last job: joining it only waits for the thread to unwind
if id != me && !running.contains(&id) {
let _ = h.join();
}
}
if !running.is_empty() {
log::warn!("job pool: {} job(s) still running after {timeout:?}; not waiting for them", running.len());
}
running.is_empty()
}
impl<S, R> Drop for JobPool<S, R> {
fn drop(&mut self) {
if self.stopped {
return;
}
#[cfg(not(target_arch = "wasm32"))]
stop(&self.shared, &mut self.workers, DROP_WAIT);
#[cfg(target_arch = "wasm32")]
close(&self.shared);
}
}
+113
View File
@@ -201,6 +201,119 @@ fn pool_runs_inline() {
assert_eq!(p.try_recv().map(|d| d.result), Some(1));
}
/// A pool with one worker that is inside a job until the returned sender is used or dropped.
fn pool_with_a_running_job() -> (JobPool<u8, ()>, std::sync::mpsc::Sender<()>) {
let mut p: JobPool<u8, ()> = JobPool::new(1);
let (started, has_started) = std::sync::mpsc::channel();
let (release, released) = std::sync::mpsc::channel::<()>();
p.submit(
1,
0,
1,
Box::new(move || {
let _ = started.send(());
let _ = released.recv_timeout(std::time::Duration::from_secs(60));
}),
);
has_started.recv().unwrap();
(p, release)
}
#[test]
fn pool_drop_waits_for_running_job() {
// issue #620: a job that outlives its pool can still be inside the GPU driver when the process exits
use std::sync::atomic::{AtomicBool, Ordering};
let mut p: JobPool<u8, ()> = JobPool::new(1);
let (started, has_started) = std::sync::mpsc::channel();
let (finished, queued_ran) = (Arc::new(AtomicBool::new(false)), Arc::new(AtomicBool::new(false)));
let f = finished.clone();
p.submit(
1,
0,
9,
Box::new(move || {
let _ = started.send(());
std::thread::sleep(std::time::Duration::from_millis(200));
f.store(true, Ordering::SeqCst);
}),
);
has_started.recv().unwrap();
let q = queued_ran.clone();
p.submit(2, 0, 1, Box::new(move || q.store(true, Ordering::SeqCst)));
drop(p);
assert!(finished.load(Ordering::SeqCst), "drop returned while the job was still running");
assert!(!queued_ran.load(Ordering::SeqCst), "a job that had not started is discarded");
}
#[test]
fn pool_shutdown_reports_workers_and_drop_does_not_wait_again() {
let (mut p, release) = pool_with_a_running_job();
assert_eq!(p.live_workers(), 1);
let t0 = std::time::Instant::now();
assert!(!p.shutdown(std::time::Duration::from_millis(100)), "the job is still running");
let waited = t0.elapsed();
assert!(waited >= std::time::Duration::from_millis(100) && waited < std::time::Duration::from_secs(30), "{waited:?}");
assert_eq!(p.live_workers(), 1);
// stopped: nothing new is queued, and the job that ends is noticed
p.submit(2, 0, 1, Box::new(|| ()));
assert_eq!(p.queued(), 0);
drop(release);
assert!(p.shutdown(std::time::Duration::from_secs(30)));
assert_eq!(p.live_workers(), 0);
let (mut p, _release) = pool_with_a_running_job();
assert!(!p.shutdown(std::time::Duration::ZERO));
let t0 = std::time::Instant::now();
drop(p);
assert!(t0.elapsed() < std::time::Duration::from_secs(1), "dropped after a shutdown: no second wait");
}
#[test]
fn pool_drop_is_bounded_when_a_job_never_ends() {
// a job blocked on something its owner holds must not hang the owner (it would hang quitting)
let (p, _release) = pool_with_a_running_job();
let t0 = std::time::Instant::now();
drop(p);
let waited = t0.elapsed();
assert!(waited >= std::time::Duration::from_secs(1) && waited < std::time::Duration::from_secs(30), "{waited:?}");
}
#[test]
fn pool_stops_after_a_job_panicked_and_when_dropped_by_its_own_job() {
// a worker that unwound is not waited for
let mut p: JobPool<u8, ()> = JobPool::new(1);
p.submit(1, 0, 1, Box::new(|| panic!("a job panics (expected in this test)")));
while p.live_workers() > 0 {
std::thread::yield_now();
}
assert!(p.shutdown(std::time::Duration::from_secs(30)));
// the last owner of a pool is one of its jobs: the worker must not wait for (or join) itself
let slot: Arc<std::sync::Mutex<Option<JobPool<u8, ()>>>> = Arc::new(std::sync::Mutex::new(None));
let (dropped, has_dropped) = std::sync::mpsc::channel();
let mut p: JobPool<u8, ()> = JobPool::new(1);
let s = slot.clone();
p.submit(
1,
0,
1,
Box::new(move || {
let own = loop {
if let Some(p) = s.lock().unwrap().take() {
break p;
}
std::thread::yield_now();
};
let t0 = std::time::Instant::now();
drop(own);
let _ = dropped.send(t0.elapsed());
}),
);
*slot.lock().unwrap() = Some(p);
let waited = has_dropped.recv_timeout(std::time::Duration::from_secs(30)).unwrap();
assert!(waited < std::time::Duration::from_secs(1), "{waited:?}");
}
#[test]
fn disk_cache_never_touches_foreign_files() {
// issue #98: a library opened on a folder that already has a `thumbs/` folder
+186 -49
View File
@@ -4,16 +4,20 @@
//! magenta/cyan (each channel limited to the smallest white-balanced clip level).
//! - [`reconstruct`] / [`reconstruct_with`]: where only some channels are clipped, rebuild them from the
//! unclipped channels using the chromaticity of reliable unclipped content nearby (bright, away from edges and
//! from the clipped pixels themselves; diffused into the clipped region coarse to fine), and neutral where there
//! is none within reach. Fully clipped pixels become neutral at the brightest plausible level; partly clipped
//! ones fade into it on their way from the first channel clipping to the last.
//! from the clipped pixels themselves; diffused into the clipped region coarse to fine) unless both the pixel's
//! own channels and the clipped surface's colour just below its clip (the rim) contradict it, and neutral where
//! there is none within reach. Right past the rim, the rim's colour carries on. Fully clipped pixels become
//! neutral at the brightest plausible level; partly clipped ones fade into it on their way from the first channel
//! clipping to the last, pale ones from the start.
//!
//! Why the care: the white-balance gains push clipped channels apart (green clips first, red and blue keep
//! rising), so whatever colour the rebuilt pixels carry is hidden at Neutral only by the tone map's roll-off to
//! white. Any darkening (Highlights, negative Exposure or Whites) brings it out. Two things used to leak colour
//! white. Any darkening (Highlights, negative Exposure or Whites) brings it out. Three things used to leak colour
//! in: the chromaticity of whatever unclipped pixel happened to be nearest (dark foliage, purple and green
//! fringes along branches, pixels demosaiced from clipped neighbours), and a colour model that does not render
//! the camera's white-balanced neutral as neutral (a look fitted to the camera's JPEG, a camera profile).
//! fringes along branches, pixels demosaiced from clipped neighbours); a colour model that does not render the
//! camera's white-balanced neutral as neutral (a look fitted to the camera's JPEG, a camera profile); and, in a
//! pale partly clipped sky, the colour of another reliable surface (bright foliage) and the sky's own faint
//! colour, which darkening and a camera profile turned teal.
use lightcraft_raster::Rgb32f;
use rayon::prelude::*;
@@ -53,15 +57,28 @@ const SUPPORT: f32 = 0.05;
/// An unclipped channel above `FLOOR_LO` of its clip level counts more and more as the clip floor it is about to
/// become (see [`reconstruct_with`]).
const FLOOR_LO: f32 = 0.75;
/// Without reliable colour around, a clipped pixel is rebuilt as neutral only as far as it is brighter (or
/// otherwise unlike) the unclipped pixels nearest its clip: the "rim", unclipped pixels above `RIM_LO` of the
/// clip level, weighted towards the clip. Up to `NEAR_RIM` stops past the rim (the rim is an average, a little
/// below the clip) the clipped channels stay at their clip, and over the next `DEPTH` stops they go over to the
/// neutral guess, so a surface runs into clipping without a step whatever its colour; well past it, or where the
/// rim is another surface (its colour differs), the neutral guess applies.
/// A clipped pixel takes its estimated colour (reliable colour around, or the neutral guess) only as far as it is
/// brighter (or otherwise unlike) the unclipped pixels nearest its clip: the "rim", unclipped pixels above `RIM_LO`
/// of the clip level, weighted towards the clip, the clipped surface's own colour just below it. Up to `NEAR_RIM`
/// stops past the rim (the rim is an average, a little below the clip) it is rebuilt with the rim's colour, and
/// over the next `DEPTH` stops it goes over to the estimate, so a surface runs into clipping without a step
/// whatever its colour and whatever lent the estimate; well past it, or where the rim is another surface (its
/// colour differs; a clipped channel differs by at least as much as it lies above the rim's), the estimate applies.
const RIM_LO: f32 = 0.85;
const NEAR_RIM: f32 = 0.03;
const DEPTH: f32 = 0.2;
/// Reliable colour counts less and less as both the pixel's own channels (see [`contradiction`]) and the rim's
/// colour contradict it, by `AGREE.0` to `AGREE.1` stops: it is another surface's, like bright foliage below a pale
/// sky, whose green would turn the sky teal. (Either alone can be wrong: a pixel's own channels are noisy and
/// can't speak for the channels that clipped, and the rim can be another surface, like a white window frame
/// around a bright view.)
const AGREE: (f32, f32) = (0.15, 0.4);
/// On the way from the first channel clipping to the last, a partly clipped pixel's brightness goes over to the
/// clipped neutral's in the second half, and so does the colour of a strongly coloured one (saturation relative to
/// the white above `PALE.1`). A pale one's colour (below `PALE.0`) goes over from the start: the few per cent of
/// colour left in a pale sky once a channel clipped is as much the estimate's error as the sky's, and darkening
/// shows it.
const PALE: (f32, f32) = (0.2, 0.4);
#[inline]
fn smoothstep(e0: f32, e1: f32, x: f32) -> f32 {
@@ -253,6 +270,35 @@ fn push<const N: usize>(levels: &mut [Level<N>], empty: [f32; N], needed: Option
}
}
/// How many stops two chromaticities differ by in any channel ratio.
fn unlike(a: [f32; 3], b: [f32; 3]) -> f32 {
let ratio = |i: usize, j: usize| (a[i].max(1e-6) * b[j].max(1e-6)) / (a[j].max(1e-6) * b[i].max(1e-6));
[(0, 1), (0, 2), (1, 2)].iter().map(|&(i, j)| ratio(i, j).log2().abs()).fold(0.0f32, f32::max)
}
/// How far channel `c` of a pixel (sensor values `p`, clipped channels `cl`) only bounds its true value from below:
/// fully once clipped, and increasingly from `FLOOR_LO` of the clip on, so that nothing compared with it jumps where
/// it clips.
fn bound(p: [f32; 3], cl: [bool; 3], clip: f32, c: usize) -> f32 {
if cl[c] { 1.0 } else { smoothstep(FLOOR_LO, 1.0, p[c] / clip) }
}
/// How many stops the channels of a partly clipped pixel (sensor values `p`, white-balanced `q`, clipped channels
/// `cl`) contradict chromaticity `r`: how far the levels they imply spread, where a clipped channel's only bounds the
/// level from below (see [`bound`]). 0 for the pixel's own colour, whatever its level.
fn contradiction(p: [f32; 3], q: [f32; 3], cl: [bool; 3], r: [f32; 3], clip: f32) -> f32 {
let (mut lo, mut hi) = (f32::INFINITY, 0f32);
for c in 0..3 {
let level = q[c] / r[c].max(1e-3);
hi = hi.max(level);
let w = bound(p, cl, clip, c);
if r[c] > 1e-3 && w < 1.0 {
lo = lo.min(level / (1.0 - w));
}
}
if hi > 0.0 && lo > 0.0 && lo.is_finite() { (hi / lo).log2().max(0.0) } else { 0.0 }
}
/// The white-balanced colour of a partly clipped pixel (sensor values `p`, white-balanced `q`, clipped channels
/// `cl`) whose colour is estimated at chromaticity `r`: clipped channels rebuilt, unclipped ones kept. `None`
/// when no unclipped channel can tell the level.
@@ -273,10 +319,11 @@ fn rebuild(p: [f32; 3], q: [f32; 3], cl: [bool; 3], r: [f32; 3], clip: f32, max_
}
// Two clipped channels must not be left at their clip levels: those differ by the white-balance gains (green
// clips first), and the pair would show as magenta or red. They keep the estimate's ratio instead, at the level
// that lifts each to its clip (no channel above the clipped neutral).
// that lifts each to its clip (no channel above the clipped neutral), and so does a channel about to clip (the
// same floor, so nothing jumps where it clips).
let floor = (0..3).filter(|&c| cl[c]).map(|c| q[c] / r[c].max(1e-3)).fold(0.0f32, f32::max);
let top = r[0].max(r[1]).max(r[2]).max(1e-3);
let sum = (sum / k as f32).max(near_clip).max(floor.min(max_level / top));
let sum = (sum / k as f32).max(near_clip.max(floor).min(max_level / top));
Some(std::array::from_fn(|c| if cl[c] { q[c].max(sum * r[c]) } else { q[c] }))
}
@@ -343,20 +390,16 @@ pub fn reconstruct_with(img: &mut Rgb32f, wb: [f32; 3], clip: f32, white: [f32;
});
}
let Some(field) = levels.first() else { return count };
// the rim, only where some clipped pixel lacks reliable colour (level 0 of its pyramid holds the estimate)
let untrusted = field.cells.par_iter().enumerate().any(|(i, c)| c[2] < 1.0 && needed(i % field.w, i / field.w));
let rims = untrusted
.then(|| {
let mut levels = vec![rim(img, wb, clip, &needed)];
while let Some(l) = levels.last().filter(|l| l.w > 1 || l.h > 1) {
let next = pull(l);
levels.push(next);
}
push(&mut levels, [0.0; 4], None);
levels
})
// a rim anywhere at all (else the neutral guess applies throughout)
.filter(|levels| levels.last().is_some_and(|top| top.cells.iter().any(|c| c[3] > 0.0)));
// the rim (level 0 of its pyramid holds the estimate), if there is one anywhere at all
let rims = {
let mut levels = vec![rim(img, wb, clip, &needed)];
while let Some(l) = levels.last().filter(|l| l.w > 1 || l.h > 1) {
let next = pull(l);
levels.push(next);
}
push(&mut levels, [0.0; 4], None);
Some(levels).filter(|levels| levels.last().is_some_and(|top| top.cells.iter().any(|c| c[3] > 0.0)))
};
let rim_field = rims.as_ref().and_then(|l| l.first());
let max_level = wb.iter().cloned().fold(0.0f32, f32::max) * clip;
img.data.par_chunks_mut(w).enumerate().for_each(|(y, row)| {
@@ -368,22 +411,51 @@ pub fn reconstruct_with(img: &mut Rgb32f, wb: [f32; 3], clip: f32, white: [f32;
}
let q = [p[0] * wb[0], p[1] * wb[1], p[2] * wb[2]];
// rebuilt with the colour of the reliable content around, and with neutral, blended by how much of it
// there is (blending the results, not the colours, keeps the estimate consistent with the pixel's own
// unclipped channels all the way)
// there is and how well the pixel agrees with it (blending the results, not the colours, keeps the
// estimate consistent with the pixel's own unclipped channels all the way)
let e = sample(field, 2.0, x, y);
let trust = e[2].clamp(0.0, 1.0);
let local = (trust > 0.0).then(|| rebuild(p, q, cl, [e[0], 1.0 - e[0] - e[1], e[1]], clip, max_level)).flatten();
let plain = (trust < 1.0).then(|| rebuild(p, q, cl, neutral, clip, max_level)).flatten().map(|b| {
// only as far as the pixel lies past the rim nearest its clip (in every unclipped channel)
let Some(rim) = rim_field else { return b };
let edge = sample(rim, 4.0, x, y);
let past = (0..3)
.filter(|&c| !cl[c])
.map(|c| if edge[c] > 1e-6 && q[c] > 1e-6 { (q[c] / edge[c]).log2().abs() } else { f32::INFINITY })
.fold(0.0f32, f32::max);
let borrowed = [e[0], 1.0 - e[0] - e[1], e[1]];
let edge = rim_field.map(|rim| sample(rim, 4.0, x, y));
let mut trust = e[2].clamp(0.0, 1.0);
if trust > 0.0 {
let mut off = contradiction(p, q, cl, borrowed, clip);
if off > AGREE.0 {
off = off.min(edge.map_or(0.0, |edge| {
let s = edge[0] + edge[1] + edge[2];
if s > 1e-6 { unlike([edge[0] / s, edge[1] / s, edge[2] / s], borrowed) } else { 0.0 }
}));
}
trust *= 1.0 - smoothstep(AGREE.0, AGREE.1, off);
}
// either applies only as far as the pixel lies past the rim nearest its clip (in any channel); nearer, the
// pixel is rebuilt with the rim's colour
let at_rim = edge.and_then(|edge| {
// stops between pixel and rim in each channel; a clipped channel's true value lies at least as far
// above the rim's as its clip, and no further below (see [`bound`])
let ratio = (0..3)
.map(|c| {
let (a, b) = (q[c].max(1e-12), edge[c].max(1e-12));
if a >= b {
return a / b;
}
match bound(p, cl, clip, c) {
w if w <= 0.0 => b / a,
w if w >= 1.0 => 1.0,
w => (b / a).powf(1.0 - w),
}
})
.fold(1.0f32, f32::max);
let past = ratio.log2().min(64.0);
let k = smoothstep(NEAR_RIM, NEAR_RIM + DEPTH, past);
std::array::from_fn(|c| q[c] + (b[c] - q[c]) * k)
let s = edge[0] + edge[1] + edge[2];
(k < 1.0 && s > 1e-6).then(|| rebuild(p, q, cl, [edge[0] / s, edge[1] / s, edge[2] / s], clip, max_level)).flatten().map(|a| (a, k))
});
let past_rim = |b: [f32; 3]| -> [f32; 3] {
let Some((a, k)) = at_rim else { return b };
std::array::from_fn(|c| a[c] + (b[c] - a[c]) * k)
};
let local = (trust > 0.0).then(|| rebuild(p, q, cl, borrowed, clip, max_level)).flatten().map(past_rim);
let plain = (trust < 1.0).then(|| rebuild(p, q, cl, neutral, clip, max_level)).flatten().map(past_rim);
let mut out = match (local, plain) {
(Some(a), Some(b)) => std::array::from_fn(|c| b[c] + (a[c] - b[c]) * trust),
(Some(a), None) | (None, Some(a)) => a,
@@ -399,12 +471,19 @@ pub fn reconstruct_with(img: &mut Rgb32f, wb: [f32; 3], clip: f32, white: [f32;
let above = (0..3).filter(|&c| cl[c]).map(|c| (out[c] / q[c].max(1e-9)).max(1.0).log2()).fold(0.0f32, f32::max);
let lowest = (0..3).filter(|&c| !cl[c]).map(|c| p[c]).fold(f32::MAX, f32::min);
let below = (clip / lowest.max(1e-6)).log2().max(0.0);
let t = if above > 0.0 && lowest < clip { smoothstep(0.5, 1.0, above / (above + below)) } else { 0.0 };
if t > 0.0 {
let way = if above > 0.0 && lowest < clip { above / (above + below) } else { 0.0 };
// The brightness goes over in the second half of the way, and so does the colour of a strongly coloured
// pixel; a pale one's goes over from the start ([`PALE`]).
let u = [out[0] / white[0], out[1] / white[1], out[2] / white[2]];
let (lo, hi) = (u[0].min(u[1]).min(u[2]), u[0].max(u[1]).max(u[2]));
let saturation = if hi > 0.0 && hi.is_finite() { 1.0 - lo.max(0.0) / hi } else { 1.0 };
let start = 0.5 * smoothstep(PALE.0, PALE.1, saturation);
let (t_level, t_colour) = (smoothstep(0.5, 1.0, way), smoothstep(start, start + 0.5, way));
if t_colour > 0.0 {
let level = (out[0] + out[1] + out[2]) / total;
let full = q.iter().cloned().fold(0.0f32, f32::max).max(max_level);
let v = level + (full - level) * t;
out = std::array::from_fn(|c| out[c] + (white[c] * v - out[c]) * t);
let v = level + (full - level) * t_level;
out = std::array::from_fn(|c| out[c] + (white[c] * v - out[c]) * t_colour);
}
*px = [out[0] / wb[0], out[1] / wb[1], out[2] / wb[2]];
}
@@ -450,21 +529,34 @@ mod tests {
(img, sky)
}
/// How far chromaticity `got` lies off the way from `from` to neutral (largest channel difference to the
/// nearest point of that way).
fn off_the_way_to_neutral(got: [f32; 3], from: [f32; 3]) -> f32 {
let d: [f32; 3] = std::array::from_fn(|c| 1.0 / 3.0 - from[c]);
let len = d.iter().map(|v| v * v).sum::<f32>();
let t = if len > 0.0 { ((0..3).map(|c| (got[c] - from[c]) * d[c]).sum::<f32>() / len).clamp(0.0, 1.0) } else { 0.0 };
(0..3).map(|c| (got[c] - (from[c] + d[c] * t)).abs()).fold(0.0, f32::max)
}
/// Issue #523: the clipped sky beside a branch took the colour of the branch's fringes and of the branch
/// itself (magenta, green), which any darkening of the highlights then showed.
/// itself (magenta, green), which any darkening of the highlights then showed. Beside the branch it is now the
/// clipped sky away from it: the sky's colour, on its way to neutral.
#[test]
fn fringes_and_dark_branches_do_not_tint_a_clipped_sky() {
let (mut img, sky) = branch_scene();
assert!(reconstruct(&mut img, WB, 0.99) > 0);
let want = chroma(sky);
let mut worst = 0f32;
for y in 40..128 {
let away = chroma(balanced(img.get(30, y)));
assert!(away == chroma(balanced(img.get(170, y))), "row {y}");
let off = off_the_way_to_neutral(away, chroma(sky));
assert!(off < 0.005, "row {y}: the clipped sky {away:?} is not the sky's colour on its way to neutral ({off})");
for x in [84, 86, 88, 101, 103, 105] {
let got = chroma(balanced(img.get(x, y)));
worst = worst.max((0..3).map(|c| (got[c] - want[c]).abs()).fold(0.0, f32::max));
worst = worst.max((0..3).map(|c| (got[c] - away[c]).abs()).fold(0.0, f32::max));
}
}
assert!(worst < 0.01, "clipped sky beside the branch is off the sky's colour by {worst}");
assert!(worst < 0.01, "clipped sky beside the branch is off the sky away from it by {worst}");
}
/// A warm light with red and green clipped: left at their clip levels the pair reads red-magenta (the white
@@ -578,6 +670,51 @@ mod tests {
}
}
/// Issue #523 (a river photo darkened by Highlights -75): a pale cyan-blue sky (`[1, 1.07, 1.2]` white-balanced)
/// brightening from left to right through the point where green clips (and later blue), above bright
/// yellow-green foliage, with thin dark branches standing in its lower half. Its rebuilt colour used to lean
/// green to teal: the foliage lent its colour to the sky (green rebuilt from it), with a step where green first
/// clips, and the sky kept its own pale colour where Lightroom shows white.
#[test]
fn pale_sky_by_dark_branches_and_foliage_is_not_tinted() {
let sky = [1.0, 1.07, 1.2];
let (w, h) = (320usize, 96usize);
let k = |x: usize| 0.7 * (1.2 * x as f32 / (w - 1) as f32).exp2();
let branches = [150usize, 200, 250];
let on_branch = |x: usize| branches.iter().any(|&b| (b.saturating_sub(1)..=b + 2).contains(&x));
let img0 = Rgb32f::from_fn(w, h, |x, y| {
sensor(if y >= 72 {
[0.5, 0.6, 0.22]
} else if y >= 40 && branches.iter().any(|&b| x == b || x == b + 1) {
[0.1, 0.09, 0.04]
} else {
sky.map(|v| v * k(x))
})
});
let mut img = img0.clone();
reconstruct(&mut img, WB, 0.99);
let at = |x: usize, y: usize| chroma(balanced(img.get(x, y)));
let first = (0..w).find(|&x| img0.get(x, 20)[1] >= 0.99).unwrap_or(0);
assert!(first > 0 && first < 150);
let diff = |a: [f32; 3], b: [f32; 3]| (0..3).map(|c| (a[c] - b[c]).abs()).fold(0.0, f32::max);
for y in [20, 50] {
// no step where green clips
let step = diff(at(first - 1, y), at(first, y));
assert!(step < 0.001, "row {y}: step of {step} where green clips");
for x in first..w {
// nothing but the sky's colour on its way to neutral (no green from the foliage)...
let off = off_the_way_to_neutral(at(x, y), chroma(sky));
assert!(on_branch(x) || off < 0.005, "row {y}, x {x}: {:?} is off the sky's way to neutral by {off}", at(x, y));
// ... the same beside the branches as above them ...
let beside = diff(at(x, y), at(x, 20));
assert!(on_branch(x) || beside < 0.005, "row {y}, x {x}: {:?} beside a branch, {:?} above", at(x, y), at(x, 20));
}
// ... and neutral well before the last channel clips (the sky is 0.7 stops past green's clip there)
let end = at(w - 1, y);
assert!(end.iter().all(|v| (v - 1.0 / 3.0).abs() < 0.002), "row {y}: {end:?} at the end");
}
}
#[test]
fn unclipped_pixels_are_left_exactly() {
let mut seed = 0x2545_f491_4f6c_dd1du64;
+6 -1
View File
@@ -215,6 +215,9 @@ pub struct Headless {
pub view: HeadlessView,
/// The largest texture the pretend GPU takes (what a WebGL device may report: 2048).
pub max_texture_side: usize,
/// The limit egui was last told. Like a native host (egui-winit) it is reported once, not in
/// every frame's input: code that reads the raw input sees `None` from then on (issue #652).
reported_texture_side: Option<usize>,
/// Logical size (points) and scale.
pub size: egui::Vec2,
pub pixels_per_point: f32,
@@ -248,6 +251,7 @@ impl Headless {
app,
view: HeadlessView::new(),
max_texture_side: 16384,
reported_texture_side: None,
size,
pixels_per_point,
time: 0.0,
@@ -280,7 +284,8 @@ impl Headless {
(self.size, self.pixels_per_point) = bounded_viewport(self.size, self.pixels_per_point);
let mut raw = HeadlessView::raw_input(self.size, self.pixels_per_point, self.time, std::mem::take(&mut self.events));
raw.viewports.entry(ViewportId::ROOT).or_default().maximized = Some(self.window_maximized);
raw.max_texture_side = Some(self.max_texture_side);
raw.max_texture_side = (self.reported_texture_side != Some(self.max_texture_side)).then_some(self.max_texture_side);
self.reported_texture_side = Some(self.max_texture_side);
self.app.raw_input_hook(&mut raw);
let app = &mut self.app;
let mut cursor = egui::CursorIcon::Default;
+17
View File
@@ -736,6 +736,23 @@ impl LightcraftApp {
}
}
/// The process is about to end (settings and library are saved by then): make sure nothing is
/// inside the GPU driver when it does (issue #620: a render running on a worker thread while
/// the process exits crashes in the driver). No GPU work starts from here on; the render
/// workers get up to `timeout` to finish their jobs, and what is left of it goes to GPU work
/// on other threads (export, denoise, the device warm-up). `false`: the deadline passed first
/// (the host exits anyway). The app renders nothing afterwards.
pub fn shutdown(&mut self, timeout: std::time::Duration) -> bool {
lightcraft_engine::gpu::begin_shutdown();
#[cfg(not(target_arch = "wasm32"))]
let t0 = std::time::Instant::now();
let stopped = self.renderer.shutdown(timeout);
#[cfg(not(target_arch = "wasm32"))]
let timeout = timeout.saturating_sub(t0.elapsed());
let idle = lightcraft_engine::gpu::wait_idle(timeout);
stopped && idle
}
/// Per-frame logic before layout (control channel, renders, shortcuts, drops).
pub fn logic(&mut self, ctx: &egui::Context) {
i18n::set_language(self.ui.language);
+23 -3
View File
@@ -187,10 +187,14 @@ pub(crate) fn navigate_gesture(app: &mut LightcraftApp, ui: &mut egui::Ui, resp:
true
}
/// The largest texture the GPU behind `ctx` takes (egui reports it; 2048 when it doesn't, the
/// The largest texture the GPU behind `ctx` takes (egui reports it; 2048 until it does, the
/// smallest limit WebGL devices have).
///
/// The value egui keeps, not the frame's raw input: a native host (egui-winit) reports the limit
/// in the first frame's input only, so the raw value is `None` from the second frame on
/// (issue #652: every window render was cut to 2048 px, with blurry strips beside it).
pub(crate) fn texture_side(ctx: &egui::Context) -> usize {
ctx.input(|i| i.raw.max_texture_side).unwrap_or(2048)
ctx.input(|i| i.max_texture_side)
}
/// The photo's own pixels as it is shown: its size after the crop and the user's rotation (what
@@ -2139,7 +2143,23 @@ fn straighten_overlay(app: &mut LightcraftApp, ui: &mut egui::Ui, resp: &egui::R
#[cfg(test)]
mod tests {
use super::film_label;
use super::{film_label, texture_side};
/// Issue #652: a native host reports the GPU's texture limit in the first frame's input only.
/// The loupe must still know it in every later frame (it read the raw input, found nothing and
/// cut every window render to 2048 px).
#[test]
fn the_texture_limit_outlives_the_frame_it_was_reported_in() {
let ctx = egui::Context::default();
assert_eq!(texture_side(&ctx), 2048, "until the host says: the smallest limit there is");
let mut seen = Vec::new();
for reported in [Some(8192), None, None] {
let raw = egui::RawInput { max_texture_side: reported, ..Default::default() };
let mut out = ctx.run_ui(raw, |ui| seen.push(texture_side(ui.ctx())));
out.textures_delta.clear();
}
assert_eq!(seen, [8192, 8192, 8192]);
}
#[test]
fn film_labels_cut_on_characters_not_bytes() {
+13
View File
@@ -464,6 +464,19 @@ impl Renderer {
self.pending.contains_key(&slot)
}
/// The app is closing: drop the jobs that have not started and wait up to `timeout` for the
/// ones running on the worker threads. Returns whether all of them have ended. Nothing is
/// rendered afterwards.
pub fn shutdown(&mut self, timeout: std::time::Duration) -> bool {
self.queue.clear();
self.pool.shutdown(timeout)
}
/// Worker threads that are still alive (0 before the first render).
pub fn live_workers(&self) -> usize {
self.pool.live_workers()
}
/// The texture to show for a grid/filmstrip thumbnail: the rendered one, else its stand-in.
pub fn thumb(&self, id: PhotoId) -> Option<&Tex> {
self.textures.get(&Slot::Thumb(id)).or_else(|| self.textures.get(&Slot::ThumbQuick(id)))
+22
View File
@@ -352,6 +352,28 @@ mod in_the_loupe {
}
}
// Issue #652: given a canvas wider than 2048 px on a desktop GPU (which reports its texture
// limit to egui once, in the first frame), when I zoom to 1:1, then the window render covers
// the whole canvas: no strip of the magnified whole-frame render beside it
#[test]
fn a_window_covers_a_canvas_wider_than_2048_px() {
let (mut h, native) = detail_in([2600.0, 1200.0]);
assert!(native > 2600, "the photo is wider than the canvas at 1:1: {native}");
h.request("engine.execute", json!({"command": "view.zoom100"}), T);
h.settle(SETTLE);
let canvas = h.app.canvas_rect.expect("the loupe was drawn");
let image = h.app.image_rect.expect("the photo was drawn");
let region = h.app.region_view.expect("a window");
// the canvas in pixels of the window's frame (scale 1, at 100 %: a point is a pixel)
let to_px = region.full.0 as f32 / image.width();
let (left, right) = (to_px * (canvas.left() - image.left()), to_px * (canvas.right() - image.left()));
assert!(right - left > 2048.0, "the canvas shows more than 2048 px: {left}..{right}");
let (x0, x1) = (region.window.x as f32, (region.window.x + region.window.w) as f32);
assert!(x0 <= left + 0.5 && x1 >= right - 0.5, "the window {x0}..{x1} leaves part of the canvas {left}..{right} to the preview");
let (w, _) = region_tile(&h).expect("the window rendered");
assert_eq!(w, region.window.w, "and that is the texture drawn");
}
// Given 400 % on a big photo, when a slider is dragged, then the work per frame follows the
// canvas, not the zoom: a draft of the whole frame at canvas scale and a draft of the window
#[test]
+40
View File
@@ -0,0 +1,40 @@
//! Issue #620: quitting while previews render must not leave a render worker running (on Linux
//! with the NVIDIA driver, one inside the GPU when the process exits is a SIGSEGV).
//! `LightcraftApp::shutdown` stops the render workers and closes the GPU for the whole process,
//! hence one test in a binary of its own.
use std::time::{Duration, Instant};
use lightcraft_ui_egui::headless::Headless;
use lightcraft_ui_egui::{LightcraftApp, Services};
#[test]
fn shutdown_leaves_no_render_worker_running() {
let services = Services { png: None, ..Default::default() };
let mut h = Headless::new(LightcraftApp::new(lightcraft_engine::Session::with_demo(), services), [1200.0, 760.0], 1.0);
// frames until the grid's thumbnails are being rendered on the workers
let t0 = Instant::now();
while h.app.renderer.live_workers() == 0 || h.app.renderer.in_flight() == 0 {
assert!(t0.elapsed() < Duration::from_secs(120), "no render started");
h.step();
}
assert!(!lightcraft_engine::gpu::shutting_down());
assert!(h.app.shutdown(Duration::from_secs(120)), "the jobs that were running ended in time");
assert_eq!(h.app.renderer.live_workers(), 0);
assert_eq!(h.app.renderer.queued(), 0, "jobs that had not started are dropped");
assert!(lightcraft_engine::gpu::shutting_down());
assert!(lightcraft_engine::gpu::wait_idle(Duration::ZERO), "nothing is inside the GPU");
// frames after it (the window's last ones) ask for renders that are not started any more
for _ in 0..3 {
h.step();
}
assert_eq!(h.app.renderer.live_workers(), 0);
assert_eq!(h.app.renderer.queued(), 0);
// a job that was caught before its render gives up: quitting doesn't wait for a CPU render
let id = h.app.session.active().or_else(|| h.app.session.catalog.photos().next().map(|p| p.id)).expect("a demo photo");
let job = h.app.session.render_job(id, 300, 200, false, true).expect("a render job");
assert_eq!(job.run().rendered.err().as_deref(), Some("LightCraft is closing"));
}
+11
View File
@@ -60,6 +60,17 @@ up (`gpu::warm_up` from the first frame's settings), and never while GPU renderi
Settings ▸ Performance ▸ *Use the GPU for rendering* unchecked (applied before the window opens),
`LIGHTCRAFT_GPU=0` or `LIGHTCRAFT_GPU_BACKEND=off`, no GPU driver is loaded for rendering at all.
**Quitting.** The devices are never dropped, and renders run on worker threads: a render still inside
the driver while the process exits crashes there (a SIGSEGV in the NVIDIA Vulkan driver on Linux,
issue #620), which no panic hook catches. So every entry into a device counts as work in flight
(`crates/gpu/src/exit.rs`), and the desktop app ends with `LightcraftApp::shutdown` (from `on_exit`,
after settings and library are saved): `gpu::begin_shutdown()` — from then on `gpu::render` returns
`None`, no device is created, denoise tiles fail over and a render job gives up instead of rendering
on the CPU — then the render pool stops (queued jobs dropped, running ones waited for) and
`gpu::wait_idle` waits for GPU work on other threads (export, denoise, the device warm-up). One
deadline of 2 s covers it all; past it the app quits anyway. A job pool that is dropped (headless
hosts, tests) waits for its running jobs the same way (`JobPool::shutdown`, `crates/preview/src/pool.rs`).
**Crash sentinel.** The desktop app writes `gpu-init.marker` into its settings folder (next to
`ui.json`: `%APPDATA%\LightCraft`, `~/Library/Application Support/LightCraft`,
`~/.config/lightcraft`) just before the compute device is created and removes it as soon as creation
+2 -2
View File
@@ -242,7 +242,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins.
| LR-EDIT-HDR-MODE | HDR editing | P2 | ✅ | `cmd:develop.hdr`, `ctl:hdr.maxEv`, `crates/ui-egui/src/panels/edit.rs` (`hdr_row`) | HDR switch in the Light panel; see Q. HDR |
| LR-EDIT-LIGHT-EXPOSURE | Exposure | P0 | ✅ | `ctl:light.exposure` | |
| LR-EDIT-LIGHT-CONTRAST | Contrast | P0 | ✅ | `ctl:light.contrast` | |
| LR-EDIT-LIGHT-HIGHLIGHTS | Highlights | P0 | ✅ | `ctl:light.highlights`, `crates/raw/src/highlight.rs` | sensor-clipped highlights stay neutral when Highlights (or Exposure, Whites) darkens them: fully clipped content is rebuilt to the neutral of the colour model applied after it, partly clipped content takes its colour from reliable unclipped content nearby (not from dark objects or lens fringes beside it) and white where there is none (#523) |
| LR-EDIT-LIGHT-HIGHLIGHTS | Highlights | P0 | ✅ | `ctl:light.highlights`, `crates/raw/src/highlight.rs` | sensor-clipped highlights stay neutral when Highlights (or Exposure, Whites) darkens them: fully clipped content is rebuilt to the neutral of the colour model applied after it, partly clipped content takes its colour from reliable unclipped content nearby (not from dark objects, lens fringes or another surface beside it) and white where there is none, and pale partly clipped content (a white sky) turns white on its way to clipping (#523) |
| LR-EDIT-LIGHT-SHADOWS | Shadows | P0 | ✅ | `ctl:light.shadows` | |
| LR-EDIT-LIGHT-WHITES | Whites | P0 | ✅ | `ctl:light.whites` | |
| LR-EDIT-LIGHT-BLACKS | Blacks | P0 | ✅ | `ctl:light.blacks` | |
@@ -519,7 +519,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins.
| LR-BEHAV-BG-TASKS | Background tasks | P0 | ✅ | `crates/preview/src/lib.rs` (`JobPool`), `crates/ui-egui/src/export_task.rs`, `crates/ui-egui/src/import.rs` | renders off the UI thread; exports started from the UI run on a worker thread with a progress panel and Cancel (`ui.inspect` → `export`), several photos of a batch side by side (`export_parallelism`, memory-gated; files, names and order as one at a time, #496); imports show a progress window. No combined activity centre |
| LR-BEHAV-OFFLINE | Offline editing | P1 | ✅ | | local-first: everything works offline |
| LR-BEHAV-WEB-SAFETY | Library safety in the browser build | P1 | 🟡 | `cmd:file.backupLibrary`, `cmd:file.restoreLibrary`, `apps/lightcraft-web/src/safety.rs`, `apps/lightcraft-web/src/backup.rs`, `apps/lightcraft-web/src/files.rs` | web only (experimental): Back Up Library (zip of the catalog and every stored original) and a non-destructive Restore; failed saves show as unsaved and are retried; a photo that can't be stored isn't added; one tab per library (Web Locks); notices for storage that isn't persistent and for a library that can't open; `?reset` asks first; a panic shows a message. Missing: `?safe` start, deleting originals of removed photos, zip64 (backups > 4 GB) |
| LR-BEHAV-GPU | GPU acceleration | P0 | ✅ | `crates/gpu/src/render.rs`, `cmd:app.gpu`, `docs/gpu-pipeline.md` | CPU fallback per render on device limits, out of memory, device errors / loss, incomplete or black results (`perf.gpuReason`, `perf.gpuFallback`); short submissions for slow iGPUs; backend choice (DX12 only on Windows, `LIGHTCRAFT_GPU_BACKEND` / `WGPU_BACKEND`), device created after the window opens, crash sentinel (issue #136); no tiling beyond the buffer limit (CPU) |
| LR-BEHAV-GPU | GPU acceleration | P0 | ✅ | `crates/gpu/src/render.rs`, `cmd:app.gpu`, `docs/gpu-pipeline.md` | CPU fallback per render on device limits, out of memory, device errors / loss, incomplete or black results (`perf.gpuReason`, `perf.gpuFallback`); short submissions for slow iGPUs; backend choice (DX12 only on Windows, `LIGHTCRAFT_GPU_BACKEND` / `WGPU_BACKEND`), device created after the window opens, crash sentinel (issue #136); quitting waits (up to 2 s) for renders in flight and starts no new GPU work, so none is inside the driver when the process exits (`crates/gpu/src/exit.rs`, issue #620; seen crashing only on Linux + NVIDIA, other drivers unverified); no tiling beyond the buffer limit (CPU) |
| LR-BEHAV-RENDER-FIDELITY | Rendering matches Lightroom | P1 | 🟡 | `crates/pipeline/src/lib.rs`, `crates/pipeline/src/tone.rs` | every slider exists and works, but the character of the result (tone curve shape, highlight recovery, texture / clarity / dehaze, noise reduction, sharpening, default look) is tuned by eye; there is no systematic side-by-side comparison against Lightroom on the same CC0 raws. Needs a fidelity suite: Lightroom reference renders kept only in the local `plan/` (never committed), compared per slider and preset with a perceptual metric. Tuning that changes how existing settings render ships as a new process version (LRC-DEV-PROCESS, `docs/process-versions.md`) |
| LR-BEHAV-DRAGDROP | Drag and drop | P1 | ✅ | `crates/ui-egui/src/lib.rs`, `crates/ui-egui/src/panels/grid.rs` (`drag_feedback`), `crates/ui-egui/src/panels/left.rs` (`drop_target`, `album_drag_over`, `top_level_drop_target`, `drag_auto_scroll`) | files → app (import); grid photos → an album row (adds the selection, with a count badge while dragging); sidebar album / folder rows → a folder (moves it; its name follows the pointer); a drag of albums or grid photos near the top or bottom edge of the sidebar scrolls it (faster nearer the edge, only while the pointer is over the sidebar) |
| LR-BEHAV-TOAST | Toast notifications | P1 | ✅ | `crates/ui-egui/src/panels/mod.rs` | |