aleleba/pdfcraft
1
0
Fork 0
mirror of https://github.com/storytold/pdfcraft.git synced 2026-11-02 22:41:47 -06:00
Code Issues Packages Projects Releases Wiki Activity

Re-resolve image selection by XObject and rect across undo/redo

Browse Source 
When undoing or redoing an edit (such as Arrange), re-resolve any active image selection to match the same image's XObject, name, and position rather than leaving the selection stuck on the old index (which could point to another image). If ambiguous or no longer present, clear the selection.
This commit is contained in:
Combiler01
2026-10-25 16:47:24 +05:30
parent fa213cfad0
commit b8b07d0a99
5 changed files with 169 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/ui-egui/src/edit_text_ui.rs
+4 -10
View File
@@ -757,11 +757,9 @@ impl LineEditor {
let spans = block.edited_spans(&self.text);
let matching_lines: Vec<&pdfcraft_engine::TextLine> = match &self.selection {
Some(range) if range.start < range.end => spans
.iter()
.filter(|(span, _)| span.start < range.end && span.end > range.start)
.filter_map(|(_, idx)| lines.get(*idx))
.collect(),
Some(range) if range.start < range.end => {
spans.iter().filter(|(span, _)| span.start < range.end && span.end > range.start).filter_map(|(_, idx)| lines.get(*idx)).collect()
}
_ => spans.iter().filter_map(|(_, idx)| lines.get(*idx)).collect(),
};
@@ -793,11 +791,7 @@ impl LineEditor {
if ["times", "serif", "roman", "courier", "helvetica", "arial"].iter().any(|s| lower.contains(s)) {
return look.family.label().to_string();
}
if name.is_empty() {
tl!("Keep source fonts").to_string()
} else {
name.to_string()
}
if name.is_empty() { tl!("Keep source fonts").to_string() } else { name.to_string() }
};
let first_font = line_font(matching_lines[0]);
crates/ui-egui/src/editing.rs
+62 -37
View File
@@ -187,40 +187,7 @@ impl PdfCraftApp {
}
if let (Some((target, ambiguous_before)), Edit::EditPageImage { page, .. }) = (target_arrange_image, &edit) {
let page_images = doc.page_images(*page);
let exact_matches: Vec<usize> = page_images
.iter()
.enumerate()
.filter(|(_, img)| img.name == target.name && img.object == target.object && img.rect == target.rect)
.map(|(idx, _)| idx)
.collect();
let new_idx = if !ambiguous_before && exact_matches.len() == 1 {
Some(exact_matches[0])
} else if !ambiguous_before && exact_matches.is_empty() {
let name_obj_matches: Vec<usize> = page_images
.iter()
.enumerate()
.filter(|(_, img)| img.name == target.name && img.object == target.object)
.map(|(idx, _)| idx)
.collect();
if name_obj_matches.len() == 1 {
Some(name_obj_matches[0])
} else if name_obj_matches.is_empty() {
let name_matches: Vec<usize> =
page_images.iter().enumerate().filter(|(_, img)| img.name == target.name).map(|(idx, _)| idx).collect();
if name_matches.len() == 1 { Some(name_matches[0]) } else { None }
} else {
None
}
} else {
None
};
if let Some(s) = view.image_selection.as_mut().filter(|s| s.page == *page) {
if let Some(idx) = new_idx {
s.index = idx;
} else {
view.image_selection = None;
}
}
re_resolve_image_selection(view, *page, &target, ambiguous_before, &page_images);
}
if std::mem::take(&mut view.comments.tool_done) && !self.comment_prefs.pinned {
self.quick_tool = crate::QuickTool::Select;
@@ -284,13 +251,33 @@ impl PdfCraftApp {
fn history_step(&mut self, undo: bool) {
let Some((i, id)) = self.active_ids() else { return };
let target_image = self.views.get(i).and_then(|v| v.image_selection.as_ref()).and_then(|s| {
let doc = self.session.get(id)?;
let images = doc.page_images(s.page);
let target = images.get(s.index).cloned()?;
let ambiguous = images.iter().filter(|img| img.name == target.name && img.object == target.object && img.rect == target.rect).count() > 1;
Some((s.page, target, ambiguous))
});
let had_selection = self.views.get(i).and_then(|v| v.image_selection.as_ref()).is_some();
let result = if undo { self.session.undo(id) } else { self.session.redo(id) };
match result {
Ok(label) => {
if let Some(doc) = self.session.get(id) {
self.views[i].document_changed(&doc.info);
if let Some(doc) = self.session.get(id)
&& let Some(view) = self.views.get_mut(i)
{
view.document_changed(&doc.info);
view.picked_image = None;
if let Some((page, target, ambiguous_before)) = target_image {
let page_images = doc.page_images(page);
re_resolve_image_selection(view, page, &target, ambiguous_before, &page_images);
} else if had_selection {
view.image_selection = None;
}
}
if let Some(view) = self.views.get_mut(i) {
view.comments.selected = None;
}
self.views[i].comments.selected = None;
let label = crate::i18n::edit_label(&label);
if undo {
self.notify_fmt("Undid {label}", &[("label", &label)]);
@@ -1019,6 +1006,44 @@ impl PdfCraftApp {
}
}
fn re_resolve_image_selection(
view: &mut crate::DocView,
page: usize,
target: &pdfcraft_engine::PageImage,
ambiguous_before: bool,
page_images: &[pdfcraft_engine::PageImage],
) {
let exact_matches: Vec<usize> = page_images
.iter()
.enumerate()
.filter(|(_, img)| img.name == target.name && img.object == target.object && img.rect == target.rect)
.map(|(idx, _)| idx)
.collect();
let new_idx = if !ambiguous_before && exact_matches.len() == 1 {
exact_matches.first().copied()
} else if !ambiguous_before && exact_matches.is_empty() {
let name_obj_matches: Vec<usize> =
page_images.iter().enumerate().filter(|(_, img)| img.name == target.name && img.object == target.object).map(|(idx, _)| idx).collect();
if name_obj_matches.len() == 1 {
name_obj_matches.first().copied()
} else if name_obj_matches.is_empty() {
let name_matches: Vec<usize> = page_images.iter().enumerate().filter(|(_, img)| img.name == target.name).map(|(idx, _)| idx).collect();
if name_matches.len() == 1 { name_matches.first().copied() } else { None }
} else {
None
}
} else {
None
};
if let Some(s) = view.image_selection.as_mut().filter(|s| s.page == page) {
if let Some(idx) = new_idx {
s.index = idx;
} else {
view.image_selection = None;
}
}
}
/// The single page a content edit changes (`None` for other edits). Like comment edits,
/// these leave every other page's bytes, geometry and text alone, so their rasters stay valid.
fn content_page(edit: &Edit) -> Option<usize> {
crates/ui-egui/src/format_text_ui.rs
+10 -6
View File
@@ -208,12 +208,16 @@ pub(crate) fn panel(ui: &mut egui::Ui, t: &Tokens, look: &mut AddedText, extras:
labelled_icon(ui, t, "arrow-up-down", tl!("Line spacing"));
let own = tl!("Auto").to_string();
let shown = if e.line_spacing == 0.0 { own.clone() } else { number(e.line_spacing) };
let spacing_resp = egui::ComboBox::from_id_salt("format-line-spacing").selected_text(shown).width(half).show_ui(ui, |ui| {
ui.selectable_value(&mut e.line_spacing, 0.0, own.as_str());
for v in LINE_SPACINGS {
ui.selectable_value(&mut e.line_spacing, v, number(v));
}
}).response;
let spacing_resp = egui::ComboBox::from_id_salt("format-line-spacing")
.selected_text(shown)
.width(half)
.show_ui(ui, |ui| {
ui.selectable_value(&mut e.line_spacing, 0.0, own.as_str());
for v in LINE_SPACINGS {
ui.selectable_value(&mut e.line_spacing, v, number(v));
}
})
.response;
spacing_resp.widget_info(|| egui::WidgetInfo::labeled(egui::WidgetType::ComboBox, ui.is_enabled(), tl!("Line spacing")));
ui.add_space(8.0);
ui.add_enabled_ui(false, |ui| {
crates/ui-egui/tests/it/control.rs
-1
View File
@@ -1931,4 +1931,3 @@ fn accessible_labels_on_format_text_and_pin_dialog() {
let char_spin = ok(&mut h, &c, "ui.inspect", json!({"query": "Character spacing", "role": "SpinButton"}));
assert!(char_spin["count"].as_u64().unwrap() >= 1, "character spacing spin button has accessible label: {char_spin}");
}
crates/ui-egui/tests/it/editing.rs
+93 -1
View File
@@ -6898,6 +6898,99 @@ fn arrange_unique_image_preserves_selection() {
assert_eq!(sel.index, 1, "selection index tracks the unique image's new position");
}
#[test]
fn undo_arrange_re_resolves_image_selection_or_clears() {
use pdfcraft_cos::{Dict, Document, Object, SaveOptions, Stream, write_incremental};
let mut doc = Document::open(std::sync::Arc::new(fixture(1))).unwrap();
let page = pdfcraft_model::pages(&doc)[0].clone();
let mut form = Dict::new();
form.set(b"Subtype".to_vec(), Object::name("Form"));
form.set(b"BBox".to_vec(), Object::Array([0, 0, 200, 300].map(Object::Int).to_vec()));
let artwork1 = doc.add(Object::Stream(Stream::flate(form.clone(), b"0.2 0.5 0.9 rg 20 20 160 260 re f")));
let artwork2 = doc.add(Object::Stream(Stream::flate(form, b"0.9 0.2 0.5 rg 40 40 100 100 re f")));
let contents = doc.add(Object::Stream(Stream::flate(Dict::new(), b"q 1 0 0 1 0 0 cm /FigA Do Q q 1 0 0 1 0 0 cm /FigB Do Q")));
doc.update_dict(page.obj, |d| {
d.set(b"Contents".to_vec(), Object::Ref(contents));
let mut xo = Dict::new();
xo.set(b"FigA".to_vec(), Object::Ref(artwork1));
xo.set(b"FigB".to_vec(), Object::Ref(artwork2));
let mut resources = Dict::new();
resources.set(b"XObject".to_vec(), Object::Dict(xo));
d.set(b"Resources".to_vec(), Object::Dict(resources));
})
.unwrap();
let bytes = write_incremental(&doc, &SaveOptions::default()).unwrap();
let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(move |_cc| {
let mut app = PdfCraftApp::new();
app.set_option("language", "en").unwrap();
app.open_bytes("undo_arrange.pdf", None, bytes.clone()).expect("opens");
app
});
h.run_steps(4);
assert!(h.state_mut().execute("edit.edit_text"));
h.run_steps(2);
let doc_id = h.state().views[0].id;
let images = h.state().session.get(doc_id).unwrap().page_images(0);
assert_eq!(images.len(), 2);
assert_eq!(images[0].name, "FigA");
assert_eq!(images[1].name, "FigB");
// Select FigA (back image, index 0)
h.state_mut().views[0].image_selection = Some(pdfcraft_ui_egui::edit_text_ui::ImageSelection {
page: 0,
index: 0,
drag: None,
crop: None,
crop_mode: false,
crop_rect: None,
turning: false,
settled: None,
});
h.run_steps(2);
// Bring to front
let applied = h.state_mut().apply_edit(pdfcraft_engine::Edit::EditPageImage {
page: 0,
index: 0,
change: pdfcraft_engine::ImageEdit::Arrange(pdfcraft_engine::ArrangeOrder::BringToFront),
});
assert!(applied);
h.run_steps(2);
// Selection now tracks FigA at index 1
let sel = h.state().views[0].image_selection.as_ref().expect("selected after arrange");
assert_eq!(sel.index, 1);
// Undo
assert!(h.state_mut().execute("edit.undo"));
h.run_steps(2);
let doc_after = h.state().session.get(doc_id).unwrap();
let images_after = doc_after.page_images(0);
assert_eq!(images_after[0].name, "FigA");
assert_eq!(images_after[1].name, "FigB");
// After undoing Bring to front, selection must either be FigA (index 0) or None, never FigB (index 1)
let sel_after = h.state().views[0].image_selection.as_ref().expect("selection re-resolved to the same image");
assert_eq!(sel_after.index, 0, "selection should point to FigA at index 0, not FigB at index 1");
assert_eq!(images_after[sel_after.index].name, "FigA");
// Redo
assert!(h.state_mut().execute("edit.redo"));
h.run_steps(2);
let doc_redo = h.state().session.get(doc_id).unwrap();
let images_redo = doc_redo.page_images(0);
assert_eq!(images_redo[0].name, "FigB");
assert_eq!(images_redo[1].name, "FigA");
let sel_redo = h.state().views[0].image_selection.as_ref().expect("selection re-resolved after redo");
assert_eq!(sel_redo.index, 1, "selection should point to FigA at index 1");
assert_eq!(images_redo[sel_redo.index].name, "FigA");
}
/// #3061: a paragraph running past its page's bottom edge (an HTML converter relying on the
/// page clip) opens in an editor that draws on its page only. It covered the next page.
#[test]
@@ -7350,4 +7443,3 @@ fn format_panel_shows_selection_size_and_updates_after_font_applied() {
let font_node = h.get_by_role_and_label(Role::ComboBox, "Font");
assert_eq!(font_node.value().as_deref(), Some("Times"), "font combo box updates to Times");
}