Merge branch 'worktree-agent-a86b971be6ee42cf1' into integration
This commit is contained in:
+34
-4
@@ -1055,11 +1055,20 @@ impl DevelopSession {
|
||||
/// The mask array is rasterised in source space at proxy size and sampled
|
||||
/// through the framing map, so one array is correct at every output size:
|
||||
/// a 256px thumbnail and a 24 MP export bind the same texture.
|
||||
///
|
||||
/// `space` is the output space `shader` was composed for, and it has to be
|
||||
/// passed rather than assumed because the **detail stage** is composed
|
||||
/// here too and the two halves must agree. When an edit has an active
|
||||
/// neighbourhood operation the fused pass stops at unclipped linear
|
||||
/// working values and the last detail pass performs the output transform;
|
||||
/// composing the fused half for Display P3 and the detail half for sRGB
|
||||
/// would encode the export in the wrong space, with nothing to notice it.
|
||||
fn render_with_masks(
|
||||
&mut self,
|
||||
shader: &dr_pipeline::operation::ComposedShader,
|
||||
w: u32,
|
||||
h: u32,
|
||||
space: dr_types::ColourSpace,
|
||||
) -> Result<(), String> {
|
||||
let ctx = self.ctx.clone();
|
||||
self.ensure_subject_fields(&ctx);
|
||||
@@ -1069,8 +1078,29 @@ impl DevelopSession {
|
||||
.then(|| self.masks.as_ref().and_then(|p| p.array()))
|
||||
.flatten();
|
||||
|
||||
// The neighbourhood stage, composed at the size actually being drawn.
|
||||
//
|
||||
// It has to be composed *per render* rather than cached with the edit,
|
||||
// because a kernel is the one thing in this pipeline that is not
|
||||
// scale-free: a sharpening radius is stated in source pixels and the
|
||||
// develop view renders at whatever the viewport needs (FR-DSP-1), so
|
||||
// the conversion is different for the canvas, the thumbnail and the
|
||||
// export. `render_scale` works the ratio out from the framing, which
|
||||
// is also what makes zooming to 1:1 restore an exact preview with no
|
||||
// second render path to maintain.
|
||||
//
|
||||
// Empty for every edit with no active neighbourhood operation — which
|
||||
// is almost all of them — and `render_detailed` then falls straight
|
||||
// through to the single masked dispatch this used to call.
|
||||
let scale = self.graph.render_scale(self.demosaiced.size(), (w, h));
|
||||
let detail = self.graph.compose_detail_for(scale, space);
|
||||
let colour_key = self
|
||||
.graph
|
||||
.invalidation()
|
||||
.through(dr_pipeline::Affects::Colour);
|
||||
|
||||
self.adjust
|
||||
.render_masked(&self.demosaiced, shader, w, h, masks)
|
||||
.render_detailed(&self.demosaiced, shader, w, h, masks, &detail, colour_key)
|
||||
.map(|_| ())
|
||||
.map_err(|e| e.to_string())
|
||||
}
|
||||
@@ -1769,7 +1799,7 @@ impl DevelopSession {
|
||||
|
||||
// Rasterise the masks first: the shader addresses array slices by
|
||||
// index, so the array has to describe *this* stack before it is bound.
|
||||
self.render_with_masks(&shader, w, h)?;
|
||||
self.render_with_masks(&shader, w, h, dr_types::ColourSpace::Srgb)?;
|
||||
let texture = self.adjust.output().ok_or("nothing was rendered")?;
|
||||
|
||||
// The import is fallible on format and usage only, and both are fixed
|
||||
@@ -1893,7 +1923,7 @@ impl DevelopSession {
|
||||
let (w, h) = self.graph.output_size(sw, sh);
|
||||
|
||||
let shader = self.graph.compose_for(space);
|
||||
self.render_with_masks(&shader, w, h)?;
|
||||
self.render_with_masks(&shader, w, h, space)?;
|
||||
|
||||
let (pixels, rw, rh) = self.adjust.export_pixels().map_err(|e| e.to_string())?;
|
||||
dr_export::Frame::in_space(rw, rh, pixels, space).map_err(|e| e.to_string())
|
||||
@@ -1920,7 +1950,7 @@ impl DevelopSession {
|
||||
let (w, h) = fit(fw, fh, edge.max(1), edge.max(1));
|
||||
|
||||
let shader = self.graph.compose_for(dr_types::ColourSpace::Srgb);
|
||||
self.render_with_masks(&shader, w, h)?;
|
||||
self.render_with_masks(&shader, w, h, dr_types::ColourSpace::Srgb)?;
|
||||
|
||||
let (pixels, rw, rh) = self.adjust.export_pixels().map_err(|e| e.to_string())?;
|
||||
Ok((rw, rh, pixels))
|
||||
|
||||
@@ -30,6 +30,13 @@ pub fn resolve(key: &str) -> String {
|
||||
"op.vibrance" => "Vibrance".into(),
|
||||
"op.saturation" => "Saturation".into(),
|
||||
"op.colour_mixer" => "Colour Mixer".into(),
|
||||
// "Sharpening" rather than what `derive` would make of the id. The id
|
||||
// says *capture* sharpening to separate it from the output sharpening
|
||||
// an export applies (FR-EXP-4), which is a distinction about where in
|
||||
// the pipeline it sits; in the develop panel there is only one, and
|
||||
// "Capture Sharpen" would name a distinction the photographer cannot
|
||||
// see from there.
|
||||
"op.capture_sharpen" => "Sharpening".into(),
|
||||
"op.framing" => "Crop & Rotate".into(),
|
||||
|
||||
// Parameters
|
||||
@@ -134,6 +141,16 @@ mod tests {
|
||||
fn catalogued_keys_resolve_to_their_label() {
|
||||
assert_eq!(resolve("op.white_balance"), "White Balance");
|
||||
assert_eq!(resolve("param.highlights"), "Highlights");
|
||||
// Catalogued precisely because `derive` would get it wrong: the id
|
||||
// carries a distinction ("capture", as against an export's output
|
||||
// sharpening) that belongs in the pipeline and not on a panel.
|
||||
assert_eq!(resolve("op.capture_sharpen"), "Sharpening");
|
||||
// Its parameters are the opposite case — the derived words are the
|
||||
// right words, so they are left uncatalogued and shared with whatever
|
||||
// asks for an amount or a radius next.
|
||||
assert_eq!(resolve("param.amount"), "Amount");
|
||||
assert_eq!(resolve("param.radius"), "Radius");
|
||||
assert_eq!(resolve("param.threshold"), "Threshold");
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user