Merge pull request #1841 from ramox81/codex/pdfimport-followups

PDFIMPORT: reference coordinate precision, unreadable file and page messages, picker automation

src/app/control/actions.rs

@@ -109,6 +109,8 @@ pub(super) const NAMES: &[&str] = &[
     "view_background",
     "image_adjust",
     "import_file_dialog",
+    "pdf_import_pick",
+    "pdf_import_dialog",
     "dgn_import_dialog",
     "refedit_dialog",
     "pdf_shx_settings",
@@ -2164,6 +2166,40 @@ impl OpenCADStudio {
                     _ => return Err(bad()),
                 }
             }
+            // The file the next PDF import picker returns, without showing
+            // it (value: the path).
+            "pdf_import_pick" => {
+                crate::sys::set_pdf_import_pick(string(req, "value")?.into());
+                return Ok(Task::none());
+            }
+            // The Import PDF window, one field per call: "page=<n>" (1-based),
+            // "on_screen=0|1" (insertion point asked after OK), "scale=<value>",
+            // "rotation=0|90|180|270", "ok" or "cancel".
+            "pdf_import_dialog" => {
+                use crate::ui::window::pdf_dialogs::{PdfDialogMsg as P, RotationChoice};
+                if self.active_modal != Some(crate::app::ModalKind::PdfImportFile) {
+                    return Err(failure("no_dialog", "The Import PDF window is not open"));
+                }
+                let spec = string(req, "value")?;
+                let (key, value) = spec.split_once('=').unwrap_or((spec, ""));
+                let bad = || failure("invalid_value", spec);
+                if key == "cancel" {
+                    return Ok(Task::done(Message::CloseModal));
+                }
+                Message::PdfDialog(match key {
+                    "ok" => P::ImportOk,
+                    "page" => P::ImportPageText(value.to_string()),
+                    "on_screen" => P::ImportInsertOnScreen(value == "1"),
+                    "scale" => P::ImportScale(value.to_string()),
+                    "rotation" => P::ImportRotation(
+                        RotationChoice::ALL
+                            .into_iter()
+                            .find(|r| value.trim().parse() == Ok(r.0))
+                            .ok_or_else(bad)?,
+                    ),
+                    _ => return Err(bad()),
+                })
+            }
             // The Import File window, one field per call: "path=<file>",
             // "format=<index or name>", "search=<text>", "filter=all|supported",
             // "recent=<n>", "ok" or "cancel".

src/app/update/mod.rs

