Compare commits

..
3 Commits
Author SHA1 Message Date
dtourolle 48c5e74fa8 Release 0.18.1
Benchmarks / CPU and I/O (per commit) (push) Successful in 5m46s
Benchmarks / Frame budget (on demand) (push) Skipped
Traceability / Requirement traces (push) Successful in 49s
Build and test / Android (aarch64) (push) Successful in 17m27s
Build and test / android-image (push) Successful in 3s
🐳 Android image / Build and push (push) Successful in 3s
Build and test / Desktop (Linux) (push) Failing after 25m32s
Build and test / windows-image (push) Successful in 1s
🐳 Windows image / Build and push (push) Successful in 1s
Build and test / Layer separation (push) Successful in 29s
Build and test / Windows (x86_64, cross) (push) Successful in 37m19s
Build and test / Publish the release (push) Skipped
2026-09-26 21:29:26 -04:00
dtourolle fc54523093 Apply a mask layer's settings as offsets to the global ones
A local adjustment ran as a second chain after every global operation,
then blended by the mask. So global contrast -30 with -20 on a face was
contrast -30, the rest of the chain, and contrast -20 again on the
result, rather than -50 where contrast runs. The two edits compounded in
ways neither slider showed; a flattening applied to an already
flattened picture is how the shadows of a night shot went magenta.

A layer's setting is now an offset from its default, added to the
global setting (clamped to the parameter's range; a moved switch or
choice replaces it) and run at that operation's own place in the chain.
At each operation the global fragment and each touching layer's
combined fragment read the same input colour, and the pixel moves by
each layer's weighted difference: c_g + sum w_i (c_i - c_g). At full
weight that is the combined setting exactly, at zero the global result
exactly, and no setting is applied twice. An offset that brings an
operation back to neutral emits an empty version, which undoes the
global setting inside the mask.

Blending the colours rather than the uniforms is deliberate: the tone
curve and colour mixer emit code only for the channels and bands that
are touched, so the global and combined versions of one operation need
not share a uniform set.

A photograph with no masks compiles to the same shader byte for byte.

Test: global -30 with a whole-frame layer at -20 renders within one
count of global -50.
2026-09-26 20:43:02 -04:00
dtourolle 8392cf772e Flatten contrast toward grey instead of scaling shadows by a ratio
Reducing contrast turned every black in a night photograph pink. The
fragment lifted each pixel's luminance to its target by multiplying the
colour by target/luma. For a pixel at 0.001 on the way to 0.09 that is
a gain of ninety, and in the deepest shadows the channels are sensor
noise: after white balance the red and blue noise sits above the green,
their multipliers being nearly twice its, so ninety times that noise is
magenta.

Flattening now mixes the colour toward middle grey, which gives the
same luminance and adds the lift as a neutral. A black goes to grey and
its noise stays the size it was.

