diff --git a/crates/automation/src/lib.rs b/crates/automation/src/lib.rs index 8d49d66ae..0ccc71c81 100644 --- a/crates/automation/src/lib.rs +++ b/crates/automation/src/lib.rs @@ -73,6 +73,12 @@ fn failed(e: impl std::fmt::Display) -> ToolError { ToolError::Failed(e.to_string()) } +impl From for ToolError { + fn from(e: pdfcraft_engine::EditError) -> Self { + failed(e) + } +} + /// Default and maximum resolution for `page_render`. const DEFAULT_DPI: f64 = 96.0; const MAX_DPI: f64 = 600.0; @@ -1243,8 +1249,8 @@ impl Automation { fn doc_create_multiple(&mut self, a: &Args) -> Result { let paths = a.strs("paths")?; - if paths.is_empty() || paths.len() > pdfcraft_engine::MAX_CREATE_FILES { - return Err(ToolError::InvalidArgs(format!("paths must list 1 to {} files", pdfcraft_engine::MAX_CREATE_FILES))); + if paths.is_empty() { + return Err(ToolError::InvalidArgs("paths must list at least one file".into())); } let separate = match a.opt_str("mode")? { None | Some("combine") => false, @@ -1259,16 +1265,16 @@ impl Automation { Some(Value::Array(v)) if v.len() == paths.len() => v.iter().map(|x| x.as_str().map(str::to_owned)).collect(), Some(_) => return Err(ToolError::InvalidArgs("pages must list a range (or null) for each path".into())), }; - let mut sources = Vec::new(); - for (p, range) in paths.into_iter().zip(ranges) { + // Each file is read, converted and copied in turn, so a long list holds one file at a time. + let sources = paths.into_iter().zip(ranges).map(|(p, range)| -> std::result::Result { let path = self.resolve(p, false)?; let bytes = std::fs::read(&path).map_err(|e| failed(format!("{}: {e}", path.display())))?; let name = path.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); let (_, pdf) = self.session.convert_to_pdf(&name, &Arc::new(bytes)).map_err(failed)?; let title = path.file_stem().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); - sources.push((title, pdf, range)); - } - let bytes = self.session.combine_ranges(&sources).map_err(failed)?; + Ok((title, pdf, range)) + }); + let bytes = self.session.combine_each(sources)?.ok_or_else(|| ToolError::InvalidArgs("paths must list at least one file".into()))?; self.deliver(a, "Combined", bytes) } diff --git a/crates/automation/src/tools.rs b/crates/automation/src/tools.rs index 60d559f43..d63522fec 100644 --- a/crates/automation/src/tools.rs +++ b/crates/automation/src/tools.rs @@ -232,7 +232,7 @@ pub fn tools() -> Vec { .cmd("create.multiple") .with(schema( json!({ - "paths": { "type": "array", "items": { "type": "string" }, "minItems": 1, "maxItems": pdfcraft_engine::MAX_CREATE_FILES }, + "paths": { "type": "array", "items": { "type": "string" }, "minItems": 1 }, "mode": { "type": "string", "enum": ["combine", "separate"] }, "pages": { "type": "array", "items": { "type": ["string", "null"] } }, "out": save_out.clone(), diff --git a/crates/automation/tests/automation.rs b/crates/automation/tests/automation.rs index b3380bbcf..b32702d6f 100644 --- a/crates/automation/tests/automation.rs +++ b/crates/automation/tests/automation.rs @@ -2077,6 +2077,17 @@ fn creating_from_multiple_files_through_tools() { assert!(!dir.join("out/a.pdf").exists()); } +#[test] +fn create_multiple_has_no_file_count_limit() { + let dir = workdir("create-many"); + let mut a = auto(&dir); + // The same 3-page PDF listed 1,001 times: every listed file is a source, however many there are. + let paths = vec!["a.pdf"; 1001]; + let made = ok(&mut a, "doc_create_multiple", json!({ "paths": paths, "out": "many.pdf", "open": true })); + assert_eq!(made["document"]["pages"], 3 * 1001, "{made}"); + assert!(dir.join("many.pdf").is_file()); +} + #[test] fn creating_images_with_dpi_through_tools() { let dir = workdir("image-dpi"); diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index e397ddc01..cbc8bd399 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -1917,9 +1917,6 @@ pub fn guard(f: impl FnOnce() -> T) -> Result { }) } -/// The most files Create ▸ Multiple files takes in one run. -pub const MAX_CREATE_FILES: usize = 1000; - /// Parse another PDF to copy pages from. fn open_source(name: &str, bytes: &Arc>) -> Result { open_source_with(name, bytes, None) @@ -2892,6 +2889,43 @@ impl Session { self.combine_unlocked(sources, &[]) } + /// Combine sources one at a time, in order (Create ▸ Multiple files), to the same result as + /// [`Self::combine_ranges`]. Each item is a source's bookmark title, its PDF bytes (see + /// [`Self::convert_to_pdf`]) and its page range (`None` for all pages). Items are pulled one at a + /// time: a source is opened, its pages are copied into the result, and its bytes are released + /// before the next item is read. So the number of sources is not limited, and one source's + /// document is held at a time. An item's error stops the combine and is returned as it is. + /// `Ok(None)` when there are no sources. + pub fn combine_each(&self, sources: I) -> Result>>, E> + where + I: IntoIterator>, + E: From, + { + let mut combiner = pdfcraft_organize::Combiner::new(); + let mut added = false; + for source in sources { + let (title, bytes, range) = source?; + let doc = open_source(&title, &bytes)?; + let range = range.as_deref().map(str::trim).filter(|r| !r.is_empty()); + let pages = match range { + Some(r) => { + let n = pdfcraft_organize::page_count(&doc).map_err(EditError::from)?; + let p = pdfcraft_print::select_pages(n, Some(r), &[], pdfcraft_print::Subset::All, false) + .map_err(|e| EditError::Print(format!("{title}: {e}")))?; + Some(p) + } + None => None, + }; + combiner.add(&title, &doc, pages.as_deref()).map_err(EditError::from)?; + added = true; + } + if !added { + return Ok(None); + } + let out = combiner.finish().map_err(EditError::from)?; + Ok(Some(self.write_new(&out)?)) + } + /// [`Self::combine_ranges`] with the password each encrypted source is opened with, by /// position (missing or `None`: no password). The result is not encrypted. pub fn combine_unlocked(&self, sources: &[CombineSource], passwords: &[Option<&str>]) -> Result>, EditError> { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 2971396a5..b978aefde 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -504,6 +504,43 @@ fn a_file_split_around_another_combines_in_order_with_one_bookmark() { assert!(matches!(s.combine_grouped(&bad, &[1], &[]), Err(EditError::Print(e)) if e.starts_with("a:"))); } +#[test] +fn create_multiple_reads_each_source_only_after_the_previous_one_is_released() { + let mut s = Session::new(); + let mut earlier: Vec>> = Vec::new(); + let texts = ["Alpha", "Beta", "Gamma"]; + let sources = texts.iter().map(|text| { + // The combine has finished with every earlier source, so their bytes are gone by now. + assert!(earlier.iter().all(|w| w.upgrade().is_none()), "a source is released before the next is read"); + let (_, pdf) = s.convert_to_pdf(&format!("{text}.txt"), &Arc::new(text.as_bytes().to_vec())).unwrap(); + earlier.push(Arc::downgrade(&pdf)); + Ok::((text.to_string(), pdf, None)) + }); + let combined = s.combine_each(sources).unwrap().unwrap(); + assert!(earlier.iter().all(|w| w.upgrade().is_none())); + let id = s.open_new("Combined.pdf", combined).unwrap(); + assert_eq!(page_texts(&s, id), ["Alpha", "Beta", "Gamma"]); + assert_eq!(outline_titles(&s.get(id).unwrap().info.outline), ["Alpha→1", "Beta→2", "Gamma→3"]); +} + +#[test] +fn create_multiple_gives_the_bytes_of_combine_ranges_and_refuses_a_bad_source() { + let s = Session::new(); + let (a, b) = (Arc::new(fixture(3)), Arc::new(fixture(2))); + let sources: Vec = + vec![("a".into(), a.clone(), Some("1-2".into())), ("b".into(), b, None), ("a".into(), a.clone(), Some("3".into()))]; + let by_ranges = s.combine_ranges(&sources).unwrap(); + let streamed = s.combine_each(sources.into_iter().map(Ok::<_, EditError>)).unwrap().unwrap(); + assert_eq!(*streamed, *by_ranges); + // A bad range, or a file that isn't a PDF, stops the combine with an error naming that source. + let bad_range: Vec> = vec![Ok(("a".into(), a.clone(), None)), Ok(("c".into(), a.clone(), Some("9".into())))]; + assert!(matches!(s.combine_each(bad_range), Err(EditError::Print(e)) if e.starts_with("c:"))); + let junk: Vec> = vec![Ok(("junk".into(), Arc::new(b"not a pdf".to_vec()), None))]; + assert!(matches!(s.combine_each(junk), Err(EditError::Source(m)) if m.starts_with("junk:"))); + // Nothing to combine is `None`, not an empty document. + assert!(matches!(s.combine_each(Vec::>::new()), Ok(None))); +} + #[test] fn a_page_with_a_form_field_shown_twice_keeps_one_field() { // Regression (found by review): the repeated page copied the whole field again, so the diff --git a/crates/organize/src/import.rs b/crates/organize/src/import.rs index b3342c9b4..72e20103c 100644 --- a/crates/organize/src/import.rs +++ b/crates/organize/src/import.rs @@ -486,7 +486,7 @@ pub fn extract_pages(src: &Document, pages: &[usize]) -> Result Result /// Combine Files with chosen pages: each source contributes `pages` (0-based, in that order; /// `None` for all of them). Bookmarks that point at pages left out lose their destination. pub fn combine_selected(sources: &[(&str, &Document, Option<&[usize]>)]) -> Result { - let mut out = Document::new_empty(); - let mut marks = Vec::new(); - let mut attachments = Vec::new(); - // The first source's print standard, and what its pages brought along; it carries over - // only when every source declares the same one. - let mut standard: Option<(crate::pdfx::PrintStandard, &Document, HashMap)> = None; - let mut agreed = true; + let mut combiner = Combiner::new(); for (title, src, chosen) in sources { + combiner.add(title, src, *chosen)?; + } + combiner.finish() +} + +/// Combine Files one source at a time, with the same result as [`combine_selected`]. [`Combiner::add`] +/// copies a source's pages into the result and keeps only what the rest of the combine needs: its +/// bookmarks (read and mapped to the copied pages), its attachments (copied), and its document only +/// when it declares output intents. So each source can be dropped as soon as it is added, and a long +/// list holds one source at a time rather than all of them. +pub struct Combiner { + out: Document, + marks: Vec, + attachments: Vec<(Vec, Object)>, + /// The first source's print standard, and what its pages brought along. + standard: Option, + /// Whether every source so far declares the first source's print standard. + agreed: bool, +} + +/// The first source's print standard (see [`Combiner`]). +struct Standard { + declared: crate::pdfx::PrintStandard, + /// The source objects the copied pages brought along, and their copies. + objects: HashMap, + /// The source itself, kept only for its output intents, which are copied from it. + source: Option, +} + +impl Default for Combiner { + fn default() -> Self { + Self::new() + } +} + +impl Combiner { + /// A combine with no sources yet. + pub fn new() -> Self { + Self { out: Document::new_empty(), marks: Vec::new(), attachments: Vec::new(), standard: None, agreed: true } + } + + /// Add `src`'s `pages` (0-based, in that order; `None` for all of them) after the pages added so far, + /// under a bookmark titled `title`. `src` need not outlive the call. + pub fn add(&mut self, title: &str, src: &Document, pages: Option<&[usize]>) -> Result<(), OrganizeError> { let n = crate::page_count(src)?; - let pages: Vec = match chosen { + let pages: Vec = match pages { Some(p) => { if let Some(bad) = p.iter().find(|i| **i >= n) { return Err(OrganizeError::NoSuchPage(*bad)); @@ -558,27 +596,38 @@ pub fn combine_selected(sources: &[(&str, &Document, Option<&[usize]>)]) -> Resu } None => (0..n).collect(), }; - let at = crate::page_count(&out)?; - let imported = import_pages_mapped(&mut out, src, &pages, at)?; + let at = crate::page_count(&self.out)?; + let imported = import_pages_mapped(&mut self.out, src, &pages, at)?; if let Some(first) = imported.pages.first() { - marks.push((title.to_string(), *first, *src, imported.page_map)); + let outline = source_outline(src, imported.page_map); + self.marks.push(Mark { title: title.to_string(), page: *first, outline }); } - collect_attachments(&mut out, src, &mut attachments); + collect_attachments(&mut self.out, src, &mut self.attachments); let declared = crate::pdfx::PrintStandard::of(src); - match &standard { - None => standard = Some((declared, *src, imported.objects)), - Some((first, _, _)) => agreed &= first.same_as(&declared), + match self.standard.as_ref().map(|first| first.declared.same_as(&declared)) { + Some(same) => self.agreed &= same, + None => { + let source = declared.declares_output_intents().then(|| src.clone()); + self.standard = Some(Standard { declared, objects: imported.objects, source }); + } } + Ok(()) } - if let Some((declared, src, objects)) = standard.filter(|_| agreed) { - crate::pdfx::carry(&mut out, src, &declared, objects)?; + + /// The combined document: the pages as added, the print standard of the first source when every + /// source declares the same one, shared resources stored once, then the bookmarks and attachments. + pub fn finish(self) -> Result { + let Combiner { mut out, marks, attachments, standard, agreed } = self; + if let Some(first) = standard.filter(|_| agreed) { + crate::pdfx::carry(&mut out, first.source.as_ref(), &first.declared, first.objects)?; + } + // Sources often share fonts, images and profiles (or are the same file): store them once. + let all: Vec = out.object_numbers().into_iter().map(|n| ObjRef::new(n, out.generation(n))).collect(); + crate::dedupe::dedupe_resources(&mut out, &all, false); + add_outline(&mut out, &marks)?; + set_attachments(&mut out, attachments)?; + Ok(out) } - // Sources often share fonts, images and profiles (or are the same file): store them once. - let all: Vec = out.object_numbers().into_iter().map(|n| ObjRef::new(n, out.generation(n))).collect(); - crate::dedupe::dedupe_resources(&mut out, &all, false); - add_outline(&mut out, &marks)?; - set_attachments(&mut out, attachments)?; - Ok(out) } /// One run of pages for [`combine_grouped`]: the file it comes from (`group`: runs with the same @@ -633,7 +682,7 @@ pub fn combine_grouped(runs: &[Run<'_>]) -> Result { let imported = import_pages_mapped(&mut out, file.src, &file.pages, at)?; copies.push(imported.pages.clone()); if let Some(first) = imported.pages.first() { - marks.push((file.title.to_string(), *first, file.src, imported.page_map)); + marks.push(Mark { title: file.title.to_string(), page: *first, outline: source_outline(file.src, imported.page_map) }); } collect_attachments(&mut out, file.src, &mut attachments); let declared = crate::pdfx::PrintStandard::of(file.src); @@ -651,7 +700,7 @@ pub fn combine_grouped(runs: &[Run<'_>]) -> Result { } rebuild(&mut out, &order)?; if let Some((declared, src, objects)) = standard.filter(|_| agreed) { - crate::pdfx::carry(&mut out, src, &declared, objects)?; + crate::pdfx::carry(&mut out, Some(src), &declared, objects)?; } let all: Vec = out.object_numbers().into_iter().map(|n| ObjRef::new(n, out.generation(n))).collect(); crate::dedupe::dedupe_resources(&mut out, &all, false); @@ -714,36 +763,132 @@ fn set_attachments(dst: &mut Document, mut entries: Vec<(Vec, Object)>) -> R Ok(()) } -type Mark<'a> = (String, ObjRef, &'a Document, HashMap); +/// A source's entry in the combined outline (see [`add_outline`]): its title, its first page in the +/// result, and its own bookmarks, read from the source when it was added. +struct Mark { + title: String, + page: ObjRef, + outline: Vec, +} -/// Replace the outline with one bookmark per source (title → its first page), each holding a -/// collapsed copy of that source's own bookmarks with destinations mapped to the copied pages. -fn add_outline(doc: &mut Document, marks: &[Mark<'_>]) -> Result<(), OrganizeError> { +/// One of a source's bookmarks, read and ready to write: its title, colour and flags, its destination +/// in the result (`None` when the page it points at was not copied), whether it is open, and its own +/// bookmarks. +struct OutlineNode { + title: Option, + colour: Option, + flags: Option, + dest: Option, + open: bool, + children: Vec, +} + +/// The bookmarks of `src`, with destinations mapped through `pages` (source page to copied page). They +/// are read while `src` is at hand, so the source itself need not be kept. +fn source_outline(src: &Document, pages: HashMap) -> Vec { + let first = src + .root() + .and_then(|r| src.get(r).as_dict().cloned()) + .and_then(|c| c.get(b"Outlines").map(|o| src.resolve(o))) + .and_then(|o| o.as_dict().and_then(|d| d.reference(b"First"))); + let mapper = Copier { src, map: HashMap::new(), pages, annots: Vec::new(), fields: Vec::new(), ocgs: Vec::new() }; + let mut budget = 10_000usize; + outline_level(&mapper, first, 0, &mut budget) +} + +/// One level of bookmarks from `first` (its siblings), each with its children. Cycle- and size-safe: +/// at most `budget` bookmarks are read in all, and nesting stops at 32 levels. +fn outline_level(m: &Copier<'_>, first: Option, depth: u8, budget: &mut usize) -> Vec { + if depth > 32 { + return Vec::new(); + } + let mut seen = std::collections::HashSet::new(); + let mut made = Vec::new(); + let mut cur = first; + while let Some(r) = cur { + if !seen.insert(r) || *budget == 0 { + break; + } + *budget -= 1; + let Some(item) = m.src.get(r).as_dict().cloned() else { break }; + cur = item.reference(b"Next"); + let title = item.get(b"Title").map(|t| m.src.resolve(t).as_ref().clone()); + let colour = item.get(b"C").map(|v| m.src.resolve(v).as_ref().clone()); + let flags = item.get(b"F").map(|v| m.src.resolve(v).as_ref().clone()); + let target = item.get(b"Dest").cloned().or_else(|| { + let a = m.src.resolve(item.get(b"A")?); + let a = a.as_dict()?; + (a.name(b"S") == Some(b"GoTo")).then(|| a.get(b"D").cloned()).flatten() + }); + let dest = target.and_then(|t| m.map_dest(&t)); + let open = item.int(b"Count").is_some_and(|c| c > 0); + let children = outline_level(m, item.reference(b"First"), depth + 1, budget); + made.push(OutlineNode { title, colour, flags, dest, open, children }); + } + made +} + +/// Write `nodes` as one level under `parent`: each bookmark is created, then its children, depth first +/// and in order, so the object numbers come out as they always did. Returns the level's first and last +/// item and how many items it has (`None` when it is empty). +fn write_outline_level(dst: &mut Document, parent: ObjRef, nodes: &[OutlineNode]) -> Result, OrganizeError> { + let mut made: Vec = Vec::new(); + for node in nodes { + let new = dst.add(Object::Null); + let mut d = Dict::new(); + if let Some(t) = &node.title { + d.set(b"Title".to_vec(), t.clone()); + } + if let Some(c) = &node.colour { + d.set(b"C".to_vec(), c.clone()); + } + if let Some(f) = &node.flags { + d.set(b"F".to_vec(), f.clone()); + } + if let Some(dest) = &node.dest { + d.set(b"Dest".to_vec(), dest.clone()); + } + d.set(b"Parent".to_vec(), Object::Ref(parent)); + if let Some(prev) = made.last() { + d.set(b"Prev".to_vec(), Object::Ref(*prev)); + dst.update_dict(*prev, |p| p.set(b"Next".to_vec(), Object::Ref(new)))?; + } + if let Some((f, l, n)) = write_outline_level(dst, new, &node.children)? { + d.set(b"First".to_vec(), Object::Ref(f)); + d.set(b"Last".to_vec(), Object::Ref(l)); + // Keep the source's open/closed state. + d.set(b"Count".to_vec(), Object::Int(if node.open { n as i64 } else { -(n as i64) })); + } + dst.set(new, Object::Dict(d)); + made.push(new); + } + Ok(match (made.first(), made.last()) { + (Some(first), Some(last)) => Some((*first, *last, made.len())), + _ => None, + }) +} + +/// Replace the outline with one bookmark per source (title → its first page), each holding a collapsed +/// copy of that source's own bookmarks with destinations mapped to the copied pages. +fn add_outline(doc: &mut Document, marks: &[Mark]) -> Result<(), OrganizeError> { if marks.is_empty() { return Ok(()); } let outlines = doc.add(Object::Null); let items: Vec = marks.iter().map(|_| doc.add(Object::Null)).collect(); let (Some(&first_item), Some(&last_item)) = (items.first(), items.last()) else { return Ok(()) }; - for (i, ((title, page, src, page_map), r)) in marks.iter().zip(&items).enumerate() { + for (i, (mark, r)) in marks.iter().zip(&items).enumerate() { let mut d = Dict::new(); - d.set(b"Title".to_vec(), Object::String(pdfcraft_cos::PdfString::text(title))); + d.set(b"Title".to_vec(), Object::String(pdfcraft_cos::PdfString::text(&mark.title))); d.set(b"Parent".to_vec(), Object::Ref(outlines)); - d.set(b"Dest".to_vec(), Object::Array(vec![Object::Ref(*page), Object::name("Fit")])); + d.set(b"Dest".to_vec(), Object::Array(vec![Object::Ref(mark.page), Object::name("Fit")])); if i > 0 { d.set(b"Prev".to_vec(), Object::Ref(items[i - 1])); } if let Some(next) = items.get(i + 1) { d.set(b"Next".to_vec(), Object::Ref(*next)); } - let first = src - .root() - .and_then(|r| src.get(r).as_dict().cloned()) - .and_then(|c| c.get(b"Outlines").map(|o| src.resolve(o))) - .and_then(|o| o.as_dict().and_then(|d| d.reference(b"First"))); - let mapper = Copier { src, map: HashMap::new(), pages: page_map.clone(), annots: Vec::new(), fields: Vec::new(), ocgs: Vec::new() }; - let mut budget = 10_000usize; - if let Some((f, l, n)) = copy_outline_level(doc, &mapper, first, *r, 0, &mut budget) { + if let Some((f, l, n)) = write_outline_level(doc, *r, &mark.outline)? { d.set(b"First".to_vec(), Object::Ref(f)); d.set(b"Last".to_vec(), Object::Ref(l)); d.set(b"Count".to_vec(), Object::Int(-(n as i64))); // collapsed @@ -764,67 +909,6 @@ fn add_outline(doc: &mut Document, marks: &[Mark<'_>]) -> Result<(), OrganizeErr Ok(()) } -/// Copy one level of a source outline (siblings from `first`) under `parent`, recursing into -/// children. Returns (first, last, visible count). Cycle- and size-safe. -fn copy_outline_level( - dst: &mut Document, - m: &Copier<'_>, - first: Option, - parent: ObjRef, - depth: u8, - budget: &mut usize, -) -> Option<(ObjRef, ObjRef, usize)> { - if depth > 32 { - return None; - } - let mut seen = std::collections::HashSet::new(); - let mut made: Vec = Vec::new(); - let mut total = 0; - let mut cur = first; - while let Some(r) = cur { - if !seen.insert(r) || *budget == 0 { - break; - } - *budget -= 1; - let Some(item) = m.src.get(r).as_dict().cloned() else { break }; - cur = item.reference(b"Next"); - let new = dst.add(Object::Null); - let mut d = Dict::new(); - if let Some(t) = item.get(b"Title").map(|t| m.src.resolve(t)) { - d.set(b"Title".to_vec(), t.as_ref().clone()); - } - for k in [&b"C"[..], b"F"] { - if let Some(v) = item.get(k) { - d.set(k.to_vec(), m.src.resolve(v).as_ref().clone()); - } - } - let target = item.get(b"Dest").cloned().or_else(|| { - let a = m.src.resolve(item.get(b"A")?); - let a = a.as_dict()?; - (a.name(b"S") == Some(b"GoTo")).then(|| a.get(b"D").cloned()).flatten() - }); - if let Some(dest) = target.and_then(|t| m.map_dest(&t)) { - d.set(b"Dest".to_vec(), dest); - } - d.set(b"Parent".to_vec(), Object::Ref(parent)); - if let Some(prev) = made.last() { - d.set(b"Prev".to_vec(), Object::Ref(*prev)); - dst.update_dict(*prev, |p| p.set(b"Next".to_vec(), Object::Ref(new))).ok()?; - } - if let Some((f, l, n)) = copy_outline_level(dst, m, item.reference(b"First"), new, depth + 1, budget) { - d.set(b"First".to_vec(), Object::Ref(f)); - d.set(b"Last".to_vec(), Object::Ref(l)); - // Keep the source's open/closed state. - let open = item.int(b"Count").is_some_and(|c| c > 0); - d.set(b"Count".to_vec(), Object::Int(if open { n as i64 } else { -(n as i64) })); - } - dst.set(new, Object::Dict(d)); - made.push(new); - total += 1; - } - Some((*made.first()?, *made.last()?, total)) -} - /// Page `page` of `src` as a form XObject in `dst` (its content and resources, no annotations), /// for backgrounds and watermarks taken from a PDF. The form's `/BBox` is the page's crop box /// and its `/Matrix` undoes the page rotation, so it draws upright as displayed. Returns the diff --git a/crates/organize/src/lib.rs b/crates/organize/src/lib.rs index 8f7bdac77..c7ac3c869 100644 --- a/crates/organize/src/lib.rs +++ b/crates/organize/src/lib.rs @@ -24,7 +24,9 @@ pub mod view; pub use boxes::{BoxSpec, PageBox, page_boxes, set_page_box}; pub use dedupe::dedupe_resources; -pub use import::{Run, SplitBy, combine, combine_grouped, combine_selected, extract_pages, import_pages, page_as_form, split, split_ranges}; +pub use import::{ + Combiner, Run, SplitBy, combine, combine_grouped, combine_selected, extract_pages, import_pages, page_as_form, split, split_ranges, +}; pub use labels::{LabelRange, LabelStyle, PageLabels, number_pages, page_label_ranges, page_labels, set_page_label_ranges}; pub use outline::{ Bookmark, BookmarkPage, OutlineEntry, OutlineError, add_bookmark, add_bookmark_tree, bookmark_page, bookmarks, delete_bookmark, move_bookmark, diff --git a/crates/organize/src/pdfx.rs b/crates/organize/src/pdfx.rs index a24b5a3a9..3241d4303 100644 --- a/crates/organize/src/pdfx.rs +++ b/crates/organize/src/pdfx.rs @@ -73,6 +73,11 @@ impl PrintStandard { } } + /// Whether the document declares output intents (its printing condition and profile). + pub(crate) fn declares_output_intents(&self) -> bool { + self.intents.is_some() + } + /// Whether `other` declares the same standard: the same version and trapping, and the same /// output intents (type, printing condition and profile). pub(crate) fn same_as(&self, other: &Self) -> bool { @@ -82,10 +87,12 @@ impl PrintStandard { /// Give `dst` the print standard `src` declares: its output intents, copied with their profiles /// (through `map`, what the pages already brought along), and, for PDF/X, the document -/// information entries and the XMP metadata that identify it. -pub(crate) fn carry(dst: &mut Document, src: &Document, standard: &PrintStandard, map: HashMap) -> Result<(), OrganizeError> { +/// information entries and the XMP metadata that identify it. The output intents are copied from +/// `src`, so a standard that declares them needs the source document (`None` otherwise). +pub(crate) fn carry(dst: &mut Document, src: Option<&Document>, standard: &PrintStandard, map: HashMap) -> Result<(), OrganizeError> { let root = dst.root().ok_or(OrganizeError::NoPageTree)?; if let Some(intents) = &standard.intents { + let src = src.ok_or_else(|| OrganizeError::Invalid("the output intents' source document was not kept".into()))?; let copied = crate::import::copy_object(dst, src, intents, map); dst.update_dict(root, |c| c.set(b"OutputIntents".to_vec(), copied))?; } diff --git a/crates/organize/src/tests.rs b/crates/organize/src/tests.rs index c6d5b8394..50f35ebe7 100644 --- a/crates/organize/src/tests.rs +++ b/crates/organize/src/tests.rs @@ -416,6 +416,41 @@ fn combine_concatenates_with_a_bookmark_per_file() { assert_eq!(dest_page, pages(&out).unwrap()[3].obj); } +#[test] +fn combiner_adds_each_source_then_releases_it() { + let bytes = Arc::new(fixture()); + let src = Document::open(bytes.clone()).unwrap(); + let mut combiner = crate::Combiner::new(); + combiner.add("A", &src, None).unwrap(); + drop(src); + // No output intents to copy, so the combine keeps nothing of the source's file. + assert_eq!(Arc::strong_count(&bytes), 1); + let picked: &[usize] = &[1]; + combiner.add("B", &open(fixture()), Some(picked)).unwrap(); + let streamed = combiner.finish().unwrap(); + let whole = combine_selected(&[("A", &open(fixture()), None), ("B", &open(fixture()), Some(picked))]).unwrap(); + assert_eq!(write_full(&streamed, &SaveOptions::default()).unwrap(), write_full(&whole, &SaveOptions::default()).unwrap()); +} + +#[test] +fn combining_hostile_outlines_ends_their_runs_too() { + // The bookmark cases of `hostile_outlines_end_their_runs_and_never_loop`, combined: each run ends + // where it would repeat, and the source's bookmarks sit under its own entry. + let cases: [(&[&str], &str); 3] = [ + (&["<< /Title (Self) /Parent 3 0 R /Next 5 0 R >>"], "H[Self]"), + (&["<< /Title (A) /Parent 3 0 R /Next 6 0 R >>", "<< /Title (B) /Parent 3 0 R /Next 5 0 R >>"], "H[A,B]"), + (&["<< /Title (A) /Parent 3 0 R /Next 99 0 R >>"], "H[A]"), + ]; + for (items, listed) in cases { + let out = full_roundtrip(&combine(&[("H", &outline_with(items))]).unwrap()); + assert_eq!(titles(&crate::bookmarks(&out)), [listed]); + } + // A bookmark whose first child is itself: nesting is cut off, so the combine still ends. + let looped = outline_with(&["<< /Title (Self) /Parent 3 0 R /First 5 0 R >>"]); + let out = full_roundtrip(&combine(&[("H", &looped)]).unwrap()); + assert_eq!(crate::bookmarks(&out).first().map(|b| b.title.as_str()), Some("H")); +} + #[test] fn inherited_attributes_travel_with_copied_pages() { let out = full_roundtrip(&extract_pages(&doc_b(), &[1]).unwrap()); diff --git a/crates/ui-egui/src/create_multiple_ui.rs b/crates/ui-egui/src/create_multiple_ui.rs index f297306b2..19b15c5ee 100644 --- a/crates/ui-egui/src/create_multiple_ui.rs +++ b/crates/ui-egui/src/create_multiple_ui.rs @@ -4,38 +4,69 @@ use std::sync::Arc; +use pdfcraft_engine::EditError; + use crate::PdfCraftApp; fn stem(name: &str) -> &str { name.rsplit_once('.').map_or(name, |(s, _)| s) } +/// Why Create Multiple stopped early: a picked file couldn't be read, or the PDF couldn't be made. +enum Stop { + Read { name: String, error: String }, + Build(EditError), +} + +impl From for Stop { + fn from(e: EditError) -> Self { + Stop::Build(e) + } +} + impl PdfCraftApp { /// Convert the picked files, join them in the order picked, and open the result in the page - /// grid. A file that can't be converted is left out (and named in a notice). - pub(crate) fn stage_create_multiple(&mut self, files: Vec<(String, Vec)>) { - let max = pdfcraft_engine::MAX_CREATE_FILES; - let too_many = files.len() > max; - let (mut sources, mut refused) = (Vec::new(), Vec::new()); - for (name, bytes) in files.into_iter().take(max) { - match self.session.convert_to_pdf(&name, &Arc::new(bytes)) { - Ok((_, pdf)) => sources.push((stem(&name).to_string(), pdf, None)), - Err(_) => refused.push(name), + /// grid. A file that can't be converted is left out (and named in a notice); a file that can't be + /// read stops the run. Files are read as their turn comes, so one file's bytes are held at a time. + pub(crate) fn stage_create_multiple(&mut self, files: I) + where + I: IntoIterator), (String, String)>>, + { + let (mut refused, mut converted) = (Vec::new(), 0usize); + let mut files = files.into_iter(); + let session = &self.session; + let sources = std::iter::from_fn(|| { + loop { + match files.next()? { + Ok((name, bytes)) => match session.convert_to_pdf(&name, &Arc::new(bytes)) { + Ok((_, pdf)) => { + converted += 1; + return Some(Ok((stem(&name).to_string(), pdf, None))); + } + Err(_) => refused.push(name), + }, + Err((name, error)) => return Some(Err(Stop::Read { name, error })), + } + } + }); + let bytes = match session.combine_each(sources) { + Ok(Some(bytes)) => bytes, + Ok(None) => { + self.notify_tr("None of those files can be made into a PDF; use PDFs, images or .txt files"); + return; + } + Err(Stop::Read { name, error }) => { + self.notify_fmt("Couldn't read {name}: {e}", &[("name", &name), ("e", &error)]); + return; + } + Err(Stop::Build(e)) => { + self.notify_fmt("Couldn't create a PDF: {e}", &[("e", &e.to_string())]); + return; } - } - if sources.is_empty() { - self.notify_tr("None of those files can be made into a PDF; use PDFs, images or .txt files"); - return; - } - let bytes = match self.session.combine_ranges(&sources) { - Ok(bytes) => bytes, - Err(e) => return self.notify_fmt("Couldn't create a PDF: {e}", &[("e", &e.to_string())]), }; - let n = sources.len().to_string(); + let n = converted.to_string(); let message = if !refused.is_empty() { crate::i18n::fmt(tl!("Created a PDF from {n} files; left out: {names}"), &[("n", &n), ("names", &refused.join(", "))]) - } else if too_many { - crate::i18n::fmt(tl!("Created a PDF from the first {n} files"), &[("n", &n)]) } else { crate::i18n::fmt(tl!("Created a PDF from {n} files"), &[("n", &n)]) }; diff --git a/crates/ui-egui/src/files.rs b/crates/ui-egui/src/files.rs index 381759e1f..4ada03012 100644 --- a/crates/ui-egui/src/files.rs +++ b/crates/ui-egui/src/files.rs @@ -172,6 +172,16 @@ impl Default for RotateDraft { } } +/// A picked file's name and bytes, or its name and why it can't be read. +#[cfg(not(target_arch = "wasm32"))] +fn read_named(path: &std::path::Path) -> Result<(String, Vec), (String, String)> { + let name = path.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| "file.pdf".into()); + match std::fs::read(path) { + Ok(bytes) => Ok((name, bytes)), + Err(e) => Err((name, e.to_string())), + } +} + impl PdfCraftApp { /// Ask for files to add to the Combine files list (its Add files… button). pub fn combine_dialog(&mut self) { @@ -239,6 +249,13 @@ impl PdfCraftApp { /// Read the picked files and use them. #[cfg(not(target_arch = "wasm32"))] pub(crate) fn use_paths(&mut self, purpose: FilePurpose, paths: &[std::path::PathBuf]) { + if purpose == FilePurpose::CreateMultiple { + // Each file is read when its turn comes, so the selection's bytes are not all held at once. + if !paths.is_empty() { + self.stage_create_multiple(paths.iter().map(|p| read_named(p))); + } + return; + } let mut files = Vec::new(); let mut modified = Vec::new(); for p in paths { @@ -359,7 +376,7 @@ impl PdfCraftApp { } } FilePurpose::Ocr => self.ocr_files(files), - FilePurpose::CreateMultiple => self.stage_create_multiple(files), + FilePurpose::CreateMultiple => self.stage_create_multiple(files.into_iter().map(Ok)), FilePurpose::RedactWords => { let Some((name, bytes)) = files.into_iter().next() else { return }; if bytes.len() > crate::redact_ui::MAX_WORD_LIST_BYTES { @@ -406,13 +423,29 @@ impl PdfCraftApp { /// Insert the pages of `files` (PDFs, images, text), in order, at the gap a "+" in the page /// grid chose, or else after the selection (or the current page); then select them. pub fn insert_files(&mut self, files: Vec<(String, Vec)>) { + self.insert_each(files.into_iter().map(Ok)); + } + + /// [`Self::insert_files`] for files read as their turn comes: one that can't be read is skipped + /// and named. + pub(crate) fn insert_each(&mut self, files: I) + where + I: IntoIterator), (String, String)>>, + { let Some(view) = self.active.and_then(|i| self.views.get_mut(i)) else { return }; let (id, chosen) = (view.id, view.insert_at.take()); let after = view.target_pages().last().map_or(0, |p| p + 1); let count = self.session.get(id).map_or(0, |d| d.info.pages.len()); let start = chosen.unwrap_or(after).min(count); let mut at = start; - for (name, bytes) in files { + for file in files { + let (name, bytes) = match file { + Ok(file) => file, + Err((name, error)) => { + self.notify_fmt("Couldn't read {name}: {e}", &[("name", &name), ("e", &error)]); + continue; + } + }; let converted = self.session.convert_to_pdf(&name, &Arc::new(bytes)).and_then(|(_, pdf)| Ok((self.session.page_count_of(&name, &pdf)?, pdf))); match converted { diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 07f3ab58a..ad9fbf6ac 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -220,10 +220,23 @@ pub enum QuickTool { ColumnSelect, } +/// A file dropped on a page grid: its name, and its bytes or, for a file on disk, the path they are +/// read from when the file's turn to be inserted comes (so a long drop holds one file at a time). +type GridFile = (String, Result, std::path::PathBuf>); + +/// Read a file dropped on a page grid (see [`GridFile`]). +fn read_grid_file((name, source): GridFile) -> Result<(String, Vec), (String, String)> { + let bytes = match source { + Ok(bytes) => bytes, + Err(path) => std::fs::read(&path).map_err(|e| (name.clone(), e.to_string()))?, + }; + Ok((name, bytes)) +} + /// Files dropped on a document's page grid. struct GridDrop { doc: pdfcraft_engine::DocId, - files: Vec<(String, Vec)>, + files: Vec, /// When to stop waiting for the pointer (egui time, seconds). deadline: f64, } @@ -1035,11 +1048,14 @@ impl PdfCraftApp { fn drop_on_grid(&mut self, dropped: Vec, ctx: &egui::Context) -> Vec { let Some(id) = self.grid_target().filter(|_| !dropped.is_empty()) else { return dropped }; let mut files = Vec::new(); - for f in dropped.iter().take(pdfcraft_engine::MAX_CREATE_FILES) { + for f in &dropped { let name = f.path().file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| "dropped.pdf".into()); - let bytes = if f.path().is_absolute() { std::fs::read(f.path()).map_err(|e| e.to_string()) } else { f.bytes() }; - match bytes { - Ok(b) => files.push((name, b)), + if f.path().is_absolute() { + files.push((name, Err(f.path().to_path_buf()))); + continue; + } + match f.bytes() { + Ok(b) => files.push((name, Ok(b))), Err(e) => self.notify_fmt("Couldn't read {name}: {e}", &[("name", &name), ("e", &e)]), } } @@ -1069,7 +1085,7 @@ impl PdfCraftApp { }; view.insert_at = Some(gap); if let Some(drop) = self.grid_drop.take() { - self.insert_files(drop.files); + self.insert_each(drop.files.into_iter().map(read_grid_file)); } } diff --git a/crates/ui-egui/tests/create.rs b/crates/ui-egui/tests/create.rs index ee81616b5..36b5e97cd 100644 --- a/crates/ui-egui/tests/create.rs +++ b/crates/ui-egui/tests/create.rs @@ -250,3 +250,15 @@ fn multiple_files_that_cannot_be_converted_open_nothing() { assert!(app.views.is_empty()); assert!(app.toast.is_some()); } + +#[test] +fn multiple_files_past_a_thousand_are_all_used() { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + let pdf = app.session.create_from_text("a", "from a pdf").unwrap(); + let files: Vec<(String, Vec)> = (0..1001).map(|i| (format!("f{i}.pdf"), pdf.to_vec())).collect(); + app.use_files(pdfcraft_ui_egui::FilePurpose::CreateMultiple, files); + assert_eq!(app.views.len(), 1); + let doc = app.session.get(app.views[0].id).unwrap(); + assert_eq!(doc.info.pages.len(), 1001, "every picked file is used, in order"); +} diff --git a/crates/ui-egui/tests/ui.rs b/crates/ui-egui/tests/ui.rs index b35743693..38cde809b 100644 --- a/crates/ui-egui/tests/ui.rs +++ b/crates/ui-egui/tests/ui.rs @@ -616,6 +616,43 @@ fn dropping_a_pdf_on_the_window_opens_it() { h.get_by_label_contains("dropped.pdf"); } +#[test] +fn a_file_dropped_on_the_page_grid_is_read_from_disk_and_inserted() { + // The grid's drop reads a file from its path when its turn to be inserted comes, and inserts it + // at the end when the pointer doesn't say where it went (about a second). + let dir = std::env::temp_dir().join(format!("pdfcraft-grid-drop-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("dropped.pdf"); + let setup = PdfCraftApp::new(); + let pdf = setup.session.create_from_text("dropped", "from disk").unwrap(); + std::fs::write(&path, pdf.as_slice()).unwrap(); + let mut h = Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_eframe(|_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + let grid = app.session.create_from_text("grid", "one page").unwrap(); + app.open_bytes("grid.pdf", None, grid.to_vec()).unwrap(); + app.views[0].organize = true; + app + }); + h.run_steps(3); + let pages = |h: &egui_kittest::Harness<'_, PdfCraftApp>| { + let app = h.state(); + app.session.get(app.views[0].id).unwrap().info.pages.len() + }; + assert_eq!(pages(&h), 1); + let file: egui::DroppedFileHandle = std::sync::Arc::new(Dropped { path: path.clone(), bytes: Vec::new() }); + h.input_mut().dropped_files.push(file); + for _ in 0..300 { + h.run_steps(1); + if pages(&h) == 2 { + break; + } + std::thread::sleep(std::time::Duration::from_millis(10)); + } + assert_eq!(pages(&h), 2, "the dropped file's page is inserted at the end of the grid"); + let _ = std::fs::remove_dir_all(&dir); +} + #[test] fn pdfs_dropped_on_the_combine_tab_join_its_list() { let mut h = Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_eframe(|_cc| {