mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-11-11 21:32:39 +00:00
fix(edit): refuse before changing anything; cover undo, reopen, re-edit and the split case
An OCR paragraph that may go on top of the page is checked (`check_on_top`) before either rewrite path changes the document, so a page whose marked content nests too deep is refused with nothing changed. `lift_on_top` no longer checks. Tests: undo restores the hidden layer and the scan, redo, save and reopen, and a second edit of the moved text all show the edit; a phrase split from visible text in the same text object shows its edit; a refused rewrite on a too-deep page leaves the saved bytes unchanged. Each new test was seen to fail with its part of the fix reverted. Parity: the note says when the edit is drawn on top and lists what is still missing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SqmHYx8FiWM5PcsoiErgp9
This commit is contained in:
1 parent
cc9d95e1fb
commit
270a11e699
6 files changed
+92
-26
No files matched your search
@@ -367,19 +367,10 @@ pub(crate) fn revealed_under_paint(invisible: bool, ops: &[pdfcraft_content::Op]
|
||||
invisible && ops.get(at..).unwrap_or_default().iter().any(|op| op.is("Do") || op.is("BI") || op.is("sh"))
|
||||
}
|
||||
|
||||
/// The operators to draw on top of `page` ([`draw_on_top`]): all of `paragraph` when `over`,
|
||||
/// after checking the page takes them, so a refusal leaves the document as it was.
|
||||
pub(crate) fn lift_on_top(
|
||||
doc: &Document,
|
||||
page: &pdfcraft_model::Page,
|
||||
over: bool,
|
||||
paragraph: &mut Vec<pdfcraft_content::Op>,
|
||||
) -> Result<Vec<pdfcraft_content::Op>, EditError> {
|
||||
if !over {
|
||||
return Ok(Vec::new());
|
||||
}
|
||||
check_on_top(doc, page)?;
|
||||
Ok(std::mem::take(paragraph))
|
||||
/// The operators to draw on top of the page ([`draw_on_top`]): all of `paragraph` when `over`,
|
||||
/// none otherwise. The page was checked to take them ([`check_on_top`]) before the edit began.
|
||||
pub(crate) fn lift_on_top(over: bool, paragraph: &mut Vec<pdfcraft_content::Op>) -> Vec<pdfcraft_content::Op> {
|
||||
if over { std::mem::take(paragraph) } else { Vec::new() }
|
||||
}
|
||||
|
||||
fn too_deep_on_top() -> EditError {
|
||||
|
||||
@@ -4550,6 +4550,21 @@ fn group_selection_refuses_incomplete_content_and_resource_budget_overruns() {
|
||||
assert!(editable_objects(&doc, 0).is_err(), "an undecodable stream is refused");
|
||||
}
|
||||
|
||||
/// Revealed OCR text may have to go on top of the page; a page whose marked content nests too deep
|
||||
/// for that is refused before either rewrite path changes anything.
|
||||
#[test]
|
||||
fn an_ocr_rewrite_refused_on_top_leaves_the_document_as_it_was() {
|
||||
let content = format!("{} BT /F1 20 Tf 3 Tr 42 44 Td (John) Tj ET /Im Do {}", "/X BMC ".repeat(10_001), "EMC ".repeat(10_001));
|
||||
let mut doc = text_page(&content);
|
||||
assert!(text::text_blocks(&doc, 0).unwrap()[0].invisible);
|
||||
let before = write_full(&doc, &SaveOptions::default()).unwrap();
|
||||
for edit in ["Jane", "Jane\nDoe"] {
|
||||
let error = text::rewrite_block(&mut doc, 0, 0, Some(edit), &text::BlockStyle::default()).unwrap_err();
|
||||
assert!(error.to_string().contains("nested too deeply"), "{edit:?}: {error}");
|
||||
assert!(write_full(&doc, &SaveOptions::default()).unwrap() == before, "{edit:?}: a refused rewrite changes nothing");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn group_move_budget_refusals_keep_the_original_page_selectable() {
|
||||
let text = text_page(&format!("{} BT /F1 12 Tf 40 400 Td (Target) Tj ET", "n ".repeat(99_995)));
|
||||
|
||||
@@ -755,6 +755,11 @@ pub(crate) fn rewrite_block_from(
|
||||
let first = members.first().copied().ok_or_else(|| EditError::Invalid("the paragraph has no source runs".into()))?;
|
||||
members.iter().try_for_each(|line| line.ensure_editable())?;
|
||||
let p = page_dict(doc, page)?;
|
||||
// Revealed OCR text may go on top of the page (see `revealed_under_paint`): a page that can't
|
||||
// take that is refused here, before either rewrite path changes anything.
|
||||
if b.invisible {
|
||||
crate::check_on_top(doc, &p)?;
|
||||
}
|
||||
let mut res = p.dict.get(b"Resources").map(|r| doc.resolve(r)).and_then(|r| r.as_dict().cloned()).unwrap_or_default();
|
||||
let mut fonts_res = res.get(b"Font").map(|f| doc.resolve(f)).and_then(|f| f.as_dict().cloned()).unwrap_or_default();
|
||||
let dispatch = choose_rewrite_path(doc, &fonts_res, &b, &members, first, text, style)?;
|
||||
@@ -884,7 +889,7 @@ pub(crate) fn rewrite_block_from(
|
||||
underline: style.underline == Some(true),
|
||||
};
|
||||
let mut block_ops = block_ops(&plan, &writer, &wrapped, font.show_font.clone())?;
|
||||
let on_top = crate::lift_on_top(doc, &p, over, &mut block_ops)?;
|
||||
let on_top = crate::lift_on_top(over, &mut block_ops);
|
||||
let n = pdfcraft_content::num;
|
||||
block_ops.extend(after.ops().into_iter().filter(|op| !op.is("Tf") || after.font.is_some()));
|
||||
if split {
|
||||
|
||||
@@ -974,7 +974,7 @@ pub(crate) fn rewrite_styled_block(
|
||||
};
|
||||
}
|
||||
block_ops.push(Op::new("Q", vec![]));
|
||||
let on_top = crate::lift_on_top(doc, &p, over, &mut block_ops)?;
|
||||
let on_top = crate::lift_on_top(over, &mut block_ops);
|
||||
block_ops.extend(after.ops());
|
||||
if split {
|
||||
block_ops.insert(0, Op::new("ET", vec![]));
|
||||
|
||||
@@ -211,6 +211,11 @@ fn visible_stretched_text_rewrites_unchanged() {
|
||||
/// A scan whose OCR layer is drawn first and the opaque page image over it, as some OCR tools
|
||||
/// write it (#3973): invisible Helvetica `text` on the scanned word, then the image.
|
||||
fn ocr_under_scan(text: &str) -> std::sync::Arc<Vec<u8>> {
|
||||
under_scan(&format!("BT /F1 20 Tf 3 Tr 42 44 Td ({text}) Tj ET"))
|
||||
}
|
||||
|
||||
/// A page whose `text` content is followed by the opaque 200 × 100 scan image.
|
||||
fn under_scan(text: &str) -> std::sync::Arc<Vec<u8>> {
|
||||
use pdfcraft_cos::{Dict, Document, Object, SaveOptions, Stream};
|
||||
let (w, h) = (200u32, 100u32);
|
||||
let mut rgb = Vec::new();
|
||||
@@ -222,7 +227,7 @@ fn ocr_under_scan(text: &str) -> std::sync::Arc<Vec<u8>> {
|
||||
}
|
||||
let mut d = Document::new_empty();
|
||||
let pages = d.add(Object::Dict(Dict::new()));
|
||||
let content = format!("BT /F1 20 Tf 3 Tr 42 44 Td ({text}) Tj ET q 200 0 0 100 0 0 cm /Im Do Q");
|
||||
let content = format!("{text} q 200 0 0 100 0 0 cm /Im Do Q");
|
||||
let contents = d.add(Object::Stream(Stream::flate(Dict::new(), content.as_bytes())));
|
||||
let mut image = Dict::new();
|
||||
image.set(b"Type".to_vec(), Object::name("XObject"));
|
||||
@@ -264,9 +269,27 @@ fn ocr_under_scan(text: &str) -> std::sync::Arc<Vec<u8>> {
|
||||
std::sync::Arc::new(pdfcraft_cos::write_full(&d, &SaveOptions::default()).unwrap())
|
||||
}
|
||||
|
||||
/// The scanned word's box shows the cover and new ink, not the scan.
|
||||
fn assert_edit_shows(s: &Session, id: pdfcraft_engine::DocId, label: &str) {
|
||||
let px = word_pixels(s, id);
|
||||
let paper = px.iter().filter(|p| near(**p, PAPER)).count();
|
||||
let ink = px.iter().filter(|p| near(**p, INK)).count();
|
||||
assert!(paper * 10 > px.len() * 4, "{label}: the cover is drawn over the scan: {paper} of {} paper", px.len());
|
||||
assert!(ink * 100 > px.len() * 3, "{label}: the new text shows: {ink} of {} ink", px.len());
|
||||
}
|
||||
|
||||
fn texts(s: &Session, id: pdfcraft_engine::DocId) -> Vec<String> {
|
||||
s.get(id).unwrap().text_blocks(0).iter().map(|b| b.text.replace('\n', " ")).collect()
|
||||
}
|
||||
|
||||
fn edit_block(s: &mut Session, id: pdfcraft_engine::DocId, block: usize, text: &str) {
|
||||
s.apply(id, Edit::EditTextBlock { page: 0, block, text: text.into(), style: Default::default() }).unwrap();
|
||||
}
|
||||
|
||||
/// The OCR layer is under the scan image: the rewritten paragraph is drawn on top of the image,
|
||||
/// not in the OCR text's place where the image would hide it (#3973). One line takes the
|
||||
/// single-style rewrite, a typed line break the styled one.
|
||||
/// single-style rewrite, a typed line break the styled one. Undo restores the hidden layer, and
|
||||
/// the edit survives redo, a save and reopen, and being edited again.
|
||||
#[test]
|
||||
fn an_ocr_layer_under_the_scan_image_shows_its_edit() {
|
||||
for edit in ["Jane", "Jane\nDoe"] {
|
||||
@@ -277,14 +300,46 @@ fn an_ocr_layer_under_the_scan_image_shows_its_edit() {
|
||||
assert!(blocks[0].invisible, "the OCR layer is invisible text");
|
||||
assert!(word_pixels(&s, id).iter().all(|p| near(*p, INK)), "the scanned word fills its box");
|
||||
|
||||
s.apply(id, Edit::EditTextBlock { page: 0, block: 0, text: edit.into(), style: Default::default() }).unwrap();
|
||||
edit_block(&mut s, id, 0, edit);
|
||||
assert_eq!(texts(&s, id), [edit.replace('\n', " ")], "{edit:?}");
|
||||
assert_edit_shows(&s, id, edit);
|
||||
|
||||
s.undo(id).unwrap();
|
||||
assert_eq!(texts(&s, id), ["John"], "{edit:?}: undo");
|
||||
assert!(s.get(id).unwrap().text_blocks(0)[0].invisible, "{edit:?}: undo hides the layer again");
|
||||
assert!(word_pixels(&s, id).iter().all(|p| near(*p, INK)), "{edit:?}: undo shows the scan again");
|
||||
s.redo(id).unwrap();
|
||||
assert_edit_shows(&s, id, &format!("{edit:?} redo"));
|
||||
|
||||
let saved = s.save_bytes(id).unwrap();
|
||||
let mut reopened = Session::new();
|
||||
let id = reopened.open("saved.pdf", None, saved, None).unwrap();
|
||||
assert_eq!(texts(&reopened, id), [edit.replace('\n', " ")], "{edit:?}: reopened");
|
||||
assert_edit_shows(&reopened, id, &format!("{edit:?} reopened"));
|
||||
|
||||
edit_block(&mut reopened, id, 0, "Ann");
|
||||
assert_eq!(texts(&reopened, id), ["Ann"], "{edit:?}: edited again");
|
||||
assert_edit_shows(&reopened, id, &format!("{edit:?} edited again"));
|
||||
}
|
||||
}
|
||||
|
||||
/// Visible text earlier in the same text object makes the rewrite split it; the OCR phrase still
|
||||
/// goes on top of the scan, and the visible text stays.
|
||||
#[test]
|
||||
fn an_ocr_phrase_split_from_visible_text_under_the_scan_shows_its_edit() {
|
||||
for edit in ["Jane", "Jane\nDoe"] {
|
||||
let mut s = Session::new();
|
||||
let page = under_scan("BT /F1 20 Tf 10 80 Td (Head) Tj 3 Tr 32 -36 Td (John) Tj ET");
|
||||
let id = s.open("split.pdf", None, page, None).unwrap();
|
||||
let blocks = s.get(id).unwrap().text_blocks(0);
|
||||
assert_eq!(blocks.iter().map(|b| b.text.replace('\n', " ")).collect::<Vec<_>>(), [edit.replace('\n', " ")], "{edit:?}");
|
||||
let px = word_pixels(&s, id);
|
||||
let paper = px.iter().filter(|p| near(**p, PAPER)).count();
|
||||
let ink = px.iter().filter(|p| near(**p, INK)).count();
|
||||
assert!(paper * 10 > px.len() * 4, "{edit:?}: the cover is drawn over the scan: {paper} of {} paper", px.len());
|
||||
assert!(ink * 100 > px.len() * 3, "{edit:?}: the new text shows: {ink} of {} ink", px.len());
|
||||
let john = blocks.iter().position(|b| b.text == "John").unwrap();
|
||||
assert!(blocks[john].invisible && blocks.iter().any(|b| b.text == "Head" && !b.invisible), "{blocks:?}");
|
||||
edit_block(&mut s, id, john, edit);
|
||||
let mut after = texts(&s, id);
|
||||
after.sort();
|
||||
let mut want = vec!["Head".to_string(), edit.replace('\n', " ")];
|
||||
want.sort();
|
||||
assert_eq!(after, want, "{edit:?}");
|
||||
assert_edit_shows(&s, id, &format!("{edit:?} split"));
|
||||
}
|
||||
}
|
||||
@@ -6628,8 +6628,8 @@ milestone = "M10"
|
||||
status = "partial"
|
||||
commands = ["edit.edit_text"]
|
||||
tools = ["text_edit"]
|
||||
evidence = ["crates/engine/tests/it/ocr_edit.rs::editing_an_ocr_word_covers_the_scan_and_shows_the_new_text", "crates/engine/tests/it/ocr_edit.rs::an_explicit_colour_is_kept_for_the_new_text", "crates/engine/tests/it/ocr_edit.rs::ocr_words_group_into_phrases_that_edit_in_place", "crates/engine/tests/it/ocr_edit.rs::a_longer_ocr_phrase_grows_rightwards", "crates/engine/tests/it/ocr_edit.rs::a_rewritten_ocr_phrase_keeps_its_word_widths", "crates/engine/tests/it/ocr_edit.rs::an_ocr_layer_under_the_scan_image_shows_its_edit", "crates/ui-egui/tests/it/ocr.rs::an_ocr_word_opens_for_editing_and_stays_open", "crates/ui-egui/tests/it/ocr.rs::editing_a_scanned_page_recognizes_its_text_first_and_only_once"]
|
||||
notes = "On demand, per edit: in Edit text & images, recognised text (the invisible OCR layer over a scan) is grouped into phrases (words across a word space; a wider gap such as a table cell, and each scanned line, stay separate) and opens for editing on a click. Applying draws the new text visibly in the scan's sampled ink colour (or the chosen one) and first covers the old words with the paper colour sampled around them, so the scanned word no longer shows; the new text is drawn on top of the page, so it shows also when the OCR layer was written under the scan image (#3973); the phrase keeps each word's position and width and grows rightwards instead of wrapping. Turning Edit text & images on for a scanned page (an image covering at least half of a page that has no text of its own, once per page per window; comments and text boxes over it do not count as text, a page with real text and a background picture is left alone; one undoable \"Recognize text\" step) now recognizes its words first, as Acrobat's \"Converting scanned page to Editable Text and Images\" does (English only until more OCR languages exist, gap 7). Still missing: separating the photographs on the page from the lettering, matching the scanned typeface (the layer's Helvetica is used), and covering on a textured or patterned background (one flat colour is used)."
|
||||
evidence = ["crates/engine/tests/it/ocr_edit.rs::editing_an_ocr_word_covers_the_scan_and_shows_the_new_text", "crates/engine/tests/it/ocr_edit.rs::an_explicit_colour_is_kept_for_the_new_text", "crates/engine/tests/it/ocr_edit.rs::ocr_words_group_into_phrases_that_edit_in_place", "crates/engine/tests/it/ocr_edit.rs::a_longer_ocr_phrase_grows_rightwards", "crates/engine/tests/it/ocr_edit.rs::a_rewritten_ocr_phrase_keeps_its_word_widths", "crates/engine/tests/it/ocr_edit.rs::an_ocr_layer_under_the_scan_image_shows_its_edit", "crates/engine/tests/it/ocr_edit.rs::an_ocr_phrase_split_from_visible_text_under_the_scan_shows_its_edit", "crates/ui-egui/tests/it/ocr.rs::an_ocr_word_opens_for_editing_and_stays_open", "crates/ui-egui/tests/it/ocr.rs::editing_a_scanned_page_recognizes_its_text_first_and_only_once"]
|
||||
notes = "On demand, per edit: in Edit text & images, recognised text (the invisible OCR layer over a scan) is grouped into phrases (words across a word space; a wider gap such as a table cell, and each scanned line, stay separate) and opens for editing on a click. Applying draws the new text visibly in the scan's sampled ink colour (or the chosen one) and first covers the old words with the paper colour sampled around them, so the scanned word no longer shows; when an image, inline image or shading is painted after the OCR layer (a layer written under its scan, #3973), the cover and new text are drawn on top of the page instead, so they show; the phrase keeps each word's position and width and grows rightwards instead of wrapping. Turning Edit text & images on for a scanned page (an image covering at least half of a page that has no text of its own, once per page per window; comments and text boxes over it do not count as text, a page with real text and a background picture is left alone; one undoable \"Recognize text\" step) now recognizes its words first, as Acrobat's \"Converting scanned page to Editable Text and Images\" does (English only until more OCR languages exist, gap 7). Still missing: separating the photographs on the page from the lettering, matching the scanned typeface (the layer's Helvetica is used), covering on a textured or patterned background (one flat colour is used), a scan painted by a pattern fill or a filled rectangle over a layer written under it, and, for text drawn on top, the clip, optional content and marked content (tags) that enclosed the OCR layer."
|
||||
|
||||
[[feature]]
|
||||
id = "ocr.ocr-downsample"
|
||||
|
||||
Reference in new issue
Block a user