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

Give every mask an eye and a colour, and put the brush where the mask is

Browse Source 
The first build of seeing a mask showed the selected layer's, in one global
style, from a strip at the top of the panel. It answered the wrong question and
answered it somewhere nobody looked. What a photographer asks of two masks is
how they meet — where the sky's edge sits against the building's — and that
needs both on screen at once, in colours that can be told apart.

So each row of the stack has an eye, drawn in the colour its mask is shown in,
and each mask has six swatches to choose that colour from. Several can be open
at once; a new one comes up open, in the first colour nothing else is using.
The style — tint, alpha, outline — is the one setting that stays global, above
the stack, because three styles at once are three pictures that cannot be read
against each other. Alpha now draws every shown mask, each in its colour, on
black. In the pipeline a `Reveal` is a list of `(layer, colour)` rather than
one layer, and every reveal block carries its own colour.

The brush moves too. Select, Paint and Erase and the three sliders under them
sat at the top of the panel, appeared only once a row was selected, and said
nothing about which mask they acted on — so "how do I paint" and "how do I
correct the model's outline" both had the same answer and nobody found it.
They sit under the selected mask's parts now, beside the swatches, and on a
subject or a category the hint says what a stroke there does: it becomes a
part of this mask, joined to the model's, and can be taken out again.

