Fill section: the canvas crop and gradient handles follow the characters' shared fill

The handles read the paint list the Fill section and the colour picker show (character_fills), so with the characters' image already in Crop mode the crop handles show and edit it, not the layer's own fill at that index; writes still go through fill.set, which moves the characters' list to the layer first. No undo step on entering crop mode. Test canvas::handles::character_fill_tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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

@@ -40,6 +40,15 @@ enum Drag {
 /// The Crop-mode image being edited: (node, fills, index, paint, image rect in the unit box).
 type UnitRect = (f64, f64, f64, f64);
 
+/// The paint list a picker paint at an index belongs to on layer `n`, as the Fill section and the
+/// colour picker list it: for fills, a text's characters' shared fills (`character_fills`: the
+/// layer's own when the characters have none), else the layer's strokes. None when a text's
+/// characters have different fills. Writes go through `fill.set`, which moves the characters'
+/// list to the layer first, so the handles read and write the same paint (#1856).
+fn edited_list(n: &figcraft_doc::Node, fills: bool) -> Option<Vec<Paint>> {
+    if fills { figcraft_engine::cmd::lists::character_fills(n) } else { Some(n.strokes.clone()) }
+}
+
 /// The paint open in the colour picker, else the image fill in crop mode (while its layer is the
 /// selection).
 fn crop_target(app: &FigApp) -> Option<(NodeId, bool, usize, ImagePaint, UnitRect)> {
@@ -53,7 +62,7 @@ fn crop_target(app: &FigApp) -> Option<(NodeId, bool, usize, ImagePaint, UnitRec
     };
     let doc = app.session.doc().ok()?;
     let n = doc.node(id)?;
-    let list = if fills { &n.fills } else { &n.strokes };
+    let list = edited_list(n, fills)?;
     let PaintKind::Image(img) = &list.get(index)?.kind else { return None };
     if img.scale_mode != ImageScaleMode::Crop {
         return None;
@@ -193,7 +202,7 @@ fn gradient_target(app: &FigApp) -> Option<(NodeId, bool, usize, figcraft_color:
         return None;
     }
     let n = app.session.doc().ok()?.node(id)?;
-    let list = if *fills { &n.fills } else { &n.strokes };
+    let list = edited_list(n, *fills)?;
     match &list.get(*index)?.kind {
         PaintKind::Gradient(g) => Some((id, *fills, *index, g.clone())),
         _ => None,
@@ -233,7 +242,7 @@ fn line_param(a: Pos2, b: Pos2, p: Pos2) -> Option<(f64, f32)> {
 
 fn set_gradient(app: &mut FigApp, id: NodeId, fills: bool, index: usize, g: figcraft_color::Gradient) {
     let Some(n) = app.session.doc().ok().and_then(|d| d.node(id)).cloned() else { return };
-    let list = if fills { &n.fills } else { &n.strokes };
+    let Some(list) = edited_list(&n, fills) else { return };
     let Some(mut paint) = list.get(index).cloned() else { return };
     paint.kind = PaintKind::Gradient(g);
     let cmd = if fills { "fill.set" } else { "stroke.set" };
@@ -652,7 +661,7 @@ fn apply(app: &mut FigApp, drag: &Drag, pos: Pos2, shift: bool) {
         Drag::Gradient { id, fills, index, which } => {
             let Some(u) = screen_to_unit(app, *id, pos) else { return };
             let Some(n) = app.session.doc().ok().and_then(|d| d.node(*id)).cloned() else { return };
-            let list = if *fills { &n.fills } else { &n.strokes };
+            let Some(list) = edited_list(&n, *fills) else { return };
             let Some(mut paint) = list.get(*index).cloned() else { return };
             if let PaintKind::Gradient(g) = &mut paint.kind {
                 match which {
@@ -727,7 +736,7 @@ fn apply(app: &mut FigApp, drag: &Drag, pos: Pos2, shift: bool) {
                 (ax.min(ax + nw), ay.min(ay + nh), ax.max(ax + nw), ay.max(ay + nh))
             };
             let Some(n) = app.session.doc().ok().and_then(|d| d.node(*id)).cloned() else { return };
-            let list = if *fills { &n.fills } else { &n.strokes };
+            let Some(list) = edited_list(&n, *fills) else { return };
             let Some(mut paint) = list.get(*index).cloned() else { return };
             if let PaintKind::Image(img) = &mut paint.kind {
                 img.crop = Some(ImagePaint::crop_from_rect(r.0, r.1, r.2, r.3));
@@ -749,3 +758,6 @@ fn apply(app: &mut FigApp, drag: &Drag, pos: Pos2, shift: bool) {
         }
     }
 }
+
+#[cfg(test)]
+mod character_fill_tests;

crates/ui-egui/src/canvas/handles/character_fill_tests.rs

@@ -0,0 +1,40 @@
+//! The canvas crop handles on a text whose characters share an image fill the layer doesn't have
+//! (#1856): they show and edit the characters' image, the one the Fill section and the colour
+//! picker show, not the layer's own fill at that index.
+
+use base64::Engine as _;
+use figcraft_color::{Color, ImageScaleMode, Paint, PaintKind};
+use figcraft_engine::Session;
+use serde_json::json;
+
+use super::crop_target;
+use crate::colorpicker::PickerTarget;
+use crate::layers_drag_tests::Ui;
+use crate::present_tests::id;
+
+#[test]
+fn the_crop_handles_follow_the_characters_shared_image_in_crop_mode() {
+    let mut s = Session::new();
+    let t = id(&s.execute("text.create", &json!({"text": "Hello", "x": 0, "y": 0})).unwrap());
+    s.execute("fill.set", &json!({"ids": [t.0], "color": "#00FF00"})).unwrap();
+    let image = image::RgbaImage::from_pixel(40, 20, image::Rgba([200, 30, 30, 255]));
+    let mut png = std::io::Cursor::new(Vec::new());
+    image.write_to(&mut png, image::ImageFormat::Png).unwrap();
+    let data = base64::engine::general_purpose::STANDARD.encode(png.into_inner());
+    s.execute("image.place", &json!({"data": data, "target": t.0, "mode": "fill"})).unwrap();
+    s.execute("image.crop", &json!({"ids": [t.0], "index": 1})).unwrap();
+    // The cropped image goes to the characters; the layer keeps only its green.
+    let cropped = s.doc().unwrap().node(t).unwrap().fills[1].clone();
+    s.execute("text.style", &json!({"id": t.0, "start": 0, "end": 5, "textFills": [cropped]})).unwrap();
+    s.doc_mut().unwrap().get_mut(t).unwrap().fills = vec![Paint::solid(Color::from_hex("#00FF00").unwrap())];
+    let mut u = Ui::new(s);
+    // The picker on the Fill section's row: the characters' image, already in Crop mode.
+    u.app.ui.picker = Some(PickerTarget::Paint { fills: true, index: 0, ids: vec![t] });
+    let (at, fills, index, img, _) = crop_target(&u.app).expect("the handles show the characters' image");
+    assert_eq!((at, fills, index), (t, true, 0));
+    assert_eq!(img.scale_mode, ImageScaleMode::Crop);
+    assert!(
+        matches!(u.app.session.doc().unwrap().node(t).unwrap().fills[0].kind, PaintKind::Solid { .. }),
+        "the layer's own fill is not what they show"
+    );
+}

docs/ui-parity.md

@@ -33,7 +33,7 @@ interaction feel not yet compared.
 | Colour picker placement and dismissal | done | Opens 8 pt beside the sidebar at the opening control, lifted to fit the screen; outside presses (including the sidebar), Escape and X close it. Reopening resets a dragged position; eyedropper sampling keeps it open. |
 | Colour picker image well | partial | the Image tab shows a checkered placeholder and Upload from computer until there is a file (no dialog at once); an image paint shows its asset preview; a file dropped on the open picker replaces the image, keeping the fill mode, crop, position and adjustments (`image.replace`). Layers' fills and strokes only (text ranges, styles and selection colours keep Upload). The Fill and Stroke swatches take no drop: a dragged file's position isn't known, so a drop with none goes to the open picker |
 | Colour picker on a new fill or stroke | done | + opens the picker on the new paint when the selection shares one paint list; on a mixed selection + adds a paint to each layer and no picker opens (#404) |
-| Fill section on a text with coloured characters | done | a text's Fill section shows what its characters show: characters coloured differently while editing are mixed content ("Click + to replace mixed content", as a mixed selection; the developer documentation's `fills`: "Text nodes can have multiple sets of fills if some characters are colored differently than others"), and characters that all share fills the layer doesn't have show those. Any layer-level fill edit (a row, its picker, +, reordering, `fill.*`, `node.set`) reaches every character, starting from the list they showed; the characters' own fills go in the same undo step (`text_mixed_fill_tests`, engine `text_fill_tests`; #1856). A range selected while editing keeps its own Fill row |
+| Fill section on a text with coloured characters | done | a text's Fill section shows what its characters show: characters coloured differently while editing are mixed content ("Click + to replace mixed content", as a mixed selection; the developer documentation's `fills`: "Text nodes can have multiple sets of fills if some characters are colored differently than others"), and characters that all share fills the layer doesn't have show those. Any layer-level fill edit (a row, its picker, +, reordering, `fill.*`, `node.set`) reaches every character, starting from the list they showed; the characters' own fills go in the same undo step; the canvas gradient and crop handles show and edit the paint the row shows, the characters' own, also when their image is already in Crop mode (`text_mixed_fill_tests`, `canvas::handles::character_fill_tests`, engine `text_fill_tests`; #1856). A range selected while editing keeps its own Fill row |
 | Blend mode menus: the layer's (Design panel) and each paint's (colour picker) | done | Pass through and Normal are distinct (Normal isolates a group's or frame's children); paints have no pass-through; plus darker drawn exactly on the canvas |
 
 ## Layers and Assets panels