From 4517d8be65b667f6fc14786ab0bcf5b1bb76ad0f Mon Sep 17 00:00:00 2001 From: Bryan Mark Date: Sun, 11 Oct 2026 16:15:57 +0900 Subject: [PATCH] Leave the active cell unshaded inside a selected range The active cell of a selected range was shaded like the rest of the range: the line meant to clear it painted a fully transparent rectangle, which changes nothing. The fill now goes around the active cell (the whole merged cell when it is one) in every area of the selection, so it is plain where typing goes. A lone active cell among several areas isn't shaded at all, and neither is a single selected merged cell. The fill is translucent, so the cell can't be cleared by painting over it: each area is shaded as up to four rectangles around it that never overlap. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/grid.rs | 114 ++++++++++++++++++++++--- crates/ui-egui/tests/selection_fill.rs | 83 ++++++++++++++++++ 2 files changed, 184 insertions(+), 13 deletions(-) create mode 100644 crates/ui-egui/tests/selection_fill.rs diff --git a/crates/ui-egui/src/grid.rs b/crates/ui-egui/src/grid.rs index 72d33e2dc..6200291ac 100644 --- a/crates/ui-egui/src/grid.rs +++ b/crates/ui-egui/src/grid.rs @@ -901,23 +901,50 @@ fn paint_cf_icon(p: &Painter, r: Rect, set: &str, idx: usize) { } } +/// Where the active cell is on screen: the whole merged cell when it is part of one. +fn active_rect(geo: &Geo, sh: &Sheet, sel: &gridcraft_engine::Selection) -> Rect { + match sh.merge_at(sel.active) { + Some(m) => geo.range_rect(sh, m), + None => geo.cell_rect(sh, sel.active), + } +} + +/// The rectangles shaded for a selection: every area except the active cell, which stays clear so +/// it is plain where typing goes. A single selected cell has no fill. +pub fn selection_fill(geo: &Geo, sh: &Sheet, sel: &gridcraft_engine::Selection) -> Vec { + if sel.is_single_cell() { + return vec![]; + } + let active = active_rect(geo, sh, sel); + sel.ranges.iter().flat_map(|r| rect_around(geo.range_rect(sh, *r), active)).collect() +} + +/// `area` without `hole`: the strips above and below the hole, then what is left beside it. The +/// selection fill is translucent, so the active cell can't be cleared by painting over it; the +/// pieces never overlap for the same reason. Pieces with no size, or whose numbers aren't finite, +/// are dropped. +fn rect_around(area: Rect, hole: Rect) -> impl Iterator { + // `intersect` takes the sane side of a NaN, which would make a broken hole cover the area. + let hole = if hole.is_finite() { area.intersect(hole) } else { Rect::NOTHING }; + let parts = if hole.is_positive() { + [ + Rect::from_min_max(area.min, pos2(area.max.x, hole.min.y)), + Rect::from_min_max(pos2(area.min.x, hole.max.y), area.max), + Rect::from_min_max(pos2(area.min.x, hole.min.y), pos2(hole.min.x, hole.max.y)), + Rect::from_min_max(pos2(hole.max.x, hole.min.y), pos2(area.max.x, hole.max.y)), + ] + } else { + [area, Rect::NOTHING, Rect::NOTHING, Rect::NOTHING] + }; + parts.into_iter().filter(|r| r.is_positive() && r.is_finite()) +} + fn paint_selection(p: &Painter, geo: &Geo, sh: &Sheet, sel: &gridcraft_engine::Selection, t: &Tokens, app: &SheetApp) { // Fill all areas except the active cell. - for r in &sel.ranges { - if r.is_single() && sel.ranges.len() == 1 { - continue; - } - let rect = geo.range_rect(sh, *r); + for rect in selection_fill(geo, sh, sel) { p.rect_filled(rect, 0.0, t.sel_fill); } - let active = geo.cell_rect(sh, sh.merge_at(sel.active).map(|m| m.start).unwrap_or(sel.active)); - let active = match sh.merge_at(sel.active) { - Some(m) => geo.range_rect(sh, m), - None => active, - }; - if !(sel.ranges.len() == 1 && sel.current().is_single()) { - p.rect_filled(active.shrink(1.0), 0.0, t.grid_bg.gamma_multiply(0.0)); - } + let active = active_rect(geo, sh, sel); let cur = geo.range_rect(sh, sel.current()); // Editing a formula: outline its references instead of the selection. if let Some(ed) = &app.editor @@ -2223,3 +2250,64 @@ fn paint_pages(app: &mut SheetApp, p: &Painter, geo: &Geo, sh: &Sheet) { } p.rect_stroke(pr, 0.0, Stroke::new(3.0, blue), StrokeKind::Outside); } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn rect_around_leaves_the_hole_and_never_overlaps() { + let area = Rect::from_min_max(pos2(100.0, 50.0), pos2(400.0, 250.0)); + let check = |hole: Rect, pieces: usize| { + let parts: Vec = rect_around(area, hole).collect(); + assert_eq!(parts.len(), pieces, "{hole:?}: {parts:?}"); + let cut = area.intersect(hole); + let cut_area = if cut.is_positive() { cut.area() } else { 0.0 }; + let shaded: f32 = parts.iter().map(Rect::area).sum(); + assert!((shaded - (area.area() - cut_area)).abs() < 0.5, "everything but the hole is shaded: {hole:?}"); + for (i, a) in parts.iter().enumerate() { + assert!(area.contains_rect(*a), "nothing outside the area: {a:?}"); + assert!(!a.shrink(0.01).intersects(hole), "the hole stays clear: {a:?} vs {hole:?}"); + // The fill is translucent: a part shaded twice would show darker. + assert!(parts.iter().skip(i + 1).all(|b| !a.shrink(0.01).intersects(*b)), "{parts:?}"); + } + }; + let cell = |x: f32, y: f32| Rect::from_min_size(pos2(x, y), vec2(64.0, 16.0)); + // In the middle, in each corner (where a drag began), along an edge. + check(cell(200.0, 100.0), 4); + check(cell(100.0, 50.0), 2); + check(cell(336.0, 50.0), 2); + check(cell(100.0, 234.0), 2); + check(cell(336.0, 234.0), 2); + check(cell(100.0, 100.0), 3); + // A merged cell as wide, or as tall, as the area; one that is all of it, or more. + check(Rect::from_min_max(pos2(100.0, 100.0), pos2(400.0, 132.0)), 2); + check(Rect::from_min_max(pos2(200.0, 50.0), pos2(264.0, 250.0)), 2); + check(area, 0); + check(area.expand(30.0), 0); + // The active cell is in another area, touches this one, or is only partly inside it. + check(cell(500.0, 300.0), 1); + check(cell(400.0, 50.0), 1); + check(cell(380.0, 240.0), 2); + } + + #[test] + fn rect_around_survives_hostile_numbers() { + let sane = Rect::from_min_max(pos2(0.0, 0.0), pos2(64.0, 16.0)); + let bad = [f32::NAN, f32::INFINITY, f32::NEG_INFINITY, f32::MAX, f32::MIN, -1.0, 0.0]; + for a in bad { + for b in bad { + let weird = Rect::from_min_max(pos2(a, b), pos2(b, a)); + for (area, hole) in [(sane, weird), (weird, sane), (weird, weird)] { + for part in rect_around(area, hole) { + assert!(part.is_positive() && part.is_finite(), "{area:?} around {hole:?}: {part:?}"); + } + } + // A hole that makes no sense leaves the whole area shaded. + if !weird.intersects(sane) { + assert_eq!(rect_around(sane, weird).collect::>(), vec![sane]); + } + } + } + } +} diff --git a/crates/ui-egui/tests/selection_fill.rs b/crates/ui-egui/tests/selection_fill.rs new file mode 100644 index 000000000..bada97d73 --- /dev/null +++ b/crates/ui-egui/tests/selection_fill.rs @@ -0,0 +1,83 @@ +//! The selection's fill leaves the active cell clear, so it is plain where typing goes. + +use egui::Rect; +use gridcraft_engine::Session; +use gridcraft_engine::core::RangeRef; +use gridcraft_ui_egui::grid::{Geo, selection_fill}; +use serde_json::json; + +fn session() -> Session { + let mut session = Session::new(); + session.new_workbook(); + session +} + +fn geo(session: &Session) -> Geo { + let sheet = session.active().unwrap().wb.active().unwrap(); + Geo::new(sheet, Rect::from_min_max(egui::pos2(0.0, 0.0), egui::pos2(1200.0, 800.0)), egui::Vec2::ZERO) +} + +/// Where a cell or range is on screen. +fn cells(session: &Session, a1: &str) -> Rect { + let sheet = session.active().unwrap().wb.active().unwrap(); + geo(session).range_rect(sheet, RangeRef::parse(a1).unwrap()) +} + +fn fill(session: &Session) -> Vec { + let doc = session.active().unwrap(); + let sheet = doc.wb.active().unwrap(); + selection_fill(&geo(session), sheet, &doc.selection) +} + +fn shaded(fill: &[Rect], cell: Rect) -> bool { + fill.iter().any(|part| part.intersects(cell.shrink(1.0))) +} + +fn area(fill: &[Rect]) -> f32 { + fill.iter().map(Rect::area).sum() +} + +#[test] +fn the_active_cell_of_a_range_is_not_shaded() { + let mut s = session(); + // At the corner where a drag began, then wherever Tab and Enter move it inside the range. + for active in ["B2", "C4", "D5"] { + s.execute("selection.set", json!({"range": "B2:D5", "active": active})).unwrap(); + let fill = fill(&s); + assert!(!shaded(&fill, cells(&s, active)), "{active} stays clear: {fill:?}"); + for other in ["B2", "B5", "C3", "C4", "D2", "D5"].into_iter().filter(|c| *c != active) { + assert!(shaded(&fill, cells(&s, other)), "{other} is shaded with {active} active"); + } + assert!((area(&fill) - (cells(&s, "B2:D5").area() - cells(&s, active).area())).abs() < 0.5, "the rest of the range, once"); + } +} + +#[test] +fn a_single_selected_cell_has_no_fill() { + let mut s = session(); + s.execute("selection.set", json!({"cell": "C3"})).unwrap(); + assert!(fill(&s).is_empty()); +} + +#[test] +fn other_areas_are_shaded_whole_and_a_lone_active_cell_not_at_all() { + let mut s = session(); + s.execute("selection.set", json!({"range": "A1:A3,C2:D3,F6", "active": "F6"})).unwrap(); + assert_eq!(s.active().unwrap().selection.ranges.len(), 3); + let fill = fill(&s); + assert!(!shaded(&fill, cells(&s, "F6"))); + assert!(shaded(&fill, cells(&s, "A2")) && shaded(&fill, cells(&s, "D3"))); + assert!((area(&fill) - (cells(&s, "A1:A3").area() + cells(&s, "C2:D3").area())).abs() < 0.5, "{fill:?}"); +} + +#[test] +fn a_merged_active_cell_is_left_clear_as_a_whole() { + let mut s = session(); + s.execute("selection.set", json!({"range": "B2:C3"})).unwrap(); + s.execute("home.mergeCells", json!({})).unwrap(); + s.execute("selection.set", json!({"range": "B2:E6", "active": "B2"})).unwrap(); + let fill = fill(&s); + assert!(!shaded(&fill, cells(&s, "B2:C3")), "the merged cell, not just its first cell: {fill:?}"); + assert!(shaded(&fill, cells(&s, "D2")) && shaded(&fill, cells(&s, "B4")) && shaded(&fill, cells(&s, "E6"))); + assert!((area(&fill) - (cells(&s, "B2:E6").area() - cells(&s, "B2:C3").area())).abs() < 0.5); +}