From 1651069e7ac6e061559815e2fe3fed4d624c0498 Mon Sep 17 00:00:00 2001 From: Justin John <34035011+justinjohn0306@users.noreply.github.com> Date: Sun, 11 Oct 2026 23:24:26 +0530 Subject: [PATCH] Viewer: Continuous scrolling is an independent checkbox over the base layout (#752) (#896) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Viewer: touchpad scrolling tracks the finger 1:1 (#759) A precision touchpad sends pixel-precise `MouseWheel` events. On Windows they arrive with no `TouchPhase::Start`, so egui never takes its no-smoothing "in touch" path and eases any delta of 8 px or more over several frames, leaving the page a frame behind the finger. The page view now applies precise (Point-unit) wheel input itself, 1:1, through the same un-eased scroll path as keyboard and middle-button auto-scroll, and suppresses egui's eased copy for a short window so its internal easing tail neither doubles nor trails the motion. A coarse mouse wheel (Line/Page units) keeps egui's smoothing, which is pleasant for chunky notches, and single-page paging still owns the vertical axis. Regression test: one 200 px precise swipe scrolls the full 200 px at once (an eased path would ramp there over ~0.1 s) and leaves no drift. Co-Authored-By: Claude Opus 4.8 * Viewer: Continuous scrolling is an independent checkbox over the base layout (#752) Page display matches Acrobat: the base layout (Single page or Two-page view) is a radio choice, and Continuous scrolling is a separate checkbox, so every combination is reachable. This also adds the previously missing "Two-page view, one spread at a time" mode. `PageLayout` now has just `Single` and `TwoUp`; a separate `continuous` flag on the view (and on the default view preferences) decides whether it scrolls. A single `display_token` ("single" | "continuous" | "two-up" | "two-page") serialises both for settings and the control channel, keeping the earlier `continuous`/`single`/`two-up` names working and adding `two-page` for the new mode. The PDF's own page-layout request now maps to the right (layout, continuous) pair, including plain TwoUp. - View ▸ Page display and the rail menu show two radios plus a Continuous scrolling checkbox; Preferences gains a default Continuous checkbox. - `view.continuous` toggles the checkbox; `view.layout.single` / `view.layout.two_up` switch the base layout without touching it. The former `view.layout.continuous` command is gone (its state is Single + continuous); the `layout`/`default-layout` options and a new `continuous` option cover scripting. - Not scrolling, the wheel and the arrow/Page keys turn whole spreads in Two-page view, and only the current page or spread is laid out. Tests: new `notches_turn_two_page_spreads`, `two_page_view_shows_one_spread_at_a_time_until_continuous`, and updated layout/command/persistence tests; the `view()` helper takes `continuous`. Co-Authored-By: Claude Opus 4.8 * Parity: the continuous-scrolling feature points at view.continuous --------- Co-authored-by: Justin John Co-authored-by: Claude Opus 4.8 Co-authored-by: Brandon Thomas --- apps/pdfcraft/src/main.rs | 4 +- crates/engine/src/commands.rs | 7 +- crates/ui-egui/src/canvas.rs | 216 +++++++++++++++++++++++---------- crates/ui-egui/src/chrome.rs | 6 + crates/ui-egui/src/commands.rs | 12 +- crates/ui-egui/src/js_ui.rs | 1 + crates/ui-egui/src/lib.rs | 60 +++++++-- crates/ui-egui/tests/view.rs | 45 +++++-- crates/ui-egui/tests/viewer.rs | 5 +- parity/acrobat-features.toml | 2 +- 10 files changed, 263 insertions(+), 95 deletions(-) diff --git a/apps/pdfcraft/src/main.rs b/apps/pdfcraft/src/main.rs index fca781e5..de93d4a2 100644 --- a/apps/pdfcraft/src/main.rs +++ b/apps/pdfcraft/src/main.rs @@ -4,10 +4,10 @@ //! `--create-images [images…]` stages the images in one PDF and asks for the page DPI. //! //! View options (applied after the files open; also the seed of the UI control channel): -//! `--page N --zoom 150 --layout continuous|two-up|single --panel comments|bookmarks|pages|fields|layers|attachments|none +//! `--page N --zoom 150 --layout single|continuous|two-up|two-page --continuous on|off --panel comments|bookmarks|pages|fields|layers|attachments|none //! --theme light|dark|system --language auto| --mode all|read|edit|convert|sign --tool --left open|closed //! --organize on --fields on --dialog properties|shortcuts|about --palette --home on -//! --cover on|off --default-layout continuous|two-up|single --default-zoom fit-width|fit-page| +//! --cover on|off --default-layout single|continuous|two-up|two-page --default-continuous on|off --default-zoom fit-width|fit-page| //! --combine-view grid|list --combine-zoom 60..200` //! //! `--new-window` opens a window of its own. Without it, on Windows, a launch that only names files diff --git a/crates/engine/src/commands.rs b/crates/engine/src/commands.rs index 0771f27f..782110ce 100644 --- a/crates/engine/src/commands.rs +++ b/crates/engine/src/commands.rs @@ -190,11 +190,12 @@ pub const COMMANDS: &[CommandSpec] = &[ c("edit.find", "Find…", EDIT, Some(Shortcut::cmd("F")), Document, "search"), c("edit.advanced_search", "Advanced search…", EDIT, Some(Shortcut::cmd_shift("F")), Document, "search"), c("view.palette", "Find tools and commands…", VIEW, Some(Shortcut::cmd("K")), Nothing, "search"), - // Page display: View ▸ Page display and the rail's button list these as radios, so - // `menu = None` keeps them out of the generated menus. The palette still runs them. - c("view.layout.continuous", "Continuous scrolling", None, None, Document, "arrow-up-down"), + // Page display: View ▸ Page display and the rail's button list the base layouts as radios and + // continuous scrolling as a checkbox, so `menu = None` keeps them out of the generated menus. + // The palette still runs them. c("view.layout.single", "Single page", None, None, Document, "file-text"), c("view.layout.two_up", "Two-page view", None, None, Document, "columns-2"), + c("view.continuous", "Continuous scrolling", None, None, Document, "arrow-up-down"), c("view.layout.cover", "Show cover page in two-page view", None, None, TwoPageView, "bookmark"), // Acrobat's view modes, a page display and a zoom at once (the rail's Page display menu). c("view.fit_width_scrolling", "Fit to width scrolling", None, None, Document, "arrow-left-right"), diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 8d49c7e7..5d73529e 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -74,28 +74,28 @@ pub enum Fit { None, } +/// The base page display: one page across, or a two-page spread. Whether it scrolls continuously +/// is a separate flag (`continuous`), as in Acrobat, so every combination is reachable (#752). #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum PageLayout { - Continuous, - TwoUp, Single, + TwoUp, } impl PageLayout { - /// Every page display, in the order the View menu, the rail's Page display button and + /// The base page displays, in the order the View menu, the rail's Page display button and /// Preferences list them. - pub const ORDER: [Self; 3] = [Self::Continuous, Self::Single, Self::TwoUp]; + pub const ORDER: [Self; 2] = [Self::Single, Self::TwoUp]; - /// The name settings and view options use: `continuous`, `single` or `two-up`. + /// The name settings and view options use for the base layout: `single` or `two-up`. pub fn as_str(self) -> &'static str { match self { - Self::Continuous => "continuous", Self::Single => "single", Self::TwoUp => "two-up", } } - /// The layout an [`as_str`](Self::as_str) name stands for, in any case. `None` for + /// The base layout an [`as_str`](Self::as_str) name stands for, in any case. `None` for /// anything else, so callers can reject a typo rather than switch layouts silently. pub fn try_parse(value: &str) -> Option { Self::ORDER.into_iter().find(|l| l.as_str().eq_ignore_ascii_case(value.trim())) @@ -104,22 +104,20 @@ impl PageLayout { /// Rail icon for this layout. pub fn icon(self) -> &'static str { match self { - Self::Continuous => "arrow-up-down", Self::Single => "file-text", Self::TwoUp => "columns-2", } } - /// Registry command that switches to this layout (`PdfCraftApp::execute`). + /// Registry command that switches to this base layout (`PdfCraftApp::execute`). pub fn command(self) -> &'static str { match self { - Self::Continuous => "view.layout.continuous", Self::Single => "view.layout.single", Self::TwoUp => "view.layout.two_up", } } - /// The layout a [`command`](Self::command) id switches to. + /// The base layout a [`command`](Self::command) id switches to. pub fn from_command(id: &str) -> Option { Self::ORDER.into_iter().find(|l| l.command() == id) } @@ -127,18 +125,43 @@ impl PageLayout { /// Menu label: the label of [`command`](Self::command), which the catalogs translate. pub fn label(self) -> &'static str { match self { - Self::Continuous => "Continuous scrolling", Self::Single => "Single page", Self::TwoUp => "Two-page view", } } } +/// The single page-display token settings and the control channel use, combining the base layout +/// with continuous scrolling. `single` and `two-page` show one page or one spread at a time; +/// `continuous` (Single, scrolling) and `two-up` (Two-page, scrolling) keep their earlier names +/// for back-compat. +pub fn display_token(layout: PageLayout, continuous: bool) -> &'static str { + match (layout, continuous) { + (PageLayout::Single, false) => "single", + (PageLayout::Single, true) => "continuous", + (PageLayout::TwoUp, true) => "two-up", + (PageLayout::TwoUp, false) => "two-page", + } +} + +/// The base layout and continuous flag a [`display_token`] names, in any case. `None` for a typo. +pub fn parse_display_token(value: &str) -> Option<(PageLayout, bool)> { + match value.trim().to_ascii_lowercase().as_str() { + "single" => Some((PageLayout::Single, false)), + "continuous" => Some((PageLayout::Single, true)), + "two-up" => Some((PageLayout::TwoUp, true)), + "two-page" | "two-up-single" => Some((PageLayout::TwoUp, false)), + _ => None, + } +} + /// How a newly opened document is shown (Preferences ▸ Documents and view). A PDF that asks /// for its own layout or zoom gets that instead. #[derive(Clone, Copy, Debug, PartialEq)] pub struct ViewDefaults { pub layout: PageLayout, + /// Whether the base layout scrolls continuously. + pub continuous: bool, /// [`Fit::Width`] or [`Fit::Page`], or [`Fit::None`] for a fixed `zoom`. pub fit: Fit, /// The zoom with [`Fit::None`] (1.0 = 100%). @@ -150,7 +173,8 @@ pub struct ViewDefaults { impl Default for ViewDefaults { fn default() -> Self { - Self { layout: PageLayout::Continuous, fit: Fit::Width, zoom: 1.0, highlight_fields: false } + // Single page, scrolling continuously at fit width: the earlier "continuous" default. + Self { layout: PageLayout::Single, continuous: true, fit: Fit::Width, zoom: 1.0, highlight_fields: false } } } @@ -263,6 +287,8 @@ pub struct DocView { pub zoom: f32, pub fit: Fit, pub layout: PageLayout, + /// Whether the base layout scrolls continuously (independent of `layout`, as in Acrobat). + pub continuous: bool, /// View rotation in degrees clockwise (0, 90, 180, 270); display only, never saved. pub rotation: u16, pub current: usize, @@ -512,6 +538,7 @@ impl DocView { zoom: defaults.zoom, fit: defaults.fit, layout: defaults.layout, + continuous: defaults.continuous, rotation: 0, current: 0, organize: false, @@ -951,8 +978,8 @@ impl DocView { (false, _) => first.saturating_sub(2), } } - PageLayout::Continuous | PageLayout::Single if forward => c + 1, - PageLayout::Continuous | PageLayout::Single => c.saturating_sub(1), + PageLayout::Single if forward => c + 1, + PageLayout::Single => c.saturating_sub(1), }; self.go_to_page(target); } @@ -1015,8 +1042,9 @@ impl DocView { self.goto = Some((self.current, 0.0)); } - /// Switch the page display. A no-op when the layout is already current, so re-selecting - /// it never yanks a scrolled view back to the top. + /// Switch the base page display (Single or Two-page), leaving continuous scrolling as it is. + /// A no-op when the layout is already current, so re-selecting it never yanks a scrolled view + /// back to the top. pub fn set_layout(&mut self, layout: PageLayout) { if self.layout == layout { return; @@ -1025,6 +1053,27 @@ impl DocView { self.goto = Some((self.current, 0.0)); } + /// Whether the view scrolls continuously (as opposed to one page or spread at a time). + pub fn continuous(&self) -> bool { + self.continuous + } + + /// Turn continuous scrolling on or off, keeping the current page in view. A no-op when it is + /// already in that state. + pub fn set_continuous(&mut self, continuous: bool) { + if self.continuous != continuous { + self.continuous = continuous; + self.goto = Some((self.current, 0.0)); + } + } + + /// Set the whole page display at once (the base layout and continuous scrolling together), + /// as the control channel and settings do through a single [`display_token`]. + pub fn set_display(&mut self, layout: PageLayout, continuous: bool) { + self.set_layout(layout); + self.set_continuous(continuous); + } + /// Set the cover page in two-page view. A no-op unless something changed. pub fn set_cover(&mut self, cover: bool) { if self.cover != cover { @@ -1038,11 +1087,12 @@ impl DocView { self.layout == PageLayout::TwoUp } - /// One wheel event in single-page view (the rules are in `wheel_pager`): `dy` is its - /// vertical delta (negative scrolls down) and `now` is egui time. With `can_turn` false - /// the gesture is followed but the page stays. Returns whether the page turned. + /// One wheel event in a non-continuous view, where the wheel turns pages or spreads (the + /// rules are in `wheel_pager`): `dy` is its vertical delta (negative scrolls down) and `now` + /// is egui time. With `can_turn` false the gesture is followed but the page stays. Returns + /// whether the page turned. pub fn single_page_wheel(&mut self, unit: egui::MouseWheelUnit, dy: f32, phase: egui::TouchPhase, now: f64, can_turn: bool) -> bool { - if self.layout != PageLayout::Single { + if self.continuous { return false; } let Some(forward) = self.wheel.feed(unit, dy, phase, now, can_turn) else { return false }; @@ -1402,11 +1452,13 @@ impl DocView { fn fit_zoom(&mut self, info: &DocInfo) { let largest = |side: fn((f32, f32)) -> f32| info.pages.iter().map(|p| side(self.display_size(p))).fold(1.0, f32::max); let max_w = largest(|s| s.0); - // Single-page view fits the page it shows. The scrolling views fit their largest page, - // so the zoom holds still while pages of other sizes scroll past. - let (w, h) = match self.layout { - PageLayout::Single => info.pages.get(self.current).map_or_else(|| (max_w, largest(|s| s.1)), |p| self.display_size(p)), - PageLayout::Continuous | PageLayout::TwoUp => (max_w, largest(|s| s.1)), + // A single page shown on its own fits that page. Every other display (scrolling, or a + // two-page spread) fits its largest page, so the zoom holds still while pages of other + // sizes scroll or page past. + let (w, h) = if self.layout == PageLayout::Single && !self.continuous { + info.pages.get(self.current).map_or_else(|| (max_w, largest(|s| s.1)), |p| self.display_size(p)) + } else { + (max_w, largest(|s| s.1)) }; let avail_w = (self.viewport_w - 2.0 * SIDE).max(100.0); let per_row = if self.layout == PageLayout::TwoUp { 2.0 } else { 1.0 }; @@ -1426,7 +1478,7 @@ impl DocView { /// Width of the scroll content. Single-page view sizes it to the page it shows, so a /// wider page elsewhere in the document can't push the shown one off-centre. fn content_width(&self, info: &DocInfo, avail_w: f32) -> f32 { - let single = (self.layout == PageLayout::Single).then(|| self.current.min(info.pages.len().saturating_sub(1))); + let single = (self.layout == PageLayout::Single && !self.continuous).then(|| self.current.min(info.pages.len().saturating_sub(1))); let shown = info.pages.iter().enumerate().filter(|&(i, _)| single.is_none_or(|c| c == i)); let max_w = shown.map(|(_, p)| self.display_size(p).0).fold(0.0, f32::max); let per_row = if self.layout == PageLayout::TwoUp { 2.0 } else { 1.0 }; @@ -1439,7 +1491,7 @@ impl DocView { let mut rects = Vec::with_capacity(info.pages.len()); let mut y = MARGIN; match self.layout { - PageLayout::Continuous | PageLayout::Single => { + PageLayout::Single => { for p in &info.pages { let (w, h) = self.display_size(p); let size = vec2(w * s, h * s); @@ -1706,7 +1758,7 @@ pub fn shortcuts(view: &mut DocView, ctx: &egui::Context) { // As in Acrobat: → / ← go to the next / previous page in every layout (#185), and so do // ⌘Page Down / ⌘Page Up, or plain Page Down / Page Up in single-page view. let command = ctx.input(|i| i.modifiers.command); - let scrolls = view.layout != PageLayout::Single; + let scrolls = view.continuous; if key(Key::ArrowRight) || (key(Key::PageDown) && (command || !scrolls)) { view.step_page(true); } @@ -1839,15 +1891,34 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { let content_w = view.content_width(info, avail.width()); let rects = view.layout(info, content_w); + // When not scrolling continuously, only one row is shown at a time: the current page, or the + // current two-page spread (with the cover rule). These indices drive paging, which pages are + // drawn, and the content height (#752). Empty for a document with no pages. + let row: Vec = if rects.is_empty() { + Vec::new() + } else { + let last = rects.len() - 1; + let cur = view.current.min(last); + match view.layout { + PageLayout::Single => vec![cur], + PageLayout::TwoUp if view.cover && cur == 0 => vec![0], + PageLayout::TwoUp => { + let first = if view.cover { cur - (cur + 1) % 2 } else { cur - cur % 2 }; + if first < last { vec![first, first + 1] } else { vec![first] } + } + } + }; let middle_gesture = view.auto_scroll.blocks_input(); - // Single page: when the whole page fits, the wheel would do nothing, so it turns pages - // instead; zoomed in far enough to pan, it pans. Touch drags are untouched: with nothing - // to pan, touch users turn pages with the rail buttons, the page box and the arrow keys. - // Runs before `visible_pages` so the frame that turns the page draws it. + // Not scrolling: when the whole page or spread fits, the wheel would do nothing, so it turns + // pages instead; zoomed in far enough to pan, it pans. Touch drags are untouched: with + // nothing to pan, touch users turn pages with the rail buttons, the page box and the arrow + // keys. Runs before `visible_pages` so the frame that turns the page draws it. let mut single_paging = false; - if view.layout == PageLayout::Single { + if !view.continuous { + // The row's bounding box on screen (one page, or a two-page spread). + let row_rect = row.iter().filter_map(|&i| rects.get(i)).copied().reduce(Rect::union); // Within a point, so layout rounding can't stop a fitting page from turning. - let fits = rects.get(view.current.min(rects.len().saturating_sub(1))).is_some_and(|r| r.height() + 2.0 * MARGIN <= avail.height() + 1.0); + let fits = row_rect.is_some_and(|r| r.height() + 2.0 * MARGIN <= avail.height() + 1.0); let can_turn = fits && !middle_gesture && unobstructed && ui.rect_contains_pointer(avail); single_paging = can_turn; // Every wheel event goes to the pager, so it follows each trackpad touch to its end @@ -1907,16 +1978,15 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { } else { precise_scroll = Vec2::ZERO; } - let visible_pages: Vec = match view.layout { - PageLayout::Single => vec![view.current.min(rects.len() - 1)], - _ => (0..rects.len()).collect(), - }; - let (y_shift, content_h) = match view.layout { - PageLayout::Single => { - let r = rects[visible_pages[0]]; - (r.top() - MARGIN, r.height() + 2.0 * MARGIN) + let visible_pages: Vec = if view.continuous { (0..rects.len()).collect() } else { row.clone() }; + let (y_shift, content_h) = if view.continuous { + (0.0, rects.last().map(|r| r.bottom() + MARGIN).unwrap_or(0.0)) + } else { + // Only the current row scrolls into view; the content is its bounding box plus margins. + match row.iter().filter_map(|&i| rects.get(i)).copied().reduce(Rect::union) { + Some(r) => (r.top() - MARGIN, r.height() + 2.0 * MARGIN), + None => (0.0, 0.0), } - _ => (0.0, rects.last().map(|r| r.bottom() + MARGIN).unwrap_or(0.0)), }; let mut scroll = egui::ScrollArea::both().auto_shrink([false, false]).scroll_source(egui::scroll_area::ScrollSource { @@ -2582,7 +2652,7 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { if current_overlap >= best_overlap - TIE || current_overlap >= current_height - TIE { current = view.current; } - if view.layout != PageLayout::Single { + if view.continuous { view.current = current; if !ui.memory(|m| m.has_focus(egui::Id::new("page-input"))) { view.page_input = (current + 1).to_string(); @@ -2641,7 +2711,7 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { }); (wanted, visible_now) }); - view.single_edges = visible_pages.first().copied().filter(|_| view.layout == PageLayout::Single).map(|page| { + view.single_edges = visible_pages.first().copied().filter(|_| !view.continuous).map(|page| { // At the top once the page's top edge shows where going to a page puts it (a gap // below the window's top). Within a point, as for wheel paging, so layout rounding // can't hide an edge. @@ -3760,7 +3830,7 @@ trailer << /Root 1 0 R >> assert!(!sharp_enough(264, 876.0), "a thumbnail stretched over a zoomed page"); assert!(!sharp_enough(876, 200.0), "far larger than drawn: wasteful"); assert!(!sharp_enough(100, 0.0) && !sharp_enough(100, f32::NAN)); - let mut v = view(3, PageLayout::Continuous); + let mut v = view(3, PageLayout::Single, true); for odd in [f32::NAN, f32::INFINITY] { v.set_grid_zoom(odd); assert_eq!(v.grid_zoom(), 1.0); @@ -3771,14 +3841,14 @@ trailer << /Root 1 0 R >> assert_eq!(v.grid_zoom(), 0.5); } - fn view(pages: usize, layout: PageLayout) -> DocView { + fn view(pages: usize, layout: PageLayout, continuous: bool) -> DocView { let info = pdfcraft_render::DocInfo { pages: (0..pages) .map(|_| pdfcraft_render::PageInfo { width: 300.0, height: 400.0, label: String::new(), crop: [0.0, 0.0, 300.0, 400.0], rotation: 0 }) .collect(), ..Default::default() }; - DocView::new(DocId(1), &info, ViewDefaults { layout, ..Default::default() }) + DocView::new(DocId(1), &info, ViewDefaults { layout, continuous, ..Default::default() }) } fn thumbnail_info(count: usize) -> DocInfo { @@ -4102,7 +4172,7 @@ trailer << /Root 1 0 R >> #[test] fn suspending_a_view_releases_owned_raster_textures_and_queue_state() { let ctx = egui::Context::default(); - let mut view = view(2, PageLayout::Continuous); + let mut view = view(2, PageLayout::Single, true); let texture = |name| ctx.load_texture(name, egui::ColorImage::filled([1, 1], Color32::WHITE), TextureOptions::LINEAR); let request = RenderRequest { page: 0, scale: 1.0, tag: 1, ..Default::default() }; view.pages.insert(0, PageTex { tag: request.tag, tex: texture("suspended-page") }); @@ -4212,7 +4282,7 @@ trailer << /Root 1 0 R >> #[test] fn notches_turn_pages_and_stop_at_the_ends() { - let mut v = view(3, PageLayout::Single); + let mut v = view(3, PageLayout::Single, false); assert!(notch(&mut v, -1.0, 1.0) && notch(&mut v, -1.0, 1.05)); assert_eq!(v.current, 2); assert!(!notch(&mut v, -1.0, 1.1), "nothing turns past the last page"); @@ -4221,10 +4291,22 @@ trailer << /Root 1 0 R >> } #[test] - fn wheel_is_single_page_only() { - for layout in [PageLayout::Continuous, PageLayout::TwoUp] { - let mut v = view(3, layout); - assert!(!notch(&mut v, -1.0, 1.0), "{layout:?} ignores the wheel"); + fn notches_turn_two_page_spreads() { + // Two-page view, shown one spread at a time (not continuous): a notch turns a whole + // spread at once, not a single page (#752). + let mut v = view(4, PageLayout::TwoUp, false); + assert!(notch(&mut v, -1.0, 1.0)); + assert_eq!(v.current, 2, "a notch turns from the first spread to the second"); + assert!(notch(&mut v, 1.0, 1.1)); + assert_eq!(v.current, 0, "a notch back turns to the previous spread"); + } + + #[test] + fn continuous_views_ignore_the_wheel_for_paging() { + // Scrolling views let the ScrollArea handle the wheel; it never turns pages. + for layout in [PageLayout::Single, PageLayout::TwoUp] { + let mut v = view(3, layout, true); + assert!(!notch(&mut v, -1.0, 1.0), "{layout:?} continuous ignores the wheel"); assert_eq!(v.current, 0); } } @@ -4232,7 +4314,7 @@ trailer << /Root 1 0 R >> #[test] fn navigation_drops_partial_wheel_motion() { use egui::{MouseWheelUnit::Point, TouchPhase::Move, TouchPhase::Start}; - let mut v = view(3, PageLayout::Single); + let mut v = view(3, PageLayout::Single, false); assert!(!v.single_page_wheel(Point, 0.0, Start, 1.0, true)); assert!(!v.single_page_wheel(Point, -30.0, Move, 1.02, true)); v.go_to_page(1); @@ -4244,12 +4326,17 @@ trailer << /Root 1 0 R >> #[test] fn reselecting_the_layout_does_not_scroll_to_top() { - let mut v = view(3, PageLayout::Continuous); + let mut v = view(3, PageLayout::Single, true); v.goto = None; - v.set_layout(PageLayout::Continuous); - assert!(v.goto.is_none(), "re-selecting the current layout is a no-op"); v.set_layout(PageLayout::Single); - assert_eq!(v.goto, Some((0, 0.0))); + assert!(v.goto.is_none(), "re-selecting the current layout is a no-op"); + v.set_continuous(true); + assert!(v.goto.is_none(), "re-selecting the current continuity is a no-op"); + v.set_layout(PageLayout::TwoUp); + assert_eq!(v.goto, Some((0, 0.0)), "switching the base layout aligns the current page"); + v.goto = None; + v.set_continuous(false); + assert_eq!(v.goto, Some((0, 0.0)), "toggling continuous scrolling aligns the current page"); } #[test] @@ -4265,8 +4352,8 @@ trailer << /Root 1 0 R >> v.current = 1; v.fit_zoom(&info); assert_eq!(v.zoom, zoom, "{fit:?}: scrolling onto a bigger page keeps the zoom"); - // Single-page view shows one page at a time, so it fits each one. - v.layout = PageLayout::Single; + // A single page shown on its own (not scrolling) fits each one. + (v.layout, v.continuous) = (PageLayout::Single, false); v.fit_zoom(&info); let big = v.zoom; v.current = 0; @@ -4283,6 +4370,7 @@ trailer << /Root 1 0 R >> let mut v = DocView::new(DocId(1), &info, ViewDefaults::default()); (v.fit, v.viewport_w, v.viewport_h) = (Fit::Width, 1000.0, 800.0); v.layout = PageLayout::Single; + v.continuous = false; v.fit_zoom(&info); let narrow = v.zoom; // The shown page fills the width between the side gutters, not centred for the wide page. @@ -4292,7 +4380,7 @@ trailer << /Root 1 0 R >> v.fit_zoom(&info); assert!((narrow / v.zoom - 2.0).abs() < 1e-3, "the narrow page gets twice the zoom of the wide one"); // Scrolling views keep one zoom for the whole document. - v.layout = PageLayout::Continuous; + v.continuous = true; v.fit_zoom(&info); let wide = v.zoom; v.current = 0; @@ -4320,7 +4408,7 @@ trailer << /Root 1 0 R >> .collect(), ..Default::default() }; - let mut v = view(3, PageLayout::Continuous); + let mut v = view(3, PageLayout::Single, true); v.fit = Fit::Width; // No position (or a malformed one): the top of the page, zoom as it was. v.go_to_dest(1, DestView::Top, &info); diff --git a/crates/ui-egui/src/chrome.rs b/crates/ui-egui/src/chrome.rs index 3367bce9..58cf8ccd 100644 --- a/crates/ui-egui/src/chrome.rs +++ b/crates/ui-egui/src/chrome.rs @@ -599,6 +599,12 @@ fn page_display_menu(ui: &mut egui::Ui, view: &DocView) -> Option<&'static str> picked = Some(l.command()); } } + ui.separator(); + // Continuous scrolling is an independent checkbox over the base layout, as in Acrobat (#752). + let mut continuous = view.continuous(); + if ui.add(egui::Checkbox::new(&mut continuous, tl!("Continuous scrolling"))).changed() { + picked = Some("view.continuous"); + } let mut cover = view.cover; if ui.add_enabled(view.cover_applies(), egui::Checkbox::new(&mut cover, tl!("Show cover page in two-page view"))).changed() { picked = Some("view.layout.cover"); diff --git a/crates/ui-egui/src/commands.rs b/crates/ui-egui/src/commands.rs index d76cf823..32113ace 100644 --- a/crates/ui-egui/src/commands.rs +++ b/crates/ui-egui/src/commands.rs @@ -222,6 +222,13 @@ impl PdfCraftApp { self.views[i].set_layout(layout); } } + // The Continuous scrolling checkbox (an independent toggle over the base layout). + "view.continuous" => { + if let Some(i) = active { + let v = &mut self.views[i]; + v.set_continuous(!v.continuous()); + } + } "view.layout.cover" => { if let Some(i) = active { let v = &mut self.views[i]; @@ -230,9 +237,10 @@ impl PdfCraftApp { } "view.fit_width_scrolling" | "view.fit_one_page" => { use crate::canvas::{Fit, PageLayout}; - let (layout, fit) = if id == "view.fit_one_page" { (PageLayout::Single, Fit::Page) } else { (PageLayout::Continuous, Fit::Width) }; + // Fit one full page shows a single page on its own; fit width scrolls it. + let (continuous, fit) = if id == "view.fit_one_page" { (false, Fit::Page) } else { (true, Fit::Width) }; if let Some(i) = active { - self.views[i].set_layout(layout); + self.views[i].set_display(PageLayout::Single, continuous); self.views[i].set_fit(fit); } } diff --git a/crates/ui-egui/src/js_ui.rs b/crates/ui-egui/src/js_ui.rs index 4ef64db4..5aee8e20 100644 --- a/crates/ui-egui/src/js_ui.rs +++ b/crates/ui-egui/src/js_ui.rs @@ -305,6 +305,7 @@ pub(crate) fn preferences_body(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tok for l in crate::canvas::PageLayout::ORDER { ui.radio_value(&mut defaults.layout, l, tl!(l.label())); } + ui.checkbox(&mut defaults.continuous, tl!("Continuous scrolling")); }); ui.horizontal(|ui| { use crate::canvas::Fit; diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 0ef6b45a..da0ab9b5 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -916,12 +916,15 @@ impl PdfCraftApp { match v.layout { // The view opened in Default page display already. L::Default => {} - L::SinglePage => view.layout = canvas::PageLayout::Single, - L::SinglePageContinuous => view.layout = canvas::PageLayout::Continuous, - L::TwoUp | L::TwoUpContinuous => view.layout = canvas::PageLayout::TwoUp, - L::TwoUpCoverPage | L::TwoUpContinuousCoverPage => { - view.layout = canvas::PageLayout::TwoUp; - view.cover = true; + L::SinglePage => (view.layout, view.continuous) = (canvas::PageLayout::Single, false), + L::SinglePageContinuous => (view.layout, view.continuous) = (canvas::PageLayout::Single, true), + L::TwoUp => (view.layout, view.continuous) = (canvas::PageLayout::TwoUp, false), + L::TwoUpContinuous => (view.layout, view.continuous) = (canvas::PageLayout::TwoUp, true), + L::TwoUpCoverPage => { + (view.layout, view.continuous, view.cover) = (canvas::PageLayout::TwoUp, false, true); + } + L::TwoUpContinuousCoverPage => { + (view.layout, view.continuous, view.cover) = (canvas::PageLayout::TwoUp, true, true); } } match v.magnification { @@ -1505,7 +1508,7 @@ impl PdfCraftApp { "theme": self.theme_preference, "scroll_speed": self.scroll_speed, "default_mode": self.default_mode, - "default_layout": self.view_defaults.layout.as_str(), + "default_layout": canvas::display_token(self.view_defaults.layout, self.view_defaults.continuous), "default_zoom": self.view_defaults.zoom_name(), "highlight_fields": self.view_defaults.highlight_fields, "language": self.language, @@ -1588,8 +1591,8 @@ impl PdfCraftApp { if let Ok(mode) = serde_json::from_value::(v["default_mode"].clone()) { self.default_mode = mode; } - if let Some(layout) = v["default_layout"].as_str().and_then(canvas::PageLayout::try_parse) { - self.view_defaults.layout = layout; + if let Some((layout, continuous)) = v["default_layout"].as_str().and_then(canvas::parse_display_token) { + (self.view_defaults.layout, self.view_defaults.continuous) = (layout, continuous); } if let Some(on) = v["highlight_fields"].as_bool() { self.view_defaults.highlight_fields = on; @@ -1789,7 +1792,15 @@ impl PdfCraftApp { } ("page", Some(v)) => v.go_to_page(value.parse::().map_err(|e| e.to_string())?.saturating_sub(1)), ("zoom", Some(v)) => v.set_zoom(value.trim_end_matches('%').parse::().map_err(|e| e.to_string())? / 100.0), - ("layout", Some(v)) => v.set_layout(canvas::PageLayout::try_parse(value).ok_or("layout must be continuous, single or two-up")?), + ("layout", Some(v)) => { + let (layout, continuous) = canvas::parse_display_token(value).ok_or("layout must be single, continuous, two-up or two-page")?; + v.set_display(layout, continuous); + } + ("continuous", Some(v)) => match value { + "on" => v.set_continuous(true), + "off" => v.set_continuous(false), + _ => return Err("continuous must be on or off".into()), + }, ("cover", Some(v)) => { let on = match value { "on" => true, @@ -1802,7 +1813,16 @@ impl PdfCraftApp { v.set_cover(on); } ("default-layout", _) => { - self.view_defaults.layout = canvas::PageLayout::try_parse(value).ok_or("default-layout must be continuous, single or two-up")?; + let (layout, continuous) = + canvas::parse_display_token(value).ok_or("default-layout must be single, continuous, two-up or two-page")?; + (self.view_defaults.layout, self.view_defaults.continuous) = (layout, continuous); + } + ("default-continuous", _) => { + self.view_defaults.continuous = match value { + "on" => true, + "off" => false, + _ => return Err("default-continuous must be on or off".into()), + }; } ("default-zoom", _) => { self.view_defaults = @@ -1926,8 +1946,22 @@ impl PdfCraftApp { self.set_combine_zoom(percent / 100.0); } (k, None) - if ["page", "zoom", "layout", "cover", "organize", "grid-zoom", "fields", "find", "rotate", "select", "notice", "comment"] - .contains(&k) => + if [ + "page", + "zoom", + "layout", + "continuous", + "cover", + "organize", + "grid-zoom", + "fields", + "find", + "rotate", + "select", + "notice", + "comment", + ] + .contains(&k) => { return Err(format!("`{k}` needs an open document")); } diff --git a/crates/ui-egui/tests/view.rs b/crates/ui-egui/tests/view.rs index 12c7e904..e1963d33 100644 --- a/crates/ui-egui/tests/view.rs +++ b/crates/ui-egui/tests/view.rs @@ -157,6 +157,24 @@ fn two_up_layout_places_pages_side_by_side() { assert!(c.min.y > a.max.y); } +#[test] +fn two_page_view_shows_one_spread_at_a_time_until_continuous() { + use pdfcraft_ui_egui::canvas::PageLayout; + // Two-page view without continuous scrolling shows just the current spread; the continuous + // checkbox reveals the rest (#752). + let mut h = harness(&[("layout", "two-page"), ("zoom", "50")]); + h.run_steps(4); + assert_eq!(h.state().views[0].layout, PageLayout::TwoUp); + assert!(!h.state().views[0].continuous(), "`two-page` is the non-scrolling two-page display"); + assert!(rect(&h, 0).is_some() && rect(&h, 1).is_some(), "the first spread is on screen"); + assert!(rect(&h, 2).is_none(), "the next spread is not shown until paged or scrolled"); + // Turning on continuous scrolling lays every spread out in one column. + h.state_mut().set_option("continuous", "on").unwrap(); + h.run_steps(4); + assert!(rect(&h, 2).is_some(), "continuous scrolling reveals the later spreads"); + assert_eq!(h.state().views[0].layout, PageLayout::TwoUp, "the base layout is unchanged"); +} + #[test] fn single_page_layout_shows_one_page_at_a_time() { let mut h = harness(&[("layout", "single"), ("zoom", "50")]); @@ -253,9 +271,15 @@ fn page_display_commands_switch_layouts() { assert!(h.state().views[0].cover, "the cover toggle flips"); assert!(h.state_mut().execute("view.layout.cover")); assert!(!h.state().views[0].cover, "toggling twice restores"); - h.state_mut().execute("view.layout.continuous"); + // Continuous scrolling is an independent toggle over the base layout (#752). + assert!(h.state().views[0].continuous(), "the default view scrolls continuously"); + h.state_mut().execute("view.continuous"); h.run_steps(3); - assert_eq!(h.state().views[0].layout, pdfcraft_ui_egui::canvas::PageLayout::Continuous); + assert!(!h.state().views[0].continuous(), "the checkbox turns continuous scrolling off"); + assert_eq!(h.state().views[0].layout, pdfcraft_ui_egui::canvas::PageLayout::TwoUp, "toggling scrolling leaves the base layout"); + h.state_mut().execute("view.layout.single"); + assert_eq!(h.state().views[0].layout, pdfcraft_ui_egui::canvas::PageLayout::Single); + assert!(!h.state().views[0].continuous(), "switching the base layout leaves scrolling off"); } #[test] @@ -264,7 +288,8 @@ fn layout_option_rejects_typos() { assert!(h.state_mut().set_option("layout", "singel").is_err()); assert!(h.state_mut().set_option("default-layout", "bogus").is_err()); assert!(h.state_mut().set_option("cover", "maybe").is_err()); - assert_eq!(h.state().views[0].layout, pdfcraft_ui_egui::canvas::PageLayout::Continuous); + assert_eq!(h.state().views[0].layout, pdfcraft_ui_egui::canvas::PageLayout::Single); + assert!(h.state().views[0].continuous(), "a rejected typo leaves the default display unchanged"); assert!(h.state_mut().set_option("cover", "on").is_err(), "cover needs two-page view first"); h.state_mut().set_option("layout", "two-up").unwrap(); h.state_mut().set_option("cover", "on").unwrap(); @@ -286,8 +311,9 @@ fn view_options_without_a_document() { #[test] fn default_page_display_is_used_for_new_documents() { use pdfcraft_ui_egui::canvas::PageLayout; - // The factory default is continuous scrolling without snap jumps. - assert_eq!(PdfCraftApp::new().view_defaults.layout, PageLayout::Continuous); + // The factory default is single page, scrolling continuously, without snap jumps. + assert_eq!(PdfCraftApp::new().view_defaults.layout, PageLayout::Single); + assert!(PdfCraftApp::new().view_defaults.continuous); let mut h = harness(&[]); h.state_mut().set_option("default-layout", "single").unwrap(); h.state_mut().open_bytes("other.pdf", None, PAGES.to_vec()).expect("opens"); @@ -309,7 +335,8 @@ fn default_page_display_is_used_for_new_documents() { let mut legacy = PdfCraftApp::new(); legacy.set_option("language", "en").unwrap(); legacy.restore("{}"); - assert_eq!(legacy.view_defaults.layout, PageLayout::Continuous); + assert_eq!(legacy.view_defaults.layout, PageLayout::Single); + assert!(legacy.view_defaults.continuous, "a legacy file without the key keeps the factory default"); } #[test] @@ -367,9 +394,9 @@ fn rail_page_display_menu_offers_acrobats_view_choices() { h.get_by_label(item).click(); h.run_steps(4); }; - let shown = |h: &Harness<'static, PdfCraftApp>| (h.state().views[0].layout, h.state().views[0].fit); + let shown = |h: &Harness<'static, PdfCraftApp>| (h.state().views[0].layout, h.state().views[0].continuous(), h.state().views[0].fit); pick(&mut h, "Fit one full page"); - assert_eq!(shown(&h), (PageLayout::Single, Fit::Page)); + assert_eq!(shown(&h), (PageLayout::Single, false, Fit::Page), "one full page at a time, not scrolling"); // The whole page fits at once, so the wheel turns pages without zooming out first. let at = h.state().views[0].viewport_rect().center(); h.hover_at(at); @@ -382,7 +409,7 @@ fn rail_page_display_menu_offers_acrobats_view_choices() { pick(&mut h, "Zoom to page level"); assert_eq!(h.state().views[0].fit, Fit::Page); pick(&mut h, "Fit to width scrolling"); - assert_eq!(shown(&h), (PageLayout::Continuous, Fit::Width)); + assert_eq!(shown(&h), (PageLayout::Single, true, Fit::Width), "single page, scrolling continuously"); } #[test] diff --git a/crates/ui-egui/tests/viewer.rs b/crates/ui-egui/tests/viewer.rs index 6fdcea47..c1b6ca70 100644 --- a/crates/ui-egui/tests/viewer.rs +++ b/crates/ui-egui/tests/viewer.rs @@ -548,7 +548,8 @@ fn arrow_and_page_keys_move_through_a_scrolling_document() { h.state_mut().active = Some(0); h.state_mut().views[0].fit = Fit::Width; h.run_steps(4); - assert_eq!(h.state().views[0].layout, PageLayout::Continuous); + assert_eq!(h.state().views[0].layout, PageLayout::Single); + assert!(h.state().views[0].continuous(), "the default view scrolls continuously"); let press = |h: &mut Harness<'static, PdfCraftApp>, key, times: usize| { for _ in 0..times { h.key_press(key); @@ -582,6 +583,7 @@ fn arrow_and_page_keys_move_through_a_scrolling_document() { assert_eq!(press(&mut h, Key::ArrowRight, 1), 2); // single-page view keeps Page Down for the next page h.state_mut().views[0].layout = PageLayout::Single; + h.state_mut().views[0].continuous = false; h.run_steps(4); assert_eq!(press(&mut h, Key::PageDown, 1), 3); } @@ -595,6 +597,7 @@ fn up_and_down_turn_pages_in_single_page_view() { let mut h = harness(); h.state_mut().active = Some(0); h.state_mut().views[0].layout = PageLayout::Single; + h.state_mut().views[0].continuous = false; h.state_mut().views[0].fit = Fit::Page; h.run_steps(4); let press = |h: &mut Harness<'static, PdfCraftApp>, key, times: usize| { diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index cc28d6ef..d1f3330b 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -828,7 +828,7 @@ area = "B" tier = "P0" milestone = "M3" status = "shipped" -commands = ["view.layout.continuous"] +commands = ["view.continuous"] tools = [] evidence = ["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"] notes = "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"