From 0668d3e3d504da2ebe26986e8dd5c0d6b5507da8 Mon Sep 17 00:00:00 2001 From: sanyamsingh001 <314051937+sanyamsingh001@users.noreply.github.com> Date: Fri, 9 Oct 2026 13:31:49 +1000 Subject: [PATCH] Preserve alternate appearances and indirect push-button values Keep rollover, down and custom appearance entries when normal appearances are regenerated, without modifying shared dictionaries. Resolve indirect button caption and layout values. Cover full and incremental saved output, and wait for raster readiness in the existing Linux scroll test. Co-authored-by: Codex --- crates/annot/src/lib.rs | 2 +- crates/annot/src/tests.rs | 67 +++++++++++++- crates/forms/src/author.rs | 10 +- crates/forms/src/lib.rs | 7 +- crates/forms/src/tests.rs | 172 +++++++++++++++++++++++++++++++++++ crates/ui-egui/tests/view.rs | 24 ++++- 6 files changed, 274 insertions(+), 8 deletions(-) diff --git a/crates/annot/src/lib.rs b/crates/annot/src/lib.rs index 2abfdce553..9b93da3d6c 100644 --- a/crates/annot/src/lib.rs +++ b/crates/annot/src/lib.rs @@ -1085,7 +1085,7 @@ pub fn set_appearance(doc: &mut Document, r: ObjRef) -> Result<(), AnnotError> { let subtype = String::from_utf8_lossy(d.name(b"Subtype").unwrap_or_default()).into_owned(); let Some(stream) = appearance::build(&d) else { return Err(AnnotError::Unsupported(subtype)) }; let ap = doc.add(Object::Stream(stream)); - let mut apd = Dict::new(); + let mut apd = d.get(b"AP").map(|a| doc.resolve(a)).and_then(|a| a.as_dict().cloned()).unwrap_or_default(); apd.set(b"N".to_vec(), Object::Ref(ap)); doc.update_dict(r, |d| { d.set(b"AP".to_vec(), Object::Dict(apd)); diff --git a/crates/annot/src/tests.rs b/crates/annot/src/tests.rs index 3664e7cabe..cb1b193a78 100644 --- a/crates/annot/src/tests.rs +++ b/crates/annot/src/tests.rs @@ -60,7 +60,7 @@ fn list(doc: &Document, page: usize) -> Vec { } fn ap_content(doc: &Document, d: &Dict) -> String { - let n = d.get(b"AP").and_then(|a| a.as_dict()).and_then(|a| a.reference(b"N")).expect("has /AP /N"); + let n = d.get(b"AP").map(|a| doc.resolve(a)).and_then(|a| a.as_dict().and_then(|a| a.reference(b"N"))).expect("has /AP /N"); let obj = doc.get(n); let Object::Stream(s) = &*obj else { panic!("AP is not a stream") }; String::from_utf8_lossy(&s.decoded().unwrap()).into_owned() @@ -735,3 +735,68 @@ fn comments_without_appearances_get_one_for_display_only() { ]); assert!(with_missing_appearances(&complete).is_none()); } + +#[test] +fn redraw_preserves_alternate_and_shared_annotation_appearance_entries() { + let alternates: [(&[u8], &[u8]); 2] = [(b"R", b"0 0 20 10 re f\n"), (b"D", b"1 1 18 8 re S\n")]; + for indirect in [false, true] { + let mut doc = fixture(); + let first = add_annotation(&mut doc, &new(0, Shape::Rectangle { rect: [10.0, 10.0, 110.0, 60.0] }), &meta("first")).unwrap(); + let other = add_annotation(&mut doc, &new(0, Shape::Rectangle { rect: [120.0, 10.0, 220.0, 60.0] }), &meta("other")).unwrap(); + let (_, first_ref) = annot_ref(&mut doc, 0, first).unwrap(); + let (_, other_ref) = annot_ref(&mut doc, 0, other).unwrap(); + let mut entries = annot_dict(&doc, first_ref).get(b"AP").unwrap().as_dict().unwrap().clone(); + for (key, bytes) in alternates { + let mut d = Dict::new(); + d.set(b"Type".to_vec(), Object::name("XObject")); + d.set(b"Subtype".to_vec(), Object::name("Form")); + d.set(b"BBox".to_vec(), num_array(&[0.0, 0.0, 20.0, 10.0])); + let r = doc.add(Object::Stream(pdfcraft_cos::Stream::from_raw(d, bytes.to_vec()))); + entries.set(key.to_vec(), Object::Ref(r)); + } + entries.set(b"VendorState".to_vec(), Object::name("Retained")); + let shared = if indirect { Object::Ref(doc.add(Object::Dict(entries))) } else { Object::Dict(entries) }; + for r in [first_ref, other_ref] { + doc.update_dict(r, |d| d.set(b"AP".to_vec(), shared.clone())).unwrap(); + } + let mut doc = reopen(&doc); + let other_dict = list(&doc, 0)[other].clone(); + let source = other_dict.get(b"AP").unwrap(); + let before = doc.resolve(source); + set_style(&mut doc, 0, first, Some([1.0, 0.0, 0.0]), None, None, &meta("")).unwrap(); + assert_eq!(doc.resolve(source), before, "a shared AP must not change"); + for full in [false, true] { + let bytes = if full { + pdfcraft_cos::write_full(&doc, &SaveOptions::default()).unwrap() + } else { + assert!(!doc.revisions().is_empty(), "incremental save needs an existing revision"); + assert!(!doc.encryption_changed() && !doc.full_save_required(), "this edit must not require a full rewrite"); + let original = doc.bytes(); + let bytes = write_incremental(&doc, &SaveOptions::default()).unwrap(); + assert!(bytes.len() > original.len(), "incremental save appends the changed appearance"); + assert!(bytes.starts_with(original.as_slice()), "incremental save preserves the complete original prefix"); + bytes + }; + hayro_syntax::Pdf::new(bytes.clone()).unwrap(); + let doc = Document::open(Arc::new(bytes)).unwrap(); + let all = list(&doc, 0); + let first_ap = doc.resolve(all[first].get(b"AP").unwrap()); + let other_ap = doc.resolve(all[other].get(b"AP").unwrap()); + let first_ap = first_ap.as_dict().unwrap(); + let other_ap = other_ap.as_dict().unwrap(); + assert_ne!(first_ap.get(b"N"), other_ap.get(b"N")); + assert_eq!(first_ap.len(), 4); + assert_eq!(other_ap.len(), 4); + for (key, bytes) in alternates { + assert_eq!(first_ap.get(key), other_ap.get(key)); + let object = doc.resolve(first_ap.get(key).unwrap()); + let Object::Stream(stream) = &*object else { panic!("alternate appearance") }; + assert_eq!(stream.decoded().unwrap(), bytes); + } + assert_eq!(first_ap.name(b"VendorState"), Some(&b"Retained"[..])); + assert_eq!(other_ap.name(b"VendorState"), Some(&b"Retained"[..])); + assert!(ap_content(&doc, &all[first]).contains("1 0 0 RG")); + assert!(ap_content(&doc, &all[other]).contains("0 0.4 1 RG")); + } + } +} diff --git a/crates/forms/src/author.rs b/crates/forms/src/author.rs index d51abda730..d60c2c816e 100644 --- a/crates/forms/src/author.rs +++ b/crates/forms/src/author.rs @@ -569,7 +569,11 @@ pub fn redraw_field(doc: &mut Document, name: &str) -> Result<(), FormError> { d } }; - doc.update_dict(w.obj, |d| d.set(b"AP".to_vec(), Object::Dict(ap)))?; + let mut existing = crate::appearance_dict(doc, w.obj); + for (key, value) in ap.iter() { + existing.set(key.clone(), value.clone()); + } + doc.update_dict(w.obj, |d| d.set(b"AP".to_vec(), Object::Dict(existing)))?; } Ok(()) } @@ -608,8 +612,8 @@ fn button_appearance(doc: &Document, w: &Widget) -> Stream { let (mut c, width, height) = frame_only(doc, w); let wobj = doc.get(w.obj); let mk = wobj.as_dict().and_then(|d| d.get(b"MK")).map(|m| doc.resolve(m)).and_then(|m| m.as_dict().cloned()).unwrap_or_default(); - let caption = mk.get(b"CA").and_then(|c| c.as_string()).map(|s| s.to_text()).unwrap_or_default(); - let icon_only = mk.int(b"TP") == Some(1); + let caption = mk.get(b"CA").map(|c| doc.resolve(c)).and_then(|c| c.as_string().map(|s| s.to_text())).unwrap_or_default(); + let icon_only = mk.get(b"TP").and_then(|t| doc.resolve(t).as_int()) == Some(1); let mut content = std::mem::take(&mut c).into_bytes(); let mut xobjects = Dict::new(); if let Some(icon) = mk.get(b"I").and_then(Object::as_ref) diff --git a/crates/forms/src/lib.rs b/crates/forms/src/lib.rs index 41ce2198c4..be8433cf35 100644 --- a/crates/forms/src/lib.rs +++ b/crates/forms/src/lib.rs @@ -880,6 +880,11 @@ fn set_states(doc: &mut Document, f: &Field, on: Option<&str>) -> Result<(), For Ok(()) } +/// Copy an existing appearance dictionary before replacing its normal appearance. +fn appearance_dict(doc: &Document, widget: ObjRef) -> Dict { + doc.get(widget).as_dict().and_then(|d| d.get(b"AP").map(|a| doc.resolve(a))).and_then(|a| a.as_dict().cloned()).unwrap_or_default() +} + /// Regenerate the normal appearance of every widget of a text or choice field. fn redraw(doc: &mut Document, f: &Field, values: &[String], scripts: &mut dyn Scripts) -> Result<(), FormError> { let shown = match values { @@ -892,7 +897,7 @@ fn redraw(doc: &mut Document, f: &Field, values: &[String], scripts: &mut dyn Sc None => appearance::field_appearance(doc, f, w, values), }; let ap = doc.add(Object::Stream(stream)); - let mut apd = Dict::new(); + let mut apd = appearance_dict(doc, w.obj); apd.set(b"N".to_vec(), Object::Ref(ap)); doc.update_dict(w.obj, |d| { d.set(b"AP".to_vec(), Object::Dict(apd)); diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index 8cdc8d3726..e80585982d 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -827,3 +827,175 @@ fn page_shapes_reads_boxes_and_rules() { assert_eq!(sh.boxes, [[20.0, 20.0, 32.0, 32.0]]); assert_eq!(sh.rules, [[40.0, 100.0, 240.0, 100.0], [60.0, 600.5, 260.0, 600.5]]); } + +#[test] +fn filling_and_authoring_preserve_alternate_and_shared_appearance_entries() { + let alternates: [(&[u8], &[u8]); 2] = [(b"R", b"0 0 20 10 re f\n"), (b"D", b"1 1 18 8 re S\n")]; + let cases = [ + (NewField::Text { multiline: false }, true), + (NewField::Text { multiline: false }, false), + (NewField::CheckBox, false), + (NewField::Button { caption: "Button".into() }, false), + (NewField::Signature, false), + ]; + for (kind, fill) in cases { + for indirect in [false, true] { + let mut doc = one_page(); + let first = add_field(&mut doc, 0, [50.0, 600.0, 250.0, 630.0], &kind, Some("First")).unwrap(); + let other = add_field(&mut doc, 0, [50.0, 550.0, 250.0, 580.0], &kind, Some("Other")).unwrap(); + let first_widget = field(&fields(&doc), &first).widgets[0].obj; + let other_widget = field(&fields(&doc), &other).widgets[0].obj; + let mut entries = doc.get(first_widget).as_dict().unwrap().get(b"AP").unwrap().as_dict().unwrap().clone(); + for (key, bytes) in alternates { + let mut d = Dict::new(); + d.set(b"Type".to_vec(), Object::name("XObject")); + d.set(b"Subtype".to_vec(), Object::name("Form")); + d.set(b"BBox".to_vec(), Object::Array(vec![0.into(), 0.into(), 20.into(), 10.into()])); + let r = doc.add(Object::Stream(pdfcraft_cos::Stream::from_raw(d, bytes.to_vec()))); + entries.set(key.to_vec(), Object::Ref(r)); + } + entries.set(b"VendorState".to_vec(), Object::name("Retained")); + let shared = if indirect { Object::Ref(doc.add(Object::Dict(entries))) } else { Object::Dict(entries) }; + for widget in [first_widget, other_widget] { + doc.update_dict(widget, |d| d.set(b"AP".to_vec(), shared.clone())).unwrap(); + } + let original = reopen(&doc); + for full in [false, true] { + let mut doc = original.clone(); + let other_widget = field(&fields(&doc), &other).widgets[0].obj; + let source = doc.get(other_widget).as_dict().unwrap().get(b"AP").unwrap().clone(); + let before = doc.resolve(&source); + if fill { + set_value(&mut doc, &first, &FieldValue::Text("Updated value".into())).unwrap(); + } else { + set_props(&mut doc, &first, &FieldProps { tooltip: Some("Updated help".into()), ..FieldProps::default() }).unwrap(); + } + assert_eq!(doc.resolve(&source), before, "a shared AP must not change"); + let bytes = if full { + pdfcraft_cos::write_full(&doc, &SaveOptions::default()).unwrap() + } else { + assert!(!doc.revisions().is_empty(), "incremental save needs an existing revision"); + assert!(!doc.encryption_changed() && !doc.full_save_required(), "this edit must not require a full rewrite"); + let original = doc.bytes(); + let bytes = write_incremental(&doc, &SaveOptions::default()).unwrap(); + assert!(bytes.len() > original.len(), "incremental save appends the changed appearance"); + assert!(bytes.starts_with(original.as_slice()), "incremental save preserves the complete original prefix"); + bytes + }; + hayro_syntax::Pdf::new(bytes.clone()).unwrap(); + let doc = Document::open(Arc::new(bytes)).unwrap(); + let all = fields(&doc); + let first = field(&all, &first); + let other = field(&all, &other); + let first_ap = doc.resolve(doc.get(first.widgets[0].obj).as_dict().unwrap().get(b"AP").unwrap()); + let other_ap = doc.resolve(doc.get(other.widgets[0].obj).as_dict().unwrap().get(b"AP").unwrap()); + let first_ap = first_ap.as_dict().unwrap(); + let other_ap = other_ap.as_dict().unwrap(); + assert_ne!(first_ap.get(b"N"), other_ap.get(b"N"), "normal appearance was regenerated"); + assert_eq!(other_ap.len(), 4); + assert_eq!(first_ap.len(), 4); + for (key, bytes) in alternates { + assert_eq!(first_ap.get(key), other_ap.get(key)); + let object = doc.resolve(first_ap.get(key).unwrap()); + let Object::Stream(stream) = &*object else { panic!("alternate appearance") }; + assert_eq!(stream.decoded().unwrap(), bytes); + } + assert_eq!(first_ap.name(b"VendorState"), Some(&b"Retained"[..])); + assert_eq!(other_ap.name(b"VendorState"), Some(&b"Retained"[..])); + if fill { + assert_eq!(first.value, ["Updated value"]); + assert!(ap(&doc, &first.widgets[0]).contains("(Updated value) Tj")); + assert!(other.value.is_empty()); + } else { + assert_eq!(first.tooltip.as_deref(), Some("Updated help")); + assert_eq!(other.tooltip, None); + } + } + } + } +} + +#[test] +fn pushbutton_redraw_resolves_caption_and_layout_after_save_and_reopen() { + fn saved(doc: &Document, full: bool) -> Document { + let bytes = if full { + pdfcraft_cos::write_full(doc, &SaveOptions::default()).unwrap() + } else { + assert!(!doc.revisions().is_empty(), "incremental save needs an existing revision"); + assert!(!doc.encryption_changed() && !doc.full_save_required(), "this edit must not require a full rewrite"); + let original = doc.bytes(); + let bytes = write_incremental(doc, &SaveOptions::default()).unwrap(); + assert!(bytes.len() > original.len(), "incremental save appends the changed appearance"); + assert!(bytes.starts_with(original.as_slice()), "incremental save preserves the complete original prefix"); + bytes + }; + hayro_syntax::Pdf::new(bytes.clone()).unwrap(); + Document::open(Arc::new(bytes)).unwrap() + } + + for indirect_caption in [false, true] { + for indirect_layout in [false, true] { + for icon_only in [false, true] { + let mut doc = one_page(); + let name = + add_field(&mut doc, 0, [50.0, 600.0, 250.0, 650.0], &NewField::Button { caption: "Original caption".into() }, Some("Picture")) + .unwrap(); + let widget = field(&fields(&doc), &name).widgets[0].obj; + let caption = Object::String(pdfcraft_cos::PdfString::literal(b"Original caption".to_vec())); + let caption = if indirect_caption { Object::Ref(doc.add(caption)) } else { caption }; + let layout = Object::Int(i64::from(icon_only)); + let layout = if indirect_layout { Object::Ref(doc.add(layout)) } else { layout }; + let mut icon_dict = Dict::new(); + icon_dict.set(b"Type".to_vec(), Object::name("XObject")); + icon_dict.set(b"Subtype".to_vec(), Object::name("Form")); + icon_dict.set(b"BBox".to_vec(), Object::Array(vec![0.into(), 0.into(), 4.into(), 2.into()])); + let icon_bytes = b"0 0 4 2 re f\n"; + let icon = doc.add(Object::Stream(pdfcraft_cos::Stream::from_raw(icon_dict, icon_bytes.to_vec()))); + let mut mk = Dict::new(); + mk.set(b"CA".to_vec(), caption); + mk.set(b"TP".to_vec(), layout); + mk.set(b"I".to_vec(), Object::Ref(icon)); + mk.set(b"VendorNote".to_vec(), Object::name("Retained")); + let mk = doc.add(Object::Dict(mk)); + doc.update_dict(widget, |d| d.set(b"MK".to_vec(), Object::Ref(mk))).unwrap(); + + // Import the operands from a real saved file before an unrelated public edit. + let original = saved(&doc, true); + for full in [false, true] { + let mut doc = original.clone(); + let widget = field(&fields(&doc), &name).widgets[0].obj; + let mk_before = doc.get(widget).as_dict().unwrap().get(b"MK").unwrap().clone(); + let original_mk = doc.resolve(&mk_before); + set_props(&mut doc, &name, &FieldProps { tooltip: Some("Picture help".into()), ..FieldProps::default() }).unwrap(); + assert_eq!(doc.get(widget).as_dict().unwrap().get(b"MK"), Some(&mk_before)); + assert_eq!(doc.resolve(&mk_before), original_mk, "redraw must not rewrite the source MK"); + let doc = saved(&doc, full); + let f = field(&fields(&doc), &name).clone(); + assert_eq!(f.tooltip.as_deref(), Some("Picture help")); + let w = &f.widgets[0]; + let wd = doc.get(w.obj); + let mk_object = doc.resolve(wd.as_dict().unwrap().get(b"MK").unwrap()); + let mk = mk_object.as_dict().unwrap(); + assert_eq!(mk.get(b"CA").unwrap().as_ref().is_some(), indirect_caption); + assert_eq!(doc.resolve(mk.get(b"CA").unwrap()).as_string().unwrap().bytes, b"Original caption"); + assert_eq!(mk.get(b"TP").unwrap().as_ref().is_some(), indirect_layout); + assert_eq!(doc.resolve(mk.get(b"TP").unwrap()).as_int(), Some(i64::from(icon_only))); + assert_eq!(mk.name(b"VendorNote"), Some(&b"Retained"[..])); + let appearance = ap(&doc, w); + assert_eq!(appearance.contains("(Original caption) Tj"), !icon_only, "{appearance}"); + assert_eq!(appearance.matches("/Icon Do").count(), 1, "{appearance}"); + assert!(appearance.contains("24.000000 0 0 24.000000 52.000 1.000 cm"), "{appearance}"); + let n = wd.as_dict().unwrap().get(b"AP").unwrap().as_dict().unwrap().reference(b"N").unwrap(); + let appearance_object = doc.get(n); + let Object::Stream(stream) = &*appearance_object else { panic!("appearance") }; + let resource_icon = + stream.dict.get(b"Resources").unwrap().as_dict().unwrap().get(b"XObject").unwrap().as_dict().unwrap().get(b"Icon").unwrap(); + assert_eq!(resource_icon, mk.get(b"I").unwrap()); + let icon_object = doc.resolve(resource_icon); + let Object::Stream(icon) = &*icon_object else { panic!("icon") }; + assert_eq!(icon.decoded().unwrap(), icon_bytes); + } + } + } + } +} diff --git a/crates/ui-egui/tests/view.rs b/crates/ui-egui/tests/view.rs index aedeff8281..1d66e7d58b 100644 --- a/crates/ui-egui/tests/view.rs +++ b/crates/ui-egui/tests/view.rs @@ -90,9 +90,25 @@ fn middle_button_scrolling_keeps_page_colours_in_both_themes() { let mut h = harness(&[("zoom", "50")]); h.state_mut().set_option("theme", theme).unwrap(); h.state_mut().set_option("organize", if organize { "on" } else { "off" }).unwrap(); - for _ in 0..100 { + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); + loop { + assert!( + std::time::Instant::now() < deadline, + "render readiness timed out: theme={theme}, organize={organize}, errors={:?}", + h.state().views[0].page_errors() + ); h.run_steps(2); if !h.state().render_pending() { + // Paint one fresh frame after all requested rasters have arrived. + h.run_steps(1); + if h.state().render_pending() { + continue; + } + assert!( + h.state().views[0].page_errors().is_empty(), + "page render errors: theme={theme}, organize={organize}, errors={:?}", + h.state().views[0].page_errors() + ); break; } std::thread::sleep(std::time::Duration::from_millis(5)); @@ -101,7 +117,11 @@ fn middle_button_scrolling_keeps_page_colours_in_both_themes() { let at = page.center(); let ppp = h.ctx.pixels_per_point(); let before = *h.render().unwrap().get_pixel((at.x * ppp) as u32, (at.y * ppp) as u32); - assert_eq!(before, image::Rgba([255, 255, 255, 255]), "the synthetic page is white"); + assert_eq!( + before, + image::Rgba([255, 255, 255, 255]), + "the synthetic page is white: theme={theme}, organize={organize}, page={page:?}, ppp={ppp}" + ); let anchor = h.state().views[0].viewport_rect().center(); h.event(egui::Event::PointerMoved(anchor)); h.event(egui::Event::PointerButton { pos: anchor, button: egui::PointerButton::Middle, pressed: true, modifiers: Modifiers::NONE });