diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md
new file mode 100644
index 000000000..9e9702005
--- /dev/null
+++ b/.github/PULL_REQUEST_TEMPLATE.md
@@ -0,0 +1,69 @@
+
+
+Fixes #
+
+
+
+## Problem
+
+
+
+## Change
+
+
+
+**Not in this PR:**
+
+## How it was checked
+
+- [ ] `cargo xtask ci` passes on this branch, rebased on current `main`
+- [ ] A test that fails without this change (for a crash fix: a regression test that panicked before it)
+- [ ] User-visible change: ran the app and looked at the result
+
+
+
+## Screenshots
+
+
+
+## Merging
+
+
+
+- **Overlap:**
+- **Existing edits and libraries:**
+- **Shared numbers taken:**
+- **Where it came from:**
+- **New dependencies, `unsafe`, build scripts, network access, spawned processes, deleted files, CI or `xtask` gate
+ changes:**
+
+## Checklist
+
+- [ ] No new panic path in non-test code: no `unwrap` / `expect` / `panic!`, no indexing or unchecked arithmetic on a
+ number from a file, a person or an agent (`get`, `checked_mul`, NaN and char boundaries handled)
+- [ ] New text on screen goes through `tr()` and has its key in `crates/ui-egui/locales/*.json`; a new shortcut is
+ free in `docs/library-shortcuts.md`
+- [ ] The feature's rows in `docs/parity-checklist.md` and its entry in `docs/gaps.md` are updated, honestly (🟡 when
+ it is partial), in the same commit
+- [ ] New code has a caller: no command, module or setting that nothing reaches yet
+- [ ] Every new image, icon or other asset has its entry in `assets/ATTRIBUTION.md`; no font or media files added
+- [ ] Ready to merge (otherwise open it as a draft)
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
new file mode 100644
index 000000000..d2fef73a6
--- /dev/null
+++ b/.github/workflows/ci.yml
@@ -0,0 +1,32 @@
+name: CI
+
+# Quality gates for every push to main and every pull request (`cargo xtask ci`), through the
+# suite's shared gate workflow in craft-common-actions.
+on:
+ push:
+ branches: [main]
+ pull_request:
+ workflow_dispatch:
+
+permissions:
+ contents: read
+
+# A newer push to a PR branch cancels its older run; every push to main runs to the end.
+concurrency:
+ group: ci-${{ github.event_name == 'pull_request' && github.ref || github.run_id }}
+ cancel-in-progress: ${{ github.event_name == 'pull_request' }}
+
+jobs:
+ gate:
+ # Tracks @main so suite-wide CI fixes reach every app without a PR in each repository.
+ uses: storytold/craft-common-actions/.github/workflows/gate.yml@main
+ with:
+ # Same craft-fonts commit as release.yml.
+ craft-fonts-ref: a71c20e30af613287c504f7e685ca8fad77752bc
+ timeout-minutes: 90
+ # xtask ci runs cargo test: the demo scene cache is per process, so nextest is slower here.
+ tools: ""
+ packages: >-
+ pkg-config libxkbcommon-dev libwayland-dev libx11-dev libxcb1-dev libxcursor-dev
+ libxrandr-dev libxi-dev libfontconfig1-dev libfreetype-dev mesa-vulkan-drivers
+ libgl1-mesa-dev libdbus-1-dev
diff --git a/.github/workflows/freebsd.yml b/.github/workflows/freebsd.yml
index d85332b23..19470cf69 100644
--- a/.github/workflows/freebsd.yml
+++ b/.github/workflows/freebsd.yml
@@ -1,23 +1,18 @@
# FreeBSD build and package check in a QEMU VM.
name: FreeBSD
+# The VM build is slow, so it runs nightly on main and on pushes to `release`, not on every pull
+# request: a drifted dependency still shows up within a day, and always before a release ships.
+# PRs that change the FreeBSD workflow or packaging itself still run it.
on:
push:
- branches: [main]
- paths:
- - ".github/workflows/freebsd.yml"
- - "packaging/freebsd/**"
- - "Cargo.toml"
- - "Cargo.lock"
- - "crates/**"
+ branches: [release]
pull_request:
- branches: [main]
paths:
- - ".github/workflows/freebsd.yml"
- - "packaging/freebsd/**"
- - "Cargo.toml"
- - "Cargo.lock"
- - "crates/**"
+ - .github/workflows/freebsd.yml
+ - packaging/freebsd/**
+ schedule:
+ - cron: "7 4 * * *"
workflow_dispatch:
concurrency:
diff --git a/.github/workflows/nix.yml b/.github/workflows/nix.yml
index 01cb9cd66..2fe7cd3ac 100644
--- a/.github/workflows/nix.yml
+++ b/.github/workflows/nix.yml
@@ -18,6 +18,44 @@ concurrency:
cancel-in-progress: true
jobs:
+ git-sources:
+ name: Git dependencies come from storytold
+ runs-on: ubuntu-latest
+ timeout-minutes: 5
+ steps:
+ - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+ - name: Git dependencies come from storytold
+ run: |
+ set -euo pipefail
+ test -f Cargo.lock || { echo "Cargo.lock is missing"; exit 1; }
+ # Source replacement, patches and path overrides in cargo config never show up
+ # in Cargo.lock. Parsed rather than grepped: TOML has many spellings of one key.
+ # Cargo still reads the legacy .cargo/config.
+ python3 - <<'PY'
+ import os, sys, tomllib
+ for path in (".cargo/config", ".cargo/config.toml"):
+ if not os.path.exists(path):
+ continue
+ config = tomllib.load(open(path, "rb"))
+ replaced = [name for name, source in config.get("source", {}).items()
+ if isinstance(source, dict) and "replace-with" in source]
+ if replaced or "patch" in config or "paths" in config:
+ print(f"{path} replaces, patches or overrides a source")
+ sys.exit(1)
+ PY
+ # Fails when Cargo.lock is out of date with the manifests.
+ cargo metadata --locked --format-version 1 > /dev/null
+ python3 - <<'PY'
+ import sys, tomllib
+ lock = tomllib.load(open("Cargo.lock", "rb"))
+ bad = sorted({p["source"] for p in lock.get("package", [])
+ if p.get("source", "").startswith("git+")
+ and not p["source"].startswith("git+https://github.com/storytold/")})
+ if bad:
+ print("Git dependencies outside github.com/storytold:", *bad, sep="\n")
+ sys.exit(1)
+ PY
+
nix:
# Intentionally tracked at @main (not SHA-pinned) so org-wide CI improvements, security fixes, and build optimizations propagate without PR churn across all craftapp repos.
uses: storytold/craft-common-actions/.github/workflows/nix.yml@main
diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
index b8fb01326..7356f66fe 100644
--- a/.github/workflows/release.yml
+++ b/.github/workflows/release.yml
@@ -20,7 +20,47 @@ concurrency:
cancel-in-progress: false
jobs:
+ git-sources:
+ name: Git dependencies come from storytold
+ runs-on: ubuntu-latest
+ permissions:
+ contents: read
+ steps:
+ - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+ - name: Git dependencies come from storytold
+ run: |
+ set -euo pipefail
+ test -f Cargo.lock || { echo "Cargo.lock is missing"; exit 1; }
+ # Source replacement, patches and path overrides in cargo config never show up
+ # in Cargo.lock. Parsed rather than grepped: TOML has many spellings of one key.
+ # Cargo still reads the legacy .cargo/config.
+ python3 - <<'PY'
+ import os, sys, tomllib
+ for path in (".cargo/config", ".cargo/config.toml"):
+ if not os.path.exists(path):
+ continue
+ config = tomllib.load(open(path, "rb"))
+ replaced = [name for name, source in config.get("source", {}).items()
+ if isinstance(source, dict) and "replace-with" in source]
+ if replaced or "patch" in config or "paths" in config:
+ print(f"{path} replaces, patches or overrides a source")
+ sys.exit(1)
+ PY
+ # Fails when Cargo.lock is out of date with the manifests.
+ cargo metadata --locked --format-version 1 > /dev/null
+ python3 - <<'PY'
+ import sys, tomllib
+ lock = tomllib.load(open("Cargo.lock", "rb"))
+ bad = sorted({p["source"] for p in lock.get("package", [])
+ if p.get("source", "").startswith("git+")
+ and not p["source"].startswith("git+https://github.com/storytold/")})
+ if bad:
+ print("Git dependencies outside github.com/storytold:", *bad, sep="\n")
+ sys.exit(1)
+ PY
+
release:
+ needs: git-sources
# Intentionally tracked at @main (not SHA-pinned) so org-wide CI improvements, security fixes, and build optimizations propagate without PR churn across all craftapp repos.
uses: storytold/craft-common-actions/.github/workflows/release.yml@main
with:
diff --git a/.github/workflows/sonarcloud.yml b/.github/workflows/sonarcloud.yml
index ecee45a6d..e78fe1ea3 100644
--- a/.github/workflows/sonarcloud.yml
+++ b/.github/workflows/sonarcloud.yml
@@ -11,7 +11,8 @@ permissions:
concurrency:
group: sonarcloud-${{ github.ref }}
- cancel-in-progress: true
+ # A push to main is the run that publishes coverage; superseded runs there would never finish.
+ cancel-in-progress: ${{ github.ref != 'refs/heads/main' }}
jobs:
sonarcloud:
diff --git a/.github/workflows/windows-arm64.yml b/.github/workflows/windows-arm64.yml
index 0033346dc..332193998 100644
--- a/.github/workflows/windows-arm64.yml
+++ b/.github/workflows/windows-arm64.yml
@@ -5,23 +5,17 @@
# on GitHub Actions' native windows-11-arm runner.
name: Windows ARM64
+# Nightly on main and on pushes to `release` (dependency and runner-image drift shows up within a
+# day, and always before a release ships), and on PRs that change the Windows packaging itself.
on:
+ schedule:
+ - cron: "3 5 * * *"
push:
- branches: [main]
- paths:
- - ".github/workflows/windows-arm64.yml"
- - "packaging/windows/**"
- - "Cargo.toml"
- - "Cargo.lock"
- - "crates/**"
+ branches: [release]
pull_request:
- branches: [main]
paths:
- - ".github/workflows/windows-arm64.yml"
- - "packaging/windows/**"
- - "Cargo.toml"
- - "Cargo.lock"
- - "crates/**"
+ - .github/workflows/windows-arm64.yml
+ - packaging/windows/**
workflow_dispatch:
concurrency:
diff --git a/CLAUDE.md b/CLAUDE.md
index d2d538d65..416ffc0a8 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -68,7 +68,10 @@ of it. Full standard: `../craftrules/standards/never-crash.md`
`Cancel::No` when the job can't stop. No progress window, panel or progress toast of its own; its tests assert the row.
- **Resolution independence:** settings use normalized image coordinates and relative radii; previews and exports must match.
- **Time goes through `chrono`** (workspace dependency). Never hand-roll UTC/calendar conversion (days-to-date algorithms, leap-year math); a crate owns only its output format string. The one exception is range extension: past the years chrono can represent, shift the date by whole 400-year Gregorian cycles (146,097 days, after which the calendar repeats), let chrono compute the month and day, and add the cycles back to the year (splitting seconds into hours, minutes and seconds is plain arithmetic).
-- **Quality gates** before every commit: `cargo xtask ci` (fmt, clippy -D warnings, tests, layers, assets, wasm).
+- **Quality gates** before every commit: `cargo xtask ci --quick` (fmt, clippy -D warnings, the changed packages'
+ tests, layers, assets, and heif / parity / wasm when touched). GitHub runs the full `cargo xtask ci` on every PR
+ (`.github/workflows/ci.yml`); run the full one locally before merging branches (below) or when `--quick` can't see
+ the risk. One `ci` runs at a time per machine; others wait their turn (`xtask/src/ci_lock.rs`).
- **Commits:** one task id per commit (`M2.3: local Laplacian highlights/shadows`). Only green states. End messages with the attribution line required by the environment.
## Assets: icons, images, fonts (ABSOLUTE RULE — never violate)
diff --git a/Cargo.lock b/Cargo.lock
index 1ec8fad29..78de058c3 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -3087,7 +3087,7 @@ dependencies = [
[[package]]
name = "lightcraft"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
"chrono",
"eframe",
@@ -3113,7 +3113,7 @@ dependencies = [
[[package]]
name = "lightcraft-catalog"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
"chrono",
"crc32fast",
@@ -3131,7 +3131,7 @@ dependencies = [
[[package]]
name = "lightcraft-cli"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
"dhat",
"lightcraft-codecs",
@@ -3147,7 +3147,7 @@ dependencies = [
[[package]]
name = "lightcraft-codecs"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
"avif-serialize",
"bytemuck",
@@ -3178,7 +3178,7 @@ dependencies = [
[[package]]
name = "lightcraft-color"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
"lightcraft-geom",
"proptest",
@@ -3187,7 +3187,7 @@ dependencies = [
[[package]]
name = "lightcraft-denoise"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
"faer-core",
"lightcraft-denoise-core",
@@ -3204,7 +3204,7 @@ dependencies = [
[[package]]
name = "lightcraft-denoise-core"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
"lightcraft-raster",
"miniz_oxide 0.8.9",
@@ -3219,7 +3219,7 @@ dependencies = [
[[package]]
name = "lightcraft-develop"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
"lightcraft-color",
"lightcraft-geom",
@@ -3231,7 +3231,7 @@ dependencies = [
[[package]]
name = "lightcraft-engine"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
"ab_glyph",
"chrono",
@@ -3275,7 +3275,7 @@ dependencies = [
[[package]]
name = "lightcraft-faces"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
"lightcraft-testkit",
"matrixmultiply",
@@ -3290,7 +3290,7 @@ dependencies = [
[[package]]
name = "lightcraft-fetch"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
"log",
"rustls",
@@ -3301,7 +3301,7 @@ dependencies = [
[[package]]
name = "lightcraft-geocode"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
"miniz_oxide 0.8.9",
"serde",
@@ -3309,7 +3309,7 @@ dependencies = [
[[package]]
name = "lightcraft-geom"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
"proptest",
"serde",
@@ -3317,7 +3317,7 @@ dependencies = [
[[package]]
name = "lightcraft-gpu"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
"bytemuck",
"lightcraft-color",
@@ -3337,7 +3337,7 @@ dependencies = [
[[package]]
name = "lightcraft-heif"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
"heic-rs",
"rayon",
@@ -3345,23 +3345,23 @@ dependencies = [
[[package]]
name = "lightcraft-inpaint"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
"rayon",
]
[[package]]
name = "lightcraft-lensdb"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
"lightcraft-develop",
"lightcraft-geom",
- "quick-xml 0.38.4",
+ "quick-xml",
]
[[package]]
name = "lightcraft-mcp"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
"lightcraft-codecs",
"lightcraft-engine",
@@ -3372,7 +3372,7 @@ dependencies = [
[[package]]
name = "lightcraft-merge"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
"lightcraft-codecs",
"lightcraft-color",
@@ -3388,21 +3388,22 @@ dependencies = [
[[package]]
name = "lightcraft-meta"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
"chrono",
"lightcraft-geom",
"lightcraft-tiff",
"proptest",
- "quick-xml 0.38.4",
+ "quick-xml",
"serde",
"serde_json",
]
[[package]]
name = "lightcraft-pipeline"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
+ "dhat",
"image",
"lightcraft-color",
"lightcraft-develop",
@@ -3419,7 +3420,7 @@ dependencies = [
[[package]]
name = "lightcraft-preview"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
"jpeg-encoder",
"lightcraft-raster",
@@ -3431,7 +3432,7 @@ dependencies = [
[[package]]
name = "lightcraft-raster"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
"libc",
"lightcraft-color",
@@ -3443,7 +3444,7 @@ dependencies = [
[[package]]
name = "lightcraft-raw"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
"dhat",
"jpeg-encoder",
@@ -3467,7 +3468,7 @@ dependencies = [
[[package]]
name = "lightcraft-scenes"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
"image",
"lightcraft-color",
@@ -3478,7 +3479,7 @@ dependencies = [
[[package]]
name = "lightcraft-segment"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
"candle-core",
"candle-nn",
@@ -3492,7 +3493,7 @@ dependencies = [
[[package]]
name = "lightcraft-slideshow"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
"lightcraft-raster",
"serde",
@@ -3501,15 +3502,15 @@ dependencies = [
[[package]]
name = "lightcraft-sysmem"
-version = "0.12.0"
+version = "0.13.0"
[[package]]
name = "lightcraft-testkit"
-version = "0.12.0"
+version = "0.13.0"
[[package]]
name = "lightcraft-tiff"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
"proptest",
"serde",
@@ -3518,7 +3519,7 @@ dependencies = [
[[package]]
name = "lightcraft-ui-egui"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
"craft-ui",
"egui",
@@ -3547,7 +3548,7 @@ dependencies = [
[[package]]
name = "lightcraft-web"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
"crc32fast",
"eframe",
@@ -4848,15 +4849,6 @@ version = "2.0.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "a993555f31e5a609f617c12db6250dedcac1b0a85076912c436e6fc9b2c8e6a3"
-[[package]]
-name = "quick-xml"
-version = "0.38.4"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b66c2058c55a409d601666cffe35f04333cf1013010882cec174a7467cd4e21c"
-dependencies = [
- "memchr",
-]
-
[[package]]
name = "quick-xml"
version = "0.41.0"
@@ -6347,7 +6339,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "338e30461b3a2b67d70eb30a6d89f8e0c93a833e07d2ae89085cd070c4a00ac0"
dependencies = [
"proc-macro2",
- "quick-xml 0.41.0",
+ "quick-xml",
"quote",
]
@@ -7017,7 +7009,7 @@ checksum = "e450f9b2ed1dff33c94c12589a87338689467b9c4f5d8a5710bd09a847d2c8a7"
[[package]]
name = "xtask"
-version = "0.12.0"
+version = "0.13.0"
dependencies = [
"brotli",
"flate2",
diff --git a/Cargo.toml b/Cargo.toml
index 1ca738315..c22395b8a 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -3,10 +3,10 @@ resolver = "3"
members = ["crates/*", "apps/*", "xtask"]
[workspace.package]
-version = "0.12.0"
+version = "0.13.0"
edition = "2024"
license = "MIT OR Apache-2.0"
-rust-version = "1.90"
+rust-version = "1.95"
repository = "https://github.com/storytold/lightcraft"
[workspace.dependencies]
@@ -60,7 +60,7 @@ libc = "0.2" # dev-only: process CPU time in benches
dhat = "0.3" # opt-in heap profiling (lightcraft-cli --features dhat-heap) and heap tests; never in product builds
# formats
-quick-xml = "0.38"
+quick-xml = "0.41"
miniz_oxide = "0.8"
# codecs
@@ -125,9 +125,11 @@ excessive_precision = "allow"
[profile.dev.package."*"]
opt-level = 2
+debug = false
[profile.dev]
opt-level = 1
+debug = "line-tables-only"
[profile.release]
debug = "line-tables-only"
diff --git a/README.md b/README.md
index c8db5a50a..78b3b3234 100644
--- a/README.md
+++ b/README.md
@@ -405,15 +405,26 @@ cargo run --release -p lightcraft -- --control 7980 # with the automation ch
cargo xtask web --serve # the same app in the browser: http://127.0.0.1:8080/
cargo run --release -p lightcraft-cli -- render photo.jpg -o out.jpg --set light.exposure=0.5
cargo xtask ci # fmt, clippy, HEIF, tests, parity, layers, assets, WASM
+cargo xtask ci --quick # only what your branch's changes can break (minutes)
```
+`cargo xtask ci --quick` compares with the merge base of `origin/main` (`--base REF` for another)
+and runs fmt, clippy, the tests of the changed packages, layers and assets, plus HEIF, parity and
+the wasm check when the changes touch them; a change to `Cargo.toml`, `Cargo.lock` or the lint
+config tests everything. GitHub runs the full `cargo xtask ci` on every pull request
+(`.github/workflows/ci.yml`). Only one `ci` runs at a time per machine: a second one, from another
+worktree say, waits for the first and names it, since several at once take longer in total than in
+turn. `LIGHTCRAFT_CI_SLOTS=2` allows two; `LIGHTCRAFT_CI_LOCK=off` skips the lock.
+
CI defaults to line-table debug information and one build job per 1.5 GB of RAM
available when it starts (at most one per CPU; 4 if memory can't be read), so an
8 GB machine with 6 GB free builds with 4, and a busy machine with little free
-builds with fewer. Test threads follow, capped at 4. Full-debuginfo linkers, one
-per CPU, otherwise exhaust RAM before any test runs. GPU coverage is unchanged. Explicit
-`CARGO_PROFILE_DEV_DEBUG`, `CARGO_BUILD_JOBS` and `RUST_TEST_THREADS` settings
-override these defaults. Ordinary development commands keep their own settings.
+builds with fewer. Test threads follow, capped at 4.
+Full-debuginfo linkers, one per CPU, otherwise exhaust RAM before any test runs. GPU coverage is
+unchanged. Explicit `CARGO_PROFILE_DEV_DEBUG`, `CARGO_BUILD_JOBS`, `RUST_TEST_THREADS` and
+`NEXTEST_TEST_THREADS` settings override these defaults. The tests run under
+`cargo test`; `LIGHTCRAFT_NEXTEST=1` runs them under [cargo-nextest](https://nexte.st) instead (one
+process per test, slower here because each regenerates the demo scenes). Ordinary development commands keep their own settings.
**Release builds on memory-constrained machines:** [#1196](https://github.com/storytold/lightcraft/issues/1196)
reports Arch/AUR release-link OOM kills on a 16 GB machine when the desktop and CLI build concurrently
diff --git a/README.zh-hans.md b/README.zh-hans.md
index 824913f14..2800db410 100644
--- a/README.zh-hans.md
+++ b/README.zh-hans.md
@@ -298,10 +298,10 @@ lightcraft --control 7980 ~/Pictures/trip
## Feature status
-LightCraft 很年轻,而且进展很快。**我们如实所处的位置**(细节见[路线图](ROADMAP.md#where-we-stand)):
+LightCraft 很年轻,而且进展很快。**我们如实所处的位置**(细节见[路线图](ROADMAP.md)):
- **按功能数量算,我们达到了约 79%**(核心功能 98%),逐行记录在
- [docs/parity.md](docs/parity.md)。
+ [docs/parity-checklist.md](docs/parity-checklist.md)。
- **作为日常使用的工具,我们更接近 60–70%。** 在单台机器上处理 JPEG/DNG 以及大多数尼康/索尼/
较老佳能的 raw 都很出色。
- **最大的缺口:**
@@ -309,7 +309,7 @@ LightCraft 很年轻,而且进展很快。**我们如实所处的位置**(
- **压缩的奥林巴斯 raw 和不受支持的 CR3 变体:** 这些在有内嵌 JPEG 预览时使用预览。富士无损/有损压缩 RAF 现在能解码传感器数据;[验证与已有图库重新加载说明](docs/raf-compression.md);
- **AI 蒙版与降噪:** 主体和天空选择目前是经典启发式算法;
- **屏幕上的 HDR 显示、视频,以及经典版式中的打印/画册/地图模块。**
-- **接下来做什么:** 见[我们要去往哪里](ROADMAP.md#where-were-going)。
+- **接下来做什么:** 见[我们要去往哪里](ROADMAP.md#upcoming)。
| 领域 | 状态 |
|---|---|
@@ -333,7 +333,7 @@ LightCraft 很年轻,而且进展很快。**我们如实所处的位置**(
| 镜头校正(畸变、暗角、自动 + 手动色差、去边、DNG 文件内嵌的镜头校正以及松下/徕卡 RW2/RWL 畸变数据)、几何(变换、限制裁剪)、垂直校正(自动/水平/垂直/完整/引导式) | ✅ · 相机镜头配置文件(我们自己的)⬜ |
| 照片合并:HDR(自动对齐、去重影)、全景(球面/圆柱/透视、边界变形、自动裁剪)、HDR 全景 → DNG | ✅ |
| GPU 管线(wgpu 计算,与 CPU 一致到 1/255 以内)、设备限制/出错时回退到 CPU | ✅ · 浏览器中的 WebGPU 🚧 |
-| AI:分割蒙版、AI 降噪、超分辨率、人脸;视频 | ⬜(见[路线图](ROADMAP.md#where-were-going)) |
+| AI:分割蒙版、AI 降噪、超分辨率、人脸;视频 | ⬜(见[路线图](ROADMAP.md#upcoming)) |
| 网页版(通过 WASM 在浏览器中使用同一界面):图库持久化在 OPFS/IndexedDB、Web Worker 渲染、导出下载 | ✅ · WebGPU、Safari/Firefox 测试 🚧 |
✅ 现已可用 · 🚧 进行中 · ⬜ 尚未开始
@@ -357,7 +357,7 @@ CI 默认使用行表调试信息,并按启动时可用内存每 1.5 GB 开一
(每个 CPU 最多一个;读不到内存时为 4 个),所以一台 8 GB、空闲 6 GB 的机器会用 4 个任务构建,
而一台空闲内存很少的繁忙机器会用更少。测试线程数随之调整,上限为 4。否则,每个 CPU 一个的
完整调试信息链接器会在任何测试运行之前就耗尽内存。GPU 覆盖范围不变。显式设置
-`CARGO_PROFILE_DEV_DEBUG`、`CARGO_BUILD_JOBS` 和 `RUST_TEST_THREADS` 会覆盖这些默认值。
+`CARGO_PROFILE_DEV_DEBUG`、`CARGO_BUILD_JOBS`、`RUST_TEST_THREADS` 和 `NEXTEST_TEST_THREADS` 会覆盖这些默认值。
日常开发命令保持它们各自的设置。
**内存受限机器上的 release 构建:** [#1196](https://github.com/storytold/lightcraft/issues/1196)
diff --git a/README.zh-hant.md b/README.zh-hant.md
index a17505f3f..12fcfb11b 100644
--- a/README.zh-hant.md
+++ b/README.zh-hant.md
@@ -375,7 +375,7 @@ CI 預設使用行表除錯資訊,並依啟動時可用記憶體每 1.5 GB 開
(每個 CPU 最多一個;讀不到記憶體時為 4 個),所以一台 8 GB、剩餘 6 GB 可用的機器會用 4 個任務建置,
而一台忙碌、剩餘記憶體很少的機器會用更少。測試執行緒數隨之調整,上限為 4。若不限制,每個 CPU 各跑
一個帶完整除錯資訊的連結器,測試還沒開始記憶體就會耗盡。GPU 測試的涵蓋範圍不受影響。明確設定
-`CARGO_PROFILE_DEV_DEBUG`、`CARGO_BUILD_JOBS` 與 `RUST_TEST_THREADS` 會覆寫這些預設值。
+`CARGO_PROFILE_DEV_DEBUG`、`CARGO_BUILD_JOBS`、`RUST_TEST_THREADS` 與 `NEXTEST_TEST_THREADS` 會覆寫這些預設值。
日常開發指令保留各自的設定。
**記憶體受限機器上的 release 建置:** [#1196](https://github.com/storytold/lightcraft/issues/1196)
diff --git a/ROADMAP.md b/ROADMAP.md
index ec392e88d..6802f9234 100644
--- a/ROADMAP.md
+++ b/ROADMAP.md
@@ -2,7 +2,7 @@
**Stage: alpha** · next: beta, ~14 points and ~340–600 h away
-> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-16 · **Change:** minor (Minolta MRW decoded, 6 bodies verified; RAW: 83% of Adobe's models decode, 38% verified after the camera-coverage sweep #1327; German catalog 91% → 97%: interface strings and What's New lines translated) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6
+> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-22 · **Change:** minor (Print: Japanese photo and postcard paper sizes — L判, 2L判, KG判, はがき, 往復はがき, 六切, 四切, 名刺; Minolta MRW decoded, 6 bodies verified; RAW: 83% of Adobe's models decode, 38% verified after the camera-coverage sweep #1327; German catalog 91% → 97%: interface strings and What's New lines translated) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6
One-page summary of how close LightCraft is to Adobe Lightroom. Agents: read this, then pick work
from [docs/gaps.md](docs/gaps.md). The full assessment, method and evidence are in
@@ -114,6 +114,11 @@ Ranked toward beta; detail in [docs/roadmap.md](docs/roadmap.md), gaps in [docs/
## Progress log
+- 2026-10-22: Print page setup and PDF output gain the Japanese photo and postcard papers — L判
+ (89 × 127 mm), 2L判 (127 × 178), KG判 (102 × 152), はがき (100 × 148), 往復はがき (148 × 200),
+ 六切 (203 × 254), 四切 (254 × 305) and 名刺 (55 × 91) — at their exact metric sizes. The Print dialog
+ and `print.set` take their customary names, and names are matched case-, space- and
+ punctuation-insensitively (`L判`, `l`, `L-size` are one paper).
- 2026-10-14: Camera-coverage sweep verifies 30 more models on 40 CC0 files, with file and sensor
hashes pinned (`docs/raw/coverage.py`: 515 → 545 verified). Eleven candidates excluded after rechecking
identity, provenance and camera-image agreement. Phone and drone DNG tone differences remain tracked in #1277.
@@ -121,7 +126,7 @@ Ranked toward beta; detail in [docs/roadmap.md](docs/roadmap.md), gaps in [docs/
RGGB and GBRG, as-shot WB from `WBG`); DiMAGE 7, 7Hi, A1, A2, A200 and Dynax / Maxxum 7D verified on
CC0 files. 551 models verified, 201 preview-only; headline numbers unchanged.
- 2026-10-14: 97 Canon CR2, Pentax and Ricoh models verified, one CC0 raw.pixls.us file each pinned and
- checked against the camera JPEG (`crates/raw/tests/canon_pentax_corpus.rs`). 334 models verified, ~50% of
+ checked against the camera JPEG (`crates/raw/tests/it/canon_pentax_corpus.rs`). 334 models verified, ~50% of
photographers on a verified body; headline numbers unchanged.
- 2026-10-12: Mamiya MEF (ZD) decodes from sensor data and is imported; verified on a CC0 file. 241 models
verified, 35 unsupported; headline numbers unchanged.
@@ -135,7 +140,7 @@ Ranked toward beta; detail in [docs/roadmap.md](docs/roadmap.md), gaps in [docs/
decode. The EOS R6 Mark III gets its as-shot white balance (`ColorData` 66). 259 models verified; RAW
decoding 77% → 83%; mainstream ~49% → ~51%, essentials ~64% → ~65%, full ~61%.
- 2026-10-12: 117 Nikon and Fujifilm bodies verified, one CC0 raw.pixls.us file each pinned and checked
- against the camera JPEG (`crates/raw/tests/body_corpus.rs`); the sweep found COOLPIX 5700 / 8400 / 8800
+ against the camera JPEG (`crates/raw/tests/it/body_corpus.rs`); the sweep found COOLPIX 5700 / 8400 / 8800
decoding wrongly. 354 models verified, ~58% of photographers on a verified body; headline numbers unchanged.
- 2026-10-11: Canon EOS R5 Mark II C-RAW decodes from sensor data (57% of its files opened as a
preview): the CRX QP map's first-symbol literal and limited-length escape; four CC0 R5 Mark II files
@@ -186,6 +191,7 @@ Ranked toward beta; detail in [docs/roadmap.md](docs/roadmap.md), gaps in [docs/
| Date | Change | Summary |
|---|---|---|
+| 2026-10-22 | minor | Print: Japanese photo and postcard paper sizes (L判, 2L判, KG判, はがき, 往復はがき, 六切, 四切, 名刺) in the page setup, Print dialog and PDF output; headline numbers unchanged |
| 2026-10-19 | minor | Indonesian interface (`id`): every plain and formats message, plus 69 command labels, locale, menu command and tests; What's New left in English; Localization ~55% → ~64% (mean of the 12 key languages), 8 of 12 present |
| 2026-10-12 | minor | Italian interface (`it`, #1192): 2,201 of 2,398 messages, locale, menu command and tests; headline numbers unchanged |
| 2026-10-13 | minor | Sony ILCA-99M2 camera colour profile in the log; numbers unchanged |
diff --git a/apps/lightcraft-cli/src/main.rs b/apps/lightcraft-cli/src/main.rs
index c8371cbb9..a27e97beb 100644
--- a/apps/lightcraft-cli/src/main.rs
+++ b/apps/lightcraft-cli/src/main.rs
@@ -935,7 +935,11 @@ fn snapshot(args: &[String]) -> Result<(), String> {
fn commands(args: &[String]) -> Result<(), String> {
let mut h = Headless::demo();
- let cmds = h.call("engine.commands", json!({}))?;
+ let mut cmds = h.call("engine.commands", json!({}))?;
+ // Listed by id so each family (`photo.*`, `mask.*`, …) reads together; the registry keeps its own order.
+ if let Some(list) = cmds.as_array_mut() {
+ list.sort_by(|a, b| a["id"].as_str().unwrap_or("").cmp(b["id"].as_str().unwrap_or("")));
+ }
let mut out = std::io::stdout().lock();
if args.iter().any(|a| a == "--json") {
writeln!(out, "{}", serde_json::to_string_pretty(&cmds).unwrap_or_default()).map_err(|e| e.to_string())?;
diff --git a/apps/lightcraft-cli/tests/cli.rs b/apps/lightcraft-cli/tests/it/cli.rs
similarity index 95%
rename from apps/lightcraft-cli/tests/cli.rs
rename to apps/lightcraft-cli/tests/it/cli.rs
index 8f56bafe6..1c8910d9d 100644
--- a/apps/lightcraft-cli/tests/cli.rs
+++ b/apps/lightcraft-cli/tests/it/cli.rs
@@ -286,6 +286,38 @@ fn commands_subcommand_lists_registry() {
assert!(String::from_utf8_lossy(&o.stdout).contains("light.exposure"));
}
+#[test]
+fn commands_subcommand_lists_commands_sorted_by_id() {
+ let o = Command::new(BIN).args(["commands", "--json"]).output().unwrap();
+ assert!(o.status.success());
+ let v: Value = serde_json::from_slice(&o.stdout).unwrap();
+ let cmds = v.as_array().unwrap();
+ for c in cmds {
+ for field in ["id", "label", "menu", "shortcut", "params", "enabled"] {
+ assert!(c.get(field).is_some(), "every command should keep its {field}: {c}");
+ }
+ }
+ let ids: Vec<&str> = cmds.iter().filter_map(|c| c["id"].as_str()).collect();
+ assert_eq!(ids.len(), cmds.len(), "every command should have a string id");
+ let unsorted = ids.windows(2).find(|w| w[0] >= w[1]);
+ assert!(unsorted.is_none(), "--json should list commands sorted by id, but {unsorted:?} are out of order");
+
+ let o = Command::new(BIN).arg("commands").output().unwrap();
+ assert!(o.status.success());
+ let text = String::from_utf8_lossy(&o.stdout);
+ let lines: Vec<&str> = text.lines().collect();
+ assert_eq!(lines.len(), 2 * ids.len(), "each command should take an id line and a params line");
+ let listed: Vec<&str> = lines.iter().step_by(2).filter_map(|l| l.split_whitespace().next()).collect();
+ assert_eq!(listed, ids, "the text output should list the same commands in the same order as --json");
+ assert!(lines.iter().skip(1).step_by(2).all(|l| l.trim_start().starts_with("params:")), "each id line should be followed by its params line");
+ for (line, c) in lines.iter().step_by(2).zip(cmds) {
+ assert!(line.contains(c["label"].as_str().unwrap()), "{line:?} should show the label of {c}");
+ if let Some(sc) = c["shortcut"].as_str() {
+ assert!(line.contains(&format!("[{sc}]")), "{line:?} should show the shortcut of {c}");
+ }
+ }
+}
+
#[test]
fn snapshot_subcommand_renders_the_ui_headlessly() {
let script = tmp("snap.jsonl");
diff --git a/apps/lightcraft-cli/tests/it/main.rs b/apps/lightcraft-cli/tests/it/main.rs
new file mode 100644
index 000000000..0c07c6438
--- /dev/null
+++ b/apps/lightcraft-cli/tests/it/main.rs
@@ -0,0 +1,4 @@
+//! The integration tests, one module per file, linked into one test binary.
+
+mod cli;
+mod mcp_shutdown;
diff --git a/apps/lightcraft-cli/tests/mcp_shutdown.rs b/apps/lightcraft-cli/tests/it/mcp_shutdown.rs
similarity index 100%
rename from apps/lightcraft-cli/tests/mcp_shutdown.rs
rename to apps/lightcraft-cli/tests/it/mcp_shutdown.rs
diff --git a/apps/lightcraft-web/src/automation.rs b/apps/lightcraft-web/src/automation.rs
new file mode 100644
index 000000000..ff11e0ee9
--- /dev/null
+++ b/apps/lightcraft-web/src/automation.rs
@@ -0,0 +1,48 @@
+//! The page's automation entry point, `lightcraft.command(id, params)` (see `web.rs`): what the
+//! params text means. Plain Rust, so it is tested natively.
+
+use serde_json::Value;
+
+/// The command parameters in `text`: a JSON object, or `{}` when the text is empty or blank (or
+/// `null`). Anything else is an error, never `{}`: a command run without its params acts on the
+/// selection or with defaults, so a typo such as `{ids: [3]}` must not delete or reset the
+/// selected photos. The control channel and MCP refuse such params the same way.
+pub fn params(text: &str) -> Result {
+ let text = text.trim();
+ if text.is_empty() {
+ return Ok(Value::Object(Default::default()));
+ }
+ match serde_json::from_str::(text) {
+ Ok(Value::Null) => Ok(Value::Object(Default::default())),
+ Ok(v @ Value::Object(_)) => Ok(v),
+ Ok(_) => Err("params must be a JSON object, e.g. {\"ids\": [3]}".into()),
+ Err(e) => Err(format!("params are not valid JSON ({e}); nothing was run")),
+ }
+}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use serde_json::json;
+
+ #[test]
+ fn blank_or_null_params_are_an_empty_object() {
+ for text in ["", " ", "\n", "null", " null "] {
+ assert_eq!(params(text), Ok(json!({})), "{text:?}");
+ }
+ }
+
+ #[test]
+ fn an_object_is_passed_through() {
+ assert_eq!(params(r#"{"ids": [3], "rating": 5}"#), Ok(json!({"ids": [3], "rating": 5})));
+ }
+
+ /// Before, any of these ran the command with `{}`: `photo.delete` then removed the selection.
+ #[test]
+ fn invalid_json_or_a_non_object_is_refused() {
+ for text in ["{ids: [3]}", "{\"ids\": [3]", "ids=3", "[3]", "3", "\"{}\"", "true"] {
+ assert!(params(text).is_err(), "{text:?} must not run the command");
+ }
+ assert!(params("{ids: [3]}").unwrap_err().contains("nothing was run"));
+ }
+}
diff --git a/apps/lightcraft-web/src/lib.rs b/apps/lightcraft-web/src/lib.rs
index 58966e6e8..f984cb34c 100644
--- a/apps/lightcraft-web/src/lib.rs
+++ b/apps/lightcraft-web/src/lib.rs
@@ -16,6 +16,7 @@
#![forbid(unsafe_code)]
#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]
+pub mod automation;
pub mod backup;
pub mod bench;
pub mod files;
diff --git a/apps/lightcraft-web/src/web.rs b/apps/lightcraft-web/src/web.rs
index 14a5d050b..827b367f8 100644
--- a/apps/lightcraft-web/src/web.rs
+++ b/apps/lightcraft-web/src/web.rs
@@ -23,7 +23,11 @@ use crate::store::{Originals, content_hash, download_name, storage_key};
use crate::wire::ThumbIndex;
use crate::workers::{THUMB_INDEX, Workers};
-const ACCEPT: &str = ".jpg,.jpeg,.png,.tif,.tiff,.webp,.dng,.cr2,.nef,.arw,.psd,.jxl,.gif,.bmp";
+/// The web picker's accept list, derived from the engine's import extensions (both
+/// cases) so no format the engine can import is silently filtered out of the dialog.
+fn accept_list() -> String {
+ lightcraft_engine::import::accept_list()
+}
/// After a failed save, wait this long before trying again (ms).
const SAVE_RETRY_MS: f64 = 2000.0;
@@ -123,7 +127,7 @@ fn open_picker(originals: Originals, backend: Option, ctx: egui::Contex
let Ok(input) = doc.create_element("input").map(|e| e.unchecked_into::()) else { return };
input.set_type("file");
input.set_multiple(true);
- input.set_accept(ACCEPT);
+ input.set_accept(&accept_list());
let inp = input.clone();
let on_change = Closure::::new(move || {
if let Some(files) = inp.files() {
@@ -273,6 +277,8 @@ async fn flush(files: Files, backend: Backend, dir: String, flushing: Rc,
first_frame_logged: bool,
+ /// The page title last set (`document.title`), so it is written only when it changes.
+ title: String,
last_save: f64,
ui_written: Vec,
}
@@ -501,6 +509,7 @@ impl WebApp {
lib_dir,
bench: opts.bench.then(|| Bench::new(origin)),
first_frame_logged: false,
+ title: String::new(),
last_save: 0.0,
ui_written,
}
@@ -508,22 +517,37 @@ impl WebApp {
fn import_dropped(&mut self, ctx: &egui::Context) {
let dropped = ctx.input(|i| i.raw.dropped_files.clone());
+ // a dialog is on screen (the card said so while the files hovered): nothing is read or imported
+ // (files read from an earlier drop still finish below)
+ if !dropped.is_empty() && self.app.ui.dialog.is_some() {
+ self.app.toast(ctx, lightcraft_ui_egui::i18n::tr("Close the open dialog, then drop the files again"));
+ return;
+ }
for f in dropped {
let (originals, backend, ctx) = (self.originals.clone(), self.backend.clone(), ctx.clone());
wasm_bindgen_futures::spawn_local(async move {
let name = f.path().to_string_lossy().to_string();
match f.bytes_async().await {
Ok(bytes) => store_file(originals, backend, name, bytes, ctx).await,
- Err(e) => log::warn!("reading dropped {name}: {e}"),
+ Err(e) => notice(lightcraft_ui_egui::i18n::dropped_file_unreadable(&name, &e.to_string())),
}
});
}
let paths = self.originals.take_pending();
if !paths.is_empty() {
- let n = paths.len();
match self.app.run("library.import", json!({"paths": paths})) {
- Ok(_) => self.app.toast(ctx, format!("Added {n} photo{}", if n == 1 { "" } else { "s" })),
- Err(e) => log::warn!("import: {e}"),
+ // what the import did, not what was submitted: duplicates and unsupported files add nothing,
+ // a drop that only finds duplicates says nothing (as the desktop import's toast does when it
+ // shows where they are), and one whose files could not be read says so
+ Ok(report) => {
+ if let Some(notice) = lightcraft_ui_egui::i18n::dropped_photos_outcome(&report) {
+ self.app.toast(ctx, notice);
+ }
+ }
+ Err(e) => {
+ log::warn!("import: {e}");
+ self.app.toast(ctx, lightcraft_ui_egui::i18n::dropped_import_failed(&e));
+ }
}
}
}
@@ -639,8 +663,11 @@ impl WebApp {
fn run_inbox(&mut self) {
let inbox = INBOX.with(|q| std::mem::take(&mut *q.borrow_mut()));
for (id, params, (resolve, reject)) in inbox {
- let params: serde_json::Value = serde_json::from_str(¶ms).unwrap_or(json!({}));
- let r = if id == "web.stats" { Ok(self.web_stats()) } else { self.app.run(&id, params) };
+ let r = match crate::automation::params(¶ms) {
+ Err(e) => Err(format!("{id}: {e}")),
+ Ok(_) if id == "web.stats" => Ok(self.web_stats()),
+ Ok(params) => self.app.run(&id, params),
+ };
let _ = match r {
Ok(v) => resolve.call1(&JsValue::NULL, &v.to_string().into()),
Err(e) => reject.call1(&JsValue::NULL, &e.into()),
@@ -686,6 +713,14 @@ impl eframe::App for WebApp {
fn ui(&mut self, ui: &mut egui::Ui, _frame: &mut eframe::Frame) {
self.app.ui(ui);
+ // eframe's web runner ignores `ViewportCommand::Title`: name the open photo in the page title
+ let title = self.app.page_title(self.files.is_dirty() || self.flushing.get());
+ if title != self.title
+ && let Some(doc) = window().and_then(|w| w.document())
+ {
+ doc.set_title(&title);
+ self.title = title;
+ }
if !self.first_frame_logged && !self.app.widgets.is_empty() {
self.first_frame_logged = true;
log::info!("lightcraft first UI frame at {:.0} ms after navigation start", perf_now());
diff --git a/apps/lightcraft-web/src/wire.rs b/apps/lightcraft-web/src/wire.rs
index 08f9e898d..53d389b6f 100644
--- a/apps/lightcraft-web/src/wire.rs
+++ b/apps/lightcraft-web/src/wire.rs
@@ -522,7 +522,7 @@ mod tests {
}
};
frame(&mut watch, &mut index); // an empty filtered view: no render request yet
- cache.clear(); // File ▸ Clear Preview Cache
+ cache.clear(); // File ▸ Discard Preview Cache
frame(&mut watch, &mut index);
// the filter is removed: the first thumbnail request (`Workers::try_start`)
gone.extend(watch.observe(&cache, cache.generation(), &mut index));
diff --git a/apps/lightcraft/src/control_server.rs b/apps/lightcraft/src/control_server.rs
index 777831942..95aa10896 100644
--- a/apps/lightcraft/src/control_server.rs
+++ b/apps/lightcraft/src/control_server.rs
@@ -240,11 +240,17 @@ mod tests {
}
impl Connection {
- fn new(open: &Arc, idle: Duration) -> Self {
+ /// `before` acts on the client before the worker starts reading, so what it sends is
+ /// already there however late this thread gets scheduled.
+ fn new(open: &Arc, idle: Duration, before: impl FnOnce(&mut TcpStream)) -> Self {
let listener = TcpListener::bind(("127.0.0.1", 0)).unwrap();
- let client = TcpStream::connect(listener.local_addr().unwrap()).unwrap();
+ let mut client = TcpStream::connect(listener.local_addr().unwrap()).unwrap();
client.set_read_timeout(Some(Duration::from_secs(3))).unwrap();
let (stream, _) = listener.accept().unwrap();
+ // Small test messages must not wait for Nagle's algorithm or a delayed ACK.
+ client.set_nodelay(true).unwrap();
+ stream.set_nodelay(true).unwrap();
+ before(&mut client);
let (tx, requests) = channel();
let worker = spawn_connection(stream, tx, egui::Context::default(), Arc::clone(open), idle).unwrap();
Self { client: BufReader::new(client), requests, worker: Some(worker) }
@@ -271,9 +277,8 @@ mod tests {
fn idle_connections_release_their_slots_without_dispatching() {
for input in ["", "{\"method\":\"engine.execute\""] {
let open = Arc::new(AtomicUsize::new(0));
- let mut connection = Connection::new(&open, Duration::from_millis(100));
+ let mut connection = Connection::new(&open, Duration::from_millis(100), |c| c.write_all(input.as_bytes()).unwrap());
assert_eq!(open.load(Ordering::SeqCst), 1);
- connection.client.get_mut().write_all(input.as_bytes()).unwrap();
let mut reply = String::new();
let closed = connection.client.read_to_string(&mut reply);
assert!(closed.is_ok(), "idle connection stayed open: {closed:?}");
@@ -285,8 +290,7 @@ mod tests {
assert!(reply["error"].as_str().unwrap().contains("idle read timed out"));
// A subsequent connection is admitted through the same accounting path.
- let mut next = Connection::new(&open, Duration::from_secs(1));
- next.client.get_mut().shutdown(std::net::Shutdown::Write).unwrap();
+ let mut next = Connection::new(&open, Duration::from_secs(1), |c| c.shutdown(std::net::Shutdown::Write).unwrap());
let mut eof = String::new();
next.client.read_to_string(&mut eof).unwrap();
next.join();
@@ -298,21 +302,32 @@ mod tests {
#[test]
fn persistent_requests_and_slow_replies_outlive_the_idle_read_timeout() {
let open = Arc::new(AtomicUsize::new(0));
- let mut connection = Connection::new(&open, Duration::from_millis(100));
+ let request = |id: u64| format!("{}\n", json!({"id": id, "method": "engine.commands"}));
+ // Each request reaches the socket before the worker reads it (the next one while the worker
+ // waits for the reply), so only the reply waits outlast the idle timeout, however slowly
+ // this thread runs.
+ let mut connection = Connection::new(&open, Duration::from_millis(100), |c| c.write_all(request(1).as_bytes()).unwrap());
for id in [1, 2] {
- writeln!(connection.client.get_mut(), "{}", json!({"id": id, "method": "engine.commands"})).unwrap();
- let req = connection.requests.recv_timeout(Duration::from_secs(3)).unwrap();
+ let req = connection.requests.recv_timeout(Duration::from_secs(10)).unwrap();
assert_eq!(req.method, "engine.commands");
assert!(req.take());
+ if id == 1 {
+ connection.client.get_mut().write_all(request(2).as_bytes()).unwrap();
+ } else {
+ connection.client.get_mut().shutdown(std::net::Shutdown::Write).unwrap();
+ }
// A dispatched command waits for its reply, not for socket input.
std::thread::sleep(Duration::from_millis(250));
req.reply.send(json!({"ok": true, "result": id})).unwrap();
+ if id == 2 {
+ // The worker must reach EOF even before the client reads its final reply.
+ connection.join();
+ }
let mut reply = String::new();
connection.client.read_line(&mut reply).unwrap();
let reply: Value = serde_json::from_str(&reply).unwrap();
assert_eq!(reply, json!({"id": id, "ok": true, "result": id}));
}
- connection.client.get_mut().shutdown(std::net::Shutdown::Write).unwrap();
let mut eof = String::new();
connection.client.read_to_string(&mut eof).unwrap();
connection.join();
@@ -327,7 +342,8 @@ mod tests {
let (req, rrx) = ControlRequest::new("ui.inspect", json!({}));
let t0 = std::time::Instant::now();
let r = await_reply(&rrx, &req.state, || 7, || {}, Duration::from_millis(50), Duration::from_secs(10));
- assert!(t0.elapsed() < Duration::from_secs(2));
+ // the frame window (50 ms), not the 10 s timeout; the margin is for a slow machine
+ assert!(t0.elapsed() < Duration::from_secs(5), "{:?}", t0.elapsed());
assert_eq!(r["ok"], false);
assert!(r["error"].as_str().unwrap().contains("not visible"), "{r}");
assert!(!req.take(), "the app does not run it once it shows up");
diff --git a/apps/lightcraft/src/editors.rs b/apps/lightcraft/src/editors.rs
index cf6d66079..3ca748f47 100644
--- a/apps/lightcraft/src/editors.rs
+++ b/apps/lightcraft/src/editors.rs
@@ -31,22 +31,14 @@ pub fn check(app: &str) -> Result<(), String> {
/// Open `path` in `app` (see [`check`]). Spawned, never waited for: the editor runs alongside.
pub fn open_with(path: &str, app: &str) -> Result<(), String> {
let mut c = match launcher(app)? {
- Launcher::Default if cfg!(target_os = "macos") => {
- let mut c = Command::new("open");
- c.arg(path);
- c
- }
+ Launcher::Default if cfg!(target_os = "macos") => return run_open(&[path]),
Launcher::Default if cfg!(target_os = "windows") => windows_default_command(path),
Launcher::Default => {
let mut c = Command::new("xdg-open");
c.arg(path);
c
}
- Launcher::MacApp(app) => {
- let mut c = Command::new("open");
- c.args(["-a", &app, path]);
- c
- }
+ Launcher::MacApp(app) => return run_open(&["-a", &app, path]),
Launcher::Program(p) => {
let mut c = Command::new(p);
c.arg(path);
@@ -61,6 +53,22 @@ pub fn open_with(path: &str, app: &str) -> Result<(), String> {
c.spawn().map(|_| ()).map_err(|e| e.to_string())
}
+/// macOS: run `open` and wait for it (it returns as soon as LaunchServices has handed the file
+/// over), so an application it can't find or start is reported instead of silently doing nothing.
+fn run_open(args: &[&str]) -> Result<(), String> {
+ let out = Command::new("open").args(args).output().map_err(|e| e.to_string())?;
+ open_result(out.status.success(), &String::from_utf8_lossy(&out.stderr))
+}
+
+/// What `open` said, as a result: its own message when it failed.
+fn open_result(ok: bool, stderr: &str) -> Result<(), String> {
+ if ok {
+ return Ok(());
+ }
+ let msg = stderr.lines().map(str::trim).find(|l| !l.is_empty()).unwrap_or("the application couldn't be started");
+ Err(msg.to_string())
+}
+
/// Open every file in `paths` with one start of `app`, as a file manager hands over a selection:
/// an editor that keeps a single window (DxO PhotoLab) loses the files that arrive while it is
/// still starting when it is started once per file. The system default opens them one by one
@@ -119,10 +127,15 @@ fn launcher(app: &str) -> Result {
fn find_photocraft() -> Option {
let env = |k: &str| std::env::var_os(k).map(|v| v.to_string_lossy().to_string());
if cfg!(target_os = "macos") {
- // LaunchServices finds the app wherever it is; these are only the usual places
+ // the usual places, then wherever Spotlight knows it (by bundle id or bundle name)
let home = env("HOME").map(|h| Path::new(&h).join("Applications/PhotoCraft.app"));
- let found = [Some(PathBuf::from("/Applications/PhotoCraft.app")), home].into_iter().flatten().find(|p| p.is_dir());
- return Some(Launcher::MacApp(found.map_or_else(|| PHOTOCRAFT.to_string(), |p| p.to_string_lossy().to_string())));
+ let usual = [Some(PathBuf::from("/Applications/PhotoCraft.app")), home].into_iter().flatten().find(|p| p.is_dir());
+ let spotlight = || {
+ let query = format!("kMDItemCFBundleIdentifier == '{PHOTOCRAFT_ID}' || kMDItemFSName == 'PhotoCraft.app'");
+ let out = Command::new("mdfind").arg(query).output().ok().filter(|o| o.status.success())?;
+ mdfind_app(&String::from_utf8_lossy(&out.stdout))
+ };
+ return usual.or_else(spotlight).map(|p| Launcher::MacApp(p.to_string_lossy().to_string()));
}
if cfg!(target_os = "windows") {
let install_dir = reg_query(r"HKLM\Software\PhotoCraft", Some("InstallDir"));
@@ -130,16 +143,34 @@ fn find_photocraft() -> Option {
return windows_candidates(install_dir, app_path, env).into_iter().find(|p| p.is_file()).map(Launcher::Program);
}
// Linux, FreeBSD: on the PATH (.deb, .rpm, tarball), a desktop entry (AppImage, which
- // registers one on first run), else the Flatpak
- let on_path =
- env("PATH").into_iter().flat_map(|p| std::env::split_paths(&p).collect::>()).map(|d| d.join("photocraft")).find(|p| p.is_file());
+ // registers one on first run; its Exec a path or a name on the PATH), an AppImage where people
+ // keep them, else the Flatpak
+ let path_dirs: Vec = env("PATH").into_iter().flat_map(|p| std::env::split_paths(&p).collect::>()).collect();
+ let on_path = |name: &Path| resolve_program(name, &path_dirs);
let from_entry = || {
desktop_entry_dirs(env).into_iter().find_map(|d| {
let text = std::fs::read_to_string(d.join(format!("{PHOTOCRAFT_ID}.desktop"))).ok()?;
- desktop_exec(&text).filter(|p| p.is_file())
+ desktop_exec(&text)
+ // a wrapper (`Exec=env VAR=1 /opt/PhotoCraft/photocraft %F`) would be started with the photo only:
+ // skip it and look further
+ .filter(|p| p.file_name().is_some_and(|n| n.to_string_lossy().to_lowercase().contains("photocraft")))
+ .and_then(|p| on_path(&p))
})
};
- if let Some(p) = on_path.or_else(from_entry) {
+ let appimage = || {
+ let home = env("HOME")?;
+ let dirs = [Path::new(&home).join("Applications"), Path::new(&home).join(".local/bin")];
+ // both folders, so the newest version wins whichever it is in
+ let mut found: Vec = dirs
+ .iter()
+ .filter_map(|d| std::fs::read_dir(d).ok())
+ .flat_map(|rd| rd.flatten().map(|e| e.path()).filter(|p| is_photocraft_appimage(p) && p.is_file()).collect::>())
+ .collect();
+ // the newest version by name (1.10 after 1.9)
+ found.sort_by_key(|p| version_key(&p.file_name().map(|n| n.to_string_lossy().to_lowercase()).unwrap_or_default()));
+ found.pop()
+ };
+ if let Some(p) = on_path(Path::new("photocraft")).or_else(from_entry).or_else(appimage) {
return Some(Launcher::Program(p));
}
let flatpak = Command::new("flatpak").args(["info", PHOTOCRAFT_ID]).output().is_ok_and(|o| o.status.success());
@@ -205,10 +236,44 @@ fn desktop_exec(entry: &str) -> Option {
Some(rest) => rest.split('"').next()?,
None => exec.split_whitespace().next()?,
};
- // a bare name (`photocraft %F`) was already looked for on the PATH. Desktop entries are a
- // freedesktop (POSIX) format: absolute means a leading `/` on every platform the parser runs on
- // (`Path::is_absolute` wants a drive letter on Windows, where the tests run too)
- program.starts_with('/').then(|| PathBuf::from(program))
+ // a path, or a bare name (`photocraft %F`) the caller looks for on the PATH
+ (!program.is_empty()).then(|| PathBuf::from(program))
+}
+
+/// A program from a desktop entry or the PATH: a path as it is (if it is a file), a bare name
+/// looked for in `dirs`. A leading `/` counts as absolute on every platform (desktop entries are
+/// a POSIX format; `Path::is_absolute` wants a drive letter on Windows, where the tests run too).
+fn resolve_program(name: &Path, dirs: &[PathBuf]) -> Option {
+ if name.is_absolute() || name.starts_with("/") {
+ return Some(name.to_path_buf()).filter(|p| p.is_file());
+ }
+ dirs.iter().map(|d| d.join(name)).find(|p| p.is_file())
+}
+
+/// A sort key that orders the digit runs in a name by value (`1.9` before `1.10`).
+fn version_key(name: &str) -> Vec<(u64, String)> {
+ let mut out = Vec::new();
+ let mut rest = name;
+ while !rest.is_empty() {
+ let digits = rest.len() - rest.trim_start_matches(|c: char| c.is_ascii_digit()).len();
+ let (num, tail) = rest.split_at(digits);
+ let text_len = tail.find(|c: char| c.is_ascii_digit()).unwrap_or(tail.len());
+ let (text, next) = tail.split_at(text_len);
+ out.push((num.parse().unwrap_or(u64::MAX), text.to_string()));
+ rest = next;
+ }
+ out
+}
+
+/// An AppImage of PhotoCraft by its file name (`PhotoCraft-1.2-x86_64.AppImage`, any case).
+fn is_photocraft_appimage(p: &Path) -> bool {
+ let name = p.file_name().map(|n| n.to_string_lossy().to_lowercase()).unwrap_or_default();
+ name.contains("photocraft") && name.ends_with(".appimage")
+}
+
+/// The first application bundle in `mdfind` output (one path per line).
+fn mdfind_app(output: &str) -> Option {
+ output.lines().map(str::trim).find(|l| l.ends_with(".app")).map(PathBuf::from)
}
#[cfg(test)]
@@ -275,13 +340,59 @@ mod tests {
let appimage = "[Desktop Entry]\nName=PhotoCraft\nExec=\"/home/simon/Apps/photocraft-1.2-linux-x86_64.AppImage\" %F\n";
assert_eq!(desktop_exec(appimage), Some(PathBuf::from("/home/simon/Apps/photocraft-1.2-linux-x86_64.AppImage")));
assert_eq!(desktop_exec("Exec=/opt/photocraft/photocraft %F\n"), Some(PathBuf::from("/opt/photocraft/photocraft")));
- assert_eq!(desktop_exec("Exec=photocraft %F\nTryExec=photocraft\n"), None, "a bare name is a PATH lookup");
+ assert_eq!(desktop_exec("Exec=photocraft %F\nTryExec=photocraft\n"), Some(PathBuf::from("photocraft")), "a bare name, for a PATH lookup");
assert_eq!(desktop_exec("[Desktop Entry]\n"), None);
let dirs = desktop_entry_dirs(|k| (k == "HOME").then(|| "/home/simon".to_string()));
assert_eq!(dirs[0], PathBuf::from("/home/simon/.local/share/applications"));
assert!(dirs.contains(&PathBuf::from("/usr/share/applications")));
}
+ #[test]
+ fn a_bare_exec_name_is_found_on_the_path() {
+ let a = std::env::temp_dir().join(format!("lc-editors-a-{}", std::process::id()));
+ let b = std::env::temp_dir().join(format!("lc-editors-b-{}", std::process::id()));
+ std::fs::create_dir_all(&a).unwrap();
+ std::fs::create_dir_all(&b).unwrap();
+ std::fs::write(b.join("photocraft"), b"").unwrap();
+ let dirs = vec![a.clone(), b.clone()];
+ assert_eq!(resolve_program(Path::new("photocraft"), &dirs), Some(b.join("photocraft")));
+ assert_eq!(resolve_program(Path::new("nothere"), &dirs), None);
+ let abs = b.join("photocraft");
+ assert_eq!(resolve_program(&abs, &[]), Some(abs.clone()), "a path needs no PATH");
+ assert_eq!(resolve_program(&b.join("missing"), &dirs), None);
+ let _ = std::fs::remove_dir_all(&a);
+ let _ = std::fs::remove_dir_all(&b);
+ }
+
+ #[test]
+ fn appimage_versions_sort_numerically() {
+ let mut names = ["photocraft-1.10-x86_64.appimage", "photocraft-1.9-x86_64.appimage", "photocraft-1.2-x86_64.appimage"];
+ names.sort_by_key(|n| version_key(n));
+ assert_eq!(names[2], "photocraft-1.10-x86_64.appimage");
+ assert_eq!(names[0], "photocraft-1.2-x86_64.appimage");
+ }
+
+ #[test]
+ fn a_failed_open_is_an_error() {
+ assert!(open_result(true, "").is_ok());
+ assert!(open_result(false, "Unable to find application\n").is_err());
+ assert!(open_result(false, "").is_err(), "even when it says nothing");
+ }
+
+ #[test]
+ fn odd_detection_input_does_not_panic() {
+ for text in ["", "Exec=", "Exec=\"", "Exec=\"\"", "\u{0}\u{fffd}", "Exec= %F", "[Desktop Entry]\nExec=\u{e9}\u{301}"] {
+ let _ = desktop_exec(text);
+ let _ = mdfind_app(text);
+ let _ = reg_value(text);
+ }
+ for p in ["", "/", ".AppImage", "\u{e9}.appimage", "a/../b"] {
+ let _ = is_photocraft_appimage(Path::new(p));
+ }
+ let _ = desktop_entry_dirs(|_| None);
+ let _ = windows_candidates(None, Some("\"".into()), |_| None);
+ }
+
#[test]
fn other_editors_are_used_as_named() {
assert_eq!(launcher("").unwrap(), Launcher::Default);
diff --git a/apps/lightcraft/src/main.rs b/apps/lightcraft/src/main.rs
index 95739b3e9..7d30e69cd 100644
--- a/apps/lightcraft/src/main.rs
+++ b/apps/lightcraft/src/main.rs
@@ -3,8 +3,10 @@
//! Usage: `lightcraft [--library DIR | --memory] [--no-demo] [--control ] [files or folders…]`
//!
//! The library (catalog, presets, thumbnail cache) lives in `--library DIR`, else
-//! `$LIGHTCRAFT_LIBRARY`, else the library last opened with Settings → Open Library…, else
-//! `~/Pictures/LightCraft Library`; a new library starts with the
+//! `$LIGHTCRAFT_LIBRARY`, else — with Settings ▸ General ▸ At launch set to *Ask me*, or Alt/Option
+//! held as the window appears — the one chosen in a list of the recent libraries, else the library
+//! last opened, else `~/Pictures/LightCraft Library`
+//! ([`lightcraft_ui_egui::panels::library_problem::launch_plan`]); a new library starts with the
//! procedural demo photos unless `--no-demo` or files are given. Files and folders on the command
//! line are imported (duplicates are skipped). `--memory` runs an in-memory session that writes
//! nothing (demo photos unless files are given; used by the README showcase scripts). A library
@@ -39,7 +41,7 @@ mod window_gpu;
use dialog_filter::FileDialogExt as _;
use lightcraft_engine::Session;
-use lightcraft_ui_egui::panels::library_problem::LibraryProblem;
+use lightcraft_ui_egui::panels::library_problem::{Launch, LibraryProblem};
use lightcraft_ui_egui::{LightcraftApp, Services, UiState};
/// Reverse-DNS app id: Wayland app id, `.desktop` file and hicolor icon name.
@@ -363,7 +365,7 @@ struct PrefsWriter {
impl PrefsWriter {
fn save(&mut self, app: &LightcraftApp) -> Result<(), String> {
- self.save_ui(&app.ui, app.library_problem.is_some())
+ self.save_ui(&app.ui, app.library_problem.as_ref().is_some_and(LibraryProblem::saves_nothing))
}
/// Write `ui` if it changed since the last write; `temporary` (the library couldn't be
@@ -718,9 +720,13 @@ fn services(cc: &eframe::CreationContext<'_>, log_file: Option<&std::path::Path>
}
/// The library session with the folders face models (`/models`) and slideshow templates
-/// (`/slideshow-templates`) are kept in.
-fn open_session(in_memory: bool, dir: Option, seed_demo: bool) -> (Session, Option) {
- let (mut s, problem) = open_library_session(in_memory, dir, seed_demo);
+/// (`/slideshow-templates`) are kept in. With [`Launch::Ask`] no library is opened yet:
+/// the window lists the recent ones.
+fn open_session(in_memory: bool, launch: &Launch, seed_demo: bool) -> (Session, Option) {
+ let (mut s, problem) = match launch {
+ Launch::Ask => (Session::new().with_fs().with_default_denoise_models().with_system_clock(), None),
+ Launch::Open { dir, .. } => open_library_session(in_memory, dir.clone(), seed_demo),
+ };
s.face_models_dir = lightcraft_engine::config::default_face_models_dir();
s.slide_templates_dir = lightcraft_engine::config::config_dir().map(|d| d.join(lightcraft_engine::slide_template::DIR_NAME));
// Control requests are answered on the UI thread: Add to Apple Photos must not wait there
@@ -769,7 +775,9 @@ lightcraft — photo library + raw developer
USAGE: lightcraft [OPTIONS] [FILES/FOLDERS…] (files and folders are imported)
OPTIONS:
- --library DIR open (or create) this library (default: ~/Pictures/LightCraft Library; env LIGHTCRAFT_LIBRARY)
+ --library DIR open (or create) this library (env LIGHTCRAFT_LIBRARY; default: the last library opened, or
+ a list of the recent ones with Settings > General > At launch: Ask me or Alt/Option held,
+ else ~/Pictures/LightCraft Library)
--no-demo don't seed a new library with the procedural demo photos
--memory throwaway in-memory session (alias --demo); nothing is saved
--control PORT serve the JSON-lines control channel on 127.0.0.1:PORT (env LIGHTCRAFT_CONTROL_PORT;
@@ -900,15 +908,15 @@ fn main() -> eframe::Result {
if let Some(ui) = &prefs {
lightcraft_ui_egui::i18n::set_language(ui.language);
}
- // --library, else the library last opened from Settings, else the default location
- let library_dir = library_dir.or_else(|| {
- prefs
- .as_ref()
- .map(|u| u.settings.library_path.clone())
- .filter(|p| !p.is_empty() && std::env::var_os("LIGHTCRAFT_LIBRARY").is_none())
- .map(Into::into)
- });
- let library_dir = library_dir.or_else(lightcraft_engine::library::default_dir);
+ // --library, else LIGHTCRAFT_LIBRARY, else the list of recent libraries if Settings say Ask
+ // me, else the library last opened from Settings, else the default location
+ let launch = lightcraft_ui_egui::panels::library_problem::launch_plan(
+ in_memory,
+ library_dir,
+ std::env::var_os("LIGHTCRAFT_LIBRARY").filter(|p| !p.is_empty()).map(Into::into),
+ prefs.as_ref().map(|u| &u.settings),
+ lightcraft_engine::library::default_dir(),
+ );
// GPU compute: off if the preference says so, or if the last launch died creating the device
// (issue #136) — before anything can create it
let gpu_crash = gpu_crash_check(in_memory).or_else(|| gpu_lost_check_at(gpu_lost_marker_path().as_deref(), in_memory));
@@ -961,7 +969,7 @@ fn main() -> eframe::Result {
// the window's renderer is up (eframe calls this after creating it)
win_files_app.started(window_renderer);
came_up_app.store(true, std::sync::atomic::Ordering::Relaxed);
- let (mut session, problem) = open_session(in_memory, library_dir, seed_demo && files.is_empty());
+ let (mut session, mut problem) = open_session(in_memory, &launch, seed_demo && files.is_empty());
// AI masks: the SAM 3 checkpoint (facebook/sam3) in /models/sam3, or LIGHTCRAFT_SAM3_DIR
// (never required: without it, AI masks offer to download it; see docs/ai-masks.md)
// and the user's own download locations, one base URL per line (LIGHTCRAFT_SAM3_MIRRORS too)
@@ -972,6 +980,11 @@ fn main() -> eframe::Result {
if let Some(ui) = prefs {
app.ui = ui;
}
+ // the recent libraries to choose from (issue #1290), checked on a worker once shown
+ if launch == Launch::Ask {
+ problem = Some(LibraryProblem::choose_later(&app.ui.settings));
+ }
+ app.alt_asks_for_library = matches!(launch, Launch::Open { alt_asks: true, .. });
lightcraft_ui_egui::i18n::set_language(app.ui.language);
app.integrated_titlebar = cfg!(target_os = "macos");
app.notices.extend(prefs_warning);
@@ -1145,12 +1158,40 @@ mod tests {
/// adds to Apple Photos on a worker (headless sessions wait instead).
#[test]
fn the_desktop_session_adds_to_apple_photos_in_the_background() {
- let (s, problem) = open_session(true, None, false);
+ let (s, problem) = open_session(true, &Launch::Open { dir: None, alt_asks: false }, false);
assert!(problem.is_none());
assert!(s.apple_photos_background);
assert!(!lightcraft_engine::Session::new().apple_photos_background);
}
+ /// Issue #1290: with Settings ▸ At launch ▸ Ask me nothing is opened (nor seeded with demo
+ /// photos) before the user chooses, and the settings are still saved while the list shows (a
+ /// removed entry is kept), unlike the failed-library window's temporary session.
+ #[test]
+ fn asking_at_launch_opens_nothing_and_still_saves_the_settings() {
+ let (s, problem) = open_session(false, &Launch::Ask, true);
+ assert!(problem.is_none() && s.library.is_none() && s.catalog.is_empty());
+ assert!(s.apple_photos_background, "the same desktop session as an opened one");
+
+ let d = dir("writer-ask");
+ let path = d.join("ui.json");
+ let mut app = LightcraftApp::new(s, Services::default());
+ app.ui.settings.recent_libraries = vec!["/gone/a".into(), "/gone/b".into()];
+ app.library_problem = Some(LibraryProblem::choose_later(&app.ui.settings));
+ let mut w = PrefsWriter { path: Some(path.clone()), ..Default::default() };
+ app.ui.settings.recent_libraries.remove(0);
+ w.save(&app).unwrap();
+ let saved: UiState = serde_json::from_slice(&std::fs::read(&path).unwrap()).unwrap();
+ assert_eq!(saved.settings.recent_libraries, ["/gone/b"]);
+ // the failed-library window still writes nothing
+ app.library_problem = Some(LibraryProblem::new("/gone/b", "not there"));
+ app.ui.settings.recent_libraries.clear();
+ w.save(&app).unwrap();
+ let saved: UiState = serde_json::from_slice(&std::fs::read(&path).unwrap()).unwrap();
+ assert_eq!(saved.settings.recent_libraries, ["/gone/b"]);
+ let _ = std::fs::remove_dir_all(&d);
+ }
+
/// Issue #103: a damaged ui.json (which holds the library location) is kept aside and
/// reported, not silently replaced by the defaults.
#[test]
diff --git a/apps/lightcraft/src/native_menu.rs b/apps/lightcraft/src/native_menu.rs
index 4ac40bcac..e9323a210 100644
--- a/apps/lightcraft/src/native_menu.rs
+++ b/apps/lightcraft/src/native_menu.rs
@@ -16,7 +16,9 @@ use std::collections::{HashMap, HashSet};
use std::sync::mpsc::{Receiver, channel};
use lightcraft_ui_egui::LightcraftApp;
-use lightcraft_ui_egui::menubar::{MenuNode, menu_bar};
+use lightcraft_ui_egui::menubar::{
+ MenuNode, menu_bar, native_item_presentation, native_menu_delivers as menu_delivers, native_menu_structure as structure_of,
+};
use muda::accelerator::{Accelerator, Code, Modifiers};
use muda::{CheckMenuItem, Menu, MenuEvent, MenuItem, PredefinedMenuItem, Submenu};
use serde_json::Value;
@@ -39,18 +41,140 @@ enum Handle {
struct Item {
handle: Handle,
+ /// The submenu the item sits in, to swap the item when its accelerator has to go (macOS).
+ parent: Submenu,
cmd: String,
params: Value,
shortcut: Option,
+ /// The accelerator the item's shortcut maps to.
accel: Option,
+ /// The accelerator the native item has now: `accel`, or `None` while it yields.
+ current: Option,
label: String,
enabled: bool,
checked: Option,
+ /// Whether the item may be replaced to drop its key. Not in the Help menu: a swap there left
+ /// the old row and added a second one (muda 0.21, macOS 27), so those keys stay.
+ swappable: bool,
+}
+
+impl Item {
+ /// The row's text as installed: `label` is already what the row shows (translated, with a
+ /// borrowed key's hint, `native_item_presentation`).
+ fn native_label(&self) -> String {
+ self.label.replace('&', "&&")
+ }
+
+ /// Puts `want` on the native item. muda's `set_accelerator(None)` does nothing on macOS (the
+ /// old key equivalent stays and the menu still takes the key), so there the item is replaced
+ /// by an identical one without an accelerator, at the same position and with the same id,
+ /// text, enabled and checked state. Only done when the accelerator changes (and again while a
+ /// native call keeps failing), never per frame otherwise.
+ fn set_accelerator(&mut self, want: Option) {
+ if self.current == want {
+ return;
+ }
+ if cfg!(target_os = "macos") && want.is_none() && self.swappable {
+ // `current` follows the native item: it changes only when the swap went through, so a
+ // failed swap (the old item, key and all, is still there) is tried again
+ let done = self.replace_without_accelerator();
+ self.current = settled(self.current, want, done);
+ return;
+ }
+ let done = match &self.handle {
+ Handle::Plain(h) => h.set_accelerator(want),
+ Handle::Check(h) => h.set_accelerator(want),
+ }
+ .is_ok();
+ self.current = settled(self.current, want, done);
+ }
+
+ /// Replaces the native item by one without an accelerator. `self.handle` changes only when the
+ /// new item is attached; on failure the old item is still the one in the submenu.
+ fn replace_without_accelerator(&mut self) -> bool {
+ let (id, old): (_, &dyn muda::IsMenuItem) = match &self.handle {
+ Handle::Plain(h) => (h.id().clone(), h),
+ Handle::Check(h) => (h.id().clone(), h),
+ };
+ let Some(position) = self.parent.items().iter().position(|k| *k.id() == id) else { return false };
+ let new = match &self.handle {
+ Handle::Plain(_) => Handle::Plain(MenuItem::with_id(id, self.native_label(), self.enabled, None)),
+ Handle::Check(h) => Handle::Check(CheckMenuItem::with_id(id, self.native_label(), self.enabled, h.is_checked(), None)),
+ };
+ let new_ref: &dyn muda::IsMenuItem = match &new {
+ Handle::Plain(h) => h,
+ Handle::Check(h) => h,
+ };
+ if !swap_item(&self.parent, old, new_ref, position) {
+ return false;
+ }
+ self.handle = new;
+ true
+ }
+}
+
+/// The accelerator the native item has after trying to put `want` on it: `want` when the native
+/// call worked, else what it had, so the caller sees the mismatch and tries again.
+fn settled(had: Option, want: Option, native_call_worked: bool) -> Option {
+ if native_call_worked { want } else { had }
+}
+
+/// Whether the per-frame accelerator sync has to run: the focus or recording state changed, or an
+/// earlier sync left an item whose native accelerator is not the wanted one (a failed swap).
+fn sync_needed(focus: bool, capturing: bool, synced_focus: bool, synced_capturing: bool, pending: bool) -> bool {
+ focus != synced_focus || capturing != synced_capturing || pending
+}
+
+/// The three submenu calls a swap needs, so the failure paths can be tested without a main-thread
+/// `muda::Menu`.
+trait SwapTarget {
+ type Row: ?Sized;
+ fn remove(&self, row: &Self::Row) -> bool;
+ fn insert(&self, row: &Self::Row, position: usize) -> bool;
+ fn append(&self, row: &Self::Row) -> bool;
+}
+
+impl SwapTarget for Submenu {
+ type Row = dyn muda::IsMenuItem;
+ fn remove(&self, row: &Self::Row) -> bool {
+ Submenu::remove(self, row).is_ok()
+ }
+ fn insert(&self, row: &Self::Row, position: usize) -> bool {
+ Submenu::insert(self, row, position).is_ok()
+ }
+ fn append(&self, row: &Self::Row) -> bool {
+ Submenu::append(self, row).is_ok()
+ }
+}
+
+/// Puts `new` where `old` is. Returns whether `new` is attached; when it is not, `old` is back in
+/// the menu (at its position, else at the end), so the caller keeps `old` as its handle.
+fn swap_item(target: &T, old: &T::Row, new: &T::Row, position: usize) -> bool {
+ if !target.remove(old) {
+ return false;
+ }
+ // (an item at the end beats a missing one)
+ if target.insert(new, position) || target.append(new) {
+ return true;
+ }
+ let _ = target.insert(old, position) || target.append(old);
+ false
+}
+
+/// The accelerator a native item has: none while a shortcut is being recorded, and none for the
+/// keys a focused text field needs.
+fn wanted_accelerator(accel: Option, sc: &str, text_focus: bool, capturing: bool) -> Option {
+ if capturing || (text_focus && yields_to_text(sc)) { None } else { accel }
}
pub struct NativeMenu {
menu: Menu,
items: HashMap,
+ /// Settings… and Quit in the application submenu: ordinary items, so they give up ⌘, and ⌘Q
+ /// to the shortcut recorder like the rest. (The system's own items there and in File/Window,
+ /// such as Hide ⌘H, Close Window ⌘W and Minimize ⌘M, are muda predefined items whose key
+ /// cannot be changed or swapped.)
+ app_items: Vec,
rx: Receiver,
/// Structure of the installed menu (ids, kinds, submenu names); a change rebuilds it.
structure: String,
@@ -58,6 +182,8 @@ pub struct NativeMenu {
/// The keyboard shortcuts editor is recording a key: every accelerator is off so the key
/// press reaches it.
capturing: bool,
+ /// An item's native accelerator is not the wanted one (its swap or accelerator call failed): sync again next frame.
+ accelerators_pending: bool,
}
/// `Cmd+Shift+Z` → a muda accelerator (`None` for keys we leave to egui, e.g. Escape).
@@ -154,13 +280,6 @@ fn yields_to_text(sc: &str) -> bool {
!sc.contains("Cmd") && !sc.contains("Ctrl") || TEXT_EDIT.contains(&sc)
}
-/// Whether AppKit hands this shortcut to the menu bar: only key presses with ⌘ or ⌃, and function
-/// keys. A plain `C` or `⇧P` never reaches it (the window's view takes the key), so egui has to run
-/// those even though their menu item shows the key.
-fn menu_delivers(sc: &str) -> bool {
- sc.split('+').any(|part| matches!(part, "Cmd" | "Ctrl") || part.strip_prefix('F').is_some_and(|n| n.parse::().is_ok()))
-}
-
/// The shortcuts the menu bar runs itself, for `LightcraftApp::native_shortcuts`.
fn owned_by_menu<'a>(installed: impl Iterator, text_focus: bool) -> HashSet {
installed.filter(|sc| menu_delivers(sc) && !(text_focus && yields_to_text(sc))).map(str::to_string).collect()
@@ -171,34 +290,6 @@ fn label_text(s: &str) -> String {
lightcraft_ui_egui::i18n::tr(s).replace('&', "&&")
}
-fn structure_of(bar: &[(String, Vec)]) -> String {
- fn walk(n: &[MenuNode], out: &mut String) {
- for x in n {
- match x {
- MenuNode::Item { id, params, checked, shortcut, .. } => {
- out.push_str(&MenuNode::key(id, params));
- out.push(if checked.is_some() { 'c' } else { 'i' });
- out.push_str(shortcut.as_deref().unwrap_or(""));
- }
- MenuNode::Separator => out.push('-'),
- MenuNode::Submenu { label, children } => {
- out.push('[');
- out.push_str(label);
- walk(children, out);
- out.push(']');
- }
- }
- out.push(';');
- }
- }
- let mut s = format!("{};", lightcraft_ui_egui::i18n::language().code());
- for (t, items) in bar {
- s.push_str(t);
- walk(items, &mut s);
- }
- s
-}
-
/// Drop a trailing separator (left behind when an item moved to the app menu).
fn tidy_separators(mut nodes: Vec) -> Vec {
while matches!(nodes.last(), Some(MenuNode::Separator)) {
@@ -216,17 +307,28 @@ impl NativeMenu {
let _ = tx.send(e.id.0);
repaint.request_repaint();
}));
- let mut m = NativeMenu { menu: Menu::new(), items: HashMap::new(), rx, structure: String::new(), text_focus: false, capturing: false };
+ let mut m = NativeMenu {
+ menu: Menu::new(),
+ items: HashMap::new(),
+ app_items: Vec::new(),
+ rx,
+ structure: String::new(),
+ text_focus: false,
+ capturing: false,
+ accelerators_pending: false,
+ };
m.rebuild(app);
app.native_menu = true;
m
}
+ /// Recreate native handles and shortcut ownership from the current shared menu model.
fn rebuild(&mut self, app: &mut LightcraftApp) {
let bar = menu_bar(app);
- self.structure = structure_of(&bar);
+ self.structure = structure_of(&bar, &app.ui.settings.keymap);
self.menu = Menu::new();
self.items.clear();
+ self.app_items.clear();
// the application menu
let app_menu = Submenu::new("LightCraft", true);
@@ -246,6 +348,22 @@ impl NativeMenu {
&PredefinedMenuItem::separator(),
&quit,
]);
+ for (handle, cmd, shortcut, text) in [(&settings, SETTINGS, SETTINGS_KEY, "Settings…"), (&quit, QUIT, "Cmd+Q", "Quit LightCraft")] {
+ let accel = accelerator(shortcut);
+ self.app_items.push(Item {
+ handle: Handle::Plain(handle.clone()),
+ parent: app_menu.clone(),
+ swappable: true,
+ cmd: cmd.to_string(),
+ params: Value::Null,
+ shortcut: Some(shortcut.to_string()),
+ accel,
+ current: accel,
+ label: lightcraft_ui_egui::menubar::display_item_label(cmd, &Value::Null, text).to_string(),
+ enabled: true,
+ checked: None,
+ });
+ }
let _ = self.menu.append(&app_menu);
for (title, nodes) in &bar {
@@ -264,7 +382,9 @@ impl NativeMenu {
.cloned()
.collect();
let nodes = tidy_separators(nodes);
- self.append_nodes(&sub, &nodes);
+ // Swapping an item in the Help menu leaves the old row in place and adds a second one
+ // (measured on macOS 27 with muda 0.21): the Help menu is not addressed row for row.
+ self.append_nodes(&sub, &nodes, &app.ui.settings.keymap, title != "Help");
if title == "File" {
// ⌘W, the system's own item
let _ = sub.append_items(&[
@@ -287,10 +407,13 @@ impl NativeMenu {
self.menu.init_for_nsapp();
self.text_focus = false;
self.capturing = false;
+ self.accelerators_pending = false;
self.publish_shortcuts(app);
}
- fn append_nodes(&mut self, sub: &Submenu, nodes: &[MenuNode]) {
+ /// Install translated rows with their effective accelerators, preserving hint-only keys.
+ /// `swappable`: whether its items may be replaced to drop their key (see `Item::swappable`).
+ fn append_nodes(&mut self, sub: &Submenu, nodes: &[MenuNode], keymap: &lightcraft_ui_egui::shortcuts::Keymap, swappable: bool) {
for n in nodes {
match n {
MenuNode::Separator => {
@@ -298,7 +421,7 @@ impl NativeMenu {
}
MenuNode::Submenu { label, children } => {
let s = Submenu::new(label_text(label), true);
- self.append_nodes(&s, children);
+ self.append_nodes(&s, children, keymap, swappable);
let _ = sub.append(&s);
}
MenuNode::Item { id, params, label, shortcut, enabled, checked } => {
@@ -306,26 +429,16 @@ impl NativeMenu {
if self.items.contains_key(&key) {
continue; // muda ids must be unique
}
- let accel = shortcut.as_deref().filter(|s| !CONTEXTUAL.contains(s)).and_then(accelerator);
+ let (native_label, native_key) = native_item_presentation(keymap, id, params, label, shortcut.as_deref());
+ let accel = native_key.filter(|s| !CONTEXTUAL.contains(s)).and_then(accelerator);
let handle = match checked {
Some(c) => {
- let it = CheckMenuItem::with_id(
- key.clone(),
- lightcraft_ui_egui::menubar::display_item_label(id, params, label).replace('&', "&&"),
- *enabled,
- *c,
- accel,
- );
+ let it = CheckMenuItem::with_id(key.clone(), native_label.replace('&', "&&"), *enabled, *c, accel);
let _ = sub.append(&it);
Handle::Check(it)
}
None => {
- let it = MenuItem::with_id(
- key.clone(),
- lightcraft_ui_egui::menubar::display_item_label(id, params, label).replace('&', "&&"),
- *enabled,
- accel,
- );
+ let it = MenuItem::with_id(key.clone(), native_label.replace('&', "&&"), *enabled, accel);
let _ = sub.append(&it);
Handle::Plain(it)
}
@@ -334,11 +447,14 @@ impl NativeMenu {
key,
Item {
handle,
+ parent: sub.clone(),
+ swappable,
cmd: id.clone(),
params: params.clone(),
shortcut: shortcut.clone(),
accel,
- label: label.clone(),
+ current: accel,
+ label: native_label,
enabled: *enabled,
checked: *checked,
},
@@ -378,22 +494,25 @@ impl NativeMenu {
}
}
let bar = menu_bar(app);
- if structure_of(&bar) != self.structure {
+ if structure_of(&bar, &app.ui.settings.keymap) != self.structure {
+ // (no early return: a rebuild installs every accelerator and resets the state they were synced
+ // to, so the sync below runs in this same frame and no key reaches the menu in between)
self.rebuild(app);
- return quit;
}
- fn walk(nodes: &[MenuNode], items: &mut HashMap) {
+ /// Synchronize live labels and states; accelerator ownership changes already triggered a rebuild.
+ fn walk(nodes: &[MenuNode], items: &mut HashMap, keymap: &lightcraft_ui_egui::shortcuts::Keymap) {
for n in nodes {
match n {
- MenuNode::Submenu { children, .. } => walk(children, items),
- MenuNode::Item { id, params, label, enabled, checked, .. } => {
+ MenuNode::Submenu { children, .. } => walk(children, items, keymap),
+ MenuNode::Item { id, params, label, shortcut, enabled, checked } => {
let Some(it) = items.get_mut(&MenuNode::key(id, params)) else { continue };
- if it.label != *label {
+ let (native_label, _) = native_item_presentation(keymap, id, params, label, shortcut.as_deref());
+ if it.label != native_label {
match &it.handle {
- Handle::Plain(h) => h.set_text(lightcraft_ui_egui::menubar::display_item_label(id, params, label).replace('&', "&&")),
- Handle::Check(h) => h.set_text(lightcraft_ui_egui::menubar::display_item_label(id, params, label).replace('&', "&&")),
+ Handle::Plain(h) => h.set_text(native_label.replace('&', "&&")),
+ Handle::Check(h) => h.set_text(native_label.replace('&', "&&")),
}
- it.label = label.clone();
+ it.label = native_label;
}
if it.enabled != *enabled {
match &it.handle {
@@ -414,24 +533,24 @@ impl NativeMenu {
}
}
}
- walk(&bar.iter().flat_map(|(_, v)| v.clone()).collect::>(), &mut self.items);
+ walk(&bar.iter().flat_map(|(_, v)| v.clone()).collect::>(), &mut self.items, &app.ui.settings.keymap);
let focus = ctx.egui_wants_keyboard_input();
let capturing = app.recording_shortcut.is_some();
- if focus != self.text_focus || capturing != self.capturing {
+ if sync_needed(focus, capturing, self.text_focus, self.capturing, self.accelerators_pending) {
self.text_focus = focus;
self.capturing = capturing;
- for it in self.items.values() {
+ let mut pending = false;
+ for it in self.items.values_mut().chain(self.app_items.iter_mut()) {
let Some(sc) = it.shortcut.as_deref() else { continue };
if it.accel.is_none() {
continue;
}
- let accel = if capturing || (focus && yields_to_text(sc)) { None } else { it.accel };
- let _ = match &it.handle {
- Handle::Plain(h) => h.set_accelerator(accel),
- Handle::Check(h) => h.set_accelerator(accel),
- };
+ let want = wanted_accelerator(it.accel, sc, focus, capturing);
+ it.set_accelerator(want);
+ pending |= it.current != want;
}
+ self.accelerators_pending = pending;
self.publish_shortcuts(app);
}
quit
@@ -454,15 +573,102 @@ mod tests {
let mut seen = std::collections::BTreeSet::new();
for language in Locale::ALL {
set_language(*language);
- assert!(seen.insert(structure_of(&bar)), "{language:?} shares another language's menu structure key");
+ assert!(seen.insert(structure_of(&bar, &app.ui.settings.keymap)), "{language:?} shares another language's menu structure key");
}
- set_language(Locale::ZhHant);
- assert_eq!(label_text("File"), "檔案");
- assert_eq!(label_text("Settings…"), "設定…");
- assert_eq!(label_text("Quit LightCraft"), "結束 LightCraft");
set_language(Locale::En);
}
+ #[test]
+ fn accelerators_yield_to_text_focus_and_shortcut_recording() {
+ let g = accelerator("G");
+ let cmd_s = accelerator("Cmd+S");
+ assert_eq!(wanted_accelerator(g, "G", false, false), g);
+ assert_eq!(wanted_accelerator(g, "G", true, false), None, "typing a G in a field");
+ assert_eq!(wanted_accelerator(cmd_s, "Cmd+S", true, false), cmd_s, "⌘S still saves while typing");
+ assert_eq!(wanted_accelerator(cmd_s, "Cmd+S", false, true), None, "the editor records the key");
+ assert_eq!(wanted_accelerator(cmd_s, "Cmd+S", true, true), None, "recording wins over typing");
+ // the application menu's Settings… and Quit are tracked like the other items
+ for sc in [SETTINGS_KEY, "Cmd+Q"] {
+ let a = accelerator(sc);
+ assert!(a.is_some());
+ assert_eq!(wanted_accelerator(a, sc, true, false), a, "{sc} stays while typing");
+ assert_eq!(wanted_accelerator(a, sc, false, true), None, "{sc} goes while recording");
+ }
+ assert_eq!(wanted_accelerator(None, "Escape", false, false), None, "no accelerator stays none");
+ }
+
+ /// A submenu of numbers whose calls can be made to fail, to run `swap_item`'s failure paths.
+ /// Row 2 is the old item and row 9 its replacement.
+ #[derive(Default)]
+ struct Fake {
+ rows: std::cell::RefCell>,
+ fail_remove: bool,
+ /// The new item (9) cannot be inserted / appended.
+ reject_new_insert: bool,
+ reject_new_append: bool,
+ /// The old item (2) cannot be inserted back.
+ reject_old_insert: bool,
+ }
+ impl SwapTarget for Fake {
+ type Row = u32;
+ fn remove(&self, row: &u32) -> bool {
+ if self.fail_remove {
+ return false;
+ }
+ self.rows.borrow_mut().retain(|x| x != row);
+ true
+ }
+ fn insert(&self, row: &u32, position: usize) -> bool {
+ if (self.reject_new_insert && *row == 9) || (self.reject_old_insert && *row == 2) {
+ return false;
+ }
+ self.rows.borrow_mut().insert(position, *row);
+ true
+ }
+ fn append(&self, row: &u32) -> bool {
+ if self.reject_new_append && *row == 9 {
+ return false;
+ }
+ self.rows.borrow_mut().push(*row);
+ true
+ }
+ }
+
+ fn swap(fake: Fake) -> (bool, Vec) {
+ *fake.rows.borrow_mut() = vec![1, 2, 3];
+ let done = swap_item(&fake, &2, &9, 1);
+ (done, fake.rows.into_inner())
+ }
+
+ #[test]
+ fn a_swap_changes_the_item_only_when_the_new_one_is_attached() {
+ assert_eq!(swap(Fake::default()), (true, vec![1, 9, 3]), "in place");
+ assert_eq!(swap(Fake { fail_remove: true, ..Default::default() }), (false, vec![1, 2, 3]), "remove fails: the old item stays");
+ assert_eq!(swap(Fake { reject_new_insert: true, ..Default::default() }), (true, vec![1, 3, 9]), "insert fails: the new item goes last");
+ assert_eq!(
+ swap(Fake { reject_new_insert: true, reject_new_append: true, ..Default::default() }),
+ (false, vec![1, 2, 3]),
+ "neither works: the old item is back where it was"
+ );
+ assert_eq!(
+ swap(Fake { reject_new_insert: true, reject_new_append: true, reject_old_insert: true, ..Default::default() }),
+ (false, vec![1, 3, 2]),
+ "the old item cannot go back in place either: it is appended"
+ );
+ }
+
+ #[test]
+ fn a_failed_native_call_is_retried_while_the_state_holds() {
+ let g = accelerator("G");
+ assert_eq!(settled(g, None, true), None, "the swap worked");
+ assert_eq!(settled(g, None, false), g, "it failed: the native item still has its key");
+ assert_eq!(settled(None, g, false), None, "restoring a key failed");
+ assert!(!sync_needed(true, false, true, false, false), "nothing changed");
+ assert!(sync_needed(true, false, false, false, false), "focus changed");
+ assert!(sync_needed(false, true, false, false, false), "recording changed");
+ assert!(sync_needed(true, false, true, false, true), "a swap failed last time: try again with the state unchanged");
+ }
+
#[test]
fn shortcuts_map_to_accelerators() {
assert_eq!(accelerator("Cmd+Shift+Z"), Some(Accelerator::new(Modifiers::META | Modifiers::SHIFT, Code::KeyZ)));
diff --git a/apps/lightcraft/src/window_gpu.rs b/apps/lightcraft/src/window_gpu.rs
index 1f84ec477..fefa65a51 100644
--- a/apps/lightcraft/src/window_gpu.rs
+++ b/apps/lightcraft/src/window_gpu.rs
@@ -93,7 +93,6 @@ pub fn choose(files: &Files) -> Chosen {
let compiled = wgpu::Instance::enabled_backend_features();
let os = std::env::consts::OS;
let order = backend::window_renderer_order(forced, remembered, failed, os, compiled, true);
- let mut notice = failed.map(|f| format!("The window could not start with its {} renderer last time.", f.name()));
let mut problems = Vec::new();
let chosen = if cfg!(windows) {
pick_probed(&order, &mut problems)
@@ -104,18 +103,7 @@ pub fn choose(files: &Files) -> Chosen {
Some((WindowRenderer::Wgpu(backend::window_backends()), None))
};
let (which, existing) = chosen.unwrap_or((WindowRenderer::Glow, None));
- if !problems.is_empty() {
- notice = Some(format!("{} {}", notice.unwrap_or_default(), problems.join(" ")).trim().to_string());
- }
- let notice = notice.map(|n| {
- format!(
- "{n} {app} now draws its window with {}. To choose another, start {app} with {var}=dx12, vulkan, gl or glow \
- (see docs/gpu-pipeline.md → Troubleshooting).",
- which.name(),
- app = "LightCraft",
- var = "LIGHTCRAFT_GPU_BACKEND",
- )
- });
+ let notice = user_notice(failed, forced, &problems, which);
log::info!("window: renderer {} (order {:?})", which.name(), order.iter().map(|r| r.name()).collect::>());
let mut wgpu_options = base_options();
let renderer = match (which, existing) {
@@ -134,6 +122,25 @@ pub fn choose(files: &Files) -> Chosen {
Chosen { renderer, wgpu_options, which, notice }
}
+/// What to tell the user about the window's renderer. A renderer that failed its probe is skipped
+/// silently (it is logged, and the one that worked is remembered): the app works, so a pop-up with
+/// wgpu's error only alarms (#1624). Shown: the last start died, or the forced renderer failed.
+fn user_notice(failed: Option, forced: Option, problems: &[String], which: WindowRenderer) -> Option {
+ let mut notice = failed.map(|f| format!("The window could not start with its {} renderer last time.", f.name()));
+ if forced.is_some() && !problems.is_empty() {
+ notice = Some(format!("{} {}", notice.unwrap_or_default(), problems.join(" ")).trim().to_string());
+ }
+ notice.map(|n| {
+ format!(
+ "{n} {app} now draws its window with {}. To choose another, start {app} with {var}=dx12, vulkan, gl or glow \
+ (see docs/gpu-pipeline.md → Troubleshooting).",
+ which.name(),
+ app = "LightCraft",
+ var = "LIGHTCRAFT_GPU_BACKEND",
+ )
+ })
+}
+
/// eframe's defaults with the app's backends and DX12 shader compiler (issues #136, #471).
pub fn base_options() -> WgpuConfiguration {
let mut c = WgpuConfiguration::default();
@@ -269,6 +276,20 @@ mod tests {
let _ = std::fs::remove_dir_all(&d);
}
+ /// #1624: a renderer skipped by the probe (DX12 → Vulkan) is no pop-up; a died start or a
+ /// failed forced renderer still is.
+ #[test]
+ fn probe_fallback_is_silent() {
+ let (dx, vk) = (WindowRenderer::Wgpu(Backends::DX12), WindowRenderer::Wgpu(Backends::VULKAN));
+ let problems = vec!["The dx12 renderer failed (Validation Error).".to_string()];
+ assert_eq!(user_notice(None, None, &problems, vk), None);
+ assert_eq!(user_notice(None, None, &[], vk), None);
+ let forced = user_notice(None, Some(dx), &problems, vk).unwrap();
+ assert!(forced.starts_with("The dx12 renderer failed") && forced.contains("now draws its window with vulkan"), "{forced}");
+ let died = user_notice(Some(dx), None, &problems, vk).unwrap();
+ assert!(died.contains("last time") && !died.contains("Validation Error"), "{died}");
+ }
+
/// The crash report: a pipeline that fails validation must come back as an error, never as
/// wgpu's default panic. Needs an adapter; skipped where there is none (CI without a GPU).
#[test]
diff --git a/contributors/people.toml b/contributors/people.toml
index 4eb002cef..0b7f9ee43 100644
--- a/contributors/people.toml
+++ b/contributors/people.toml
@@ -38,6 +38,9 @@ sync_github_name = true
[people.Losminek]
sync_github_name = true
+[people.nicolasfalesy]
+sync_github_name = true
+
[people.FooVoo]
real_name = "Aleksandr Batyrev"
sync_github_name = true
@@ -58,6 +61,9 @@ sync_github_name = false
real_name = "Ege Gursel"
sync_github_name = true
+[people.valenfys]
+sync_github_name = true
+
[people.mahmatias]
display_name = "Márcio Matias de Andrade"
sync_github_name = true
diff --git a/crates/catalog/src/dates.rs b/crates/catalog/src/dates.rs
index 2b71049ee..2dfebbf90 100644
--- a/crates/catalog/src/dates.rs
+++ b/crates/catalog/src/dates.rs
@@ -43,19 +43,63 @@ pub fn weekday(iso: &str) -> Option<&'static str> {
/// Shift an ISO time by `secs`, keeping anything after the seconds (fractions, zone) as it was.
/// A date without a time gets one. `None` if it doesn't parse, or if the result leaves years
/// 0000–9999 (an ISO date here has four year digits, so it couldn't be read back); `secs` may be
-/// anything, it never overflows.
+/// anything, it never overflows. Stricter than [`iso_seconds`], which reads what it can for sorting:
+/// a time cut short, seconds that aren't digits or anything after the zone is `None` here, not
+/// written back as a clean-looking time.
pub fn shift_iso(iso: &str, secs: i64) -> Option {
+ let iso = iso.trim();
+ let tail = iso.get(tail_start(iso)?..)?;
let t = iso_seconds(iso)?.checked_add(secs)?;
let (first, last) = (iso_seconds("0000-01-01T00:00:00")?, iso_seconds("9999-12-31T23:59:59")?);
if !(first..=last).contains(&t) {
return None;
}
- let iso = iso.trim();
- // the seconds are optional (`10:00+02:00`): without them the tail starts right after the minutes
- let tail = iso.get(if iso.get(16..17) == Some(":") { 19 } else { 16 }..).unwrap_or("");
Some(format!("{}{tail}", civil(t)))
}
+/// Where the part of an ISO time that [`shift_iso`] keeps as it was (fraction, zone) starts, or
+/// `None` unless `iso` is a date alone (`2026-01-01`) or a date and `HH:MM`, optional `:SS` with
+/// an optional fraction (`.5`), and an optional zone (`Z`, `+02:00`, `-0500`, `+02`), and nothing after.
+fn tail_start(iso: &str) -> Option {
+ let b = iso.as_bytes();
+ let digits = |x: &[u8]| !x.is_empty() && x.iter().all(u8::is_ascii_digit);
+ let digits_at = |a: usize, z: usize| b.get(a..z).is_some_and(digits);
+ let at = |i: usize, c: u8| b.get(i) == Some(&c);
+ if !(digits_at(0, 4) && at(4, b'-') && digits_at(5, 7) && at(7, b'-') && digits_at(8, 10)) {
+ return None;
+ }
+ if b.len() == 10 {
+ return Some(10);
+ }
+ if !((at(10, b'T') || at(10, b' ')) && digits_at(11, 13) && at(13, b':') && digits_at(14, 16)) {
+ return None;
+ }
+ // the seconds are optional (`10:00+02:00`): without them the tail starts right after the minutes
+ let start = if !at(16, b':') {
+ 16
+ } else if digits_at(17, 19) {
+ 19
+ } else {
+ return None;
+ };
+ let mut zone = b.get(start..)?;
+ // a fraction only after the seconds: `10:00.5` is half a minute, not half a second
+ if let ([b'.', rest @ ..], 19) = (zone, start) {
+ let n = rest.iter().take_while(|c| c.is_ascii_digit()).count();
+ zone = rest.get(n..)?;
+ if n == 0 {
+ return None;
+ }
+ }
+ let ok = match zone {
+ [] | [b'Z'] => true,
+ [b'+' | b'-', h1, h2, b':', m1, m2] => digits(&[*h1, *h2, *m1, *m2]),
+ [b'+' | b'-', hm @ ..] => matches!(hm.len(), 2 | 4) && digits(hm),
+ _ => false,
+ };
+ ok.then_some(start)
+}
+
/// Normalize user input to `YYYY-MM-DDTHH:MM:SS` (accepts a space instead of `T`, missing
/// seconds or time). `None` if it isn't a valid date.
pub fn normalize_iso(s: &str) -> Option {
@@ -260,6 +304,65 @@ mod tests {
assert_eq!(iso_seconds("2026-01-01T10:00+02:00"), iso_seconds("2026-01-01T10:00"));
}
+ #[test]
+ fn shift_refuses_a_time_it_cant_fully_read() {
+ for bad in [
+ "2026-01-01T10:0",
+ "2026-01-01T10",
+ "2026-01-01T",
+ "2026-01-01T10:00:xx",
+ "2026-01-01T10:00:0",
+ "2026-01-01T10:00:00+",
+ "2026-01-01T10:00:00+2",
+ "2026-01-01T10:00:00+02:0",
+ "2026-01-01T10:00:00.",
+ "2026-01-01T10:00:00.5.5",
+ "2026-01-01T10:00.5",
+ "2026-01-01T10:00:00ZZ",
+ "2026-01-01T10:00:00 and more",
+ "2026-01-01T10:00 and more",
+ "2026-01-01x10:00:00",
+ "2026/01/01T10:00:00",
+ ] {
+ assert_eq!(shift_iso(bad, 3600), None, "{bad} doesn't fully read, so it must not be written back");
+ }
+ // multi-byte characters where the time, the seconds and the zone start never panic
+ for at in [11, 16, 18, 21] {
+ let mut s = String::from("2026-01-01T10:00:00+02:00");
+ s.replace_range(at..at + 1, "é");
+ assert_eq!(shift_iso(&s, 3600), None, "{s}");
+ assert_eq!(shift_iso(s.get(..at + 2).unwrap(), 3600), None, "{s}");
+ }
+ }
+
+ #[test]
+ fn shift_keeps_a_fraction_and_zone_and_reads_back() {
+ for (from, to) in [
+ ("2026-01-01", "2026-01-01T01:00:00"),
+ (" 2026-01-01T10:00 ", "2026-01-01T11:00:00"),
+ ("2026-01-01 10:00:30", "2026-01-01T11:00:30"),
+ ("2026-01-01T10:00:30.5", "2026-01-01T11:00:30.5"),
+ ("2026-01-01T10:00:30Z", "2026-01-01T11:00:30Z"),
+ ("2026-01-01T10:00:30+02:00", "2026-01-01T11:00:30+02:00"),
+ ("2026-01-01T10:00:30-0500", "2026-01-01T11:00:30-0500"),
+ ("2026-01-01T10:00:30+02", "2026-01-01T11:00:30+02"),
+ ("2026-01-01T10:00:30.125+02:00", "2026-01-01T11:00:30.125+02:00"),
+ ] {
+ assert_eq!(shift_iso(from, 3600).as_deref(), Some(to), "{from}");
+ assert_eq!(shift_iso(to, 0).as_deref(), Some(to), "{to} reads back");
+ }
+ }
+
+ /// Sorting, grouping and date rules read with `iso_seconds`, which stays lenient: only the
+ /// write path refuses what it can't fully read.
+ #[test]
+ fn reading_stays_lenient_for_sorting() {
+ let at_ten = iso_seconds("2026-01-01T10:00:00");
+ assert_eq!(iso_seconds("2026-01-01T10:00:00 and more"), at_ten);
+ assert_eq!(iso_seconds("2026-01-01T10:00:xx"), at_ten);
+ assert_eq!(iso_seconds("2026-01-01T10:0"), iso_seconds("2026-01-01"));
+ }
+
#[test]
fn impossible_times_are_rejected_not_rolled_over() {
for bad in ["2026-04-01T10:75:00", "2026-04-01 10:00:75", "2026-04-01T10:60", "2026-04-01T24:00:00", "2026-04-01T10:59:60"] {
diff --git a/crates/catalog/src/folders.rs b/crates/catalog/src/folders.rs
index aab3b867b..e2d3b312d 100644
--- a/crates/catalog/src/folders.rs
+++ b/crates/catalog/src/folders.rs
@@ -207,14 +207,31 @@ impl Catalog {
/// folder that is no longer there and are dropped (undo restores them). Empty when there is
/// nothing to carry or drop.
pub fn folder_records_follow(&self, from: &str, to: &str) -> Vec {
+ self.folder_records_carried(from, to, true)
+ }
+
+ /// [`Catalog::folder_records_follow`] for a folder located where it already is on disk
+ /// (`folder.updateLocation`): nothing arrived at `to`, the library only learned that the
+ /// folder is there. So what it knows about folders at or below `to` (other folders of the
+ /// library there) stays; a record carried over replaces the one at the same place.
+ pub fn folder_records_located(&self, from: &str, to: &str) -> Vec {
+ self.folder_records_carried(from, to, false)
+ }
+
+ fn folder_records_carried(&self, from: &str, to: &str, clear_destination: bool) -> Vec {
let (Some(root), Some(dest)) = (record_key(from), record_key(to)) else { return Vec::new() };
if root == dest {
return Vec::new();
}
+ // a folder located below itself: what is already at or below `to` belongs to the folders
+ // of the library there, not to the folder located, so it is neither carried nor cleared
+ // (the record of `from` itself still lands at `to`, replacing the one there)
+ let nested = !clear_destination && key_within(&dest, &root);
+ let carried = |k: &str| key_within(k, &root) && !(nested && key_within(k, &dest));
let moved: Vec<(String, FolderRecord)> = self
.folder_records
.iter()
- .filter(|(k, _)| key_within(k, &root))
+ .filter(|(k, _)| carried(k))
.filter_map(|(k, r)| {
let rest = folder_rest(k, &root)?;
let base = dest.trim_end_matches('/');
@@ -224,7 +241,7 @@ impl Catalog {
.collect();
// what was known at the old place goes, and so does what is left at the new one (also
// when the moved folder carries nothing: the folder now there is not the one it was about)
- let stale = self.folder_records.keys().filter(|k| key_within(k, &root) || key_within(k, &dest));
+ let stale = self.folder_records.keys().filter(|k| carried(k) || (clear_destination && key_within(k, &dest)));
let mut ops: Vec = stale.map(|k| Op::SetFolderRecord { folder: k.clone(), record: None }).collect();
ops.extend(moved.into_iter().map(|(k, r)| Op::SetFolderRecord { folder: k, record: Some(r) }));
ops
diff --git a/crates/catalog/src/journal.rs b/crates/catalog/src/journal.rs
index edcfae22a..2600d794a 100644
--- a/crates/catalog/src/journal.rs
+++ b/crates/catalog/src/journal.rs
@@ -263,14 +263,36 @@ fn newer(what: String) -> CatalogError {
CatalogError::Newer(format!("{what}; this version reads catalog format v{VERSION} and older"))
}
+/// The most `resync` spends re-validating the suffixes of one line whose whole bytes are not valid
+/// UTF-8 (a real fragment ends at any byte): one line of a modest malformed log that holds a start
+/// of a record in every few bytes used to re-validate the whole remaining suffix at every attempt
+/// (CWE-407). Past the budget the rest of the line stays a torn tail, which the damaged path keeps
+/// and reports, same state as before, minus the CPU.
+const RESYNC_BUDGET_BYTES: usize = 64 << 20; // 64 MiB
+
/// A line that doesn't decode but ends with a whole record: a fragment of a failed append followed
/// by the record that retried it (the retry re-encodes the same ops with the same `seq`s). Returns
/// the fragment's length and the record, if that record continues the log (`seq <= last + 1`).
/// The record's CRC must match, so damage is never mistaken for it.
fn resync(line: &[u8], last: u64) -> Option<(usize, (u64, Op))> {
const START: &[u8] = b"{\"seq\":";
+ // One record per line, so a line of valid UTF-8 holds a valid suffix at every record start and
+ // one validation covers every candidate; only a line that is not wholly valid re-checks, and
+ // then up to the budget rather than the suffix length times the candidate count.
+ let whole = std::str::from_utf8(line).ok();
+ let mut spent = 0usize;
(1..line.len().saturating_sub(START.len() - 1)).filter(|&i| line[i..].starts_with(START)).find_map(|i| {
- let rec = std::str::from_utf8(&line[i..]).ok().map(|l| l.trim_end_matches('\r')).and_then(decode_record)?;
+ let text = match &whole {
+ Some(t) => &t[i..],
+ None => {
+ if spent >= RESYNC_BUDGET_BYTES {
+ return None;
+ }
+ spent = spent.saturating_add(line.len() - i);
+ std::str::from_utf8(&line[i..]).ok()?
+ }
+ };
+ let rec = decode_record(text.trim_end_matches('\r'))?;
(rec.0 <= last.saturating_add(1)).then_some((i, rec))
})
}
@@ -714,3 +736,28 @@ impl Drop for Journal {
}
}
}
+
+#[cfg(test)]
+mod tests_resync {
+ use super::*;
+
+ /// A torn first line may end at any byte and a failed append's retried records may follow its
+ /// own start `{"seq":` more than once: `resync` finds the CRC-valid record after a fragment of
+ /// many record starts. UTF-8 is validated once for a valid line, and for a line that isn't
+ /// wholly valid only up to the budget instead of the suffix length per candidate (CWE-407).
+ #[test]
+ fn resync_finds_the_retried_record_after_a_fragment_of_many_starts() {
+ let op = Op::Batch { ops: vec![] };
+ let record = encode_record(5, &op);
+ let mut line = Vec::from(b"\xf2" as &[u8]); // an invalid byte, so the whole line is not UTF-8
+ for i in 0..1024 {
+ line.extend_from_slice(format!("{{\"seq\":{},\"crc\":{{", i).as_bytes());
+ }
+ let fragment = line.len();
+ line.extend_from_slice(record.as_bytes());
+ assert_eq!(resync(&line, 4).map(|(at, (seq, _))| (at, seq)), Some((fragment, 5)));
+ // a retry is still recognised past `last`: the fragment logs nothing valid, but the CRC
+ // matches, so the double `Some` is only reached with `last >= 4`
+ assert_eq!(resync(&line, 100).map(|(at, (seq, _))| (at, seq)), Some((fragment, 5)));
+ }
+}
diff --git a/crates/catalog/src/keywords.rs b/crates/catalog/src/keywords.rs
index 0d8b666a1..6b0c97f5a 100644
--- a/crates/catalog/src/keywords.rs
+++ b/crates/catalog/src/keywords.rs
@@ -48,7 +48,11 @@ pub fn reparent(k: &str, from: &str, to: &str) -> String {
/// name, its parents' and synonyms flat besides its path). Each once, cleaned.
pub fn from_file(flat: &[String], hierarchical: &[String]) -> Vec {
let mut out: Vec = hierarchical.iter().map(|k| clean(k)).collect();
- out.extend(flat.iter().map(|k| clean(k)).filter(|k| !hierarchical.iter().any(|h| h.split(SEP).any(|level| same(level.trim(), k)))));
+ // One lowered set of every hierarchical level, not a search of `hierarchical` for each flat
+ // keyword: disjoint lists from sidecars made the old approach quadratic in both (CWE-407), and
+ // `same` allocates two lower-cased strings per comparison.
+ let levels: std::collections::HashSet = hierarchical.iter().flat_map(|h| h.split(SEP).map(|l| l.trim().to_lowercase())).collect();
+ out.extend(flat.iter().map(|k| clean(k)).filter(|k| !levels.contains(&k.to_lowercase())));
dedupe(&mut out);
out
}
@@ -78,6 +82,9 @@ pub fn parse_keyword_list(text: &str) -> Result> {
let mut out: Vec<(String, KeywordInfo)> = Vec::new();
// where each keyword is in `out`, by lower-case path (a search of `out` per line was quadratic)
let mut index: std::collections::HashMap = std::collections::HashMap::new();
+ // synonyms already accepted, by keyword (a search of each keyword's growing synonym list per
+ // line was quadratic and re-allocated both spellings every comparison — CWE-407)
+ let mut synonyms: std::collections::HashMap> = std::collections::HashMap::new();
// the keyword at each level above the line: its path and where it is in `out`
let mut above: Vec<(String, usize)> = Vec::new();
for (i, line) in text.split('\n').enumerate() {
@@ -103,9 +110,9 @@ pub fn parse_keyword_list(text: &str) -> Result> {
if synonym.contains(SEP) {
return Err(bad(n, "“|” separates a keyword's levels: a synonym can't hold one"));
}
- if let Some((_, info)) = out.get_mut(at)
- && !synonym.is_empty()
- && !info.synonyms.iter().any(|s| same(s, synonym))
+ if !synonym.is_empty()
+ && synonyms.entry(at).or_default().insert(synonym.to_lowercase())
+ && let Some((_, info)) = out.get_mut(at)
{
info.synonyms.push(synonym.to_string());
}
@@ -264,7 +271,14 @@ impl Catalog {
let mut chain: Vec = Vec::new();
let mut path = String::new();
let mut lower = String::new();
- for part in k.split(SEP).map(str::trim).filter(|s| !s.is_empty()) {
+ // Level paths hold their every prefix (`path` and `lower` both), so a photo's XMP
+ // keyword split far more levels than a list file accepts (`MAX_LEVELS`) made the
+ // stored prefixes quadratic in the level count: a keyword nests [MAX_LEVELS]
+ // levels here, as a list file would, and what lies below is left out (CWE-400).
+ for (part_n, part) in (1..).zip(k.split(SEP).map(str::trim).filter(|s| !s.is_empty())) {
+ if part_n > MAX_LEVELS {
+ break;
+ }
if !path.is_empty() {
path.push(SEP);
lower.push(SEP);
@@ -505,7 +519,24 @@ impl Catalog {
}
}
let mut file = KeywordListFile::default();
+ // The tree is capped at [`MAX_LEVELS`] (its level rows hold every level's prefix twice, so a
+ // synthetic keyword past that depth made the stored bytes quadratic); whatever lies deeper
+ // comes from the photos' and list's own spellings instead, named unwritable as before.
+ let mut over_deep: std::collections::HashSet = Default::default();
+ for k in self.listed_keywords().map(|k| &k.path) {
+ if k.split(SEP).count() > MAX_LEVELS {
+ over_deep.insert(k.clone());
+ }
+ }
+ for p in self.photos().filter(|p| p.in_library()) {
+ for k in &p.meta.keywords {
+ if clean(k).split(SEP).count() > MAX_LEVELS {
+ over_deep.insert(k.clone());
+ }
+ }
+ }
write(self, &self.keyword_tree(), 0, &mut file);
+ file.unwritable.extend(over_deep);
file
}
@@ -1470,4 +1501,35 @@ mod tests {
let s = c.keyword_suggestions(&[], "E", 5);
assert_eq!(s, ["eagle", "beach", "parade"], "prefix matches before substring matches");
}
+
+ /// A flat keyword already covered by a hierarchical level only in another spelling is not kept
+ /// again, whatever the size of either list (the per-flat-keyword search of every level, twice
+ /// lower-cased per comparison, was quadratic: CWE-407).
+ #[test]
+ fn from_file_matches_levels_in_any_case() {
+ let flat = ["Rome".to_string(), "Seaside".to_string()];
+ let hierarchical = ["travel|ITALY|rome".to_string()];
+ assert_eq!(from_file(&flat, &hierarchical), ["travel|ITALY|rome", "Seaside"]);
+ }
+
+ /// The same synonym, in any spelling, is kept once (the search of each keyword's growing
+ /// synonym list per line was quadratic, with lower-cased copies made per comparison).(CWE-407)
+ #[test]
+ fn parse_keyword_list_keeps_a_synonym_once() {
+ let read = parse_keyword_list("Travel\n\t{Trip}\n\t{journey}\n\t{TRIP}\n").unwrap();
+ assert_eq!(read.len(), 1);
+ assert_eq!(read[0].1.synonyms, ["Trip", "journey"]);
+ }
+
+ /// A keyword split into far more levels than a list file accepts is cut at [`MAX_LEVELS`] in the
+ /// tree, whose level rows hold every prefix twice: unbounded it made the stored bytes quadratic
+ /// in the level count of one short keyword (CWE-400).
+ #[test]
+ fn keyword_tree_caps_a_keyword_at_the_list_depth() {
+ let deep: Vec = (0..4096).map(|i| format!("k{i}")).collect();
+ let joined = deep.join("|");
+ let (c, _) = lib(&[&[joined.as_str()]]);
+ let deepest = tree_paths(&c.keyword_tree()).iter().map(|(p, _)| p.split(SEP).count()).max().unwrap_or(0);
+ assert_eq!(deepest, MAX_LEVELS);
+ }
}
diff --git a/crates/catalog/src/lib.rs b/crates/catalog/src/lib.rs
index 1776c895c..7e094200c 100644
--- a/crates/catalog/src/lib.rs
+++ b/crates/catalog/src/lib.rs
@@ -36,7 +36,7 @@ pub use folders::{FolderNode, FolderRecord};
pub use journal::{Journal, LoadReport, PersistStats, SnapshotPolicy, SnapshotTiming};
pub use keywords::KeywordNode;
use lightcraft_develop::DevelopSettings;
-pub use local::{DEFAULT_FORGET_DAYS, ForgetPlan, folder_of};
+pub use local::{DEFAULT_FORGET_DAYS, ForgetPlan, LocalRecords, folder_of};
pub use lock::{LibraryLock, LockError, LockOwner};
pub use model::*;
pub use query::{DateGroup, Filter, Person, RatingOp, Sort, SortKey, mix64};
@@ -284,7 +284,8 @@ pub struct Catalog {
/// What the library keeps about its folders (folder identity → record), see [`folders`].
#[serde(default, skip_serializing_if = "BTreeMap::is_empty")]
folder_records: BTreeMap,
- /// Increments on every applied op.
+ /// Increments on every applied op, and when a batch fails and its rollback cannot undo it whole (the catalog
+ /// changed, so what is keyed by the revision is out of date).
#[serde(skip)]
pub revision: u64,
/// Which photos the latest ops touched ([`Catalog::touched_since`]).
@@ -348,6 +349,8 @@ struct Changes {
/// outside (a library just loaded takes a range of its own, so that nothing kept from another library is
/// taken for this one's): nothing is known of how the catalog got to where it is.
seen: u64,
+ /// A batch that failed could not be rolled back whole: what it changed stays, so what it noted must too.
+ torn: bool,
}
impl PartialEq for Changes {
@@ -610,13 +613,40 @@ impl Catalog {
self.albums.get_mut(&id).ok_or(CatalogError::NoAlbum(id))
}
- /// Apply an op; returns its inverse. On error nothing changes.
+ /// Apply an op; returns its inverse. On error the catalog is as it was, the revision included, and nothing is
+ /// remembered of the op in what the latest ops touched. The one exception is a batch whose rollback fails: the
+ /// catalog did change, so the revision advances and anything may be taken to have changed.
pub fn apply(&mut self, op: Op) -> Result {
if self.changes.seen != self.revision {
// the revision was set from outside: what is remembered is of another past
self.changes = Changes { floor: self.revision, seen: self.revision, ..Default::default() };
}
- let inv = self.apply_inner(op)?;
+ let broad = self.changes.broad;
+ self.changes.torn = false;
+ let inv = match self.apply_inner(op) {
+ Ok(inv) => inv,
+ Err(e) => {
+ let made = self.revision.saturating_add(1);
+ if self.changes.torn {
+ // (a batch the rollback of which failed changed something and says not what: it makes a
+ // revision, so what is kept for the one before is dropped, and anything may have changed, as
+ // after an op of a wider reach)
+ self.changes.torn = false;
+ self.changes.broad = made;
+ self.revision = made;
+ self.changes.seen = made;
+ } else {
+ // (everything the failed op, or the rollback of a batch, noted was made for the revision it
+ // would have made: nothing earlier has that one. A note the cap already dropped stays
+ // dropped, which only says less than is known, as the cap does.)
+ while self.changes.touched.back().is_some_and(|(r, _, _)| *r == made) {
+ self.changes.touched.pop_back();
+ }
+ self.changes.broad = broad;
+ }
+ return Err(e);
+ }
+ };
self.revision += 1;
self.changes.seen = self.revision;
Ok(inv)
@@ -962,7 +992,9 @@ impl Catalog {
Err(e) => {
// roll back what was applied
for inv in inverses.into_iter().rev() {
- let _ = self.apply_inner(inv);
+ if self.apply_inner(inv).is_err() {
+ self.changes.torn = true;
+ }
}
return Err(e);
}
@@ -1067,21 +1099,50 @@ impl Catalog {
st.photos.len() >= 2
});
let parents: Vec<(AlbumId, Option)> = self.albums.values().map(|a| (a.id, a.parent)).collect();
+ // A folder chain is shared by every album under it, so each chain is validated once and its
+ // verdict is what later albums reused: a valid chain encoded in a flat snapshot made the old
+ // per-album walk quadratic in the album count (CWE-407).
+ enum Chain {
+ Ok,
+ Bad,
+ }
+ let mut chains: std::collections::HashMap = std::collections::HashMap::new();
for (id, parent) in parents {
let Some(p) = parent else { continue };
- let mut cur = Some(p);
- let mut steps = 0usize;
- let ok = loop {
- match cur {
- None => break true,
- Some(c) if c == id || steps > self.albums.len() => break false,
- Some(c) => match self.albums.get(&c) {
- Some(a) if a.folder => cur = a.parent,
- _ => break false,
- },
+ if chains.contains_key(&id) {
+ continue;
+ }
+ // Walk up from `p` once, recording everything seen; what it reaches decides every node
+ // walked (a chain already judged by an earlier album stops the walk early — the rest of
+ // the chain was judged then).
+ let mut walked: Vec = Vec::new();
+ let ok = {
+ let mut cur = Some(p);
+ let mut on: std::collections::HashSet = std::collections::HashSet::new();
+ loop {
+ match cur {
+ None => break true,
+ Some(c) => {
+ if c == id || !on.insert(c) {
+ break false;
+ }
+ match chains.get(&c) {
+ Some(Chain::Bad) => break false,
+ Some(Chain::Ok) => break true,
+ None => {}
+ }
+ match self.albums.get(&c) {
+ Some(a) if a.folder => walked.push(c),
+ _ => break false,
+ }
+ cur = self.albums[&c].parent;
+ }
+ }
}
- steps += 1;
};
+ for c in walked {
+ chains.insert(c, if ok { Chain::Ok } else { Chain::Bad });
+ }
if !ok && let Some(a) = self.albums.get_mut(&id) {
changed.push(format!("album {}: its folder {} is missing, not a folder or inside it: moved to the top level", id.0, p.0));
a.parent = None;
@@ -1164,4 +1225,6 @@ mod tests_repair;
#[cfg(test)]
mod tests_scale;
#[cfg(test)]
+mod tests_tally;
+#[cfg(test)]
mod tests_torn_append;
diff --git a/crates/catalog/src/local.rs b/crates/catalog/src/local.rs
index 093c22427..20cb3d5b4 100644
--- a/crates/catalog/src/local.rs
+++ b/crates/catalog/src/local.rs
@@ -34,6 +34,15 @@ use serde::Serialize;
use crate::{Catalog, Op, Photo, PhotoId, Source};
+/// The Local records of some files ([`Catalog::local_records_of`]).
+#[derive(Clone, Debug, Default, PartialEq, Eq)]
+pub struct LocalRecords {
+ /// Only a record of having looked: nothing was changed on them and nothing refers to them.
+ pub untouched: Vec,
+ /// Holding something of the person's, with their files.
+ pub kept: Vec<(PhotoId, String)>,
+}
+
/// Default for how long a Local folder may go unbrowsed before its untouched records are
/// forgotten (days).
pub const DEFAULT_FORGET_DAYS: u32 = 30;
@@ -64,7 +73,8 @@ pub fn folder_of(p: &Photo) -> Option {
// `/` and `\` are both separators whatever the platform (a catalog can move between them)
Source::File { path } => {
let key = crate::query::folder_key(path);
- key.rsplit_once('/').map(|(dir, _)| if dir.is_empty() { "/".to_string() } else { dir.to_string() })
+ // a drive or share root keeps its trailing `/` in its key (`c:/`, `//srv/share/`)
+ key.rsplit_once('/').map(|(dir, _)| if dir.is_empty() { "/".to_string() } else { crate::query::folder_key(&format!("{dir}/")) })
}
Source::Demo { .. } => None,
}
@@ -176,6 +186,34 @@ impl Catalog {
&self.browsed
}
+ /// The Local records of `files` (paths as stored), for library photos about to point at
+ /// those files (a folder located where it was browsed, `folder.updateLocation`): a file has one
+ /// record, so the Local one gives way. Told apart: those that are only a record of having
+ /// looked (untouched, see the module docs, and nothing refers to them), which can go, and
+ /// those that hold something of the person's (a rating, an edit, a place in an album), with
+ /// their files. Records in Recently Deleted are not Local records to this.
+ pub fn local_records_of(&self, files: &HashSet) -> LocalRecords {
+ let mut out = LocalRecords::default();
+ if files.is_empty() {
+ return out;
+ }
+ let referenced = self.referenced();
+ for p in self.photos.values().filter(|p| p.local && !p.deleted) {
+ let Source::File { path } = &p.source else { continue };
+ if !files.contains(path) {
+ continue;
+ }
+ if p.untouched_local() && !referenced.contains(&p.id) {
+ out.untouched.push(p.id);
+ } else {
+ out.kept.push((p.id, path.clone()));
+ }
+ }
+ out.untouched.sort_unstable();
+ out.kept.sort_unstable();
+ out
+ }
+
/// Plan forgetting untouched Local records whose folder wasn't browsed for `days` days (as of
/// `now`, ISO 8601). `in_use` keeps records the caller still needs. `days == 0` (never) or an
/// unreadable `now` forgets nothing. See the module docs.
diff --git a/crates/catalog/src/lock.rs b/crates/catalog/src/lock.rs
index d699da049..f701087a6 100644
--- a/crates/catalog/src/lock.rs
+++ b/crates/catalog/src/lock.rs
@@ -25,7 +25,7 @@
//! - Browser storage (OPFS) has no file locks: see the web host for its own guard.
use std::fs::{File, OpenOptions, TryLockError};
-use std::io;
+use std::io::{self, Write};
use std::path::{Path, PathBuf};
/// The lock file in the library directory.
@@ -116,9 +116,11 @@ impl LibraryLock {
}
Err(TryLockError::Error(e)) => return Ok(unlocked(e)),
}
+ // A new file in place of whatever is at the name: a link planted there in a library folder
+ // someone else prepared is removed, never written through (as for the staging files, #1502).
let owner = LockOwner::this_process(program);
if let Ok(json) = serde_json::to_vec_pretty(&owner)
- && let Err(e) = std::fs::write(dir.join(OWNER), json)
+ && let Err(e) = crate::store::create_staging(&dir.join(OWNER)).and_then(|mut f| f.write_all(&json))
{
log::warn!("library {}: can't write {OWNER}: {e}", dir.display());
}
diff --git a/crates/catalog/src/query.rs b/crates/catalog/src/query.rs
index a94421369..ff94268a8 100644
--- a/crates/catalog/src/query.rs
+++ b/crates/catalog/src/query.rs
@@ -29,6 +29,10 @@ pub struct Filter {
/// Only these photos (Find Similar results…); empty = no constraint.
#[serde(skip_serializing_if = "Vec::is_empty")]
pub only: Vec,
+ /// Show nothing: an explicit empty result (Find Similar with no matches), since an empty
+ /// `only` means "no constraint".
+ #[serde(skip_serializing_if = "std::ops::Not::not")]
+ pub only_none: bool,
/// Any of these labels (the filter bar's multi-select); empty = no constraint.
#[serde(skip_serializing_if = "Vec::is_empty")]
pub labels: Vec,
@@ -236,6 +240,17 @@ fn has_person(p: &Photo, name: &str) -> bool {
}
impl Filter {
+ /// The labels that `label` and `labels` let through together (they AND), in `labels`' order;
+ /// `None` when neither is set, empty when they exclude each other. The chip and the filter
+ /// bar's buttons show this, so they match what is filtered.
+ pub fn labels_shown(&self) -> Option> {
+ match (self.label, self.labels.is_empty()) {
+ (None, true) => None,
+ (Some(l), true) => Some(vec![l]),
+ (l, false) => Some(self.labels.iter().copied().filter(|x| l.is_none_or(|l| l == *x)).collect()),
+ }
+ }
+
/// Human-readable summary of the active rules (smart album tooltips, `album.list`).
pub fn describe(&self) -> String {
self.describe_in(None)
@@ -374,6 +389,9 @@ impl Filter {
if !self.labels.is_empty() && !p.label.is_some_and(|l| self.labels.contains(&l)) {
return false;
}
+ if self.only_none {
+ return false;
+ }
if !self.only.is_empty() && !prepared.only.as_ref().map_or_else(|| self.only.contains(&p.id), |only| only.contains(&p.id)) {
return false;
}
diff --git a/crates/catalog/src/rules.rs b/crates/catalog/src/rules.rs
index 667509d6f..2976a2659 100644
--- a/crates/catalog/src/rules.rs
+++ b/crates/catalog/src/rules.rs
@@ -432,12 +432,79 @@ thread_local! {
std::cell::RefCell::new(std::collections::HashMap::new());
}
+thread_local! {
+ /// While [`AlbumPass`] is alive on this thread: the photos of each smart album that was counted whole, with
+ /// the longest chain of smart albums (itself included) its rules lead through.
+ static PASS: std::cell::RefCell