From 4ca5fccf9e1e22ff6fa47f19e4a3baff7e0d7f77 Mon Sep 17 00:00:00 2001 From: Ethunxxx Date: Mon, 9 Nov 2026 03:03:30 +0100 Subject: [PATCH] M4: Initial View magnification offers 25% and 100% (#5254) (#5265) --- crates/organize/src/tests.rs | 50 +++++++++++++++++++++++++++++++ crates/organize/src/view.rs | 12 +++++++- crates/ui-egui/src/dialogs.rs | 3 +- crates/ui-egui/tests/it/viewer.rs | 47 +++++++++++++++++++++++++++++ parity/acrobat-features.toml | 2 +- 5 files changed, 111 insertions(+), 3 deletions(-) diff --git a/crates/organize/src/tests.rs b/crates/organize/src/tests.rs index 593efd2600..cc2fbf51e3 100644 --- a/crates/organize/src/tests.rs +++ b/crates/organize/src/tests.rs @@ -2467,6 +2467,56 @@ fn an_unchanged_opening_destination_keeps_its_position_and_name() { } } +/// A percentage is written as the `/XYZ` zoom factor (ISO 32000-2 Table 149), not the percentage: +/// 25% as 0.25 and 100% as 1, and each survives a save. +#[test] +fn initial_view_percentages_are_written_as_zoom_factors() { + use crate::view::Magnification; + for (percent, factor) in [(25.0, 0.25), (100.0, 1.0), (150.0, 1.5)] { + let mut doc = open(fixture()); + let v = crate::initial_view(&doc); + crate::set_initial_view(&mut doc, &crate::InitialView { magnification: Magnification::Percent(percent), ..v }).unwrap(); + let doc = save_and_reopen(&doc); + let root = doc.root().unwrap(); + let open_action = doc.get(root).as_dict().and_then(|c| c.get(b"OpenAction").cloned()).map(|o| doc.resolve(&o).as_ref().clone()); + let zoom = open_action.as_ref().and_then(Object::as_array).and_then(|a| a.get(4)).and_then(Object::as_f64); + assert_eq!(zoom, Some(factor), "{percent}%: {open_action:?}"); + } +} + +#[test] +fn choosing_100_percent_keeps_an_actual_size_opening_destination() { + use crate::view::Magnification; + let page2 = Object::Ref(pdfcraft_cos::ObjRef::new(5, 0)); + let xyz = Object::Array(vec![page2.clone(), Object::name("XYZ"), Object::Int(100), Object::Int(500), Object::Int(1)]); + let mut named = pdfcraft_cos::Dict::new(); + named.set(b"S".to_vec(), Object::name("GoTo")); + named.set(b"D".to_vec(), Object::name("there")); + let mut dests = pdfcraft_cos::Dict::new(); + dests.set(b"there".to_vec(), Object::Array(vec![page2, Object::name("XYZ"), Object::Null, Object::Null, Object::Int(1)])); + for action in [xyz, Object::Dict(named)] { + let mut doc = open(fixture()); + let root = doc.root().unwrap(); + doc.update_dict(root, |c| { + c.set(b"OpenAction".to_vec(), action.clone()); + c.set(b"Dests".to_vec(), Object::Dict(dests.clone())); + }) + .unwrap(); + let v = crate::initial_view(&doc); + assert_eq!(v.magnification, Magnification::ActualSize); + // 100% is the same zoom: the action keeps its position or name. + crate::set_initial_view(&mut doc, &crate::InitialView { magnification: Magnification::Percent(100.0), ..v }).unwrap(); + let catalog = doc.get(root).as_dict().cloned().unwrap(); + assert_eq!(catalog.get(b"OpenAction"), Some(&action)); + } + // Without one, 100% writes zoom 1, which reads back as Actual Size. + let mut doc = open(fixture()); + let v = crate::initial_view(&doc); + crate::set_initial_view(&mut doc, &crate::InitialView { magnification: Magnification::Percent(100.0), ..v }).unwrap(); + let doc = save_and_reopen(&doc); + assert_eq!(crate::initial_view(&doc).magnification, Magnification::ActualSize); +} + /// A document whose `/OpenAction` is `action`, with the named destination `there` (page 3 at /// 200%) in the catalog `/Dests` and `here` (page 1, fit) in a `/Names /Dests` tree. fn doc_opening_at(action: Object) -> Document { diff --git a/crates/organize/src/view.rs b/crates/organize/src/view.rs index 899293b980..bb39dcdf9a 100644 --- a/crates/organize/src/view.rs +++ b/crates/organize/src/view.rs @@ -163,13 +163,23 @@ pub fn displays_doc_title(doc: &Document) -> bool { .unwrap_or(false) } +/// Whether `a` and `b` open at the same magnification. 100% writes the zoom Actual Size writes, +/// which [`initial_view`] reads back as Actual Size. +fn same_magnification(a: Magnification, b: Magnification) -> bool { + let actual = |m: Magnification| match m { + Magnification::Percent(p) if (p / 100.0 - 1.0).abs() < 1e-6 => Magnification::ActualSize, + m => m, + }; + actual(a) == actual(b) +} + /// Write the initial view. An opening action that isn't a plain destination (a script, say) is /// kept unless the page or magnification asks for a destination, and an opening action whose page /// and magnification `v` leaves as [`initial_view`] reads them is kept as it is. pub fn set_initial_view(doc: &mut Document, v: &InitialView) -> Result<(), OrganizeError> { let same_destination = { let now = initial_view(doc); - now.page == v.page && now.magnification == v.magnification + now.page == v.page && same_magnification(now.magnification, v.magnification) }; let (root, c) = catalog(doc)?; let pages = crate::walk(doc)?; diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 982878cb43..b88a567140 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -624,7 +624,8 @@ fn dialog_properties(ui: &mut egui::Ui, app: &mut PdfCraftApp, req: &mut Pending for (m, n) in names { ui.selectable_value(&mut v.magnification, m, n); } - for p in [50.0, 75.0, 125.0, 150.0, 200.0] { + // 100% is saved as Actual Size is (/XYZ zoom 1), so it reads back as Actual Size. + for p in [25.0, 50.0, 75.0, 100.0, 125.0, 150.0, 200.0] { ui.selectable_value(&mut v.magnification, M::Percent(p), format!("{p:.0}%")); } }) diff --git a/crates/ui-egui/tests/it/viewer.rs b/crates/ui-egui/tests/it/viewer.rs index 341e718c16..852c66657b 100644 --- a/crates/ui-egui/tests/it/viewer.rs +++ b/crates/ui-egui/tests/it/viewer.rs @@ -974,6 +974,53 @@ fn initial_view_is_edited_and_honoured_on_open() { assert_eq!((view.current, view.cover, app.right), (1, true, Some(pdfcraft_ui_egui::RightPanel::Bookmarks))); } +#[test] +fn initial_view_magnification_offers_25_and_100_percent() { + use egui::accesskit::Role; + use pdfcraft_engine::Magnification; + use pdfcraft_ui_egui::canvas::Fit; + fn open_initial_view(h: &mut Harness<'static, PdfCraftApp>) { + h.state_mut().dialog = Some(Dialog::Properties(pdfcraft_ui_egui::PropsTab::InitialView)); + // The dialog's height settles a frame after it opens; egui centers from the previous size. + h.run_steps(6); + } + fn pick(h: &mut Harness<'static, PdfCraftApp>, label: &str) { + h.get_by_role_and_label(Role::ComboBox, "Magnification").click(); + h.run_steps(3); + // The list is taller than the popup, so the item may need scrolling into view. + h.get_by_role_and_label(Role::Button, label).scroll_to_me(); + h.run_steps(3); + h.get_by_role_and_label(Role::Button, label).click(); + h.run_steps(2); + } + /// Clicks OK, then reopens the saved file: its initial magnification, and the zoom and fit + /// its view opens at. + fn save(h: &mut Harness<'static, PdfCraftApp>) -> (Magnification, f32, Fit) { + h.get_by_label("OK").click(); + h.run_steps(3); + let s = h.state(); + let id = s.views[s.active.unwrap()].id; + let bytes = s.session.save_bytes(id).unwrap(); + let mut app = PdfCraftApp::new(); + app.open_bytes("again.pdf", None, bytes.to_vec()).unwrap(); + let view = &app.views[0]; + (app.session.get(view.id).unwrap().initial_view().magnification, view.zoom, view.fit) + } + let mut h = harness(); + open_initial_view(&mut h); + for (label, percent) in [("100%", 100.0), ("25%", 25.0)] { + pick(&mut h, label); + let (_, v) = h.state().view_draft.as_ref().expect("seeded"); + assert_eq!(v.magnification, Magnification::Percent(percent), "{label}"); + } + assert_eq!(save(&mut h), (Magnification::Percent(25.0), 0.25, Fit::None)); + // 100% is saved as Actual Size is (/XYZ zoom 1), so it reads back as Actual Size. A file + // without an opening zoom also opens at zoom 1, but fitting the width. + open_initial_view(&mut h); + pick(&mut h, "100%"); + assert_eq!(save(&mut h), (Magnification::ActualSize, 1.0, Fit::None)); +} + fn drag(h: &mut Harness<'static, PdfCraftApp>, a: egui::Pos2, b: egui::Pos2) { h.hover_at(a); h.run_steps(1); diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 1556b7b72d..e3f5fb2d8d 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -3574,7 +3574,7 @@ milestone = "M4" status = "shipped" commands = ["file.properties"] tools = ["doc_initial_view"] -evidence = ["crates/organize/src/view.rs::initial_view_resolves_indirect_boolean_preferences", "crates/ui-egui/tests/it/ui.rs::a_document_asking_for_full_screen_is_asked_about_first", "crates/organize/src/tests.rs::initial_view_round_trips_and_keeps_scripts", "crates/ui-egui/tests/it/viewer.rs::initial_view_is_edited_and_honoured_on_open", "crates/automation/tests/it/automation.rs::initial_view_through_tools", "crates/render/src/inspect/navigation.rs::a_go_to_action_opens_at_its_destination_explicit_or_named", "crates/ui-egui/tests/it/open_action.rs::an_xyz_open_action_puts_its_point_at_the_window_top_left", "crates/ui-egui/tests/it/open_action.rs::a_fith_open_action_puts_its_top_at_the_window_top", "crates/ui-egui/tests/it/open_action.rs::a_named_open_destination_opens_its_page", "crates/ui-egui/tests/it/open_action.rs::a_full_screen_document_shows_its_non_full_screen_panel_outside_full_screen", "crates/organize/src/tests.rs::an_unchanged_opening_destination_keeps_its_position_and_name", "crates/automation/tests/it/automation.rs::doc_info_reports_the_open_destination", "crates/organize/src/tests.rs::choosing_page_one_replaces_a_named_opening_destination", "crates/organize/src/tests.rs::a_named_opening_destination_is_kept_while_it_still_holds"] +evidence = ["crates/organize/src/view.rs::initial_view_resolves_indirect_boolean_preferences", "crates/ui-egui/tests/it/ui.rs::a_document_asking_for_full_screen_is_asked_about_first", "crates/organize/src/tests.rs::initial_view_round_trips_and_keeps_scripts", "crates/ui-egui/tests/it/viewer.rs::initial_view_is_edited_and_honoured_on_open", "crates/ui-egui/tests/it/viewer.rs::initial_view_magnification_offers_25_and_100_percent", "crates/organize/src/tests.rs::choosing_100_percent_keeps_an_actual_size_opening_destination", "crates/automation/tests/it/automation.rs::initial_view_through_tools", "crates/render/src/inspect/navigation.rs::a_go_to_action_opens_at_its_destination_explicit_or_named", "crates/ui-egui/tests/it/open_action.rs::an_xyz_open_action_puts_its_point_at_the_window_top_left", "crates/ui-egui/tests/it/open_action.rs::a_fith_open_action_puts_its_top_at_the_window_top", "crates/ui-egui/tests/it/open_action.rs::a_named_open_destination_opens_its_page", "crates/ui-egui/tests/it/open_action.rs::a_full_screen_document_shows_its_non_full_screen_panel_outside_full_screen", "crates/organize/src/tests.rs::an_unchanged_opening_destination_keeps_its_position_and_name", "crates/automation/tests/it/automation.rs::doc_info_reports_the_open_destination", "crates/organize/src/tests.rs::choosing_page_one_replaces_a_named_opening_destination", "crates/organize/src/tests.rs::a_named_opening_destination_is_kept_while_it_still_holds"] notes = "navigation panel, layout, magnification, page, window and interface options; honoured on open; script open actions kept. Open in Full Screen mode (/PageMode /FullScreen) asks first (Full Screen / Not now), as Acrobat does, instead of taking the screen. Window and interface options also read indirect boolean ViewerPreferences values. Missing: Acrobat's Full Screen preference to turn that alert off and its \"remember my choice for this document\" (misc.prefs-full-screen). On open the /OpenAction destination places the view as ISO 32000-2 Table 149 says (the /XYZ point at the window's top-left, /FitH and /FitV top or left at the edge), and a named destination (name or string, /Names /Dests tree or catalog /Dests, ยง12.3.2.4) resolves; Always use my default zoom keeps only the page. A full-screen document shows its /NonFullScreenPageMode panel outside full screen (Table 147). doc_info reports the opening destination. Document Properties keeps an opening action whose page and magnification are left alone, but still edits only those two; it shows a named destination's page and magnification, and choosing another page or magnification replaces the name with an explicit destination, except that page 1 at the default magnification removes /OpenAction so the first page opens. A name that resolves nowhere reads as page 1 and is kept until another page is chosen" [[feature]]