From faebcfd2649e42ee892d64d04d19a5ed05d02e42 Mon Sep 17 00:00:00 2001 From: archamon Date: Sun, 1 Nov 2026 20:58:16 +0000 Subject: [PATCH] fix(annot,ui): stabilize ink width bounds, search translated tool labels, and fix hotbar settings label --- crates/annot/src/create.rs | 18 +++++++----- crates/annot/src/modify.rs | 41 +++++++++++++++++++++------ crates/annot/src/tests.rs | 34 ++++++++++++++++++++-- crates/ui-egui/src/hotbar.rs | 31 +++++++++++++++----- crates/ui-egui/src/workflows/js_ui.rs | 8 ++++-- crates/ui-egui/tests/it/comments.rs | 31 +++++++++++++++----- crates/ui-egui/tests/it/ui.rs | 4 +-- 7 files changed, 131 insertions(+), 36 deletions(-) diff --git a/crates/annot/src/create.rs b/crates/annot/src/create.rs index d897133bce..1904745e95 100644 --- a/crates/annot/src/create.rs +++ b/crates/annot/src/create.rs @@ -33,6 +33,16 @@ pub(crate) fn grow(r: [f64; 4], by: f64) -> [f64; 4] { [r[0] - by, r[1] - by, r[2] + by, r[3] + by] } +/// Tight bounding box of ink strokes, including Bezier control points for strokes with three or +/// more points (matching [`appearance::smooth_segments`]). +pub(crate) fn ink_bounds(strokes: &[Vec<[f64; 2]>]) -> Option<[f64; 4]> { + let controls = strokes.iter().filter(|s| s.len() > 2).flat_map(|s| { + let pts: Vec<(f64, f64)> = s.iter().map(|p| (p[0], p[1])).collect(); + appearance::smooth_segments(&pts).into_iter().flat_map(|[a, b, _]| [[a.0, a.1], [b.0, b.1]]) + }); + bounds(strokes.iter().flatten().copied().chain(controls)) +} + pub(crate) fn ending_draws(ending: LineEnding) -> bool { ending != LineEnding::None } @@ -91,13 +101,7 @@ pub(crate) fn rect_for(shape: &Shape, style: &Style) -> Result<[f64; 4], AnnotEr if strokes.iter().all(Vec::is_empty) || !strokes.iter().flatten().all(|p| finite(p)) { return Err(bad("drawing (no points)")); } - // Strokes of three or more points are drawn as curves, which stay within their - // points and control points. - let controls = strokes.iter().filter(|s| s.len() > 2).flat_map(|s| { - let pts: Vec<(f64, f64)> = s.iter().map(|p| (p[0], p[1])).collect(); - appearance::smooth_segments(&pts).into_iter().flat_map(|[a, b, _]| [[a.0, a.1], [b.0, b.1]]) - }); - grow(bounds(strokes.iter().flatten().copied().chain(controls)).unwrap_or_default(), half + 1.0) + grow(ink_bounds(strokes).unwrap_or_default(), half + 1.0) } Shape::Polygon { vertices, cloud } => { let b = bounds(vertices.iter().copied()) diff --git a/crates/annot/src/modify.rs b/crates/annot/src/modify.rs index fdddbc8718..a615b8e013 100644 --- a/crates/annot/src/modify.rs +++ b/crates/annot/src/modify.rs @@ -1,5 +1,5 @@ use super::create::{ - add_annotation, base_dict, bounds, callout_attach, ending_draws, ending_pair, finite, grow, normalize, num_array, rect_for, rgb, + add_annotation, base_dict, bounds, callout_attach, ending_draws, ending_pair, finite, grow, ink_bounds, normalize, num_array, rect_for, rgb, }; use super::draw::{draw_appearance, redraw_or_restore, set_appearance}; use super::info::{line_endings_of, text_value}; @@ -529,7 +529,33 @@ fn apply_text_box(doc: &mut Document, r: ObjRef, rect: [f64; 4], meta: &Meta, ne } fn border_width_of(d: &Dict) -> f64 { - 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).max(0.0) + appearance::border_width(d) +} + +/// Recompute `/Rect` for an `Ink` annotation when its stroke width changes from `old_w` to `w`. +/// Preserves any baseline margin beyond `old_w / 2.0` (at least `1.0` pt) without accumulating +/// padding across repeated width increases and decreases. +fn ink_rect_for_width(d: &Dict, old_w: f64, w: f64) -> Option<[f64; 4]> { + let list = d.get(b"InkList")?.as_array()?; + let mut strokes: Vec> = Vec::with_capacity(list.len()); + for s in list { + let pts: Vec = s.as_array()?.iter().map(Object::as_f64).collect::>()?; + strokes.push(pts.as_chunks::<2>().0.iter().map(|p| [p[0], p[1]]).collect()); + } + let inner = ink_bounds(&strokes)?; + let vals: Vec = d.get(b"Rect").and_then(Object::as_array).map(|a| a.iter().filter_map(Object::as_f64).collect()).unwrap_or_default(); + let Some(old_rect) = four(&vals).map(normalize) else { + return Some(grow(inner, w / 2.0 + 1.0)); + }; + let old_half = old_w / 2.0; + let new_half = w / 2.0; + let base = [ + (inner[0] - old_rect[0] - old_half).max(1.0), + (inner[1] - old_rect[1] - old_half).max(1.0), + (old_rect[2] - inner[2] - old_half).max(1.0), + (old_rect[3] - inner[3] - old_half).max(1.0), + ]; + Some([inner[0] - base[0] - new_half, inner[1] - base[1] - new_half, inner[2] + base[2] + new_half, inner[3] + base[3] + new_half]) } /// Change a comment's colour, opacity, line width and/or line endings, and redraw it. @@ -582,13 +608,10 @@ pub fn set_style(doc: &mut Document, page: usize, index: usize, change: StyleCha bs.set(b"W".to_vec(), Object::Real(w)); d.set(b"BS".to_vec(), Object::Dict(bs)); d.remove(b"Border"); - if d.name(b"Subtype") == Some(b"Ink") && w > old_w { - let vals: Vec = - d.get(b"Rect").and_then(|o| o.as_array()).map(|a| a.iter().filter_map(|x| x.as_f64()).collect()).unwrap_or_default(); - if let Some(rect) = four(&vals) { - let expanded = grow(normalize(rect), (w - old_w) / 2.0); - d.set(b"Rect".to_vec(), num_array(&expanded)); - } + if d.name(b"Subtype") == Some(b"Ink") + && let Some(rect) = ink_rect_for_width(d, old_w, w) + { + d.set(b"Rect".to_vec(), num_array(&rect)); } } if let Some(ends) = endings { diff --git a/crates/annot/src/tests.rs b/crates/annot/src/tests.rs index 985e98fb01..cf800fa98a 100644 --- a/crates/annot/src/tests.rs +++ b/crates/annot/src/tests.rs @@ -2214,9 +2214,39 @@ fn freehand_highlight_is_compatible_ink_with_marker_blending() { assert_eq!(blend.name(b"BM"), Some(&b"Multiply"[..])); let mut doc = doc; - let rect_before: Vec = list(&doc, 0)[index].get(b"Rect").unwrap().as_array().unwrap().iter().filter_map(Object::as_f64).collect(); + let rect_of = |doc: &Document, idx: usize| -> Vec { + list(doc, 0)[idx].get(b"Rect").unwrap().as_array().unwrap().iter().filter_map(Object::as_f64).collect() + }; + let rect_before = rect_of(&doc, index); set_style(&mut doc, 0, index, StyleChange { color: None, opacity: None, width: Some(34.0), endings: None }, &meta("wider")).unwrap(); - let rect_after: Vec = list(&doc, 0)[index].get(b"Rect").unwrap().as_array().unwrap().iter().filter_map(Object::as_f64).collect(); + let rect_after = rect_of(&doc, index); assert!((rect_after[0] - (rect_before[0] - 10.0)).abs() < 1e-4); assert!((rect_after[2] - (rect_before[2] + 10.0)).abs() < 1e-4); + + // Repeated increase/decrease cycles must keep bounds stable and never accumulate padding. + set_style(&mut doc, 0, index, StyleChange { color: None, opacity: None, width: Some(14.0), endings: None }, &meta("narrower")).unwrap(); + let rect_restored = rect_of(&doc, index); + for (a, b) in rect_restored.iter().zip(rect_before.iter()) { + assert!((a - b).abs() < 1e-4, "expected {rect_before:?}, got {rect_restored:?}"); + } + set_style(&mut doc, 0, index, StyleChange { color: None, opacity: None, width: Some(34.0), endings: None }, &meta("wider-again")).unwrap(); + let rect_second_wide = rect_of(&doc, index); + for (a, b) in rect_second_wide.iter().zip(rect_after.iter()) { + assert!((a - b).abs() < 1e-4, "expected {rect_after:?}, got {rect_second_wide:?}"); + } + + // Imported ink whose width is specified only by `/Border [0 0 20]` must use 20 pt as its + // previous width (shrinking by 5 pt on a decrease to 10 pt rather than expanding as if 1 pt). + let (_, r) = lookup::annot_ref(&mut doc, 0, index).unwrap(); + doc.update_dict(r, |d| { + d.remove(b"BS"); + d.set(b"Border".to_vec(), Object::Array(vec![Object::Real(0.0), Object::Real(0.0), Object::Real(20.0)])); + d.set(b"Rect".to_vec(), create::num_array(&[5.0, 85.0, 135.0, 115.0])); + }) + .unwrap(); + set_style(&mut doc, 0, index, StyleChange { color: None, opacity: None, width: Some(10.0), endings: None }, &meta("border-shrink")).unwrap(); + let rect_from_border = rect_of(&doc, index); + for (a, b) in rect_from_border.iter().zip([10.0, 90.0, 130.0, 110.0]) { + assert!((a - b).abs() < 1e-4, "expected [10, 90, 130, 110], got {rect_from_border:?}"); + } } diff --git a/crates/ui-egui/src/hotbar.rs b/crates/ui-egui/src/hotbar.rs index 88c2fd4c6a..f6a0c14c7f 100644 --- a/crates/ui-egui/src/hotbar.rs +++ b/crates/ui-egui/src/hotbar.rs @@ -88,10 +88,7 @@ impl Hotbar { fn settings_button(ui: &mut egui::Ui, cell: f32, picker_open: &mut bool) { let settings = ui .push_id("hotbar-settings", |ui| { - let mut btn = icons::button(ui, "settings", cell, false, tl!("Hotbar settings")); - btn = btn.on_hover_text(tl!("Hotbar settings / add tools")); - btn.widget_info(|| egui::WidgetInfo::labeled(egui::Role::Button, true, "+")); - btn + icons::button(ui, "settings", cell, false, tl!("Hotbar settings")).on_hover_text(tl!("Hotbar settings / add tools")) }) .inner; if settings.clicked() { @@ -99,6 +96,13 @@ fn settings_button(ui: &mut egui::Ui, cell: f32, picker_open: &mut bool) { } } +/// Whether `spec` matches the hotbar picker search query across the translated label, canonical +/// English label, or command identifier (case-insensitively). +fn matches_search(spec: &pdfcraft_engine::commands::CommandSpec, query: &str) -> bool { + let search = query.to_lowercase(); + tl!(spec.label).to_lowercase().contains(&search) || spec.label.to_lowercase().contains(&search) || spec.id.to_lowercase().contains(&search) +} + /// Render the configured tool shortcut buttons and their removal context menus. fn tool_buttons(ui: &mut egui::Ui, app: &PdfCraftApp, tools: &[String], cell: f32, run: &mut Option<&'static str>, remove: &mut Option) { for (index, id) in tools.iter().enumerate() { @@ -270,18 +274,19 @@ pub(crate) fn show(app: &mut PdfCraftApp, area: Rect, ctx: &egui::Context) { } } }); + let strip_hint = tl!("Extend the docked toolbar along the entire side. Choose a side above to enable."); ui.add_enabled(bar.dock.is_some(), crate::widgets::checkbox(&mut bar.full_side, tl!("Full-side strip"))) - .on_hover_text(tl!("Extend the docked toolbar along the entire side. Choose a side above to enable.")); + .on_hover_text(strip_hint) + .on_disabled_hover_text(strip_hint); if ui.button(tl!("Close menu")).clicked() { ui.close(); } ui.separator(); ui.label(tl!("Add tools to hotbar")); ui.add(egui::TextEdit::singleline(&mut bar.search).hint_text(tl!("Search all tools…"))); - let search = bar.search.to_lowercase(); egui::ScrollArea::vertical().max_height(300.0).show(ui, |ui| { for spec in COMMANDS { - if !spec.label.to_lowercase().contains(&search) && !spec.id.contains(&search) { + if !matches_search(spec, &bar.search) { continue; } let mut selected = bar.tools.iter().any(|id| id == spec.id); @@ -352,4 +357,16 @@ mod tests { assert_eq!(app.hotbar.tools, ["comment.ink"]); assert_eq!(app.hotbar.position, [0.0, 1.0]); } + + #[test] + fn hotbar_search_matches_translated_and_canonical_labels() { + let ink = command("comment.ink").unwrap(); + crate::i18n::set_current(crate::i18n::lang_from_tag("fr").unwrap()); + assert_eq!(tl!(ink.label), "Dessiner à main levée"); + assert!(matches_search(ink, "dessiner"), "translated French label must match"); + assert!(matches_search(ink, "freehand"), "canonical English label must still match"); + assert!(matches_search(ink, "comment.ink"), "command ID must still match"); + assert!(!matches_search(ink, "introuvable")); + crate::i18n::set_current(crate::i18n::Lang::EN); + } } diff --git a/crates/ui-egui/src/workflows/js_ui.rs b/crates/ui-egui/src/workflows/js_ui.rs index 208ea30770..70c3f775e9 100644 --- a/crates/ui-egui/src/workflows/js_ui.rs +++ b/crates/ui-egui/src/workflows/js_ui.rs @@ -438,12 +438,16 @@ pub(crate) fn preferences_body(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tok ui.label(tl!("Navigation rail side")); let right = app.side_panels.rail_on_right(); let left_btn = ui.selectable_label(!right, tl!("Left")); - left_btn.widget_info(|| egui::WidgetInfo::selected(egui::Role::Button, true, !right, tl!("Navigation rail on left"))); + left_btn.widget_info(|| { + egui::WidgetInfo::selected(egui::Role::Button, true, !right, format!("{}: {}", tl!("Navigation rail side"), tl!("Left"))) + }); if left_btn.clicked() { app.side_panels.rail_side = crate::side_panels::ToolsSide::Left; } let right_btn = ui.selectable_label(right, tl!("Right")); - right_btn.widget_info(|| egui::WidgetInfo::selected(egui::Role::Button, true, right, tl!("Navigation rail on right"))); + right_btn.widget_info(|| { + egui::WidgetInfo::selected(egui::Role::Button, true, right, format!("{}: {}", tl!("Navigation rail side"), tl!("Right"))) + }); if right_btn.clicked() { app.side_panels.rail_side = crate::side_panels::ToolsSide::Right; } diff --git a/crates/ui-egui/tests/it/comments.rs b/crates/ui-egui/tests/it/comments.rs index 41f5040294..6805732422 100644 --- a/crates/ui-egui/tests/it/comments.rs +++ b/crates/ui-egui/tests/it/comments.rs @@ -3119,7 +3119,7 @@ fn freehand_highlighter_draws_translucent_yellow_ink() { #[test] fn hotbar_picker_adds_and_runs_freehand_highlight() { let mut h = harness(|_| {}); - h.get_by_label("+").click(); + h.get_by_label("Hotbar settings").click(); h.run_steps(3); field(&h, "Search all tools…").click(); h.run_steps(2); @@ -3142,6 +3142,23 @@ fn hotbar_picker_adds_and_runs_freehand_highlight() { assert_eq!(h.state().quick_tool, QuickTool::Comment(pdfcraft_ui_egui::comments::CommentTool::FreehandHighlight)); } +#[test] +fn hotbar_picker_searches_translated_tool_labels() { + let mut h = harness(|app| { + app.set_option("language", "fr").unwrap(); + }); + h.get_by_label("Hotbar settings").click(); + h.run_steps(3); + field(&h, "Search all tools…").click(); + h.run_steps(2); + field(&h, "Search all tools…").type_text("Dessiner"); + h.run_steps(3); + h.get_by_role_and_label(Role::CheckBox, "Dessiner à main levée").click(); + h.run_steps(3); + let saved: serde_json::Value = serde_json::from_str(&h.state().persist()).unwrap(); + assert_eq!(saved["hotbar"]["tools"], serde_json::json!(["comment.ink"])); +} + #[test] fn hotbar_drag_moves_and_persists_position() { let mut h = harness(|_| {}); @@ -3189,15 +3206,15 @@ fn hotbar_settings_reopen_even_with_remembered_collapsed_state() { egui::collapsing_header::CollapsingState::load_with_default_open(&h.ctx, egui::Id::unique("hotbar-picker").with("collapsing"), false); collapsed.set_open(false); collapsed.store(&h.ctx); - h.get_by_label("+").click(); + h.get_by_label("Hotbar settings").click(); h.run_steps(5); assert!(h.query_by_label("Close menu").is_some()); - h.get_by_label("+").click(); + h.get_by_label("Hotbar settings").click(); h.run_steps(3); - assert!(h.query_by_label("Close menu").is_some(), "+ always shows settings"); + assert!(h.query_by_label("Close menu").is_some(), "Hotbar settings always shows settings"); h.get_by_label("Close menu").click(); h.run_steps(3); - h.get_by_label("+").click(); + h.get_by_label("Hotbar settings").click(); h.run_steps(3); assert!(h.query_by_label("Close menu").is_some()); } @@ -3208,7 +3225,7 @@ fn hotbar_full_side_strip_spans_each_edge_and_returns_to_float() { app.set_option("hotbar-side", "right").unwrap(); app.set_option("hotbar-size", "48").unwrap(); }); - h.get_by_label("+").click(); + h.get_by_label("Hotbar settings").click(); h.run_steps(5); h.get_by_label("Full-side strip").click(); h.run_steps(3); @@ -3222,7 +3239,7 @@ fn hotbar_full_side_strip_spans_each_edge_and_returns_to_float() { let (length, thickness) = if matches!(side, "left" | "right") { (size.y, size.x) } else { (size.x, size.y) }; assert!(length > 300.0, "strip must fill the edge: {side} {size:?}"); assert!(thickness <= 64.0, "strip must stay narrow: {size:?}"); - h.get_by_label("+"); + h.get_by_label("Hotbar settings"); } let saved: serde_json::Value = serde_json::from_str(&h.state().persist()).unwrap(); assert_eq!(saved["hotbar"]["full_side"], true); diff --git a/crates/ui-egui/tests/it/ui.rs b/crates/ui-egui/tests/it/ui.rs index b5565a53bb..61ff002224 100644 --- a/crates/ui-egui/tests/it/ui.rs +++ b/crates/ui-egui/tests/it/ui.rs @@ -362,10 +362,10 @@ fn navigation_rail_toggles_between_right_and_left_and_persists() { assert_eq!(h2.state().side_panels.rail_side, pdfcraft_ui_egui::side_panels::ToolsSide::Left); h2.state_mut().dialog = Some(Dialog::Preferences); h2.run_steps(3); - h2.get_by_label("Navigation rail on right").click(); + h2.get_by_label("Navigation rail side: Right").click(); h2.run_steps(3); assert_eq!(h2.state().side_panels.rail_side, pdfcraft_ui_egui::side_panels::ToolsSide::Right); - h2.get_by_label("Navigation rail on left").click(); + h2.get_by_label("Navigation rail side: Left").click(); h2.run_steps(3); assert_eq!(h2.state().side_panels.rail_side, pdfcraft_ui_egui::side_panels::ToolsSide::Left); }