From f492d27454b0ec62c4558e3769edb899ff8f74a5 Mon Sep 17 00:00:00 2001 From: dan1982code Date: Mon, 19 Oct 2026 12:37:00 -0400 Subject: [PATCH 1/2] Save a picture from a PDF with the context menu A PDF names its picture by where its object sits rather than by a path, so the image lines the parser wrote carried no reference at all and the context menu's "Save image as..." never appeared on one. The marker now carries `pdf::`, written for an untagged page's image objects, for the images a tagged page's tree never claims, and for a real Figure element's picture (found through the marked-content ids its content names). The picture is read back through pdfium at the resolution the file stores it in, converted from whichever bitmap layout pdfium hands back, and written as PNG; a protected file is reopened with the password the session was given. Co-Authored-By: Claude Code --- crates/paperback-core/src/parser/pdf.rs | 43 ++- .../paperback-core/src/parser/pdf/images.rs | 129 ++------ .../src/parser/pdf/structure.rs | 52 +++- .../src/parser/pdf/structure/tests.rs | 9 +- crates/paperback-core/src/pdfium.rs | 4 +- crates/paperback-core/src/pdfium/page.rs | 293 ++++++++++++++++++ crates/paperback-core/src/session.rs | 4 + crates/paperback-core/src/session/images.rs | 33 ++ crates/paperback-core/src/session/tests.rs | 3 + .../src/session/tests/accessors.rs | 5 + .../paperback-core/src/session/tests/audio.rs | 1 + .../src/session/tests/find_all.rs | 1 + .../paperback-core/src/session/tests/links.rs | 1 + .../src/session/tests/navigation.rs | 1 + .../src/session/tests/webview.rs | 4 + crates/paperback-core/src/session/window.rs | 1 + doc/readme.md | 2 +- 17 files changed, 472 insertions(+), 114 deletions(-) diff --git a/crates/paperback-core/src/parser/pdf.rs b/crates/paperback-core/src/parser/pdf.rs index df89d7e1..bf558fc4 100644 --- a/crates/paperback-core/src/parser/pdf.rs +++ b/crates/paperback-core/src/parser/pdf.rs @@ -20,9 +20,7 @@ mod tagged; mod text; mod toc; -use images::{ - PAGE_IMAGE_COVERAGE, append_images, images_before_each_paragraph, page_image_tops, page_largest_image_coverage, -}; +use images::{PAGE_IMAGE_COVERAGE, append_image, images_before_each_paragraph, page_largest_image_coverage}; use links::{PendingLink, collect_annotation_links, collect_web_links, place_links}; use metadata::{map_load_error, metadata_title, metadata_value}; pub use paragraphs::join_wrapped_lines; @@ -53,6 +51,9 @@ struct PageContent { annotation_links: Vec, /// The top edge of each image the page draws, ordered down the page. image_tops: Vec, + /// Where each of those images sits in the page's content stream, in the same order, so the + /// picture behind an image line can be saved later. + image_paths: Vec, /// The fraction of the page the largest single image covers. Near 1.0 marks a scanned page. largest_image_coverage: f64, } @@ -76,8 +77,10 @@ fn read_page( // because each page places its own images and because an image-only page still needs its OCR // placeholder when earlier pages contributed text. Taken before the text so that a tagged // page whose tree names no figure can place them among its blocks as it writes them. + let announced = page.announced_images(); let mut content = PageContent { - image_tops: page_image_tops(&page), + image_tops: announced.iter().map(|image| image.top).collect(), + image_paths: announced.into_iter().map(|image| image.path).collect(), largest_image_coverage: page_largest_image_coverage(&page), ..Default::default() }; @@ -97,6 +100,7 @@ fn read_page( &mut tagged.toc_items, render_tables_inline, &content.image_tops, + &content.image_paths, evidence, ) { content.tagged = Some(tagged); @@ -183,8 +187,9 @@ fn looks_like_page_number(text: &str) -> bool { enum UntaggedLine { /// A paragraph, and whether the size it was set in marked it out as a heading. Text(String, bool), - /// An image the page drew here. An untagged page has no description to give for one. - Image, + /// An image the page drew here, and where its picture sits in the content stream. An + /// untagged page has no description to give for one. + Image(String), } /// Work out what an untagged page contributes: its running headers dropped, its wrapped lines @@ -215,11 +220,21 @@ fn untagged_page_lines(page: &PageContent, running_text: &RunningText, join: boo .collect(); let image_counts = images_before_each_paragraph(&page.image_tops, ¶graph_tops); let mut lines = Vec::with_capacity(paragraphs.len()); + let mut next_image = 0usize; + let take_image = |next_image: &mut usize| { + let path = page.image_paths.get(*next_image).cloned().unwrap_or_default(); + *next_image += 1; + UntaggedLine::Image(path) + }; for (index, (text, is_heading, _)) in paragraphs.iter().enumerate() { - lines.extend((0..image_counts[index]).map(|_| UntaggedLine::Image)); + for _ in 0..image_counts[index] { + lines.push(take_image(&mut next_image)); + } lines.push(UntaggedLine::Text(text.clone(), *is_heading)); } - lines.extend((0..image_counts[paragraphs.len()]).map(|_| UntaggedLine::Image)); + for _ in 0..image_counts[paragraphs.len()] { + lines.push(take_image(&mut next_image)); + } lines } @@ -436,8 +451,16 @@ impl Parser for PdfParser { } else { for line in untagged_page_lines(&page, &running_text, context.join_pdf_paragraphs) { match line { - UntaggedLine::Image => { - append_images(1, &mut buffer, &mut page_display_text, &mut current_lines_info); + UntaggedLine::Image(path) => { + let reference = format!("pdf:{page_index}:{path}"); + append_image( + &mut buffer, + MarkerType::Image, + "", + &mut page_display_text, + &mut current_lines_info, + &reference, + ); } UntaggedLine::Text(text, is_heading) => { has_any_text = true; diff --git a/crates/paperback-core/src/parser/pdf/images.rs b/crates/paperback-core/src/parser/pdf/images.rs index 8ae1f01c..fe500730 100644 --- a/crates/paperback-core/src/parser/pdf/images.rs +++ b/crates/paperback-core/src/parser/pdf/images.rs @@ -5,9 +5,8 @@ //! this purpose, so [`super::structure`] places one by walking the tree and calls //! [`append_image`] as it meets one. //! -//! An untagged PDF says neither. All it has is image objects with page coordinates, so -//! [`page_image_tops`] collects those and the caller places each one among the page's text lines -//! by how far down the page it sits. +//! An untagged PDF says neither. All it has is image objects with page coordinates, so the +//! caller places each one among the page's text lines by how far down the page it sits. //! //! A tagged PDF that tags none of its images gets the same treatment through //! [`UnclaimedImages`]. A page like that draws pictures and claims not to have any, so trusting @@ -26,13 +25,7 @@ use crate::{ /// chain of them this deep is a malformed file rather than a page anyone meant to draw. const MAX_FORM_DEPTH: u32 = 8; -/// Shortest side, in PDF user units, an image must have to be worth a line of its own. A page -/// laid out from HTML draws its spacers and its hairline rules as images a fraction of a point -/// across; announcing one of those as an image gives the reader a stop with nothing at it. -const MIN_IMAGE_SIDE: f32 = 4.0; - -/// How much of the page one image can cover and still be a picture on the page rather than the page itself. A scanned book with an OCR text layer draws each sheet as one image under its invisible text, and announcing that put an "[Image]" with nothing behind it on every page (#1046). -pub(super) const PAGE_IMAGE_COVERAGE: f64 = 0.9; +pub(super) use crate::pdfium::PAGE_IMAGE_COVERAGE; /// Writes one image's line into the page, with a marker spanning it. /// @@ -45,13 +38,19 @@ pub(super) fn append_image( description: &str, page_display_text: &mut String, current_lines_info: &mut Vec<(usize, String)>, + reference: &str, ) { // TRANSLATORS: Label inserted before a figure or image's description, e.g. "[Figure: a cat sleeping]" let label = if mtype == MarkerType::Figure { t("Figure") } else { t("Image") }; let line = if description.is_empty() { format!("[{label}]") } else { format!("[{label}: {description}]") }; let offset = buffer.current_position(); current_lines_info.push((offset, line.clone())); - buffer.add_marker(Marker::new(mtype, offset).with_text(description.to_string()).with_length(display_len(&line))); + buffer.add_marker( + Marker::new(mtype, offset) + .with_text(description.to_string()) + .with_length(display_len(&line)) + .with_reference(reference.to_string()), + ); buffer.append(&line); buffer.append("\n"); // Kept in step with the buffer because `place_links` measures its offsets against this, and @@ -60,24 +59,6 @@ pub(super) fn append_image( page_display_text.push('\n'); } -/// The top edge, in PDF user units, of every image the page draws, ordered down the page. -/// -/// Images reached through a form `XObject` count. A page whose content is wrapped in one form (a -/// scanner's output and a placed-artwork export both look like this) draws no image object of its -/// own, so stopping at the top level would find nothing on it at all. -pub(super) fn page_image_tops(page: &PdfPage) -> Vec { - let page_area = f64::from(page.width()) * f64::from(page.height()); - let mut tops = Vec::new(); - for i in 0..page.object_count() { - if let Some(object) = page.object(i) { - collect_image_tops(&object, 0, page_area, &mut tops); - } - } - // Y grows up the page, so the image nearest the top of it has the largest one. - tops.sort_unstable_by(|a, b| b.total_cmp(a)); - tops -} - /// The fraction of the page one single image covers at most, from 0.0 to 1.0. /// /// A scanned page is one big image laid over the whole sheet, so a value near 1.0 is the mark of @@ -119,43 +100,6 @@ fn collect_largest_image_area(object: &PageObject, depth: u32, largest: &mut f64 } } -fn collect_image_tops(object: &PageObject, depth: u32, page_area: f64, tops: &mut Vec) { - match object.kind() { - ObjectKind::Image => { - if let Some(bounds) = object.bounds() - && is_announced_image((bounds.right - bounds.left).abs(), (bounds.top - bounds.bottom).abs(), page_area) - { - tops.push(f64::from(bounds.top)); - } - } - ObjectKind::Form if depth < MAX_FORM_DEPTH => { - for i in 0..object.form_object_count() { - if let Some(child) = object.form_object(i) { - collect_image_tops(&child, depth + 1, page_area, tops); - } - } - } - _ => {} - } -} - -fn is_announced_image(width: f32, height: f32, page_area: f64) -> bool { - let covers_page = page_area > 0.0 && f64::from(width) * f64::from(height) / page_area >= PAGE_IMAGE_COVERAGE; - width >= MIN_IMAGE_SIDE && height >= MIN_IMAGE_SIDE && !covers_page -} - -/// Writes `count` undescribed images into the page, one line each. -pub(super) fn append_images( - count: usize, - buffer: &mut DocumentBuffer, - page_display_text: &mut String, - current_lines_info: &mut Vec<(usize, String)>, -) { - for _ in 0..count { - append_image(buffer, MarkerType::Image, "", page_display_text, current_lines_info); - } -} - /// The images a page draws that its structure tree never claimed, waiting to go out among the /// page's blocks. /// @@ -165,18 +109,21 @@ pub(super) fn append_images( /// block that starts below it. pub(super) struct UnclaimedImages { /// Down the page, nearest the top first, and drained from the front as they are placed. - tops: std::collections::VecDeque, + images: std::collections::VecDeque<(f64, String)>, + /// The page they were found on, which is part of the reference that saves one. + page_index: i32, } impl UnclaimedImages { - /// The images to place. Empty for a page whose tree claims its figures, which is every tagged - /// page written the usual way, and the walk then does none of this. - pub(super) fn new(tops: &[f64]) -> Self { - Self { tops: tops.iter().copied().collect() } + /// The images to place, as their tops and content-stream paths. Empty for a page whose tree + /// claims its figures, which is every tagged page written the usual way, and the walk then + /// does none of this. + pub(super) fn new(tops: &[f64], paths: &[String], page_index: i32) -> Self { + Self { images: tops.iter().copied().zip(paths.iter().cloned()).collect(), page_index } } pub(super) fn is_empty(&self) -> bool { - self.tops.is_empty() + self.images.is_empty() } /// Writes every image drawn above `top`. Called with the top edge of each block just before @@ -188,9 +135,11 @@ impl UnclaimedImages { page_display_text: &mut String, current_lines_info: &mut Vec<(usize, String)>, ) { - while self.tops.front().is_some_and(|image_top| *image_top > top) { - self.tops.pop_front(); - append_image(buffer, MarkerType::Image, "", page_display_text, current_lines_info); + while self.images.front().is_some_and(|(image_top, _)| *image_top > top) { + if let Some((_, path)) = self.images.pop_front() { + let reference = format!("pdf:{}:{path}", self.page_index); + append_image(buffer, MarkerType::Image, "", page_display_text, current_lines_info, &reference); + } } } } @@ -218,27 +167,9 @@ pub(super) fn images_before_each_paragraph(image_tops: &[f64], paragraph_tops: & #[cfg(test)] mod tests { - use super::{UnclaimedImages, append_image, images_before_each_paragraph, is_announced_image}; + use super::{UnclaimedImages, append_image, images_before_each_paragraph}; use crate::document::{DocumentBuffer, MarkerType}; - /// The page from #1046: 323 by 518 points, under a 323 by 496 point scan. - const PAGE_AREA: f64 = 323.0 * 518.0; - - #[test] - fn a_scan_under_the_whole_page_is_not_announced() { - assert!(!is_announced_image(323.0, 496.0, PAGE_AREA)); - } - - #[test] - fn a_figure_on_the_page_is_announced() { - assert!(is_announced_image(250.0, 300.0, PAGE_AREA)); - } - - #[test] - fn a_hairline_rule_is_not_announced() { - assert!(!is_announced_image(300.0, 0.5, PAGE_AREA)); - } - /// A described figure reads as its description, and the marker spans the whole line so that /// arrowing onto any part of it lands on the figure. #[test] @@ -246,7 +177,7 @@ mod tests { let mut buffer = DocumentBuffer::new(); let mut page_text = String::new(); let mut lines_info = Vec::new(); - append_image(&mut buffer, MarkerType::Figure, "a cat sleeping", &mut page_text, &mut lines_info); + append_image(&mut buffer, MarkerType::Figure, "a cat sleeping", &mut page_text, &mut lines_info, "pdf:2:0/1"); assert_eq!(buffer.content, "[Figure: a cat sleeping]\n"); assert_eq!(page_text, buffer.content, "the display text has to mirror the buffer for link placement"); assert_eq!(lines_info.len(), 1); @@ -254,6 +185,7 @@ mod tests { assert_eq!(marker.position, 0); assert_eq!(marker.text, "a cat sleeping", "the marker keeps the description on its own"); assert_eq!(marker.length, "[Figure: a cat sleeping]".len()); + assert_eq!(marker.reference, "pdf:2:0/1", "the marker names where the picture sits so it can be saved"); } /// An image nobody described still gets a line: it is the only sign a reader who cannot see @@ -263,7 +195,7 @@ mod tests { let mut buffer = DocumentBuffer::new(); let mut page_text = String::new(); let mut lines_info = Vec::new(); - append_image(&mut buffer, MarkerType::Image, "", &mut page_text, &mut lines_info); + append_image(&mut buffer, MarkerType::Image, "", &mut page_text, &mut lines_info, ""); assert_eq!(buffer.content, "[Image]\n"); assert!(buffer.markers.iter().any(|m| m.mtype == MarkerType::Image)); } @@ -298,7 +230,8 @@ mod tests { /// sits below every block and goes out when the page ends. #[test] fn unclaimed_images_go_in_front_of_the_first_block_below_each() { - let mut images = UnclaimedImages::new(&[700.0, 500.0, 100.0]); + let mut images = + UnclaimedImages::new(&[700.0, 500.0, 100.0], &["0".to_string(), "1".to_string(), "2".to_string()], 0); let mut buffer = DocumentBuffer::new(); let mut page_text = String::new(); let mut lines_info = Vec::new(); @@ -324,7 +257,7 @@ mod tests { /// writes an image line of its own. #[test] fn a_page_whose_tree_claims_its_figures_places_nothing() { - let mut images = UnclaimedImages::new(&[]); + let mut images = UnclaimedImages::new(&[], &[], 0); let mut buffer = DocumentBuffer::new(); let mut page_text = String::new(); let mut lines_info = Vec::new(); diff --git a/crates/paperback-core/src/parser/pdf/structure.rs b/crates/paperback-core/src/parser/pdf/structure.rs index e256fccc..b7024551 100644 --- a/crates/paperback-core/src/parser/pdf/structure.rs +++ b/crates/paperback-core/src/parser/pdf/structure.rs @@ -52,6 +52,7 @@ pub(super) fn extract_tagged_page_text( flat_toc_items: &mut Vec<(u32, TocItem)>, render_tables_inline: bool, image_tops: &[f64], + image_paths: &[String], evidence: &mut HyphenEvidence, ) -> bool { let Some(struct_tree) = page.tags() else { return false }; @@ -63,7 +64,11 @@ pub(super) fn extract_tagged_page_text( // A tree that names its figures is trusted with them, and the page's own image objects are // then left alone: a `Figure` is not always one drawn image, so counting both would announce // some of them twice. - let unclaimed = UnclaimedImages::new(if facts.claims_figures { &[] } else { image_tops }); + let unclaimed = UnclaimedImages::new( + if facts.claims_figures { &[] } else { image_tops }, + if facts.claims_figures { &[] } else { image_paths }, + page_index, + ); let content = marked_content::read(text_page, &facts, !unclaimed.is_empty(), evidence); let coverage = content.coverage; let tagged_trusted = coverage >= MIN_MCID_COVERAGE; @@ -89,7 +94,16 @@ pub(super) fn extract_tagged_page_text( let mut current_block = String::new(); let mut pending_label = String::new(); let mut page_text = PageText::new(&content.text); - let mut images = ImagePlacement { mcid_tops: &content.tops, unclaimed, block_top: None }; + // Only a tree that names figures can ask for one's picture, so the walk over the page's + // objects that answers where each image sits is skipped for the rest. + let figure_images = if facts.claims_figures { page.image_paths_by_mcid() } else { HashMap::new() }; + let mut images = ImagePlacement { + mcid_tops: &content.tops, + unclaimed, + page_index, + figure_images: &figure_images, + block_top: None, + }; for i in 0..child_count { if let Some(child) = struct_tree.child(i) { process_struct_element( @@ -120,6 +134,11 @@ pub(super) fn extract_tagged_page_text( struct ImagePlacement<'a> { mcid_tops: &'a HashMap, unclaimed: UnclaimedImages, + /// The page the images belong to, which a marker's reference names so its picture can be saved. + page_index: i32, + /// Where each marked-content id's picture sits in the content stream. A `Figure` element + /// points at its picture by marked-content id, and this is what turns that into a reference. + figure_images: &'a HashMap, /// The top edge of the block being built, from the first piece of text in it. block_top: Option, } @@ -161,6 +180,13 @@ impl ImagePlacement<'_> { self.unclaimed.place_above(top, buffer, page_display_text, current_lines_info); } + /// The reference that saves the picture behind `elem`, or an empty string when its content + /// names no image Paperback can find on the page. + fn figure_reference(&self, elem: &Tag) -> String { + find_image_path(elem, self.figure_images) + .map_or_else(String::new, |path| format!("pdf:{}:{path}", self.page_index)) + } + /// Write the images left below everything the page wrote. fn after_page( &mut self, @@ -172,6 +198,25 @@ impl ImagePlacement<'_> { } } +/// The content-stream path of the first image under `elem`, found by the marked-content ids its +/// leaves carry. A figure's picture is usually a direct child, but a maker that wraps it in a +/// `Div` or a `Caption`'s sibling nests it a level or two down, so the whole subtree is searched. +fn find_image_path(elem: &Tag, image_paths: &HashMap) -> Option { + let count = elem.child_count(); + for i in 0..count { + if let Some(child) = elem.child(i) { + if let Some(path) = find_image_path(&child, image_paths) { + return Some(path); + } + } else if let Some(mcid) = elem.child_mcid(i) + && let Some(path) = image_paths.get(&mcid) + { + return Some(path.clone()); + } + } + None +} + /// Replaces a list label made only of private-use characters with a plain bullet. /// /// The Symbol and Wingdings bullets reach the text layer as private-use code points (U+F0B7 for @@ -303,7 +348,8 @@ fn process_struct_element( .unwrap_or_default(); images.note_element(elem); images.before_line(buffer, page_display_text, current_lines_info); - append_image(buffer, MarkerType::Figure, &description, page_display_text, current_lines_info); + let reference = images.figure_reference(elem); + append_image(buffer, MarkerType::Figure, &description, page_display_text, current_lines_info, &reference); // The description goes in beside what the figure draws rather than in place of it, which // is what a PDF reader following /Alt to the letter would do. A figure is not always only // a picture: a travel guide measured for this wraps its page numbers, its offers and the diff --git a/crates/paperback-core/src/parser/pdf/structure/tests.rs b/crates/paperback-core/src/parser/pdf/structure/tests.rs index 6f1276a2..3e39d721 100644 --- a/crates/paperback-core/src/parser/pdf/structure/tests.rs +++ b/crates/paperback-core/src/parser/pdf/structure/tests.rs @@ -111,7 +111,14 @@ fn a_list_label_joins_the_line_that_follows_it() { let mut label = "\u{2022}".to_string(); let mut block = String::new(); let tops = HashMap::new(); - let mut no_images = ImagePlacement { mcid_tops: &tops, unclaimed: UnclaimedImages::new(&[]), block_top: None }; + let figure_images = HashMap::new(); + let mut no_images = ImagePlacement { + mcid_tops: &tops, + unclaimed: UnclaimedImages::new(&[], &[], 0), + page_index: 0, + figure_images: &figure_images, + block_top: None, + }; // The paragraph inside the LBody starts by flushing, with nothing yet collected. flush_block(&mut label, &mut block, &mut buffer, &mut page_text, &mut lines_info, &mut no_images); assert_eq!(buffer.content, "", "an empty block emits nothing and keeps the label waiting"); diff --git a/crates/paperback-core/src/pdfium.rs b/crates/paperback-core/src/pdfium.rs index c296e7eb..afc9faa3 100644 --- a/crates/paperback-core/src/pdfium.rs +++ b/crates/paperback-core/src/pdfium.rs @@ -33,7 +33,9 @@ mod text; // touch. Some of these are only ever reached as the return type of a method rather than named // outright, which is what the allow is for. #[allow(unused_imports)] -pub use page::{AnnotationLink, Bounds, LinkTarget, ObjectKind, PageBitmap, PageObject, PdfPage}; +pub use page::{ + AnnotationLink, Bounds, LinkTarget, ObjectKind, PAGE_IMAGE_COVERAGE, PageBitmap, PageImage, PageObject, PdfPage, +}; use pdfium_render::prelude::{FPDF_BOOKMARK, FPDF_DOCUMENT, Pdfium, PdfiumLibraryBindings}; #[allow(unused_imports)] pub use tags::{Tag, TagTree}; diff --git a/crates/paperback-core/src/pdfium/page.rs b/crates/paperback-core/src/pdfium/page.rs index 15cb305c..3219eabb 100644 --- a/crates/paperback-core/src/pdfium/page.rs +++ b/crates/paperback-core/src/pdfium/page.rs @@ -68,6 +68,52 @@ impl<'doc> PdfPage<'doc> { PageObject::of(unsafe { bindings().FPDFPage_GetObject(self.handle, index) }) } + /// Every image the page draws that is worth a line of its own, nearest the top first. + /// + /// This is the one list the parser places image lines from, so a page's images are announced + /// in a fixed order. + pub fn announced_images(&self) -> Vec> { + let page_area = f64::from(self.width()) * f64::from(self.height()); + let mut images = Vec::new(); + for i in 0..self.object_count() { + if let Some(object) = self.object(i) { + collect_images(&object, 0, page_area, true, &i.to_string(), &mut images); + } + } + images.sort_by(|a: &PageImage<'_>, b| b.top.total_cmp(&a.top)); + images + } + + /// The picture at `path`, encoded as PNG. `path` is the one an announced image carries in + /// [`PageImage::path`], as it was written into an image marker's reference. + pub fn image_png(&self, path: &str) -> Option> { + let mut parts = path.split('/'); + let first = i32::try_from(parts.next()?.parse::().ok()?).ok()?; + let object = self.object(first)?; + let handle = descend_object(&object, parts)?; + encode_image_png(handle) + } + + /// The image path for each marked-content id on the page. A tagged PDF's `Figure` element + /// points at its picture by marked-content id, and this is what turns that into a reference + /// that can be saved, whether or not the picture is one Paperback announces. + pub fn image_paths_by_mcid(&self) -> std::collections::HashMap { + let page_area = f64::from(self.width()) * f64::from(self.height()); + let mut all = Vec::new(); + for i in 0..self.object_count() { + if let Some(object) = self.object(i) { + collect_images(&object, 0, page_area, false, &i.to_string(), &mut all); + } + } + let mut paths = std::collections::HashMap::new(); + for image in all { + for mcid in image.marked_content_ids() { + paths.entry(mcid).or_insert_with(|| image.path.clone()); + } + } + paths + } + /// Rasterizes the page `width` pixels across, keeping its aspect ratio, as RGBA8. /// /// Pdfium draws BGRA, so the channels are swapped on the way out rather than leaving every @@ -183,6 +229,189 @@ impl Drop for PdfPage<'_> { } } +/// Shortest side, in PDF user units, an image must have to be worth a line of its own. A page +/// laid out from HTML draws its spacers and its hairline rules as images a fraction of a point +/// across; announcing one of those as an image gives the reader a stop with nothing at it. +const MIN_IMAGE_SIDE: f32 = 4.0; + +/// How much of the page one image can cover and still be a picture on the page rather than the +/// page itself. A scanned book with an OCR text layer draws each sheet as one image under its +/// invisible text, and announcing that put an "[Image]" with nothing behind it on every page +/// (#1046). +pub const PAGE_IMAGE_COVERAGE: f64 = 0.9; + +/// `FPDFBitmap_BGRA`, `FPDFBitmap_BGRx`, `FPDFBitmap_BGR` and `FPDFBitmap_Gray`, the four +/// bitmap layouts pdfium hands back. +const BITMAP_GRAY: i32 = 1; +const BITMAP_BGR: i32 = 2; +const BITMAP_BGRX: i32 = 3; + +/// One image a page draws, as Paperback announces it. +/// +/// Borrows the page it came from. Nothing is closed here; the page owns the object. +pub struct PageImage<'page> { + /// The image's top edge, in PDF user units. Y grows up the page, so the nearest the top has + /// the largest one. + pub top: f64, + /// Where the image object sits in the page's content stream, as slash-separated indices. + /// This is what an image marker's reference carries, so [`PdfPage::image_png`] can find the + /// picture again without depending on the order images were listed in. + pub path: String, + handle: FPDF_PAGEOBJECT, + _page: std::marker::PhantomData<&'page ()>, +} + +impl PageImage<'_> { + /// The marked-content ids this image carries, which is how a tagged `Figure` names it. + fn marked_content_ids(&self) -> Vec { + let bindings = bindings(); + let mut ids = Vec::new(); + if let Some(id) = marked_content_id(self.handle) { + ids.push(id); + } + for index in 0..unsafe { bindings.FPDFPageObj_CountMarks(self.handle) } { + let mark = unsafe { bindings.FPDFPageObj_GetMark(self.handle, c_ulong::try_from(index).unwrap_or(0)) }; + if mark.is_null() { + continue; + } + let mut value = 0; + if unsafe { bindings.FPDFPageObjMark_GetParamIntValue(mark, "MCID", &mut value) } != 0 { + ids.push(value); + } + } + ids.sort_unstable(); + ids.dedup(); + ids + } +} + +fn marked_content_id(handle: FPDF_PAGEOBJECT) -> Option { + let id = unsafe { bindings().FPDFPageObj_GetMarkedContentID(handle) }; + (id >= 0).then_some(id) +} + +/// Follows the rest of a content-stream path into a form's children. +fn descend_object<'p, 'a>(object: &PageObject<'p>, mut path: impl Iterator) -> Option { + let Some(component) = path.next() else { return Some(object.handle()) }; + let index = i32::try_from(component.parse::().ok()?).ok()?; + let child = object.form_object(index)?; + descend_object(&child, path) +} + +/// The image's own pixels, encoded as PNG. +/// +/// The picture is taken at the resolution the file stores rather than as it is drawn on the +/// page, so a photo placed small still saves at full size. An image mask is not applied: the +/// PDF keeps the mask as a separate object, and the picture on its own is what was asked for. +fn encode_image_png(handle: FPDF_PAGEOBJECT) -> Option> { + let bindings = bindings(); + let bitmap = unsafe { bindings.FPDFImageObj_GetBitmap(handle) }; + if bitmap.is_null() { + return None; + } + let width = unsafe { bindings.FPDFBitmap_GetWidth(bitmap) }; + let height = unsafe { bindings.FPDFBitmap_GetHeight(bitmap) }; + let stride = unsafe { bindings.FPDFBitmap_GetStride(bitmap) }; + let format = unsafe { bindings.FPDFBitmap_GetFormat(bitmap) }; + let buffer = unsafe { bindings.FPDFBitmap_GetBuffer(bitmap) }; + let rgba = bitmap_to_rgba(buffer, width, height, stride, format); + unsafe { bindings.FPDFBitmap_Destroy(bitmap) }; + let (width, height, rgba) = rgba?; + let mut out = Vec::new(); + image::RgbaImage::from_raw(width, height, rgba)? + .write_to(&mut std::io::Cursor::new(&mut out), image::ImageFormat::Png) + .ok()?; + Some(out) +} + +/// Copies an image's bitmap into RGBA8, whatever layout pdfium handed back. +fn bitmap_to_rgba( + buffer: *const c_void, + width: i32, + height: i32, + stride: i32, + format: i32, +) -> Option<(u32, u32, Vec)> { + if buffer.is_null() || width <= 0 || height <= 0 { + return None; + } + let (w, h) = (usize::try_from(width).ok()?, usize::try_from(height).ok()?); + let stride = usize::try_from(stride).ok()?; + let bytes_per_pixel = match format { + BITMAP_GRAY => 1, + BITMAP_BGR => 3, + BITMAP_BGRX | BITMAP_BGRA => 4, + _ => return None, + }; + if stride < w * bytes_per_pixel { + return None; + } + let source = unsafe { std::slice::from_raw_parts(buffer.cast::(), stride * h) }; + let mut rgba = vec![255u8; w * h * 4]; + for row in 0..h { + for column in 0..w { + let from = row * stride + column * bytes_per_pixel; + let to = (row * w + column) * 4; + if format == BITMAP_GRAY { + let gray = source[from]; + rgba[to] = gray; + rgba[to + 1] = gray; + rgba[to + 2] = gray; + } else { + rgba[to] = source[from + 2]; + rgba[to + 1] = source[from + 1]; + rgba[to + 2] = source[from]; + if format == BITMAP_BGRA { + rgba[to + 3] = source[from + 3]; + } + } + } + } + Some((u32::try_from(width).ok()?, u32::try_from(height).ok()?, rgba)) +} + +/// Collects the image objects under `object`, walking into form `XObject`s and recording where +/// each one sits in the content stream. `only_announced` leaves out the images that are too +/// small to be worth a line or that cover the page themselves. +fn collect_images( + object: &PageObject<'_>, + depth: u32, + page_area: f64, + only_announced: bool, + path: &str, + images: &mut Vec>, +) { + match object.kind() { + ObjectKind::Image => { + let announced = object.bounds().is_some_and(|bounds| { + is_announced_image((bounds.right - bounds.left).abs(), (bounds.top - bounds.bottom).abs(), page_area) + }); + if !only_announced || announced { + let top = object.bounds().map_or(0.0, |bounds| f64::from(bounds.top)); + images.push(PageImage { + top, + path: path.to_string(), + handle: object.handle(), + _page: std::marker::PhantomData, + }); + } + } + ObjectKind::Form if depth < 8 => { + for i in 0..object.form_object_count() { + if let Some(child) = object.form_object(i) { + collect_images(&child, depth + 1, page_area, only_announced, &format!("{path}/{i}"), images); + } + } + } + _ => {} + } +} + +fn is_announced_image(width: f32, height: f32, page_area: f64) -> bool { + let covers_page = page_area > 0.0 && f64::from(width) * f64::from(height) / page_area >= PAGE_IMAGE_COVERAGE; + width >= MIN_IMAGE_SIDE && height >= MIN_IMAGE_SIDE && !covers_page +} + /// Copies pdfium's BGRA rows out into a tight RGBA buffer. /// /// The rows are padded to a stride, so this cannot be one flat copy; each row is taken at its @@ -260,6 +489,11 @@ impl PageObject<'_> { } } + /// The raw handle, for callers that speak FFI directly. + pub fn handle(&self) -> FPDF_PAGEOBJECT { + self.handle + } + pub fn bounds(&self) -> Option { let (mut left, mut bottom, mut right, mut top) = (0.0, 0.0, 0.0, 0.0); let ok = unsafe { bindings().FPDFPageObj_GetBounds(self.handle, &mut left, &mut bottom, &mut right, &mut top) }; @@ -319,3 +553,62 @@ impl ObjectMark<'_> { (ok != 0).then_some(value) } } + +#[cfg(test)] +mod tests { + use super::{BITMAP_BGR, BITMAP_BGRA, BITMAP_GRAY, bitmap_to_rgba, is_announced_image}; + + /// The page from #1046: 323 by 518 points, under a 323 by 496 point scan. + const PAGE_AREA: f64 = 323.0 * 518.0; + + #[test] + fn a_scan_under_the_whole_page_is_not_announced() { + assert!(!is_announced_image(323.0, 496.0, PAGE_AREA)); + } + + #[test] + fn a_figure_on_the_page_is_announced() { + assert!(is_announced_image(250.0, 300.0, PAGE_AREA)); + } + + #[test] + fn a_hairline_rule_is_not_announced() { + assert!(!is_announced_image(300.0, 0.5, PAGE_AREA)); + } + + /// Pdfium hands BGRA back, and a PNG wants RGBA. + #[test] + fn a_bgra_bitmap_becomes_rgba() { + // One pixel: blue, green, red, alpha. + let mut pixels = [0u8, 0, 255, 128]; + let (width, height, rgba) = bitmap_to_rgba(pixels.as_mut_ptr().cast(), 1, 1, 4, BITMAP_BGRA).unwrap(); + assert_eq!((width, height), (1, 1)); + assert_eq!(rgba, vec![255, 0, 0, 128]); + } + + /// Rows are padded to a stride, so the padding must not be read as pixels. + #[test] + fn a_padded_row_does_not_shift_the_next_one() { + // Two rows of two BGRA pixels with four bytes of padding between them. + let mut pixels = [1u8, 2, 3, 4, 5, 6, 7, 8, 0, 0, 0, 0, 9, 10, 11, 12, 13, 14, 15, 16]; + let (_, _, rgba) = bitmap_to_rgba(pixels.as_mut_ptr().cast(), 2, 2, 12, BITMAP_BGRA).unwrap(); + assert_eq!(&rgba[0..4], &[3, 2, 1, 4]); + assert_eq!(&rgba[8..12], &[11, 10, 9, 12], "the first pixel of the second row"); + } + + /// A grayscale bitmap, which an image PDF can hold, widens to RGBA. + #[test] + fn a_gray_bitmap_widens_to_rgba() { + let mut pixels = [17u8, 34]; + let (_, _, rgba) = bitmap_to_rgba(pixels.as_mut_ptr().cast(), 2, 1, 2, BITMAP_GRAY).unwrap(); + assert_eq!(rgba, vec![17, 17, 17, 255, 34, 34, 34, 255]); + } + + /// Three channels and no alpha, which pdfium can return for an image without transparency. + #[test] + fn a_bgr_bitmap_is_opaque() { + let mut pixels = [1u8, 2, 3]; + let (_, _, rgba) = bitmap_to_rgba(pixels.as_mut_ptr().cast(), 1, 1, 3, BITMAP_BGR).unwrap(); + assert_eq!(rgba, vec![3, 2, 1, 255]); + } +} diff --git a/crates/paperback-core/src/session.rs b/crates/paperback-core/src/session.rs index 9cfdb101..9ffacfd0 100644 --- a/crates/paperback-core/src/session.rs +++ b/crates/paperback-core/src/session.rs @@ -229,6 +229,9 @@ pub struct DocumentSession { parser_flags: ParserFlags, /// The settings the document was parsed with, which views that re-render its source must follow too. parse_settings: ParseSettings, + /// The password the document was opened with, so a second look at the file for something the + /// parse did not keep (the picture behind an image line) can get past the same lock. + password: String, } #[cfg_attr(feature = "uniffi", derive(uniffi::Record))] @@ -356,6 +359,7 @@ impl DocumentSession { history_index: 0, parser_flags, parse_settings: settings, + password: password.to_string(), }) } diff --git a/crates/paperback-core/src/session/images.rs b/crates/paperback-core/src/session/images.rs index 47508599..6bc91214 100644 --- a/crates/paperback-core/src/session/images.rs +++ b/crates/paperback-core/src/session/images.rs @@ -71,6 +71,15 @@ impl DocumentSession { if is_data_uri(source) { return decode_data_uri(source); } + // A PDF names its pictures by where their objects sit rather than by a file path, so the + // reference itself says which reader they need and the document's extension cannot. + if source.starts_with("pdf:") { + let (page_index, path) = parse_pdf_image_reference(source)?; + let password = (!self.password.is_empty()).then_some(self.password.as_str()); + let document = crate::pdfium::PdfDocument::open(&self.file_path, password)?; + let page = document.page(page_index)?; + return page.image_png(&path).context("the picture could not be read from the pdf"); + } let extension = Path::new(&self.file_path).extension().map(|ext| ext.to_string_lossy().to_ascii_lowercase()); match extension.as_deref() { Some("epub" | "cbz" | "cbr") => { @@ -103,6 +112,16 @@ fn local_image_path(document_path: &str, source: &str) -> std::path::PathBuf { Path::new(document_path).parent().unwrap_or_else(|| Path::new("")).join(source) } +/// The page and content-stream path a PDF image marker's reference names, which the parser +/// writes as `pdf::`. +fn parse_pdf_image_reference(source: &str) -> Result<(i32, String)> { + let mut parts = source.splitn(3, ':'); + let bad = || anyhow::anyhow!("malformed pdf image reference"); + let page = parts.nth(1).and_then(|text| text.parse().ok()).ok_or_else(bad)?; + let path = parts.next().filter(|path| !path.is_empty()).ok_or_else(bad)?; + Ok((page, path.to_string())) +} + fn decode_data_uri(uri: &str) -> Result> { let (header, data) = uri.split_once(',').context("malformed data URI")?; if header.to_ascii_lowercase().ends_with(";base64") { @@ -120,6 +139,9 @@ fn strip_whitespace(text: &str) -> String { fn image_file_name(source: &str) -> String { // TRANSLATORS: File name stem offered when saving a picture that has no file name of its own let fallback = t("image"); + if source.starts_with("pdf:") { + return pdf_image_file_name(source, &fallback); + } if is_data_uri(source) { let media_type = source[5..].split([';', ',']).next().unwrap_or_default().to_ascii_lowercase(); let subtype = media_type.strip_prefix("image/").unwrap_or_default().split('+').next().unwrap_or_default(); @@ -135,6 +157,16 @@ fn image_file_name(source: &str) -> String { if name.is_empty() { fallback } else { name } } +/// The name a PDF picture is offered under: the page it was found on, and where it sat in that +/// page's content stream, so a second picture from the same book does not land on the first. +fn pdf_image_file_name(source: &str, fallback: &str) -> String { + let Some((page, path)) = source.strip_prefix("pdf:").and_then(|rest| rest.split_once(':')) else { + return format!("{fallback}.png"); + }; + let page = page.parse::().map_or(0, |page| page + 1); + format!("{fallback} {page} {}.png", path.replace('/', "-")) +} + #[cfg(test)] mod tests { use rstest::rstest; @@ -150,6 +182,7 @@ mod tests { #[case("data:image/jpeg;base64,AAAA", "image.jpg")] #[case("data:image/svg+xml,%3Csvg%3E", "image.svg")] #[case("data:;base64,AAAA", "image")] + #[case("pdf:2:0/1", "image 3 0-1.png")] fn a_picture_is_offered_under_its_own_name(#[case] source: &str, #[case] expected: &str) { assert_eq!(image_file_name(source), expected); } diff --git a/crates/paperback-core/src/session/tests.rs b/crates/paperback-core/src/session/tests.rs index cada7ab1..70fd4a50 100644 --- a/crates/paperback-core/src/session/tests.rs +++ b/crates/paperback-core/src/session/tests.rs @@ -40,6 +40,7 @@ fn sample_session(parser_flags: ParserFlags) -> DocumentSession { history_index: 0, parser_flags, parse_settings: ParseSettings::default(), + password: String::new(), } } @@ -54,6 +55,7 @@ fn session_with_content(content: &str) -> DocumentSession { history_index: 0, parser_flags: ParserFlags::empty(), parse_settings: ParseSettings::default(), + password: String::new(), } } @@ -67,5 +69,6 @@ fn session_from_buffer(buffer: DocumentBuffer) -> DocumentSession { history_index: 0, parser_flags: ParserFlags::empty(), parse_settings: ParseSettings::default(), + password: String::new(), } } diff --git a/crates/paperback-core/src/session/tests/accessors.rs b/crates/paperback-core/src/session/tests/accessors.rs index 3347205d..55dce7d1 100644 --- a/crates/paperback-core/src/session/tests/accessors.rs +++ b/crates/paperback-core/src/session/tests/accessors.rs @@ -105,6 +105,7 @@ fn get_formatting_markers_returns_only_bold_italic_underline_markers() { history_index: 0, parser_flags: ParserFlags::NONE, parse_settings: ParseSettings::default(), + password: String::new(), }; let markers = session.get_formatting_markers(); assert_eq!(markers.len(), 3); @@ -154,6 +155,7 @@ fn heading_tree_builds_parent_links_and_closest_index() { history_index: 0, parser_flags: ParserFlags::NONE, parse_settings: ParseSettings::default(), + password: String::new(), }; let tree = session.heading_tree(3); assert_eq!(tree.items.len(), 3); @@ -176,6 +178,7 @@ fn get_current_section_path_returns_none_when_reference_empty() { history_index: 0, parser_flags: ParserFlags::NONE, parse_settings: ParseSettings::default(), + password: String::new(), }; assert!(session.get_current_section_path(0).is_none()); } @@ -200,6 +203,7 @@ fn table_session() -> DocumentSession { history_index: 0, parser_flags: ParserFlags::NONE, parse_settings: ParseSettings::default(), + password: String::new(), } } @@ -237,6 +241,7 @@ fn get_table_at_position_handles_multibyte_extent() { history_index: 0, parser_flags: ParserFlags::NONE, parse_settings: ParseSettings::default(), + password: String::new(), }; // Position 5 is within [0, 6) by display length but would be outside [0, 1) by char count. assert_eq!(session.get_table_at_position(5).as_deref(), Some("")); diff --git a/crates/paperback-core/src/session/tests/audio.rs b/crates/paperback-core/src/session/tests/audio.rs index 913fd424..538198be 100644 --- a/crates/paperback-core/src/session/tests/audio.rs +++ b/crates/paperback-core/src/session/tests/audio.rs @@ -17,6 +17,7 @@ fn session_with_audio(timeline: AudioTimeline) -> DocumentSession { history_index: 0, parser_flags: ParserFlags::empty(), parse_settings: ParseSettings::default(), + password: String::new(), } } diff --git a/crates/paperback-core/src/session/tests/find_all.rs b/crates/paperback-core/src/session/tests/find_all.rs index d9b9e681..2b39620d 100644 --- a/crates/paperback-core/src/session/tests/find_all.rs +++ b/crates/paperback-core/src/session/tests/find_all.rs @@ -15,6 +15,7 @@ fn session_with_marks(content: &str, markers: Vec) -> DocumentSession { history_index: 0, parser_flags: ParserFlags::empty(), parse_settings: ParseSettings::default(), + password: String::new(), } } diff --git a/crates/paperback-core/src/session/tests/links.rs b/crates/paperback-core/src/session/tests/links.rs index 0d47d23d..627ca37f 100644 --- a/crates/paperback-core/src/session/tests/links.rs +++ b/crates/paperback-core/src/session/tests/links.rs @@ -105,6 +105,7 @@ fn activate_link_returns_not_found_when_reference_missing() { history_index: 0, parser_flags: ParserFlags::NONE, parse_settings: ParseSettings::default(), + password: String::new(), }; let result = session.activate_link(7); assert!(!result.found); diff --git a/crates/paperback-core/src/session/tests/navigation.rs b/crates/paperback-core/src/session/tests/navigation.rs index 92c9275c..b7e408dd 100644 --- a/crates/paperback-core/src/session/tests/navigation.rs +++ b/crates/paperback-core/src/session/tests/navigation.rs @@ -240,6 +240,7 @@ fn navigate_section_in_an_audio_only_book_announces_the_file_name() { history_index: 0, parser_flags: ParserFlags::SUPPORTS_SECTIONS, parse_settings: ParseSettings::default(), + password: String::new(), }; let first = session.navigate_section(-1, false, true); assert!(first.found); diff --git a/crates/paperback-core/src/session/tests/webview.rs b/crates/paperback-core/src/session/tests/webview.rs index 9d9d1c9b..4a390853 100644 --- a/crates/paperback-core/src/session/tests/webview.rs +++ b/crates/paperback-core/src/session/tests/webview.rs @@ -17,6 +17,7 @@ fn webview_target_path_returns_none_for_missing_markdown_file() { history_index: 0, parser_flags: ParserFlags::NONE, parse_settings: ParseSettings::default(), + password: String::new(), }; assert!(session.webview_target_path(0, "C:\\temp").is_none()); } @@ -119,6 +120,7 @@ fn extract_resource_returns_false_for_non_epub_files() { history_index: 0, parser_flags: ParserFlags::NONE, parse_settings: ParseSettings::default(), + password: String::new(), }; assert_eq!(session.extract_resource("anything", "out.file").ok(), Some(false)); } @@ -131,6 +133,7 @@ fn session_with_path(file_path: &str) -> DocumentSession { history_index: 0, parser_flags: ParserFlags::NONE, parse_settings: ParseSettings::default(), + password: String::new(), } } @@ -213,6 +216,7 @@ fn extract_resource_for_missing_epub_returns_error() { history_index: 0, parser_flags: ParserFlags::NONE, parse_settings: ParseSettings::default(), + password: String::new(), }; assert!(session.extract_resource("x", "y").is_err()); } diff --git a/crates/paperback-core/src/session/window.rs b/crates/paperback-core/src/session/window.rs index 631d7461..dbc79fbb 100644 --- a/crates/paperback-core/src/session/window.rs +++ b/crates/paperback-core/src/session/window.rs @@ -157,6 +157,7 @@ mod tests { history_index: 0, parser_flags: ParserFlags::empty(), parse_settings: ParseSettings::default(), + password: String::new(), } } diff --git a/doc/readme.md b/doc/readme.md index 26f64d63..e44cae7e 100644 --- a/doc/readme.md +++ b/doc/readme.md @@ -192,7 +192,7 @@ Shortcuts below are for Windows. Where macOS differs, the equivalent is noted in * `Ctrl+Tab` and `Ctrl+Shift+Tab`: Go to the next or previous open document, wrapping around at either end. * `Enter` or `Space` in the document text: Follow a link or open a table or formula view at the cursor. * `Enter` on a scanned PDF page: Recognize the page with OCR. -* `Shift+F10` or the Menu/Application key in the document text: Open the context menu. On a picture in an EPUB, HTML, Markdown, CHM, FB2 or comic book, it also offers to save the picture to a file. +* `Shift+F10` or the Menu/Application key in the document text: Open the context menu. On a picture in a PDF, EPUB, HTML, Markdown, CHM, FB2 or comic book, it also offers to save the picture to a file. ## iOS and Android From 397630506f8824f38fcd1cc0d16539267e4f43a6 Mon Sep 17 00:00:00 2001 From: Quin Gillespie Date: Tue, 20 Oct 2026 12:54:49 -0600 Subject: [PATCH 2/2] Share the bitmap conversion and form depth limit, unwrap comments Co-Authored-By: Claude Opus 5.5 --- crates/paperback-core/src/parser/pdf.rs | 10 +-- .../paperback-core/src/parser/pdf/images.rs | 16 +--- .../src/parser/pdf/structure.rs | 13 +--- crates/paperback-core/src/pdfium.rs | 3 +- crates/paperback-core/src/pdfium/page.rs | 77 +++++-------------- crates/paperback-core/src/session.rs | 3 +- crates/paperback-core/src/session/images.rs | 9 +-- 7 files changed, 36 insertions(+), 95 deletions(-) diff --git a/crates/paperback-core/src/parser/pdf.rs b/crates/paperback-core/src/parser/pdf.rs index bf558fc4..61abd2c1 100644 --- a/crates/paperback-core/src/parser/pdf.rs +++ b/crates/paperback-core/src/parser/pdf.rs @@ -6,7 +6,7 @@ use crate::{ document::{Document, DocumentBuffer, Marker, MarkerType, ParserContext, TocItem}, ocr::image_only_placeholder, parser::{Parser, add_heading_markers, util::path::extract_title_from_path}, - pdfium::PdfDocument, + pdfium::{PAGE_IMAGE_COVERAGE, PdfDocument}, }; mod images; @@ -20,7 +20,7 @@ mod tagged; mod text; mod toc; -use images::{PAGE_IMAGE_COVERAGE, append_image, images_before_each_paragraph, page_largest_image_coverage}; +use images::{append_image, images_before_each_paragraph, page_largest_image_coverage}; use links::{PendingLink, collect_annotation_links, collect_web_links, place_links}; use metadata::{map_load_error, metadata_title, metadata_value}; pub use paragraphs::join_wrapped_lines; @@ -51,8 +51,7 @@ struct PageContent { annotation_links: Vec, /// The top edge of each image the page draws, ordered down the page. image_tops: Vec, - /// Where each of those images sits in the page's content stream, in the same order, so the - /// picture behind an image line can be saved later. + /// Where each of those images sits in the page's content stream, in the same order, so the picture behind an image line can be saved later. image_paths: Vec, /// The fraction of the page the largest single image covers. Near 1.0 marks a scanned page. largest_image_coverage: f64, @@ -187,8 +186,7 @@ fn looks_like_page_number(text: &str) -> bool { enum UntaggedLine { /// A paragraph, and whether the size it was set in marked it out as a heading. Text(String, bool), - /// An image the page drew here, and where its picture sits in the content stream. An - /// untagged page has no description to give for one. + /// An image the page drew here, and where its picture sits in the content stream. An untagged page has no description to give for one. Image(String), } diff --git a/crates/paperback-core/src/parser/pdf/images.rs b/crates/paperback-core/src/parser/pdf/images.rs index fe500730..913674e7 100644 --- a/crates/paperback-core/src/parser/pdf/images.rs +++ b/crates/paperback-core/src/parser/pdf/images.rs @@ -5,8 +5,7 @@ //! this purpose, so [`super::structure`] places one by walking the tree and calls //! [`append_image`] as it meets one. //! -//! An untagged PDF says neither. All it has is image objects with page coordinates, so the -//! caller places each one among the page's text lines by how far down the page it sits. +//! An untagged PDF says neither. All it has is image objects with page coordinates, so the caller places each one among the page's text lines by how far down the page it sits. //! //! A tagged PDF that tags none of its images gets the same treatment through //! [`UnclaimedImages`]. A page like that draws pictures and claims not to have any, so trusting @@ -15,18 +14,11 @@ use crate::{ document::{DocumentBuffer, Marker, MarkerType}, - pdfium::{ObjectKind, PageObject, PdfPage}, + pdfium::{MAX_FORM_DEPTH, ObjectKind, PageObject, PdfPage}, t, util::text::display_len, }; -/// How far to follow form `XObject`s into each other looking for the images they draw. A form -/// holding a form is ordinary enough (a placed logo, an annotation's appearance stream), while a -/// chain of them this deep is a malformed file rather than a page anyone meant to draw. -const MAX_FORM_DEPTH: u32 = 8; - -pub(super) use crate::pdfium::PAGE_IMAGE_COVERAGE; - /// Writes one image's line into the page, with a marker spanning it. /// /// `description` is the image's alt text, and is empty when it has none. An undescribed image @@ -115,9 +107,7 @@ pub(super) struct UnclaimedImages { } impl UnclaimedImages { - /// The images to place, as their tops and content-stream paths. Empty for a page whose tree - /// claims its figures, which is every tagged page written the usual way, and the walk then - /// does none of this. + /// The images to place, as their tops and content-stream paths. Empty for a page whose tree claims its figures, which is every tagged page written the usual way, and the walk then does none of this. pub(super) fn new(tops: &[f64], paths: &[String], page_index: i32) -> Self { Self { images: tops.iter().copied().zip(paths.iter().cloned()).collect(), page_index } } diff --git a/crates/paperback-core/src/parser/pdf/structure.rs b/crates/paperback-core/src/parser/pdf/structure.rs index b7024551..ae23dc04 100644 --- a/crates/paperback-core/src/parser/pdf/structure.rs +++ b/crates/paperback-core/src/parser/pdf/structure.rs @@ -94,8 +94,7 @@ pub(super) fn extract_tagged_page_text( let mut current_block = String::new(); let mut pending_label = String::new(); let mut page_text = PageText::new(&content.text); - // Only a tree that names figures can ask for one's picture, so the walk over the page's - // objects that answers where each image sits is skipped for the rest. + // Only a tree that names figures can ask for one's picture, so the walk over the page's objects that answers where each image sits is skipped for the rest. let figure_images = if facts.claims_figures { page.image_paths_by_mcid() } else { HashMap::new() }; let mut images = ImagePlacement { mcid_tops: &content.tops, @@ -136,8 +135,7 @@ struct ImagePlacement<'a> { unclaimed: UnclaimedImages, /// The page the images belong to, which a marker's reference names so its picture can be saved. page_index: i32, - /// Where each marked-content id's picture sits in the content stream. A `Figure` element - /// points at its picture by marked-content id, and this is what turns that into a reference. + /// Where each marked-content id's picture sits in the content stream. A `Figure` element points at its picture by marked-content id, and this is what turns that into a reference. figure_images: &'a HashMap, /// The top edge of the block being built, from the first piece of text in it. block_top: Option, @@ -180,8 +178,7 @@ impl ImagePlacement<'_> { self.unclaimed.place_above(top, buffer, page_display_text, current_lines_info); } - /// The reference that saves the picture behind `elem`, or an empty string when its content - /// names no image Paperback can find on the page. + /// The reference that saves the picture behind `elem`, or an empty string when its content names no image Paperback can find on the page. fn figure_reference(&self, elem: &Tag) -> String { find_image_path(elem, self.figure_images) .map_or_else(String::new, |path| format!("pdf:{}:{path}", self.page_index)) @@ -198,9 +195,7 @@ impl ImagePlacement<'_> { } } -/// The content-stream path of the first image under `elem`, found by the marked-content ids its -/// leaves carry. A figure's picture is usually a direct child, but a maker that wraps it in a -/// `Div` or a `Caption`'s sibling nests it a level or two down, so the whole subtree is searched. +/// The content-stream path of the first image under `elem`, found by the marked-content ids its leaves carry. A figure's picture is usually a direct child, but a maker that wraps it in a `Div` or a `Caption`'s sibling nests it a level or two down, so the whole subtree is searched. fn find_image_path(elem: &Tag, image_paths: &HashMap) -> Option { let count = elem.child_count(); for i in 0..count { diff --git a/crates/paperback-core/src/pdfium.rs b/crates/paperback-core/src/pdfium.rs index afc9faa3..6017c242 100644 --- a/crates/paperback-core/src/pdfium.rs +++ b/crates/paperback-core/src/pdfium.rs @@ -34,7 +34,8 @@ mod text; // outright, which is what the allow is for. #[allow(unused_imports)] pub use page::{ - AnnotationLink, Bounds, LinkTarget, ObjectKind, PAGE_IMAGE_COVERAGE, PageBitmap, PageImage, PageObject, PdfPage, + AnnotationLink, Bounds, LinkTarget, MAX_FORM_DEPTH, ObjectKind, PAGE_IMAGE_COVERAGE, PageBitmap, PageImage, + PageObject, PdfPage, }; use pdfium_render::prelude::{FPDF_BOOKMARK, FPDF_DOCUMENT, Pdfium, PdfiumLibraryBindings}; #[allow(unused_imports)] diff --git a/crates/paperback-core/src/pdfium/page.rs b/crates/paperback-core/src/pdfium/page.rs index 3219eabb..fdeaf5f2 100644 --- a/crates/paperback-core/src/pdfium/page.rs +++ b/crates/paperback-core/src/pdfium/page.rs @@ -70,8 +70,7 @@ impl<'doc> PdfPage<'doc> { /// Every image the page draws that is worth a line of its own, nearest the top first. /// - /// This is the one list the parser places image lines from, so a page's images are announced - /// in a fixed order. + /// This is the one list the parser places image lines from, so a page's images are announced in a fixed order. pub fn announced_images(&self) -> Vec> { let page_area = f64::from(self.width()) * f64::from(self.height()); let mut images = Vec::new(); @@ -84,8 +83,7 @@ impl<'doc> PdfPage<'doc> { images } - /// The picture at `path`, encoded as PNG. `path` is the one an announced image carries in - /// [`PageImage::path`], as it was written into an image marker's reference. + /// The picture at `path`, encoded as PNG. `path` is the one an announced image carries in [`PageImage::path`], as it was written into an image marker's reference. pub fn image_png(&self, path: &str) -> Option> { let mut parts = path.split('/'); let first = i32::try_from(parts.next()?.parse::().ok()?).ok()?; @@ -94,9 +92,7 @@ impl<'doc> PdfPage<'doc> { encode_image_png(handle) } - /// The image path for each marked-content id on the page. A tagged PDF's `Figure` element - /// points at its picture by marked-content id, and this is what turns that into a reference - /// that can be saved, whether or not the picture is one Paperback announces. + /// The image path for each marked-content id on the page. A tagged PDF's `Figure` element points at its picture by marked-content id, and this is what turns that into a reference that can be saved, whether or not the picture is one Paperback announces. pub fn image_paths_by_mcid(&self) -> std::collections::HashMap { let page_area = f64::from(self.width()) * f64::from(self.height()); let mut all = Vec::new(); @@ -134,10 +130,10 @@ impl<'doc> PdfPage<'doc> { unsafe { bindings().FPDF_RenderPageBitmap(bitmap, self.handle, 0, 0, width, height, 0, 0) }; let stride = unsafe { bindings().FPDFBitmap_GetStride(bitmap) }; let buffer = unsafe { bindings().FPDFBitmap_GetBuffer(bitmap) }; - let rgba = bgra_to_rgba(buffer, width, height, stride); + let rgba = bitmap_to_rgba(buffer, width, height, stride, BITMAP_BGRA); unsafe { bindings().FPDFBitmap_Destroy(bitmap) }; - let rgba = rgba?; - Some(PageBitmap { width: u32::try_from(width).ok()?, height: u32::try_from(height).ok()?, rgba }) + let (width, height, rgba) = rgba?; + Some(PageBitmap { width, height, rgba }) } /// Every link annotation on the page, with the box it covers and where it points. @@ -229,19 +225,16 @@ impl Drop for PdfPage<'_> { } } -/// Shortest side, in PDF user units, an image must have to be worth a line of its own. A page -/// laid out from HTML draws its spacers and its hairline rules as images a fraction of a point -/// across; announcing one of those as an image gives the reader a stop with nothing at it. +/// Shortest side, in PDF user units, an image must have to be worth a line of its own. A page laid out from HTML draws its spacers and its hairline rules as images a fraction of a point across; announcing one of those as an image gives the reader a stop with nothing at it. const MIN_IMAGE_SIDE: f32 = 4.0; -/// How much of the page one image can cover and still be a picture on the page rather than the -/// page itself. A scanned book with an OCR text layer draws each sheet as one image under its -/// invisible text, and announcing that put an "[Image]" with nothing behind it on every page -/// (#1046). +/// How much of the page one image can cover and still be a picture on the page rather than the page itself. A scanned book with an OCR text layer draws each sheet as one image under its invisible text, and announcing that put an "[Image]" with nothing behind it on every page (#1046). pub const PAGE_IMAGE_COVERAGE: f64 = 0.9; -/// `FPDFBitmap_BGRA`, `FPDFBitmap_BGRx`, `FPDFBitmap_BGR` and `FPDFBitmap_Gray`, the four -/// bitmap layouts pdfium hands back. +/// How far to follow form `XObject`s into each other looking for the images they draw. A form holding a form is ordinary enough (a placed logo, an annotation's appearance stream), while a chain of them this deep is a malformed file rather than a page anyone meant to draw. +pub const MAX_FORM_DEPTH: u32 = 8; + +/// `FPDFBitmap_BGRA`, `FPDFBitmap_BGRx`, `FPDFBitmap_BGR` and `FPDFBitmap_Gray`, the four bitmap layouts pdfium hands back. const BITMAP_GRAY: i32 = 1; const BITMAP_BGR: i32 = 2; const BITMAP_BGRX: i32 = 3; @@ -250,12 +243,9 @@ const BITMAP_BGRX: i32 = 3; /// /// Borrows the page it came from. Nothing is closed here; the page owns the object. pub struct PageImage<'page> { - /// The image's top edge, in PDF user units. Y grows up the page, so the nearest the top has - /// the largest one. + /// The image's top edge, in PDF user units. Y grows up the page, so the nearest the top has the largest one. pub top: f64, - /// Where the image object sits in the page's content stream, as slash-separated indices. - /// This is what an image marker's reference carries, so [`PdfPage::image_png`] can find the - /// picture again without depending on the order images were listed in. + /// Where the image object sits in the page's content stream, as slash-separated indices. This is what an image marker's reference carries, so [`PdfPage::image_png`] can find the picture again without depending on the order images were listed in. pub path: String, handle: FPDF_PAGEOBJECT, _page: std::marker::PhantomData<&'page ()>, @@ -275,7 +265,7 @@ impl PageImage<'_> { continue; } let mut value = 0; - if unsafe { bindings.FPDFPageObjMark_GetParamIntValue(mark, "MCID", &mut value) } != 0 { + if unsafe { bindings.FPDFPageObjMark_GetParamIntValue(mark, "MCID", &raw mut value) } != 0 { ids.push(value); } } @@ -300,9 +290,7 @@ fn descend_object<'p, 'a>(object: &PageObject<'p>, mut path: impl Iterator Option> { let bindings = bindings(); let bitmap = unsafe { bindings.FPDFImageObj_GetBitmap(handle) }; @@ -370,9 +358,7 @@ fn bitmap_to_rgba( Some((u32::try_from(width).ok()?, u32::try_from(height).ok()?, rgba)) } -/// Collects the image objects under `object`, walking into form `XObject`s and recording where -/// each one sits in the content stream. `only_announced` leaves out the images that are too -/// small to be worth a line or that cover the page themselves. +/// Collects the image objects under `object`, walking into form `XObject`s and recording where each one sits in the content stream. `only_announced` leaves out the images that are too small to be worth a line or that cover the page themselves. fn collect_images( object: &PageObject<'_>, depth: u32, @@ -396,7 +382,7 @@ fn collect_images( }); } } - ObjectKind::Form if depth < 8 => { + ObjectKind::Form if depth < MAX_FORM_DEPTH => { for i in 0..object.form_object_count() { if let Some(child) = object.form_object(i) { collect_images(&child, depth + 1, page_area, only_announced, &format!("{path}/{i}"), images); @@ -412,30 +398,6 @@ fn is_announced_image(width: f32, height: f32, page_area: f64) -> bool { width >= MIN_IMAGE_SIDE && height >= MIN_IMAGE_SIDE && !covers_page } -/// Copies pdfium's BGRA rows out into a tight RGBA buffer. -/// -/// The rows are padded to a stride, so this cannot be one flat copy; each row is taken at its -/// own offset and the padding left behind. -fn bgra_to_rgba(buffer: *const c_void, width: i32, height: i32, stride: i32) -> Option> { - if buffer.is_null() || stride < width * 4 { - return None; - } - let (w, h, stride) = (width as usize, height as usize, stride as usize); - let source = unsafe { std::slice::from_raw_parts(buffer.cast::(), stride * h) }; - let mut rgba = vec![0u8; w * h * 4]; - for row in 0..h { - for column in 0..w { - let from = row * stride + column * 4; - let to = (row * w + column) * 4; - rgba[to] = source[from + 2]; - rgba[to + 1] = source[from + 1]; - rgba[to + 2] = source[from]; - rgba[to + 3] = source[from + 3]; - } - } - Some(rgba) -} - /// A rasterized page. pub struct PageBitmap { pub width: u32, @@ -489,8 +451,7 @@ impl PageObject<'_> { } } - /// The raw handle, for callers that speak FFI directly. - pub fn handle(&self) -> FPDF_PAGEOBJECT { + pub const fn handle(&self) -> FPDF_PAGEOBJECT { self.handle } diff --git a/crates/paperback-core/src/session.rs b/crates/paperback-core/src/session.rs index 9ffacfd0..f1108b9f 100644 --- a/crates/paperback-core/src/session.rs +++ b/crates/paperback-core/src/session.rs @@ -229,8 +229,7 @@ pub struct DocumentSession { parser_flags: ParserFlags, /// The settings the document was parsed with, which views that re-render its source must follow too. parse_settings: ParseSettings, - /// The password the document was opened with, so a second look at the file for something the - /// parse did not keep (the picture behind an image line) can get past the same lock. + /// The password the document was opened with, so a second look at the file for something the parse did not keep (the picture behind an image line) can get past the same lock. password: String, } diff --git a/crates/paperback-core/src/session/images.rs b/crates/paperback-core/src/session/images.rs index 6bc91214..676d0fad 100644 --- a/crates/paperback-core/src/session/images.rs +++ b/crates/paperback-core/src/session/images.rs @@ -71,8 +71,7 @@ impl DocumentSession { if is_data_uri(source) { return decode_data_uri(source); } - // A PDF names its pictures by where their objects sit rather than by a file path, so the - // reference itself says which reader they need and the document's extension cannot. + // A PDF names its pictures by where their objects sit rather than by a file path, so the reference itself says which reader they need and the document's extension cannot. if source.starts_with("pdf:") { let (page_index, path) = parse_pdf_image_reference(source)?; let password = (!self.password.is_empty()).then_some(self.password.as_str()); @@ -112,8 +111,7 @@ fn local_image_path(document_path: &str, source: &str) -> std::path::PathBuf { Path::new(document_path).parent().unwrap_or_else(|| Path::new("")).join(source) } -/// The page and content-stream path a PDF image marker's reference names, which the parser -/// writes as `pdf::`. +/// The page and content-stream path a PDF image marker's reference names, which the parser writes as `pdf::`. fn parse_pdf_image_reference(source: &str) -> Result<(i32, String)> { let mut parts = source.splitn(3, ':'); let bad = || anyhow::anyhow!("malformed pdf image reference"); @@ -157,8 +155,7 @@ fn image_file_name(source: &str) -> String { if name.is_empty() { fallback } else { name } } -/// The name a PDF picture is offered under: the page it was found on, and where it sat in that -/// page's content stream, so a second picture from the same book does not land on the first. +/// The name a PDF picture is offered under: the page it was found on, and where it sat in that page's content stream, so a second picture from the same book does not land on the first. fn pdf_image_file_name(source: &str, fallback: &str) -> String { let Some((page, path)) = source.strip_prefix("pdf:").and_then(|rest| rest.split_once(':')) else { return format!("{fallback}.png");