A failed feature has a mark of its own in the timeline and the Feature History (#972) (#1180)

A failed feature gets its own mark in the timeline and Feature History, distinct from warnings, with tests.

assets/icons/ui/source/icons.rs

@@ -1363,6 +1363,13 @@ pub(crate) fn paint_with(p: &Painter, r: Rect, name: &str, ink: Color32, fill: C
             pen.line(&[(12.0, 9.0), (12.0, 14.0)], ink);
             pen.dot(12.0, 17.0, ink);
         }
+        // A failed feature: a disc with a cross, a different shape from the warning's triangle, so
+        // it reads without colour (#972). Drawn in the caller's colours only.
+        "error" => {
+            pen.circle(12.0, 12.0, 10.0, pen.fill, ink);
+            pen.line(&[(8.0, 8.0), (16.0, 16.0)], ink);
+            pen.line(&[(16.0, 8.0), (8.0, 16.0)], ink);
+        }
         _ => {
             pen.poly(&[(5.0, 5.0), (19.0, 5.0), (19.0, 19.0), (5.0, 19.0)], pen.fill, ink);
         }

crates/ui-egui/src/lib.rs

@@ -59,6 +59,8 @@ mod menu_tests;
 #[cfg(test)]
 mod surface_clamp_tests;
 #[cfg(test)]
+mod timeline_marks_tests;
+#[cfg(test)]
 mod status_tests {
     #[test]
     fn the_status_bar_names_commands_by_label() {

crates/ui-egui/src/timeline.rs

@@ -242,6 +242,10 @@ pub fn timeline(app: &mut SolveApp, ui: &mut egui::Ui) {
                     if *suppressed {
                         p.line_segment([br.left_bottom() + vec2(4.0, -4.0), br.right_top() + vec2(-4.0, 4.0)], Stroke::new(1.5, t.text_dim));
                     }
+                    // A failed feature has a mark of its own, not only its colour (#972).
+                    if err.is_some() {
+                        icons::paint(&p, Rect::from_min_size(br.right_top() - vec2(10.0, 0.0), vec2(10.0, 10.0)), "error", t.panel, t.error, t.error);
+                    }
                     if warn.is_some() && err.is_none() {
                         icons::paint(
                             &p,

crates/ui-egui/src/timeline_left.rs

@@ -719,6 +719,11 @@ fn vertical_features(app: &mut SolveApp, ui: &mut egui::Ui) {
             egui::FontId::proportional(12.0),
             text_colour,
         );
+        // A failed feature has a mark of its own, not only its colour (#972).
+        if error.is_some() {
+            let at = Rect::from_min_size(pos2(rect.right() - 15.0, rect.center().y - 6.0), vec2(12.0, 12.0));
+            icons::paint(ui.painter(), at, "error", t.panel, t.error, t.error);
+        }
         if warning.is_some() && error.is_none() {
             icons::paint(
                 ui.painter(),

crates/ui-egui/src/timeline_marks_tests.rs

@@ -0,0 +1,107 @@
+//! A failed feature is marked in the bottom timeline and the Feature History by a shape, not only
+//! its colour (#972): the error mark (a disc with a cross) where a warning's mark goes.
+
+use egui::{Pos2, Rect, Shape, vec2};
+use serde_json::json;
+
+use crate::SolveApp;
+
+/// Every shape in `s`, the parts of a list of shapes too.
+fn each(s: &Shape, f: &mut impl FnMut(&Shape)) {
+    match s {
+        Shape::Vec(v) => v.iter().for_each(|s| each(s, f)),
+        s => f(s),
+    }
+}
+
+/// The sample with Fillet1 failing (the plate made narrower than its fillets).
+fn failing_sample() -> SolveApp {
+    let mut s = solidcraft_engine::Session::default();
+    s.run_script(&solidcraft_engine::sample::script()).unwrap();
+    s.execute("parameters.change", &json!({"name": "width", "expression": "5 mm"})).unwrap();
+    SolveApp::new(s, crate::Services::default())
+}
+
+/// Each feature's name and whether it failed.
+fn features(app: &SolveApp) -> Vec<(String, bool)> {
+    app.session.doc.features.iter().map(|f| (f.name.clone(), app.session.model.result(f.id).is_some_and(|r| r.error.is_some()))).collect()
+}
+
+/// The discs filled in the error colour (either theme's) that one frame of `draw` paints.
+fn error_discs(app: &mut SolveApp, draw: fn(&mut SolveApp, &mut egui::Ui)) -> Vec<Pos2> {
+    crate::scenario::clear_handles();
+    let ctx = egui::Context::default();
+    let screen = Rect::from_min_size(Pos2::ZERO, vec2(1400.0, 900.0));
+    let out = ctx.run_ui(egui::RawInput { screen_rect: Some(screen), ..Default::default() }, |ui| draw(app, ui));
+    let errors = [crate::theme::Tokens::light().error, crate::theme::Tokens::dark().error];
+    let mut found = Vec::new();
+    for clipped in &out.shapes {
+        each(&clipped.shape, &mut |s| {
+            if let Shape::Circle(c) = s
+                && errors.contains(&c.fill)
+            {
+                found.push(c.center);
+            }
+        });
+    }
+    found
+}
+
+/// The features that have a disc where their mark goes (`at`: the mark's centre from the item's).
+fn marked(app: &mut SolveApp, draw: fn(&mut SolveApp, &mut egui::Ui), at: impl Fn(Pos2, Pos2) -> bool) -> Vec<(String, bool)> {
+    let discs = error_discs(app, draw);
+    let handles = crate::scenario::handles("timeline:");
+    features(app)
+        .into_iter()
+        .map(|(name, _)| {
+            let item = handles.iter().find(|(h, _)| *h == format!("timeline:{name}")).map(|(_, p)| *p);
+            let item = item.unwrap_or_else(|| panic!("{name} is not drawn"));
+            let has = discs.iter().any(|d| at(item, *d));
+            (name, has)
+        })
+        .collect()
+}
+
+#[test]
+fn a_failed_feature_is_marked_in_the_bottom_timeline() {
+    let mut app = failing_sample();
+    let want = features(&app);
+    assert!(want.iter().any(|(n, failed)| *failed && n == "Fillet1"), "the sample's Fillet1 fails: {want:?}");
+    assert!(want.iter().filter(|(_, failed)| !failed).count() >= 3, "and others don't");
+    // The mark sits in the item's top-right corner: 10 points square, inside the 28-point item.
+    let got = marked(&mut app, crate::timeline::timeline, |item, d| (d.x - (item.x + 9.0)).abs() < 1.5 && (d.y - (item.y - 9.0)).abs() < 1.5);
+    assert_eq!(got, want, "an error mark on each failed feature and on no other");
+}
+
+#[test]
+fn a_failed_feature_is_marked_in_the_feature_history() {
+    let mut app = failing_sample();
+    let want = features(&app);
+    // The mark sits at the row's right end, on its centre line.
+    let got = marked(&mut app, crate::timeline_left::left_timeline, |item, d| (d.y - item.y).abs() < 1.5 && d.x > item.x);
+    assert_eq!(got, want, "an error mark on each failed feature and on no other");
+}
+
+/// The mark is its own drawing: a disc with a cross in the caller's colours, not the fallback
+/// square, and a different shape from the warning's triangle.
+#[test]
+fn the_error_mark_is_a_disc_with_a_cross() {
+    let ctx = egui::Context::default();
+    let (ink, fill) = (egui::Color32::from_rgb(1, 2, 3), egui::Color32::from_rgb(4, 5, 6));
+    let draw = |name: &'static str| {
+        let out = ctx.run_ui(egui::RawInput::default(), |ui| {
+            crate::icons::paint(ui.painter(), Rect::from_min_size(Pos2::ZERO, vec2(24.0, 24.0)), name, ink, fill, fill);
+        });
+        let mut shapes = Vec::new();
+        for c in &out.shapes {
+            each(&c.shape, &mut |s| shapes.push(s.clone()));
+        }
+        shapes
+    };
+    let error = draw("error");
+    let discs = error.iter().filter(|s| matches!(s, Shape::Circle(c) if c.fill == fill && c.stroke.color == ink)).count();
+    let strokes = error.iter().filter(|s| matches!(s, Shape::Path(p) if !p.closed && p.stroke.color == egui::epaint::ColorMode::Solid(ink))).count();
+    assert_eq!((discs, strokes), (1, 2), "one disc and the two strokes of a cross: {error:?}");
+    assert_ne!(format!("{error:?}"), format!("{:?}", draw("warning")));
+    assert_ne!(format!("{error:?}"), format!("{:?}", draw("no such icon")), "drawn, not the fallback square");
+}