From fdffff7b85895c1793ebddcd19cad163fd8ec229 Mon Sep 17 00:00:00 2001 From: Ethunxxx Date: Mon, 9 Nov 2026 03:04:00 +0100 Subject: [PATCH] M7: links that go to a page open at a chosen zoom (edit.action-goto) (#5277) --- crates/annot/src/links.rs | 131 +++++++++++++-- crates/annot/src/tests.rs | 202 ++++++++++++++++++++++- crates/automation/src/links.rs | 78 +++++++-- crates/automation/src/tools.rs | 8 +- crates/automation/tests/it/automation.rs | 74 +++++++++ crates/engine/src/lib.rs | 2 +- crates/engine/src/tests.rs | 27 ++- crates/ui-egui/src/tools/link_ui.rs | 129 +++++++++++++-- crates/ui-egui/tests/it/fill_sign.rs | 2 +- crates/ui-egui/tests/it/pdf_links.rs | 61 ++++++- parity/acrobat-features.toml | 6 +- 11 files changed, 666 insertions(+), 54 deletions(-) diff --git a/crates/annot/src/links.rs b/crates/annot/src/links.rs index 7cf4328c7c..e2199e4d23 100644 --- a/crates/annot/src/links.rs +++ b/crates/annot/src/links.rs @@ -8,13 +8,77 @@ use crate::{AnnotError, annots, page_ref, page_refs, set_annots}; /// Where a link goes. #[derive(Clone, Debug, PartialEq)] pub enum LinkAction { - /// A page of this document (0-based), shown fitting the window (`/Fit`). - Page(usize), + /// A page of this document (0-based), shown at `zoom`. + Page { + page: usize, + zoom: LinkZoom, + }, Uri(String), /// Something PdfCraft doesn't edit yet (named destination, JavaScript…): kept as is. Other(String), } +/// How a go-to-page link shows its page: Acrobat's Go to a page view zoom, written as an +/// explicit destination (ISO 32000-2 §12.3.2.2, Table 149). Positions are written `null`, so the +/// reader keeps its position in that direction, as a new bookmark does. +#[derive(Clone, Copy, Debug, PartialEq)] +pub enum LinkZoom { + /// `/Fit`: the whole page in the window. + FitPage, + /// `/FitH null`: the page's width across the window. + FitWidth, + /// `/FitV null`: the page's height down the window. + FitHeight, + /// `/FitB`: the page's content (its bounding box) in the window. + FitVisible, + /// `/XYZ null null null`: the reader's zoom is kept. + Inherit, + /// `/XYZ null null z` with z = percent / 100; 100 is Actual Size. A new zoom is written + /// within 1–6400%; a link rewritten with the zoom it has keeps its own factor. + Percent(f64), +} + +impl LinkZoom { + /// The zoom an explicit destination array (`[page /XYZ …]`, `[page /Fit]`, …) asks for. The + /// boxed variants read as their unboxed ones, and a `/FitR` rectangle or a missing or unknown + /// type as Fit Page, the view links had before they carried a zoom. A link whose page and zoom + /// are unchanged is not rewritten, so its position is kept; one given a new page keeps how its + /// zoom is written (`write_action`) but not its position. + fn of_dest(doc: &Document, dest: &[Object]) -> Self { + let kind = dest.get(1).map(|k| doc.resolve(k)).and_then(|k| k.as_name().map(<[u8]>::to_vec)); + match kind.as_deref() { + Some(b"FitB") => LinkZoom::FitVisible, + Some(b"FitH" | b"FitBH") => LinkZoom::FitWidth, + Some(b"FitV" | b"FitBV") => LinkZoom::FitHeight, + Some(b"XYZ") => match dest.get(4).and_then(|z| doc.resolve(z).as_f64()) { + // Finite even for a factor near f64::MAX. A link given a new page with this zoom + // keeps the factor itself, however far outside 1–6400% (`write_action`). + Some(z) if z.is_finite() && z > 0.0 => LinkZoom::Percent((z * 100.0).min(f64::MAX)), + // A zoom of 0 or null keeps the reader's (Table 149). + _ => LinkZoom::Inherit, + }, + _ => LinkZoom::FitPage, + } + } + + /// The destination's elements after the page reference. + fn operands(self) -> Result, AnnotError> { + Ok(match self { + LinkZoom::FitPage => vec![Object::name("Fit")], + LinkZoom::FitWidth => vec![Object::name("FitH"), Object::Null], + LinkZoom::FitHeight => vec![Object::name("FitV"), Object::Null], + LinkZoom::FitVisible => vec![Object::name("FitB")], + LinkZoom::Inherit => vec![Object::name("XYZ"), Object::Null, Object::Null, Object::Null], + LinkZoom::Percent(p) => { + if !(p.is_finite() && p > 0.0) { + return Err(AnnotError::Invalid("the link's zoom must be a positive percentage".into())); + } + vec![Object::name("XYZ"), Object::Null, Object::Null, Object::Real((p / 100.0).clamp(0.01, 64.0))] + } + }) + } +} + /// Acrobat's Link Properties ▸ Appearance. #[derive(Clone, Copy, Debug, PartialEq)] pub struct LinkStyle { @@ -87,17 +151,18 @@ fn nums(doc: &Document, o: Option<&Object>) -> Vec { } fn action_of(doc: &Document, d: &Dict, pages: &[ObjRef], uri_budget: &mut usize) -> LinkAction { - let dest_page = |dest: &Object| -> Option { + let page_view = |dest: &Object| -> Option { let a = doc.resolve(dest); - let first = a.as_array()?.first()?.clone(); - match first { - Object::Ref(r) => pages.iter().position(|p| *p == r), - Object::Int(i) => usize::try_from(i).ok(), + let a = a.as_array()?; + let page = match a.first()? { + Object::Ref(r) => pages.iter().position(|p| p == r), + Object::Int(i) => usize::try_from(*i).ok(), _ => None, - } + }?; + Some(LinkAction::Page { page, zoom: LinkZoom::of_dest(doc, a) }) }; if let Some(dest) = d.get(b"Dest") { - return dest_page(dest).map_or_else(|| LinkAction::Other("named destination".into()), LinkAction::Page); + return page_view(dest).unwrap_or_else(|| LinkAction::Other("named destination".into())); } let Some(a) = d.get(b"A").map(|a| doc.resolve(a)).and_then(|a| a.as_dict().cloned()) else { return LinkAction::Other("no action".into()) }; match a.name(b"S") { @@ -115,7 +180,7 @@ fn action_of(doc: &Document, d: &Dict, pages: &[ObjRef], uri_budget: &mut usize) None => LinkAction::Other("web address too long".into()), } } - Some(b"GoTo") => a.get(b"D").and_then(dest_page).map_or_else(|| LinkAction::Other("named destination".into()), LinkAction::Page), + Some(b"GoTo") => a.get(b"D").and_then(page_view).unwrap_or_else(|| LinkAction::Other("named destination".into())), Some(s) => LinkAction::Other(String::from_utf8_lossy(s).into_owned()), None => LinkAction::Other("no action".into()), } @@ -214,12 +279,52 @@ fn uri_bytes(u: &str) -> Vec { out } +/// The explicit destination array link `d` goes to now: its `/Dest`, or a GoTo action's `/D`. +fn current_dest(doc: &Document, d: &Dict) -> Option> { + let dest = match d.get(b"Dest") { + Some(dest) => doc.resolve(dest), + None => { + let action = doc.resolve(d.get(b"A")?); + let go_to = action.as_dict().filter(|a| a.name(b"S") == Some(b"GoTo"))?; + doc.resolve(go_to.get(b"D")?) + } + }; + dest.as_array().cloned() +} + fn write_action(doc: &Document, d: &mut Dict, a: &LinkAction) -> Result<(), AnnotError> { match a { - LinkAction::Page(p) => { - let target = page_ref(doc, *p)?; + LinkAction::Page { page, zoom } => { + let target = page_ref(doc, *page)?; + let mut operands = zoom.operands()?; + let current = current_dest(doc, d).filter(|c| LinkZoom::of_dest(doc, c) == *zoom); + if let Some(c) = current { + // The page and zoom the link has (`link_edit` naming its own page): the + // destination is left as it is, position on the page and GoTo form included. + let same_page = match c.first() { + Some(Object::Ref(r)) => *r == target, + Some(Object::Int(i)) => usize::try_from(*i).ok() == Some(*page), + _ => false, + }; + if same_page { + return Ok(()); + } + // A new page with the zoom the link has keeps how that zoom is written: a boxed + // fit (`/FitBH`, `/FitBV`) and its own `/XYZ` factor, even one outside the 1–6400% + // a new zoom is written within. Positions belong to the old page and are dropped. + let (slot, own) = match zoom { + LinkZoom::Percent(_) => (operands.get_mut(3), c.get(4)), + LinkZoom::FitWidth | LinkZoom::FitHeight => (operands.get_mut(0), c.get(1)), + _ => (None, None), + }; + if let (Some(slot), Some(own)) = (slot, own) { + *slot = doc.resolve(own).as_ref().clone(); + } + } + let mut dest = vec![Object::Ref(target)]; + dest.extend(operands); d.remove(b"A"); - d.set(b"Dest".to_vec(), Object::Array(vec![Object::Ref(target), Object::name("Fit")])); + d.set(b"Dest".to_vec(), Object::Array(dest)); } LinkAction::Uri(u) => { let u = u.trim(); diff --git a/crates/annot/src/tests.rs b/crates/annot/src/tests.rs index 7561938676..bfb780dabb 100644 --- a/crates/annot/src/tests.rs +++ b/crates/annot/src/tests.rs @@ -1003,12 +1003,12 @@ fn stamps_draw_their_label_and_by_line() { #[test] fn links_are_added_edited_listed_and_removed() { - use crate::links::{self, Highlight, LinkAction, LinkStyle}; + use crate::links::{self, Highlight, LinkAction, LinkStyle, LinkZoom}; let mut doc = fixture(); let before = links::list(&doc).len(); let i = links::add(&mut doc, 0, [10.0, 10.0, 110.0, 30.0], &LinkAction::Uri("https://example.org".into()), &LinkStyle::default()).unwrap(); let style = LinkStyle { visible: true, color: [1.0, 0.0, 0.0], width: 2.0, dashed: true, underline: false, highlight: Highlight::Outline }; - links::add(&mut doc, 1, [10.0, 10.0, 60.0, 30.0], &LinkAction::Page(0), &style).unwrap(); + links::add(&mut doc, 1, [10.0, 10.0, 60.0, 30.0], &LinkAction::Page { page: 0, zoom: LinkZoom::FitPage }, &style).unwrap(); let doc = reopen(&doc); let all = links::list(&doc); assert_eq!(all.len(), before + 2); @@ -1017,7 +1017,7 @@ fn links_are_added_edited_listed_and_removed() { (web.action.clone(), web.style.visible, web.style.highlight), (LinkAction::Uri("https://example.org".into()), false, Highlight::Invert) ); - let page = all.iter().find(|l| l.page == 1 && l.action == LinkAction::Page(0)).unwrap().clone(); + let page = all.iter().find(|l| l.page == 1 && l.action == LinkAction::Page { page: 0, zoom: LinkZoom::FitPage }).unwrap().clone(); assert_eq!(page.style, style); let mut doc = doc; links::set(&mut doc, 1, page.index, Some([0.0, 0.0, 50.0, 20.0]), Some(&LinkAction::Uri("mailto:ada@example.org".into())), None).unwrap(); @@ -1027,7 +1027,201 @@ fn links_are_added_edited_listed_and_removed() { assert!(links::delete(&mut doc, 0, 999).is_err(), "no such annotation"); let n = links::remove_all(&mut doc, None).unwrap(); assert!(n >= 1 && links::list(&doc).is_empty()); - assert!(links::add(&mut doc, 0, [0.0, 0.0, 1.0, 1.0], &LinkAction::Page(0), &LinkStyle::default()).is_err(), "too small"); + assert!( + links::add(&mut doc, 0, [0.0, 0.0, 1.0, 1.0], &LinkAction::Page { page: 0, zoom: LinkZoom::FitPage }, &LinkStyle::default()).is_err(), + "too small" + ); +} + +#[test] +fn page_links_keep_their_zoom() { + use crate::links::{self, LinkAction, LinkStyle, LinkZoom}; + let page1 = page_refs(&fixture()).unwrap()[1]; + let dest_of = |doc: &Document, obj: ObjRef| doc.get(obj).as_dict().and_then(|d| d.get(b"Dest").cloned()); + // Each zoom is written as its destination type (Table 149) and reads back after a save. + let null = Object::Null; + for (zoom, written) in [ + (LinkZoom::FitPage, vec![Object::name("Fit")]), + (LinkZoom::FitWidth, vec![Object::name("FitH"), null.clone()]), + (LinkZoom::FitHeight, vec![Object::name("FitV"), null.clone()]), + (LinkZoom::FitVisible, vec![Object::name("FitB")]), + (LinkZoom::Inherit, vec![Object::name("XYZ"), null.clone(), null.clone(), null.clone()]), + (LinkZoom::Percent(100.0), vec![Object::name("XYZ"), null.clone(), null.clone(), Object::Real(1.0)]), + (LinkZoom::Percent(250.0), vec![Object::name("XYZ"), null.clone(), null.clone(), Object::Real(2.5)]), + ] { + let mut doc = fixture(); + let i = links::add(&mut doc, 0, [10.0, 10.0, 60.0, 30.0], &LinkAction::Page { page: 1, zoom }, &LinkStyle::default()).unwrap(); + let link = links::list(&doc).into_iter().find(|l| l.page == 0 && l.index == i).unwrap(); + let mut expected = vec![Object::Ref(page1)]; + expected.extend(written); + assert_eq!(dest_of(&doc, link.obj.unwrap()), Some(Object::Array(expected)), "{zoom:?}"); + let doc = reopen(&doc); + let link = links::list(&doc).into_iter().find(|l| l.page == 0 && l.index == i).unwrap(); + assert_eq!(link.action, LinkAction::Page { page: 1, zoom }, "{zoom:?} after a save"); + } + // What other writers use reads as the nearest zoom: a zero zoom keeps the reader's, the boxed + // fits as the plain ones, a rectangle as Fit Page, and a GoTo action like a /Dest. + let mut doc = fixture(); + let i = links::add(&mut doc, 0, [10.0, 10.0, 60.0, 30.0], &LinkAction::Page { page: 1, zoom: LinkZoom::FitPage }, &LinkStyle::default()).unwrap(); + let obj = links::list(&doc).into_iter().find(|l| l.page == 0 && l.index == i).unwrap().obj.unwrap(); + let dest = |kind: &str, operands: Vec| { + let mut d = vec![Object::Ref(page1), Object::name(kind)]; + d.extend(operands); + Object::Array(d) + }; + for (key, value, zoom) in [ + ("Dest", dest("XYZ", vec![Object::Int(100), Object::Int(700), Object::Int(0)]), LinkZoom::Inherit), + ("Dest", dest("FitBH", vec![Object::Int(500)]), LinkZoom::FitWidth), + ("Dest", dest("FitBV", vec![Object::Int(20)]), LinkZoom::FitHeight), + ("Dest", dest("FitR", vec![Object::Int(0), Object::Int(0), Object::Int(10), Object::Int(10)]), LinkZoom::FitPage), + ("A", dest("XYZ", vec![null.clone(), null.clone(), Object::Real(1.5)]), LinkZoom::Percent(150.0)), + // A factor outside what PdfCraft writes reads as the destination has it, not capped, so + // it agrees with the viewer's `destination`; one whose percentage overflows stays finite. + ("Dest", dest("XYZ", vec![null.clone(), null.clone(), Object::Real(100.0)]), LinkZoom::Percent(10000.0)), + ("Dest", dest("XYZ", vec![null.clone(), null.clone(), Object::Real(0.005)]), LinkZoom::Percent(0.5)), + ("Dest", dest("XYZ", vec![null.clone(), null.clone(), Object::Real(1e307)]), LinkZoom::Percent(f64::MAX)), + ] { + let mut d = doc.get(obj).as_dict().cloned().unwrap(); + d.remove(b"Dest"); + d.remove(b"A"); + if key == "A" { + let mut go_to = pdfcraft_cos::Dict::new(); + go_to.set(b"S".to_vec(), Object::name("GoTo")); + go_to.set(b"D".to_vec(), value); + d.set(b"A".to_vec(), Object::Dict(go_to)); + } else { + d.set(b"Dest".to_vec(), value); + } + doc.set(obj, Object::Dict(d)); + let link = links::list(&doc).into_iter().find(|l| l.page == 0 && l.index == i).unwrap(); + assert_eq!(link.action, LinkAction::Page { page: 1, zoom }, "{key}"); + } + // A zoom that isn't a positive percentage is refused before anything is written. + for p in [0.0, -50.0, f64::NAN, f64::INFINITY] { + let before = links::list(&doc).len(); + let refused = + links::add(&mut doc, 0, [10.0, 10.0, 60.0, 30.0], &LinkAction::Page { page: 1, zoom: LinkZoom::Percent(p) }, &LinkStyle::default()); + assert!(refused.is_err(), "{p}"); + assert_eq!(links::list(&doc).len(), before, "{p}: no link added"); + } +} + +/// A link rewritten with the zoom it has keeps its own `/XYZ` factor, however far outside the +/// 1–6400% a new zoom is written within: giving it a new page (Link Properties' OK, `link_edit` +/// without `zoom`) must not change its zoom. A zoom chosen anew is still written within 1–6400%. +#[test] +fn a_rewritten_link_keeps_its_own_zoom_factor() { + use crate::links::{self, LinkAction, LinkStyle, LinkZoom}; + let pages = page_refs(&fixture()).unwrap(); + let dest_of = |doc: &Document, obj: ObjRef| doc.get(obj).as_dict().and_then(|d| d.get(b"Dest").cloned()); + let xyz = |page: ObjRef, factor: Object| Object::Array(vec![Object::Ref(page), Object::name("XYZ"), Object::Null, Object::Null, factor]); + for (key, factor) in [ + ("Dest", Object::Real(0.005)), + ("Dest", Object::Real(100.0)), + ("Dest", Object::Int(100)), + ("Dest", Object::Real(1e307)), + ("A", Object::Real(0.005)), + ] { + let mut doc = fixture(); + let i = + links::add(&mut doc, 0, [10.0, 10.0, 60.0, 30.0], &LinkAction::Page { page: 1, zoom: LinkZoom::FitPage }, &LinkStyle::default()).unwrap(); + let obj = links::list(&doc).into_iter().find(|l| l.page == 0 && l.index == i).unwrap().obj.unwrap(); + let mut d = doc.get(obj).as_dict().cloned().unwrap(); + d.remove(b"Dest"); + if key == "A" { + let mut go_to = pdfcraft_cos::Dict::new(); + go_to.set(b"S".to_vec(), Object::name("GoTo")); + go_to.set(b"D".to_vec(), xyz(pages[1], factor.clone())); + d.set(b"A".to_vec(), Object::Dict(go_to)); + } else { + d.set(b"Dest".to_vec(), xyz(pages[1], factor.clone())); + } + doc.set(obj, Object::Dict(d)); + let LinkAction::Page { zoom, .. } = links::list(&doc).into_iter().find(|l| l.page == 0 && l.index == i).unwrap().action else { + panic!("{key} {factor:?}") + }; + links::set(&mut doc, 0, i, None, Some(&LinkAction::Page { page: 0, zoom }), None).unwrap(); + assert_eq!(dest_of(&doc, obj), Some(xyz(pages[0], factor.clone())), "{key} {factor:?}: the factor is the link's own"); + let doc = reopen(&doc); + assert_eq!( + links::list(&doc).into_iter().find(|l| l.page == 0 && l.index == i).unwrap().action, + LinkAction::Page { page: 0, zoom }, + "{key} {factor:?}" + ); + } + // A zoom other than the link's is written within 1–6400%. + let mut doc = fixture(); + let i = links::add(&mut doc, 0, [10.0, 10.0, 60.0, 30.0], &LinkAction::Page { page: 1, zoom: LinkZoom::Percent(50.0) }, &LinkStyle::default()) + .unwrap(); + let obj = links::list(&doc).into_iter().find(|l| l.page == 0 && l.index == i).unwrap().obj.unwrap(); + links::set(&mut doc, 0, i, None, Some(&LinkAction::Page { page: 1, zoom: LinkZoom::Percent(10000.0) }), None).unwrap(); + assert_eq!(dest_of(&doc, obj), Some(xyz(pages[1], Object::Real(64.0)))); +} + +/// A go-to edited without a change (its own page and zoom, as `link_edit` naming its page does) +/// is left as it is: the `/XYZ` position, a fit's position and the GoTo form stay. Given a new +/// page with its zoom, a link keeps how that zoom is written, a boxed fit (`/FitBH`, `/FitBV`) +/// included, but not its position, which belongs to the old page. +#[test] +fn an_unchanged_go_to_keeps_its_destination_and_a_new_page_keeps_its_fit() { + use crate::links::{self, LinkAction, LinkStyle, LinkZoom}; + let pages = page_refs(&fixture()).unwrap(); + let dest = |page: ObjRef, kind: &str, operands: Vec| { + let mut d = vec![Object::Ref(page), Object::name(kind)]; + d.extend(operands); + Object::Array(d) + }; + let link_with = |key: &str, value: Object| { + let mut doc = fixture(); + let i = + links::add(&mut doc, 0, [10.0, 10.0, 60.0, 30.0], &LinkAction::Page { page: 1, zoom: LinkZoom::FitPage }, &LinkStyle::default()).unwrap(); + let obj = links::list(&doc).into_iter().find(|l| l.page == 0 && l.index == i).unwrap().obj.unwrap(); + let mut d = doc.get(obj).as_dict().cloned().unwrap(); + d.remove(b"Dest"); + if key == "A" { + let mut go_to = pdfcraft_cos::Dict::new(); + go_to.set(b"S".to_vec(), Object::name("GoTo")); + go_to.set(b"D".to_vec(), value); + d.set(b"A".to_vec(), Object::Dict(go_to)); + } else { + d.set(b"Dest".to_vec(), value); + } + doc.set(obj, Object::Dict(d)); + (doc, i, obj) + }; + let (num, null) = (|n: i64| Object::Int(n), Object::Null); + // The same page and zoom: nothing is rewritten. + for (key, value) in [ + ("Dest", dest(pages[1], "XYZ", vec![num(72), num(500), Object::Real(2.0)])), + ("Dest", dest(pages[1], "FitBH", vec![num(250)])), + ("Dest", dest(pages[1], "FitV", vec![num(20)])), + ("A", dest(pages[1], "XYZ", vec![num(72), num(500), Object::Real(0.005)])), + ] { + let (mut doc, i, obj) = link_with(key, value); + let before = doc.get(obj).as_dict().cloned(); + let LinkAction::Page { page, zoom } = links::list(&doc).into_iter().find(|l| l.page == 0 && l.index == i).unwrap().action else { + panic!("{key}") + }; + links::set(&mut doc, 0, i, None, Some(&LinkAction::Page { page, zoom }), None).unwrap(); + assert_eq!(doc.get(obj).as_dict().cloned(), before, "{key} {zoom:?}: left as it is"); + } + // A new page: the fit's kind and the factor stay, positions go. + for (value, written) in [ + (dest(pages[1], "FitBH", vec![num(250)]), dest(pages[0], "FitBH", vec![null.clone()])), + (dest(pages[1], "FitBV", vec![num(20)]), dest(pages[0], "FitBV", vec![null.clone()])), + (dest(pages[1], "FitH", vec![num(250)]), dest(pages[0], "FitH", vec![null.clone()])), + ( + dest(pages[1], "XYZ", vec![num(72), num(500), Object::Real(2.0)]), + dest(pages[0], "XYZ", vec![null.clone(), null.clone(), Object::Real(2.0)]), + ), + ] { + let (mut doc, i, obj) = link_with("Dest", value.clone()); + let LinkAction::Page { zoom, .. } = links::list(&doc).into_iter().find(|l| l.page == 0 && l.index == i).unwrap().action else { + panic!("{value:?}") + }; + links::set(&mut doc, 0, i, None, Some(&LinkAction::Page { page: 0, zoom }), None).unwrap(); + assert_eq!(doc.get(obj).as_dict().and_then(|d| d.get(b"Dest").cloned()), Some(written), "{value:?}"); + } } #[test] diff --git a/crates/automation/src/links.rs b/crates/automation/src/links.rs index 41fda9b7db..a3bf9cd28b 100644 --- a/crates/automation/src/links.rs +++ b/crates/automation/src/links.rs @@ -2,7 +2,7 @@ //! all links. Rectangles are points from the top-left of the displayed page. use pdfcraft_engine::{ - Edit, LinkAction, LinkHighlight, LinkStyle, read_remote_document, remote_document_path_is_absolute, resolve_remote_document_path, + Edit, LinkAction, LinkHighlight, LinkStyle, LinkZoom, read_remote_document, remote_document_path_is_absolute, resolve_remote_document_path, }; use pdfcraft_render::{DestView, LinkTarget, RemoteDestination}; use serde_json::{Value, json}; @@ -25,18 +25,30 @@ impl Automation { Ok(Some([u0[0].min(u1[0]) as f64, u0[1].min(u1[1]) as f64, u0[0].max(u1[0]) as f64, u0[1].max(u1[1]) as f64])) } - fn link_action(&self, a: &Args) -> Result> { + /// The action `url`, `to_page` and `zoom` ask for, if any. `current` is the link's action now + /// (link_edit): a new page keeps the link's zoom, and a zoom alone keeps its page. + fn link_action(&self, a: &Args, current: Option<&LinkAction>) -> Result> { + let zoom = link_zoom(a)?; + let (current_page, current_zoom) = match current { + Some(LinkAction::Page { page, zoom }) => (Some(*page), Some(*zoom)), + _ => (None, None), + }; match (a.opt_str("url")?, a.opt_int("to_page")?) { (Some(_), Some(_)) => Err(bad("pass url or to_page, not both")), + (Some(_), None) if zoom.is_some() => Err(bad("zoom goes with to_page, not url")), (Some(u), None) => Ok(Some(LinkAction::Uri(u.to_string()))), (None, Some(p)) => { let n = self.doc(a)?.info.pages.len() as i64; if p < 1 || p > n { return Err(bad(format!("to_page must be 1–{n}"))); } - Ok(Some(LinkAction::Page(p as usize - 1))) + Ok(Some(LinkAction::Page { page: p as usize - 1, zoom: zoom.or(current_zoom).unwrap_or(LinkZoom::FitPage) })) } - (None, None) => Ok(None), + (None, None) => match (zoom, current_page) { + (None, _) => Ok(None), + (Some(zoom), Some(page)) => Ok(Some(LinkAction::Page { page, zoom })), + (Some(_), None) => Err(bad("zoom needs to_page: the link doesn't go to a page")), + }, } } @@ -86,11 +98,16 @@ impl Automation { if let Some(d) = doc.info.pages.get(page).and_then(|p| crate::destination(view, p)) { v["destination"] = d; } - if matches!(action, LinkAction::Other(_)) { - v["named"] = json!(true); + match action { + LinkAction::Page { zoom, .. } => v["zoom"] = zoom_value(*zoom), + LinkAction::Other(_) => v["named"] = json!(true), + LinkAction::Uri(_) => {} } } - (LinkAction::Page(p), None) => v["to_page"] = json!(p + 1), + (LinkAction::Page { page, zoom }, None) => { + v["to_page"] = json!(page + 1); + v["zoom"] = zoom_value(*zoom); + } (LinkAction::Other(o), None) => v["other"] = json!(o), } v @@ -168,12 +185,13 @@ impl Automation { pub(crate) fn link_add(&mut self, a: &Args) -> Result { let page = self.page(a)?; let rect = self.view_rect(a, page)?.ok_or_else(|| bad("link_add needs rect"))?; - let action = self.link_action(a)?.ok_or_else(|| bad("pass url or to_page"))?; + let action = self.link_action(a, None)?.ok_or_else(|| bad("pass url or to_page"))?; let style = self.link_style(a, LinkStyle::default())?; self.apply(a, Edit::AddLink { page, rect, action, style }) } - fn link_index(&self, a: &Args) -> Result<(usize, usize, LinkStyle)> { + /// The link `page` and `index` name: its page, index in `/Annots`, appearance and action. + fn link_index(&self, a: &Args) -> Result<(usize, usize, LinkStyle, LinkAction)> { let page = self.page(a)?; let index = a.int("index")?; let doc = self.doc(a)?; @@ -182,19 +200,19 @@ impl Automation { .iter() .find(|l| l.page == page && l.index as i64 + 1 == index) .ok_or_else(|| failed(format!("page {} has no link {index} (see link_list)", page + 1)))?; - Ok((page, l.index, l.style)) + Ok((page, l.index, l.style, l.action.clone())) } pub(crate) fn link_edit(&mut self, a: &Args) -> Result { - let (page, index, style) = self.link_index(a)?; + let (page, index, style, current) = self.link_index(a)?; let rect = self.view_rect(a, page)?; - let action = self.link_action(a)?; + let action = self.link_action(a, Some(¤t))?; let new_style = self.link_style(a, style)?; self.apply(a, Edit::SetLink { page, index, rect, action, style: (new_style != style).then_some(new_style) }) } pub(crate) fn link_delete(&mut self, a: &Args) -> Result { - let (page, index, _) = self.link_index(a)?; + let (page, index, ..) = self.link_index(a)?; self.apply(a, Edit::DeleteLink { page, index }) } @@ -225,6 +243,40 @@ impl Automation { } } +/// The zoom a `zoom` argument names: fit_page, fit_width, fit_height, fit_visible, actual +/// (100%), inherit (keep the reader's zoom), or a percentage, kept within 1–6400. +fn link_zoom(a: &Args) -> Result> { + match a.get("zoom") { + None | Some(Value::Null) => Ok(None), + Some(Value::Number(p)) => match p.as_f64() { + Some(p) if p.is_finite() && p > 0.0 => Ok(Some(LinkZoom::Percent(p.clamp(1.0, 6400.0)))), + _ => Err(bad("zoom must be a positive percentage")), + }, + Some(Value::String(z)) => Ok(Some(match z.as_str() { + "fit_page" => LinkZoom::FitPage, + "fit_width" => LinkZoom::FitWidth, + "fit_height" => LinkZoom::FitHeight, + "fit_visible" => LinkZoom::FitVisible, + "actual" => LinkZoom::Percent(100.0), + "inherit" => LinkZoom::Inherit, + o => return Err(bad(format!("unknown zoom {o:?}"))), + })), + Some(_) => Err(bad("zoom is a name or a percentage")), + } +} + +/// How `link_list` reports a link's zoom: the names `link_zoom` reads, a percentage as a number. +fn zoom_value(zoom: LinkZoom) -> Value { + match zoom { + LinkZoom::FitPage => json!("fit_page"), + LinkZoom::FitWidth => json!("fit_width"), + LinkZoom::FitHeight => json!("fit_height"), + LinkZoom::FitVisible => json!("fit_visible"), + LinkZoom::Inherit => json!("inherit"), + LinkZoom::Percent(p) => json!(p), + } +} + fn dest_view_value(view: DestView) -> Value { match view { DestView::Top => json!({ "kind": "top" }), diff --git a/crates/automation/src/tools.rs b/crates/automation/src/tools.rs index 227672df06..5d20fe1dae 100644 --- a/crates/automation/src/tools.rs +++ b/crates/automation/src/tools.rs @@ -812,7 +812,7 @@ pub fn tools() -> Vec { t("content_delete", "Delete added content", "Delete an added item (page, index from content_list). Undoable.") .destructive() .with(schema(json!({ "doc": doc(), "page": { "type": "integer", "minimum": 1 }, "index": { "type": "integer", "minimum": 1 } }), &["doc", "page", "index"])), - t("link_list", "List links", "Every link: page, 1-based index (for link_edit/link_delete), rect (points from the top-left of the page), and where it goes (url or to_page).") + t("link_list", "List links", "Every link: page, 1-based index (for link_edit/link_delete), rect (points from the top-left of the page), and where it goes (url, or to_page with its zoom).") .ro() .with(schema(json!({ "doc": doc() }), &["doc"])), t("link_open", "Open a linked PDF", "Follow a local GoToR link in PdfCraft: open the referenced PDF as a new document, respecting --root. page and index use link_list numbering. Named destinations are reported but not applied. Absolute file specifications require allow_absolute: true after reviewing the target; relative links need no opt-in.") @@ -820,22 +820,24 @@ pub fn tools() -> Vec { json!({ "doc": doc(), "page": { "type": "integer", "minimum": 1 }, "index": { "type": "integer", "minimum": 1 }, "password": { "type": "string", "description": "Optional password for the linked PDF." }, "allow_absolute": { "type": "boolean", "description": "Explicitly allow an absolute local GoToR target (default false). Existing validation and --root confinement still apply." } }), &["doc", "page", "index"], )), - t("link_add", "Add a link", "Add a link over rect on page that opens url or goes to to_page. Appearance: visible (rectangle), color, width 1-3, highlight none/invert/outline/inset. Undoable.") + t("link_add", "Add a link", "Add a link over rect on page that opens url or goes to to_page, shown at zoom: fit_page (default), fit_width, fit_height, fit_visible, actual, inherit (keep the reader's zoom) or a percentage. Appearance: visible (rectangle), color, width 1-3, highlight none/invert/outline/inset. Undoable.") .cmd("edit.link") .with(schema( json!({ "doc": doc(), "page": { "type": "integer", "minimum": 1 }, "rect": { "type": "array", "items": { "type": "number" }, "minItems": 4, "maxItems": 4 }, "url": { "type": "string" }, "to_page": { "type": "integer", "minimum": 1 }, + "zoom": { "description": "fit_page, fit_width, fit_height, fit_visible, actual, inherit, or a percentage" }, "visible": { "type": "boolean" }, "color": { "type": "string" }, "width": { "type": "number" }, "highlight": { "type": "string", "enum": ["none", "invert", "outline", "inset"] }, }), &["doc", "page", "rect"], )), - t("link_edit", "Edit a link", "Change a link's rect, destination (url or to_page) or appearance. Undoable.") + t("link_edit", "Edit a link", "Change a link's rect, destination (url, or to_page and zoom as in link_add; a new to_page keeps the link's zoom, a zoom alone keeps its page) or appearance. Undoable.") .with(schema( json!({ "doc": doc(), "page": { "type": "integer", "minimum": 1 }, "index": { "type": "integer", "minimum": 1 }, "rect": { "type": "array", "items": { "type": "number" }, "minItems": 4, "maxItems": 4 }, "url": { "type": "string" }, "to_page": { "type": "integer", "minimum": 1 }, + "zoom": { "description": "fit_page, fit_width, fit_height, fit_visible, actual, inherit, or a percentage" }, "visible": { "type": "boolean" }, "color": { "type": "string" }, "width": { "type": "number" }, "highlight": { "type": "string", "enum": ["none", "invert", "outline", "inset"] }, }), &["doc", "page", "index"], diff --git a/crates/automation/tests/it/automation.rs b/crates/automation/tests/it/automation.rs index 6e05d752b7..b837b51316 100644 --- a/crates/automation/tests/it/automation.rs +++ b/crates/automation/tests/it/automation.rs @@ -6201,6 +6201,80 @@ fn links_through_tools() { assert!(matches!(a.call("link_add", &json!({ "doc": doc, "page": 1, "rect": [0, 0, 50, 20] })), Err(ToolError::InvalidArgs(_)))); } +#[test] +fn link_zoom_through_tools() { + let dir = workdir("link-zoom"); + std::fs::write(dir.join("notes.txt"), "Contents").unwrap(); + let mut a = auto(&dir); + let doc = ok(&mut a, "doc_create", json!({ "from": "text", "path": "notes.txt" }))["doc"].as_u64().unwrap(); + let link = |a: &mut Automation| ok(a, "link_list", json!({ "doc": doc }))["links"][0].clone(); + // A page link opens at Fit Page unless told otherwise. + ok(&mut a, "link_add", json!({ "doc": doc, "page": 1, "rect": [72, 300, 200, 320], "to_page": 1 })); + assert_eq!((link(&mut a)["to_page"].clone(), link(&mut a)["zoom"].clone()), (json!(1), json!("fit_page"))); + let index = link(&mut a)["index"].clone(); + // A zoom alone keeps the page; a page alone keeps the zoom. + ok(&mut a, "link_edit", json!({ "doc": doc, "page": 1, "index": index, "zoom": 200 })); + let l = link(&mut a); + assert_eq!((l["to_page"].clone(), l["zoom"].clone()), (json!(1), json!(200.0))); + assert_eq!(l["destination"]["zoom"], 2.0, "the viewer reads the zoom the link writes: {l}"); + ok(&mut a, "link_edit", json!({ "doc": doc, "page": 1, "index": index, "zoom": "fit_width" })); + ok(&mut a, "link_edit", json!({ "doc": doc, "page": 1, "index": index, "to_page": 1 })); + assert_eq!(link(&mut a)["zoom"], "fit_width"); + for (name, zoom) in [("actual", json!(100.0)), ("inherit", json!("inherit")), ("fit_visible", json!("fit_visible"))] { + ok(&mut a, "link_edit", json!({ "doc": doc, "page": 1, "index": index, "zoom": name })); + assert_eq!(link(&mut a)["zoom"], zoom, "{name}"); + } + // A zoom needs a page to show: refused with a web address, unknown, or not positive. + for args in [ + json!({ "doc": doc, "page": 1, "rect": [72, 400, 200, 420], "url": "https://example.org", "zoom": "fit_page" }), + json!({ "doc": doc, "page": 1, "rect": [72, 400, 200, 420], "zoom": "fit_page" }), + json!({ "doc": doc, "page": 1, "rect": [72, 400, 200, 420], "to_page": 1, "zoom": "sideways" }), + json!({ "doc": doc, "page": 1, "rect": [72, 400, 200, 420], "to_page": 1, "zoom": 0 }), + json!({ "doc": doc, "page": 1, "rect": [72, 400, 200, 420], "to_page": 1, "zoom": [2] }), + ] { + assert!(matches!(a.call("link_add", &args), Err(ToolError::InvalidArgs(_))), "{args}"); + } + ok(&mut a, "link_edit", json!({ "doc": doc, "page": 1, "index": index, "url": "https://example.org" })); + assert!(matches!(a.call("link_edit", &json!({ "doc": doc, "page": 1, "index": index, "zoom": 150 })), Err(ToolError::InvalidArgs(_)))); +} + +/// `link_edit` with a page and no zoom keeps the link's zoom, and with it the link's own `/XYZ` +/// factor, even outside the 1–6400% a chosen zoom is written within: 0.5% and 10000% stay so, in +/// link_list and in the viewer's destination. Naming the link's own page changes nothing, so its +/// position stays too. A zoom chosen anew is still kept within 1–6400%. +#[test] +fn link_edit_keeps_a_links_own_zoom_outside_the_range_it_writes() { + let (_base, root) = sandbox("link-own-zoom"); + let pdf = pdf_of(&[ + "<< /Type /Catalog /Pages 2 0 R >>", + "<< /Type /Pages /Kids [3 0 R 4 0 R] /Count 2 /MediaBox [0 0 300 400] >>", + "<< /Type /Page /Parent 2 0 R /Annots [5 0 R 6 0 R 7 0 R] >>", + "<< /Type /Page /Parent 2 0 R >>", + "<< /Type /Annot /Subtype /Link /Rect [10 10 100 30] /Border [0 0 0] /Dest [3 0 R /XYZ null null 0.005] >>", + "<< /Type /Annot /Subtype /Link /Rect [10 50 100 70] /Border [0 0 0] /Dest [3 0 R /XYZ null null 100] >>", + "<< /Type /Annot /Subtype /Link /Rect [10 90 100 110] /Border [0 0 0] /Dest [3 0 R /XYZ 72 300 2] >>", + ]); + std::fs::write(root.join("zooms.pdf"), pdf).unwrap(); + let mut a = auto(&root); + let doc = ok(&mut a, "doc_open", json!({ "path": "zooms.pdf" }))["doc"].as_u64().unwrap(); + let link = |a: &mut Automation, index: u64| { + ok(a, "link_list", json!({ "doc": doc }))["links"].as_array().unwrap().iter().find(|l| l["index"] == index).unwrap().clone() + }; + for (index, percent, factor) in [(1, 0.5, 0.005), (2, 10000.0, 100.0)] { + ok(&mut a, "link_edit", json!({ "doc": doc, "page": 1, "index": index, "to_page": 2 })); + let l = link(&mut a, index); + assert_eq!((l["to_page"].clone(), l["zoom"].clone()), (json!(2), json!(percent)), "{l}"); + let shown = l["destination"]["zoom"].as_f64().unwrap(); + assert!((shown - factor).abs() <= factor * 1e-6, "the viewer's zoom is the link's own: {l}"); + } + ok(&mut a, "link_edit", json!({ "doc": doc, "page": 1, "index": 3, "to_page": 1 })); + let l = link(&mut a, 3); + // `/XYZ 72 300` on a 400 pt page: 72 from the left and 100 from the top, as the tools measure. + assert_eq!((&l["zoom"], &l["destination"]["left"], &l["destination"]["top"]), (&json!(200.0), &json!(72.0), &json!(100.0)), "{l}"); + ok(&mut a, "link_edit", json!({ "doc": doc, "page": 1, "index": 1, "zoom": 10000 })); + assert_eq!(link(&mut a, 1)["zoom"], json!(6400.0)); +} + /// #4168: a Launch action whose file is a PDF (here with Windows `\` separators, relative to the /// source as a remote link is) opens that PDF in PdfCraft through link_open, within the root; one /// naming a program is listed but never followed. diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index ce0bb5c784..9be9666590 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -117,7 +117,7 @@ pub use pdfcraft_fonts::{MAX_SIGNATURE_CHARS, ScriptOutline, script_missing, scr /// Comment geometry helpers (text-box line breaking) for frontends. pub use pdfcraft_annot::appearance as annot_text; -pub use pdfcraft_annot::links::{Highlight as LinkHighlight, LinkAction, LinkItem, LinkStyle}; +pub use pdfcraft_annot::links::{Highlight as LinkHighlight, LinkAction, LinkItem, LinkStyle, LinkZoom}; pub use pdfcraft_annot::{ Activation3d, AppearanceCache, AttachIcon, BorderShape, FillMark, LineEnding, Markup, NOTE_SIZE, NewAnnotation, NoteIcon, OverlayFont, OverlayLook, Props as CommentProps, ReviewState, Rgb, Shape, StampGroup, StampKind, Style, rect_quad, diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 15cffd9325..1bf1b847c3 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -4887,6 +4887,25 @@ fn split_by_size_and_bookmarks_and_page_filters() { assert_eq!(filter_pages(info, &all, PageParity::Odd, PageOrientation::Portrait), [0, 2, 4]); } +/// A zoomed link follows its target page, not its number: deleting a page before the target keeps +/// the link on that page at its zoom. Deleting the target page drops the link's destination, as +/// for any link to a deleted page, and undo brings it back with its zoom. +#[test] +fn a_zoomed_link_follows_its_page_and_loses_it_when_the_page_is_deleted() { + let (mut s, id) = session_with(3); + let zoom = LinkZoom::Percent(200.0); + let rect = [10.0, 10.0, 60.0, 30.0]; + s.apply(id, Edit::AddLink { page: 0, rect, action: LinkAction::Page { page: 2, zoom }, style: LinkStyle::default() }).unwrap(); + let action = |s: &Session| s.get(id).unwrap().links()[0].action.clone(); + s.apply(id, Edit::DeletePages { pages: vec![1] }).unwrap(); + assert_eq!(action(&s), LinkAction::Page { page: 1, zoom }, "the link follows its page"); + s.apply(id, Edit::DeletePages { pages: vec![1] }).unwrap(); + // The destination is removed, not left pointing at a page that is gone. + assert_eq!(action(&s), LinkAction::Other("no action".into()), "no destination left"); + s.undo(id).unwrap(); + assert_eq!(action(&s), LinkAction::Page { page: 1, zoom }, "undo restores the destination and its zoom"); +} + #[test] fn links_from_urls_and_link_edits() { // One page whose text contains a web address. @@ -4928,8 +4947,12 @@ fn links_from_urls_and_link_edits() { assert_eq!(s.get(id).unwrap().links().len(), 1); // Link Properties and delete. let l = s.get(id).unwrap().links()[0].clone(); - s.apply(id, Edit::SetLink { page: 0, index: l.index, rect: None, action: Some(LinkAction::Page(0)), style: None }).unwrap(); - assert_eq!(s.get(id).unwrap().links()[0].action, LinkAction::Page(0)); + s.apply( + id, + Edit::SetLink { page: 0, index: l.index, rect: None, action: Some(LinkAction::Page { page: 0, zoom: LinkZoom::FitPage }), style: None }, + ) + .unwrap(); + assert_eq!(s.get(id).unwrap().links()[0].action, LinkAction::Page { page: 0, zoom: LinkZoom::FitPage }); s.apply(id, Edit::DeleteLink { page: 0, index: l.index }).unwrap(); assert!(s.get(id).unwrap().links().is_empty()); assert!(s.apply(id, Edit::RemoveLinks { pages: None }).is_err(), "nothing left to remove"); diff --git a/crates/ui-egui/src/tools/link_ui.rs b/crates/ui-egui/src/tools/link_ui.rs index f2e880983d..6249288c4d 100644 --- a/crates/ui-egui/src/tools/link_ui.rs +++ b/crates/ui-egui/src/tools/link_ui.rs @@ -3,7 +3,7 @@ //! their outlines, a click selects one, a double-click edits it and Delete removes it. use egui::{Color32, CornerRadius, Pos2, Rect, Stroke}; -use pdfcraft_engine::{Edit, LinkAction, LinkHighlight, LinkItem, LinkStyle}; +use pdfcraft_engine::{Edit, LinkAction, LinkHighlight, LinkItem, LinkStyle, LinkZoom}; use pdfcraft_render::{DocInfo, LinkTarget}; use crate::canvas::{DocView, PageToolContext, PageXform}; @@ -46,9 +46,52 @@ pub struct LinkDraft { pub url: String, /// 1-based target page. pub target: usize, + /// How the target page is shown. + pub zoom: LinkZoom, + /// The link's action when the dialog opened (`None` for a new link). OK rewrites the action + /// only when it changed, so an appearance edit keeps a destination's position and kind + /// (`/XYZ left top`, `/FitR`…) that the dialog doesn't show. + pub current: Option, pub style: LinkStyle, } +/// The zooms Link Properties offers for a page view. +const ZOOMS: [LinkZoom; 12] = [ + LinkZoom::FitPage, + LinkZoom::FitWidth, + LinkZoom::FitHeight, + LinkZoom::FitVisible, + LinkZoom::Percent(100.0), + LinkZoom::Inherit, + LinkZoom::Percent(50.0), + LinkZoom::Percent(75.0), + LinkZoom::Percent(125.0), + LinkZoom::Percent(150.0), + LinkZoom::Percent(200.0), + LinkZoom::Percent(400.0), +]; + +fn zoom_label(zoom: LinkZoom) -> String { + match zoom { + LinkZoom::FitPage => tl!("Fit Page").to_string(), + LinkZoom::FitWidth => tl!("Fit Width").to_string(), + LinkZoom::FitHeight => tl!("Fit Height").to_string(), + LinkZoom::FitVisible => tl!("Fit Visible").to_string(), + LinkZoom::Inherit => tl!("Inherit Zoom").to_string(), + LinkZoom::Percent(p) if (p - 100.0).abs() < 1e-9 => tl!("Actual Size").to_string(), + // Another writer's factor can be anything up to f64::MAX, or a tiny fraction: exponent + // form keeps it short. + LinkZoom::Percent(p) if !(0.01..1e6).contains(&p) => format!("{p:.1e}%"), + // A fractional one (another writer's 0.5% or 62.5%) shows up to two places, so it agrees + // with what link_list reports instead of rounding to 0%. + LinkZoom::Percent(p) if p.fract() != 0.0 => { + let places = format!("{p:.2}"); + format!("{}%", places.trim_end_matches('0').trim_end_matches('.')) + } + LinkZoom::Percent(p) => format!("{p:.0}%"), + } +} + fn screen_rect(xf: &PageXform, info: &DocInfo, page: usize, r: [f64; 4]) -> Rect { xf.user_rect(info, page, [r[0] as f32, r[1] as f32, r[2] as f32, r[3] as f32]) } @@ -202,9 +245,14 @@ impl PdfCraftApp { _ => "https://".into(), }, target: match l.action { - LinkAction::Page(p) => p + 1, + LinkAction::Page { page, .. } => page + 1, _ => 1, }, + zoom: match l.action { + LinkAction::Page { zoom, .. } => zoom, + _ => LinkZoom::FitPage, + }, + current: Some(l.action.clone()), style: l.style, }, None => LinkDraft { @@ -215,6 +263,8 @@ impl PdfCraftApp { web: true, url: "https://".into(), target: 1, + zoom: LinkZoom::FitPage, + current: None, style: LinkStyle::default(), }, }; @@ -339,6 +389,19 @@ pub(crate) fn body(ui: &mut egui::Ui, d: &mut LinkDraft, pages: usize, t: &Token ui.add(egui::DragValue::new(&mut d.target).range(1..=pages.max(1))); ui.label(crate::i18n::fmt(tl!("of {name}"), &[("name", &pages.to_string())])); }); + ui.horizontal(|ui| { + ui.add_space(24.0); + let label = ui.label(tl!("Zoom")); + craft_ui::ComboBox::from_id_salt("link-zoom") + .selected_text(zoom_label(d.zoom)) + .show_ui(ui, |ui| { + for z in ZOOMS { + ui.selectable_value(&mut d.zoom, z, zoom_label(z)); + } + }) + .response + .labelled_by(label.id); + }); }); } let ok = d.preserve_action || !d.web || (d.url.trim().len() > 3 && d.url.trim() != "https://"); @@ -358,21 +421,20 @@ pub(crate) fn body(ui: &mut egui::Ui, d: &mut LinkDraft, pages: usize, t: &Token (a, c) } +/// The action a confirmed draft sets. +fn action_for(d: &LinkDraft) -> LinkAction { + if d.web { LinkAction::Uri(d.url.trim().to_string()) } else { LinkAction::Page { page: d.target.saturating_sub(1), zoom: d.zoom } } +} + /// The edit for a confirmed draft. pub(crate) fn edit_for(d: &LinkDraft) -> Edit { match d.index { - None => Edit::AddLink { - page: d.page, - rect: d.rect, - action: if d.web { LinkAction::Uri(d.url.trim().to_string()) } else { LinkAction::Page(d.target.saturating_sub(1)) }, - style: d.style, - }, + None => Edit::AddLink { page: d.page, rect: d.rect, action: action_for(d), style: d.style }, Some(index) => Edit::SetLink { page: d.page, index, rect: None, - action: (!d.preserve_action) - .then(|| if d.web { LinkAction::Uri(d.url.trim().to_string()) } else { LinkAction::Page(d.target.saturating_sub(1)) }), + action: (!d.preserve_action).then(|| action_for(d)).filter(|a| d.current.as_ref() != Some(a)), style: Some(d.style), }, } @@ -380,13 +442,54 @@ pub(crate) fn edit_for(d: &LinkDraft) -> Edit { #[cfg(test)] mod tests { - use super::{LinkDraft, edit_for}; - use pdfcraft_engine::{Edit, LinkStyle}; + use super::{LinkDraft, edit_for, zoom_label}; + use pdfcraft_engine::{Edit, LinkAction, LinkStyle, LinkZoom}; + + fn draft(current: LinkAction) -> LinkDraft { + LinkDraft { + page: 0, + rect: [0.0; 4], + index: Some(3), + preserve_action: matches!(current, LinkAction::Other(_)), + web: false, + url: String::new(), + target: 1, + zoom: LinkZoom::FitPage, + current: Some(current), + style: LinkStyle::default(), + } + } #[test] fn appearance_edit_preserves_an_existing_unsupported_action() { let style = LinkStyle::default(); - let draft = LinkDraft { page: 0, rect: [0.0; 4], index: Some(3), preserve_action: true, web: false, url: String::new(), target: 1, style }; + let draft = draft(LinkAction::Other("named destination".into())); assert_eq!(edit_for(&draft), Edit::SetLink { page: 0, index: 3, rect: None, action: None, style: Some(style) }); } + + #[test] + fn only_a_changed_destination_is_rewritten() { + // A page view the dialog shows as it is: OK leaves the destination (and its position) alone. + let mut d = draft(LinkAction::Page { page: 0, zoom: LinkZoom::Percent(200.0) }); + d.zoom = LinkZoom::Percent(200.0); + assert!(matches!(edit_for(&d), Edit::SetLink { action: None, .. })); + d.zoom = LinkZoom::FitWidth; + assert!(matches!(edit_for(&d), Edit::SetLink { action: Some(LinkAction::Page { page: 0, zoom: LinkZoom::FitWidth }), .. })); + d.zoom = LinkZoom::Percent(200.0); + d.target = 2; + assert!(matches!(edit_for(&d), Edit::SetLink { action: Some(LinkAction::Page { page: 1, .. }), .. })); + } + + #[test] + fn a_zoom_label_stays_short_for_any_factor() { + assert_eq!(zoom_label(LinkZoom::Percent(200.0)), "200%"); + // The destination's own zoom, past what PdfCraft writes, is shown as it is. + assert_eq!(zoom_label(LinkZoom::Percent(10000.0)), "10000%"); + assert_eq!(zoom_label(LinkZoom::Percent(f64::MAX)), "1.8e308%"); + // A fractional zoom keeps its places instead of rounding to 0%. + assert_eq!(zoom_label(LinkZoom::Percent(0.5)), "0.5%"); + assert_eq!(zoom_label(LinkZoom::Percent(62.5)), "62.5%"); + assert_eq!(zoom_label(LinkZoom::Percent(33.333)), "33.33%"); + assert_eq!(zoom_label(LinkZoom::Percent(0.001)), "1.0e-3%"); + } } diff --git a/crates/ui-egui/tests/it/fill_sign.rs b/crates/ui-egui/tests/it/fill_sign.rs index fe65314fa4..2807f1e2f0 100644 --- a/crates/ui-egui/tests/it/fill_sign.rs +++ b/crates/ui-egui/tests/it/fill_sign.rs @@ -1055,7 +1055,7 @@ fn image_signature_preview_follows_pointer_at_page_size_with_zoom_and_rotation() h.state_mut().apply_edit(pdfcraft_engine::Edit::AddLink { page: 0, rect: [30.0, 230.0, 60.0, 270.0], - action: pdfcraft_engine::LinkAction::Page(0), + action: pdfcraft_engine::LinkAction::Page { page: 0, zoom: pdfcraft_engine::LinkZoom::FitPage }, style: pdfcraft_engine::LinkStyle::default(), }); h.state_mut().signature = Some(SavedSig::Image(image)); diff --git a/crates/ui-egui/tests/it/pdf_links.rs b/crates/ui-egui/tests/it/pdf_links.rs index 7ed2903bc5..40be0d0fc2 100644 --- a/crates/ui-egui/tests/it/pdf_links.rs +++ b/crates/ui-egui/tests/it/pdf_links.rs @@ -77,13 +77,72 @@ fn drawing_editing_and_deleting_links() { } h.get_by_label("OK").click(); h.run_steps(3); - assert_eq!(links(&h)[0].action, LinkAction::Page(0)); + assert_eq!(links(&h)[0].action, LinkAction::Page { page: 0, zoom: pdfcraft_engine::LinkZoom::FitPage }); // Delete the selected link. h.key_press(egui::Key::Delete); h.run_steps(3); assert!(links(&h).is_empty()); } +#[test] +fn link_properties_sets_the_page_view_zoom() { + use egui::accesskit::Role; + use pdfcraft_engine::LinkZoom; + let mut h = harness(); + assert!(h.state_mut().execute("edit.link")); + let (a, b) = (at(&h, 40.0, 300.0), at(&h, 160.0, 320.0)); + drag(&mut h, a, b); + h.get_by_label("Create Link"); + h.get_by_label("Go to a page view").click(); + h.run_steps(3); + // The toolbar has a zoom box of its own; the link's shows Fit Page until changed. + h.get_by(|n| n.role() == Role::ComboBox && n.value().as_deref() == Some("Fit Page")).click(); + h.run_steps(3); + h.get_by_role_and_label(Role::Button, "200%").scroll_to_me(); + h.run_steps(3); + h.get_by_role_and_label(Role::Button, "200%").click(); + h.run_steps(2); + h.get_by_label("OK").click(); + h.run_steps(3); + assert_eq!(links(&h)[0].action, LinkAction::Page { page: 0, zoom: LinkZoom::Percent(200.0) }); +} + +/// One page with a link whose destination has a position, which Link Properties doesn't show. +const POSITIONED: &[u8] = b"%PDF-1.7 +1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj +2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj +3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 300 400] /Annots [4 0 R] >> endobj +4 0 obj << /Type /Annot /Subtype /Link /Rect [40 40 160 60] /Border [0 0 0] /Dest [3 0 R /XYZ 100 300 2] >> endobj +trailer << /Root 1 0 R >> +%%EOF"; + +#[test] +fn an_appearance_edit_keeps_the_destination_as_written() { + use pdfcraft_render::{DestView, LinkTarget}; + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(|_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("positioned.pdf", None, POSITIONED.to_vec()).unwrap(); + app + }); + h.run_steps(6); + let positioned = LinkTarget::Page(0, DestView::Xyz { left: Some(100.0), top: Some(300.0), zoom: Some(2.0) }); + let target = |h: &Harness<'static, PdfCraftApp>| { + let s = h.state(); + s.session.get(s.views[0].id).unwrap().info.links[0].target.clone() + }; + assert_eq!(target(&h), positioned); + h.state_mut().views[0].links.open_existing = Some((0, 0)); + h.run_steps(3); + assert_eq!(h.state().dialog, Some(Dialog::LinkProps)); + h.state_mut().link_draft.as_mut().unwrap().style.color = [1.0, 0.0, 0.0]; + h.run_steps(3); + h.get_by_label("OK").click(); + h.run_steps(3); + assert_eq!(links(&h)[0].style.color, [1.0, 0.0, 0.0], "the edit applied"); + assert_eq!(target(&h), positioned, "the destination is not rewritten without its position"); +} + #[test] fn removing_all_links_and_undo() { let mut h = harness(); diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index e3f5fb2d8d..e531fc0f8b 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -2333,9 +2333,9 @@ tier = "P1" milestone = "M7" status = "partial" commands = ["edit.link"] -tools = ["link_add"] -evidence = ["crates/annot/src/tests.rs::links_are_added_edited_listed_and_removed", "crates/ui-egui/tests/it/pdf_links.rs::drawing_editing_and_deleting_links"] -notes = "go to a page (Fit view); named destinations and zoom pending" +tools = ["link_add", "link_edit", "link_list"] +evidence = ["crates/annot/src/tests.rs::links_are_added_edited_listed_and_removed", "crates/ui-egui/tests/it/pdf_links.rs::drawing_editing_and_deleting_links", "crates/annot/src/tests.rs::page_links_keep_their_zoom", "crates/automation/tests/it/automation.rs::link_zoom_through_tools", "crates/ui-egui/tests/it/pdf_links.rs::link_properties_sets_the_page_view_zoom", "crates/ui-egui/tests/it/pdf_links.rs::an_appearance_edit_keeps_the_destination_as_written", "crates/annot/src/tests.rs::a_rewritten_link_keeps_its_own_zoom_factor", "crates/annot/src/tests.rs::an_unchanged_go_to_keeps_its_destination_and_a_new_page_keeps_its_fit", "crates/automation/tests/it/automation.rs::link_edit_keeps_a_links_own_zoom_outside_the_range_it_writes"] +notes = "go to a page at a zoom: Fit Page, Fit Width, Fit Height, Fit Visible, Actual Size, Inherit Zoom or a percentage (Link Properties ▸ Zoom; link_add and link_edit zoom), written as /Fit, /FitH, /FitV, /FitB or /XYZ with the position null (ISO 32000-2 Table 149). Links other tools wrote read as the nearest zoom. One given a new page with its zoom unchanged keeps how that zoom is written (a /FitBH or /FitBV fit, its own /XYZ factor even outside the 1–6400% a chosen zoom is written within) but not its position; one edited with its own page and zoom (Link Properties, link_edit) is left as it is, position included. Missing: named destinations as targets, and a position on the page (Acrobat's Set Link takes the current view)" [[feature]] id = "edit.action-uri"