Add fills and border widths to text boxes and callouts (#3366)

* Support fills and border widths for text comments

* Allow borderless text comment defaults

crates/annot/src/appearance.rs

@@ -174,7 +174,7 @@ fn interior_color(d: &Dict, fallback: Option<Rgb>) -> Option<Rgb> {
     color(d, b"IC").flatten().or(fallback)
 }
 
-fn border_width(d: &Dict) -> f64 {
+pub(crate) fn border_width(d: &Dict) -> f64 {
     if let Some(w) = d.get(b"BS").and_then(|b| b.as_dict()).and_then(|b| b.get(b"W")).and_then(|w| w.as_f64()) {
         return w.max(0.0);
     }

crates/annot/src/lib.rs

@@ -1987,13 +1987,12 @@ pub fn set_font_size(doc: &mut Document, page: usize, index: usize, size: f64, m
     result
 }
 
-/// Whether a comment of this subtype has an interior that can be filled (`/IC`: rectangles,
-/// ovals, polygons and clouds, §12.5.6.8–9).
+/// Whether a comment has a fill: `/IC` for shapes, `/C` for FreeText (§12.5.6).
 pub fn fillable(subtype: &str) -> bool {
-    matches!(subtype, "Square" | "Circle" | "Polygon")
+    matches!(subtype, "Square" | "Circle" | "Polygon" | "FreeText")
 }
 
-/// Fill a rectangle, oval or polygon comment with `fill`, or remove its fill (`None`), and
+/// Fill a shape or FreeText comment with `fill`, or remove its fill (`None`), and
 /// redraw its appearance (Acrobat: Properties ▸ Fill Color).
 pub fn set_fill(doc: &mut Document, page: usize, index: usize, fill: Option<Rgb>, meta: &Meta) -> Result<(), AnnotError> {
     let (_, r) = annot_ref(doc, page, index)?;
@@ -2001,7 +2000,7 @@ pub fn set_fill(doc: &mut Document, page: usize, index: usize, fill: Option<Rgb>
     let d = annot_dict(doc, r);
     let subtype = String::from_utf8_lossy(d.name(b"Subtype").unwrap_or_default()).into_owned();
     if !fillable(&subtype) {
-        return Err(AnnotError::Invalid("only rectangles, ovals and polygons can be filled".into()));
+        return Err(AnnotError::Invalid("only rectangles, ovals, polygons and text boxes can be filled".into()));
     }
     // Check before changing anything: a stale appearance would contradict the new fill.
     if appearance::build(&d).is_none() {
@@ -2013,10 +2012,11 @@ pub fn set_fill(doc: &mut Document, page: usize, index: usize, fill: Option<Rgb>
     refuse_foreign_text(doc, &d)?;
     let before = d;
     doc.update_dict(r, |d| {
+        let key: &[u8] = if subtype == "FreeText" { b"C" } else { b"IC" };
         match fill {
-            Some(c) => d.set(b"IC".to_vec(), rgb(c)),
+            Some(c) => d.set(key.to_vec(), rgb(c)),
             None => {
-                d.remove(b"IC");
+                d.remove(key);
             }
         }
         touch(d, meta);
@@ -2205,7 +2205,7 @@ pub struct Props {
     pub subject: String,
     pub color: Option<Rgb>,
     pub opacity: f64,
-    /// Border / line width, for shapes, lines and drawings.
+    /// Border / line width, for shapes, lines, drawings and text boxes.
     pub width: Option<f64>,
     /// Notes only.
     pub icon: Option<NoteIcon>,
@@ -2217,9 +2217,9 @@ pub struct Props {
     pub locked: bool,
     /// `/LE`: two names for a line or polyline (`None` when unset), one for a callout.
     pub endings: Option<Vec<LineEnding>>,
-    /// Rectangles, ovals and polygons: their interior can be filled (see [`fillable`]).
+    /// Whether the comment has a fill control (see [`fillable`]).
     pub fillable: bool,
-    /// `/IC`, the interior colour (`None`: no fill).
+    /// The interior colour (`None`: no fill).
     pub fill: Option<Rgb>,
     /// Text boxes, typewriter text and callouts: the `/DA` font size in points.
     pub font_size: Option<f64>,
@@ -2234,8 +2234,13 @@ pub fn props(doc: &Document, page: usize, index: usize) -> Option<Props> {
     let subtype = String::from_utf8_lossy(d.name(b"Subtype")?).into_owned();
     let c: Vec<f64> = d.get(b"C").and_then(|o| o.as_array().map(|a| a.iter().filter_map(|x| x.as_f64()).collect())).unwrap_or_default();
     let color = if subtype == "FreeText" { Some(appearance::parse_da(d).0) } else { (c.len() == 3).then(|| [c[0], c[1], c[2]]) };
-    let width = matches!(subtype.as_str(), "Square" | "Circle" | "Line" | "Ink" | "Polygon" | "PolyLine")
-        .then(|| d.get(b"BS").and_then(|b| b.as_dict()).and_then(|b| b.get(b"W")).and_then(|w| w.as_f64()).unwrap_or(1.0));
+    let width = matches!(subtype.as_str(), "Square" | "Circle" | "Line" | "Ink" | "Polygon" | "PolyLine" | "FreeText").then(|| {
+        if subtype == "FreeText" {
+            if !d.contains(b"BS") && !d.contains(b"Border") { 0.0 } else { appearance::border_width(d) }
+        } else {
+            d.get(b"BS").and_then(|b| b.as_dict()).and_then(|b| b.get(b"W")).and_then(|w| w.as_f64()).unwrap_or(1.0)
+        }
+    });
     Some(Props {
         author: text_value(doc, d, b"T").unwrap_or_default(),
         subject: text_value(doc, d, b"Subj").unwrap_or_default(),
@@ -2254,9 +2259,10 @@ pub fn props(doc: &Document, page: usize, index: usize) -> Option<Props> {
     })
 }
 
-/// `/IC` as an RGB colour. An empty array means no fill; grey and CMYK fills are converted.
+/// A shape’s `/IC` or FreeText `/C` as RGB. Grey and CMYK fills are converted.
 fn fill_of(d: &Dict) -> Option<Rgb> {
-    let c: Vec<f64> = d.get(b"IC")?.as_array()?.iter().filter_map(|x| x.as_f64()).collect();
+    let key: &[u8] = if d.name(b"Subtype") == Some(b"FreeText") { b"C" } else { b"IC" };
+    let c: Vec<f64> = d.get(key)?.as_array()?.iter().filter_map(|x| x.as_f64()).collect();
     let rgb = match c.as_slice() {
         [g] => [*g; 3],
         [r, g, b] => [*r, *g, *b],

crates/annot/src/tests.rs

@@ -1335,3 +1335,50 @@ fn text_box_font_size_changes_the_da_and_grows_the_box() {
         assert_eq!(props(&doc, 0, i).unwrap().font_size, None);
     }
 }
+
+#[test]
+fn text_boxes_and_callouts_can_change_fill_and_border() {
+    let mut doc = fixture();
+    let shapes = [
+        Shape::TextBox { rect: [300.0, 600.0, 500.0, 660.0], font_size: 12.0 },
+        Shape::Callout {
+            rect: [300.0, 600.0, 500.0, 660.0],
+            knee: [260.0, 560.0],
+            point: [200.0, 520.0],
+            font_size: 12.0,
+            ending: LineEnding::OpenArrow,
+        },
+    ];
+    for shape in shapes {
+        let index = add_annotation(&mut doc, &new(0, shape), &meta("test")).unwrap();
+        assert!(props(&doc, 0, index).unwrap().fillable);
+        set_fill(&mut doc, 0, index, Some([1.0, 1.0, 1.0]), &meta("test")).unwrap();
+        set_style(&mut doc, 0, index, None, None, Some(3.0), None, &meta("test")).unwrap();
+        let saved = reopen(&doc);
+        let properties = props(&saved, 0, index).unwrap();
+        assert_eq!(properties.fill, Some([1.0; 3]));
+        assert_eq!(properties.width, Some(3.0));
+        let ap = ap_content(&saved, &list(&saved, 0)[index]);
+        assert!(ap.contains("1 1 1 rg") && ap.contains(" re f") && ap.contains("3 w"), "{ap}");
+        set_fill(&mut doc, 0, index, None, &meta("test")).unwrap();
+        assert_eq!(props(&doc, 0, index).unwrap().fill, None);
+    }
+}
+
+#[test]
+fn free_text_properties_show_legacy_or_absent_border_width() {
+    let mut doc = fixture();
+    let index = add_annotation(&mut doc, &new(0, Shape::TextBox { rect: [100.0, 100.0, 200.0, 140.0], font_size: 12.0 }), &meta("test")).unwrap();
+    let (_, reference) = annot_ref(&mut doc, 0, index).unwrap();
+    doc.update_dict(reference, |d| {
+        d.remove(b"BS");
+        d.remove(b"Border");
+    })
+    .unwrap();
+    assert_eq!(props(&doc, 0, index).unwrap().width, Some(0.0));
+    doc.update_dict(reference, |d| {
+        d.set(b"Border".to_vec(), Object::Array([0.0, 0.0, 4.0].into_iter().map(Object::Real).collect()));
+    })
+    .unwrap();
+    assert_eq!(props(&doc, 0, index).unwrap().width, Some(4.0));
+}

crates/automation/src/tools.rs

@@ -784,13 +784,13 @@ pub fn tools() -> Vec<ToolDef> {
                 json!({ "doc": doc(), "sort": { "type": "string", "enum": ["page", "author", "date", "type"] }, "out": save_out, "open": open }),
                 &["doc"],
             )),
-        t("comment_edit", "Edit a comment", "Change a comment's text, colour, opacity, line width, font size (text box/typewriter/callout), fill (rectangle/oval/polygon; \"none\" removes it), rectangle (rectangle/oval/text box/stamp) or position (`move` [dx, dy] in points). Stamps keep their original appearance when resized. One undo step.").with(schema(
+        t("comment_edit", "Edit a comment", "Change a comment's text, colour, opacity, line width, font size (text box/typewriter/callout), fill (rectangle/oval/polygon/text box/callout; \"none\" removes it), rectangle (rectangle/oval/text box/stamp) or position (`move` [dx, dy] in points). Stamps keep their original appearance when resized. One undo step.").with(schema(
             comment_ref(json!({
                 "contents": { "type": "string" },
                 "color": color(),
                 "opacity": { "type": "number", "minimum": 0, "maximum": 1 },
                 "width": { "type": "number", "minimum": 0 },
-                "fill": { "type": "string", "description": "Interior colour of a rectangle, oval or polygon: #RRGGBB, a colour name, or \"none\" for no fill." },
+                "fill": { "type": "string", "description": "Interior colour of a rectangle, oval, polygon, text box or callout: #RRGGBB, a colour name, or \"none\" for no fill." },
                 "font_size": { "type": "number", "minimum": 1, "maximum": 400, "description": "Font size in points of a text box, typewriter text or callout; the box grows to hold the text." },
                 "rect": { "type": "array", "items": { "type": "number" }, "minItems": 4, "maxItems": 4 },
                 "move": point(),

crates/automation/tests/automation.rs

@@ -6765,3 +6765,34 @@ fn a_root_less_write_output_is_atomic_too() {
     assert!(matches!(&e, ToolError::Failed(m) if m.contains("is a folder")), "{e:?}");
     let _ = std::fs::remove_dir_all(&dir);
 }
+
+#[test]
+fn text_box_and_callout_fill_and_border_through_tools() {
+    let dir = workdir("textbox-fill");
+    let mut a = auto(&dir);
+    let doc = ok(&mut a, "doc_open", json!({ "path": "a.pdf" }))["doc"].as_u64().unwrap();
+    for kind in ["textbox", "callout"] {
+        let mut args = json!({ "doc": doc, "page": 1, "type": kind, "rect": [80, 40, 180, 100], "contents": "Example" });
+        if kind == "callout" {
+            args["to"] = json!([20, 120]);
+        }
+        let added = ok(&mut a, "comment_add", args);
+        let comment = &added["comment"];
+        ok(&mut a, "comment_edit", json!({ "doc": doc, "page": 1, "index": comment["index"], "fill": "white", "width": 3 }));
+    }
+    ok(&mut a, "doc_save", json!({ "doc": doc, "path": "filled.pdf" }));
+    let mut session = pdfcraft_engine::Session::new();
+    let bytes = std::sync::Arc::new(std::fs::read(dir.join("filled.pdf")).unwrap());
+    let saved = session.open("filled.pdf", None, bytes, None).unwrap();
+    for index in 0..2 {
+        let props = session.get(saved).unwrap().comment_props(0, index).unwrap();
+        assert_eq!((props.fill, props.width), (Some([1.0; 3]), Some(3.0)));
+        ok(&mut a, "comment_edit", json!({ "doc": doc, "page": 1, "index": index + 1, "fill": "none" }));
+    }
+    ok(&mut a, "doc_save", json!({ "doc": doc, "path": "transparent.pdf" }));
+    let bytes = std::sync::Arc::new(std::fs::read(dir.join("transparent.pdf")).unwrap());
+    let saved = session.open("transparent.pdf", None, bytes, None).unwrap();
+    for index in 0..2 {
+        assert_eq!(session.get(saved).unwrap().comment_props(0, index).unwrap().fill, None);
+    }
+}

crates/ui-egui/src/comment_props.rs

@@ -109,8 +109,7 @@ impl PdfCraftApp {
             style.fill = p.fill;
         }
         self.comment_prefs.set_style(tool, style);
-        // Opacity and width go through the clamping setters: the dialog allows 0% opacity, and a
-        // default that draws invisible comments can't be found again.
+        // Keep invisible defaults out of tools where a zero stroke would make them disappear.
         self.comment_prefs.set_opacity(tool, p.opacity);
         if let Some(w) = p.width {
             self.comment_prefs.set_width(tool, w);
@@ -240,9 +239,10 @@ pub(crate) fn body(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens) -> (boo
                         e.opacity = pct / 100.0;
                     }
                     ui.end_row();
+                    let min_width = if e.font_size.is_some() { 0.0 } else { 0.5 };
                     if let Some(w) = e.width.as_mut() {
                         ui.label(tl!("Thickness"));
-                        ui.add(egui::Slider::new(w, 0.5..=12.0).step_by(0.5).suffix(" pt"));
+                        ui.add(egui::Slider::new(w, min_width..=12.0).step_by(0.5).suffix(" pt"));
                         ui.end_row();
                     }
                     if let Some(ends) = e.endings.as_mut() {

crates/ui-egui/src/comments.rs

@@ -188,12 +188,17 @@ impl CommentTool {
 
     /// Whether the line-thickness control applies.
     pub fn has_width(self) -> bool {
-        self.draws() || self.clicks_points()
+        self.draws() || self.clicks_points() || matches!(self, Self::TextBox | Self::Callout)
     }
 
-    /// Whether the fill-colour control applies: shapes with an interior (#686).
+    /// Text comments may have no border while retaining visible text or fill.
+    fn min_width(self) -> f64 {
+        if matches!(self, Self::TextBox | Self::Callout) { 0.0 } else { 0.5 }
+    }
+
+    /// Whether the fill-colour control applies to this tool.
     pub fn has_fill(self) -> bool {
-        matches!(self, Self::Rectangle | Self::Oval | Self::Polygon | Self::Cloud)
+        matches!(self, Self::Rectangle | Self::Oval | Self::Polygon | Self::Cloud | Self::TextBox | Self::Callout)
     }
 
     /// A placeholder shape of this kind (for per-tool default styles).
@@ -284,7 +289,7 @@ impl CommentPrefs {
 
     pub fn set_width(&mut self, tool: CommentTool, w: f64) {
         if let Some((_, s)) = self.styles.iter_mut().find(|(t, _)| *t == tool) {
-            s.width = w.clamp(0.5, 12.0);
+            s.width = w.clamp(tool.min_width(), 12.0);
         }
     }
 
@@ -295,7 +300,7 @@ impl CommentPrefs {
         }
     }
 
-    /// The tool's fill (`None`: no fill). Only shapes with an interior keep one.
+    /// The tool's fill (`None`: no fill).
     pub fn set_fill(&mut self, tool: CommentTool, fill: Option<Rgb>) {
         if let Some((_, s)) = self.styles.iter_mut().find(|(t, _)| *t == tool) {
             s.fill = if tool.has_fill() { fill.filter(|c| c.iter().all(|x| x.is_finite())).map(|c| c.map(|x| x.clamp(0.0, 1.0))) } else { None };
@@ -1899,7 +1904,7 @@ pub(crate) fn quick_bar_controls(ui: &mut egui::Ui, tool: CommentTool, prefs: &m
             ui.set_min_width(180.0);
             let mut w = style.width;
             ui.label(egui::RichText::new(tl!("Line thickness")).font(theme::semibold(12.0)));
-            if ui.add(egui::Slider::new(&mut w, 0.5..=12.0).step_by(0.5).suffix(" pt")).changed() {
+            if ui.add(egui::Slider::new(&mut w, tool.min_width()..=12.0).step_by(0.5).suffix(" pt")).changed() {
                 prefs.set_width(tool, w);
             }
         });

crates/ui-egui/tests/comments.rs

@@ -1571,3 +1571,77 @@ fn text_box_properties_change_the_font_size() {
     assert_eq!(doc.comment_props(0, 0).unwrap().font_size, Some(30.0));
     assert_eq!(doc.can_undo(), Some("Change comment properties"));
 }
+
+#[test]
+fn text_box_and_callout_properties_include_fill_and_thickness() {
+    use pdfcraft_engine::{Edit, LineEnding, NewAnnotation, Shape};
+    use pdfcraft_ui_egui::comments::CommentTool;
+    let mut h = harness(|_| {});
+    for (tool, shape) in [
+        (CommentTool::TextBox, Shape::TextBox { rect: [40.0, 600.0, 240.0, 640.0], font_size: 12.0 }),
+        (
+            CommentTool::Callout,
+            Shape::Callout {
+                rect: [40.0, 500.0, 240.0, 540.0],
+                knee: [30.0, 460.0],
+                point: [10.0, 440.0],
+                font_size: 12.0,
+                ending: LineEnding::OpenArrow,
+            },
+        ),
+    ] {
+        let index = comments(&h).len();
+        h.state_mut().apply_edit(Edit::AddAnnotation(NewAnnotation {
+            page: 0,
+            shape,
+            style: Default::default(),
+            contents: "Hello".into(),
+            author: "Ada".into(),
+        }));
+        h.run_steps(2);
+        h.state_mut().open_comment_props(0, index);
+        h.run_steps(2);
+        h.get_by_label("Fill colour");
+        {
+            let draft = h.state_mut().comment_props.as_mut().unwrap();
+            assert!(draft.original.fillable && draft.original.width.is_some());
+            draft.edited.fill = Some([1.0; 3]);
+            draft.edited.width = Some(3.0);
+        }
+        h.get_by_label("OK").click();
+        h.run_steps(3);
+        let doc = h.state().session.get(h.state().views[0].id).unwrap();
+        let p = doc.comment_props(0, index).unwrap();
+        assert_eq!((p.fill, p.width), (Some([1.0; 3]), Some(3.0)));
+        h.state_mut().make_comment_default(0, index);
+        assert_eq!(h.state().comment_prefs.style(tool).fill, Some([1.0; 3]));
+        assert_eq!(h.state().comment_prefs.style(tool).width, 3.0);
+    }
+}
+
+#[test]
+fn text_comments_can_save_a_zero_width_default() {
+    use pdfcraft_engine::{Edit, NewAnnotation, Shape};
+    use pdfcraft_ui_egui::comments::CommentTool;
+    let mut h = harness(|_| {});
+    let shape = Shape::TextBox { rect: [40.0, 600.0, 240.0, 640.0], font_size: 12.0 };
+    let mut style = pdfcraft_engine::Style::default_for(&shape);
+    style.width = 0.0;
+    h.state_mut().apply_edit(Edit::AddAnnotation(NewAnnotation { page: 0, shape, style, contents: "Hello".into(), author: "Ada".into() }));
+    h.run_steps(2);
+    h.state_mut().open_comment_props(0, 0);
+    h.run_steps(2);
+    assert_eq!(h.state().comment_props.as_ref().unwrap().edited.width, Some(0.0));
+    h.get_by_label("OK").click();
+    h.run_steps(2);
+    h.state_mut().make_comment_default(0, 0);
+    assert_eq!(h.state().comment_prefs.style(CommentTool::TextBox).width, 0.0);
+    h.state_mut().comment_prefs.set_width(CommentTool::Callout, 0.0);
+    h.state_mut().comment_prefs.set_width(CommentTool::Rectangle, 0.0);
+    assert_eq!(h.state().comment_prefs.style(CommentTool::Callout).width, 0.0);
+    assert_eq!(h.state().comment_prefs.style(CommentTool::Rectangle).width, 0.5);
+    let mut restored = PdfCraftApp::new();
+    restored.restore(&h.state().persist());
+    assert_eq!(restored.comment_prefs.style(CommentTool::TextBox).width, 0.0);
+    assert_eq!(restored.comment_prefs.style(CommentTool::Callout).width, 0.0);
+}

parity/acrobat-features.toml

@@ -3860,8 +3860,8 @@ milestone = "M5"
 status = "shipped"
 commands = ["comment.list"]
 tools = ["comment_edit"]
-evidence = ["crates/ui-egui/tests/comments.rs::comment_properties_change_appearance_and_author", "crates/annot/src/tests.rs::author_subject_and_note_icon_change", "crates/ui-egui/tests/comments.rs::shapes_take_a_fill_colour_from_the_quick_bar_and_properties", "crates/ui-egui/tests/comments.rs::text_box_properties_change_the_font_size", "crates/annot/src/tests.rs::text_box_font_size_changes_the_da_and_grows_the_box", "crates/automation/tests/automation.rs::text_box_font_size_through_the_tool"]
-notes = "Appearance (colour, fill colour for rectangles/ovals/polygons, opacity, thickness, note icon, font size of text boxes, typewriter text and callouts: 1-400 pt, written to /DA and the rich-text strings, the box grows to fit and never shrinks; comment_edit font_size), General (author, subject), Review History. Still missing: font family, bold/italic, alignment and per-run formatting of text boxes; the quick bar has no font size for new text boxes (they start at 12 pt, callouts 10 pt)"
+evidence = ["crates/ui-egui/tests/comments.rs::comment_properties_change_appearance_and_author", "crates/annot/src/tests.rs::author_subject_and_note_icon_change", "crates/ui-egui/tests/comments.rs::shapes_take_a_fill_colour_from_the_quick_bar_and_properties", "crates/ui-egui/tests/comments.rs::text_box_properties_change_the_font_size", "crates/ui-egui/tests/comments.rs::text_box_and_callout_properties_include_fill_and_thickness", "crates/annot/src/tests.rs::text_boxes_and_callouts_can_change_fill_and_border", "crates/automation/tests/automation.rs::text_box_and_callout_fill_and_border_through_tools", "crates/annot/src/tests.rs::text_box_font_size_changes_the_da_and_grows_the_box", "crates/automation/tests/automation.rs::text_box_font_size_through_the_tool"]
+notes = "Appearance (colour, fill colour for rectangles/ovals/polygons/text boxes/callouts, opacity, thickness, note icon, font size of text boxes, typewriter text and callouts: 1-400 pt, written to /DA and the rich-text strings, the box grows to fit and never shrinks; comment_edit font_size), General (author, subject), Review History. Still missing: font family, bold/italic, alignment and per-run formatting of text boxes; the quick bar has no font size for new text boxes (they start at 12 pt, callouts 10 pt)"
 
 [[feature]]
 id = "comment.properties-bar"