M14: Reuse form enumeration during document open (#1011)

Co-authored-by: Brandon Thomas <[email protected]>
This commit is contained in:
cyfung1031andBrandon Thomas authored and GitHub committed 2026-10-11 12:21:02 -04:00
1 parent 3b7a01eac5
commit db26eb266d
7 files changed
+86 -10

No files matched your search

+1
View File
@@ -146,6 +146,7 @@ Newest first: the date, what moved, and the new overall estimate.
- **2026-10-11 (M13):** CLI run and MCP reject missing, empty, repeated and option-valued --root arguments before any tool/script execution or MCP initialization. A single-hyphen directory name remains valid; synthetic CLI regressions preserve a would-be overwritten file. Estimates unchanged.
- **2026-10-11 (M14, #307, Plan 2):** Forms now return terminal fields and the page-widget adoption count from one enumeration, and engine open reuses that result while retaining XFA script-button marking. The 12,000-page/no-form PDF (1,442,052 bytes) and 2,500-orphan-widget PDF (673,432 bytes) produce byte-identical `doc_open`/`doc_info` outputs against upstream `33c14e07cfd742a82bf03baa6b103c01008fa68c`, including the orphan repair note. After one warm-up per binary and three interleaved base/candidate rounds, median open times were 0.16/0.15 s and 0.08/0.07 s, with median peak RSS 62.69/59.98 MiB and 33.28/32.20 MiB. Workspace tests, Clippy, formatting, focused forms/engine/organize tests, and the WASM check pass. A behavior-preserving local binding in `organize` also lets the workspace Clippy gate pass. No estimate change.
- **2026-10-10 (M14, interface size):** Preferences ▸ Interface size scales the whole interface (50–250 %, setting `ui_scale`, control option `ui-scale`); 50 % brings an interface the system draws at 200 % back to 100 % (#456). Auto, the default, follows the desktop's text scaling: on Linux and the BSDs the desktop app reads GNOME's `text-scaling-factor` once at launch, so with Large Text (1.25) on a 1× display the interface no longer draws 13 px labels that show their pixels next to the system's text. The size is egui's zoom factor, so `pixels_per_point` grows with it and page rasters stay one texel per screen pixel; egui's own ⌘+/⌘− interface zoom is turned off so those keys stay page zoom. Still missing: KDE's font DPI and Windows' "Make text bigger" aren't read (both scale only text there), and egui's text is still greyscale-antialiased without the system's gamma, so small text looks rougher than GTK's at 100 %.
- **2026-10-10 (M14, #496):** Shortcut displays share a UI formatter across toolbar search, registry menus, command palette, Help and tooltips. Modifier and key names use a dedicated catalog context in every bundled translation; missing entries keep the English key name rather than a command translation. German shows Strg/Umschalt/Entf, macOS retains its symbols, and runtime language changes refresh the display. Regression coverage checks localized layout widths, English/German switching, platform conventions and unchanged command execution. Milestone and effort estimates unchanged.
+8 -6
View File
@@ -2390,12 +2390,14 @@ impl Session {
};
let display = display_bytes(editor.as_ref(), &bytes);
let renderer = RenderPool::new(display.clone(), render_threads(), config.clone());
let mut form = editor.as_ref().map(|e| pdfcraft_forms::fields(&e.cos)).unwrap_or_default();
let mut adopted = 0;
if let Some(e) = editor.as_ref() {
xfa::mark_script_buttons(&e.cos, &mut form);
adopted = pdfcraft_forms::adopted_page_fields(&e.cos);
}
let (form, adopted) = editor
.as_ref()
.map(|e| {
let mut fields = pdfcraft_forms::fields_with_adopted(&e.cos);
xfa::mark_script_buttons(&e.cos, &mut fields.items);
(fields.items, fields.adopted_page_fields)
})
.unwrap_or_default();
// Fields the file lists only as page widgets are adopted leniently; the file says so.
if adopted > 0
&& let Some(e) = &mut editor
+3 -1
View File
@@ -2400,6 +2400,8 @@ fn xfa_scripts_initialize_calculate_validate_toggle_and_add_rows() {
let bytes = Arc::new(pdfcraft_xfa::fixtures::shell(&pdfcraft_xfa::fixtures::scripted_template()));
let mut s = Session::new().with_clock(|| 1_700_000_000);
let id = s.open("scripted.pdf", None, bytes, None).expect("opens");
let add_row = s.get(id).unwrap().form.iter().find(|f| f.name == "addRow").expect("the XFA button is in the form snapshot");
assert!(matches!(add_row.button.as_ref(), Some(pdfcraft_forms::af::ButtonAction::Script(_))));
let value = |s: &Session, n: &str| {
s.get(id).unwrap().form.iter().find(|f| f.name == n).map(|f| f.value.clone()).unwrap_or_else(|| panic!("no field {n}"))
};
@@ -2828,7 +2830,7 @@ fn adopting_page_only_fields_is_noted_as_a_repair() {
let doc = s.get(id).unwrap();
let names: Vec<&str> = doc.form.iter().map(|f| f.name.as_str()).collect();
assert_eq!(names, ["alpha", "beta"]);
assert!(doc.repair_log().iter().any(|l| l.contains("2 fields") && l.contains("page annotations")), "{:?}", doc.repair_log());
assert_eq!(doc.repair_log(), vec!["the form's /Fields list omits 2 fields; they were read from the page annotations".to_string()]);
}
/// A page too large for the renderer at the asked resolution exports at the most it allows (and
+3
View File
@@ -7,6 +7,9 @@ Interactive forms (AcroForm, ISO 32000-2 §12.7). Layer L3; depends on `pdfcraft
```rust
let all: Vec<Field> = fields(&doc); // terminal fields with widgets, options, flags, /DA
let snapshot = fields_with_adopted(&doc); // fields and page-only adoption count in one pass
let all = snapshot.items;
let adopted = snapshot.adopted_page_fields;
set_value(&mut doc, "name", &FieldValue::Text("Ada".into()))?;
set_value(&mut doc, "agree", &FieldValue::Check(true))?;
set_value(&mut doc, "size", &FieldValue::Radio(Some("L".into())))?;
+17 -2
View File
@@ -487,13 +487,28 @@ struct Inherited {
/// Every terminal field: the `/Fields` tree in order, then the fields reachable only through the
/// page annotations (see [`adopt_page_fields`]).
pub fn fields(doc: &Document) -> Vec<Field> {
enumerate(doc).0
fields_with_adopted(doc).items
}
/// How many of [`fields`] were reachable only through the page annotations: the form's `/Fields`
/// list names none or only some of them, so the leniency that adopts them is worth recording.
pub fn adopted_page_fields(doc: &Document) -> usize {
enumerate(doc).1
fields_with_adopted(doc).adopted_page_fields
}
/// The terminal fields and page-only fields adopted while reading a form.
#[derive(Clone, Debug, PartialEq)]
pub struct Fields {
/// Terminal fields in tree order, followed by adopted page-only fields.
pub items: Vec<Field>,
/// How many fields were reachable only through page annotations.
pub adopted_page_fields: usize,
}
/// Read all terminal fields and the page-only adoption count in one enumeration.
pub fn fields_with_adopted(doc: &Document) -> Fields {
let (items, adopted_page_fields) = enumerate(doc);
Fields { items, adopted_page_fields }
}
fn enumerate(doc: &Document) -> (Vec<Field>, usize) {
+52
View File
@@ -62,6 +62,20 @@ fn reopen(doc: &Document) -> Document {
Document::open(Arc::new(bytes)).expect("reopens")
}
fn legacy_two_call_enumeration(doc: &Document) -> (Vec<Field>, usize) {
(enumerate(doc).0, enumerate(doc).1)
}
fn assert_combined_matches_legacy(doc: &Document, adopted: usize) {
let (old_fields, old_adopted) = legacy_two_call_enumeration(doc);
let combined = fields_with_adopted(doc);
assert_eq!(combined.items, old_fields);
assert_eq!(combined.adopted_page_fields, old_adopted);
assert_eq!(combined.adopted_page_fields, adopted);
assert_eq!(fields(doc), old_fields, "the compatibility wrapper keeps the old fields");
assert_eq!(adopted_page_fields(doc), old_adopted, "the compatibility wrapper keeps the old count");
}
fn field<'a>(all: &'a [Field], name: &str) -> &'a Field {
all.iter().find(|f| f.name == name).unwrap_or_else(|| panic!("no field {name}"))
}
@@ -94,6 +108,7 @@ fn appearance_text(doc: &Document, w: &Widget, font_name: &[u8]) -> (Object, Vec
#[test]
fn the_field_tree_is_read_with_inheritance() {
let doc = fixture();
assert_combined_matches_legacy(&doc, 0);
let all = fields(&doc);
let names: Vec<&str> = all.iter().map(|f| f.name.as_str()).collect();
assert_eq!(names, ["name", "notes", "zip", "pin", "id", "address.city", "agree", "size", "country", "toppings", "go", "bare"]);
@@ -158,6 +173,7 @@ fn fields_listed_only_on_the_pages_are_adopted() {
"<< /Type /Annot /Subtype /Widget /Rect [400 700 415 715] /P 3 0 R >>".into(), // 15 no /FT, /T or /Parent: not a field
];
let mut doc = document(&objs);
assert_combined_matches_legacy(&doc, 3);
let all = fields(&doc);
let names: Vec<&str> = all.iter().map(|f| f.name.as_str()).collect();
assert_eq!(names, ["listed", "alpha", "beta", "pair"]);
@@ -175,6 +191,7 @@ fn fields_listed_only_on_the_pages_are_adopted() {
let mut objs = objs;
objs[3] = "<< /DA (/Helv 0 Tf 0 g) /DR << /Font << /Helv 6 0 R >> >> >>".into();
let doc = document(&objs);
assert_combined_matches_legacy(&doc, 4);
let all = fields(&doc);
let names: Vec<&str> = all.iter().map(|f| f.name.as_str()).collect();
assert_eq!(names, ["listed", "alpha", "beta", "pair"]);
@@ -499,10 +516,44 @@ fn reset_restores_defaults() {
#[test]
fn documents_without_forms_say_so() {
let mut doc = Document::new_empty();
assert_combined_matches_legacy(&doc, 0);
assert!(fields(&doc).is_empty());
assert_eq!(set_value(&mut doc, "x", &FieldValue::Text("y".into())), Err(FormError::NoForm));
}
#[test]
fn combined_enumeration_keeps_page_order_across_pages() {
let objs: Vec<String> = vec![
"<< /Type /Catalog /Pages 2 0 R /AcroForm 5 0 R >>".into(),
"<< /Type /Pages /Kids [3 0 R 4 0 R] /Count 2 /MediaBox [0 0 600 800] >>".into(),
"<< /Type /Page /Parent 2 0 R /Annots [6 0 R] >>".into(),
"<< /Type /Page /Parent 2 0 R /Annots [7 0 R] >>".into(),
"<< /Fields [6 0 R] >>".into(),
"<< /Type /Annot /Subtype /Widget /FT /Tx /T (listed) /Rect [50 700 250 720] /P 3 0 R >>".into(),
"<< /Type /Annot /Subtype /Widget /FT /Tx /T (adopted) /Rect [50 700 250 720] /P 4 0 R >>".into(),
];
let doc = document(&objs);
assert_combined_matches_legacy(&doc, 1);
let combined = fields_with_adopted(&doc);
assert_eq!(combined.items.iter().map(|f| f.name.as_str()).collect::<Vec<_>>(), ["listed", "adopted"]);
assert_eq!(combined.items.iter().map(|f| f.widgets[0].page).collect::<Vec<_>>(), [Some(0), Some(1)]);
}
#[test]
fn combined_enumeration_keeps_leniency_for_dangling_field_entries() {
let objs: Vec<String> = vec![
"<< /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >>".into(),
"<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 600 800] >>".into(),
"<< /Type /Page /Parent 2 0 R /Annots [] >>".into(),
"<< /Fields [99 0 R null 5 0 R] >>".into(),
"<< /FT /Tx /T (valid) >>".into(),
];
let doc = document(&objs);
assert_combined_matches_legacy(&doc, 0);
assert_eq!(fields_with_adopted(&doc).items.iter().map(|f| f.name.as_str()).collect::<Vec<_>>(), ["valid"]);
}
#[test]
fn every_field_type_can_be_added_named_and_drawn() {
let mut doc = Document::new_empty();
@@ -1010,6 +1061,7 @@ fn field_actions_round_trip_on_every_trigger() {
assert_eq!(field_actions(&doc, &name).unwrap(), [(Trigger::MouseExit, FieldAction::Named("NextPage".into()))]);
// The Mouse Up script is what the button runs.
set_field_actions(&mut doc, &name, &[(Trigger::MouseUp, FieldAction::JavaScript("this.print();".into()))]).unwrap();
assert_combined_matches_legacy(&doc, 0);
assert_eq!(fields(&doc).iter().find(|f| f.name == "go").unwrap().button, Some(af::ButtonAction::Named("Print".into())));
}
+2 -1
View File
@@ -274,7 +274,8 @@ fn drop_named_destinations(doc: &mut Document, gone: &[ObjRef]) -> Result<Vec<Ve
}
}
Some(Object::Dict(mut d)) => {
if prune_legacy(doc, &mut d, gone, &mut dead) {
let changed = prune_legacy(doc, &mut d, gone, &mut dead);
if changed {
doc.update_dict(root, |c| c.set(b"Dests".to_vec(), Object::Dict(d)))?;
}
}