Fix viewport jitter when annotating with search active (#1254) (#1283)

When an annotation was added or edited, page_changed cleared search matches for the page and reset f.current to None. When the background text reload finished, refresh_find_page saw f.current.is_none() and invoked flash_match(), setting view.goto and causing the canvas viewport to jump after every drawn element.

- Preserve search state in page_changed instead of prematurely clearing matches and current match index.
- In refresh_find_page, gate flash_match() so it only jumps when no match was previously selected (e.g. initial search query), avoiding viewport jumps during background text reloads.
- Add regression test marking_while_searching_does_not_jitter_or_reset_search_state.

Fixes #1254
This commit is contained in:
Omar Elders 2026-10-12 08:29:39 +03:00 committed by GitHub
parent 05a862995d
commit 31938de20a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 66 additions and 9 deletions

View File

@ -840,10 +840,6 @@ impl DocView {
if self.selection.is_some_and(|s| s.page == page) {
self.selection = None;
}
if let Some(f) = self.find.as_mut() {
f.matches.retain(|(p, _)| *p != page);
f.current = None;
}
}
/// Pages drawn last frame and their screen rectangles.
@ -916,17 +912,16 @@ impl DocView {
f.matches.retain(|(p, _)| *p != page);
f.matches.extend(text.find_opts(&f.case_query, f.case_sensitive, f.whole_words).into_iter().map(|r| (page, r)));
f.matches.sort_by_key(|(p, r)| (*p, r.start));
f.current = match current_key {
Some(k) => f.matches.iter().position(|m| *m == k),
None => None,
};
f.current = current_key.as_ref().and_then(|k| f.matches.iter().position(|m| m == k));
if f.current.is_none() && !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);
if current_key.is_none() {
self.flash_match(p);
}
}
}
@ -1548,6 +1543,11 @@ impl DocView {
None => {
log::warn!("page {} text: {}", r.request.page + 1, r.error.unwrap_or_default());
self.text_failed.insert(r.request.page);
if let Some(f) = self.find.as_mut() {
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));
}
}
}
return 0;

View File

@ -169,6 +169,63 @@ fn find_shortcuts_advance_and_retreat_matches() {
assert_eq!(h.state().views[0].find.as_ref().unwrap().current, Some((start + n - 1) % n));
}
#[test]
fn marking_while_searching_does_not_jitter_or_reset_search_state() {
let mut h = harness();
h.state_mut().active = Some(0);
h.run_steps(2);
h.state_mut().views[0].open_find();
h.state_mut().views[0].find.as_mut().unwrap().query = "page".into();
h.state_mut().views[0].rerun_find();
let deadline = std::time::Instant::now() + std::time::Duration::from_secs(20);
while std::time::Instant::now() < deadline {
h.run_steps(2);
std::thread::sleep(std::time::Duration::from_millis(5));
if h.state().views[0].find.as_ref().unwrap().matches.len() >= 15 {
break;
}
}
assert_eq!(h.state().views[0].find.as_ref().unwrap().matches.len(), 15);
let current_before = h.state().views[0].find.as_ref().unwrap().current;
assert!(current_before.is_some());
// Clear any pending goto from initial find
h.run_steps(2);
assert!(h.state().views[0].goto.is_none());
// User draws or marks an annotation on page 0
let shape = pdfcraft_engine::Shape::Ink {
strokes: vec![vec![[10.0, 10.0], [20.0, 20.0]]],
};
let stroke = pdfcraft_engine::Edit::AddAnnotation(pdfcraft_engine::NewAnnotation {
page: 0,
style: pdfcraft_engine::Style::default_for(&shape),
shape,
contents: String::new(),
author: "tester".into(),
});
h.state_mut().apply_edit(stroke);
// Right after applying the edit, search matches and current match must still be valid
assert_eq!(
h.state().views[0].find.as_ref().unwrap().current,
current_before,
"applying an annotation must not wipe current search match"
);
// Let the canvas process the reload
let reload_deadline = std::time::Instant::now() + std::time::Duration::from_secs(5);
while std::time::Instant::now() < reload_deadline {
h.run_steps(2);
std::thread::sleep(std::time::Duration::from_millis(5));
assert!(h.state().views[0].goto.is_none(), "page reload after annotation must not set goto / trigger display jitter");
if h.state().views[0].glyph_screen_pos(0, 0).is_some() {
break;
}
}
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);
}
#[test]
fn layouts_fit_height_labels_and_system_theme() {
use pdfcraft_ui_egui::canvas::{Fit, PageLayout};