From fefdccc8903ec4c96b63443dd02ac1d93633bea8 Mon Sep 17 00:00:00 2001 From: DjPeepjay <120863013+DjPeepjay@users.noreply.github.com> Date: Sun, 18 Oct 2026 17:27:05 -0500 Subject: [PATCH] fix(viewer): preserve gap toggles in organize and comparison views M3 / PR #3101: avoid stale organizer anchors and synchronize the active comparison gap choice across both panes through menu and control paths. Three regression tests fail before these fixes and pass after them. Document the gap helpers and test contracts. --- ROADMAP.md | 2 + crates/ui-egui/src/canvas.rs | 3 +- crates/ui-egui/src/commands.rs | 5 +-- crates/ui-egui/src/compare_ui.rs | 11 +++++ crates/ui-egui/src/lib.rs | 4 +- crates/ui-egui/tests/view.rs | 75 ++++++++++++++++++++++++++++++++ docs/page-display.md | 5 ++- docs/roadmap.md | 4 +- parity/acrobat-features.toml | 4 +- 9 files changed, 102 insertions(+), 11 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 0f9d5757fc..70ca973506 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -171,6 +171,8 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md ## Progress log +- **2026-10-18 (M3, PR #3101 review):** Gap changes in Organize Pages preserve document scroll position; menu and control changes apply to both comparison panes without changing unrelated tabs. Three synthetic UI regressions were observed failing before the fixes. Readiness estimates unchanged. + - **2026-10-18 (M3, PR #3101):** Optional gapless continuous page display with persisted defaults and UI control support; both new labels translated in all 22 catalogs. Rebased onto current main, preserving navigation and rendering fixes. Readiness estimates unchanged. - **2026-10-18 (M6, #2892):** Push-button normal labels are editable per widget through Options and form_set_props, independently of field names; rendering, undo/redo and incremental/full save/reopen are covered. Estimates unchanged. diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 3580064a59..14bb654bc4 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -1399,7 +1399,7 @@ impl DocView { if self.show_page_gaps == show { return; } - if self.viewport_screen.is_positive() { + if !self.organize && self.viewport_screen.is_positive() { let at = self.viewport_screen.center(); if let Some((page, r)) = self .screen_rects @@ -1414,6 +1414,7 @@ impl DocView { self.show_page_gaps = show; } + /// Separation between adjacent page rectangles, in logical screen points. fn page_gap(&self) -> f32 { if self.show_page_gaps { GAP } else { 0.0 } } diff --git a/crates/ui-egui/src/commands.rs b/crates/ui-egui/src/commands.rs index 15b9e96770..2dca1bb3d7 100644 --- a/crates/ui-egui/src/commands.rs +++ b/crates/ui-egui/src/commands.rs @@ -252,9 +252,8 @@ impl PdfCraftApp { } } "view.page_gaps" => { - if let Some(i) = active { - let v = &mut self.views[i]; - v.set_page_gaps(!v.show_page_gaps); + if let Some(view) = active.and_then(|i| self.views.get(i)) { + self.set_active_page_gaps(!view.show_page_gaps); } } "view.layout.cover" => { diff --git a/crates/ui-egui/src/compare_ui.rs b/crates/ui-egui/src/compare_ui.rs index 353059e879..51ca0eff57 100644 --- a/crates/ui-egui/src/compare_ui.rs +++ b/crates/ui-egui/src/compare_ui.rs @@ -163,6 +163,17 @@ pub(crate) fn panel(ui: &mut egui::Ui, t: &Tokens, state: &Option, } impl PdfCraftApp { + /// Set gaps for the active document and both visible comparison panes, leaving other tabs alone. + pub(crate) fn set_active_page_gaps(&mut self, show: bool) { + let Some((index, id)) = self.active_ids() else { return }; + let partner = side_by_side(self, index).and_then(|(old, _)| self.views.get(old).map(|v| v.id)); + for view in &mut self.views { + if view.id == id || partner == Some(view.id) { + view.set_page_gaps(show); + } + } + } + /// Compare the chosen older document with the active one and show the differences. pub fn run_compare(&mut self) { let Some((i, new)) = self.active_ids() else { return }; diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 7ee50e83ff..d44a786901 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -2599,13 +2599,13 @@ impl PdfCraftApp { "off" => v.set_continuous(false), _ => return Err("continuous must be on or off".into()), }, - ("page-gaps", Some(v)) => { + ("page-gaps", Some(_)) => { let show = match value { "on" => true, "off" => false, _ => return Err("page-gaps must be on or off".into()), }; - v.set_page_gaps(show); + self.set_active_page_gaps(show); } ("default-page-gaps", _) => { self.view_defaults.show_page_gaps = match value { diff --git a/crates/ui-egui/tests/view.rs b/crates/ui-egui/tests/view.rs index ff6a4cbeb2..7c7d19896a 100644 --- a/crates/ui-egui/tests/view.rs +++ b/crates/ui-egui/tests/view.rs @@ -146,6 +146,7 @@ fn continuous_layout_stacks_pages_vertically() { assert!((a.center().x - b.center().x).abs() < 1.0, "same column"); } +/// The toggle joins page edges while preserving zoom and leaving the PDF unmodified. #[test] fn page_gaps_toggle_joins_edges_without_changing_the_document() { let mut h = harness(&[("layout", "continuous"), ("zoom", "50")]); @@ -166,6 +167,77 @@ fn page_gaps_toggle_joins_edges_without_changing_the_document() { assert!(((rect(&h, 1).unwrap().top() - rect(&h, 0).unwrap().bottom()) - original).abs() < 0.01); } +/// Changing gaps in the organizer must not anchor document pages to the grid viewport. +#[test] +fn page_gaps_in_organizer_do_not_move_the_document_scroll_position() { + let mut h = harness(&[("layout", "continuous"), ("zoom", "100"), ("page", "2")]); + let before = rect(&h, 1).unwrap(); + let gap = before.top() - rect(&h, 0).unwrap().bottom(); + assert!(gap > 1.0); + assert!(h.state_mut().execute("page.organize")); + h.run_steps(4); + assert!(h.state().views[0].organize); + assert!(h.state_mut().execute("view.page_gaps")); + h.run_steps(2); + assert!(!h.state().views[0].show_page_gaps); + assert!(h.state_mut().execute("page.organize")); + h.run_steps(4); + let after = rect(&h, 1).unwrap(); + // Keeping the scroll offset moves page 2 up by exactly the removed gap before it. + assert!((after.top() - before.top() + gap).abs() < 1.0, "stale grid anchoring moved the page: {before:?} -> {after:?}, gap={gap}"); + assert_eq!(h.state().views[0].current, 1); +} + +/// A three-page comparison, with the old pane leading while the new document stays active. +fn page_gaps_comparison_harness() -> Harness<'static, PdfCraftApp> { + 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.set_option("left", "closed").unwrap(); + app.open_bytes("old.pdf", None, PAGES.to_vec()).unwrap(); + app.open_bytes("new.pdf", None, PAGES.to_vec()).unwrap(); + app.set_option("zoom", "50").unwrap(); + app.compare_old = Some(app.views[0].id); + app.run_compare(); + app + }); + h.run_steps(4); + let at = h.state().views[0].viewport_rect().center(); + h.event(egui::Event::PointerMoved(at)); + h.run_steps(2); + assert_eq!(h.state().compare.as_ref().unwrap().leader, 0); + assert_eq!(h.state().active, Some(1)); + h +} + +/// A page-display menu choice must reach both panes before the old leader synchronizes them. +#[test] +fn page_gaps_comparison_menu_preserves_the_active_panes_choice() { + use egui_kittest::kittest::Queryable; + let mut h = page_gaps_comparison_harness(); + h.get_by_label_contains("Page display").click(); + h.run_steps(2); + h.get_by_label("Show gaps between pages").click(); + h.run_steps(4); + assert!(h.state().views.iter().all(|v| !v.show_page_gaps), "the old leader must not restore gaps after the active pane's menu action"); +} + +/// The control setting has the same synchronized behavior and leaves unrelated views alone. +#[test] +fn page_gaps_comparison_control_preserves_both_states() { + let mut h = page_gaps_comparison_harness(); + for (value, show) in [("off", false), ("on", true)] { + h.state_mut().set_option("page-gaps", value).unwrap(); + h.run_steps(4); + assert!(h.state().views.iter().all(|v| v.show_page_gaps == show), "comparison rejected page-gaps={value}"); + } + h.state_mut().open_bytes("unrelated.pdf", None, PAGES.to_vec()).unwrap(); + h.state_mut().set_option("page-gaps", "off").unwrap(); + assert!(h.state().views[0].show_page_gaps && h.state().views[1].show_page_gaps); + assert!(!h.state().views[2].show_page_gaps); +} + +/// The saved default survives restart and applies to newly opened documents. #[test] fn page_gaps_default_survives_restart_and_applies_to_new_documents() { let mut app = PdfCraftApp::new(); @@ -181,6 +253,7 @@ fn page_gaps_default_survives_restart_and_applies_to_new_documents() { assert!(h.state_mut().set_option("page-gaps", "invalid").is_err()); } +/// The real menu action removes gutters, shadows and outlines from the seam pixels. #[test] fn page_gaps_menu_joins_pages_without_a_shadow_at_the_seam() { use egui_kittest::kittest::Queryable; @@ -200,6 +273,7 @@ fn page_gaps_menu_joins_pages_without_a_shadow_at_the_seam() { } } +/// Gapless geometry supports mixed sizes and two-page rows and survives document reload. #[test] fn page_gaps_work_for_mixed_sizes_and_two_page_rows() { let h = mixed_harness(&[("layout", "continuous"), ("zoom", "50"), ("page-gaps", "off")]); @@ -215,6 +289,7 @@ fn page_gaps_work_for_mixed_sizes_and_two_page_rows() { assert!(!h.state().views[0].show_page_gaps, "a file reload preserves the view setting"); } +/// Page edges stay adjacent at integer and fractional physical-pixel scales. #[test] fn page_gaps_stay_closed_at_fractional_device_scales() { for ppp in [1.0, 1.25, 1.5, 2.0] { diff --git a/docs/page-display.md b/docs/page-display.md index 8decd6834c..fa2c7f1aae 100644 --- a/docs/page-display.md +++ b/docs/page-display.md @@ -1,6 +1,6 @@ # Continuous page display -> **Last updated:** 2026-10-17 · Viewer: optional page gaps. +> **Last updated:** 2026-10-18 · Viewer: optional page gaps. To read pages as a continuous strip, open **View → Page display**, choose **Single page**, turn **Continuous scrolling** on, and turn **Show gaps between pages** off. The Page display @@ -27,3 +27,6 @@ and `set key=default-page-gaps value=off` for future documents. Its `command` me `id=view.page_gaps`, and `state` reports `active.page_gaps` and `active.continuous`. Revision history: 2026-10-17 — added local gapless-display controls and documentation. + +In a side-by-side comparison, the gap setting applies to both panes. Changing it in +Organize Pages keeps your document scroll position when you return to the viewer. diff --git a/docs/roadmap.md b/docs/roadmap.md index d3c21d8d9e..308aefe9d3 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -1,6 +1,6 @@ # PdfCraft roadmap: milestones and what's next -> **Last reviewed:** 2026-10-18 · **Last updated:** 2026-10-18 · **Change:** minor (M3, PR #3101: optional gapless page display with translations in all 22 catalogs; estimates unchanged) +> **Last reviewed:** 2026-10-18 · **Last updated:** 2026-10-18 · **Change:** minor (M3, PR #3101: optional gapless page display with translations in all 22 catalogs; organizer and comparison toggles corrected; estimates unchanged) > **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (M14/#2383 GPU-limit candidate; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) > **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (M7: the Edit text caret is as tall as the row it is on; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) @@ -115,7 +115,7 @@ total matches the parity estimate. | M2 | Model, render, text | 200–350 | 15% | 130–230 | hayro bootstrap renderer (vendored patches); inspector predictor scratch is bounded by available input, and JPX sample rescaling handles 1–32 bits safely. Text extraction reaches word-F1 0.98 against pdftotext. Short same-line redraws split before placement rather than interleaving. Missing: model crate, fonts, DisplayList, renderer independent of hayro. Inspector page-label traversal and output are resource-bounded, with physical-number fallback warnings. Packed TIFF-predictor RGB rows retain later rows in decoding and rendering. Page marks, added content and print output retain small page-unit transform coefficients when serialized. | | M2 | Model, render, text | 200–350 | 15% | 130–230 | hayro bootstrap renderer (vendored patches); inspector predictor scratch is bounded by available input, and JPX sample rescaling handles 1–32 bits safely. Text extraction reaches word-F1 0.98 against pdftotext. Short same-line redraws split before placement rather than interleaving. Missing: model crate, fonts, DisplayList, renderer independent of hayro. Inspector page-label traversal and output are resource-bounded, with physical-number fallback warnings. Packed TIFF-predictor RGB rows retain later rows in decoding and rendering. The renderer keeps its one-point crop-size floor in physical points after UserUnit scaling, with regression coverage for crop origins, rotations and f32 reciprocal rounding. Existing unit policy and nearly-zero fallback are retained. | | M2 | Model, render, text | 200–350 | 15% | 130–230 | hayro bootstrap renderer (vendored patches); inspector predictor scratch is bounded by available input, and JPX sample rescaling handles 1–32 bits safely. Text extraction reaches word-F1 0.98 against pdftotext. Short same-line redraws split before placement rather than interleaving. Missing: model crate, fonts, DisplayList, renderer independent of hayro. Inspector page-label traversal and output are resource-bounded, with physical-number fallback warnings | -| M3 | Viewer app (native + web) | 80–150 | 90% | 10–20 | Acrobat-style shell, find, select, panels, tiles, web build, UI control channel for agents (opt-in). Missing: 60 fps test on a 500-page document, snapshot tests of every panel
Done since: fit visible, document title in the window; Linux middle-button scrolling; shared three-state theme preferences in the toolbar, View menu and control channel; numeric Linux portal events and idle wakeups completing #881, with native appearance inheritance retained (#446); late browser startup downloads preserve the current document and Save target (#171). Selected-text Copy in Edit and the canvas context menu is covered by focused regressions and native clipboard checks (#1225); document copy restrictions apply to the floating action too. #1364 candidate: absolute local GoToR links ask before target access; headless callers explicitly opt in. Native-opening candidate: chosen tools survive preparation and password retries; startup intake waits for pending preparation. Page-label navigation displays logical labels, retains the current page on unchanged submission and refreshes focused same-document replacements; clean file-watcher reload preserves the chosen scrolling mode, preventing a Single Page reader from jumping to page 1 during replacement. Synthetic control regressions cover these behaviors. #3149: page-list changes discard render demand based on old geometry; actual raster dimensions are checked against the GPU limit before upload. Synthetic crash and continued-rendering regressions cover the replacement path. Optional page gaps join page edges without cropping PDF margins; current-document toggle, persisted default and UI control options are covered by viewer tests. | +| M3 | Viewer app (native + web) | 80–150 | 90% | 10–20 | Acrobat-style shell, find, select, panels, tiles, web build, UI control channel for agents (opt-in). Missing: 60 fps test on a 500-page document, snapshot tests of every panel
Done since: fit visible, document title in the window; Linux middle-button scrolling; shared three-state theme preferences in the toolbar, View menu and control channel; numeric Linux portal events and idle wakeups completing #881, with native appearance inheritance retained (#446); late browser startup downloads preserve the current document and Save target (#171). Selected-text Copy in Edit and the canvas context menu is covered by focused regressions and native clipboard checks (#1225); document copy restrictions apply to the floating action too. #1364 candidate: absolute local GoToR links ask before target access; headless callers explicitly opt in. Native-opening candidate: chosen tools survive preparation and password retries; startup intake waits for pending preparation. Page-label navigation displays logical labels, retains the current page on unchanged submission and refreshes focused same-document replacements; clean file-watcher reload preserves the chosen scrolling mode, preventing a Single Page reader from jumping to page 1 during replacement. Synthetic control regressions cover these behaviors. #3149: page-list changes discard render demand based on old geometry; actual raster dimensions are checked against the GPU limit before upload. Synthetic crash and continued-rendering regressions cover the replacement path. Optional page gaps join page edges without cropping PDF margins; current-document toggle, persisted default and UI control options are covered by viewer tests. Gap changes in Organize Pages preserve document scroll position, and both comparison panes follow menu and control choices. | | M3 | Viewer app (native + web) | 80–150 | 90% | 10–20 | Acrobat-style shell, find, select, panels, tiles, web build, UI control channel for agents (opt-in). Missing: 60 fps test on a 500-page document, snapshot tests of every panel
Done since: fit visible, document title in the window; Linux middle-button scrolling; shared three-state theme preferences in the toolbar, View menu and control channel; numeric Linux portal events and idle wakeups completing #881, with native appearance inheritance retained (#446); late browser startup downloads preserve the current document and Save target (#171). Selected-text Copy in Edit and the canvas context menu is covered by focused regressions and native clipboard checks (#1225); document copy restrictions apply to the floating action too. #1364 candidate: absolute local GoToR links ask before target access; headless callers explicitly opt in. Native-opening candidate: chosen tools survive preparation and password retries; startup intake waits for pending preparation. Page-label navigation displays logical labels, retains the current page on unchanged submission and refreshes focused same-document replacements; clean file-watcher reload preserves the chosen scrolling mode, preventing a Single Page reader from jumping to page 1 during replacement. Synthetic control regressions cover these behaviors. Wrapped revision buttons stay above Binding in Document Properties (#2739); a synthetic UI test checks layout and saved revision content. | | M3 | Viewer app (native + web) | 80–150 | 90% | 10–20 | Acrobat-style shell, find, select, panels, tiles, web build, UI control channel for agents (opt-in). Missing: 60 fps test on a 500-page document, snapshot tests of every panel
Done since: fit visible, document title in the window; Linux middle-button scrolling; shared three-state theme preferences in the toolbar, View menu and control channel; numeric Linux portal events and idle wakeups completing #881, with native appearance inheritance retained (#446); late browser startup downloads preserve the current document and Save target (#171). #1364 candidate: absolute local GoToR links ask before target access; headless callers explicitly opt in. Native-opening candidate: chosen tools survive preparation and password retries; startup intake waits for pending preparation. Later native-open replies stay queued while a modal or picker owns the foreground; regressions cover recipient ownership, completion/cancellation and one-shot wakes. | | M3 | Viewer app (native + web) | 80–150 | 90% | 10–20 | Acrobat-style shell, find, select, panels, tiles, web build, UI control channel for agents (opt-in). Missing: 60 fps test on a 500-page document, snapshot tests of every panel
Done since: fit visible, document title in the window; Linux middle-button scrolling; shared three-state theme preferences in the toolbar, View menu and control channel; numeric Linux portal events and idle wakeups completing #881, with native appearance inheritance retained (#446); late browser startup downloads preserve the current document and Save target (#171). #1364 candidate: absolute local GoToR links ask before target access; headless callers explicitly opt in. Native-opening candidate: chosen tools survive preparation and password retries; startup intake waits for pending preparation. #1980: exact quit modifiers preserve Ctrl+Alt+Q email input; Ctrl+Q/Cmd+Q asks before discarding unsaved documents. | diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 50c0059145..4e0d198229 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -852,9 +852,9 @@ area = "B" tier = "P0" milestone = "M3" status = "shipped" -commands = ["view.continuous"] +commands = ["view.continuous", "view.page_gaps"] tools = [] -evidence = ["crates/ui-egui/tests/view.rs::page_gaps_stay_closed_at_fractional_device_scales", "crates/ui-egui/tests/view.rs::page_gaps_toggle_joins_edges_without_changing_the_document", "crates/ui-egui/tests/view.rs::page_gaps_default_survives_restart_and_applies_to_new_documents", "crates/ui-egui/tests/view.rs::page_gaps_menu_joins_pages_without_a_shadow_at_the_seam", "crates/ui-egui/tests/view.rs::page_gaps_work_for_mixed_sizes_and_two_page_rows", "crates/ui-egui/tests/view.rs::continuous_layout_stacks_pages_vertically", "crates/ui-egui/tests/view.rs::page_display_commands_switch_layouts", "crates/ui-egui/tests/view.rs::default_page_display_is_used_for_new_documents", "crates/ui-egui/src/canvas.rs::fit_page_and_height_hold_still_while_mixed_page_sizes_scroll_past", "crates/ui-egui/tests/view.rs::touchpad_swipe_glides_on_after_the_fingers_lift_and_stops", "crates/ui-egui/tests/view.rs::touching_the_touchpad_again_or_turning_animations_off_stops_the_glide", "crates/ui-egui/tests/view.rs::rapid_wheel_notches_accelerate_and_follow_touchpad_input", "crates/ui-egui/tests/view.rs::changing_continuous_layout_resets_wheel_acceleration"] +evidence = ["crates/ui-egui/tests/view.rs::page_gaps_in_organizer_do_not_move_the_document_scroll_position", "crates/ui-egui/tests/view.rs::page_gaps_comparison_menu_preserves_the_active_panes_choice", "crates/ui-egui/tests/view.rs::page_gaps_comparison_control_preserves_both_states", "crates/ui-egui/tests/view.rs::page_gaps_stay_closed_at_fractional_device_scales", "crates/ui-egui/tests/view.rs::page_gaps_toggle_joins_edges_without_changing_the_document", "crates/ui-egui/tests/view.rs::page_gaps_default_survives_restart_and_applies_to_new_documents", "crates/ui-egui/tests/view.rs::page_gaps_menu_joins_pages_without_a_shadow_at_the_seam", "crates/ui-egui/tests/view.rs::page_gaps_work_for_mixed_sizes_and_two_page_rows", "crates/ui-egui/tests/view.rs::continuous_layout_stacks_pages_vertically", "crates/ui-egui/tests/view.rs::page_display_commands_switch_layouts", "crates/ui-egui/tests/view.rs::default_page_display_is_used_for_new_documents", "crates/ui-egui/src/canvas.rs::fit_page_and_height_hold_still_while_mixed_page_sizes_scroll_past", "crates/ui-egui/tests/view.rs::touchpad_swipe_glides_on_after_the_fingers_lift_and_stops", "crates/ui-egui/tests/view.rs::touching_the_touchpad_again_or_turning_animations_off_stops_the_glide", "crates/ui-egui/tests/view.rs::rapid_wheel_notches_accelerate_and_follow_touchpad_input", "crates/ui-egui/tests/view.rs::changing_continuous_layout_resets_wheel_acceleration"] notes = "Page gaps, shadows and outlines can be hidden through the page-display menu, with a persisted default in Preferences and matching UI control options; page margins in the PDF are unchanged. Continuous scrolling remains the default for PDFs that don't ask for a layout (Preferences ▸ Default page display); pages stack with no snapping. Fit page and Fit height fit the largest page, so the zoom holds still across mixed page sizes. Rapid line-wheel notches gain a bounded, constant-space arrival-rate boost (#1072); isolated notches still use the chosen Scroll speed, and page units stay unboosted. A touchpad swipe glides on after the fingers lift on Wayland (#531; macOS glides by itself; Windows and X11 report no lift through winit), unless Animation effects is off; a click, key, zoom or new scroll catches it. Not yet: fingers resting on the touchpad don't catch it (needs Wayland hold gestures, which the vendored winit doesn't bind)" [[feature]]