crates/engine/src/media.rs: main moved render_now and export_job to media/session.rs; the two size checks ported there
crates/engine/src/tests_export.rs: the test is appended to tests/export.rs
Conflicts resolved:
- crates/engine/src/media.rs: main split it; the render and display-profile hunks are ported into media/render.rs
- crates/ui-egui/src/lib.rs, panels/detail.rs: main split them; alt_held is set in app/frame.rs, view_overlay and overlay_supports_window ported (panels/detail/window.rs)
- crates/ui-egui/src/tests_alt_preview.rs moved to tests/alt_preview.rs
- crates/pipeline/src/finish.rs, local.rs: main's code kept, sharpen_mask_weight and texture_sigma applied to it
- docs: both sides kept; summary counts from main (regenerated later)
Record the out-of-memory failure when the staging buffer can't be allocated, and don't publish the zeroed buffer to SHARED_SOURCE when the render has failed, so a concurrent render can't reuse it.
Clear SHARED_SOURCE in the OutOfMemory branch so the next render does not reuse a zeroed buffer, and keep the OOM test's allocations within the device's max_buffer_size, failing on unexpected validation errors.
An upload whose buffer couldn't be allocated tried again through create_buffer_init, which maps the new buffer and panics inside wgpu when that allocation fails too ("Failed to get mapped range for buffer created with mapped_at_creation"). The panic stopped GPU use for the session. Uploads and kernel parameters now go through the queue into an unmapped buffer, and a failed upload is not retried, so out of memory stays an out-of-memory fallback to the CPU for that render.
Claude-Session: https://claude.ai/code/session_01SF8KiKFYZpuGzdTuzsm3cr
Render cost is pixels x samples (CPU and GPU), and only mask.add / mask.update capped the list, so a
sidecar's lc:settings, a preset, develop.merge, render --settings or a catalog / history load could
carry thousands and stall a render or lose the GPU device. MAX_COLOR_RANGE_SAMPLES moves to
lightcraft-develop and ColorRange::samples deserializes through a capped visitor (the first 64 in
order, the rest skipped as IgnoredAny); the CPU and GPU renders also take at most 64 for settings
built in code.
The GPU colour tools took the hue of every pixel with atan2(a, b). For a black pixel both are
0, and WGSL leaves atan2(0, 0) undefined: on an RTX 3090 (Vulkan) it is NaN, so with any
colour tool on (saturation, vibrance, the mixer) every exactly black pixel became NaN. NaN is
written as 0, so it stayed black and nothing showed, until a later step lightened it: with a
positive vignette the CPU lifts the black corners and the GPU left them black (up to 127
levels apart in 8-bit). A neutral pixel now gets hue 0, as Rust's atan2 gives on the CPU.
Regression test: black bars under a light vignette with saturation, vibrance or the mixer
(fails on main: mean 12.8, max 127 levels).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014ti8KemZbjE2ZgHgV1q1BM
- RawImage::develop demosaics only the default crop with AHD when there
is no OpcodeList3 (ahd_window), instead of demosaicing the margin too
and then copying the crop out of the full picture on one thread.
- Gpu::upload writes through queue.write_buffer_with and copies into
the staging buffer in parallel parts. create_buffer_init zeroed a
staging buffer and then copied over it, two single-threaded passes
over a full-size photo's ~290 MB.
- finish_and_read copies the mapped readback out in parallel parts (one
thread read the 91 MB of a 24 MP export at ~2 GB/s, ~45 ms).
Output is byte-identical (ahd_window_is_the_crop_of_the_whole,
uploads_read_back_exactly).
Export of a 24 MP ARW to full-size JPEG (examples/export_bench, 75
interleaved runs each on a shared 16-thread Ryzen):
main 802 ms median / 614 ms min -> this branch 647 / 525 (-19.3% / -14.5%).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
render_export / export_job now refuse an output box past 65535 px per side or
2^30 pixels (lightcraft_pipeline::check_output_size) before anything is sized
from it, and the GPU render checks w*h (and its 12-byte buffers) with
checked_mul, failing with a Limit error instead of overflowing.
Sources: own design.
(cherry picked from commit e0b279ad8e)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The kernels were only compiled when a device was created, so a module that doesn't build (a
function defined twice after an automatic merge, a type error) passed CI on machines without a
GPU and failed on users' machines. naga (re-exported by wgpu) now parses and validates every
module as it is handed to the device: the finish, blur, resize, map, mask and geometry modules
with their generated constants and bindings, the denoise convolution for every block size the
planner picks, the pooling kernel and the face detector's two. A test checks that a duplicate
definition is caught.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Gpu::new built the 27 compute pipelines one after another. On DX12 each one is compiled by FXC
(issue #471), single-threaded, inside create_compute_pipeline, so every process waited ~2.2 s for
them on an Intel Core Ultra 7 165U iGPU (app.gpu 2.4 s median) while the other cores idled.
build_kernels creates the shader modules and layouts on the calling thread and shares the pipelines
out to worker threads (std::thread::scope, an atomic job counter; the calling thread works too, so
every kernel is built even when no thread can be started). wgpu's error scopes are per thread, so
each worker pushes its own validation and internal scopes: a refused kernel or a panicked worker
still means no GPU, never a half-built one. app.gpu: 2.4 s -> 1.2 s median on that machine, exports
pixel-identical. The gpu: info line now reports the kernel count and build time.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018QWU3BMJMENFqFqxDHzV3b
set_enabled now stores the preference with SeqCst, as the warm-up thread reads it after releasing
its start, so a switch back on made meanwhile is always seen there (CodeRabbit). The start flag
uses SeqCst throughout.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Found by radialmonster: turned off before `lc-gpu-init` created the device and back on,
`warm_up` saw the start still held and returned, then the thread released it with no device,
leaving Settings > Performance at "GPU: starting…". The thread now releases the flag first, then
reads the preference again and tries again itself when GPU rendering is back on (it stops when
another start took over, at shutdown or with the environment's off switch). Settings >
Performance also calls the idempotent `gpu::warm_up()` in its "starting" branch (their
suggestion), on the repaint it already asks for.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- `app.gpu {wait: false}` with GPU rendering on starts creating the device in the background
(`warm_up`), so a caller that never waits still gets an answer later. `warm_up` starts one
thread at most, and may start again if GPU rendering was turned off before it ran.
- Settings > Performance asks for a repaint every 250 ms while it shows "GPU: starting…": the
device is created on another thread, which doesn't wake the window.
- CLI test: `{enabled: true, wait: false}` answers enabled with available, adapter and reason
null (skips with LIGHTCRAFT_GPU=0, since it starts creating a device).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MAX_STROKE_DABS bounds one stroke at two million, but the two callers
that will not look at nearly that many check the length after the
vector exists, which bounds nothing: it only discards what was already
built.
gpu/render.rs rejects a brush mask past MAX_DABS, 4096 across the
whole mask, after collecting every stroke's dabs. Each stroke may
construct two million first, so the sum that rejects them is reached
only once they all exist.
spots.rs skips an Auto Mask stroke past MAX_DABS_CONSIDERED, 200,000,
after brush_dabs has built the whole thing.
brush_dabs_within takes the caller's own budget, and push_dabs_within
charges against it instead of MAX_STROKE_DABS. Both callers now ask for
one more than they accept, so a stroke past the threshold is still
recognised as past it without the rest being built: 4097 rather than
two million per stroke on the GPU path, 200,001 rather than two million
for the spot window.
Behaviour is unchanged for anything either caller keeps. A result
short of its budget was never truncated, so it is the whole stroke, and
a truncated one only ever reaches a caller that was going to reject it.
A truncated stroke is not a prefix: running out of room makes a segment
lay its remaining dabs further apart rather than stop mid-segment,
which is how push_dabs_counted already behaved at MAX_STROKE_DABS.
Conflicts:
- xtask/src/main.rs: upstream's runners (cargo test or nextest, no
examples, doctests apart, one wasm `cargo check --keep-going`) with
#2302's `ci --quick` on top: test_selected builds the same targets
(--lib --bins --tests), the wasm check takes the changed crates in one
call, the full test step is upstream's.
- apps/lightcraft/src/control_server.rs: upstream's `before` closure, with
the batch's no-delay sockets kept.
- docs/parity-checklist.md: LR-BEHAV-ACCESS from main (tests/it path),
LR-BEHAV-LOCALE / LR-BEHAV-LOCALIZE from the batch (Polish).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* CI: dev profile keeps line tables for workspace crates and no debug info for dependencies
* Control server tests: persistent_requests_and_slow_replies_outlive_the_idle_read_timeout no longer races the idle timeout
Each request (and the closing half-shutdown) reaches the socket before the
worker reads, so on a slow machine only the reply waits outlast the 100 ms
idle timeout, as the test intends. The idle test sends its input the same
way, and the no-frames test allows 5 s instead of 2 s to give up.
* CI: one integration-test binary per crate
Each crate's tests/*.rs become modules of tests/it/main.rs (51 test files
into 13 binaries, plus 6 that stay their own). Inner #![cfg] attributes
move to the mod lines. Files that change state for the whole process keep
a binary of their own: codecs jxl_tail_heap and raw sr2_heap (dhat global
allocator), gpu shutdown, shutdown_first and memory, ui-egui shutdown.
Docs, comments and the GPU probe name the new paths and targets
(--test it <module>::); the CI report names an it binary's skips after
the crate.
* CI: tests under nextest, doctests apart, one wasm check
xtask ci runs the tests with cargo nextest run --lib --bins --tests when
cargo-nextest is installed, else cargo test with the same targets and a
one-line notice; doctests run after them with cargo test --workspace --doc.
The heif and hardware GPU steps use the same runner. Examples stay out of
the test build (clippy --all-targets still checks them). Concurrent tests
under nextest follow the RUST_TEST_THREADS cap (NEXTEST_TEST_THREADS).
The wasm check is one cargo check --keep-going over the same crates, and
clippy runs with --keep-going.
* CI: tests run under cargo test by default, nextest with LIGHTCRAFT_NEXTEST=1 (per-process scene cache)
Dropping a wgpu buffer after the device's thread-locals are gone can
abort; the device owns the memory either way.
🤖 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
The GPU runner's workspace pool kept its free list and its count of made
workspaces behind two mutexes, and forget() lowered the count without
the free list's lock that a waiting caller holds. If the tiles in flight
failed between a caller finding every workspace in use and that caller
starting to wait, the wakeup was lost and the denoise thread blocked
forever, out of reach of cancel. The free list and the count now share
one lock in a small Workspaces type, so a give or a forget can't slip in
between the check and the wait. A caller woken that way after the device
was lost returns the GPU's error instead of making a new workspace on it.
The dark channel is a Gaussian blur, not a patch minimum, so near a
bright neighbour it can credit a dark pixel with more veil than its own
minimum channel holds; (c - air*(1-t))/t then went negative and was
clamped to black - the blotches on CR3 raws. Bound the credited veil by
the pixel's own minimum channel (d = min(dark/air_pre, c_min/air)),
which provably keeps every channel non-negative for any gain, airlight
estimate or windowed fixed_air, in both the CPU and GPU paths. Also
zero-guards the divisions, fixing a 0/0 NaN on an all-black frame.
Regression test: a uniform dark field beside a bright haze must render
uniformly; pre-fix the bleed zone crushed to black.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Held for the owner: #995 and #999 overlap, and #995's author offered on 10-12 to
close#995 once his commit was in #999. docs/parity-checklist.md: header keeps the
#1485 note and drops the #995 part.
On a profiled display the Grey and Sharpening-mask views rendered in the display's primaries, so
their histogram counted display-primary pixels, not the photo's sRGB ones. They now render in sRGB
like the clipping views (`Overlay::renders_in_srgb`) and are converted for the display after; they
show no photo colour, so nothing is lost.
The GPU equivalence test now covers Grey (mean/max LSB limits) and the shadow/highlight clipping
views (share of differing pixels, as for Visualize Spots: both renderers draw them with the same
`visualize::apply`, and a 1-LSB difference at 0/255 flips a pixel).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A max-pool, a channel-copy 2x2 transposed convolution (mode 1) and the
channel-copy 1x1 layer give an exact answer, so a store fault in mode 1
can't hide in tolerance (CodeRabbit review of #2380).
On the GTX 1070 this test passes even without mode 1's stepped stores:
FXC merges them the same way, but the driver runs that store right, so
only mode 0 was shown wrong. Mode 1 keeps the stepped stores as a
precaution; the shader comment and docs/gpu-pipeline.md now say so.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On DX12, FXC merged the conv kernel's four output stores (modes 0 and 1)
into one 16-byte store_raw. The DXBC is valid and aligned, but NVIDIA's
driver ran it wrong on a GTX 1070: every second pixel's channels came
out shifted, so AI Denoise on the GPU was silently wrong on Windows.
DXC and Vulkan were right on the same card.
The stores now step by `1 + p.zero` (the always-0 uniform, renamed from
`pad`), which FXC cannot fold, so it keeps them apart. A two-layer
channel-copy network checks the stores exactly; it fails on that card
without the fix.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GPU handles(): only output-device proofs stay on the CPU; Show All Masks now renders zoom windows (upstream); proof-boundary tolerance in the GPU equivalence test; device-profile draft test.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
On an Intel Mac with an AMD Radeon Pro 5300M the Metal compiler crashes
on candle's mlx_gemm kernel, candle-metal-kernels panics on the compile
error, and the worker reported "the AI model failed unexpectedly" and
loaded the model on Metal again for every next job, so AI masks never
worked there.
The worker now retries a job that fails on the GPU (a panic or a candle
error) once on the CPU, after freeing the model. When that works, the
model stays on the CPU for the session and the job's outcome carries a
notice that segment_poll shows once; when the CPU fails too, the GPU
was not the cause and the next job uses it again. Missing or damaged
model files, render and remote-worker errors are not retried. Only
builds with a GPU path (macOS) retry; LIGHTCRAFT_GPU=0 and
LIGHTCRAFT_GPU_BACKEND=off now keep SAM 3 on the CPU from the start
(lightcraft_gpu::env_off_reason), like AI Denoise and the face models.
lightcraft-segment: Sam3::load_cpu, GPU_BUILD; text_logits returns the
crate's Error so Describe failures are classified like clicks.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Conflict only in docs/parity-checklist.md: the generated summary is
refreshed with `cargo xtask parity --write`, and the Last updated line
keeps main's format with this PR's change note (today's date) instead of
the duplicated note the branch carried. ⌥-drag preview still wins over
AllMasks and the selected mask in view_overlay; overlay kinds 6–9 unchanged.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Once a Vulkan or DX12 device is lost, wgpu 30 panics in `Device::poll`
instead of returning an error: the device error is not a `PollError`, so
`wgpu_core.rs` hands it to `handle_error_fatal`. The readback assumed the
wait would fail. A render whose readback found the device lost panicked
(caught by `try_render`), and while it unwound, a buffer going back to
the pool polled again from a destructor (`Gpu::trim`). That second panic
aborted the app ("panic in a destructor during cleanup"), as in the
reporter's logs on an Intel UHD 620 under Vulkan, each time a merge lost
the device.
The render device's three polls (the readback, `trim`, `sync`) now go
through `Gpu::poll`, which stops polling a lost device (the device-lost
callback, or a poll that panicked) and turns a panicking poll into an
error that retires the GPU. The render then fails over to the CPU and the
app keeps running. A GPU retired for another reason (a validation error)
keeps a live device and is still polled, so its pooled buffers are freed.
Regression tests run in a child process (they lose the device):
- `a_poll_on_a_lost_device_neither_panics_nor_aborts`: polls panic as
wgpu's do on a lost Vulkan device; a readback, then an unwinding render
whose destructor trims the pool. Before the fix the child aborts with
SIGABRT and the log line above.
- `a_really_lost_device_is_not_polled_again`: a Metal device really lost
(4200 unsubmitted command buffers, past wgpu-hal's 4096) is not polled
again. Before the fix the poll still reaches wgpu.
- `a_retired_but_live_device_is_still_polled`: a GPU retired by a
validation error still polls (and releases its buffers).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RbXK4njy12cpm4PG12Xrqt
Holding ⌥ (Option) while dragging a slider shows a diagnostic view in the loupe, as in
Lightroom; releasing ⌥ or the slider brings the photo back and nothing is saved:
- Blacks: shadow clipping (white; black where every channel is 0, the colour of the
unclipped channels where only some are).
- Whites / Exposure: highlight clipping (black; white where every channel is 255, the
clipped channels' colour where only some are).
- Sharpening Masking: the weight sharpening gets at each pixel, white on black.
- Sharpening Amount / Radius / Detail, luminance NR: the image in grey.
New `Overlay` variants (ShadowClip, HighlightClip, SharpenMask, Grey) are drawn by the shared
`visualize::apply`, so CPU and GPU agree; the sharpening mask needs the detail planes and
renders on the CPU (like HdrRange). The mask uses the same weight function as the render
(`finish::sharpen_weight`). With a display profile the clipping views are tested in sRGB, like
the histogram, and diagnostic views keep the photo's histogram instead of the view's.
Tracker: LR-VIEW-CLIPPING ✅ → 🟡 (J only frames the photo, it doesn't paint clipped pixels)
and LR-EDIT-DETAIL-SHARPEN ✅ → 🟡 (Radius and Detail don't reach the render).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
aiongg