Refine search match selection and text failure jitter handling (#1291)
* Fix search match selection order and text reload failure recovery 1. rerun_find: Accumulate and sort matches across all loaded pages and pick the first match on or after the currently viewed page before flashing once, avoiding arbitrary viewport jumps to page 0 when viewing subsequent pages. 2. receive_result: Gracefully fall back to the next match at or after the viewed page without flashing when page text extraction fails, avoiding latent None state that caused viewport jitter on subsequent page reloads. 3. Add tests verifying match selection on viewed page, continuous multi-page drawing with active search, and text failure recovery. * rustfmt Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Clippy: RenderRequest is Copy Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Brandon Thomas <echelon@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Omar Elders
GitHub
parent
f81e23bd4e
commit
d57f740003
|
|
@ -898,9 +898,24 @@ impl DocView {
|
|||
f.case_query = f.query.clone();
|
||||
f.matches.clear();
|
||||
f.current = None;
|
||||
let pages: Vec<usize> = self.texts.keys().copied().collect();
|
||||
for p in pages {
|
||||
self.refresh_find_page(p);
|
||||
if f.case_query.trim().is_empty() {
|
||||
return;
|
||||
}
|
||||
let mut pages: Vec<usize> = self.texts.keys().copied().collect();
|
||||
pages.sort();
|
||||
for &p in &pages {
|
||||
if let Some(text) = self.texts.get(&p) {
|
||||
f.matches.extend(text.find_opts(&f.case_query, f.case_sensitive, f.whole_words).into_iter().map(|r| (p, r)));
|
||||
}
|
||||
}
|
||||
f.matches.sort_by_key(|(p, r)| (*p, r.start));
|
||||
if !f.matches.is_empty() {
|
||||
// First match at or after the page being viewed.
|
||||
let cur = self.current;
|
||||
let i = f.matches.iter().position(|(p, _)| *p >= cur).unwrap_or(0);
|
||||
f.current = Some(i);
|
||||
let (p, _) = f.matches[i];
|
||||
self.flash_match(p);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -1548,6 +1563,10 @@ impl DocView {
|
|||
let current_key = f.current.and_then(|c| f.matches.get(c).cloned());
|
||||
f.matches.retain(|(p, _)| *p != r.request.page);
|
||||
f.current = current_key.and_then(|k| f.matches.iter().position(|m| *m == k));
|
||||
if f.current.is_none() && !f.matches.is_empty() {
|
||||
let cur = self.current;
|
||||
f.current = Some(f.matches.iter().position(|(p, _)| *p >= cur).unwrap_or(0));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -5184,4 +5203,37 @@ trailer << /Root 1 0 R >>
|
|||
v.go_to_dest(1, DestView::FitH { top: Some(100.0) }, &empty);
|
||||
assert_eq!((v.current, v.goto_point), (1, None));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn page_text_failure_preserves_valid_search_match_without_jitter() {
|
||||
let mut v = view(3, PageLayout::Single, true);
|
||||
v.current = 1;
|
||||
v.find = Some(Find {
|
||||
query: "test".into(),
|
||||
case_query: "test".into(),
|
||||
matches: vec![(0, 0..4), (1, 5..9)],
|
||||
current: Some(0),
|
||||
..Default::default()
|
||||
});
|
||||
let req = RenderRequest { page: 0, kind: RequestKind::Text, ..Default::default() };
|
||||
v.last_queue = vec![req];
|
||||
let ctx = egui::Context::default();
|
||||
v.receive_result(
|
||||
&ctx,
|
||||
pdfcraft_render::RenderedPage {
|
||||
request: req,
|
||||
width: 0,
|
||||
height: 0,
|
||||
rgba: Default::default(),
|
||||
error: Some("extraction failed".into()),
|
||||
text: None,
|
||||
warnings: Vec::new(),
|
||||
millis: 0,
|
||||
},
|
||||
);
|
||||
// Active match was on page 0 which failed: current should fall back to match on page 1 without jitter
|
||||
assert_eq!(v.find.as_ref().unwrap().current, Some(0));
|
||||
assert_eq!(v.find.as_ref().unwrap().matches, vec![(1, 5..9)]);
|
||||
assert!(v.goto.is_none());
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -220,6 +220,38 @@ fn marking_while_searching_does_not_jitter_or_reset_search_state() {
|
|||
}
|
||||
assert_eq!(h.state().views[0].find.as_ref().unwrap().current, current_before);
|
||||
assert_eq!(h.state().views[0].find.as_ref().unwrap().matches.len(), 15);
|
||||
|
||||
// If user is viewing page 3 and reruns find, the selected match must be on page 3 (index 9), not page 0
|
||||
h.state_mut().views[0].current = 3;
|
||||
h.state_mut().views[0].rerun_find();
|
||||
let current_p3 = h.state().views[0].find.as_ref().unwrap().current;
|
||||
assert_eq!(current_p3, Some(9), "rerun_find must select the first match at or after the viewed page (page 3), but selected {:?}", current_p3);
|
||||
|
||||
// Continuous drawing across multiple pages must never trigger viewport jumps or wipe search state
|
||||
h.run_steps(2);
|
||||
h.state_mut().views[0].goto = None;
|
||||
for p in 1..=4 {
|
||||
let shape = pdfcraft_engine::Shape::Ink { strokes: vec![vec![[15.0 * p as f64, 20.0], [30.0 * p as f64, 40.0]]] };
|
||||
let stroke = pdfcraft_engine::Edit::AddAnnotation(pdfcraft_engine::NewAnnotation {
|
||||
page: p,
|
||||
style: pdfcraft_engine::Style::default_for(&shape),
|
||||
shape,
|
||||
contents: String::new(),
|
||||
author: "tester".into(),
|
||||
});
|
||||
h.state_mut().apply_edit(stroke);
|
||||
h.run_steps(2);
|
||||
assert!(h.state().views[0].goto.is_none(), "drawing on page {p} must not set goto");
|
||||
assert_eq!(h.state().views[0].find.as_ref().unwrap().current, Some(9));
|
||||
}
|
||||
let multi_deadline = std::time::Instant::now() + std::time::Duration::from_secs(5);
|
||||
while std::time::Instant::now() < multi_deadline {
|
||||
h.run_steps(2);
|
||||
std::thread::sleep(std::time::Duration::from_millis(5));
|
||||
assert!(h.state().views[0].goto.is_none(), "multi-page reload after annotations must not set goto");
|
||||
}
|
||||
assert_eq!(h.state().views[0].find.as_ref().unwrap().current, Some(9));
|
||||
assert_eq!(h.state().views[0].find.as_ref().unwrap().matches.len(), 15);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
|
|||
Loading…
Reference in New Issue