Say that `photocraft --help` lists the flags and their variables, that
a flag wins and an empty variable is unset, and what is a usage error;
that a bad --present-mode or --frame-latency is one while a bad
variable is still ignored; list PHOTOCRAFT_IN_WINDOW_MENUS; and that a
token with a token file is a usage error for the control server too.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Step 5 of the issue: launch_args' hand-written loop, USAGE and
LaunchArgs become a clap derive, which flattens photocraft-automation's
ControlArgs and AutomationArgs. `photocraft --help` comes from the doc
comments; `--version` prints the same `photocraft <long version>` line
(build_info::long_version_static), and `-V` now does too. --control
uses photocraft_automation::args::control_port instead of a local copy.
The cases the loop handled by hand are kept:
- An old macOS Finder's -psn_... argument is dropped before clap,
unless it comes after `--`, where it is a file name.
- The per-process options for the single-instance hand-off come from
ArgMatches::value_source, in a fixed order. A PerProcess enum names
each (and PHOTOCRAFT_GPU_ADAPTER): id() gives a flag's clap id,
Display the flag or the variable, so single_instance's log line reads
as before. As since #7242, a flag's variable counts as the flag: one
clap reads (PHOTOCRAFT_IN_WINDOW_MENUS when it turns the menus on, an
automation root that isn't blank) is counted here, as clap read it;
env_launch_options keeps #7242's rules for the three read elsewhere
(present mode, frame latency, stroke trace) and returns PerProcess.
- A file name that isn't valid Unicode is still reported in a notice
instead of being opened (#1108); file_names returns
FileNames { openable, unreadable }.
- Windows release builds still have no console: help, version and
usage errors show only when redirected, as since #6435. clap's
Error::exit, like print_stdout, never panics on a closed stream.
The variables are now declared on their flags: PHOTOCRAFT_CONTROL_PORT
on --control, the token variables through ControlArgs (so
security::token_inputs, which nothing else used, is removed), and
PHOTOCRAFT_AUTOMATION_*_ROOT added on the desktop's Command only, so
photocraft-cli still ignores them (#1551).
Behaviour changes, as step 0 of the issue decided:
- A bad or missing --present-mode, --frame-latency or --stroke-trace
value is a usage error (exit 2) instead of a warning. Their
variables stay lenient (logged and ignored); presentation's value
checks serve both, and the flags refuse a value with a ValueError.
- A repeated flag is a usage error, and a value can't start with `-`
unless it is a negative number (write --opt=-value).
- clap reports the first usage error, not all of them. On an unknown
argument clap resolves the pending option value but drops its error,
so `--control nope --safegpu` names `--safegpu`.
- An empty or blank variable counts as unset; before, only an empty
PHOTOCRAFT_CONTROL_PORT did, and an empty token or root was passed
on as given. PHOTOCRAFT_IN_WINDOW_MENUS also reads false, no and
off as off (FalseyValueParser). A variable that isn't valid Unicode
where text is expected is a usage error instead of being ignored.
- With --control (or its variable), a token together with a token
file, from flags or variables, is clap's either/or usage error at
parse time, before the server's own check. Without it no token is
read, so a stale exported pair stops nothing, as before.
- With --control given, PHOTOCRAFT_CONTROL_PORT is not read, so a
bad value there is no longer reported too.
The CPU-compatibility relaunch puts --safe-gpu before the original
arguments, so a `--` among them can't turn it into a file name.
The launch_args unit tests now check clap's errors instead of the error
list. The process tests put a late usage error (--control 0 with the
token pair) where they used a bad port, so stderr still shows how the
other arguments were taken, and cover the variables.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Two process tests of today's behaviour that the clap port keeps:
`--version` prints `photocraft <long version>` and stops reading the
arguments, and PHOTOCRAFT_AUTOMATION_READ_ROOT / _WRITE_ROOT reach the
control server's workspace like their flags. The process tests return
a Run struct instead of tuples, and control_exit_code gains run_with
to set variables.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
The hand-written argument loop, its LaunchArgs, USAGE and helpers
(parse_control_port, usage_exit_code, print_stdout) and their unit
tests move unchanged from main.rs into a new launch_args module; only
the module doc, the `mod`/`use` lines and pub(crate) visibility are
new. main.rs keeps the environment defaults and the exits.
This is the step before the clap port, so that one reads as a rewrite
of launch_args.rs rather than a large main.rs deletion. Check with
`git show --color-moved=dimmed-zebra`.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
With the environment fallbacks, clap's conflicts_with on the token pair
made a stale exported PHOTOCRAFT_CONTROL_TOKEN and
PHOTOCRAFT_CONTROL_TOKEN_FILE a usage error for every mcp and serve, though
headless mcp and stdio serve read no token. As on main, the pair is now
refused only by serve on TCP and mcp bridging, still as clap's either/or
usage error naming each half as it was given.
An automation root on the command line with PHOTOCRAFT_CONTROL_PORT set
is a usage error too: the variable would switch mcp to bridging, which
drops the roots, with only a note on stderr.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Say that PHOTOCRAFT_CONTROL_PORT is the fallback for serve --port and
mcp --bridge (with the stderr notice and the empty-is-unset escape),
and list the CLI's variables on the book's CLI page, which also says
the automation root variables stay the desktop app's.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
The desktop app's --control already reads PHOTOCRAFT_CONTROL_PORT, and
the CLI read the token variables at run time. Now every fallback is
declared with `env =` on its flag (clap's `env` workspace feature, no
new crate in Cargo.lock):
- --control-token and --control-token-file in ControlArgs fall back to
PHOTOCRAFT_CONTROL_TOKEN and PHOTOCRAFT_CONTROL_TOKEN_FILE, as
security::token_inputs did at run time. The CLI no longer calls
token_inputs, so the variables are read in one place; the desktop
app still uses it until step 5. Help names the token's variable but
never shows its value (hide_env_values). A token from the
environment together with the other flag is now clap's either/or
usage error (exit 2) rather than the automation crate's (exit 1).
- `serve` takes PHOTOCRAFT_CONTROL_PORT as --port and `mcp` as the
bridge port when the flag is absent, as #2495 proposed. The variable
must be a port: `mcp` refuses 127.0.0.1:7878 or :7878 from it, as the
desktop app's --control would; only --bridge takes host:port.
- The automation roots stay flag-only for the CLI: a
PHOTOCRAFT_AUTOMATION_*_ROOT exported for the desktop app must not
widen headless file access (#1551).
- A flag wins, and an empty or blank variable counts as unset
(blank_env_is_unset; clap would hand the empty value to the parser),
so stdio and headless stay the default.
- A bad value or conflict from a variable is a usage error naming the
variable instead of the flag it stands in for (name_env_vars).
- The port variable changes what the command does, so when it (not the
flag) turns on TCP or bridging, the command says so on stderr and
names the way back (ArgMatches::value_source tells which).
bin() in the tests removes the token and port variables, so an
exported one can't change a test. New tests:
control_port_env_switches_modes_with_a_notice_and_flags_win (ported
from #2495, also checking the bad values) and
control_token_env_vars_are_the_flags_fallbacks.
Co-authored-by: ahmedhosnypro <ahhosnyas@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Step 4 of the issue: the control token flags and the automation roots
move to photocraft-automation's new args module as two clap structs,
which mcp and serve flatten and the desktop app can flatten in step 5
without moving them again. clap is a plain dependency of the crate.
- ControlArgs holds --control-token and --control-token-file;
AutomationArgs holds the roots, and AutomationArgs::workspace opens
them (replacing the CLI's automation_workspace).
- The port value parser is photocraft_automation::args::control_port,
with its unit tests, for serve --port and --bridge now and the
desktop app's --control later. It ignores surrounding spaces and
refuses anything else with a typed NotAPort.
- --bridge parses to Bridge::Port or Bridge::Addr: it takes a bare
port, meaning 127.0.0.1:<port>, or any value containing a ':', which
the bridge client then validates. Anything else is a usage error
(exit 2) instead of a bridge error.
- 127.0.0.1 addresses are built from Ipv4Addr::LOCALHOST.
mcp_bridge_takes_a_port_or_an_address is new: `--bridge nope` is a
usage error, and `--bridge 1` bridges to 127.0.0.1:1, which a tool
call names when nothing listens there.
No new environment behaviour: the token variables are still read at
run time through security::token_inputs, and the roots stay flag-only.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Name what typed values made a usage error: a value out of range
(--quality 0), malformed --new or --params JSON, <FILE> together with
--new, and --control-token together with --control-token-file. A path
that is not valid Unicode now reaches the command: convert takes it,
info, run and batch fail through the automation layer (exit 1), and
droplet, which takes text, refuses it as a usage error.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Step 3 of the issue: handlers get values that are already checked, so
a bad one is a clap usage error naming its flag (exit 2) and the
"bad --x" errors (exit 1) are gone.
- --quality is a u8 in 1..=100; --tga-bits, --dpx-bits and
--dds-compression parse to photocraft_io's TgaBits, DpxBits and
DdsCompression through their from_bits/from_name, and --tiff-compression
and --jpeg-format to photocraft_codecs' TiffCompression and JpegFormat,
refusing a bad one with a typed ValueError; serve --port is a u16. export_opts becomes `impl From<&ExportArgs> for ExportOptions`,
which can't fail.
- run's --new and --params parse to serde_json::Value, so JSON that
doesn't parse is a usage error; whether it is an object is still the
command's to say (exit 1). The --cmd/--params pairing keeps using
ArgMatches::indices_of, now over typed Step values.
- Paths are PathBufs (convert, info, run, batch, the control token
file and automation roots), and run hands the OsStrings to clap
instead of refusing non-Unicode up front: a Latin-1 file name works
as a path (#1108), and one given as text is clap's usage error.
load_actions_file takes a &Path, and photocraft_engine's same_file
takes any AsRef<Path>, so convert's #7437 check compares the PathBufs.
- Rules live in the types: run needs exactly one of <FILE> or --new
(ArgGroup "source") and a --cmd or --out ("work"); batch's --actions,
--in and --out are required; --control-token conflicts with
--control-token-file. The runtime checks and their messages are
deleted. The automation crate's own token check stays, as it also
sees PHOTOCRAFT_CONTROL_TOKEN(_FILE).
Tests changed for this: run_errors, quality_outside_1_to_100_is_refused,
dpx_converts_both_ways_with_the_bits_asked_for,
dds_converts_both_ways_with_the_compression_asked_for,
tga_bits_choose_the_targa_depth, jpeg_format_chooses_the_scans
(exit 2, the error names the flag) and
the #1108 test, now
a_non_unicode_path_is_taken_and_non_unicode_text_is_a_usage_error:
convert reads and writes such a file name end to end, while info opens
through the automation layer, whose paths must still be UTF-8, and
refuses it (exit 1). either_or_flags_are_usage_errors is new.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
The CLI page said parsing reports malformed flags and JSON. Name what
the step 0 decisions made a usage error: a single-value flag given twice
(--cmd and --params repeat), how to pass a value starting with '-', and
that `version` is only --version/-V. Say why --quality allows negative
numbers.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
run took &[String] and run_os &[OsString], and run_os only checked
each argument was Unicode before calling run. A single run generic over
Into<OsString> (as clap's try_get_matches_from is) does that check once
and serves main's args_os and the tests' &str slices alike.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
The behaviour changes decided in step 0 of the issue:
- A repeated single-value flag is a usage error (exit 2) instead of
the last one winning: args_override_self is gone, and with it the
argument scan that found repeats for mcp and serve (#8035). --cmd and
--params stay repeatable.
- A value starting with '-' was already written --out=-image.png or
./-image.png since the clap port (a flag can't be a value since
#8035); this pins it, and that `--filter -1` still works and
`--quality -1` is still refused by its range.
- The bare `version` subcommand is gone; clap's version flag prints
the same `photocraft-cli <long version>` line for --version and now
-V too. `help <subcommand>` stays.
clap's version wants a &'static str, so build_info gains
long_version_static, long_version worked out once in a OnceLock, next
to long_version. The desktop app will need the same in step 5.
flag_value_forms_and_repeats and top_level_help_version_and_no_arguments
now pin the new behaviour; a_value_starting_with_a_dash_is_written_with_equals
is new.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
batch --help printed the "--out that is the --in folder is refused"
sentence twice, in the subcommand's long help and on --in-place. Keep
it on the flag, where -h shows it too.
steps() read --cmd/--params with get_many, which panics on a type
mismatch, against the crate's never-panic rule. Use try_get_many and
treat an error as no values; behaviour is otherwise the same.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Behaviour change from the clap port, not listed in its commit: `help`
followed by an unknown name or extra arguments is a usage error
(exit 2); before, `help …` always printed the whole USAGE with exit 0.
usage_and_unknown_command now pins `help frobnicate` exiting 2 with
the error on stderr.
The --control-token-file help says it is used instead of
--control-token, as the old USAGE showed them as either/or. The
--in-place help carries the "--out equal to --in is refused" sentence,
so `batch -h` shows it and not only `batch --help`.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Replace the hand-rolled parser (USAGE, SUBCOMMANDS, Args::get/has) with
clap derive types in args.rs: a Cli parser, one Args struct per
subcommand, and flattened ExportArgs (convert, run, batch) and
ControlArgs (mcp, serve). The USAGE prose moves into doc comments, so
-h and --help carry it. Handlers keep their bodies; only their argument
type and the lines reading flags change.
Behaviour is kept: the last of a repeated flag wins (args_override_self),
values stay strings checked by the handlers with the same messages and
exit 1, a value flag never takes the next flag as its value, and help
there (`--out --help`) is still help, --version/version are checked
before clap, help goes to stdout with exit 0 and usage errors to stderr
with exit 2. run pairs each --params with the --cmd before it by putting
both back in command-line order with ArgMatches::indices_of.
mcp and serve keep #8035's launch checks: no positional arguments, no
repeated or empty options, --control-token or --control-token-file but
not both, a token flag only with --bridge or --port, and no automation
roots with --bridge. clap gives the first four (a repeat is found by
scanning the arguments while args_override_self stays on); a small
check after parsing gives the last two, with #8035's messages.
Behaviour changes (allowed by the issue's step 2):
- Help and error text is clap's: per-subcommand help instead of the one
USAGE page, "unexpected argument '--fromat' found" instead of
"unknown flag --fromat for batch", and so on.
- A wrong positional count is a usage error (exit 2) instead of a
handler error (exit 1): convert with one or three paths, info with
none, droplet with no inputs, run with two files.
- A positional starting with '-' (other than "-") needs a '--' before
it, and '--' now ends the flags instead of being an unknown flag.
- A value starting with a single '-' is written --out=-image.png (or
./-image.png), as one starting with '--' already was; --quality and
--filter keep negative numbers (allow_negative_numbers).
- `help <subcommand>` prints that subcommand's help.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
invoke returned (code, stdout, stderr) and ok (stdout, stderr), so each
call site had to remember the order. Return Invoke and Printed structs
instead.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Every failure was formatted into a String where it happened, so callers
could only print it. Subcommands now return a CliError, the actions
loaders an ActionsError and the hand-written parser a UsageError; each
variant prints exactly the text the String did, and the engine and
automation errors pass through unchanged.
Also: print_json takes a JsonStyle instead of a bool, the engine's
missing_font_warnings replaces the CLI's copy (same text), batch warns
through warn_all, run drops a second, no-op insert into its reported
font set, and the src tests share one Invoke helper.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
lib.rs held the whole CLI in one 754-line file. Split it into args,
output, document, actions, batch and automation modules so the argument
parser can be replaced with clap in a separate, reviewable change.
Function bodies, doc comments and tests move unchanged. The only edits
are mod/use lines, pub(crate) visibility on items now shared across
modules, and the imports the moved test modules need. The public API
(run, run_os, parse_actions, load_actions, load_actions_file, Steps,
USAGE) is re-exported from lib.rs.
Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
* perf(seam): drop the original-column map from the shrink proxy, same pixels (-15 % at 24 MP)
Content-Aware Scale's proxy seam search tracked each panel column's original
column in an idx map, one usize per proxy pixel, shifted by every seam. The
original column is x plus the number of columns already taken at or left of it,
so a per-row list of the taken columns answers the same question without a
buffer the size of the energy map.
The recorded P28 budget number predates the proxies: the 2026-10-05 baseline
commit carves at full height and recomputes energy per seam, and measures
266 s for the 24 MP case on the machine this was timed on, against 2.6 s now.
Documented in docs/seam-carving-performance.md; enforce stays off until a
budgeted-class run can vouch for the whole row.
* review: generate exactly ch samples per pixel in bench_seam, reject ch > 4
The fixture flattened four samples per pixel and then truncated to w*h*ch, so a
three-channel run grouped samples across pixel boundaries and any ch above four
built a buffer shorter than the rows carve_width indexes.
Generate ch samples inside the per-pixel closure and refuse ch outside 1..=4.
The three-channel row is dropped from the perf note rather than re-measured
here: it moved inside the timer noise either way, and the other agents' builds
on this machine make a fresh number worthless. The four-channel hashes are
unchanged, which is the check that this edit touched only the fixture.
* perf(seam): find the removed column by binary search, not a linear scan
Recording a seam recovered each removed original column by scanning the row's
list of already-taken columns, so a deep shrink paid a comparison per removed
column on every row and every seam: quadratic in the seam count, and worse than
the index map this PR replaced once a shrink got deep enough.
`o - i` never decreases along the row's sorted list, so that count is a binary
search, and the original column belongs where the search stopped; the rows are
independent and update in parallel, like the pixel shift. What is left per
removed column is one insert.
Output is unchanged: same columns, same order, same hashes (10 % and 50 % width
cases below), and `counted_removed_columns_match_the_tracked_map` now runs a
tracking and a counting carver side by side to one column and compares every
row's list after every seam.
docs/seam-carving-performance.md gains those cases, states that the ~104x
figure is the two example runs rather than the baseline row, and names
perf/baseline.json as the source of the 954 s.
---------
Co-authored-by: ebosido444-ai <ebosido444-ai@users.noreply.github.com>
* Speed up Crystallize and Pointillize with cell-space rasterization
* Measure pixelate commands on unlocked layers and Background separately
* Record pixel-exact 24 MP filter and command benchmarks
* security(engine): bound script, batch, adjustment, import and export work from file input
Scripts calling scripts share depth and step budgets; batch inside a played action charges the playback tree; destructive adjustments and mask adjustments visit stored tiles when the content bounds are a huge sparse grid; Copy Layers checks group depth; aux flattening and brush import refuse oversized canvases; video footage frames are budgeted, Render Video names frames after the leaf of the document name and reads no linked files for restricted sessions; ATN names are clipped; profile paths are read as regular files only; prefs.set no longer treats colorSettingsX as colorSettings.
* style(engine): rustfmt
* engine: address review - refuse unbounded sparse adjustments, carry batch playback charges, charge played steps to scripts, clip ATN targets, stricter colorSettings paths, cap image-sequence frames
* security(io): charge sparse layers, shape caches and shared decodes
ORA and PSD export refuse dense boxes past a cap; PSD import and ABR mapping decode shared masks/tips once; Affinity and SVG shape caches are charged to the import budget; SVG embedded JPEG size is probed; Krita tiles are charged for the backing tiles they land in.
* fix(io): clippy range pattern; align the sparse Krita test tiles to the backing grid
* io: address review - offset-aware KRA tile budget, saturating stroke reach, OpenRaster entry limit, PSD retained-surface budget, halved PSD gather cap
The CMYK proptests failed on main now and then (the_split_route_agrees_with_the_exact_path
after #8663). Three gaps, each found by the proptests at 5000 cases:
- cmyk.rs: a clipped Dissolve layer blended like Normal; it now shows each pixel fully or not
at all by the compositor's noise, as the RGB compositor does since #3052.
- cmyk.rs: a Dissolve adjustment layer mixed by its opacity; opacity and mask are now the chance
a pixel takes the adjustment, as on the RGB compositor (#3050).
- native.rs: the display/Flatten ink route didn't round to the document's levels after an
adjustment layer, where the RGB compositor and cmyk.rs do; coverage in a pass-through group
then differed by up to half a level.
The test reference model had the same two Dissolve gaps (ref_clip passed no position, so
Dissolve fell back to Normal), which is why random_documents_match_the_reference never caught
the first one. Three named regression tests cover the minimal cases.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* Fix magnetic lasso gradient polarity cost
* Add magnetic lasso gradient polarity regressions
* style: cargo fmt
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Brandon Thomas <echelon@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* skip redundant smart object resamples and per-pixel reads
* resample smart objects only when their re-render fails
---------
Co-authored-by: Brandon Thomas <echelon@gmail.com>
A Light layer is stored under the Brightness/Contrast `brit` key with a
`CgEd` descriptor carrying more settings; it imported as
Brightness/Contrast with only Contrast. A `brit` whose `CgEd` has keys
outside the Brightness/Contrast set now stays Adjustment::Unsupported, is
written back byte for byte, and the import warns naming the layer.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Branch Sync <branch-sync@localhost>
Co-authored-by: Brandon Thomas <echelon@gmail.com>
* Crop: Delete Cropped Pixels keeps type and smart objects whole (#7116)
crop_doc cropped every surface for_each_surface visits, including the
rendered caches of type layers and smart objects. Those move by shifting
their stored pixels, so the cut stayed and showed as soon as they were
moved. Like Photoshop, Delete Cropped Pixels now deletes pixel-layer data
(and fill caches) only; shapes are re-rendered as before.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Address review: Delete Cropped Pixels crops stored video frames again (#4956)
The pixel-layer-only crop path skipped a video layer's stored frames and
original footage, so scrubbing brought the cropped pixels back. Crop them
with the displayed content, as before, and test a shrinking crop.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* Address CodeRabbit review: the turned crop keeps type and smart objects whole before rotating
The pass that drops what a turned crop deletes before the rotation still cut
every cached surface. The rotation re-renders type and smart objects, but a
smart object whose source is unavailable keeps its cache, so it stayed clipped.
That pass now uses the same pixel-layer-only rule as the crop itself.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Brandon Thomas <echelon@gmail.com>
* fix(format): report a failed directory flush after a save instead of ignoring it
atomic_write and the bundle writer discarded the directory fsync result once the rename had published the file, so a save whose rename might not survive a power loss reported success and its recovery data could be discarded. A real flush failure is now returned (the file is in place, the message says the save may not last), so callers keep the document unsaved and its recovery data, and a bundle save stops before removing objects the old manifest needs. A file system that can't flush directories at all (EINVAL, ENOTSUP, Unsupported) still counts as saved. Fixes#4968.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(format): a folder that can't be opened for a flush is not a failed save
A write-and-search-only folder (no read permission) can't be opened to fsync it, so no flush can be attempted there. Treating that open failure as a failed flush made saves into such folders fail (the directory_symlinks permission tests on Linux). Only a flush that is attempted and fails is reported now.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(format): flush bundle folders like atomic_write, and test a failed manifest flush
The bundle writer flushed the folder with sync_all, which on Apple
platforms is F_FULLFSYNC and fails on SMB shares; it now goes through the
same RealFs::sync_dir as atomic_write (sync_file's plain-fsync fallback,
#1336). open_dir_for_sync and dir_synced are crate-private. A test-only
hook fails the manifest's folder flush: the save fails, and the objects
the previous manifest references stay until the next save.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(format): keep stale previews until the manifest commits; errno 45 only where it is ENOTSUP
A directory save removed a thumbnail or composite the new document no
longer has before writing the new manifest, so a failed manifest flush
left the previous manifest naming a preview that was gone. They are now
removed after the manifest commits, best-effort like unreferenced
objects; the failed-flush test checks the thumbnail survives.
errno 45 counted as "can't flush directories" on every Unix, but it is
ENOTSUP only on Apple platforms and the BSDs (illumos: EDEADLK). Other
Unix targets now accept only EINVAL and ErrorKind::Unsupported.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(format): flush the object folders before every manifest commit
When an object's folder flush failed, a retried save found the object
already written, reused it without flushing its folder, and then
committed a manifest naming it: success was reported while the object's
directory entry was still unconfirmed. Each directory save now flushes
tiles/ and blobs/ once before writing the manifest, so a reused object is
confirmed too. The flush error is shared with write_bundle_file
(confirm_flushed), and a test retries a one-tile save whose tile folder
flush failed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(format): keep stale-preview removal before the manifest commit, as main requires
f77ffa3e moved stale-preview removal after the manifest commit. That
broke the hardening in 2d5a7497 (#4957), whose test
denied_stale_preview_cleanup_preserves_the_previous_manifest requires a
committed manifest never to sit beside a stale preview, so a failed
removal must fail the save. Restore main's ordering; the object-folder
flush and the failed-flush tests stay.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(format): flush the composite folder after removing a stale composite
The stale composite/preview.png was removed before the manifest commit,
but nothing flushed composite/, so a crash could bring it back beside
the new manifest. The folder is now flushed when a stale composite was
removed (the thumbnail lives in the bundle root, flushed with the
manifest).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Brandon Thomas <echelon@gmail.com>
* Drop live distortion tools when switching or closing documents.
Puppet Warp, Liquify, and the other modal distort tools kept session UI on a global prompt, so undo, delete, and pixels leaked across tabs. Cancel the live tool with the document that owns it.
* Fix Liquify isolation test: new documents have nothing to undo.
file.new leaves an empty history, so the second document needs a layer step before edit.undo can prove Ctrl+Z is not swallowed.
* Stop expecting Puppet Warp's preview to follow a new document.
display_doc is keyed to the session's document (#5115), so a File › New
tab no longer shows the old warp. drop_foreign still closes the session.
---------
Co-authored-by: Brandon Thomas <echelon@gmail.com>
* PSD smart filters: Sharpen, Dust & Scratches and Hue/Saturation stay editable (#6267)
They came in as psd.unsupportedFilter and were dropped when the smart
object re-rendered (Replace Contents, Relink). They now map to
filter.sharpen.sharpen (no Fltr, keyed by filterID), filter.noise.
dustAndScratches (Rds/Thsh) and image.adjustments.hueSaturation (Clrz,
Hst2 entries with LclR ranges), and are written back the same way.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* rustfmt; merge main (CI Format step)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Apply suggestion from @coderabbitai[bot]
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* PSD smart filters: keep every Hue/Saturation range on import and export
- A repeated `LclR` range (or a second master entry) in `Adjs` would
overwrite the first on import; such a filter now stays verbatim as an
unsupported filter instead.
- Export rejects a colour range given as a non-object (`"reds": 5`),
as the engine's Hue/Saturation params do, instead of dropping it.
- Colorize's 25% saturation default applies to the master entry only;
a colour range's saturation defaults to 0, as in the engine.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Remove DstS and Shrp filter handling from smart_map
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Brandon Thomas <echelon@gmail.com>
* Paste: keep the pasted image on the canvas (#6759)
Paste centred the clipboard image on the view centre with no clamp, so an
image that fit the canvas could land partly off it, and one larger than
the canvas could leave blank edges. Now, per axis, an image that fits stays
wholly inside the canvas, as close to the view centre as it can, and one
larger than the canvas starts at the canvas's top-left edge, as in
Photoshop. Paste Into / Outside still centre on the selection
("clampToCanvas": false). Centres are computed edge by edge in f64, so
extreme coordinates no longer overflow.
* Paste: doc comments for Paste Into and the paste placement tests
---------
Co-authored-by: Brandon Thomas <echelon@gmail.com>
* Auto-Align: a locked layer becomes the reference instead of being moved (#7080)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Address review: Auto-Align moves the Background as a normal layer
With the Background selected and a Cylindrical or Spherical result (every
layer is reprojected), or another layer as the reference, the Background's
own position lock made Auto-Align refuse. Like Free Transform and Photoshop,
the Background's built-in position lock no longer pins it: it becomes a
normal layer ("Layer 0") when it has to move. Lock All, Lock Pixels and a
user's Lock Position on any other layer still keep it as the reference.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* Address CodeRabbit review: the Auto-Align params doc names the locked layer as the default reference
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Brandon Thomas <echelon@gmail.com>
* Color pickers: pick on press and follow small drags (#7793)
The Color Picker dialog's field and slider, and the Color panel's
field and hue strip, only applied the colour on dragged()/clicked().
egui holds dragged() back until the pointer moves past its click
threshold (~6 pt) or is held a while, so a press picked nothing at
first and small adjustments jumped.
Add widgets::pick_pos, which reads the pointer from the frame the
button goes down (is_pointer_button_down_on), and use it on all four
surfaces. The Color panel now restyles selected type only when the
picked colour changes.
Tests: color_picker_ui::field_and_slider_pick_on_press_and_follow_small_moves,
panels::color_tests::color_panel_field_and_hue_strip_pick_on_press.
* #1 Reverted accidental scoreboard change when regenerating it
#2 Fixed color picker not checking for pointer press, but any mouse click
* Update crates/ui-egui/src/widgets.rs
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* Color pickers: rustfmt pick_pos
* test(ui): use main's rgb_to_hsb in the colour picker tests
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Brandon Thomas <echelon@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* Keep shape fill and stroke alpha through PSD round-trip (#6180).
Photoshop colour descriptors have no alpha, so fill opacity was dropped
on save. Write a PhotoCraft-only `pcAl` key for fill colour, and fold
stroke colour alpha into `strokeStyleOpacity` so Appearance still matches.
* Write pcAl only on solid shape fills, not every colour (#6180).
Gradient stops already store alpha in Trns. Putting pcAl on color_to_desc
made Ramp sample that alpha twice. Generic colour descriptors stay
opaque; fill_to_desc adds pcAl only for SoCo.
* Return color_to_desc's match directly so clippy stays clean (#6180).
The leftover let-and-return after dropping the pcAl branch failed
clippy -D warnings on Windows integration.
* Restore rectangle fill and stroke appearance alphas after PSD save (#6180).
The ticket is Appearance after load, then a redraw. Fill already used pcAl.
Stroke folded colour alpha into strokeStyleOpacity and dropped the colour
alpha, so Appearance still showed a maxed stroke colour. Write pcAl on the
original stroke colour, keep the folded opacity for Photoshop, and unfold
on read. Cover it with a rectangle document round-trip and redraw.
* fix(io): ignore a non-finite pcAl alpha from a file
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Brandon Thomas <echelon@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* Perspective Crop: the swap button swaps W and H (#7059)
The W/H swap button called crop_size::swap, which only acts in the Crop tool's W x H x Resolution mode; Perspective Crop never sets that mode, so the button did nothing there. The button now always swaps the fields; X keeps the guard.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Address review: the test clicks Perspective Crop's ⇄ button
The test now renders the real options bar with the Perspective Crop tool,
clicks ⇄ through the egui_kittest harness and checks that W and H swapped,
instead of calling `swap_fields` directly. It fails if the button goes back
to calling `swap`.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Brandon Thomas <echelon@gmail.com>
* Curves: the graph grows with the Properties panel and a resizable dialog (#8984)
The graph was fixed at 300 points. It now takes the width it is given,
no taller than the window leaves room for beside the other rows. The
Image > Adjustments > Curves dialog gets a resize grip and reopens at
the size it was left at.
* Curves: the dialog stops widening where the graph stops; a squeezed size isn't remembered (#8984)
The dialog's resize grip goes no wider than the graph can grow in this
window, so no blank column. A size at the window's limit no longer
replaces the size the user left the dialog at.
* test(ui): keep the Issue #43 doc comment on its own test
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Brandon Thomas <echelon@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
`style.presets.new` with `includeBlending: false` and an explicit `opacity` stores only that
opacity, but `apply` with `add: true` skipped the blending block unless the preset had a blend
mode, so the layer kept its opacity. Test: an_opacity_only_style_added_sets_just_the_opacity
(also checks the layer's blend mode and Fill stay).
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Two PRs that landed together each added [people.ronibalthazar], and a
duplicate table makes the file invalid TOML (ui-egui's build script
warns that it can't parse it, so the About window's names are lost).
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
The .atn importer kept only bicubic, bilinear and nearest from a
transform's Intr, so a recorded Bicubic Automatic, Sharper or Smoother
Transform Selection replayed with the command's default (bilinear) and
re-exported without its choice. The bicubic variants are now kept (the
transforms already render them as Bicubic), Automatic becomes Bicubic
Automatic and Preserve Details Bicubic, and both transform commands list
the variants (#8697).
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Brandon Thomas <echelon@gmail.com>
