diff --git a/crates/ui-egui/src/workflows/ocr_ui.rs b/crates/ui-egui/src/workflows/ocr_ui.rs index 756da56177..bd6e3bb41f 100644 --- a/crates/ui-egui/src/workflows/ocr_ui.rs +++ b/crates/ui-egui/src/workflows/ocr_ui.rs @@ -233,6 +233,12 @@ pub(crate) fn keep_asking_while_probing(ctx: &egui::Context) { } } +/// The dialog's height outside its scrolling settings: the title, a notice of up to two lines, +/// the Cancel / Recognize text row and the modal's own margins. +const OCR_DIALOG_RESERVE: f32 = 220.0; +/// The least the settings scroll in, however short the window. +const OCR_MIN_SCROLL: f32 = 120.0; + pub(crate) fn body(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens) -> (bool, bool) { let pages = app.active_ids().and_then(|(_, id)| app.session.get(id)).map_or(1, |d| d.info.pages.len().max(1)); let d = &mut app.ocr_draft; @@ -248,115 +254,127 @@ pub(crate) fn body(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens) -> (boo }); ui.add_space(8.0); }; - group(ui, "Pages", &mut |ui| { - widgets::radio_value(ui, &mut d.pages, OcrPages::All, tl!("All pages")); - widgets::radio_value(ui, &mut d.pages, OcrPages::Current, tl!("Current page")); - ui.horizontal(|ui| { - widgets::radio_value(ui, &mut d.pages, OcrPages::Range, tl!("From")); - let on = d.pages == OcrPages::Range; - ui.add_enabled(on, egui::DragValue::new(&mut d.from).range(1..=pages)); - ui.label(tl!("to")); - ui.add_enabled(on, egui::DragValue::new(&mut d.to).range(1..=pages)); + // The settings outgrow a short window or a large display scale (#6599): they scroll, and + // the notices and the Cancel / Recognize text row stay below the scroll, always in reach. + let max = (ui.ctx().content_rect().height() - OCR_DIALOG_RESERVE).max(OCR_MIN_SCROLL); + egui::ScrollArea::vertical() + .id_salt("ocr-dialog-scroll") + .max_height(max) + // Only applies when the settings overflow: the dialog then opens at its full height + // instead of growing frame by frame, as Preferences does. + .min_scrolled_height(max) + .auto_shrink([false, true]) + .show(ui, |ui| { + group(ui, "Pages", &mut |ui| { + widgets::radio_value(ui, &mut d.pages, OcrPages::All, tl!("All pages")); + widgets::radio_value(ui, &mut d.pages, OcrPages::Current, tl!("Current page")); + ui.horizontal(|ui| { + widgets::radio_value(ui, &mut d.pages, OcrPages::Range, tl!("From")); + let on = d.pages == OcrPages::Range; + ui.add_enabled(on, egui::DragValue::new(&mut d.from).range(1..=pages)); + ui.label(tl!("to")); + ui.add_enabled(on, egui::DragValue::new(&mut d.to).range(1..=pages)); + }); + }); + group(ui, "Settings", &mut |ui| { + egui::Grid::new("ocr-settings").num_columns(2).spacing([12.0, 8.0]).show(ui, |ui| { + ui.label(tl!("Document language")); + let name = LANGUAGES.iter().find(|l| l.0 == d.language).map_or("English", |l| l.1); + craft_ui::ComboBox::from_id_salt("ocr-language").selected_text(tl!(name)).width(220.0).show_ui(ui, |ui| { + for (code, name) in LANGUAGES { + ui.selectable_value(&mut d.language, (*code).to_string(), tl!(*name)); + } + }); + ui.end_row(); + ui.label(tl!("Engine")); + let engine_name = match d.engine { + EngineChoice::Auto => tl!("Automatic"), + EngineChoice::Ocrs => tl!("Built-in"), + EngineChoice::Tesseract => tl!("Tesseract"), + }; + craft_ui::ComboBox::from_id_salt("ocr-engine").selected_text(engine_name).width(220.0).show_ui(ui, |ui| { + let choices: [(EngineChoice, &str); 3] = + [(EngineChoice::Auto, "Automatic"), (EngineChoice::Ocrs, "Built-in"), (EngineChoice::Tesseract, "Tesseract")]; + for (choice, name) in choices { + ui.selectable_value(&mut d.engine, choice, tl!(name)); + } + }); + ui.end_row(); + ui.label(tl!("Combine engines")); + let strategy_name = match d.strategy { + MergeStrategy::PrimaryOnly => tl!("Primary only"), + MergeStrategy::ConfidenceWeighted => tl!("Confidence weighted"), + MergeStrategy::RoverVote => tl!("ROVER vote"), + }; + craft_ui::ComboBox::from_id_salt("ocr-strategy").selected_text(strategy_name).width(220.0).show_ui(ui, |ui| { + let choices: [(MergeStrategy, &str); 3] = [ + (MergeStrategy::PrimaryOnly, "Primary only"), + (MergeStrategy::ConfidenceWeighted, "Confidence weighted"), + (MergeStrategy::RoverVote, "ROVER vote"), + ]; + for (choice, name) in choices { + ui.selectable_value(&mut d.strategy, choice, tl!(name)); + } + }); + ui.end_row(); + ui.label(tl!("Output")); + let mode_name = match d.output_mode { + OutputMode::Searchable => tl!("Searchable Image (Exact)"), + OutputMode::EditableText => tl!("Editable text"), + }; + craft_ui::ComboBox::from_id_salt("ocr-output").selected_text(mode_name).width(220.0).show_ui(ui, |ui| { + let s = ui + .selectable_label(d.output_mode == OutputMode::Searchable, tl!("Searchable Image (Exact)")) + .on_hover_text(tl!("Adds invisible text over each word; the page image is not changed")); + if s.clicked() { + d.output_mode = OutputMode::Searchable; + } + let e = ui + .selectable_label(d.output_mode == OutputMode::EditableText, tl!("Editable text")) + .on_hover_text(tl!("Writes a new document of visible text; the scan is not changed")); + if e.clicked() { + d.output_mode = OutputMode::EditableText; + } + }); + ui.end_row(); + ui.label(tl!("Downsample to")); + craft_ui::ComboBox::from_id_salt("ocr-dpi").selected_text(format!("{} dpi", d.dpi)).width(220.0).show_ui(ui, |ui| { + for v in [600, 300, 150, 72] { + ui.selectable_value(&mut d.dpi, v, format!("{v} dpi")); + } + }); + ui.end_row(); + }); + ui.add_space(4.0); + ui.add(crate::widgets::checkbox(&mut d.text_pages, tl!("Also recognize pages that already have text"))) + .on_hover_text(tl!("Pictures on those pages are recognized too; words already in the page's text are not added again")); + ui.add(crate::widgets::checkbox(&mut d.auto_rotate, tl!("Straighten pages scanned sideways or upside down"))); + ui.add(crate::widgets::checkbox(&mut d.deskew, tl!("Straighten skewed text lines"))); + ui.add(crate::widgets::checkbox(&mut d.denoise, tl!("Remove scan speckle"))); + ui.add(crate::widgets::checkbox(&mut d.binarize, tl!("Black and white (binarize)"))); + }); + group(ui, "Skip", &mut |ui| { + ui.add(crate::widgets::checkbox(&mut d.skip_text_pages, tl!("Skip pages that already have text"))) + .on_hover_text(tl!("Pages with renderable text are left alone and reported as skipped")); + ui.add(crate::widgets::checkbox(&mut d.skip_text_files, tl!("Skip files that already have text"))) + .on_hover_text(tl!("In multiple files: a file with any text is its own skipped bucket, and nothing is written for it")); + ui.add(crate::widgets::checkbox(&mut d.force_ocr, tl!("Force OCR (override skip options)"))) + .on_hover_text(tl!("Reads pages and files even where the skip options would leave them alone")); + }); + group(ui, "Output folder", &mut |ui| { + ui.horizontal(|ui| match &d.output_folder { + Some(f) => { + ui.monospace(f).on_hover_text(tl!("The searchable copies are written here under the source names")); + if ui.small_button(tl!("Clear")).clicked() { + d.output_folder = None; + } + } + None => { + ui.label(tl!("Automatic")).on_hover_text(tl!("A folder is asked for when a multiple-file run starts")); + } + }); + }); }); - }); - group(ui, "Settings", &mut |ui| { - egui::Grid::new("ocr-settings").num_columns(2).spacing([12.0, 8.0]).show(ui, |ui| { - ui.label(tl!("Document language")); - let name = LANGUAGES.iter().find(|l| l.0 == d.language).map_or("English", |l| l.1); - craft_ui::ComboBox::from_id_salt("ocr-language").selected_text(tl!(name)).width(220.0).show_ui(ui, |ui| { - for (code, name) in LANGUAGES { - ui.selectable_value(&mut d.language, (*code).to_string(), tl!(*name)); - } - }); - ui.end_row(); - ui.label(tl!("Engine")); - let engine_name = match d.engine { - EngineChoice::Auto => tl!("Automatic"), - EngineChoice::Ocrs => tl!("Built-in"), - EngineChoice::Tesseract => tl!("Tesseract"), - }; - craft_ui::ComboBox::from_id_salt("ocr-engine").selected_text(engine_name).width(220.0).show_ui(ui, |ui| { - let choices: [(EngineChoice, &str); 3] = - [(EngineChoice::Auto, "Automatic"), (EngineChoice::Ocrs, "Built-in"), (EngineChoice::Tesseract, "Tesseract")]; - for (choice, name) in choices { - ui.selectable_value(&mut d.engine, choice, tl!(name)); - } - }); - ui.end_row(); - ui.label(tl!("Combine engines")); - let strategy_name = match d.strategy { - MergeStrategy::PrimaryOnly => tl!("Primary only"), - MergeStrategy::ConfidenceWeighted => tl!("Confidence weighted"), - MergeStrategy::RoverVote => tl!("ROVER vote"), - }; - craft_ui::ComboBox::from_id_salt("ocr-strategy").selected_text(strategy_name).width(220.0).show_ui(ui, |ui| { - let choices: [(MergeStrategy, &str); 3] = [ - (MergeStrategy::PrimaryOnly, "Primary only"), - (MergeStrategy::ConfidenceWeighted, "Confidence weighted"), - (MergeStrategy::RoverVote, "ROVER vote"), - ]; - for (choice, name) in choices { - ui.selectable_value(&mut d.strategy, choice, tl!(name)); - } - }); - ui.end_row(); - ui.label(tl!("Output")); - let mode_name = match d.output_mode { - OutputMode::Searchable => tl!("Searchable Image (Exact)"), - OutputMode::EditableText => tl!("Editable text"), - }; - craft_ui::ComboBox::from_id_salt("ocr-output").selected_text(mode_name).width(220.0).show_ui(ui, |ui| { - let s = ui - .selectable_label(d.output_mode == OutputMode::Searchable, tl!("Searchable Image (Exact)")) - .on_hover_text(tl!("Adds invisible text over each word; the page image is not changed")); - if s.clicked() { - d.output_mode = OutputMode::Searchable; - } - let e = ui - .selectable_label(d.output_mode == OutputMode::EditableText, tl!("Editable text")) - .on_hover_text(tl!("Writes a new document of visible text; the scan is not changed")); - if e.clicked() { - d.output_mode = OutputMode::EditableText; - } - }); - ui.end_row(); - ui.label(tl!("Downsample to")); - craft_ui::ComboBox::from_id_salt("ocr-dpi").selected_text(format!("{} dpi", d.dpi)).width(220.0).show_ui(ui, |ui| { - for v in [600, 300, 150, 72] { - ui.selectable_value(&mut d.dpi, v, format!("{v} dpi")); - } - }); - ui.end_row(); - }); - ui.add_space(4.0); - ui.add(crate::widgets::checkbox(&mut d.text_pages, tl!("Also recognize pages that already have text"))) - .on_hover_text(tl!("Pictures on those pages are recognized too; words already in the page's text are not added again")); - ui.add(crate::widgets::checkbox(&mut d.auto_rotate, tl!("Straighten pages scanned sideways or upside down"))); - ui.add(crate::widgets::checkbox(&mut d.deskew, tl!("Straighten skewed text lines"))); - ui.add(crate::widgets::checkbox(&mut d.denoise, tl!("Remove scan speckle"))); - ui.add(crate::widgets::checkbox(&mut d.binarize, tl!("Black and white (binarize)"))); - }); - group(ui, "Skip", &mut |ui| { - ui.add(crate::widgets::checkbox(&mut d.skip_text_pages, tl!("Skip pages that already have text"))) - .on_hover_text(tl!("Pages with renderable text are left alone and reported as skipped")); - ui.add(crate::widgets::checkbox(&mut d.skip_text_files, tl!("Skip files that already have text"))) - .on_hover_text(tl!("In multiple files: a file with any text is its own skipped bucket, and nothing is written for it")); - ui.add(crate::widgets::checkbox(&mut d.force_ocr, tl!("Force OCR (override skip options)"))) - .on_hover_text(tl!("Reads pages and files even where the skip options would leave them alone")); - }); - group(ui, "Output folder", &mut |ui| { - ui.horizontal(|ui| match &d.output_folder { - Some(f) => { - ui.monospace(f).on_hover_text(tl!("The searchable copies are written here under the source names")); - if ui.small_button(tl!("Clear")).clicked() { - d.output_folder = None; - } - } - None => { - ui.label(tl!("Automatic")).on_hover_text(tl!("A folder is asked for when a multiple-file run starts")); - } - }); - }); keep_asking_while_probing(ui.ctx()); // The web build has no model files: it downloads the models the first time they're needed. #[cfg(target_arch = "wasm32")] diff --git a/crates/ui-egui/tests/it/ocr.rs b/crates/ui-egui/tests/it/ocr.rs index ec6af6d7de..e94bc8dbae 100644 --- a/crates/ui-egui/tests/it/ocr.rs +++ b/crates/ui-egui/tests/it/ocr.rs @@ -465,3 +465,34 @@ fn ocr_verify_screen_focus_ring_and_empty_walk() { assert!(moved, "Ctrl+Tab moves the focus"); assert_eq!(h.state().ocr_verify.focus, start, "the ring returns after the four panes"); } + +/// #6599: in a short window (a small screen, or a large display scale) the dialog's settings +/// scroll, and Cancel and Recognize text stay on screen below them instead of running off it. +#[test] +fn recognize_text_keeps_its_buttons_in_a_short_window() { + use egui::accesskit::Role; + for height in [400.0, 480.0, 560.0, 700.0] { + let mut h = Harness::builder().with_size(egui::vec2(1000.0, height)).build_eframe(|_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("scan.pdf", None, scan()).unwrap(); + app + }); + h.run_steps(4); + assert!(h.state_mut().execute("ocr.recognize")); + h.run_steps(6); + let on_screen = |r: egui::Rect| r.min.y >= 0.0 && r.max.y <= height; + let title = h.get_by(|n| n.role() == Role::Label && n.value().as_deref() == Some("Recognize Text")).rect(); + let run = h.get_by_role_and_label(Role::Button, "Recognize text").rect(); + let cancel = h.get_by_role_and_label(Role::Button, "Cancel").rect(); + assert!(on_screen(title), "{height}: the title at {title:?} is off the window"); + assert!(on_screen(run), "{height}: Recognize text at {run:?} is off the window"); + assert!(on_screen(cancel), "{height}: Cancel at {cancel:?} is off the window"); + // The last group scrolls in between the title and the buttons, which do not move. + h.get_by(|n| n.role() == Role::Label && n.value().as_deref() == Some("Output folder")).scroll_to_me(); + h.run_steps(4); + let last = h.get_by(|n| n.role() == Role::Label && n.value().as_deref() == Some("Output folder")).rect(); + assert!(last.min.y >= title.max.y && last.max.y <= run.min.y, "{height}: Output folder {last:?} between {title:?} and {run:?}"); + assert_eq!(h.get_by_role_and_label(Role::Button, "Recognize text").rect(), run, "{height}: the buttons stay put"); + } +}