Merge pull request #1775 from TrevorBrown-dev/fix/pencil-dark-background

M4: the Pencil draws white on a dark page or frame, and previews in that colour (#1741)

crates/engine/src/brush_tests.rs

@@ -107,3 +107,45 @@ fn the_pencil_records_pressure_and_its_brush() {
     let last = doc.nodes().filter(|n| matches!(n.kind, NodeKind::Vector { .. })).max_by_key(|n| n.id).unwrap();
     assert!(last.stroke.profile().is_none() && last.stroke.brush.is_none());
 }
+
+/// The Pencil draws black on a light page and white on a dark page or inside a dark frame, the
+/// rule new text follows (#1741; Figma: "in black, unless you're sketching on a dark canvas or
+/// frame"), and the stroke in progress previews in that colour.
+#[test]
+fn the_pencil_draws_white_on_a_dark_background() {
+    use figcraft_color::Color;
+    let mut s = Session::new();
+    s.execute("tool.set", &json!({"tool": "pencil"})).unwrap();
+    // A stroke from (x, y); its colour, and the preview colour the overlay carried while drawing.
+    let stroke = |s: &mut Session, x: f64, y: f64| {
+        s.pointer(&PointerEvent::at(PointerKind::Down, x, y, 1.0)).unwrap();
+        s.pointer(&PointerEvent::at(PointerKind::Move, x + 40.0, y + 10.0, 1.0)).unwrap();
+        let preview = s.tool.overlay.pen_color;
+        s.pointer(&PointerEvent::at(PointerKind::Move, x + 80.0, y, 1.0)).unwrap();
+        s.pointer(&PointerEvent::at(PointerKind::Up, x + 80.0, y, 1.0)).unwrap();
+        assert!(s.tool.overlay.pen_color.is_none(), "no preview colour after the stroke");
+        let doc = s.doc().unwrap();
+        let last = doc.nodes().filter(|n| matches!(n.kind, NodeKind::Vector { .. })).max_by_key(|n| n.id).unwrap();
+        (last.strokes.first().and_then(|p| p.solid_color()).unwrap(), preview)
+    };
+    assert_eq!(stroke(&mut s, 0.0, 0.0), (Color::BLACK, Some(Color::BLACK)), "a light page");
+    s.execute("page.setBackground", &json!({"color": "#1E1E1E"})).unwrap();
+    assert_eq!(stroke(&mut s, 0.0, 100.0), (Color::WHITE, Some(Color::WHITE)), "a dark page");
+    // A dark frame on a light page, and a light frame on a dark page.
+    s.execute("page.setBackground", &json!({"color": "#F5F5F5"})).unwrap();
+    s.execute("node.create", &json!({"type": "frame", "x": 300, "y": 300, "width": 200, "height": 200, "props": {"fills": ["#202020"]}})).unwrap();
+    assert_eq!(stroke(&mut s, 320.0, 320.0), (Color::WHITE, Some(Color::WHITE)), "inside a dark frame");
+    s.execute("page.setBackground", &json!({"color": "#1E1E1E"})).unwrap();
+    s.execute("node.create", &json!({"type": "frame", "x": 600, "y": 300, "width": 200, "height": 200, "props": {"fills": ["#FFFFFF"]}})).unwrap();
+    assert_eq!(stroke(&mut s, 620.0, 320.0), (Color::BLACK, Some(Color::BLACK)), "inside a light frame on a dark page");
+    // A frame without a fill, or with a see-through one, is looked through to what is behind it.
+    s.execute("node.create", &json!({"type": "frame", "x": 900, "y": 300, "width": 200, "height": 200, "props": {"fills": []}})).unwrap();
+    assert_eq!(stroke(&mut s, 920.0, 320.0), (Color::WHITE, Some(Color::WHITE)), "a frame without a fill on a dark page");
+    s.execute("node.create", &json!({"type": "frame", "x": 1200, "y": 300, "width": 200, "height": 200, "props": {"fills": ["#FFFFFF40"]}})).unwrap();
+    assert_eq!(stroke(&mut s, 1220.0, 320.0), (Color::WHITE, Some(Color::WHITE)), "a mostly transparent light frame on a dark page");
+    // The threshold is Color::is_dark: white reads better than black from #757575 down.
+    for (bg, want) in [("#767676", Color::BLACK), ("#757575", Color::WHITE)] {
+        s.execute("page.setBackground", &json!({"color": bg})).unwrap();
+        assert_eq!(stroke(&mut s, 0.0, 600.0).0, want, "on {bg}");
+    }
+}

crates/engine/src/canvas/tools.rs

@@ -328,6 +328,9 @@ pub struct Overlay {
     pub cursor: Cursor,
     /// Pen tool: the in-progress path's anchor points and the rubber band to the pointer.
     pub pen: Vec<[f64; 2]>,
+    /// Pencil: the colour of the stroke in progress, for its preview (white over a dark page or
+    /// frame); None outside a Pencil stroke.
+    pub pen_color: Option<figcraft_color::Color>,
     pub rubber: Option<[f64; 4]>,
     /// Pen tool: the handles shaping the segments at the pen (anchor x, y, handle x, y): the last
     /// anchor's in and out handles and the one before's out handle.

crates/engine/src/canvas/tools/draw.rs

@@ -237,6 +237,7 @@ impl Session {
             }
             Gesture::Pencil { mut points, mut pressure } => {
                 self.tool.overlay.pen.clear();
+                self.tool.overlay.pen_color = None;
                 // The stroke ends where the button came up, even without a move to get there.
                 if points.last().is_none_or(|l| (*l - ev.p()).hypot() > 0.5 / ev.zoom.max(1e-6)) {
                     points.push(ev.p());

crates/engine/src/canvas/tools/handles.rs

@@ -36,16 +36,33 @@ pub fn anchor_name(a: (u8, u8)) -> &'static str {
     ANCHORS.get(usize::from(a.1.min(2)) * 3 + usize::from(a.0.min(2))).copied().unwrap_or("tl")
 }
 
+/// Whether what is drawn in `parent` sits on a dark background: the nearest layer from `parent`
+/// out whose top visible fill is a mostly opaque (alpha 0.5 or more) solid colour, else the page
+/// (#362); a frame without such a fill is looked through to what is behind it. Dark means white
+/// reads better on it than black (`Color::is_dark`: `#757575` is dark, `#767676` is light).
+pub(crate) fn on_dark_background(doc: &Document, parent: NodeId) -> bool {
+    std::iter::once(parent)
+        .chain(doc.ancestors(parent))
+        .filter_map(|id| doc.node(id))
+        .find_map(|n| match &n.kind {
+            NodeKind::Page { background, .. } => Some(background.is_dark()),
+            _ => n.fills.iter().rev().find(|p| p.visible).and_then(|p| p.solid_color()).filter(|c| c.a >= 0.5).map(|c| c.is_dark()),
+        })
+        .unwrap_or(false)
+}
+
 /// The fill new text takes to be readable where it is drawn, in `parent`: white on a dark
-/// background (None: the model's default, black). The background is the nearest layer from `parent`
-/// out whose top visible fill is a mostly opaque solid colour, else the page (#362). For text made
-/// in the app: by the Text tool, or by a command that names no fill while the interface is up.
+/// background (None: the model's default, black). For text made in the app: by the Text tool, or
+/// by a command that names no fill while the interface is up.
 pub(crate) fn readable_text_fills(doc: &Document, parent: NodeId) -> Option<Vec<figcraft_color::Paint>> {
-    let dark = std::iter::once(parent).chain(doc.ancestors(parent)).filter_map(|id| doc.node(id)).find_map(|n| match &n.kind {
-        NodeKind::Page { background, .. } => Some(background.is_dark()),
-        _ => n.fills.iter().rev().find(|p| p.visible).and_then(|p| p.solid_color()).filter(|c| c.a >= 0.5).map(|c| c.is_dark()),
-    });
-    dark.unwrap_or(false).then(|| vec![figcraft_color::Paint::solid(figcraft_color::Color::WHITE)])
+    on_dark_background(doc, parent).then(|| vec![figcraft_color::Paint::solid(figcraft_color::Color::WHITE)])
+}
+
+/// The colour the Pencil draws with in `parent`: [`PENCIL_STROKE`]'s black, or white on a dark
+/// background by the rule text follows (#1741; "in black, unless you're sketching on a dark canvas
+/// or frame").
+pub(crate) fn pencil_color(doc: &Document, parent: NodeId) -> figcraft_color::Color {
+    if on_dark_background(doc, parent) { figcraft_color::Color::WHITE } else { PENCIL_STROKE.0 }
 }
 
 const ANCHORS: [&str; 9] = ["tl", "t", "tr", "l", "c", "r", "bl", "b", "br"];

crates/engine/src/canvas/tools/pen.rs

@@ -333,7 +333,9 @@ impl Session {
         let mut local = fitted.clone();
         local.apply_affine(pw.inverse());
         let _ = d;
-        let (color, weight) = PENCIL_STROKE;
+        // Black, or white on a dark page or frame (#1741).
+        let (_, weight) = PENCIL_STROKE;
+        let color = super::handles::pencil_color(self.doc()?, parent);
         let mut props = json!({"fills": [], "strokes": [color.to_css_hex()], "strokeWeight": weight, "strokeCap": "round", "strokeJoin": "round", "strokeAlign": "center"});
         // Pen pressure becomes the stroke's width profile (brushes follow it too).
         if let Some(points) = pressure_profile(points, pressure) {
@@ -348,4 +350,13 @@ impl Session {
         )?;
         Ok(())
     }
+
+    /// The colour a Pencil stroke starting at `p` will have: it goes into the frame under `p`
+    /// (else the page), and is white when that is dark (`pencil_color`).
+    pub(super) fn pencil_color_at(&self, p: figcraft_geom::Point) -> Result<figcraft_color::Color> {
+        let page = self.page()?;
+        let doc = self.doc()?;
+        let parent = frame_at(doc, page, p, &[]).unwrap_or(page);
+        Ok(super::handles::pencil_color(doc, parent))
+    }
 }

crates/engine/src/canvas/tools/pointer.rs

@@ -160,6 +160,8 @@ impl Session {
                 ToolId::TextOnPath => self.text_on_path_down(ev),
                 ToolId::Pen => self.pen_down(ev),
                 ToolId::Pencil => {
+                    // The preview draws in the colour the stroke will have (#1741).
+                    self.tool.overlay.pen_color = self.pencil_color_at(ev.p()).ok();
                     self.tool.gesture = Gesture::Pencil { points: vec![ev.p()], pressure: vec![ev.pressure] };
                     Ok(())
                 }

crates/ui-egui/src/canvas/overlays.rs

@@ -434,8 +434,10 @@ fn pen_overlay(app: &FigApp, painter: &egui::Painter, zoom: f64) {
     if !ov.pen.is_empty() {
         let pts: Vec<Pos2> = ov.pen.iter().map(|[x, y]| app.to_screen(*x, *y)).collect();
         if app.session.tool.tool == ToolId::Pencil {
-            // The stroke it will make, in its colour and weight (at least a point when zoomed out).
-            let (color, weight) = figcraft_engine::tools::PENCIL_STROKE;
+            // The stroke it will make, in its colour and weight (at least a point when zoomed out):
+            // white over a dark page or frame, as the stroke will be (#1741).
+            let (default, weight) = figcraft_engine::tools::PENCIL_STROKE;
+            let color = ov.pen_color.unwrap_or(default);
             painter.add(Shape::line(pts, stroke(doc_c32(painter.ctx(), color), ((weight * zoom) as f32).max(1.0))));
         } else {
             for [x0, y0, x1, y1] in &ov.pen_handles {

crates/ui-egui/src/canvas_fixes_tests.rs

@@ -186,6 +186,23 @@ fn the_pencil_previews_its_stroke_in_its_colour_and_weight() {
     assert!((w - 1.0).abs() < 0.01, "at least a point: {w}");
 }
 
+/// Over a dark page the Pencil's preview is white, as the stroke it makes will be (#1741).
+#[test]
+fn the_pencil_previews_white_over_a_dark_page() {
+    let mut u = Ui::new(Session::new());
+    u.app.run("page.setBackground", json!({"color": "#1E1E1E"}));
+    u.app.run("tool.set", json!({"tool": "pencil"}));
+    let white = egui::epaint::ColorMode::Solid(crate::theme::c32(figcraft_color::Color::WHITE));
+    let at = |u: &Ui, x: f64, y: f64| u.app.to_screen(x, y);
+    u.button(at(&u, 10.0, 10.0), true);
+    for i in 1..=6 {
+        let p = at(&u, 10.0 + 12.0 * f64::from(i), 10.0 + 6.0 * f64::from(i % 2));
+        u.frame(vec![egui::Event::PointerMoved(p)]);
+    }
+    let shapes = flat(&u.frame_output(vec![]).shapes);
+    assert!(shapes.iter().any(|s| matches!(s, Shape::Path(p) if !p.closed && p.points.len() >= 3 && p.stroke.color == white)), "a white preview");
+}
+
 #[test]
 fn a_selected_layer_shows_its_constraint_guides_in_its_frame() {
     // A child pinned left (the default) in a frame without auto layout: a dashed guide runs from its