Eyes and colours are viewing state, on the session and not on the layer, so a
photograph reopened has every eye closed — the stored-mask round-trip test
asserts it.
This commit is contained in:
dtourolle
2026-09-11 19:03:51 +02:00
parent 936490880b
commit a87139b838
12 changed files with 759 additions and 287 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ui/dr-ui/src/develop.rs
+183 -64
View File
@@ -683,6 +683,32 @@ impl CropAspect {
}
}
/// TRACES: FR-DEV-19c
/// How one layer's mask is shown on the canvas.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
struct MaskView {
shown: bool,
/// Index into [`MASK_COLOURS`].
colour: usize,
}
/// TRACES: FR-DEV-19c
/// The colours a mask may be shown in, in linear sRGB.
///
/// Six, chosen to be told apart at half strength over a photograph rather
/// than to be pretty: red and green and blue at the corners, and the three
/// between them. Exposed so the panel draws its swatches from the same table
/// the shader is handed, and a seventh colour is one line here and nowhere
/// else.
pub const MASK_COLOURS: [[f32; 3]; 6] = [
[0.85, 0.10, 0.15],
[0.15, 0.80, 0.25],
[0.20, 0.45, 1.00],
[0.95, 0.80, 0.10],
[0.90, 0.20, 0.85],
[0.15, 0.85, 0.90],
];
pub struct DevelopSession {
/// This session's name, for work that outlives the frame it started on.
id: SessionId,
@@ -866,19 +892,27 @@ pub struct DevelopSession {
/// Whether to draw the false-coloured region overlay.
show_overlay: bool,
/// TRACES: FR-DEV-19c
/// How the selected layer's mask is being shown, or `None` for not at all.
/// How shown masks are drawn — one style for all of them.
///
/// Interface state, like `show_overlay` beside it and `active_masks` above
/// — it changes no pixel of the photograph, it is not in the sidecar and
/// it is not on the undo stack. It reaches the pipeline as an argument to
/// the one composition that draws the canvas, which is what makes an
/// export structurally unable to carry it (`EditGraph::compose_revealing`).
reveal_style: dr_pipeline::mask::RevealStyle,
/// TRACES: FR-DEV-19c
/// Per layer: whether its mask is shown, and in what colour.
///
/// The *style* is stored and not the layer, because the layer is always
/// the selected one: a reveal that stayed pointed at a mask nobody was
/// working on would be describing the wrong thing every time the selection
/// moved, and there is no gesture that wants it.
reveal_style: Option<dr_pipeline::mask::RevealStyle>,
/// Per layer rather than "the selected one", because the question a
/// photographer asks of two masks is how they meet — where the sky's edge
/// sits against the building's — and that needs both on screen at once,
/// in colours that can be told apart. Keyed by id, and an id that is no
/// longer in the stack is simply never asked for; `reveal` walks the
/// stack, not this map.
///
/// Viewing state and not edit state, for the reason the style is: it does
/// not travel in a sidecar, so a photograph reopened has every eye closed.
mask_views: std::collections::HashMap<String, MaskView>,
/// Which attribute the panel is filtered to, or all of them.
///
/// `None` is "show everything" and is what a frontend that ignores
@@ -1034,7 +1068,8 @@ impl DevelopSession {
),
selected_spot: None,
show_overlay: false,
reveal_style: None,
reveal_style: dr_pipeline::mask::RevealStyle::Tint,
mask_views: std::collections::HashMap::new(),
active_tab: None,
curve_channel: 0,
display_space: dr_types::ColourSpace::Srgb,
@@ -1981,9 +2016,16 @@ impl DevelopSession {
// beneath it.
let reveal = self.reveal();
mix(reveal.as_ref().map_or(0, |r| {
// Every shown id, not only which are shown: a layer joining the
// shown set renumbers every slot after it, exactly as one joining
// the active set does. Colours are left out — they change what
// the shader draws, not which field it draws through.
let mut h: u64 = 1;
for b in r.layer.as_bytes() {
h = h.wrapping_mul(31).wrapping_add(*b as u64);
for l in &r.layers {
for b in l.layer.as_bytes() {
h = h.wrapping_mul(31).wrapping_add(*b as u64);
}
h = h.wrapping_mul(31).wrapping_add(0x1f);
}
h
}));
@@ -2476,51 +2518,125 @@ impl DevelopSession {
/// TRACES: FR-DEV-19c
/// What the canvas should draw over the photograph, if anything.
///
/// **Only when exactly one layer is selected**, on the same rule the part
/// list and the brush follow: a mask is one thing, and drawing the union
/// of three because three rows were control-clicked would answer a
/// question nobody asked. `None` there rather than picking the first, so
/// the reveal disappearing is itself the signal that the selection is not
/// what it needs to be.
/// Every layer whose eye is open, in stack order, each in its colour —
/// and `None` when no eye is, so the rasteriser and the composer can
/// take the path they always took.
///
/// Rebuilt per call rather than kept in step with the selection, because
/// it is two fields and a clone of one id — cheaper than the invalidation
/// a cached copy would need every time a layer is added, removed, renamed
/// Rebuilt per call rather than kept in step with the stack, because it
/// is a walk over at most eight layers — cheaper than the invalidation a
/// cached copy would need every time a layer is added, removed, renamed
/// or reordered.
pub(crate) fn reveal(&self) -> Option<dr_pipeline::mask::Reveal> {
let style = self.reveal_style?;
let [id] = self.active_masks.as_slice() else {
use dr_pipeline::mask::{Reveal, RevealedLayer};
let layers: Vec<RevealedLayer> = self
.graph
.masks()
.layers()
.iter()
.filter_map(|l| {
let view = self.mask_views.get(&l.id).filter(|v| v.shown)?;
Some(RevealedLayer {
layer: l.id.clone(),
colour: MASK_COLOURS[view.colour % MASK_COLOURS.len()],
})
})
.collect();
if layers.is_empty() {
return None;
};
Some(dr_pipeline::mask::Reveal {
layer: id.clone(),
style,
}
Some(Reveal {
layers,
style: self.reveal_style,
})
}
/// How the selected layer's mask is being shown, as an index into
/// [`dr_pipeline::mask::RevealStyle::ALL`], or `0` for not at all.
/// How shown masks are drawn, as an index into
/// [`dr_pipeline::mask::RevealStyle::ALL`].
///
/// An index because the panel offers it as a strip of chips and an index
/// is what a strip of chips reports. The enum stays the thing that is
/// stored, so a fourth style is a variant and a label rather than a number
/// two files have to agree on.
pub fn mask_view(&self) -> usize {
use dr_pipeline::mask::RevealStyle;
self.reveal_style
.and_then(|s| RevealStyle::ALL.iter().position(|&a| a == s))
.map_or(0, |i| i + 1)
pub fn mask_view_style(&self) -> usize {
dr_pipeline::mask::RevealStyle::ALL
.iter()
.position(|&a| a == self.reveal_style)
.unwrap_or(0)
}
/// Show the selected layer's mask, or stop.
/// Choose how shown masks are drawn.
///
/// Takes no history step and marks nothing dirty: this is how the
/// photograph is being *looked at*, not an edit to it.
pub fn set_mask_view(&mut self, view: usize) {
use dr_pipeline::mask::RevealStyle;
self.reveal_style = view
.checked_sub(1)
.and_then(|i| RevealStyle::ALL.get(i).copied());
pub fn set_mask_view_style(&mut self, style: usize) {
if let Some(&s) = dr_pipeline::mask::RevealStyle::ALL.get(style) {
self.reveal_style = s;
}
}
/// Whether this layer's mask is drawn over the photograph.
pub fn mask_shown(&self, id: &str) -> bool {
self.mask_views.get(id).is_some_and(|v| v.shown)
}
/// Open or close one layer's eye.
pub fn set_mask_shown(&mut self, id: &str, shown: bool) {
let colour = self.next_mask_colour();
self.mask_views
.entry(id.to_string())
.or_insert(MaskView {
shown: false,
colour,
})
.shown = shown;
}
/// Whether any mask at all is being shown.
///
/// What the region overlay asks before drawing: two overlays that mean
/// different things, on top of each other, is neither.
pub fn any_mask_shown(&self) -> bool {
self.reveal().is_some()
}
/// Which of [`MASK_COLOURS`] this layer is shown in.
pub fn mask_colour(&self, id: &str) -> usize {
self.mask_views
.get(id)
.map_or(0, |v| v.colour % MASK_COLOURS.len())
}
/// Give this layer a colour from [`MASK_COLOURS`].
///
/// Choosing a colour is asking to see it: a swatch pressed on a layer
/// whose eye was closed opens the eye, because nothing else the press
/// could mean would change a pixel.
pub fn set_mask_colour(&mut self, id: &str, colour: usize) {
let colour = colour % MASK_COLOURS.len();
self.mask_views
.entry(id.to_string())
.and_modify(|v| {
v.colour = colour;
v.shown = true;
})
.or_insert(MaskView {
shown: true,
colour,
});
}
/// The colour the next layer to be shown should take: the first not
/// already in use, or round the palette again once all are.
///
/// So that two masks made one after the other come up in two colours
/// without anyone having to choose — which is the case that matters,
/// since "how do these two meet" is the question two masks are shown to
/// answer.
fn next_mask_colour(&self) -> usize {
let used: Vec<usize> = self.mask_views.values().map(|v| v.colour).collect();
(0..MASK_COLOURS.len())
.find(|c| !used.contains(c))
.unwrap_or(self.mask_views.len() % MASK_COLOURS.len())
}
/// TRACES: FR-DEV-19c
@@ -2531,18 +2647,22 @@ impl DevelopSession {
/// derivable from anything on screen, the layer carries no adjustment yet,
/// and the list it was chosen from says "architecture 23%" and nothing
/// about *which* 23%. So the mask appears with the layer rather than
/// waiting to be asked for a second time.
/// waiting to be asked for a second time — its eye open, in the next
/// colour nothing else is using.
///
/// Only from the resting position, and only on an explicit action — the
/// same rule arming a brush follows. Somebody who has switched to the
/// alpha or the outline keeps it, and this never re-arms in the background;
/// the trap `Masking.overlay-hidden` documents is an automatic reveal that
/// undoes a switch a photographer turned off, and every caller of this is
/// a press that asked for a new mask.
fn show_new_mask(&mut self) {
if self.reveal_style.is_none() {
self.reveal_style = Some(dr_pipeline::mask::RevealStyle::Tint);
}
/// Only this layer's eye. Every other layer keeps whatever the
/// photographer set it to, which is the trap `Masking.overlay-hidden`
/// documents: an automatic reveal that undoes a switch somebody turned
/// off is worse than none.
fn show_new_mask(&mut self, id: &str) {
let colour = self.next_mask_colour();
self.mask_views.insert(
id.to_string(),
MaskView {
shown: true,
colour,
},
);
}
// ----------------------------------------------------------------------
@@ -3378,7 +3498,7 @@ impl DevelopSession {
return None;
}
self.active_masks = vec![id.clone()];
self.show_new_mask();
self.show_new_mask(&id);
self.history
.record(&self.graph, Edit::Action(labels::step::MASK_ADDED));
Some(id)
@@ -3422,7 +3542,7 @@ impl DevelopSession {
return None;
}
self.active_masks = vec![id.clone()];
self.show_new_mask();
self.show_new_mask(&id);
self.history
.record(&self.graph, Edit::Action(labels::step::MASK_ADDED));
Some(id)
@@ -3453,7 +3573,7 @@ impl DevelopSession {
return None;
}
self.active_masks = vec![id.clone()];
self.show_new_mask();
self.show_new_mask(&id);
self.history
.record(&self.graph, Edit::Action(labels::step::MASK_ADDED));
Some(id)
@@ -3485,7 +3605,7 @@ impl DevelopSession {
}
self.active_masks = vec![id.clone()];
self.active_part = 0;
self.show_new_mask();
self.show_new_mask(&id);
self.history
.record(&self.graph, Edit::Action(labels::step::MASK_ADDED));
Some(id)
@@ -3512,7 +3632,7 @@ impl DevelopSession {
return None;
}
self.active_masks = vec![id.clone()];
self.show_new_mask();
self.show_new_mask(&id);
self.history
.record(&self.graph, Edit::Action(labels::step::MASK_ADDED));
Some(id)
@@ -6571,15 +6691,15 @@ mod tests {
let mut live = session_with_a_left_half_subject(&ctx);
let id = live.add_subject_mask(0).expect("a subject layer");
// TRACES: FR-DEV-19c
// Making a mask shows it (`show_new_mask`), and this test is about the
// pixels the *edit* produces. Put away here rather than left on, and
// the asymmetry is the point rather than an inconvenience: the reveal
// is how somebody is looking at a photograph, so a session that has
// just made a layer legitimately draws a frame that a session which
// read the same layer out of a file does not. Both of those are
// Making a mask opens its eye (`show_new_mask`), and this test is
// about the pixels the *edit* produces. Closed here rather than left
// open, and the asymmetry is the point rather than an inconvenience:
// the reveal is how somebody is looking at a photograph, so a session
// that has just made a layer legitimately draws a frame that a session
// which read the same layer out of a file does not. Both of those are
// correct, and only one of them is what a stored raster has to
// reproduce.
live.set_mask_view(0);
live.set_mask_shown(&id, false);
live.graph
.masks_mut()
.get_mut(&id)
@@ -6611,10 +6731,9 @@ mod tests {
// reopened with its masks tinted red. Checked here because this is the
// one test that puts an edit through a file and renders both ends, so
// it is where the property would first go wrong.
assert_eq!(
reopened.mask_view(),
0,
"restoring an edit must not turn an overlay on"
assert!(
!reopened.any_mask_shown(),
"restoring an edit must not open an eye"
);
let from_the_file = read_back(&ctx, &reopened.render(64, 64).expect("render"));