@@ -1872,6 +1872,12 @@ impl OpenCADStudio {
 
             Message::PdfImportPick => Task::perform(
                 async {
+                    // A file named by automation comes back without the picker.
+                    if let Some(path) = crate::sys::take_pdf_import_pick() {
+                        return std::fs::read(&path)
+                            .map(|bytes| (path, std::sync::Arc::new(bytes)))
+                            .map_err(|e| e.to_string());
+                    }
                     let handle = crate::sys::file_dialog()
                         .set_title(crate::t!("Select PDF File").as_ref())
                         .add_filter(crate::t!("PDF Files").as_ref(), &["pdf", "PDF"])

src/modules/insert/pdf_import.rs

@@ -201,7 +201,8 @@ pub struct PdfFileImport {
 impl PdfFileImport {
     /// Where the page's content goes: one drawing unit per page inch times
     /// the scale, or, on a page with a viewport measure (a plot), its
-    /// measured units (a 1:1 millimetre plot comes in at its millimetres).
+    /// measured units (a 1:1 millimetre plot comes in at its millimetres);
+    /// a page's user unit multiplies either.
     pub fn placement(&self) -> Underlay {
         let mut u = Underlay::pdf();
         u.insertion_point = Vector3::new(self.insertion.x, self.insertion.y, self.insertion.z);
@@ -256,6 +257,25 @@ enum Step {
     Rotation,
 }
 
+/// What a file name that is no readable PDF (damaged, empty, not a PDF)
+/// prints before the name is asked for again.
+const UNREADABLE_FILE: &str = "This file cannot be opened.\n\
+The problem can be caused by several things:\n\
+- The data in the file is corrupted\n\
+- The file is an earlier format that is no longer used\n\
+- The file format is not supported\n\
+Try to obtain a better copy of the file, recreate the file using different settings, \
+or convert the file to a supported file format.";
+
+/// A typed page number: digits only (no sign, decimals or ranges); one too
+/// large for a 32-bit count is the largest one.
+fn page_number(token: &str) -> Option<usize> {
+    if token.is_empty() || !token.bytes().all(|b| b.is_ascii_digit()) {
+        return None;
+    }
+    Some(token.parse::<u64>().map_or(i32::MAX as u64, |n| n.min(i32::MAX as u64)) as usize)
+}
+
 pub struct PdfImportCommand {
     step: Step,
     handle: Handle,
@@ -331,10 +351,24 @@ impl PdfImportCommand {
         let Ok(bytes) = std::fs::read(&path) else {
             return CmdResult::ReportError(format!("File {shown} does not exist."));
         };
+        // A damaged file is repaired only when it still has its trailer (the
+        // `trailer` dictionary or a cross-reference stream); a cut-off one
+        // cannot be opened.
+        // ponytail: byte search; a compressed stream holding these bytes
+        // would pass the test.
+        let has = |key: &[u8]| bytes.windows(key.len()).any(|w| w == key);
+        if !has(b"trailer") && !has(b"/XRef") {
+            return CmdResult::ReportError(UNREADABLE_FILE.to_string());
+        }
         crate::scene::model::pdf_raster::register_source(&path, std::sync::Arc::new(bytes));
-        let Some(pages) = crate::scene::model::pdf_raster::page_count(&path) else {
+        // An encrypted file is asked for again without a message.
-            // ponytail: wording for an unreadable PDF not measured.
+        if crate::scene::model::pdf_raster::is_encrypted(&path) {
-            return CmdResult::ReportError(format!("{shown} is not a valid PDF file."));
+            return CmdResult::NeedPoint;
+        }
+        let Some(pages) =
+            crate::scene::model::pdf_raster::page_count(&path).filter(|&pages| pages > 0)
+        else {
+            return CmdResult::ReportError(UNREADABLE_FILE.to_string());
         };
         self.pages = pages;
         self.file.path = path;
@@ -351,18 +385,18 @@ impl PdfImportCommand {
         let page = if token.is_empty() {
             1
         } else {
-            match token.parse::<i64>() {
+            match page_number(token) {
-                Ok(n) if n > 0 => n as usize,
                 // Zero is asked again without a message.
-                Ok(0) => return CmdResult::NeedPoint,
+                Some(0) => return CmdResult::NeedPoint,
-                _ => {
+                Some(n) => n,
+                None => {
                     return CmdResult::ReportError("Please enter page number or '?'.".to_string())
                 }
             }
         };
-        // A page past the last one is asked again without a message.
         if page > self.pages {
-            return CmdResult::NeedPoint;
+            let shown = crate::entities::underlay::display_path(&self.file.path);
+            return CmdResult::ReportError(format!("There is no {page} in {shown}."));
         }
         self.file.page = page.to_string();
         self.step = Step::Insertion;
@@ -371,16 +405,23 @@ impl PdfImportCommand {
 
     fn page_list(&mut self, text: &str) -> CmdResult {
         let token = text.trim();
-        self.step = Step::Page;
         if token.is_empty() || token == "*" {
+            self.step = Step::Page;
             let all: Vec<String> = (1..=self.pages).map(|n| n.to_string()).collect();
             return CmdResult::ReportMeasurement(all.join("\n"));
         }
-        match token.parse::<usize>() {
+        match page_number(token) {
-            Ok(n) if (1..=self.pages).contains(&n) => {
+            // Zero, or anything but a page number, is asked again.
-                CmdResult::ReportMeasurement(format!("Found page {n}."))
+            Some(0) => CmdResult::NeedPoint,
+            None => CmdResult::ReportError("Please enter page number or *.".to_string()),
+            Some(n) => {
+                self.step = Step::Page;
+                CmdResult::ReportMeasurement(if n <= self.pages {
+                    format!("Found page {n}.")
+                } else {
+                    "No matching page found.".to_string()
+                })
             }
-            _ => CmdResult::ReportMeasurement("No matching page found.".to_string()),
         }
     }
 
@@ -576,7 +617,6 @@ impl CadCommand for PdfImportCommand {
             Step::Scale => match token.parse::<f64>() {
                 Ok(v) => self.scale(v),
                 Err(_) if token.is_empty() => self.scale(1.0),
-                // ponytail: wording for a non-number scale not measured.
                 Err(_) => CmdResult::ReportError(
                     "Requires numeric distance or second point.".to_string(),
                 ),

src/scene/model/pdf_raster.rs

@@ -276,6 +276,17 @@ pub fn page_count(path: &str) -> Option<usize> {
     Some(pdf.pages().len())
 }
 
+/// Whether a registered PDF is encrypted: it does not open with a password
+/// no document uses (an unencrypted file ignores the password).
+pub fn is_encrypted(path: &str) -> bool {
+    source_bytes(path).is_some_and(|bytes| {
+        matches!(
+            Pdf::new_with_password(bytes, "\u{1}"),
+            Err(hayro::hayro_syntax::LoadPdfError::Decryption(_))
+        )
+    })
+}
+
 /// Size of a 1-based page in inches (its rendered width and height at 72
 /// points per inch). An underlay is one drawing unit per page inch.
 pub fn page_size_inches(path: &str, page: &str) -> Option<(f64, f64)> {
@@ -287,25 +298,35 @@ pub fn page_size_inches(path: &str, page: &str) -> Option<(f64, f64)> {
     Some((w as f64 / 72.0, h as f64 / 72.0))
 }
 
-/// Drawing units per page inch that a page's measure gives: the X ratio of
+/// Drawing units per page inch a page imports at: the X ratio of the
-/// the first viewport's measure dictionary (`/VP [<< /Measure << /X [<< /C
+/// first viewport's measure dictionary (`/VP [<< /Measure << /X [<< /C
-/// units-per-point >>] >> >>]`, written by plotters with the plot scale),
+/// units-per-point >>] >> >>]`, written by plotters with the plot scale)
-/// times 72. `None` when the page has no viewport measure.
+/// times 72, or one unit per inch without one; times the page's
+/// `/UserUnit` (points per user-space unit) when it has one.
 pub fn page_units_per_inch(path: &str, page: &str) -> Option<f64> {
     use hayro::hayro_syntax::object::{Array, Dict, Number};
     let bytes = source_bytes(path)?;
     let pdf = Pdf::new(bytes).ok()?;
     let page_no = page.trim().parse::<usize>().ok()?;
     let page = pdf.pages().get(page_no.checked_sub(1)?)?;
-    let viewport = page.raw().get::<Array<'_>>(b"VP")?.iter::<Dict<'_>>().next()?;
+    let positive = |v: f64| (v.is_finite() && v > 0.0).then_some(v);
-    let ratio = viewport
+    let measured = (|| {
-        .get::<Dict<'_>>(b"Measure")?
+        let viewport = page.raw().get::<Array<'_>>(b"VP")?.iter::<Dict<'_>>().next()?;
-        .get::<Array<'_>>(b"X")?
+        let ratio = viewport
-        .iter::<Dict<'_>>()
+            .get::<Dict<'_>>(b"Measure")?
-        .next()?
+            .get::<Array<'_>>(b"X")?
-        .get::<Number>(b"C")?
+            .iter::<Dict<'_>>()
-        .as_f64();
+            .next()?
-    (ratio.is_finite() && ratio > 0.0).then_some(ratio * 72.0)
+            .get::<Number>(b"C")?
+            .as_f64();
+        positive(ratio).map(|r| r * 72.0)
+    })();
+    let user_unit = page
+        .raw()
+        .get::<Number>(b"UserUnit")
+        .and_then(|n| positive(n.as_f64()))
+        .unwrap_or(1.0);
+    Some(measured.unwrap_or(1.0) * user_unit)
 }
 
 /// Display adjustments of an underlay page, applied to its display raster.

src/scene/model/pdf_vector.rs

@@ -267,6 +267,7 @@ fn read_page_with(path: &str, page: &str, images: bool) -> Option<PageVectors> {
     );
     let mut device = Collector {
         want_images: images,
+        visible: vec![Rect::new(0.0, 0.0, w as f64 / 72.0, h as f64 / 72.0)],
         ..Collector::default()
     };
     interpret_page(page, &mut context, &mut device);
@@ -331,6 +332,9 @@ struct Collector {
     want_images: bool,
     images: Vec<PdfImage>,
     order: Vec<Drawn>,
+    /// The visible area, in inches: the page, then each clip path's box
+    /// within the one before it (the last is the current one).
+    visible: Vec<Rect>,
 }
 
 impl Collector {
@@ -358,14 +362,31 @@ fn pt(p: Point) -> [f64; 2] {
     [p.x, p.y]
 }
 
+/// The page transform (user space → inches) at the precision PDF import
+/// reads geometry with: the current transformation matrix, in points, as
+/// 16.16 fixed point held in single precision — each coefficient times
+/// 65536 to the nearest integer, then to the nearest single-precision
+/// number (a 0.12 scale is 7864/65536; a 345.678 offset 22654352/65536) —
+/// and each user-space coordinate as a single-precision number kept to six
+/// decimals (100.3 is 100.300003).
+fn import_precision(transform: Affine) -> impl Fn(Point) -> Point {
+    let points = (Affine::scale(72.0) * transform)
+        .as_coeffs()
+        .map(|v| (v * 65536.0).round() as f32 as f64 / 65536.0);
+    let transform = Affine::scale(1.0 / 72.0) * Affine::new(points);
+    let six = |v: f64| ((v as f32) as f64 * 1e6).round() / 1e6;
+    move |p: Point| transform * Point::new(six(p.x), six(p.y))
+}
+
 fn subpaths(path: &BezPath, transform: Affine) -> Vec<SubPath> {
+    let transform = import_precision(transform);
     let mut out: Vec<SubPath> = Vec::new();
     let mut start = Point::ZERO;
     let mut last = Point::ZERO;
     for el in path.elements() {
         match *el {
             PathEl::MoveTo(p) => {
-                let p = transform * p;
+                let p = transform(p);
                 out.push(SubPath {
                     segments: Vec::new(),
                     closed: false,
@@ -374,14 +395,14 @@ fn subpaths(path: &BezPath, transform: Affine) -> Vec<SubPath> {
                 last = p;
             }
             PathEl::LineTo(p) => {
-                let p = transform * p;
+                let p = transform(p);
                 if let Some(sp) = out.last_mut() {
                     sp.segments.push(Segment::Line(pt(last), pt(p)));
                 }
                 last = p;
             }
             PathEl::QuadTo(c, p) => {
-                let (c, p) = (transform * c, transform * p);
+                let (c, p) = (transform(c), transform(p));
                 let c1 = last + (c - last) * (2.0 / 3.0);
                 let c2 = p + (c - p) * (2.0 / 3.0);
                 if let Some(sp) = out.last_mut() {
@@ -391,7 +412,7 @@ fn subpaths(path: &BezPath, transform: Affine) -> Vec<SubPath> {
                 last = p;
             }
             PathEl::CurveTo(c1, c2, p) => {
-                let (c1, c2, p) = (transform * c1, transform * c2, transform * p);
+                let (c1, c2, p) = (transform(c1), transform(c2), transform(p));
                 if let Some(sp) = out.last_mut() {
                     sp.segments
                         .push(Segment::Cubic(pt(last), pt(c1), pt(c2), pt(p)));
@@ -438,6 +459,26 @@ impl<'a> Device<'a> for Collector {
         if subpaths.is_empty() {
             return;
         }
+        // A path wholly outside the visible area is left out; one that
+        // reaches into it is kept whole.
+        if let Some(area) = self.visible.last() {
+            let (mut lo, mut hi) = ([f64::INFINITY; 2], [f64::NEG_INFINITY; 2]);
+            for seg in subpaths.iter().flat_map(|sp| &sp.segments) {
+                let points: &[[f64; 2]] = match seg {
+                    Segment::Line(a, b) => &[*a, *b],
+                    Segment::Cubic(a, b, c, d) => &[*a, *b, *c, *d],
+                };
+                for p in points {
+                    for k in 0..2 {
+                        lo[k] = lo[k].min(p[k]);
+                        hi[k] = hi[k].max(p[k]);
+                    }
+                }
+            }
+            if hi[0] < area.x0 || lo[0] > area.x1 || hi[1] < area.y0 || lo[1] > area.y1 {
+                return;
+            }
+        }
         let (stroke, fill) = match mode {
             PathDrawMode::Stroke(props) => {
                 // Width in points: the user-space width through the transform
@@ -454,7 +495,11 @@ impl<'a> Device<'a> for Collector {
             fill,
         });
     }
-    fn push_clip_path(&mut self, _: &ClipPath) {}
+    fn push_clip_path(&mut self, clip: &ClipPath) {
+        let bounds = clip.path.bounding_box();
+        let area = self.visible.last().map_or(bounds, |a| a.intersect(bounds));
+        self.visible.push(area);
+    }
     fn push_transparency_group(&mut self, _: f32, _: Option<SoftMask<'a>>, _: BlendMode) {}
     fn draw_glyph(
         &mut self,
@@ -588,7 +633,11 @@ impl<'a> Device<'a> for Collector {
             self.images.push(image);
         }
     }
-    fn pop_clip_path(&mut self) {}
+    fn pop_clip_path(&mut self) {
+        if self.visible.len() > 1 {
+            self.visible.pop();
+        }
+    }
     fn pop_transparency_group(&mut self) {}
 }
 

src/sys.rs

@@ -602,6 +602,18 @@ pub fn set_file_dia(on: bool) {
     FILE_DIA.store(on, std::sync::atomic::Ordering::Relaxed);
 }
 
+/// A file the next PDF import picker returns without showing itself (set
+/// by automation, used once).
+static PDF_IMPORT_PICK: std::sync::Mutex<Option<std::path::PathBuf>> = std::sync::Mutex::new(None);
+
+pub fn set_pdf_import_pick(path: std::path::PathBuf) {
+    *PDF_IMPORT_PICK.lock().unwrap_or_else(|e| e.into_inner()) = Some(path);
+}
+
+pub fn take_pdf_import_pick() -> Option<std::path::PathBuf> {
+    PDF_IMPORT_PICK.lock().unwrap_or_else(|e| e.into_inner()).take()
+}
+
 /// The file type ATTACH's picker was last left on (1-based; 7 = Drawing).
 static ATTACH_FILTER: std::sync::atomic::AtomicU32 = std::sync::atomic::AtomicU32::new(7);