diff --git a/crates/annot/src/appearance.rs b/crates/annot/src/appearance.rs index 0f0706428..3e1dd4a6b 100644 --- a/crates/annot/src/appearance.rs +++ b/crates/annot/src/appearance.rs @@ -174,7 +174,7 @@ fn interior_color(d: &Dict, fallback: Option) -> Option { 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); } diff --git a/crates/annot/src/lib.rs b/crates/annot/src/lib.rs index b7f9d6269..587cd4466 100644 --- a/crates/annot/src/lib.rs +++ b/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, 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 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 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, pub opacity: f64, - /// Border / line width, for shapes, lines and drawings. + /// Border / line width, for shapes, lines, drawings and text boxes. pub width: Option, /// Notes only. pub icon: Option, @@ -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>, - /// 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, /// Text boxes, typewriter text and callouts: the `/DA` font size in points. pub font_size: Option, @@ -2234,8 +2234,13 @@ pub fn props(doc: &Document, page: usize, index: usize) -> Option { let subtype = String::from_utf8_lossy(d.name(b"Subtype")?).into_owned(); let c: Vec = 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 { }) } -/// `/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 { - let c: Vec = 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 = 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], diff --git a/crates/annot/src/tests.rs b/crates/annot/src/tests.rs index d3d448e52..4a4dfb106 100644 --- a/crates/annot/src/tests.rs +++ 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)); +} diff --git a/crates/automation/src/tools.rs b/crates/automation/src/tools.rs index df7ca9100..e154e1eeb 100644 --- a/crates/automation/src/tools.rs +++ b/crates/automation/src/tools.rs @@ -784,13 +784,13 @@ pub fn tools() -> Vec { 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(), diff --git a/crates/automation/tests/automation.rs b/crates/automation/tests/automation.rs index 2f0b02684..c6e1a574f 100644 --- a/crates/automation/tests/automation.rs +++ b/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); + } +} diff --git a/crates/ui-egui/src/comment_props.rs b/crates/ui-egui/src/comment_props.rs index 052fcd55f..19f7187af 100644 --- a/crates/ui-egui/src/comment_props.rs +++ b/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() { diff --git a/crates/ui-egui/src/comments.rs b/crates/ui-egui/src/comments.rs index 68b1a72a0..7fd9b7a62 100644 --- a/crates/ui-egui/src/comments.rs +++ b/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) { 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); } }); diff --git a/crates/ui-egui/tests/comments.rs b/crates/ui-egui/tests/comments.rs index 3d53f1e3b..01166a41d 100644 --- a/crates/ui-egui/tests/comments.rs +++ b/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); +} diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 3fce5fb98..8d7fabed1 100644 --- a/parity/acrobat-features.toml +++ b/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"