dtourolle/DarkRoom
1
0
Fork 0
Code Issues 39 Pull Requests Actions Packages Projects Releases 20 Wiki Activity

Merge branch 'master' into worktree-faces-scrfd-mbf
Build and test / Desktop (Linux) (push) Failing after 25s
Details
Build and test / Layer separation (push) Successful in 22s
Details
Traceability / Requirement traces (push) Successful in 58s
Details
🐳 Android image / Build and push (push) Successful in 1s
Details
Build and test / android-image (push) Successful in 1s
Details
Build and test / Android (aarch64) (push) Failing after 33m26s
Details

Browse Source 
# Conflicts:
#	docs/traceability.md
#	ui/dr-ui/src/develop.rs
#	ui/dr-ui/src/segmentation.rs
This commit is contained in:
dtourolle
2026-08-27 11:57:38 +02:00
parent 25c88d9dbd 7e1c33ebed
commit d777f7f44d
51 changed files with 7732 additions and 586 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ui/dr-ui/src/develop.rs
+648 -73
View File
File diff suppressed because it is too large Load Diff
ui/dr-ui/src/labels.rs
+243 -56
View File
@@ -9,47 +9,153 @@
//! a derived label, before anyone writes a translation for it. That is the
//! behaviour FR-DEV-3c promises: adding an operation needs no UI change.
/// TRACES: FR-DEV-5
/// The history steps this interface records that no descriptor can name.
///
/// Constants rather than string literals at the call sites, and [`ALL`] rather
/// than a list written out again in a test. A key spelled one way where the
/// step is recorded and another way where it is catalogued resolves through
/// `derive` to something *plausible* — "Mask Toggled" — so the mistake does
/// not look like one. Naming each key once removes the opportunity.
///
/// A step that moved a parameter or an operation is deliberately not here: it
/// is named out of the descriptor, through the same `op.` and `param.` entries
/// the develop panel resolves. That is what lets an operation added as a YAML
/// declaration appear in the history correctly named with nothing written for
/// it here (FR-DEV-3c).
pub mod step {
use dr_pipeline::LocalizedKey;
pub const PASTE: LocalizedKey = LocalizedKey("history.paste");
pub const FILM: LocalizedKey = LocalizedKey("history.film");
pub const RESET_ALL: LocalizedKey = LocalizedKey("history.reset_all");
pub const RESET_OP: LocalizedKey = LocalizedKey("history.reset_op");
pub const RESET_PARAM: LocalizedKey = LocalizedKey("history.reset_param");
pub const RESET_FRAMING: LocalizedKey = LocalizedKey("history.reset_framing");
pub const ROTATE: LocalizedKey = LocalizedKey("history.rotate");
pub const FLIP_H: LocalizedKey = LocalizedKey("history.flip_h");
pub const FLIP_V: LocalizedKey = LocalizedKey("history.flip_v");
pub const MASK_ADDED: LocalizedKey = LocalizedKey("history.mask_added");
pub const MASK_REMOVED: LocalizedKey = LocalizedKey("history.mask_removed");
pub const MASK_TOGGLED: LocalizedKey = LocalizedKey("history.mask_toggled");
pub const MASK_INVERTED: LocalizedKey = LocalizedKey("history.mask_inverted");
pub const MASK_MOVED: LocalizedKey = LocalizedKey("history.mask_moved");
pub const MASK_FEATHER: LocalizedKey = LocalizedKey("history.mask_feather");
pub const MASK_MORPH: LocalizedKey = LocalizedKey("history.mask_morph");
pub const MASK_OPACITY: LocalizedKey = LocalizedKey("history.mask_opacity");
pub const MASK_FALLOFF: LocalizedKey = LocalizedKey("history.mask_falloff");
pub const MASK_MORPHOLOGY: LocalizedKey = LocalizedKey("history.mask_morphology");
/// TRACES: FR-DEV-8
/// A repair's own settings changed — its radius, its source, its opacity.
pub const SPOT: LocalizedKey = LocalizedKey("history.spot");
pub const SPOT_PLACED: LocalizedKey = LocalizedKey("history.spot_placed");
pub const SPOT_MOVED: LocalizedKey = LocalizedKey("history.spot_moved");
pub const SPOT_REMOVED: LocalizedKey = LocalizedKey("history.spot_removed");
/// Every step above, plus the two the core names for itself.
///
/// Exists so the catalogue can be checked against the keys that are
/// actually recorded rather than against a second copy of them. A new step
/// left out of this list is a step with no test — which is the failure
/// this whole arrangement is guarding, so it is worth saying plainly:
/// **add the constant to this slice.**
///
/// Test-only, because checking is the whole of what it is for. Resolving a
/// key at runtime goes through [`super::resolve`] one key at a time and
/// never needs the roll.
#[cfg(test)]
pub const ALL: &[LocalizedKey] = &[
dr_pipeline::history::OPENED,
dr_pipeline::history::UNNAMED,
PASTE,
FILM,
RESET_ALL,
RESET_OP,
RESET_PARAM,
RESET_FRAMING,
ROTATE,
FLIP_H,
FLIP_V,
MASK_ADDED,
MASK_REMOVED,
MASK_TOGGLED,
MASK_INVERTED,
MASK_MOVED,
MASK_FEATHER,
MASK_MORPH,
MASK_OPACITY,
MASK_FALLOFF,
MASK_MORPHOLOGY,
SPOT,
SPOT_PLACED,
SPOT_MOVED,
SPOT_REMOVED,
];
}
/// Resolve a key, deriving a fallback where none is catalogued.
pub fn resolve(key: &str) -> String {
match key {
catalogued(key).map_or_else(|| derive(key), str::to_string)
}
/// The label this build has actually chosen for `key`, if it has chosen one.
///
/// Split out from [`resolve`] so that "is this catalogued?" can be asked, and
/// it is asked by the test that guards the history rows. Those are read as a
/// column of short phrases naming decisions, and `derive` is not up to that
/// job even when it produces real words: it turns `history.mask_toggled` into
/// "Mask Toggled", which is a description of a field rather than of something
/// the photographer did — and, being perfectly readable, is a mistake nobody
/// would look at twice.
///
/// Deriving stays right for `op.` and `param.`, where an operation added as a
/// YAML declaration must show up usable before anyone writes its translation
/// (FR-DEV-3c). The difference is that those keys name a thing and these name
/// an act.
fn catalogued(key: &str) -> Option<&'static str> {
Some(match key {
// Operations
// The attribute names. Short on purpose: these are read as a strip
// of tabs, where a long word crowds out the next one.
"attr.tone" => "Light".into(),
"attr.colour" => "Colour".into(),
"attr.detail" => "Detail".into(),
"attr.optics" => "Optics".into(),
"attr.geometry" => "Geometry".into(),
"attr.effect" => "Effects".into(),
"attr.tone" => "Light",
"attr.colour" => "Colour",
"attr.detail" => "Detail",
"attr.optics" => "Optics",
"attr.geometry" => "Geometry",
"attr.effect" => "Effects",
"op.white_balance" => "White Balance".into(),
"op.exposure" => "Exposure".into(),
"op.highlights_shadows" => "Highlights & Shadows".into(),
"op.blacks_whites" => "Blacks & Whites".into(),
"op.brilliance" => "Brilliance".into(),
"op.vibrance" => "Vibrance".into(),
"op.saturation" => "Saturation".into(),
"op.colour_mixer" => "Colour Mixer".into(),
"op.white_balance" => "White Balance",
"op.exposure" => "Exposure",
"op.highlights_shadows" => "Highlights & Shadows",
"op.blacks_whites" => "Blacks & Whites",
"op.brilliance" => "Brilliance",
"op.vibrance" => "Vibrance",
"op.saturation" => "Saturation",
"op.colour_mixer" => "Colour Mixer",
// "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(),
"op.capture_sharpen" => "Sharpening",
"op.framing" => "Crop & Rotate",
// Parameters
"param.temperature" => "Temperature".into(),
"param.tint" => "Tint".into(),
"param.exposure" => "Exposure".into(),
"param.highlights" => "Highlights".into(),
"param.shadows" => "Shadows".into(),
"param.blacks" => "Blacks".into(),
"param.whites" => "Whites".into(),
"param.brilliance" => "Brilliance".into(),
"param.vibrance" => "Vibrance".into(),
"param.saturation" => "Saturation".into(),
"param.temperature" => "Temperature",
"param.tint" => "Tint",
"param.exposure" => "Exposure",
"param.highlights" => "Highlights",
"param.shadows" => "Shadows",
"param.blacks" => "Blacks",
"param.whites" => "Whites",
"param.brilliance" => "Brilliance",
"param.vibrance" => "Vibrance",
"param.saturation" => "Saturation",
// What a faceted parameter adjusts — the mixer's three channels.
//
@@ -57,9 +163,9 @@ pub fn resolve(key: &str) -> String {
// "Lum" from the keys. These name a run of twelve rows apiece, and an
// abbreviation at the head of a section is a word the reader has to
// expand every time they scan past it.
"param.channel.hue" => "Hue".into(),
"param.channel.sat" => "Saturation".into(),
"param.channel.lum" => "Luminance".into(),
"param.channel.hue" => "Hue",
"param.channel.sat" => "Saturation",
"param.channel.lum" => "Luminance",
// The tone curve's four curves, which its points are *subject* to.
//
@@ -69,10 +175,10 @@ pub fn resolve(key: &str) -> String {
// colours would derive correctly and are written out beside it anyway,
// since a list where one entry is translated and three are guessed is
// the shape a half-finished translation takes.
"channel.rgb" => "RGB".into(),
"channel.red" => "Red".into(),
"channel.green" => "Green".into(),
"channel.blue" => "Blue".into(),
"channel.rgb" => "RGB",
"channel.red" => "Red",
"channel.green" => "Green",
"channel.blue" => "Blue",
// The hue bands, which a faceted row is *subject* to.
//
@@ -82,32 +188,76 @@ pub fn resolve(key: &str) -> String {
// nothing. These are also the only place a swatch's meaning is written
// down in words, which is what a photographer who cannot separate the
// squares by eye has to go on.
"band.red" => "Red".into(),
"band.orange" => "Orange".into(),
"band.yellow" => "Yellow".into(),
"band.chartreuse" => "Yellow-Green".into(),
"band.green" => "Green".into(),
"band.spring" => "Blue-Green".into(),
"band.cyan" => "Cyan".into(),
"band.azure" => "Azure".into(),
"band.blue" => "Blue".into(),
"band.violet" => "Violet".into(),
"band.magenta" => "Magenta".into(),
"band.rose" => "Rose".into(),
"band.red" => "Red",
"band.orange" => "Orange",
"band.yellow" => "Yellow",
"band.chartreuse" => "Yellow-Green",
"band.green" => "Green",
"band.spring" => "Blue-Green",
"band.cyan" => "Cyan",
"band.azure" => "Azure",
"band.blue" => "Blue",
"band.violet" => "Violet",
"band.magenta" => "Magenta",
"band.rose" => "Rose",
// Framing. "Straighten" rather than "Angle" because that is the task
// the control performs; the number it reports is still degrees.
"param.angle" => "Straighten".into(),
"param.rotation" => "Rotate".into(),
"param.flip_h" => "Flip Horizontal".into(),
"param.flip_v" => "Flip Vertical".into(),
"param.crop_x" => "Crop Left".into(),
"param.crop_y" => "Crop Top".into(),
"param.crop_w" => "Crop Width".into(),
"param.crop_h" => "Crop Height".into(),
"param.angle" => "Straighten",
"param.rotation" => "Rotate",
"param.flip_h" => "Flip Horizontal",
"param.flip_v" => "Flip Vertical",
"param.crop_x" => "Crop Left",
"param.crop_y" => "Crop Top",
"param.crop_w" => "Crop Width",
"param.crop_h" => "Crop Height",
other => derive(other),
}
// TRACES: FR-DEV-5
// The steps that are not a parameter moving.
//
// Catalogued rather than derived because these are read as a *column*
// of short phrases, where `derive` would give "Mask Added" and "Reset
// Op" — the second of which names a function rather than an action,
// and the first of which reads as a label for a thing rather than for
// something the photographer did. A history is a list of decisions, so
// the rows are written as decisions.
//
// A step naming an operation or a parameter is *not* here: it resolves
// through the same `op.` and `param.` entries above that the panel
// uses, which is what lets an operation added as a YAML declaration
// appear in the history correctly named with no entry written for it
// (FR-DEV-3c).
"history.opened" => "Opened",
"history.edit" => "Edit",
"history.paste" => "Paste Settings",
"history.film" => "Film Stock",
"history.reset_all" => "Reset Everything",
"history.reset_op" => "Reset Group",
"history.reset_param" => "Reset Control",
"history.reset_framing" => "Reset Crop & Rotate",
"history.rotate" => "Rotate",
"history.flip_h" => "Flip Horizontal",
"history.flip_v" => "Flip Vertical",
"history.mask_added" => "Add Mask",
"history.mask_removed" => "Delete Mask",
"history.mask_toggled" => "Mask On/Off",
"history.mask_inverted" => "Invert Mask",
"history.mask_moved" => "Move Mask",
"history.mask_feather" => "Mask Feather",
"history.mask_morph" => "Mask Edge",
"history.mask_opacity" => "Mask Opacity",
"history.mask_falloff" => "Mask Falloff",
"history.mask_morphology" => "Mask Grow/Shrink",
"history.spot" => "Adjust Repair",
"history.spot_placed" => "Add Repair",
"history.spot_moved" => "Move Repair",
"history.spot_removed" => "Delete Repair",
_ => return None,
})
}
/// Turn `op.some_new_thing` into `Some New Thing`.
@@ -137,6 +287,43 @@ fn derive(key: &str) -> String {
mod tests {
use super::*;
#[test]
fn every_step_the_interface_records_has_a_name() {
// A history is only as useful as its rows are distinguishable, and
// these are the ones no descriptor can name — the resets, the flips,
// the mask actions. A key missing from the catalogue derives something
// readable rather than nothing, so the failure this guards is subtler:
// two different steps deriving the *same* word, or a word that names
// the function that ran rather than the decision the user made.
let mut seen: Vec<String> = Vec::new();
for key in step::ALL.iter().map(|k| k.0) {
let label = catalogued(key).unwrap_or_else(|| {
panic!(
"{key} is not catalogued; it would fall through to `derive` \
and read as {:?}, which names a field rather than an act",
derive(key)
)
});
assert!(!label.is_empty(), "{key} resolved to nothing");
assert!(
!seen.contains(&label.to_string()),
"{key} shares the label {label:?} with an earlier step"
);
seen.push(label.to_string());
}
}
#[test]
fn a_step_naming_an_operation_borrows_the_panels_label() {
// What keeps the history honest as the pipeline grows: a step that
// moved a parameter is named by the same entry the slider beside it
// is, so the two can never disagree, and an operation nobody has
// catalogued yet still reads correctly in both places.
assert_eq!(resolve("op.white_balance"), "White Balance");
assert_eq!(resolve("op.tone_curve"), "Tone Curve");
assert_eq!(resolve("param.exposure"), "Exposure");
}
#[test]
fn catalogued_keys_resolve_to_their_label() {
assert_eq!(resolve("op.white_balance"), "White Balance");
ui/dr-ui/src/lib.rs
+122 -3
View File
@@ -43,9 +43,10 @@ mod segmentation;
mod settings_store;
mod settings_ui;
mod sidecar_cache;
mod spots_ui;
mod trash;
use std::cell::RefCell;
use std::cell::{Cell, RefCell};
use std::path::{Path, PathBuf};
use std::rc::Rc;
@@ -295,6 +296,11 @@ fn reset_view_state(window: &AppWindow) {
// overlay or the crosshair to the next one would offer a selection of
// regions that are not in the picture on screen.
masks_ui::reset(window);
// TRACES: FR-DEV-8
// The repairs belong to one photograph too. The next one's arrive with its
// sidecar a moment later, and until they do the canvas must not be showing
// the last one's.
spots_ui::reset(window);
}
/// Push the framing back to the geometry panel.
@@ -738,6 +744,7 @@ enum PointsUpdate {
///
/// `None` means develop is unavailable and the viewer falls back to embedded
/// previews — the same degradation as a machine with no adapter at all.
#[cfg(not(target_os = "android"))]
fn shared_gpu() -> Option<dr_gpu::GpuContext> {
let shared = match pollster::block_on(dr_gpu::GpuContext::new_shared()) {
Ok(shared) => shared,
@@ -778,6 +785,45 @@ fn shared_gpu() -> Option<dr_gpu::GpuContext> {
Some(ctx)
}
/// TRACES: FR-DSP-1 | AC-8
/// Open the compute device on Android — and do **not** give it to Slint.
///
/// # Why this platform is different
///
/// The desktop version above exists so one `wgpu::Texture` can be written by
/// the adjust pass and sampled by the compositor without a round-trip. That
/// requires Slint to be drawing with wgpu, and on Android drawing with wgpu
/// means drawing on wgpu's Vulkan swapchain — which hardcodes
/// `preTransform = IDENTITY` (gfx-rs/wgpu#3345).
///
/// On a tablet whose panel is mounted landscape, portrait then presents an
/// unrotated buffer, every present returns `VK_SUBOPTIMAL_KHR`, and the frames
/// arrive torn. Measured on the device: portrait sits on `composition=CLIENT`
/// with `bufferTransform=ROT_270`, landscape on `composition=DEVICE` with
/// `ROT_180`, and only portrait tore. Taking Slint off wgpu — it then uses
/// Skia over OpenGL, where the driver owns the rotation — fixed it.
///
/// # What it costs, and why that is the right trade here
///
/// Without a shared device the display path needs a readback:
/// `AdjustPass::export_pixels` instead of `Image::try_from(texture)`. That is
/// the transfer ARCH §6.1 and AC-8 exist to avoid, and it is still the better
/// bargain on this platform — the alternative is not a faster develop view, it
/// is a torn one, in the orientation a tablet is mostly held in.
///
/// The compute passes are untouched: demosaic and adjust still run on the GPU,
/// on this device. Only the last hop to the screen goes through memory.
#[cfg(target_os = "android")]
fn shared_gpu() -> Option<dr_gpu::GpuContext> {
match pollster::block_on(dr_gpu::GpuContext::new_headless()) {
Ok(ctx) => Some(ctx),
Err(e) => {
log::warn!("no GPU for the develop passes: {e}");
None
}
}
}
/// TRACES: M-13 | M-14
/// Build and run the viewer.
pub fn run(paths: Vec<PathBuf>) -> Result<()> {
@@ -1325,9 +1371,16 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
//
// Called on every slider change, so it must do no more than run the
// adjust pass — the demosaic is not repeated.
// TRACES: FR-DEV-5
// The history revision the panel was last built from. See the use below:
// the list is rebuilt when it would read differently, not when the picture
// is redrawn, and those are very different rates.
let drawn_history: Rc<Cell<Option<u64>>> = Rc::new(Cell::new(None));
let render_now: Render = {
let session = session.clone();
let viewport = viewport.clone();
let drawn_history = drawn_history.clone();
Rc::new(move |window: &AppWindow, draft: bool| {
let mut slot = session.borrow_mut();
let Some(s) = slot.as_mut() else { return };
@@ -1340,6 +1393,20 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
window.set_can_undo(s.can_undo());
window.set_can_redo(s.can_redo());
// TRACES: FR-DEV-5 | FR-DEV-7
// The list, on the same path and for the same reason — but only
// when it would read differently. A drag ends in a redraw per
// frame while folding into one step, so an unconditional rebuild
// here would tear down and recreate every row of the panel sixty
// times a second to arrive back at the list already on screen.
let revision = s.history_revision();
if drawn_history.get() != Some(revision) {
drawn_history.set(Some(revision));
window
.set_history_rows(slint::ModelRc::new(slint::VecModel::from(s.history_rows())));
window.set_undo_label(s.undo_label().into());
}
// TRACES: FR-DEV-3
// Which part of the region overlay the view is showing. Here
// rather than in the panel's own sync because a pan or a zoom
@@ -1347,6 +1414,18 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
// those ends in a redraw.
masks_ui::sync_overlay_view(window, s);
// TRACES: FR-DEV-8
// And where the repairs are drawn, for the same reason: a pan or a
// zoom moves every circle while touching no repair, and a circle
// left where the mark used to be is worse than no circle at all.
spots_ui::sync_handles(window, s);
// The column's own numbers, pushed from here as well so that a
// photograph opened with repairs already on it arrives with the
// panel describing them — the sidecar lands after the callbacks
// have all been installed, and a redraw is the one path every
// arrival takes.
spots_ui::sync_panel(window, s);
let (mut w, mut h) = *viewport.borrow();
// **Half resolution while the gesture is still moving.**
@@ -2050,6 +2129,7 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
}
masks_ui::wire(&window, &session, &rows, &redraw);
spots_ui::wire(&window, &session, redraw.clone());
// A way to land on the Identity Manager at startup, for looking at it
// without a mouse. Off unless the variable is set, so it costs a getenv
@@ -2139,7 +2219,7 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
.and_then(dr_film::find)
.and_then(dr_film::default_print)
.is_some();
s.choose_film(*stock, print);
s.pick_film(*stock, print);
}
sync_rows(&w, &rows, &session);
redraw(&w);
@@ -2153,7 +2233,7 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
let Some(w) = weak.upgrade() else { return };
if let Some(s) = session.borrow_mut().as_mut() {
if let Some((stock, _)) = s.film().map(|(a, b)| (a.to_string(), b)) {
s.choose_film(Some(&stock), print);
s.pick_film(Some(&stock), print);
}
}
sync_film(&w, &session);
@@ -2199,6 +2279,27 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
});
}
// TRACES: FR-DEV-5 | FR-DEV-7
// Clicking a row. Arriving six steps away costs what arriving from one
// does, because a step is a whole state — see `History::go_to`.
{
let weak = window.as_weak();
let session = session.clone();
let redraw = redraw.clone();
let rows = rows.clone();
window.on_history_picked(move |index| {
let Some(w) = weak.upgrade() else { return };
let stepped = session
.borrow_mut()
.as_mut()
.is_some_and(|s| s.go_to_history(index));
if stepped {
sync_rows(&w, &rows, &session);
redraw(&w);
}
});
}
// ---- zoom, pan and crop ---------------------------------------------
//
// Zoom and pan are viewing state and touch no parameter, so unlike the
@@ -2275,14 +2376,32 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
w.set_crop_h(c.height);
}
ViewMode::Local => s.set_overlay(true),
// TRACES: FR-DEV-8
// Nothing to arm: the circles are drawn whenever there are
// repairs, and what the mode changes is whether a click on
// the photograph makes another one. Leaving the mask
// selection behind would re-point the column at a layer's
// chain while the canvas is showing repairs, which is the
// fault this whole strip exists to prevent.
ViewMode::Spots => {
s.set_overlay(false);
s.set_active_mask(None);
}
ViewMode::Photo => {
s.set_overlay(false);
s.set_active_mask(None);
// A repair stays on the photograph; only the *selection*
// goes, so the source circle does not hang about over a
// frame nobody is repairing any more.
s.select_spot(None);
}
}
}
w.set_view_mode(mode);
masks_ui::sync(&w, &session);
if let Some(s) = session.borrow().as_ref() {
spots_ui::sync_handles(&w, s);
}
// The scope may have just changed, so the panel below is now
// describing a different chain.
sync_rows(&w, &rows, &session);
ui/dr-ui/src/presets.rs
+3 -2
View File
@@ -606,7 +606,7 @@ mod tests {
let version = sidecar.default_version().expect("a default version");
let mut reopened = EditGraph::default_chain();
version.apply(&mut reopened);
version.apply(&mut reopened).expect_no_film();
assert_eq!(
reopened.masks().len(),
@@ -646,7 +646,8 @@ mod tests {
sidecar
.default_version()
.expect("a version")
.apply(&mut restored);
.apply(&mut restored)
.expect_no_film();
assert_eq!(restored.param(exposure::ID, exposure::EXPOSURE), Some(1.5));
}
ui/dr-ui/src/segmentation.rs
+237 -11
View File
@@ -34,6 +34,7 @@ use std::sync::Arc;
use dr_gpu::GpuContext;
use dr_pipeline::mask::segmentation_signature;
use dr_types::Orientation;
/// One recognised object.
#[derive(Debug, Clone)]
@@ -243,27 +244,41 @@ impl Default for Options {
/// Find what can be selected in this photograph.
///
/// `rgb` is the proxy the model reads — tightly packed RGB floats at
/// `(width, height)`. Passed in rather than derived here because the caller
/// already has the rendered proxy, and re-deriving it would mean a second
/// readback of something the CPU is holding.
/// `rgb` is the rendered proxy — tightly packed RGB floats at
/// `(width, height)`, in the **sensor's** own orientation. Passed in rather
/// than derived here because the caller already has it, and re-deriving it
/// would mean a second readback of something the CPU is holding.
///
/// `orientation` is the file's EXIF tag composed with whatever turns the
/// photographer has since applied — `Framing::effective_orientation`, one
/// permutation covering both. The model is shown the picture through it and
/// its answers come back without it, so everything this returns is in sensor
/// space exactly as it was before the detector was taught to read.
pub fn compute(
_ctx: &GpuContext,
rgb: &[f32],
width: usize,
height: usize,
orientation: Orientation,
options: &Options,
) -> Result<Segmentation, String> {
let found = detect(rgb, width, height, options.fine)?;
// The model reads the photograph; everything else here speaks sensor.
let (stood_up, uw, uh) = upright(rgb, width, height, orientation);
let found = detect(&stood_up, uw, uh, options.fine)?;
let instances: Vec<InstanceSummary> = found
.iter()
.filter(|i| i.score >= options.confidence)
.map(|i| InstanceSummary {
class_name: i.class_name.clone(),
score: i.score,
mask: quantise(&i.mask),
bbox: i.bbox,
.map(|i| {
let (mask, bbox) = lay_down(&i.mask, i.bbox, uw, uh, orientation);
InstanceSummary {
class_name: i.class_name.clone(),
score: i.score,
// Quantised after the permutation, so the byte stored is a
// rounding of the model's own coverage and not of a copy.
mask: quantise(&mask),
bbox,
}
})
.collect();
@@ -278,21 +293,122 @@ pub fn compute(
// a signature, a layer built against the coarse pass would be silently
// reinterpreted against the fine one. That is a *wrong* mask, which is
// far worse than a stale one, because nothing announces it.
//
// The orientation is in for the same reason and it is not hypothetical:
// turning the photograph changes what the model recognises, so a run
// before a quarter turn and a run after it are different instance
// lists. Two lists that happened to come out the same length would
// otherwise share a signature, and a layer built against the first
// would be silently re-indexed into the second.
options.confidence.to_bits() as u64
^ if options.fine {
0x9E37_79B9_7F4A_7C15
} else {
0
},
}
^ orientation_key(orientation),
);
Ok(Segmentation {
instances,
signature,
// **Sensor space, not the model's.** `lay_down` put every mask back,
// so the grid a stored layer indexes into is the one it always was —
// see `upright` for why the model saw a different one.
proxy: (width, height),
})
}
/// TRACES: FR-DEV-3 | FR-DEV-3h
/// Turn the proxy the way the photographer is looking at it.
///
/// **Why this exists at all.** A camera held sideways writes its sensor rows
/// the way it always does, and the render puts them right by way of
/// `Framing`. The proxy the model reads is deliberately rendered through a
/// *neutral* graph — the detection has to survive an exposure change, or
/// every slider would invalidate the masks built on it — and neutral took the
/// orientation with it. So the detector was handed a portrait frame lying on
/// its side, and a model trained on upright photographs is very bad at those.
/// Measured end to end on one 22 MP frame of two people and a dog: `person
/// 0.36` and nothing else, against `dog 0.82, person 0.61, person 0.49` for
/// the same pixels stood up.
///
/// One line, because the permutation belongs to
/// [`dr_types::Orientation`] and every other consumer goes through the same
/// one — the grid's thumbnails included, which is what makes "upright" mean
/// one thing across the application rather than one thing per caller.
pub(crate) fn upright(
rgb: &[f32],
width: usize,
height: usize,
orientation: Orientation,
) -> (Vec<f32>, usize, usize) {
let (out, w, h) = orientation.into_shown(rgb, width as u32, height as u32, 3);
(out, w as usize, h as usize)
}
/// TRACES: FR-DEV-3
/// Put what the model answered back onto the sensor's grid.
///
/// The counterpart of [`upright`], and the two are always used as a pair: a
/// mask is only ever in the model's frame between those two calls. Returned
/// together rather than as two functions a caller composes, because calling
/// one and forgetting the other is silent — the mask lands a quarter turn off
/// the subject, which reads as a bad detection rather than as a bug.
///
/// `dw`/`dh` are the *shown* dimensions, as [`upright`] returned them.
pub(crate) fn lay_down(
mask: &[f32],
bbox: (f32, f32, f32, f32),
dw: usize,
dh: usize,
orientation: Orientation,
) -> (Vec<f32>, (f32, f32, f32, f32)) {
let (out, sw, sh) = orientation.into_stored(mask, dw as u32, dh as u32, 1);
(out, lay_down_bbox(bbox, dw, dh, sw, sh, orientation))
}
/// [`lay_down`] for a box.
///
/// Normalised on the way in and scaled on the way out, so the turn itself is
/// `Orientation::into_stored_rect` rather than a fourth copy of the corner
/// arithmetic. A box is the one place a permutation can be *nearly* right —
/// the corners land correctly and `x0 > x1` — so the shared map takes the
/// extremes and this only has to say what space it is in.
fn lay_down_bbox(
bbox: (f32, f32, f32, f32),
dw: usize,
dh: usize,
sw: u32,
sh: u32,
orientation: Orientation,
) -> (f32, f32, f32, f32) {
if dw == 0 || dh == 0 {
return bbox;
}
let (fw, fh) = (dw as f32, dh as f32);
let shown = dr_types::ShownRect {
x: bbox.0 / fw,
y: bbox.1 / fh,
width: (bbox.2 - bbox.0) / fw,
height: (bbox.3 - bbox.1) / fh,
};
let stored = orientation.into_stored_rect(shown);
(
stored.x * sw as f32,
stored.y * sh as f32,
(stored.x + stored.width) * sw as f32,
(stored.y + stored.height) * sh as f32,
)
}
/// One of eight transforms, as bits a signature can carry.
fn orientation_key(orientation: Orientation) -> u64 {
u64::from(orientation.quarter_turns)
| (u64::from(orientation.flip_h) << 2)
| (u64::from(orientation.flip_v) << 3)
}
/// Classes a recognised face may put a name on.
///
/// Only these. A face inside a `tv` or a `laptop` is a photograph of someone on
@@ -499,6 +615,116 @@ mod tests {
assert_eq!(quantise(&[-1.0, 2.0]), vec![0, 255]);
}
/// A non-square, wholly asymmetric grid: every pixel is its own index, so
/// any permutation that is not the intended one shows up as a mismatch
/// rather than being hidden by a symmetry.
fn ramp(w: usize, h: usize) -> Vec<f32> {
(0..w * h).flat_map(|i| [i as f32, 0.0, 0.0]).collect()
}
fn red(rgb: &[f32]) -> Vec<f32> {
rgb.chunks_exact(3).map(|p| p[0]).collect()
}
/// The property the whole fix rests on: what the model is shown and what
/// comes back are the same permutation, run in opposite directions. If
/// they ever disagree, every subject mask lands somewhere other than its
/// subject — and looks like a mask while doing it.
#[test]
fn standing_a_frame_up_and_laying_it_down_is_the_identity() {
const W: usize = 5;
const H: usize = 3;
let source = ramp(W, H);
for tag in 1..=8u16 {
let o = Orientation::from_exif(tag);
let (up, uw, uh) = upright(&source, W, H, o);
let (ow, oh) = o.oriented_size(W as u32, H as u32);
assert_eq!(
(uw, uh),
(ow as usize, oh as usize),
"tag {tag}: the upright size is the oriented one"
);
let (back, _) = lay_down(&red(&up), (0.0, 0.0, 1.0, 1.0), uw, uh, o);
assert_eq!(back, red(&source), "tag {tag} did not come back");
}
}
/// The colour channels must travel together. Reading a pixel three times
/// with one index arithmetic mistake gives a plausible image with its
/// channels sheared, which the model would still detect *something* in.
#[test]
fn a_turn_carries_whole_pixels() {
let rgb: Vec<f32> = (0..2 * 3)
.flat_map(|i| [i as f32, i as f32 + 100.0, i as f32 + 200.0])
.collect();
let o = Orientation::from_exif(6);
let (up, uw, uh) = upright(&rgb, 2, 3, o);
assert_eq!((uw, uh), (3, 2));
for p in up.chunks_exact(3) {
assert_eq!(p[1], p[0] + 100.0, "green left its pixel");
assert_eq!(p[2], p[0] + 200.0, "blue left its pixel");
}
}
/// A portrait frame is the case this exists for: the sensor is landscape,
/// the photograph is not, and the model has to be given the photograph.
#[test]
fn a_sideways_frame_reaches_the_model_upright() {
let o = Orientation::from_exif(6);
assert!(!o.is_normal());
let (_, uw, uh) = upright(&ramp(1600, 1066), 1600, 1066, o);
assert_eq!((uw, uh), (1066, 1600), "the model still got a landscape");
}
/// Where the model's box ends up, worked out by hand for the one turn a
/// portrait phone or a sideways body actually writes.
#[test]
fn a_box_comes_back_in_sensor_pixels() {
let o = Orientation::from_exif(6);
// Shown 4x6; the sensor it came from is 6x4.
let bbox = lay_down_bbox((0.0, 0.0, 2.0, 3.0), 4, 6, 6, 4, o);
assert_eq!(bbox, (0.0, 2.0, 3.0, 4.0));
}
/// Whatever a turn does to a box, it must still read low-to-high — a
/// permutation exchanges which corner is which, and the rest of the mask
/// pipeline measures `(x1 - x0)` without checking the sign.
#[test]
fn a_restored_box_keeps_its_corners_in_order() {
for tag in 1..=8u16 {
let o = Orientation::from_exif(tag);
let (sw, sh) = o.oriented_size(9, 6);
let (x0, y0, x1, y1) = lay_down_bbox((1.0, 2.0, 7.0, 5.0), 9, 6, sw, sh, o);
assert!(x0 <= x1, "tag {tag}: x runs backwards");
assert!(y0 <= y1, "tag {tag}: y runs backwards");
// A permutation moves a box; it does not resize one.
let (sw, sh) = o.oriented_size(9, 6);
assert!(x1 <= sw as f32 && y1 <= sh as f32, "tag {tag}: box escaped");
assert!((((x1 - x0) * (y1 - y0)) - 18.0).abs() < 1e-3, "tag {tag}");
}
}
/// Turning the photograph changes what the model recognises, so the two
/// runs are different instance lists. If they could share a signature, a
/// layer built against one would be silently re-indexed into the other —
/// the same failure the tiling flag is in the signature to prevent.
#[test]
fn turning_the_photograph_changes_the_signature() {
let mut seen = std::collections::HashSet::new();
for tag in 1..=8u16 {
let o = Orientation::from_exif(tag);
assert!(
seen.insert(orientation_key(o)),
"tag {tag} shares a key with an earlier one"
);
}
assert_eq!(seen.len(), 8, "eight tags, but some collapsed");
}
#[test]
fn confidence_changes_the_signature() {
// A different threshold is a different instance list, so the indices a
ui/dr-ui/src/spots_ui.rs
+292
View File
@@ -0,0 +1,292 @@
//! TRACES: FR-DEV-8
//! Wiring the repair tool to the develop session.
//!
//! Translation only, exactly as [`crate::masks_ui`] is: circles out in one
//! direction, gestures in the other, and every decision in
//! [`crate::develop::DevelopSession`]. What this module does decide is when the
//! canvas is redrawn and when the panel is rebuilt, and those are the two
//! things that go wrong quietly here — see [`sync_handles`] for the one that
//! took a gesture out from under the finger.
use std::cell::RefCell;
use std::rc::Rc;
use slint::{ComponentHandle as _, Model as _, ModelRc, VecModel};
use crate::develop::DevelopSession;
use crate::{AppWindow, SpotHandle};
/// TRACES: FR-DEV-8 | FR-UI-3
/// Move the circles to where the repairs now are.
///
/// # Why this is not `set_spot_handles(VecModel::from(…))`
///
/// **A fresh model kills the gesture that is moving them.** The circles are a
/// repeater over this model, and handing Slint a new `ModelRc` makes it throw
/// the repeated items away and build new ones — the `TouchArea` holding the
/// pointer included. The drag then dies under the finger with the button still
/// down. `masks_ui::sync_handles` carries the same warning, and `develop.rs`
/// carries it about the parameter rows, where it broke slider drags: this is
/// the third place the same mistake is available, which is why it is written
/// down in all three.
///
/// So the model is kept and its rows are rewritten in place.
pub(crate) fn sync_handles(window: &AppWindow, session: &DevelopSession) {
let next = session.spot_handles();
let model = handle_model();
while model.row_count() > next.len() {
model.remove(model.row_count() - 1);
}
for (i, handle) in next.into_iter().enumerate() {
if i < model.row_count() {
// Only where it actually moved: an unchanged row written back is
// still a change notification.
if model.row_data(i).as_ref() != Some(&handle) {
model.set_row_data(i, handle);
}
} else {
model.push(handle);
}
}
window.set_spot_handles(ModelRc::from(model));
window.set_selected_spot(session.selected_spot_id().unwrap_or_default().into());
}
/// The circles' model, held for the life of the process — one shared identity,
/// for the reason [`sync_handles`] gives.
fn handle_model() -> Rc<VecModel<SpotHandle>> {
thread_local! {
static HANDLES: Rc<VecModel<SpotHandle>> = Rc::new(VecModel::default());
}
HANDLES.with(Clone::clone)
}
/// Take the repairs off the canvas between photographs.
///
/// Emptied rather than replaced, keeping the model's identity for the reason
/// [`sync_handles`] gives.
pub(crate) fn reset(window: &AppWindow) {
let model = handle_model();
while model.row_count() > 0 {
model.remove(model.row_count() - 1);
}
window.set_spot_handles(ModelRc::from(model));
window.set_selected_spot(Default::default());
window.set_spot_count(0);
}
/// Install the repair tool's callbacks.
pub(crate) fn wire(
window: &AppWindow,
session: &Rc<RefCell<Option<DevelopSession>>>,
redraw: Rc<dyn Fn(&AppWindow)>,
) {
// --- placing ----------------------------------------------------------
{
let weak = window.as_weak();
let session = session.clone();
let redraw = redraw.clone();
window.on_spot_placed(move |x, y| {
let Some(w) = weak.upgrade() else { return };
let placed = session
.borrow_mut()
.as_mut()
.and_then(|s| s.place_spot(x, y));
if placed.is_none() {
// The letterbox margin, or a full set. Neither is an error and
// neither should clear the selection: a near-miss that threw
// away what the column was describing would read as hostile.
return;
}
if let Some(s) = session.borrow().as_ref() {
sync_handles(&w, s);
sync_panel(&w, s);
}
sync_undo(&w, &session);
redraw(&w);
});
}
// --- dragging ---------------------------------------------------------
//
// The repair as it stood when the press landed, held for the gesture. A
// drag is applied to *that* rather than accumulated frame by frame: the
// clamps would otherwise compound, so a source dragged past its limit and
// back would not return to where it started, and the result would depend on
// how many pointer events the platform happened to deliver.
let dragging: Rc<RefCell<Option<dr_pipeline::Spot>>> = Rc::new(RefCell::new(None));
{
let weak = window.as_weak();
let session = session.clone();
let redraw = redraw.clone();
let dragging = dragging.clone();
window.on_spot_handle_dragged(move |id, role, from_x, from_y, to_x, to_y| {
let Some(w) = weak.upgrade() else { return };
let origin = dragging.borrow().clone();
let started = session.borrow_mut().as_mut().and_then(|s| {
s.drag_spot(
id.as_str(),
role,
origin.as_ref(),
(from_x, from_y),
(to_x, to_y),
)
});
if started.is_none() {
return;
}
*dragging.borrow_mut() = started;
// Only the circles. Rebuilding the panel on every frame of a drag
// is work for no difference, and a full rebuild would take the
// gesture out from under the finger — see `sync_handles`.
if let Some(s) = session.borrow().as_ref() {
sync_handles(&w, s);
}
redraw(&w);
});
}
{
let weak = window.as_weak();
let session = session.clone();
let dragging = dragging.clone();
window.on_spot_handle_released(move || {
// Forgotten on release, so the next gesture measures from wherever
// this one left the repair.
if dragging.borrow_mut().take().is_none() {
// A press with no movement — a selection, not a drag. Recording
// a step would put an identical snapshot on the undo stack.
return;
}
if let Some(s) = session.borrow_mut().as_mut() {
s.commit_spot_drag();
}
if let Some(w) = weak.upgrade() {
sync_undo(&w, &session);
}
});
}
// --- the column's controls --------------------------------------------
//
// One closure shape, four callbacks: each hands the session a change to
// make to whichever repair is selected, and the session decides whether
// there is one. Reading the selection here instead would put the same
// `Option` check in four places and let them disagree.
macro_rules! control {
($install:ident, $apply:expr) => {{
let weak = window.as_weak();
let session = session.clone();
let redraw = redraw.clone();
window.$install(move |value| {
let Some(w) = weak.upgrade() else { return };
let changed = session
.borrow_mut()
.as_mut()
.is_some_and(|s| s.set_selected_spot(|spot| $apply(spot, value)));
if !changed {
return;
}
if let Some(s) = session.borrow().as_ref() {
sync_handles(&w, s);
sync_panel(&w, s);
}
sync_undo(&w, &session);
redraw(&w);
});
}};
}
control!(on_spot_radius_changed, |spot: &mut dr_pipeline::Spot, v| {
spot.set_radius(v)
});
control!(
on_spot_feather_changed,
|spot: &mut dr_pipeline::Spot, v| { spot.set_feather(v) }
);
control!(
on_spot_opacity_changed,
|spot: &mut dr_pipeline::Spot, v| { spot.set_opacity(v) }
);
control!(on_spot_mode_picked, |spot: &mut dr_pipeline::Spot, i| {
spot.mode = if i == 1 {
dr_pipeline::SpotMode::Clone
} else {
dr_pipeline::SpotMode::Heal
}
});
// --- selecting and removing -------------------------------------------
{
let weak = window.as_weak();
let session = session.clone();
let redraw = redraw.clone();
window.on_spot_selected(move |id| {
let Some(w) = weak.upgrade() else { return };
if let Some(s) = session.borrow_mut().as_mut() {
s.select_spot(Some(id.as_str()));
}
if let Some(s) = session.borrow().as_ref() {
sync_handles(&w, s);
sync_panel(&w, s);
}
// The canvas changes — the selected repair gains its source circle
// — but no pixel does, so this is a repaint of the overlay rather
// than of the photograph. `redraw` is the only hook there is, and
// it is cheap enough: the render caches on the invalidation key,
// which a selection does not move.
redraw(&w);
});
}
{
let weak = window.as_weak();
let session = session.clone();
let redraw = redraw.clone();
window.on_spot_removed(move |id| {
let Some(w) = weak.upgrade() else { return };
let removed = session
.borrow_mut()
.as_mut()
.is_some_and(|s| s.remove_spot(id.as_str()));
if !removed {
return;
}
if let Some(s) = session.borrow().as_ref() {
sync_handles(&w, s);
sync_panel(&w, s);
}
sync_undo(&w, &session);
redraw(&w);
});
}
}
/// TRACES: FR-DEV-8
/// Push the selected repair's settings into the column.
///
/// Separate from [`sync_handles`] because they answer different questions and
/// change at different times: the circles move on every frame of a drag and on
/// every pan, and these move when a repair is selected or a control is used.
pub(crate) fn sync_panel(window: &AppWindow, session: &DevelopSession) {
window.set_spot_count(session.spot_count() as i32);
if let Some(spot) = session.selected_spot() {
window.set_spot_radius(spot.radius);
window.set_spot_feather(spot.feather);
window.set_spot_opacity(spot.opacity);
window.set_spot_mode(match spot.mode {
dr_pipeline::SpotMode::Heal => 0,
dr_pipeline::SpotMode::Clone => 1,
});
}
}
/// Push whether undo and redo have anywhere to go.
///
/// Every repair is a history step, so every one of them moves these — and a
/// disabled undo button after an edit that *is* undoable is the kind of small
/// lie that stops people trusting the button at all.
fn sync_undo(window: &AppWindow, session: &Rc<RefCell<Option<DevelopSession>>>) {
window.set_can_undo(session.borrow().as_ref().is_some_and(|s| s.can_undo()));
window.set_can_redo(session.borrow().as_ref().is_some_and(|s| s.can_redo()));
}