diff --git a/crates/ui-egui/src/dialogs/settings/appearance.rs b/crates/ui-egui/src/dialogs/settings/appearance.rs index f714a97e6..5f8f428d4 100644 --- a/crates/ui-egui/src/dialogs/settings/appearance.rs +++ b/crates/ui-egui/src/dialogs/settings/appearance.rs @@ -221,7 +221,7 @@ fn theme_preview(ui: &mut egui::Ui, kind: crate::theme::ThemeKind, width: f32) { let thumb = cell.shrink(2.5); painter.rect_filled(thumb, 0.0, Color32::from_rgb(0x6f + 8 * i as u8, 0x80, 0x8f)); if selected { - painter.rect_stroke(thumb, 0.0, Stroke::new(1.0, p.pick), StrokeKind::Outside); + crate::panels::grid::selection_ring(&painter, &p, thumb, lightcraft_engine::SelectionState::Active); } } } diff --git a/crates/ui-egui/src/panels/compare.rs b/crates/ui-egui/src/panels/compare.rs index 9b9a23673..88463a285 100644 --- a/crates/ui-egui/src/panels/compare.rs +++ b/crates/ui-egui/src/panels/compare.rs @@ -2,7 +2,7 @@ //! pan) and **Survey** (the selected photos tiled). Rating, flag and label keys act on the active //! photo only here, and with Auto Advance on they move to the next candidate / photo. -use egui::{Align2, Color32, Rect, Sense, Stroke, StrokeKind, pos2, vec2}; +use egui::{Align2, Color32, Rect, Sense, Stroke, pos2, vec2}; use lightcraft_catalog::{Flag, PhotoId}; use serde_json::{Value, json}; @@ -286,7 +286,7 @@ fn photo_tile(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, slot: Slo display = super::detail::fit_rect(img_area, texture_aspect, zoom, native, ppp, app.ui.pan); p.image(tex.tex.id(), display, uv, Color32::WHITE); } else { - p.rect_filled(img.intersect(img_area), 0.0, Tokens::get(ui.ctx()).skeleton); + p.rect_filled(img.intersect(img_area), 0.0, t.placeholder); } if photo.flag == Flag::Reject { p.rect_filled(display, 0.0, Color32::from_black_alpha(110)); @@ -296,7 +296,7 @@ fn photo_tile(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, slot: Slo } let active = app.session.selection.active == Some(id); if active { - p.rect_stroke(display.intersect(img_area.shrink(1.0)), 0.0, Stroke::new(2.0, Color32::WHITE), StrokeKind::Outside); + super::grid::selection_ring(&p, &t, display.intersect(img_area.shrink(3.0)), lightcraft_engine::SelectionState::Active); } // Compare and Reference are where sharpness is judged: as in the loupe, say when what is on // screen isn't the render on its way (a thumbnail, the last render after an edit, a magnified diff --git a/crates/ui-egui/src/panels/grid.rs b/crates/ui-egui/src/panels/grid.rs index 656ddda91..ed7614590 100644 --- a/crates/ui-egui/src/panels/grid.rs +++ b/crates/ui-egui/src/panels/grid.rs @@ -651,16 +651,17 @@ pub fn layout(aspects: &[f32], groups: &[(usize, usize)], avail_w: f32, target: out } -/// The ring round a selected photo, drawn outside `photo`. The active one is 3 pt: a 1 pt inner line -/// in [`Tokens::ring_inner`] against the photo's edge (so the ring reads on a white photo and on a -/// black one) and a 2 pt line in [`Tokens::ring_active`]. The others are a single 1.5 pt line in +/// The ring round a selected photo, drawn outside `photo`. The active one is 3 pt in +/// [`Tokens::ring_active`] with a 1 pt line in [`Tokens::ring_inner`] over its inner edge, against +/// the photo (so the ring reads on a white photo and on a black one). The others are a single 1.5 pt line in /// [`Tokens::ring_selected`]; the two are told apart by weight, not by lightness alone. pub(crate) fn selection_ring(p: &egui::Painter, t: &Tokens, photo: Rect, state: lightcraft_engine::SelectionState) { use lightcraft_engine::SelectionState; match state { SelectionState::Active => { + // both on the same rect, so no seam opens between them at a fractional scale + p.rect_stroke(photo, 0.0, Stroke::new(3.0, t.ring_active), StrokeKind::Outside); p.rect_stroke(photo, 0.0, Stroke::new(1.0, t.ring_inner), StrokeKind::Outside); - p.rect_stroke(photo.expand(1.0), 0.0, Stroke::new(2.0, t.ring_active), StrokeKind::Outside); } SelectionState::Selected => { p.rect_stroke(photo, 0.0, Stroke::new(1.5, t.ring_selected), StrokeKind::Outside); diff --git a/crates/ui-egui/src/panels/people.rs b/crates/ui-egui/src/panels/people.rs index 73d19cbaa..f81b90372 100644 --- a/crates/ui-egui/src/panels/people.rs +++ b/crates/ui-egui/src/panels/people.rs @@ -174,7 +174,7 @@ fn card(app: &mut LightcraftApp, ui: &mut egui::Ui, person: &Person, r: Rect, ed p.rect_filled(pill, 4.0, Color32::from_black_alpha(175)); p.galley(pill.min + vec2(5.0, 3.0), g, Color32::WHITE); if selected { - p.rect_stroke(face, 3.0, Stroke::new(2.0, Color32::WHITE), StrokeKind::Outside); + super::grid::selection_ring(p, &t, face, lightcraft_engine::SelectionState::Active); } else if resp.hovered() { p.rect_stroke(face, 3.0, Stroke::new(1.0, t.text_dim), StrokeKind::Outside); } diff --git a/crates/ui-egui/src/tests/compare.rs b/crates/ui-egui/src/tests/compare.rs index c816b3c67..b0ccb0b14 100644 --- a/crates/ui-egui/src/tests/compare.rs +++ b/crates/ui-egui/src/tests/compare.rs @@ -318,3 +318,44 @@ fn info_panel_label_in_compare_labels_the_active_photo_and_shows_it() { assert_eq!(photo(&h, cand).rating, 3); assert_eq!(photo(&h, sel).rating, 0, "the select is not rated with it"); } + +/// Survey frames its active tile in the theme's active ring, which must not be the white that +/// vanishes on a light theme's canvas. +#[test] +fn survey_rings_the_active_tile_in_the_theme_colour() { + use egui::epaint::Shape; + let mut h = demo(); + h.app.ui.settings.appearance_mode = crate::state::AppearanceMode::Light; + let vis: Vec = h.app.session.visible_cloned().iter().map(|p| p.0).collect(); + h.request("engine.execute", json!({"command": "library.select", "params": {"ids": [vis[0], vis[1], vis[2]], "active": vis[1]}}), T); + let r = h.request("engine.execute", json!({"command": "view.survey"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + let t = crate::theme::Tokens::get(&h.view.ctx); + assert_ne!(t.ring_active, egui::Color32::WHITE); + // the filmstrip below rings its active cell the same way: only the tile counts here + let films: Vec = h.app.widgets.iter().filter(|(w, _)| w.starts_with("film:")).map(|(_, r)| *r).collect(); + let rings: Vec<(f32, egui::Color32)> = h + .view + .painted_shapes() + .iter() + .filter_map(|s| match s { + Shape::Rect(rs) + if rs.stroke_kind == egui::StrokeKind::Outside + && rs.stroke.color == t.ring_active + && !films.iter().any(|f| f.intersects(rs.rect)) => + { + Some((rs.stroke.width, rs.stroke.color)) + } + _ => None, + }) + .collect(); + assert_eq!(rings, vec![(3.0, t.ring_active)], "one active tile, ringed once"); + let white = h + .view + .painted_shapes() + .iter() + .filter(|s| matches!(s, Shape::Rect(rs) if rs.stroke.width == 2.0 && rs.stroke.color == egui::Color32::WHITE)) + .count(); + assert_eq!(white, 0, "no white 2 pt frame"); +} diff --git a/crates/ui-egui/src/tests/filmstrip.rs b/crates/ui-egui/src/tests/filmstrip.rs index a95530b2a..de05996d1 100644 --- a/crates/ui-egui/src/tests/filmstrip.rs +++ b/crates/ui-egui/src/tests/filmstrip.rs @@ -95,8 +95,7 @@ fn the_filmstrip_marks_the_active_photo_heavier_than_the_other_selected_one() { .collect() }; let (on_active, on_other, on_unselected) = (strokes(active), strokes(other), strokes(c)); - assert!(on_active.iter().map(|(w, _)| w).sum::() >= 3.0 && on_active.iter().any(|(_, col)| *col == t.ring_active), "{on_active:?}"); - assert_eq!(on_other.iter().map(|(_, col)| *col).collect::>(), vec![t.ring_selected], "{on_other:?}"); - assert!(on_other.iter().map(|(w, _)| w).sum::() < 3.0, "{on_other:?}"); + assert_eq!(on_active, vec![(3.0, t.ring_active), (1.0, t.ring_inner)]); + assert_eq!(on_other, vec![(1.5, t.ring_selected)]); assert!(on_unselected.is_empty(), "{on_unselected:?}"); } diff --git a/crates/ui-egui/src/tests/grid.rs b/crates/ui-egui/src/tests/grid.rs index 888021198..4e32756e4 100644 --- a/crates/ui-egui/src/tests/grid.rs +++ b/crates/ui-egui/src/tests/grid.rs @@ -164,24 +164,25 @@ fn unloaded_thumbnails_show_their_selection() { .collect() }; let mut checked = 0; + let mut saw_active = false; for (w, r) in h.app.widgets.iter().filter(|(w, _)| w.starts_with("thumb:")) { - if !canvas.shrink(4.0).contains_rect(*r) || r.min.x < 6.0 { + if !canvas.shrink(4.0).contains_rect(*r) { continue; } let id = w.trim_start_matches("thumb:").parse::().unwrap(); let strokes = ring(*r); - let width: f32 = strokes.iter().map(|(w, _)| w).sum(); if id == active.0 { - assert!(width >= 3.0, "photo {id}: active ring {strokes:?}"); - assert!(strokes.iter().any(|(_, c)| *c == t.ring_active), "photo {id}: {strokes:?}"); - assert!(strokes.iter().all(|(_, c)| *c != egui::Color32::WHITE), "photo {id}: a white ring vanishes on the light grid: {strokes:?}"); + assert_ne!(t.ring_active, egui::Color32::WHITE, "the light theme's ring must not vanish on its grid"); + assert_eq!(strokes, vec![(3.0, t.ring_active), (1.0, t.ring_inner)], "photo {id}: active ring"); + saw_active = true; } else { - assert!((1.0..3.0).contains(&width), "photo {id}: selected ring {strokes:?}"); + assert_eq!(strokes.iter().map(|(w, _)| *w).sum::(), 1.5, "photo {id}: selected ring {strokes:?}"); assert_eq!(strokes.iter().map(|(_, c)| *c).collect::>(), vec![t.ring_selected], "photo {id}"); } checked += 1; } assert!(checked >= 3, "checked {checked}"); + assert!(saw_active, "the active photo (top left) was checked"); } /// A folder of unedited raws on a slow share: the grid must not decode every raw to draw a 512 px diff --git a/crates/ui-egui/src/theme.rs b/crates/ui-egui/src/theme.rs index a7b129e8c..d57258a4a 100644 --- a/crates/ui-egui/src/theme.rs +++ b/crates/ui-egui/src/theme.rs @@ -140,7 +140,7 @@ impl Default for Tokens { skeleton: Color32::from_rgb(0x25, 0x25, 0x25), ring_active: Color32::from_rgb(0xff, 0xff, 0xff), ring_inner: Color32::from_rgb(0x00, 0x00, 0x00), - ring_selected: Color32::from_rgb(0xaa, 0xaa, 0xaa), + ring_selected: Color32::from_rgb(0xc0, 0xc0, 0xc0), divider: Color32::from_rgb(0x1c, 0x1c, 0x1c), inset: Color32::from_rgb(0x23, 0x23, 0x23), field: Color32::from_rgb(0x22, 0x22, 0x22), @@ -203,7 +203,7 @@ impl Tokens { ThemeKind::Silver => Tokens { ring_active: rgb(0x141414), ring_inner: rgb(0xffffff), - ring_selected: rgb(0x5a5a5a), + ring_selected: rgb(0x4a4a4a), chrome: rgb(0xe6e6e6), canvas: rgb(0xcfcfcf), grid_bg: rgb(0xd9d9d9), @@ -237,7 +237,7 @@ impl Tokens { ThemeKind::Paper => Tokens { ring_active: rgb(0x141414), ring_inner: rgb(0xffffff), - ring_selected: rgb(0x5a5a5a), + ring_selected: rgb(0x4a4a4a), chrome: rgb(0xfafafa), canvas: rgb(0xececec), grid_bg: rgb(0xf2f2f2), @@ -489,7 +489,18 @@ mod tests { for (what, surface) in [("grid", t.grid_bg), ("cell", t.cell), ("selected cell", t.cell_selected)] { assert!(contrast(t.ring_active, surface) >= 3.0, "{kind:?}: active ring on the {what}: {:.2}", contrast(t.ring_active, surface)); } - assert!(contrast(t.ring_selected, t.grid_bg) >= 3.0, "{kind:?}: selected ring on the grid: {:.2}", contrast(t.ring_selected, t.grid_bg)); + // the selected ring also sits on a selected cell (square grid, filmstrip), label-tinted or not + let mut selected_cells = vec![("grid", t.grid_bg), ("selected cell", t.cell_selected)]; + for label in lightcraft_catalog::ColorLabel::ALL { + selected_cells.push(("labelled selected cell", label_background(t.cell_selected, Some(label), true))); + } + for (what, surface) in selected_cells { + assert!( + contrast(t.ring_selected, surface) >= 3.0, + "{kind:?}: selected ring on the {what}: {:.2}", + contrast(t.ring_selected, surface) + ); + } // the thin inner line of the active ring is the ring's opposite: it parts the ring from the photo's edge assert!(contrast(t.ring_inner, t.ring_active) >= 3.0, "{kind:?}: inner line against the active ring"); }