The same fragment clamped luma/0.36 into the curve's 0..1 domain, which
scaled every tone above twice middle grey down to 0.36 in either
direction: contrast +10 took a 230 grey to 162. Those tones are now left
where they are, which is continuous with the curve's top (value 1,
slope 0).
2026-09-26 20:43:02 -04:00
10 changed files with 670 additions and 187 deletions
Generated
+25 -25
View File
@@ -1265,7 +1265,7 @@ checksum = "f27ae1dd37df86211c42e150270f82743308803d90a6f6e6651cd730d5e1732f"
[[package]]
name = "darkroom-android"
version = "0.18.0"
version = "0.18.1"
dependencies = [
"android_logger",
"dr-plat",
@@ -1278,7 +1278,7 @@ dependencies = [
[[package]]
name = "darkroom-desktop"
version = "0.18.0"
version = "0.18.1"
dependencies = [
"anyhow",
"dr-plat",
@@ -1454,7 +1454,7 @@ checksum = "d8b14ccef22fc6f5a8f4d7d768562a182c04ce9a3b3157b91390b52ddfdf1a76"
[[package]]
name = "dr-bench"
version = "0.18.0"
version = "0.18.1"
dependencies = [
"anyhow",
"dr-catalog",
@@ -1471,7 +1471,7 @@ dependencies = [
[[package]]
name = "dr-catalog"
version = "0.18.0"
version = "0.18.1"
dependencies = [
"dr-face",
"dr-plat",
@@ -1486,7 +1486,7 @@ dependencies = [
[[package]]
name = "dr-decode"
version = "0.18.0"
version = "0.18.1"
dependencies = [
"dr-types",
"env_logger",
@@ -1500,7 +1500,7 @@ dependencies = [
[[package]]
name = "dr-export"
version = "0.18.0"
version = "0.18.1"
dependencies = [
"dr-decode",
"dr-gpu",
@@ -1519,7 +1519,7 @@ dependencies = [
[[package]]
name = "dr-face"
version = "0.18.0"
version = "0.18.1"
dependencies = [
"dr-inference-engine",
"env_logger",
@@ -1532,7 +1532,7 @@ dependencies = [
[[package]]
name = "dr-film"
version = "0.18.0"
version = "0.18.1"
dependencies = [
"log",
"serde",
@@ -1541,7 +1541,7 @@ dependencies = [
[[package]]
name = "dr-gpu"
version = "0.18.0"
version = "0.18.1"
dependencies = [
"bytemuck",
"dr-decode",
@@ -1559,7 +1559,7 @@ dependencies = [
[[package]]
name = "dr-inference-engine"
version = "0.18.0"
version = "0.18.1"
dependencies = [
"env_logger",
"libloading",
@@ -1574,7 +1574,7 @@ dependencies = [
[[package]]
name = "dr-ingest"
version = "0.18.0"
version = "0.18.1"
dependencies = [
"dr-plat",
"dr-types",
@@ -1586,7 +1586,7 @@ dependencies = [
[[package]]
name = "dr-lens"
version = "0.18.0"
version = "0.18.1"
dependencies = [
"lensfun",
"log",
@@ -1594,7 +1594,7 @@ dependencies = [
[[package]]
name = "dr-pano"
version = "0.18.0"
version = "0.18.1"
dependencies = [
"dr-decode",
"dr-inference-engine",
@@ -1608,7 +1608,7 @@ dependencies = [
[[package]]
name = "dr-pipeline"
version = "0.18.0"
version = "0.18.1"
dependencies = [
"dr-types",
"log",
@@ -1617,7 +1617,7 @@ dependencies = [
[[package]]
name = "dr-plat"
version = "0.18.0"
version = "0.18.1"
dependencies = [
"android-native-keyring-store",
"dr-types",
@@ -1633,7 +1633,7 @@ dependencies = [
[[package]]
name = "dr-preset-xmp"
version = "0.18.0"
version = "0.18.1"
dependencies = [
"dr-pipeline",
"log",
@@ -1643,7 +1643,7 @@ dependencies = [
[[package]]
name = "dr-segment"
version = "0.18.0"
version = "0.18.1"
dependencies = [
"dr-inference-engine",
"env_logger",
@@ -1656,7 +1656,7 @@ dependencies = [
[[package]]
name = "dr-sync"
version = "0.18.0"
version = "0.18.1"
dependencies = [
"async-trait",
"dr-plat",
@@ -1670,7 +1670,7 @@ dependencies = [
[[package]]
name = "dr-sync-folder"
version = "0.18.0"
version = "0.18.1"
dependencies = [
"async-trait",
"dr-sync",
@@ -1682,7 +1682,7 @@ dependencies = [
[[package]]
name = "dr-sync-nextcloud"
version = "0.18.0"
version = "0.18.1"
dependencies = [
"async-trait",
"dr-decode",
@@ -1704,7 +1704,7 @@ dependencies = [
[[package]]
name = "dr-thumbs"
version = "0.18.0"
version = "0.18.1"
dependencies = [
"dr-types",
"jpeg-encoder",
@@ -1716,7 +1716,7 @@ dependencies = [
[[package]]
name = "dr-types"
version = "0.18.0"
version = "0.18.1"
dependencies = [
"serde",
"serde_json",
@@ -1725,7 +1725,7 @@ dependencies = [
[[package]]
name = "dr-ui"
version = "0.18.0"
version = "0.18.1"
dependencies = [
"anyhow",
"async-trait",
@@ -1773,7 +1773,7 @@ dependencies = [
[[package]]
name = "dr-xmp"
version = "0.18.0"
version = "0.18.1"
dependencies = [
"dr-types",
"log",
@@ -7109,7 +7109,7 @@ checksum = "8df9b6e13f2d32c91b9bd719c00d1958837bc7dec474d94952798cc8e69eeec3"
[[package]]
name = "traceability"
version = "0.18.0"
version = "0.18.1"
dependencies = [
"anyhow",
"pulldown-cmark",
+1 -1
View File
@@ -32,7 +32,7 @@ members = [
exclude = ["third_party"]
[workspace.package]
version = "0.18.0"
version = "0.18.1"
edition = "2021"
rust-version = "1.92"
license = "GPL-3.0-or-later"
+1 -1
View File
@@ -93,7 +93,7 @@ controls, its place in the chain and its tests.
## Where it stands
**0.18.0**, twenty-six tagged releases in. 192 numbered requirements in
**0.18.1**, twenty-seven tagged releases in. 192 numbered requirements in
scope, 84% of them claimed by code and [traced to it](docs/dev/traceability.md);
the rest are written down rather than merely absent.
+79
View File
@@ -0,0 +1,79 @@
//! Contrast, on a device, at the two ends of the tonal range.
//!
//! The descriptor tests check that the fragment says the right words; these
//! check what those words do to a pixel. Both failures here passed every
//! descriptor test for months, because each is a property of the arithmetic
//! at the extremes rather than of the shape of the code.
use dr_gpu::{AdjustPass, DemosaicedImage, GpuContext};
use dr_pipeline::descriptor::ParamId;
use dr_pipeline::operation::compose;
use dr_pipeline::ops;
const SIZE: u32 = 8;
fn ctx() -> Option<GpuContext> {
pollster::block_on(GpuContext::new_headless()).ok()
}
/// A flat frame of one sRGB colour, contrast set to `amount`, rendered and
/// read back as the colour of one pixel.
fn render(ctx: &GpuContext, rgb: [u8; 3], amount: f32) -> [u8; 3] {
let data: Vec<u8> = (0..SIZE * SIZE)
.flat_map(|_| [rgb[0], rgb[1], rgb[2], 255])
.collect();
let source = DemosaicedImage::from_rgba8(ctx, &data, SIZE, SIZE).expect("upload");
let mut chain = ops::chain();
let op = chain
.iter_mut()
.find(|o| o.descriptor().id.0 == "contrast")
.expect("contrast is in the chain");
op.set_param(ParamId("contrast"), amount);
let shader = compose(&chain);
let mut adjust = AdjustPass::new(ctx);
adjust.render(&source, &shader, SIZE, SIZE).expect("render");
let pixels = adjust.export_pixels().expect("readback").0;
[pixels[0], pixels[1], pixels[2]]
}
/// **The pink-blacks bug.** A near-black pixel whose red and blue sit a count
/// above its green — what white-balanced sensor noise in a night shadow looks
/// like — must come out grey when contrast is reduced, not magenta.
///
/// The ratio form lifted it by a gain of well over a hundred, and a hundred
/// times a one-count cast is a saturated colour.
#[test]
fn reducing_contrast_lifts_a_black_to_grey_not_to_magenta() {
let Some(ctx) = ctx() else {
eprintln!("no GPU adapter; skipping");
return;
};
let [r, g, b] = render(&ctx, [4, 1, 4], -50.0);
let spread = r.max(g).max(b) - r.min(g).min(b);
assert!(
g > 40,
"a black at half contrast should be lifted toward grey, got ({r}, {g}, {b})"
);
assert!(
spread <= 6,
"the lift must be neutral: ({r}, {g}, {b}) has a cast of {spread}"
);
}
/// **The pinned highlights.** A light tone, above twice middle grey, must not
/// be pulled down to the top of the curve by the smallest positive contrast.
#[test]
fn a_little_contrast_leaves_a_highlight_where_it_was() {
let Some(ctx) = ctx() else {
eprintln!("no GPU adapter; skipping");
return;
};
let before = render(&ctx, [230, 230, 230], 0.0)[1];
let after = render(&ctx, [230, 230, 230], 10.0)[1];
assert!(
after >= before.saturating_sub(2),
"contrast +10 took a highlight from {before} to {after}"
);
}
+73
View File
@@ -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<dyn dr_pipeline::operation::Operation>],
stack: &MaskStack,
field: Option<&LabelField>,
) -> Vec<u8> {
// A ramp, so both ends of the tonal range are in the comparison.
let data: Vec<u8> = (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<Box<dyn dr_pipeline::operation::Operation>> {
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}"
);
}
+62 -28
View File
@@ -35,41 +35,62 @@ helpers: [luminance, apply_tone_gain]
define:
contrast_curve: |
// A symmetric S-curve on a 0..1 perceptual position.
// The steepening S, on a 0..1 perceptual position.
//
// `amount` above zero steepens, below zero flattens. The smoothstep form is
// used for the steepening direction because it has zero gradient at both
// ends, so the curve cannot invert however hard it is pushed — the failure
// that makes naive gain-about-a-pivot unusable past moderate settings.
// Blends toward a smoothstep, which has zero gradient at both ends, so the
// curve cannot invert however hard it is pushed — the failure that makes
// naive gain-about-a-pivot unusable past moderate settings. Only the
// positive direction comes here: flattening is not a curve at all (see
// the fragment).
fn contrast_curve(x: f32, amount: f32) -> f32 {
let clamped = clamp(x, 0.0, 1.0);
if (amount >= 0.0) {
// Blend toward a smoothstep, which is the S.
let s = clamped * clamped * (3.0 - 2.0 * clamped);
return mix(clamped, s, amount);
}
// Flattening: pull toward the mid-point. At amount = -1 every tone
// collapses to 0.5, which is the meaningful limit of 'no contrast'.
return mix(clamped, 0.5, -amount);
let s = clamped * clamped * (3.0 - 2.0 * clamped);
return mix(clamped, s, amount);
}
wgsl: |
let luma = luminance(c);
if (luma > 0.0001) {
// Work on luminance and rescale the colour by the ratio, rather than
// curving each channel independently. Per-channel contrast shifts hue
// wherever the channels differ — the classic symptom being skies going
// cyan as contrast rises.
if (amount < 0.0) {
// **Flattening mixes toward middle grey; it does not scale.**
//
// MIDDLE_GREY is 0.18: the linear value the eye reads as mid-tone. The
// curve operates on luma/(2*0.18) so that middle grey lands at the
// curve's own 0.5 pivot.
let pos = clamp(luma / 0.36, 0.0, 1.0);
let curved = contrast_curve(pos, amount);
// Not `target`: that is a WGSL reserved keyword, and using it produces a
// parse error in generated code rather than anywhere a reader would look.
let curved_luma = curved * 0.36;
c = apply_tone_gain(c, curved_luma / luma);
// Every tone moves the same fraction of the way to 0.18, which at -1
// collapses the picture to grey — the meaningful limit of 'no contrast'.
// In luminance this is exactly what the ratio form below would compute,
// but the ratio form reaches it by multiplying: a pixel at 0.001 has to
// be lifted to 0.09, a gain of ninety, and in the deepest shadows the
// channels are sensor noise, not a colour. After white balance the red
// and blue noise sits above the green (their multipliers are nearly
// twice its), so ninety times that noise is magenta — every black in the
// frame turned pink. Mixing adds the lift as a neutral, so a black goes
// to grey and its noise stays the size it was.
//
// The grey is (1, 1, 1) scaled, because this runs after white balance
// in the camera's space, where that is what neutral is.
c = mix(c, vec3<f32>(0.18), -amount);
} else {
let luma = luminance(c);
// Only up to twice middle grey, which is the curve's whole domain. Above
// it the curve's value is 1 and its slope 0, so leaving those tones
// alone is the continuous continuation — where scaling them to the
// curve's top, as this once did through a clamp, pinned every highlight
// in the photograph to 0.36 at the smallest touch of the slider.
if (luma > 0.0001 && luma < 0.36) {
// Work on luminance and rescale the colour by the ratio, rather than
// curving each channel independently. Per-channel contrast shifts hue
// wherever the channels differ — the classic symptom being skies going
// cyan as contrast rises. Safe here where it was not for flattening:
// the S only ever pulls a shadow down, so the gain is at most one
// below the pivot and noise is never amplified.
//
// MIDDLE_GREY is 0.18: the linear value the eye reads as mid-tone.
// The curve operates on luma/(2*0.18) so that middle grey lands at
// the curve's own 0.5 pivot.
let pos = luma / 0.36;
let curved = contrast_curve(pos, amount);
// Not `target`: that is a WGSL reserved keyword, and using it produces a
// parse error in generated code rather than anywhere a reader would look.
let curved_luma = curved * 0.36;
c = apply_tone_gain(c, curved_luma / luma);
}
}
c = max(c, vec3<f32>(0.0));
@@ -104,6 +125,19 @@ tests:
propagate through everything downstream.
expect_wgsl: ["luma > 0.0001"]
- name: flattening_mixes_toward_grey_rather_than_scaling
why: |
Lifting a shadow by a luminance ratio multiplies its noise by the same
ratio — ninety at the bottom of a night photograph — and after white
balance that noise is magenta. A mix adds the lift as a neutral.
expect_wgsl: ["mix(c, vec3<f32>(0.18), -amount)"]
- name: highlights_are_not_pinned_to_the_top_of_the_curve
why: |
The curve covers 0..0.36. A clamp into that range scaled every brighter
pixel down to 0.36; tones above it are left as they are.
expect_wgsl: ["luma < 0.36"]
- name: the_curve_cannot_invert
why: |
A gain-about-a-pivot form produces a non-monotonic curve past moderate
+291 -82
View File
@@ -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<Box<dyn Operation>>,
}
/// 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<f32>,
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<LocalOp>,
pub helpers: Vec<crate::operation::Helper>,
/// 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<dyn Operation>],
) -> 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<dyn Operation>])) -> 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<dyn Operation>], 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<f32>(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]
+125 -37
View File
@@ -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<u32>) {{
}
}
/// 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<f32>(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
File diff suppressed because one or more lines are too long
+1 -1
View File
@@ -4,7 +4,7 @@
# makes `makepkg -si` in this directory install what you are actually working
# on. Swap `source` for a tagged tarball when there is something to release.
pkgname=darkroom
pkgver=0.18.0
pkgver=0.18.1
# Back to 1 with the version: a new pkgver is a new archive name, so there is
# nothing for makepkg to reuse and nothing for a release number to disambiguate.
pkgrel=1