CodeRabbit counts the functions around the lines a PR changes, not only the
changed ones, so the PR's docstring coverage stays low (59.49%, threshold 80%)
unless the untouched neighbours are documented too. Comments only; no code
changes.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CodeRabbit measured the docstring coverage of the PR at 59.49%, under its 80%
threshold. Add short doc comments saying what each helper does and which
behaviour each test pins. Comments only; no code changes.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Found while testing crates/engine/src/commands.rs:117-120 (turn) with crates/engine/src/canvas_geom.rs:224 (Turn::affine); applied in crates/engine/src/lib.rs:807-814 (Session::edit)
Co-authored-by: Claude Haiku 5.5 <noreply@anthropic.com>
Edit > Presets > Preset Manager renamed and deleted brush presets directly, so a rename
left the current preset pointing at the old name (brush.presets.update then failed), accepted
a case-insensitive duplicate name, and a delete left the current preset dangling. It now uses
the same rules as brush.presets.rename / brush.presets.delete through two small shared
helpers (rename_at, forget_deleted_preset). Patterns and custom shapes are unchanged.
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Found while testing crates/engine/src/notes_cmds.rs:105
Co-authored-by: Claude Haiku 5.5 <noreply@anthropic.com>
Co-authored-by: Brandon Thomas <echelon@gmail.com>
Snap To Layers built each layer's edges and centre from transform_bounds, which clips the
layer to the selection. With a selection, every layer's targets were the selection's own
bounds, so dragging the outline with a marquee snapped it back to where it started (shown
as layer alignments) and beat nearby guides and document edges. Layer targets now use the
layer's whole content bounds, as in Photoshop; the moving box (layer_rect) is unchanged.
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Covers #5044's two names: BMDoHyeon (family "BM DoHyeon", guessed "BM Do Hyeon") and Daum_SemiBold (guessed "Daum_Semi Bold"). Both resolve to their installed faces and aren't reported missing on main, through the exact PostScript lookup (#1844) that missing-font checks follow since #608.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
fontSize is in points and type.create renders it at resolution/72 px
per point, but the bar height, label height and baseline treated it as
pixels. At 300 ppi a 12 pt label (50 px) overlapped the bar. Lay out in
font_size * dpi / 72 px; type.create still gets points. 72 ppi is
unchanged.
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
paste_lorem decided whether to insert into existing text from the active
layer, while with_text edits the layer the "layer" param names. With a
pixel layer active, {"layer": T} made a new paragraph layer and left T
unchanged. An explicit "layer" now always goes through with_text, so a
type layer gets the text and any other layer is an error; without
"layer" the behaviour is unchanged.
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The "Open color-less EXR as ACEScg" option was only applied by file.openAs; background
opens (File > Open, drag-drop, Open Recent via Session::start_open) and file.openExrParts
kept io's linear-sRGB default. Both now retag through tag_exr_colorspace, start_open before
the Ask When Opening prompt so it sees the ACEScg profile.
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* fix(atn): a Paint Bucket step keeps its explicit colour through .atn export and import (#7199)
* test(atn): an explicit Paint Bucket colour survives an .atn round trip (#7199)
* fix(engine): flip and rotate presets turn about the selection's bounds, as Free Transform does (#6928)
* test(engine): cover flips and turns about the selection (#6928)
Found while testing crates/engine/src/variables_cmds.rs:285-292 (import_data_sets_text; the file is read in import_data_sets at line 279)
Co-authored-by: Claude Haiku 5.5 <noreply@anthropic.com>
Found while testing crates/algo/src/camera_raw.rs:592 (value taken unchecked in crates/engine/src/lens_cmds.rs:137 raw_params)
Co-authored-by: Claude Haiku 5.5 <noreply@anthropic.com>
Image > Trap wrote the active layer's pixels without checking locks, so a
layer under Lock All or the pixel lock (its own or its group's) was trapped
anyway. Check check_pixels_unlocked before writing, like the fill commands.
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Reveal in Finder on Linux opens the linked file's parent with xdg-open,
which launches whatever it is given. A link stored as `tool.jar/a.png`
names a file as its parent, so Reveal launched that file. Reveal now
refuses a parent that is not a folder.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
`file.package` absolutised the caller's `dir` and then built `folder`, `document`
and every `links` entry on it with `join`, which always writes `/`. On Windows
that left one string spelt both ways:
`C:\Users\...\pkg/Print Me/Links/art.png`. The control channel takes `/` and
rejects alternate separators, so a reported path could not be handed back to it.
Spell the absolutised directory with `/` as well, keeping a leading `\\`
(verbatim and UNC prefixes, where Win32 does not accept `/`) intact.
Main prints through craft-print, which runs `lp`; a Flatpak has no `lp`,
so File › Print fails there. This keeps craft-print as the route wherever
`lp` is on the path (and on Windows) and adds two more behind it:
- `photocraft-ipp`, a pure-Rust IPP client (CUPS-Get-Printers/Classes/
Default, Print-Job over HTTP/1.1) that talks to CUPS over its socket or
TCP; the Flatpak manifests grant --socket=cups for it. A `printServer`
named in the request always takes this route. `file.print.printers`
lists the queues.
- The XDG Print portal as the app's spooler (`Session::print_spooler`),
for a sandbox without the CUPS socket or a system without a printer:
the system print dialog, seeded with copies, paper and orientation.
PHOTOCRAFT_PRINT_PORTAL=1 prefers it, =0 removes it.
A route that exists but fails (unknown printer, refused job) is an error;
only a missing route moves on. Results gain `backend` (lp, windows, ipp,
portal). docs/printing.md describes the routes.
* Image Assets: two layers naming the same file no longer overwrite each other (#6614) [WIP]
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Image Assets: two layers naming the same file no longer overwrite each other (#6614)
generate_assets wrote every requested asset, so two layers (or two
default variants) naming one file silently replaced each other and the
result listed the file twice. Each file is now written once per run:
layers go top first, as the Layers panel lists them, the first writer
keeps the name, and later ones are reported in errors. Names are
compared ignoring case, since Logo.png and logo.png are one file on
Windows and macOS.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
With Export Preferences › Location = Same Folder, Quick Export wrote
<name>.<format> beside the document's file with no check, so a document
opened from photo.png was exported over photo.png itself. The app now
opens the save dialog for that case, and file.export.quickExport refuses
it without a `path`. Overwriting an earlier export beside a PSD is kept.
Adds file_cmds::same_file (text, dev+inode on Unix, case-insensitive
canonical path on Windows).
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
#5344 added the export_presets field to Persisted; #5616 rewrote load_presets_json to read each set with take() and its Persisted initializer lacked the new field, so main stopped compiling (E0063 at presets/mod.rs:180). Read export_presets the same way as the other sets; the existing load step and round-trip test cover it.
Co-authored-by: 00xJ0 <56056764+00xJ0@users.noreply.github.com>
One malformed set in the preferences file (gradients, swatches, styles...)
made load_presets_json ignore the whole presets blob, so saved New Document
presets vanished at every launch: the shell merges back only changed keys,
leaving the damaged set on disk. Each set now loads on its own; null and
malformed sets keep their defaults.
Refs storytold/photocraft#5602
Co-authored-by: Cursor <cursoragent@cursor.com>
Strict workspace Clippy stopped on collapsible_match in affinity, raw, plugins, engine, ui-egui and xtask. Pure conditions become guarded match arms (each falling through to an existing empty arm), and the ruler drag state and the measurement command result are bound to locals, preserving evaluation order, error messages and fallbacks.
The same lint fixes also appear as a side commit in #5157 (David2766), which can drop it on rebase.
A command that fails on a layer id or name the document doesn't have now says which layers there are, top-down as id and name (at most 20, names cut at 64 characters), so an agent can recover from the error alone: ids are not positions, and a freshly opened PNG's Background is layer 2. NoLayer becomes UnknownLayer { id, known } at both command entry points, keeping the 'no such layer' prefix. A layer name that matches no layer exactly matches regardless of case; an exact match always wins.
Fixes#5058.
photocraft_algo::faces::detect finds faces in an RGBA image with a score, a box and five landmarks, through a pure-Rust port of libfacedetection's YuNet network (Shiqi Yu, BSD-3-Clause; licence, NOTICE and ATTRIBUTION rows included). The weights are the library's, converted to half precision (107 KB), parsed defensively. image.detectFaces reports the document's faces to scripts and agents; it is read-only with no history step and no menu entry. The network is compiled out of the browser build, where the command reports itself unavailable. Head outputs are pinned against the C++ reference.
Part of #3603.
brush.defineFromSelection allocated the sampled tip and the selection mask infallibly over the whole selection, so a large selection could abort the process. Both now go through the engine's fallible allocator and return a normal error that leaves the brush, presets, history and document untouched. A failpoint test asserts all four.
Part of #212.
Paths these features keep in the document go through the same resolver as
linked smart objects (relative to the document's folder; other computers,
devices and URLs refused):
- A Displace smart filter's `mapPath` and a Color Lookup smart filter's LUT
`file`, re-read on every re-render: a path that is not followed leaves
the pixels as they are, as for a missing map or LUT.
- Apply Data Set's Pixel Replacement images.
- Reload Frame of a video layer's footage.
Apply Data Set and Reload Frame say why a path is not followed.
Save As PSD/PSB, layered TIFF and Package as PSD read a linked smart
object's file to embed it. The path is resolved with the same resolver as
other linked-file reads, against the document's own file (the new
ExportOptions::document_path / PsdExportOptions::document_path, set by the
engine's saves and the app's Save As). A link that is not followed is
written as pixels with a warning that says why, as for a missing file.
Nested smart-object documents resolve against the same document.
A linked smart object's path comes from the document. Every place that
follows it now goes through one resolver (photocraft_io::doc_path, built on
craft-docpath): a relative path is taken from the document's folder, and
only local files or files on the share the document itself is on are
opened. Paths to other computers, device paths and URLs are treated like a
missing file: the layer keeps its last rendering.
- Link watch: such links are not stamped, and no background look starts
when no link of the document is followed.
- Update Modified Content and other re-renders read only followed files;
the error names why a link is not followed.
- Package lists such links as missing instead of copying them.
- Reveal in Finder for a linked layer shows only a followed file; the
document's own Reveal is unchanged. The macOS invocation ends options
with `--`, and Explorer is not asked to select a path containing a comma
or a quotation mark.
- Place Linked stores a relative path as an absolute one.
The document path travels to smart-object renders in a thread-local scope
entered by Session::edit, command dispatch and background jobs.
Co-authored-by: David Mann <daaavid.mann@gmail.com>
Conflicts:
- distort_ui.rs: main's `drop_foreign` (#5092, #5123) closes the Content-Aware Fill workspace
too when another document becomes active or its own is closed, stopping its background
preview as Cancel does; what Apply wrote stays. New test
`the_workspace_is_dropped_when_another_document_becomes_active`.
- sr, tr, vi catalogs: both sides' rows. Dropped es's High, Low, Duplicate Layer and Contract,
which main has now with the same translations.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* GPU memory: a graphics card's own memory isn't charged for the document; GPU Memory preference; GPU Memory Full in the status bar
Many layers hit a cliff: past what the GPU memory budget leaves for layer pages, every edit
that redraws the whole document (opacity, visibility, a nudge) goes to the CPU compositor.
Measured on 3000x2000 full-canvas layers (RTX 5090, 31 GB RAM, Memory Usage 8 GB): 150
layers took 2.6 s per such edit, 300 layers 6 s. The budget was Memory Usage minus the
document's pixels and History, which is right where the GPU shares system memory, but a
discrete card's memory doesn't hold the document, so 150 layers of 6 MP (3.6 GB) squeezed
the budget below what they needed.
- Automatic budget: discrete adapters are no longer charged for the document's pixels
(integrated and software adapters still are). 150 layers: 2.6 s -> 25 ms.
- Preferences › Performance › GPU Memory (MB, 0 = Automatic; performance.gpuMemoryMb) sets
the budget outright. 300 layers with 12 GB: 6 s -> 45-72 ms.
- The status bar shows "GPU Memory Full" while the active document's layers don't fit
(both status bar styles); hovering gives the numbers, clicking opens Preferences ›
Performance. Its Graphics rows show the budget and what the layers need. The slow CPU
redraw notice gives the numbers and points at GPU Memory.
- Headless: the preference is a normal pref (prefs.set), and ui.state's perf.timings
reports gpuLayerFit {doc, needed, budget} next to gpuBudgetMb.
Next (owner-approved plan): after #5370, read the card's real limit from the driver
(wgpu memory budget thresholds) and shrink on out-of-memory; then redraw the visible
area on the GPU first when layers still don't fit.
* GPU memory: typed discrete check; the status and budget follow a GPU Memory change at once (measure_fit); perf rows P60/P61 for whole-document edits among 150 full-canvas layers under the app's budget
* GPU memory on top of #5234: memory_budget takes the card's memory (vram) and the GPU Memory value; the document's pixels are charged unless the card's own memory is known; a set GPU Memory is capped at the card's share; benches use video_memory
* Security: enforce the automation read boundary on stored smart-filter params
Smart filters store a command id and params inside the document, so a
crafted .pcraft could resolve stored ambient paths (displace mapPath,
color-lookup LUT files) at render time without the authorize gate:
synchronous smart-object rendering has no Session argument, spawned job
threads drop the requesting session's authority, and a trusted UI
render could warm the process-wide stack cache with a stage the gate
never saw.
Thread the requesting session's gate through a thread-local in jobs.rs
(entered around command execution, reinstalled on spawned workers and on
job apply, restored on drop), re-check stored filter commands in
render_in before source resolution and any cache lookup and in
apply_filter_to_surface, and re-authorize layer.smartFilter.setParam
with the complete merged params after merging stored document params —
including edits whose request contains no path and actions.play steps.
Denial is conservative: the document is unchanged and no worker starts.
Regression tests in the automation crate cover rerender, cached-stage
reuse, merged-param edits, action playback, background jobs, and filter
transfer, for both the displace map and color-lookup LUT vectors.
* Review: carry render authority through Session::edit and surface denials
Session::edit is the engine's universal mutation entry (linked-file
refresh, transforms, place); renders reached through it ran with no
authorization scope installed, so stored smart-filter params read
ambiently there even after the execute-path fix. Install the scope in
Session::edit as well, keyed on the session's authorize gate.
apply_filter_to_surface also conflated a denied filter with an unknown
one: authorize_render_command's Result was discarded with .ok()?, so a
denial silently rendered unfiltered pixels and the stage entered the
process-wide stack cache. The signature becomes
Result<Option<Surface>>: Err for denials (callers propagate), None for
ids that aren't pixel filters (pass-through semantics unchanged), and an
unreadable map keeps its deliberate pass-through. apply_smart_filter and
apply_smart_filters propagate instead of cur.clone().
Two tests: a refresh driven outside command execution (Session::edit)
is denied and leaves the document unchanged; a denied render caches no
stage. Nine authorization tests pass, engine suite and strict Clippy
clean, panic hunt green.
* ui_set description: name the layerFilter field main added
The Layers-panel filter dropdown (#5048) added layerFilter to the
control method's UI_SET_FIELDS without extending the ui_set tool's
description, so the drift guard (mcp.rs, ui_set_description_names_every_
accepted_field) fails on Windows CI for any branch built on main. Name
the field with its patch shape: by (kind, name or mode), kinds (a list
of layer kinds), name (matches part of the layer name), mode (a blend
mode name). The 40 MCP tests pass; strict Clippy clean.
* wasm size: compose and algo join cms at opt-level "z" to clear the 24 MiB gate
Main's browser build is 171 KB over the 24 MiB wasm gate in CI
(25,337,343 bytes), so every branch built on current main fails the
web size check. Demote three pixel crates in the wasm-release profile:
- cms: transforms are built once and cached (transform::cached),
so "z" costs little runtime (measured -42 KB).
- compose: the browser app composites per frame on the GPU (crates/gpu,
WebGPU/WebGL2), so the CPU compositor is not the browser's hot path
there (measured -122 KB).
- algo: filters run per pixel on every interactive filter application.
"z" there is the lever of last resort (measured -331 KB), taken
because the gate is enforced in CI; if browser filters feel too slow,
restore 3 here and reclaim the bytes from a feature-gated decoder.
Final size 24,811,949 bytes locally, 354 KB under the gate; the native
profiles are untouched. If a maintainer prefers keeping algo at 3, the
alternative is raising the self-imposed 24 MiB early-warning limit
(Cloudflare's hard cap is 25 MiB) - a policy call in packaging/web/
package.sh, not a code fix.
* Tests: Free Transform applies when another tool, layer or New Layer is picked (#5665, #5667)
Whole-app regression tests for the paths #5665 and #5667 report (a moved box, and a placed image moved or untouched; toolbar tool click, Layers panel row click, New Layer). They pass on main, so these issues don't reproduce here; the tests pin Photoshop's behaviour (apply, never silently cancel) while the reporters narrow down their environment.
* New Layer while Free Transform is open applies the transform first (History: the transform, then New Layer; a placed layer's step no longer swallows New Layer); tests: a real placed fixture, the New Layer button, History order
* Free Transform + New Layer: check the command can run before applying the transform; a refused transform keeps the box and the placed layer and returns the refusal
* Free Transform + New Layer: Session::precheck validates the call (params shape, New Layer's values, precondition) before the box is applied
* Free Transform + New Layer: commit-first only for PRECHECKED commands; precheck shares dispatch's admission checks (named-layer precondition, jobs, hidden target) and checks a named layer exists; Group from Layers documents its layer param; test: precheck agrees with execute
* Free Transform + New Layer: precheck resolves a layer given by name as the command does; Layer via Copy/Cut leave PRECHECKED (their failures show only when they run); rebased (lib.rs mod lines)
* Shortcuts without the UI: shell commands and tools in the engine, reserved keys, set files
A retro against "everything is drivable headless; the UI is a layer" found the shortcut
work leaning on the UI. Fixed:
- The shell's command table (UI_COMMANDS: View › Zoom In, the panels…) moves into the
engine as data (shell_commands::SHELL_COMMANDS; menus::UI_COMMANDS is now an alias), with
the tools (shell_commands::TOOLS: id, Photoshop name, default key; a test keeps it equal
to the toolbar). edit.keyboardShortcuts binds, lists, imports, exports and checks them for
conflicts with no UI: a headless .kys import takes tool keys, and an export is the whole set.
Tools may share a key (Shift cycling); a command given a tool's key takes it from them.
- Reserved system keys are refused by the engine (the dialog uses the same check) and left
out of imported sets.
- importKys / exportKys / summarize take {"path": …} to read or write files, like
actions.save; the automation gate refuses those paths for MCP.
* Keyboard shortcuts: bindings commit only after their exports are written (a bad target changes nothing); docs: the command knows the tools
* Keyboard shortcuts: both export targets are resolved and checked before either file is written; a failed later write removes the earlier one
* docs: reserved keys per platform
* Keyboard shortcuts export: files are staged next to their targets and moved into place only when all are written; a failure leaves the user's files untouched
* Keyboard shortcut exports all or nothing: unique create_new staging and backups beside each target, restored on any failure; the same target twice refused; docs: Ctrl+Shift+Esc
* Shortcut exports: a failed backup move removes its placeholder; undo restores backups without deleting first and names any it couldn't; read-only targets refused up front
* PHOTOCRAFT_PRESENT_MODE / PHOTOCRAFT_FRAME_LATENCY launch overrides for latency A/B tests (#1343)
Linux traces put the remaining stroke latency in presentation queueing (the app's frame fits
in one refresh; the rest waits on the swap chain), and an NVIDIA user cured it by forcing
vsync off. Testers can now compare without rebuilding: PHOTOCRAFT_PRESENT_MODE (fifo,
mailbox, immediate, autovsync, autonovsync) and PHOTOCRAFT_FRAME_LATENCY (1-3), applied over
the platform default. Bad values are logged and ignored. The active mode is logged at start,
kept in GpuInfo.present and shown in every stroke trace line.
RTX 5090, Vulkan, 120 Hz: default autovsync/latency 1 waits 6 ms per 8.3 ms frame; mailbox
renders every 2.3 ms with 0.2 ms wait, so each vsync shows the newest frame.
* docs: present mode and frame latency overrides
* Present mode overrides never request a mode the driver may lack (Mailbox crashed on X11/NVIDIA)
* Preferences › Performance › Low-Latency Drawing, on by default: no vsync wait while drawing (#1343)
Testers' traces put the ~50 ms stroke lag in the swap chain's vsync wait. With vsync off it
disappears (X11/NVIDIA; Windows/NVIDIA via the driver), and a shorter queue doesn't help. The
surface now presents with AutoNoVsync by default: wgpu picks Immediate, else Mailbox, else
Fifo by what the driver offers, so no unsupported mode is ever requested. Turning the
preference off brings vsync back (smooth, but late). It applies at the next launch, like the
other graphics settings. Where only Immediate exists the canvas may tear while it changes;
frames render as fast as input arrives during a stroke and stop when idle.
* Runtime and launch controls: ui.set strokeTrace, --stroke-trace, --present-mode, --frame-latency
The retro's remaining gap: the stroke trace and the presentation overrides were env-only. The trace now switches at runtime over the control channel (ui.set {"strokeTrace": true|false|"stderr"|path}) and with --stroke-trace; --present-mode / --frame-latency sit beside their env vars and win over them. Bad values are reported and ignored; none fails a launch.
* docs: strokeTrace beside the other ui.set fields
* docs: regenerate the scorecard after rebasing onto main
* Low-Latency Drawing: frames capped at 240 fps without vsync (after #4472); ui.set strokeTrace takes on/off only (a CSV path is launch-only); tearing documented as usual (Immediate); pref hidden on web; doc comment order
* stroke_trace: from_value doc says ui.set passes only on or off
* Low-Latency Drawing ships off (opt-in), per the owner's decision
* docs: present mode default is vsync (Low-Latency Drawing ships off)
