mirror of
https://github.com/storytold/lightcraft.git
synced 2026-10-31 21:48:53 -06:00
LR-EDIT-COLOR-TEMP: a one-field white balance partial lands where a mode answers its own white (#2910)
Reviewing #2952 turned up two more shapes of the same bug, both on the path this branch is about. `effective_wb` ignores the temp / tint stored on an `As Shot` white balance — it answers the file's as-shot white — and on a raw edited in Kelvin it also ignores a named preset mode's stored numbers, through `WbMode::preset`. So a one-field partial that names its scale, `wb {temp: 5800, scale: "kelvin"}`, merged into such a photo, was stored and then read by nothing: the panel shows 5800 K while the render stays at as shot, and the stored tint no longer matches what renders. An unmarked partial already materialized a Custom value through the relative-scale conversion; the named-scale ones never did. The photo's preparation now writes out the white the photo really renders as Custom values before the partial lands, for `As Shot` and for preset modes on a Kelvin source. `Auto` keeps its mode — its gray is resolved and stored when the mode is set, not at render time — and a partial that names a `mode` itself stays authoritative. Sources without the Kelvin scale (a raw whose camera has no matrices, a rendered file) had the same swallow, and the number reaches their render now. The issue asks for one photo-aware merge in every caller that writes settings onto a photo, and names preset apply. `preset.apply` ran `Preset::apply` → `apply_partial` → `DevelopSettings::merged` and never saw the photo's scale, so a preset authored by hand naming one white balance number still had the original bug; measured on a photo at 5000 K / +12, it landed the photo on the relative scale and moved the tint it never mentioned. The preparation and the normalize-back now wrap `Preset::apply` per photo, with the amount blending and the mask merging untouched, which also puts an amount below 100 % on a single scale. Presets the app writes (`preset.create` → `extract_groups`) and the crs / XMP importers name mode, temp, tint and scale together and so land in the branch that behaves exactly as before. Tests: the `As Shot` and preset-mode cases in the #2910 merge test, and the new `preset.apply` test, fail before this commit (the first stores the number in a mode that renders the photo's old white; the second demotes the photo to `Legacy`); the same test then pins that `preset.apply` and `develop.merge` agree on a one-field white balance, which is the point of having one merge. Docs: the rule for an agent is in `docs/control-protocol.md` and `docs/mcp.md`, recorded on the `LR-EDIT-COLOR-TEMP` row of `docs/parity-checklist.md`, and in `docs/whats-new.md`. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -51,6 +51,54 @@ fn refit_crop_angle(touched: bool, before: CropGeometry, d: &mut DevelopSettings
|
||||
|
||||
const CROP_ANGLE: &str = "crop.angle";
|
||||
|
||||
/// Whether a white balance mode renders its own white instead of the numbers stored on the photo: `As Shot`
|
||||
/// renders the source's as-shot white, and on a raw edited in Kelvin a named preset renders its own Kelvin / tint
|
||||
/// ([`WbMode::preset`]). Either way a number written into `temp` / `tint` is stored but read by nothing, so the
|
||||
/// panel shows the number while the render uses the mode's white. `Custom` reads its stored numbers, and so does
|
||||
/// `Auto` on such a raw: its white is resolved and stored when the mode is set (`develop.wb`), not at render time.
|
||||
fn wb_mode_ignores_its_numbers(mode: WbMode, info: &lightcraft_pipeline::SourceInfo) -> bool {
|
||||
mode == WbMode::AsShot || (info.raw && !info.relative_wb && mode.preset().is_some())
|
||||
}
|
||||
|
||||
/// Bring a photo's settings onto the scale and mode a white balance partial names, so a one-field partial means
|
||||
/// what it says (issue #2910, and its review). `None` when the partial needs no such preparation: it names both
|
||||
/// numbers, neither, or a mode of its own. [`merge_settings_for_photo`] and `preset.apply` both start here, and
|
||||
/// both finish with `normalize_wb` so the photo ends on the scale it is edited on.
|
||||
pub(crate) fn prepare_photo_for_wb_partial(d: &DevelopSettings, partial: &Value, info: &lightcraft_pipeline::SourceInfo) -> Option<DevelopSettings> {
|
||||
let mut d = (*d).clone();
|
||||
let wb = partial.get("wb").and_then(Value::as_object);
|
||||
// the partial names one of the two numbers, and not both or neither
|
||||
let one_field = wb.is_some_and(|wb| wb.contains_key("temp") != wb.contains_key("tint"));
|
||||
if !one_field {
|
||||
return None;
|
||||
}
|
||||
// a photo whose source is edited in Kelvin is the one with two scales to trip over
|
||||
let kelvin_source = info.raw && !info.relative_wb;
|
||||
// the scale the partial's numbers are written on: an unmentioned scale means the relative scale, as `merged` reads it
|
||||
let partial_scale = match wb.as_ref().and_then(|wb| wb.get("scale")).and_then(Value::as_str) {
|
||||
Some("kelvin") => lightcraft_develop::WbScale::Kelvin,
|
||||
_ => lightcraft_develop::WbScale::Legacy,
|
||||
};
|
||||
// put the photo onto the scale it is edited on: its values marked Kelvin, or a look still on the relative
|
||||
// scale converted to the Kelvin / tint that renders the same
|
||||
lightcraft_pipeline::local::normalize_wb(&mut d, info);
|
||||
// a mode that answers its own white would swallow the partial's number: write the white the photo really
|
||||
// renders as Custom values, so the number means what it says and the other field carries that white's look
|
||||
if !wb.is_some_and(|wb| wb.contains_key("mode")) && wb_mode_ignores_its_numbers(d.wb.mode, info) {
|
||||
let (t, tint) = lightcraft_pipeline::local::effective_wb(info, &d);
|
||||
(d.wb.mode, d.wb.temp, d.wb.tint) = (WbMode::Custom, t, tint);
|
||||
}
|
||||
if kelvin_source && partial_scale == lightcraft_develop::WbScale::Legacy {
|
||||
// and onto the partial's scale, so the field the partial leaves out is the photo's own look read there
|
||||
let as_shot = (info.as_shot_temp, info.as_shot_tint);
|
||||
let (kt, ktint) = lightcraft_pipeline::local::effective_wb(info, &d);
|
||||
if let Some((lt, ltint)) = lightcraft_pipeline::local::kelvin_to_legacy(kt, ktint, as_shot) {
|
||||
(d.wb.mode, d.wb.temp, d.wb.tint, d.wb.scale) = (WbMode::Custom, lt, ltint, lightcraft_develop::WbScale::Legacy);
|
||||
}
|
||||
}
|
||||
Some(d)
|
||||
}
|
||||
|
||||
/// Merge a partial settings JSON into a photo's settings (issue #2910). `DevelopSettings::merged` keeps a white
|
||||
/// balance that names no scale on the relative scale; on a photo edited in Kelvin the field the partial does not
|
||||
/// name (a partial with only `temp`, or only `tint`) would then be read on the other scale, giving a white neither
|
||||
@@ -58,36 +106,19 @@ const CROP_ANGLE: &str = "crop.angle";
|
||||
/// first: the photo's white balance is brought onto the partial's scale with the photo's as-shot white
|
||||
/// (`kelvin_to_legacy` / `legacy_to_kelvin` via `normalize_wb`), the partial merges unchanged, and the result lands
|
||||
/// back on the photo's editing scale (`normalize_wb`), where the kept field carries the look the photo had.
|
||||
/// Partials naming both fields, or neither, mean the same as before; so do presets and the crs / Luminar importers,
|
||||
/// which always write `temp`, `tint` and `scale` together through their own paths.
|
||||
/// Partials naming both fields, or neither, mean the same as before. A photo whose white balance mode renders its
|
||||
/// own white — `As Shot`, or a named preset on a raw edited in Kelvin — has that white written out as Custom values
|
||||
/// first ([`wb_mode_ignores_its_numbers`]), because a mode that answers its own white would otherwise store the
|
||||
/// partial's number without any render reading it; a partial that names a `mode` itself stays authoritative.
|
||||
pub(crate) fn merge_settings_for_photo(
|
||||
d: &DevelopSettings,
|
||||
partial: &Value,
|
||||
info: &lightcraft_pipeline::SourceInfo,
|
||||
) -> std::result::Result<DevelopSettings, serde_json::Error> {
|
||||
let mut d = (*d).clone();
|
||||
let wb = partial.get("wb").and_then(Value::as_object);
|
||||
let one_field = wb.is_some_and(|wb| wb.contains_key("temp") != wb.contains_key("tint"));
|
||||
// only a photo whose source is edited in Kelvin has two scales to trip over
|
||||
let kelvin_source = one_field && info.raw && !info.relative_wb;
|
||||
if kelvin_source {
|
||||
// the scale the partial's numbers are written on: an unmentioned scale means the relative scale, as `merged` reads it
|
||||
let partial_scale = match wb.and_then(|wb| wb.get("scale")).and_then(Value::as_str) {
|
||||
Some("kelvin") => lightcraft_develop::WbScale::Kelvin,
|
||||
_ => lightcraft_develop::WbScale::Legacy,
|
||||
};
|
||||
// put the photo onto the Kelvin scale (converting a relative look once), then onto the partial's
|
||||
lightcraft_pipeline::local::normalize_wb(&mut d, info);
|
||||
if d.wb.scale != partial_scale && partial_scale == lightcraft_develop::WbScale::Legacy {
|
||||
let as_shot = (info.as_shot_temp, info.as_shot_tint);
|
||||
let (kt, ktint) = lightcraft_pipeline::local::effective_wb(info, &d);
|
||||
if let Some((lt, ltint)) = lightcraft_pipeline::local::kelvin_to_legacy(kt, ktint, as_shot) {
|
||||
(d.wb.mode, d.wb.temp, d.wb.tint, d.wb.scale) = (WbMode::Custom, lt, ltint, lightcraft_develop::WbScale::Legacy);
|
||||
}
|
||||
}
|
||||
}
|
||||
let mut out = d.merged(partial)?;
|
||||
if kelvin_source {
|
||||
let prepared = prepare_photo_for_wb_partial(d, partial, info);
|
||||
let on_kelvin_scale = prepared.is_some() && info.raw && !info.relative_wb;
|
||||
let mut out = prepared.as_ref().unwrap_or(d).merged(partial)?;
|
||||
if on_kelvin_scale {
|
||||
// and the result back onto the scale this photo is edited on
|
||||
lightcraft_pipeline::local::normalize_wb(&mut out, info);
|
||||
}
|
||||
@@ -1061,7 +1092,20 @@ pub fn specs() -> Vec<CommandSpec> {
|
||||
let ops = ids
|
||||
.iter()
|
||||
.filter_map(|id| s.develop_of(*id).map(|d| (*id, d)))
|
||||
.filter_map(|(id, d)| s.develop_op(id, preset.apply(&d, amount), &format!("Preset: {}", preset.name)))
|
||||
.filter_map(|(id, d)| {
|
||||
// a hand-written preset may name one white balance number only, and then it means the same as
|
||||
// a partial: bring the photo onto that scale first, with its own as-shot white, and put the
|
||||
// result back on the scale the photo is edited on (issue #2910) — the merge `develop.merge` uses
|
||||
let info = s.source_info(id);
|
||||
let prepared = prepare_photo_for_wb_partial(&d, &preset.settings, &info);
|
||||
let on_kelvin_scale = prepared.is_some() && info.raw && !info.relative_wb;
|
||||
let base: &DevelopSettings = prepared.as_ref().unwrap_or(&d);
|
||||
let mut next = preset.apply(base, amount);
|
||||
if on_kelvin_scale {
|
||||
lightcraft_pipeline::local::normalize_wb(&mut next, &info);
|
||||
}
|
||||
s.develop_op(id, next, &format!("Preset: {}", preset.name))
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
s.commit(&format!("Preset: {}", preset.name), Op::Batch { ops })?;
|
||||
ok()
|
||||
|
||||
@@ -296,6 +296,64 @@ fn one_field_wb_partial_merge_keeps_the_other_field_on_its_scale() {
|
||||
nearly(&d.wb, &l2k(5800.0, 0.0), "the as-shot tint keeps its meaning");
|
||||
assert_eq!(d.wb.scale, WbScale::Kelvin);
|
||||
|
||||
// A mode that answers its own white (As Shot, or a named preset on a raw edited in Kelvin) ignores the numbers
|
||||
// stored on the photo, so a one-field partial naming its scale landed on a white balance that stored the number
|
||||
// where no render would read it: the panel showed 5800 while the photo stayed at as shot. The white the photo
|
||||
// really renders is written out as Custom values first, so the named number means what it says and the field the
|
||||
// partial leaves out carries that same white (review of #2952).
|
||||
let as_shot_photo = DevelopSettings {
|
||||
wb: lightcraft_develop::WhiteBalance { mode: WbMode::AsShot, temp: 4986.0, tint: -2.0, scale: WbScale::Kelvin },
|
||||
..DevelopSettings::for_raw(as_shot.0, as_shot.1)
|
||||
};
|
||||
s.set_develop(kelvin, as_shot_photo.clone(), "as shot").unwrap();
|
||||
assert_eq!(lightcraft_engine_effective(&s.source_info(kelvin), &as_shot_photo), as_shot, "as shot renders the file's own white");
|
||||
s.execute("develop.merge", &json!({"settings": {"wb": {"temp": 5800.0, "scale": "kelvin"}}})).unwrap();
|
||||
let d = dev(&s, kelvin);
|
||||
assert_eq!((d.wb.mode, d.wb.temp, d.wb.tint, d.wb.scale), (WbMode::Custom, 5800.0, -2.0, WbScale::Kelvin), "{:?}", d.wb);
|
||||
assert_eq!(lightcraft_engine_effective(&s.source_info(kelvin), &d), (5800.0, -2.0), "the temperature the partial asked for renders");
|
||||
// the tint side of the same hole
|
||||
s.set_develop(kelvin, as_shot_photo.clone(), "as shot").unwrap();
|
||||
s.execute("develop.merge", &json!({"settings": {"wb": {"tint": 20.0, "scale": "kelvin"}}})).unwrap();
|
||||
let d = dev(&s, kelvin);
|
||||
assert_eq!((d.wb.mode, d.wb.temp, d.wb.tint, d.wb.scale), (WbMode::Custom, as_shot.0, 20.0, WbScale::Kelvin), "{:?}", d.wb);
|
||||
assert_eq!(lightcraft_engine_effective(&s.source_info(kelvin), &d), (as_shot.0, 20.0));
|
||||
// a named preset mode answers its own Kelvin white on such a raw, so its numbers are swallowed the same way
|
||||
let preset_photo = DevelopSettings {
|
||||
wb: lightcraft_develop::WhiteBalance { mode: WbMode::Cloudy, temp: 4986.0, tint: -2.0, scale: WbScale::Kelvin },
|
||||
..DevelopSettings::for_raw(as_shot.0, as_shot.1)
|
||||
};
|
||||
s.set_develop(kelvin, preset_photo.clone(), "cloudy").unwrap();
|
||||
let cloudy = lightcraft_engine_effective(&s.source_info(kelvin), &preset_photo);
|
||||
assert_ne!(cloudy, (4986.0, -2.0), "the preset mode renders its own white, not the stored numbers");
|
||||
s.execute("develop.merge", &json!({"settings": {"wb": {"temp": 5800.0, "scale": "kelvin"}}})).unwrap();
|
||||
let d = dev(&s, kelvin);
|
||||
assert_eq!((d.wb.mode, d.wb.temp, d.wb.tint, d.wb.scale), (WbMode::Custom, 5800.0, cloudy.1, WbScale::Kelvin), "{:?}", d.wb);
|
||||
assert_eq!(lightcraft_engine_effective(&s.source_info(kelvin), &d), (5800.0, cloudy.1), "the kept tint is that white's own tint");
|
||||
// `Auto` resolves its gray when the mode is set and reads its stored numbers back: its mode stays, nothing frozen
|
||||
let auto_photo = DevelopSettings {
|
||||
wb: lightcraft_develop::WhiteBalance { mode: WbMode::Auto, temp: 5100.0, tint: 3.0, scale: WbScale::Kelvin },
|
||||
..DevelopSettings::for_raw(as_shot.0, as_shot.1)
|
||||
};
|
||||
s.set_develop(kelvin, auto_photo, "auto").unwrap();
|
||||
s.execute("develop.merge", &json!({"settings": {"wb": {"temp": 5800.0, "scale": "kelvin"}}})).unwrap();
|
||||
let d = dev(&s, kelvin);
|
||||
assert_eq!((d.wb.mode, d.wb.temp, d.wb.tint), (WbMode::Auto, 5800.0, 3.0), "{:?}", d.wb);
|
||||
// a partial that names a mode stays authoritative: naming `asShot` puts the photo back on the file's white
|
||||
s.set_develop(kelvin, as_shot_photo.clone(), "as shot").unwrap();
|
||||
s.execute("develop.merge", &json!({"settings": {"wb": {"mode": "asShot", "temp": 5800.0, "scale": "kelvin"}}})).unwrap();
|
||||
let d = dev(&s, kelvin);
|
||||
assert_eq!((d.wb.mode, lightcraft_engine_effective(&s.source_info(kelvin), &d)), (WbMode::AsShot, as_shot), "{:?}", d.wb);
|
||||
// the same swallow on a source without the Kelvin scale (a raw with no matrices): the number was stored and read
|
||||
// by nothing there too, and now reaches the render
|
||||
s.execute("library.select", &json!({"ids": [relative.0], "active": relative.0})).unwrap();
|
||||
let before = dev(&s, relative);
|
||||
assert_eq!(lightcraft_engine_effective(&s.source_info(relative), &before), (6500.0, 0.0), "as shot on this body is 6500 / 0");
|
||||
s.execute("develop.merge", &json!({"settings": {"wb": {"temp": 5800.0}}})).unwrap();
|
||||
let d = dev(&s, relative);
|
||||
assert_eq!((d.wb.mode, d.wb.temp, d.wb.tint, d.wb.scale), (WbMode::Custom, 5800.0, 0.0, WbScale::Legacy), "{:?}", d.wb);
|
||||
assert_eq!(lightcraft_engine_effective(&s.source_info(relative), &d), (5800.0, 0.0), "the number the partial named renders");
|
||||
s.execute("library.select", &json!({"ids": [kelvin.0], "active": kelvin.0})).unwrap();
|
||||
|
||||
// the other direction: an old relative Custom value (no scale) plus a Kelvin temp-only partial — the photo's
|
||||
// white balance is taken onto the Kelvin scale first, so the kept tint is the Kelvin value of the photo's look
|
||||
let old = DevelopSettings {
|
||||
@@ -332,3 +390,77 @@ fn one_field_wb_partial_merge_keeps_the_other_field_on_its_scale() {
|
||||
assert_eq!(d.light.contrast, 20.0);
|
||||
assert_eq!(d.wb, wb_before, "the white balance is untouched");
|
||||
}
|
||||
|
||||
/// The issue asks for one photo-aware merge in every caller that writes settings onto a photo, and that includes
|
||||
/// `preset.apply`: a preset a person or an agent authors by hand can name a single white balance number, and on a
|
||||
/// photo edited in Kelvin that demoted the field it never mentioned to the relative scale, exactly as
|
||||
/// `develop.merge` did. Presets the app makes and the crs / Luminar importers name `mode`, `temp`, `tint` and
|
||||
/// `scale` together, and those keep their old path through `Preset::apply` → `apply_partial` unchanged.
|
||||
#[test]
|
||||
fn one_field_wb_preset_apply_keeps_the_other_field_on_its_scale() {
|
||||
use lightcraft_catalog::{MediaKind, Op, Photo, Source};
|
||||
use lightcraft_develop::{DevelopSettings, WbMode, WbScale};
|
||||
let mut s = Session::new();
|
||||
let add = |s: &mut Session, name: &str, camera: &str, as_shot: (f64, f64)| {
|
||||
let id = s.catalog.alloc_photo_id();
|
||||
let mut p = Photo::new(id, Source::File { path: format!("/photos/{name}") }, name, "ARW", 64, 64, "2026-01-01T00:00:00");
|
||||
p.kind = MediaKind::Raw;
|
||||
p.meta.camera = camera.into();
|
||||
p.as_shot_wb = Some(as_shot);
|
||||
p.develop = std::sync::Arc::new(p.camera_defaults());
|
||||
s.catalog.apply(Op::AddPhoto { photo: Box::new(p) }).unwrap();
|
||||
id
|
||||
};
|
||||
let as_shot = (4986.0, -2.0);
|
||||
let kelvin = add(&mut s, "preset-wb.arw", "SONY ILCE-7M3", as_shot);
|
||||
let l2k = |t, ti| lightcraft_pipeline::local::legacy_to_kelvin(t, ti, as_shot);
|
||||
let k2l = |t, ti| lightcraft_pipeline::local::kelvin_to_legacy(t, ti, as_shot).unwrap();
|
||||
let nearly = |wb: &lightcraft_develop::WhiteBalance, want: &(f64, f64), why: &str| {
|
||||
assert!((wb.temp - want.0).abs() < 1.0 && (wb.tint - want.1).abs() < 0.5, "{why}: {wb:?} vs {want:?}");
|
||||
};
|
||||
s.execute("library.select", &json!({"ids": [kelvin.0], "active": kelvin.0})).unwrap();
|
||||
let edited = DevelopSettings {
|
||||
wb: lightcraft_develop::WhiteBalance { mode: WbMode::Custom, temp: 5000.0, tint: 12.0, scale: WbScale::Kelvin },
|
||||
..DevelopSettings::for_raw(as_shot.0, as_shot.1)
|
||||
};
|
||||
s.set_develop(kelvin, edited.clone(), "kelvin edit").unwrap();
|
||||
|
||||
// a hand-written preset naming one number: the tint it never mentions keeps its Kelvin look, instead of the
|
||||
// old path that stored 5800 and re-read the photo's +12 tint as a relative shift
|
||||
s.presets.push(lightcraft_develop::Preset {
|
||||
id: "user.temp-only".into(),
|
||||
name: "Temp only".into(),
|
||||
group: "User".into(),
|
||||
settings: json!({"wb": {"temp": 5800.0}}),
|
||||
favorite: false,
|
||||
builtin: false,
|
||||
});
|
||||
s.execute("preset.apply", &json!({"id": "user.temp-only"})).unwrap();
|
||||
let d = (*s.develop_of(kelvin).unwrap()).clone();
|
||||
let keep = k2l(5000.0, 12.0);
|
||||
assert_eq!(d.wb.mode, WbMode::Custom, "{:?}", d.wb);
|
||||
assert_eq!(d.wb.scale, WbScale::Kelvin, "the preset left the photo on the scale it is edited on: {:?}", d.wb);
|
||||
nearly(&d.wb, &l2k(5800.0, keep.1), "the preset's number keeps the meaning an unmarked scale always had");
|
||||
// an amount below 100 % blends between the photo's own white and the preset's, on one scale, and never
|
||||
// mentions the other field at all
|
||||
s.set_develop(kelvin, edited.clone(), "kelvin edit").unwrap();
|
||||
s.execute("preset.apply", &json!({"id": "user.temp-only", "amount": 50})).unwrap();
|
||||
let half = (*s.develop_of(kelvin).unwrap()).clone();
|
||||
let full = l2k(5800.0, keep.1);
|
||||
assert_eq!(half.wb.scale, WbScale::Kelvin, "{:?}", half.wb);
|
||||
assert!(half.wb.temp < 5000.0 && half.wb.temp > full.0, "between the photo's own white and the preset's: {:?}", half.wb);
|
||||
nearly(&half.wb, &(half.wb.temp, 12.0), "the tint the preset never named stays exactly the photo's");
|
||||
// the same number through `develop.merge` gives the same white: that is the point of one shared merge
|
||||
s.set_develop(kelvin, edited.clone(), "kelvin edit").unwrap();
|
||||
s.execute("develop.merge", &json!({"settings": {"wb": {"temp": 5800.0}}})).unwrap();
|
||||
let merged = (*s.develop_of(kelvin).unwrap()).clone();
|
||||
nearly(&merged.wb, &(d.wb.temp, d.wb.tint), "preset apply and develop.merge agree on a one-field white balance");
|
||||
// a preset made from the photo names the whole white balance: its own path, unchanged
|
||||
s.execute("preset.create", &json!({"name": "My kelvin look", "groups": ["whiteBalance"]})).unwrap();
|
||||
let made = s.presets.iter().find(|p| p.name == "My kelvin look").cloned().unwrap();
|
||||
let look = (*s.develop_of(kelvin).unwrap()).clone();
|
||||
s.execute("develop.merge", &json!({"settings": {"wb": {"mode": "asShot"}}})).unwrap();
|
||||
s.execute("preset.apply", &json!({"id": made.id})).unwrap();
|
||||
let back = (*s.develop_of(kelvin).unwrap()).clone();
|
||||
assert_eq!(back.wb, look.wb, "a preset made from the photo puts its whole white balance back: {:?} vs {:?}", back.wb, look.wb);
|
||||
}
|
||||
|
||||
@@ -117,13 +117,19 @@ raws with colour matrices — in the file (DNG) or measured for the camera model
|
||||
#730) — and on raws whose white-balance presets give the scale (`lightcraft_raw::kelvin_presets`), where As Shot
|
||||
reads the file's as-shot white; rendered files and raws without matrices use a relative scale
|
||||
whose 6500 K / 0 is the as-shot look (the Edit panel shows it as −100..+100). The stored `wb` of such a Kelvin raw
|
||||
says `"scale": "kelvin"`; a partial settings JSON given to `develop.merge` or a preset whose `wb` sets `temp` / `tint`
|
||||
**without** `scale` keeps the relative meaning it always had (6500 = as shot) on every raw, so agents that mean
|
||||
Kelvin on a Kelvin raw pass `"scale": "kelvin"` or use `develop.set` / `develop.wb`. A partial that names only one
|
||||
of `temp` / `tint` is resolved against the photo (issue #2910): the photo's white balance is first brought onto the
|
||||
partial's scale with the photo's own as-shot white, so the field the partial does not name keeps the look it had
|
||||
instead of being re-read on the partial's scale, and the stored result is back on the scale the photo is edited on
|
||||
(`"scale": "kelvin"` on a Kelvin raw). Partials naming both fields mean the same as before.
|
||||
says `"scale": "kelvin"`; a partial settings JSON given to `develop.merge`, to a preset's `wb`, or to
|
||||
`lightcraft-cli render --settings` and which sets `temp` / `tint` **without** `scale` keeps the relative meaning it
|
||||
always had (6500 = as shot) on every raw, so agents that mean Kelvin on a Kelvin raw pass `"scale": "kelvin"` or use
|
||||
`develop.set` / `develop.wb`. A partial that names only one of `temp` / `tint` is resolved against the photo
|
||||
(issue #2910): the photo's white balance is first brought onto the partial's scale with the photo's own as-shot
|
||||
white, so the field the partial does not name keeps the look it had instead of being re-read on the partial's scale,
|
||||
and the stored result is back on the scale the photo is edited on (`"scale": "kelvin"` on a Kelvin raw). The same
|
||||
holds for `preset.apply` when the preset names only one of the two numbers; a preset written by the app, or imported
|
||||
from crs / XMP, names `mode`, `temp`, `tint` and `scale` together and behaves exactly as before. A photo whose white
|
||||
balance mode answers its own white — `As Shot`, or a named preset on a raw edited in Kelvin — has that white written
|
||||
out as `Custom` values first, because a number merged into such a mode would otherwise be stored where no render
|
||||
reads it (the panel would show 5800 while the photo stayed at as shot); a partial that names a `mode` itself stays
|
||||
authoritative. Partials naming both fields mean the same as before.
|
||||
|
||||
Image navigation is also available directly as the UI command `view.navigate`, with
|
||||
`{zoom?: "fit" | "fill" | {"percent": number}, pan?: [x, y]}`. Percentage zoom accepts fractional
|
||||
|
||||
+1
-1
@@ -175,7 +175,7 @@ per-request `_meta` receive `resultType: complete` and list/read cache hints; re
|
||||
| `select_photos {ids, active?, mode?}` | Set the selection / active photo. Every id (and `active`) must be in the library: an unknown id is a tool error (`no such photo 9999`) and the selection and active photo are left as they were |
|
||||
| `list_controls {section?}` | Every develop slider: id (`light.exposure`…), range, default, current value |
|
||||
| `get_develop {id?}` | Full develop-settings JSON, with `process`: the rendering process the photo is on ([process-versions.md](process-versions.md)) |
|
||||
| `set_develop {id?, values?, settings?, label?}` | `values`: `{controlId: number}`; `settings`: partial develop JSON deep-merged. Undoable. A `crop.angle` value turns the crop as `crop.straighten` does: kept if it still fits, else shrunk to the largest crop of its aspect inside the photo (`settings` is merged as given, unfitted). White balance: `values` with `wb.temp` / `wb.tint` is Kelvin on raws with colour matrices (DNG, and cameras LightCraft has matrices for) and relative (6500 = as shot) elsewhere; a `settings.wb` with `temp` / `tint` and no `"scale": "kelvin"` is read on the relative scale on every raw, and one that names only `temp` or only `tint` resolves the field it leaves out against the photo, so that field keeps its look (#2910) (see [control-protocol.md](control-protocol.md)) |
|
||||
| `set_develop {id?, values?, settings?, label?}` | `values`: `{controlId: number}`; `settings`: partial develop JSON deep-merged. Undoable. A `crop.angle` value turns the crop as `crop.straighten` does: kept if it still fits, else shrunk to the largest crop of its aspect inside the photo (`settings` is merged as given, unfitted). White balance: `values` with `wb.temp` / `wb.tint` is Kelvin on raws with colour matrices (DNG, and cameras LightCraft has matrices for) and relative (6500 = as shot) elsewhere; a `settings.wb` with `temp` / `tint` and no `"scale": "kelvin"` is read on the relative scale on every raw, and one that names only `temp` or only `tint` resolves the field it leaves out against the photo, so that field keeps its look; on a photo still on As Shot (or on a named white balance preset, on a raw with matrices) the white it really renders is written out as Custom values first, so the number named takes effect instead of being stored unread (#2910, and its review); `apply_preset` resolves a preset that names one number the same way (see [control-protocol.md](control-protocol.md)) |
|
||||
| `apply_preset {preset, amount?, ids?}` | Apply a preset (ids from `cmd_presets_list`) |
|
||||
| `crop {id?, rect?, angle?, reset?}` | Normalized crop rect `[x0,y0,x1,y1]` and straighten angle; at least one of `rect`, `angle`, `reset: true`. The crop always ends inside the photo: an `angle` alone keeps the crop if it still fits, else shrinks it to the largest of its aspect that does |
|
||||
| `render_photo {id?, size?, format?, path?}` | Render with current settings → **image content** (PNG, or JPEG with `format: "jpeg"`), long edge `size` (default 1024) |
|
||||
|
||||
@@ -191,7 +191,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts).
|
||||
| LR-EDIT-LIGHT-CURVE-TAT | Drag-on-image curve adjust | P1 | ✅ | `cmd:develop.targeted` (`target: curve`) | |
|
||||
| LR-EDIT-COLOR-WB-PRESET | White-balance presets | P0 | ✅ | `cmd:develop.wb` | |
|
||||
| LR-EDIT-COLOR-WB-PICKER | White-balance eyedropper | P0 | ✅ | `cmd:tool.wbPicker`, `cmd:develop.wbPick`, `crates/ui-egui/src/tests_wb_selector.rs` | the Edit panel's selector button does what W does (from Compare, Survey or the grid it opens the photo in Detail); no magnified loupe while picking |
|
||||
| LR-EDIT-COLOR-TEMP | Temperature | P0 | ✅ | `ctl:wb.temp`, `crates/pipeline/src/local.rs` (`wb_matrix_for`, `legacy_white`), `crates/raw/src/color.rs` (`rebalance`), `crates/raw/src/spectral.rs` (`find_camera`), `crates/catalog/src/model.rs` (`relative_wb_camera`), `crates/develop/src/controls.rs` (`ControlSpec::position`), `crates/raw/src/kelvin_presets.rs` | Kelvin (2000–50000 K, As Shot read from the file) on raws with colour matrices: in the file (DNG: re-developed for the chosen white through the camera's own matrices, camera-space white balance as Lightroom; the eyedropper / Auto invert the same model) or measured for the camera model (`lightcraft_raw::spectral`, 52 bodies, issue #730: the matrices turn the as-shot multipliers into Kelvin and the slider back, the look stays the file's fit, and the picture is adapted from the as-shot white, Bradford), or read from the camera's own white-balance presets (Sony ARW `SR2SubIFD`, Sony ILCA-99M2, ILCA-77M2, SLT-A33, DSC-RX10, ILCE-3500, ILCE-7S: Sony's Kelvin dial reads back within 0.3 % on 5 settings of 4 bodies; Canon CR2 `ColorData` presets, PowerShot G1 X Mark III: the camera's own As Shot temperature reads back within 5 % on its 9 white-balance settings; or built-in presets from the camera's own stated temperatures, `crates/raw/src/kelvin_builtin.rs`: Nikon Z5II, whose Natural Auto shot reads back at 5036 K against the 5030 K the camera wrote). The Kelvin slider's track is Lightroom's: 7000 K in the middle, 2000–7000 K evenly on the left half, logarithmic up to 50000 K on the right (within 2 % of positions measured in Lightroom Classic), for drawing, clicks and drags; wheel / arrow-key nudges and Quick Develop still step a fixed 50 / 100 / 500 K; the relative slider stays linear. Settings written on the relative scale before a camera's matrices reached it keep their look (`WbScale`). A partial white balance naming only `temp` or only `tint` (`develop.merge`, `render --settings`, MCP `set_develop`, a hand-written preset) is resolved against the photo, so the field it leaves out keeps the look it had: `merge_settings_for_photo` in `crates/engine/src/cmd/develop.rs` and `kelvin_to_legacy` in `crates/pipeline/src/local.rs`, issue #2910, tests in `crates/engine/src/tests_color.rs`. Not verified against Lightroom's Kelvin: on the ILCE-7M3 the matrices read 7–10 % below Sony's dial (issue #730), and whether the matrices or the dial are off is open; raws without matrices or presets anywhere and rendered files use the relative −100..+100 scale (6500 K / 0 = as shot). |
|
||||
| LR-EDIT-COLOR-TEMP | Temperature | P0 | ✅ | `ctl:wb.temp`, `crates/pipeline/src/local.rs` (`wb_matrix_for`, `legacy_white`), `crates/raw/src/color.rs` (`rebalance`), `crates/raw/src/spectral.rs` (`find_camera`), `crates/catalog/src/model.rs` (`relative_wb_camera`), `crates/develop/src/controls.rs` (`ControlSpec::position`), `crates/raw/src/kelvin_presets.rs` | Kelvin (2000–50000 K, As Shot read from the file) on raws with colour matrices: in the file (DNG: re-developed for the chosen white through the camera's own matrices, camera-space white balance as Lightroom; the eyedropper / Auto invert the same model) or measured for the camera model (`lightcraft_raw::spectral`, 52 bodies, issue #730: the matrices turn the as-shot multipliers into Kelvin and the slider back, the look stays the file's fit, and the picture is adapted from the as-shot white, Bradford), or read from the camera's own white-balance presets (Sony ARW `SR2SubIFD`, Sony ILCA-99M2, ILCA-77M2, SLT-A33, DSC-RX10, ILCE-3500, ILCE-7S: Sony's Kelvin dial reads back within 0.3 % on 5 settings of 4 bodies; Canon CR2 `ColorData` presets, PowerShot G1 X Mark III: the camera's own As Shot temperature reads back within 5 % on its 9 white-balance settings; or built-in presets from the camera's own stated temperatures, `crates/raw/src/kelvin_builtin.rs`: Nikon Z5II, whose Natural Auto shot reads back at 5036 K against the 5030 K the camera wrote). The Kelvin slider's track is Lightroom's: 7000 K in the middle, 2000–7000 K evenly on the left half, logarithmic up to 50000 K on the right (within 2 % of positions measured in Lightroom Classic), for drawing, clicks and drags; wheel / arrow-key nudges and Quick Develop still step a fixed 50 / 100 / 500 K; the relative slider stays linear. Settings written on the relative scale before a camera's matrices reached it keep their look (`WbScale`). A partial white balance naming only `temp` or only `tint` (`develop.merge`, `render --settings`, MCP `set_develop`, or `preset.apply` of a hand-written preset naming one of them) is resolved against the photo, so the field it leaves out keeps the look it had, and a photo whose mode answers its own white (`As Shot`, or a named preset on a raw with a Kelvin scale) has the white it really renders written out as Custom values the partial can move: `prepare_photo_for_wb_partial` and `merge_settings_for_photo` in `crates/engine/src/cmd/develop.rs` and `kelvin_to_legacy` in `crates/pipeline/src/local.rs`, issue #2910, tests in `crates/engine/src/tests_color.rs`. Not verified against Lightroom's Kelvin: on the ILCE-7M3 the matrices read 7–10 % below Sony's dial (issue #730), and whether the matrices or the dial are off is open; raws without matrices or presets anywhere and rendered files use the relative −100..+100 scale (6500 K / 0 = as shot). |
|
||||
| LR-EDIT-COLOR-TINT | Tint | P0 | ✅ | `ctl:wb.tint`, `docs/tint-direction.md` | DNG / Lightroom sign: negative (left) adds green, positive (right) magenta; XMP `crs:Tint` values render as in Lightroom (issue #188); CPU/GPU regression tests |
|
||||
| LR-EDIT-COLOR-VIBRANCE | Vibrance | P0 | ✅ | `ctl:color.vibrance` | |
|
||||
| LR-EDIT-COLOR-SATURATION | Saturation | P0 | ✅ | `ctl:color.saturation` | |
|
||||
|
||||
+13
-7
@@ -4,13 +4,19 @@
|
||||
|
||||
### A one-field white balance edit no longer shifts the other one
|
||||
- Settings that name only a white balance temperature, or only a tint — `develop.merge`,
|
||||
`lightcraft-cli render --settings`, an MCP `develop.merge` call, or a hand-written preset — used to change the
|
||||
colour balance of photos they never mentioned. On a raw edited in Kelvin the field the partial kept was re-read
|
||||
on the other scale: `wb {temp: 5800}` on a photo at 5000 K / +12 tint came back at 5800 K with the +12 read as a
|
||||
relative shift, moving the green–magenta balance by a visible amount. The kept field now carries the look the
|
||||
photo had, and the result is stored on the scale that photo is edited on. Presets, XMP import and settings that
|
||||
name both fields (or neither) behave exactly as before, and a partial that says `"scale": "kelvin"` is still
|
||||
read in Kelvin.
|
||||
`lightcraft-cli render --settings <partial JSON>`, an MCP `set_develop {settings: …}` call, or a hand-written
|
||||
preset naming one of the two — used to change the colour balance of photos they never mentioned. On a raw edited in
|
||||
Kelvin the field the partial left out was re-read on the other scale: `wb {temp: 5800}` on a photo at 5000 K /
|
||||
+12 tint came back at 5800 K with the +12 read as a relative shift, moving the green–magenta balance by a visible
|
||||
amount. The kept field now carries the look the photo had, and the result is stored on the scale that photo is
|
||||
edited on.
|
||||
- The same partial on a photo still on As Shot — or on a named white balance preset, on a raw with a Kelvin scale —
|
||||
stored the number without any render ever reading it: those modes answer their own white, so the panel showed
|
||||
5800 K over a picture still at the file's white. That white is now written out as Custom values before the partial
|
||||
lands; a partial that names a mode itself still gets that mode.
|
||||
- Presets made from a photo and the crs / XMP and Luminar importers name mode, temperature, tint and scale together,
|
||||
and partials naming both fields (or neither) behave exactly as before; a partial that says `"scale": "kelvin"` is
|
||||
still read in Kelvin.
|
||||
|
||||
## 2026-10-24
|
||||
|
||||
|
||||
Reference in New Issue
Block a user