diff --git a/core/dr-gpu/tests/local_adjustments.rs b/core/dr-gpu/tests/local_adjustments.rs index 5b8653b..89e8046 100644 --- a/core/dr-gpu/tests/local_adjustments.rs +++ b/core/dr-gpu/tests/local_adjustments.rs @@ -1135,3 +1135,76 @@ fn two_shown_masks_are_drawn_each_in_its_own_colour() { "between them, alpha shows black: ({r}, {g}, {b})" ); } + +/// Render a mid-grey-and-shadows frame through `chain` and `stack`. +fn render_chain( + ctx: &GpuContext, + chain: &[Box], + stack: &MaskStack, + field: Option<&LabelField>, +) -> Vec { + // A ramp, so both ends of the tonal range are in the comparison. + let data: Vec = (0..SIZE * SIZE) + .flat_map(|i| { + let v = ((i % SIZE) * 255 / (SIZE - 1)) as u8; + [v, v / 2, v, 255] + }) + .collect(); + let source = DemosaicedImage::from_rgba8(ctx, &data, SIZE, SIZE).expect("upload"); + let shader = compose_full( + chain, + &Framing::new(), + ColourSpace::Srgb, + stack, + &SpotSet::new(), + &[], + ); + let mut masks = MaskPass::new(ctx).expect("mask pass"); + let array = masks + .render(stack, field, None, None, SIZE, SIZE) + .expect("rasterise"); + let mut adjust = AdjustPass::new(ctx); + adjust + .render_masked(&source, &shader, SIZE, SIZE, Some(array)) + .expect("render"); + adjust.export_pixels().expect("readback").0 +} + +fn contrast_chain(v: f32) -> Vec> { + let mut chain = ops::chain(); + chain + .iter_mut() + .find(|o| o.descriptor().id.0 == "contrast") + .expect("contrast") + .set_param(ParamId("contrast"), v); + chain +} + +/// A layer's setting is an offset to the global one, applied once: global +/// −30 with a whole-frame layer at −20 is exactly global −50 — not −30 and +/// then −20 again on the result, which is what a layer used to do. +#[test] +fn a_whole_frame_layer_adds_its_setting_to_the_global_one() { + let Some(ctx) = ctx() else { + eprintln!("no GPU adapter; skipping"); + return; + }; + let mut layer = MaskLayer::new("m1", whole_frame()); + layer.set_param("contrast", ParamId("contrast"), -20.0); + let mut stack = MaskStack::new(); + stack.push(layer); + + let field = split_field(&ctx); + let offset = render_chain(&ctx, &contrast_chain(-30.0), &stack, Some(&field)); + let direct = render_chain(&ctx, &contrast_chain(-50.0), &MaskStack::new(), None); + let worst = offset + .iter() + .zip(&direct) + .map(|(a, b)| a.abs_diff(*b)) + .max() + .unwrap(); + assert!( + worst <= 1, + "layer offset differs from the summed setting by {worst}" + ); +} diff --git a/core/dr-pipeline/src/mask.rs b/core/dr-pipeline/src/mask.rs index 59fb088..f14c034 100644 --- a/core/dr-pipeline/src/mask.rs +++ b/core/dr-pipeline/src/mask.rs @@ -73,7 +73,7 @@ use std::fmt::Write as _; use std::sync::Arc; use crate::coverage::Coverage; -use crate::descriptor::{Attribute, OpDescriptor, ParamId}; +use crate::descriptor::{Attribute, OpDescriptor, ParamId, ParamKind}; use crate::operation::Operation; use crate::ops; @@ -1368,17 +1368,19 @@ pub struct MaskLayer { /// This layer's adjustments. /// /// A full chain, the same one [`crate::EditGraph`] holds. That is the - /// whole reason local adjustments need no per-operation support: the - /// composer already knows how to turn a chain into WGSL, and a mask layer - /// is a chain that happens to be multiplied by a mask afterwards. + /// whole reason local adjustments need no per-operation support: each + /// setting here is an offset from its default, added to the global chain's + /// setting and run where that operation runs, weighted by the mask (see + /// [`offset_onto`]). pub ops: Vec>, } /// The chain a mask layer holds: every point operation, and neither the /// neighbourhood ones nor the optical corrections. /// -/// A layer's adjustments are fused into the colour dispatch and multiplied by -/// the mask afterwards, which is exactly why a layer needs no per-operation +/// A layer's adjustments are fused into the colour dispatch, each beside the +/// global operation it offsets and weighted by the mask, which is exactly why +/// a layer needs no per-operation /// support — the composer already knows how to turn a chain into WGSL. A /// neighbourhood operation cannot go through that path at all: it runs as its /// own dispatch in [`crate::detail`], after the fused pass and after the masks @@ -1642,8 +1644,8 @@ impl MaskLayer { /// The operations in this layer's chain that reach the shader. /// /// Neighbourhood operations are excluded, and not as an oversight. A - /// layer's chain is *fused into the point-operation pass* and multiplied - /// by the mask afterwards; the detail stage runs once, over the whole + /// layer's chain is *fused into the point-operation pass*, weighted by the + /// mask at each operation; the detail stage runs once, over the whole /// frame, after that pass has finished (see [`crate::detail`]). There is /// nowhere in that arrangement for a sharpening confined to one mask to /// happen, so a detail operation in a layer would contribute an empty @@ -1975,7 +1977,9 @@ impl MaskStack { Some(self.layers.remove(i)) } - /// Reorder, since later layers composite over earlier ones. + /// Reorder. Layers add their changes, so order no longer decides the + /// picture — but it is the order the panel lists them in and the order + /// their slots are assigned. pub fn move_to(&mut self, id: &str, index: usize) { let Some(from) = self.layers.iter().position(|l| l.id == id) else { return; @@ -2041,25 +2045,83 @@ impl MaskStack { } } -/// One layer's contribution to the generated shader. +/// The layers' contribution to the generated shader. +/// +/// Not a block of its own any more. A layer's adjustments are *offsets to the +/// global ones*, applied at each operation's own place in the chain, so what +/// this hands back is pieces the composer threads through its loop over the +/// global operations: the weights, sampled once before the first operation, +/// and one [`LocalOp`] per layer per operation the layer moved. pub(crate) struct LayerShader { pub uniform_fields: String, pub uniform_values: Vec, - pub body: String, + /// Each layer's shaped mask, `mask_w{slot}`, sampled once ahead of the + /// operations that read it. Empty when no layer changes a pixel. + pub weights: String, + /// Every layer's version of every operation it moved, in layer order and + /// then chain order — see [`LocalOp`]. + pub ops: Vec, pub helpers: Vec, /// TRACES: FR-DEV-19c /// The block that draws one layer's mask over the finished picture, empty /// when nothing is being revealed. /// - /// Kept apart from `body` because it belongs at the other end of the - /// shader. Everything in `body` runs on scene-referred colour in the - /// working space, where a flat tint would then be pushed through the base - /// curve and the camera matrix and arrive as some other colour, and a + /// Kept apart from the rest because it belongs at the other end of the + /// shader. Everything else runs on scene-referred colour in the working + /// space, where a flat tint would then be pushed through the base curve + /// and the camera matrix and arrive as some other colour, and a /// white-on-black alpha would arrive as neither. This runs after the /// output transform, so what is written is what is seen. pub reveal: String, } +/// One layer's version of one operation: the global settings with the layer's +/// offsets added, as a fragment reading this layer's own uniforms. +/// +/// The composer runs it beside the global fragment on the same input colour, +/// and moves the pixel toward its result by the layer's weight — see +/// `operation::local_block`. +pub(crate) struct LocalOp { + pub op: &'static str, + pub slot: usize, + /// Empty when the offsets cancel the global setting back to neutral. That + /// is still an entry, because it still means something: inside the mask + /// this operation does nothing at all. + pub fragment: String, +} + +/// A layer's settings for one operation, applied as offsets to the global +/// operation's. +/// +/// **This is what a local adjustment means**, and the reason it is not a +/// second chain run over the finished picture. A photographer who sets +/// contrast −30 on the whole frame and −20 on a face means −50 on the face, +/// at the place contrast sits in the chain — not −30, then everything after +/// contrast, then −20 applied again to the result. Stacked that way the two +/// edits compound in ways neither slider shows, and a flattening applied to an +/// already-flattened picture is how a shadow's noise ended up magenta. +/// +/// Per parameter: one the layer left at its default takes the global value; a +/// moved scalar adds its distance from default to the global value, clamped to +/// the parameter's range; a moved switch or choice replaces it, since there is +/// no such thing as half a variant. +fn offset_onto(dst: &mut dyn Operation, local: &dyn Operation, global: Option<&dyn Operation>) { + let desc = local.descriptor(); + for p in &desc.params { + let here = local.param(p.id); + let base = global.map_or(p.default, |g| g.param(p.id)); + let value = if here == p.default { + base + } else { + match p.kind { + ParamKind::Scalar { .. } => p.clamp(base + (here - p.default)), + ParamKind::Bool | ParamKind::Enum { .. } => here, + } + }; + dst.set_param(p.id, value); + } +} + /// Emit the WGSL for every layer that renders, and for the mask being looked /// at. /// @@ -2068,11 +2130,18 @@ pub(crate) struct LayerShader { /// an adjustment on it, which is why the two are one sequence and why every /// other half of the pipeline has to be given the same `reveal` for the slots /// to mean the same thing. -pub(crate) fn compose_layers_revealing(stack: &MaskStack, reveal: Option<&Reveal>) -> LayerShader { +/// +/// `global` is the chain the layers are offsets to. +pub(crate) fn compose_layers_revealing( + stack: &MaskStack, + reveal: Option<&Reveal>, + global: &[Box], +) -> LayerShader { let mut out = LayerShader { uniform_fields: String::new(), uniform_values: Vec::new(), - body: String::new(), + weights: String::new(), + ops: Vec::new(), helpers: Vec::new(), reveal: String::new(), }; @@ -2104,13 +2173,14 @@ pub(crate) fn compose_layers_revealing(stack: &MaskStack, reveal: Option<&Reveal ); out.uniform_values.extend_from_slice(&layer.uniforms()); - let _ = writeln!( - out.body, - "\n // ======== mask {slot}: {} ({}) ========", - layer.display_name(), - layer.base().source.kind() - ); - let _ = writeln!(out.body, " {{"); + // A layer only being looked at moves no pixel, so it needs a slot for + // the reveal and no weight. + if layer.active_ops().next().is_none() { + continue; + } + + // The weight, once per pixel, ahead of every operation that reads it. + // // **`uv_src`, not `gid.xy`.** The mask array is rasterised in *source* // space, and `uv_src` is the source position this output pixel came // from — after the crop, the zoom, the pan, the straightening and the @@ -2123,33 +2193,49 @@ pub(crate) fn compose_layers_revealing(stack: &MaskStack, reveal: Option<&Reveal // place. A second copy here would be a second thing to keep in step // with `Framing::wgsl_prologue`, and the failure would be a mask that // is subtly wrong only when straightened. - let _ = writeln!(out.body, " var m = sample_mask(uv_src, {slot});"); + let w = format!("mask_w{slot}"); let _ = writeln!( - out.body, - " m = select(m, 1.0 - m, u.{prefix}_invert > 0.5);" + out.weights, + "\n // ======== mask {slot}: {} ({}) ========", + layer.display_name(), + layer.base().source.kind() + ); + let _ = writeln!(out.weights, " var {w} = sample_mask(uv_src, {slot});"); + let _ = writeln!( + out.weights, + " {w} = select({w}, 1.0 - {w}, u.{prefix}_invert > 0.5);" ); let _ = writeln!( - out.body, - " m = clamp(m * u.{prefix}_opacity, 0.0, 1.0);" + out.weights, + " {w} = clamp({w} * u.{prefix}_opacity, 0.0, 1.0);" ); - // Skipping the work where the mask is empty is most of the point of a - // local adjustment: a mask covering a tenth of the frame should cost - // about a tenth of the shader. Safe as non-uniform control flow — - // nothing inside samples with derivatives or synchronises. - let _ = writeln!(out.body, " if (m > 0.0) {{"); - // `masked` is the outer-scope carrier: op fragments write to a `c` - // they expect to own, so the inner block shadows `c` and copies the - // result back out. Assigning the outer `c` from inside is not possible - // precisely because it is shadowed. - let _ = writeln!(out.body, " var masked = c;"); - let _ = writeln!(out.body, " {{"); - let _ = writeln!(out.body, " var c = masked;"); - for op in layer.active_ops() { - let id = op.descriptor().id.0; + // A fresh chain to hold the combined settings: the layer's own ops + // are its offsets and must stay that way. + let mut combined = layer_chain(); + for (dst, local) in combined.iter_mut().zip(&layer.ops) { + let local = local.as_ref(); + if !local.is_active() || local.detail().is_some() { + continue; + } + let id = local.descriptor().id.0; + let g = global + .iter() + .map(|o| o.as_ref()) + .find(|o| o.descriptor().id.0 == id); + offset_onto(dst.as_mut(), local, g); + + if !dst.is_active() { + out.ops.push(LocalOp { + op: id, + slot, + fragment: String::new(), + }); + continue; + } + let op_prefix = format!("{prefix}_{}", crate::operation::sanitise(id)); - - let op_uniforms = op.uniforms(); + let op_uniforms = dst.uniforms(); if !op_uniforms.is_empty() { let _ = writeln!(out.uniform_fields, " // mask {slot}: {id}"); } @@ -2158,13 +2244,13 @@ pub(crate) fn compose_layers_revealing(stack: &MaskStack, reveal: Option<&Reveal out.uniform_values.push(u.value); } - for h in op.helpers() { + for h in dst.helpers() { if !out.helpers.iter().any(|e| e.name == h.name) { out.helpers.push(*h); } } - let mut fragment = op.wgsl_body(); + let mut fragment = dst.wgsl_body(); for u in &op_uniforms { fragment = crate::operation::rewrite_uniform( &fragment, @@ -2172,20 +2258,12 @@ pub(crate) fn compose_layers_revealing(stack: &MaskStack, reveal: Option<&Reveal &format!("u.{op_prefix}_{}", u.name), ); } - - let _ = writeln!(out.body, " // ---- {id} ----"); - let _ = writeln!(out.body, " {{"); - for line in fragment.lines() { - let _ = writeln!(out.body, " {line}"); - } - let _ = writeln!(out.body, " }}"); + out.ops.push(LocalOp { + op: id, + slot, + fragment, + }); } - - let _ = writeln!(out.body, " masked = c;"); - let _ = writeln!(out.body, " }}"); - let _ = writeln!(out.body, " c = mix(c, masked, m);"); - let _ = writeln!(out.body, " }}"); - let _ = writeln!(out.body, " }}"); } out @@ -2317,7 +2395,10 @@ mod tests { let mut stack = MaskStack::new(); stack.push(layer); assert!(stack.is_neutral()); - assert_eq!(compose_layers_revealing(&stack, None).body, ""); + assert_eq!( + compose_layers_revealing(&stack, None, &ops::chain()).weights, + "" + ); } #[test] @@ -2403,11 +2484,11 @@ mod tests { stack.push(lit_layer("m1", 1.0)); stack.push(lit_layer("m2", -1.0)); - let shader = compose_layers_revealing(&stack, None); - assert!(shader.body.contains("sample_mask(uv_src, 0)")); - assert!(shader.body.contains("sample_mask(uv_src, 1)")); - assert!(shader.body.contains("u.mask0_opacity")); - assert!(shader.body.contains("u.mask1_opacity")); + let shader = compose_layers_revealing(&stack, None, &ops::chain()); + assert!(shader.weights.contains("sample_mask(uv_src, 0)")); + assert!(shader.weights.contains("sample_mask(uv_src, 1)")); + assert!(shader.weights.contains("u.mask0_opacity")); + assert!(shader.weights.contains("u.mask1_opacity")); } /// The slot a layer renders through must follow `active()`, not the raw @@ -2420,12 +2501,12 @@ mod tests { stack.push(off); stack.push(lit_layer("m2", -1.0)); - let shader = compose_layers_revealing(&stack, None); + let shader = compose_layers_revealing(&stack, None, &ops::chain()); assert!( - shader.body.contains("sample_mask(uv_src, 0)"), + shader.weights.contains("sample_mask(uv_src, 0)"), "the one active layer must use slot 0, not slot 1" ); - assert!(!shader.body.contains("sample_mask(uv_src, 1)")); + assert!(!shader.weights.contains("sample_mask(uv_src, 1)")); } /// TRACES: FR-DEV-19c @@ -2445,7 +2526,7 @@ mod tests { stack.push(MaskLayer::new("m2", MaskSource::brush())); let reveal = Reveal::one("m2", RevealStyle::Alpha); - let shader = compose_layers_revealing(&stack, Some(&reveal)); + let shader = compose_layers_revealing(&stack, Some(&reveal), &ops::chain()); assert_eq!( stack.rendered_count(Some(&reveal)), @@ -2483,7 +2564,7 @@ mod tests { ], style: RevealStyle::Tint, }; - let shader = compose_layers_revealing(&stack, Some(&reveal)); + let shader = compose_layers_revealing(&stack, Some(&reveal), &ops::chain()); let sky = shader .reveal @@ -2509,7 +2590,9 @@ mod tests { fn nothing_is_revealed_unless_it_was_asked_for() { let mut stack = MaskStack::new(); stack.push(lit_layer("m1", 1.0)); - assert!(compose_layers_revealing(&stack, None).reveal.is_empty()); + assert!(compose_layers_revealing(&stack, None, &ops::chain()) + .reveal + .is_empty()); } /// A reveal aimed at a layer that is not in the stack is not a slot, and @@ -2520,9 +2603,11 @@ mod tests { stack.push(lit_layer("m1", 1.0)); let reveal = Reveal::one("gone", RevealStyle::Tint); assert_eq!(stack.rendered_count(Some(&reveal)), 1); - assert!(compose_layers_revealing(&stack, Some(&reveal)) - .reveal - .is_empty()); + assert!( + compose_layers_revealing(&stack, Some(&reveal), &ops::chain()) + .reveal + .is_empty() + ); } #[test] @@ -2531,7 +2616,7 @@ mod tests { stack.push(lit_layer("m1", 1.0)); stack.push(lit_layer("m2", -1.0)); - let shader = compose_layers_revealing(&stack, None); + let shader = compose_layers_revealing(&stack, None, &ops::chain()); assert!(shader.uniform_fields.contains("mask0_exposure_")); assert!(shader.uniform_fields.contains("mask1_exposure_")); assert_eq!( @@ -2545,16 +2630,140 @@ mod tests { ); } + /// The whole shader for `stack` over a global chain with `global` + /// applied to it. + fn composed_with(stack: &MaskStack, global: impl FnOnce(&mut [Box])) -> String { + let mut chain = ops::chain(); + global(&mut chain); + crate::operation::compose_full( + &chain, + &crate::Framing::new(), + dr_types::ColourSpace::Srgb, + stack, + &crate::spot::SpotSet::new(), + &[], + ) + .source + } + + fn set(chain: &mut [Box], op: &str, param: &'static str, v: f32) { + chain + .iter_mut() + .find(|o| o.descriptor().id.0 == op) + .expect("op in chain") + .set_param(ParamId(param), v); + } + + fn contrast_layer(v: f32) -> MaskLayer { + let mut layer = MaskLayer::new("m1", regions(&[1])); + layer.set_param("contrast", ParamId("contrast"), v); + layer + } + #[test] - fn the_inner_block_shadows_c_and_copies_back() { + fn the_layer_version_shadows_c_and_blends_by_its_difference() { let mut stack = MaskStack::new(); stack.push(lit_layer("m1", 1.0)); - let body = compose_layers_revealing(&stack, None).body; + let src = composed_with(&stack, |_| {}); - assert!(body.contains("var masked = c;")); - assert!(body.contains("var c = masked;")); - assert!(body.contains("masked = c;")); - assert!(body.contains("c = mix(c, masked, m);")); + assert!(src.contains("var c = local_in;")); + assert!(src.contains("local_sum = local_sum + mask_w0 * (c - local_global);")); + assert!(src.contains("c = max(local_sum, vec3(0.0));")); + } + + /// **The bug this shape exists for.** A layer's contrast is added to the + /// global contrast, not run a second time on top of it. + #[test] + fn a_layer_setting_is_an_offset_to_the_global_one() { + let mut stack = MaskStack::new(); + stack.push(contrast_layer(-20.0)); + let mut chain = ops::chain(); + set(&mut chain, "contrast", "contrast", -30.0); + + let shader = compose_layers_revealing(&stack, None, &chain); + let at = shader + .uniform_fields + .lines() + .filter(|l| l.trim_start().starts_with("mask")) + .position(|l| l.contains("mask0_contrast_amount")) + .expect("the layer carries its own contrast"); + assert_eq!( + shader.uniform_values[at], -0.5, + "global -30 and local -20 is -50 inside the mask" + ); + } + + /// Where the operation runs is where the layer's version of it runs — + /// between the global operations either side, not after all of them. + #[test] + fn a_layer_runs_at_its_operations_place_in_the_chain() { + let mut stack = MaskStack::new(); + stack.push(contrast_layer(-20.0)); + let src = composed_with(&stack, |c| set(c, "saturation", "saturation", 20.0)); + + let contrast = src.find("// ---- contrast ----").expect("contrast block"); + let blend = src.find("mask_w0 * (c - local_global)").expect("blend"); + let saturation = src + .find("// ---- saturation ----") + .expect("saturation block"); + assert!(contrast < blend && blend < saturation); + } + + /// **No setting is applied twice.** The layer's version of an operation + /// starts from the colour the operation was handed, not from the global + /// result, and reads only its own combined setting — so global −30 and + /// local −20 is one contrast of −50 inside the mask, never −30 and then + /// −50 again, and the operation does not run a second time after the + /// chain as it once did. + #[test] + fn a_setting_is_applied_once_not_stacked() { + let mut stack = MaskStack::new(); + stack.push(contrast_layer(-20.0)); + let src = composed_with(&stack, |c| set(c, "contrast", "contrast", -30.0)); + + assert_eq!( + src.matches("// ---- contrast ----").count(), + 1, + "contrast runs at one place in the chain" + ); + let version = &src[src.find("if (mask_w0 > 0.0)").expect("layer version")..]; + let version = &version[..version.find("local_sum = local_sum").unwrap()]; + assert!( + version.contains("var c = local_in;"), + "starts from the operation's input" + ); + assert!(version.contains("u.mask0_contrast_amount")); + assert!( + !version.contains("u.contrast_amount"), + "the global setting is already inside the combined one" + ); + } + + /// An offset that cancels the global setting is not nothing: inside the + /// mask the operation is back at neutral, so the layer's version is empty + /// and the blend pulls toward the colour the operation was handed. + #[test] + fn an_offset_back_to_neutral_undoes_the_global_setting() { + let mut stack = MaskStack::new(); + stack.push(contrast_layer(30.0)); + let mut chain = ops::chain(); + set(&mut chain, "contrast", "contrast", -30.0); + + let shader = compose_layers_revealing(&stack, None, &chain); + let local: Vec<_> = shader.ops.iter().filter(|l| l.op == "contrast").collect(); + assert_eq!(local.len(), 1); + assert!(local[0].fragment.is_empty()); + } + + /// A global chain with nothing moved still hands a layer's operation a + /// place to run: the global side of the blend is simply empty. + #[test] + fn an_operation_only_a_layer_moved_still_runs_in_its_place() { + let mut stack = MaskStack::new(); + stack.push(contrast_layer(-20.0)); + let src = composed_with(&stack, |_| {}); + assert!(src.contains("// ---- contrast ----")); + assert!(!src.contains("(local only)")); } #[test] diff --git a/core/dr-pipeline/src/operation.rs b/core/dr-pipeline/src/operation.rs index 6b3db36..3eed7be 100644 --- a/core/dr-pipeline/src/operation.rs +++ b/core/dr-pipeline/src/operation.rs @@ -597,10 +597,11 @@ pub fn compose_with_framing( /// TRACES: FR-DEV-3 /// Compose the global chain, the framing, and the local adjustments. /// -/// Mask layers are emitted **after** every global operation and before the -/// conversion out of camera space, so a local exposure acts on the tones the -/// global chain settled on — which is what a photographer means by "and then -/// lift the shadows on her face". +/// A mask layer's settings are **offsets to the global ones**, applied at each +/// operation's own place in the chain: global contrast −30 and a face at −20 +/// is contrast −50 on the face, run where contrast runs. They once ran as a +/// second chain after every global operation, which compounded the two edits +/// in ways neither slider showed — see `mask::offset_onto`. /// /// The fused-dispatch property survives: three global adjustments and two /// masked ones are still one shader, one read and one write. The masks @@ -858,48 +859,76 @@ fn compose_inner( } } - for op in &active { + // TRACES: FR-DEV-3 + // The local adjustments. Composed first because they are threaded through + // the loop below rather than appended after it: a layer's settings are + // offsets to the global ones, applied at each operation's own place in + // the chain (see `mask::offset_onto` for why). The weights are sampled + // here, once per pixel, ahead of every operation that reads them. + let layers = crate::mask::compose_layers_revealing(masks, reveal, ops); + body.push_str(&layers.weights); + + // Every point operation that is active globally *or* in some layer. One + // that only a layer moved still runs here, at its place in the chain, + // with nothing on the global side of its blend. + for op in ops + .iter() + .map(|o| o.as_ref()) + .filter(|o| o.detail().is_none()) + { let id = op.descriptor().id.0; + let local: Vec<&crate::mask::LocalOp> = layers.ops.iter().filter(|l| l.op == id).collect(); + if !op.is_active() && local.is_empty() { + continue; + } let prefix = sanitise(id); - // Each op's uniforms are prefixed, so two operations may both declare - // a field called `amount` without colliding. - let op_uniforms = op.uniforms(); - if !op_uniforms.is_empty() { - let _ = writeln!(uniform_fields, " // {id}"); - } - for u in &op_uniforms { - let _ = writeln!(uniform_fields, " {prefix}_{}: f32,", u.name); - uniform_values.push(u.value); - } + let mut fragment = String::new(); + if op.is_active() { + // Each op's uniforms are prefixed, so two operations may both + // declare a field called `amount` without colliding. + let op_uniforms = op.uniforms(); + if !op_uniforms.is_empty() { + let _ = writeln!(uniform_fields, " // {id}"); + } + for u in &op_uniforms { + let _ = writeln!(uniform_fields, " {prefix}_{}: f32,", u.name); + uniform_values.push(u.value); + } - for h in op.helpers() { - if !helpers.iter().any(|existing| existing.name == h.name) { - helpers.push(*h); + for h in op.helpers() { + if !helpers.iter().any(|existing| existing.name == h.name) { + helpers.push(*h); + } + } + + // Rewrite bare uniform names to their prefixed struct fields, so a + // fragment is written without knowing about any other operation. + fragment = op.wgsl_body(); + for u in &op_uniforms { + fragment = rewrite_uniform(&fragment, u.name, &format!("u.{prefix}_{}", u.name)); } } - // Rewrite bare uniform names to their prefixed struct fields, so a - // fragment is written without knowing about any other operation. - let mut fragment = op.wgsl_body(); - for u in &op_uniforms { - fragment = rewrite_uniform(&fragment, u.name, &format!("u.{prefix}_{}", u.name)); - } - let _ = writeln!(body, "\n // ---- {id} ----"); - let _ = writeln!(body, " {{"); - for line in fragment.lines() { - let _ = writeln!(body, " {line}"); - } - let _ = writeln!(body, " }}"); + body.push_str(&local_block(&fragment, &local)); + } + + // A layer's operation the global chain does not hold at all. Not a case + // any editor produces — both chains come from `ops::chain` — but a layer + // must not lose an edit because a caller composed a shorter chain. + let mut orphans: Vec<&'static str> = Vec::new(); + for l in &layers.ops { + if !orphans.contains(&l.op) && !ops.iter().any(|o| o.descriptor().id.0 == l.op) { + orphans.push(l.op); + } + } + for id in orphans { + let local: Vec<&crate::mask::LocalOp> = layers.ops.iter().filter(|l| l.op == id).collect(); + let _ = writeln!(body, "\n // ---- {id} (local only) ----"); + body.push_str(&local_block("", &local)); } - // The local adjustments, after every global one: a masked exposure should - // act on the tones the global chain arrived at, not on the ones it started - // from. Their uniforms follow the global ops' in the block for the same - // reason those follow framing's — slot order is emission order, and - // nothing addresses a slot by number. - let layers = crate::mask::compose_layers_revealing(masks, reveal); // TRACES: FR-DEV-19c // Held apart from the body, because it belongs after the output transform // rather than among the operations — see `mask::LayerShader::reveal`. @@ -908,7 +937,6 @@ fn compose_inner( let reveal_block = layers.reveal.clone(); uniform_fields.push_str(&layers.uniform_fields); uniform_values.extend_from_slice(&layers.uniform_values); - body.push_str(&layers.body); for h in &layers.helpers { if !helpers.iter().any(|existing| existing.name == h.name) { helpers.push(*h); @@ -1255,6 +1283,66 @@ fn main(@builtin(global_invocation_id) gid: vec3) {{ } } +/// One operation's block: its global fragment, and each layer's version of it +/// blended in by that layer's weight. +/// +/// Every version reads the same input — the colour as it arrived at this +/// operation — and the pixel moves from the global result by each layer's +/// difference from it: `c_g + Σ w_i (c_i − c_g)`. At full weight that is the +/// layer's combined setting exactly, at zero it is the global result exactly, +/// and two overlapping layers add their changes rather than one repainting +/// the other. +/// +/// With no layer touching the operation this is the block the composer always +/// emitted, byte for byte: a photograph with no masks compiles to the shader +/// it did before layers were offsets. +fn local_block(global: &str, local: &[&crate::mask::LocalOp]) -> String { + let mut out = String::new(); + let _ = writeln!(out, " {{"); + if local.is_empty() { + for line in global.lines() { + let _ = writeln!(out, " {line}"); + } + let _ = writeln!(out, " }}"); + return out; + } + let _ = writeln!(out, " let local_in = c;"); + let _ = writeln!(out, " {{"); + for line in global.lines() { + let _ = writeln!(out, " {line}"); + } + let _ = writeln!(out, " }}"); + let _ = writeln!(out, " let local_global = c;"); + let _ = writeln!(out, " var local_sum = c;"); + for l in local { + let w = format!("mask_w{}", l.slot); + // Skipping where the mask is empty is most of the point of a local + // adjustment: a mask covering a tenth of the frame should cost about a + // tenth of the extra work. Safe as non-uniform control flow — nothing + // inside samples with derivatives or synchronises. + let _ = writeln!(out, " if ({w} > 0.0) {{"); + // Fragments write to a `c` they expect to own, so the layer's version + // gets one of its own, shadowing the outer one and starting from what + // this operation was handed. + let _ = writeln!(out, " var c = local_in;"); + let _ = writeln!(out, " {{"); + for line in l.fragment.lines() { + let _ = writeln!(out, " {line}"); + } + let _ = writeln!(out, " }}"); + let _ = writeln!( + out, + " local_sum = local_sum + {w} * (c - local_global);" + ); + let _ = writeln!(out, " }}"); + } + // Two layers pulling the same way can overshoot below zero, and a + // negative component poisons every operation after this one. + let _ = writeln!(out, " c = max(local_sum, vec3(0.0));"); + let _ = writeln!(out, " }}"); + out +} + /// The WGSL converting linear sRGB into the output space's primaries. /// /// A constant matrix rather than a uniform: the space is chosen when the