From d135b15ab6338d84c5d404aa4fe3ec2e4ec4aafd Mon Sep 17 00:00:00 2001 From: Afonsop Date: Thu, 17 Sep 2026 18:42:15 +0100 Subject: [PATCH] fix: harden the issue-fix wave after full adversarial review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five reviewers (PDF, snap, commands, otrack/help, printing) covered the eight issue-fix commits; every blocker and major is fixed here. Printing (#1261 follow-up): - printto params now quote the printer name and pad the empty driver/ port slots handlers substitute ("Microsoft Print to PDF" was split into three arguments and the job silently lost). - Level-2 enumeration also merges when the level-4 list omits the configured default, not only when it is empty; a level-2 error no longer discards the default-printer merge. - The printer list loads in a background task instead of blocking the UI thread (level-2 queries can wait out RPC timeouts). - CTB registration: collisions keep the existing file but now say so, and only container-parsed tables are registered (the permissive zlib heuristic stays session-only so junk cannot persist). Snapping (#1030/#1319 follow-up): - Edge-wire key_vertices gain NaN separators: junctions between unrelated mesh edges no longer fabricate phantom midpoints. - The wire cap counts segments (not separator points) and decimates instead of dropping, so dense meshes keep partial endpoint coverage instead of losing snap entirely. - f64 endpoint recovery is a one-shot per-wire map instead of an O(points x geoms) rescan on every mouse move. Commands (#1082 class): the same typed-value-on-web bug also hit CYLINDER/BOX/WEDGE/CONE/PYRAMID heights, HELIX final height (wrong value, not just dropped), and SWEEP reference/new lengths — all now commit as text at exactly those steps, each pinned by a regression test that fails without the override (plus the missing native-route twin for EXTRUDE). PDF (#702 follow-up): heavy CTB pens bypass the Form XObject (their half-width exceeds the /BBox SDF margin and was flat-chopped), CTM scale entries round to 4 significant digits (0.01 quantized small text by ~1%), degenerate quads skip emission, the atlas snapshot is taken once per export instead of per text entity, and the rounding comments no longer conflate page points with glyph-plane units. OTRACK (#1313 follow-up): the guide's polar quantization diverges from the snapper's drafting-gated polar during window-corner drags — documented; an orphaned grid-step doc block is reunited with its function. --- src/app/automation.rs | 472 +++++++++++++++++++++++++++++ src/app/mod.rs | 13 +- src/app/update/file.rs | 16 +- src/app/update/mod.rs | 14 +- src/app/view/mod.rs | 3 + src/io/mod.rs | 145 ++++++++- src/io/pdf_export.rs | 217 ++++++++++--- src/io/plot_style.rs | 55 +++- src/io/print_to_printer.rs | 69 ++++- src/modules/draw/draw/helix.rs | 15 + src/modules/insert/solid3d_cmds.rs | 15 + src/modules/model/cylinder_cmd.rs | 12 + src/modules/model/primitive_cmd.rs | 27 ++ src/scene/model/mesh_model.rs | 103 +++++-- src/scene/model/solid_model.rs | 3 +- src/snap.rs | 196 +++++++++--- src/ui/overlay.rs | 12 +- tests/pdf_size_repro.rs | 77 +++++ 18 files changed, 1326 insertions(+), 138 deletions(-) diff --git a/src/app/automation.rs b/src/app/automation.rs index 4ee5810f4..8b928df20 100644 --- a/src/app/automation.rs +++ b/src/app/automation.rs @@ -1771,4 +1771,476 @@ mod tests { "EXTRUDE must finish after the typed height" ); } + + /// (min, max) of `coord` over every wire point of the Solid3D entities in + /// tab 0's document. Lets the typed-value regression tests below assert + /// the committed geometry (the height / swept size), not just that some + /// solid appeared. + fn solid_wire_extent( + app: &OpenCADStudio, + coord: impl Fn(&acadrust::types::Vector3) -> f64, + ) -> Option<(f64, f64)> { + let mut extent: Option<(f64, f64)> = None; + for entity in app.tabs[0].scene.document.entities() { + let acadrust::EntityType::Solid3D(solid) = entity else { + continue; + }; + for point in solid.wires.iter().flat_map(|wire| wire.points.iter()) { + let value = coord(point); + extent = Some(match extent { + Some((low, high)) => (low.min(value), high.max(value)), + None => (value, value), + }); + } + } + extent + } + + fn solid_count(app: &OpenCADStudio) -> usize { + app.tabs[0] + .scene + .document + .entities() + .filter(|e| matches!(e, acadrust::EntityType::Solid3D(_))) + .count() + } + + /// The native twin of `extrude_typed_height_from_dyn_field`: the same + /// EXTRUDE Height step driven through the focused command line + /// (`CommandInput` → `CommandSubmit`) — the route a desktop user whose + /// command line holds keyboard focus takes. Both routes must extrude by + /// the typed height (#1082). + #[test] + fn extrude_typed_height_from_command_line() { + use crate::app::Message; + let mut app = OpenCADStudio::new_for_test(); + app.automation_op(r#"{"op":"new"}"#); + // A closed rectangle in the XY plane. + use acadrust::entities::LwVertex; + use acadrust::types::Vector2; + let rect = acadrust::LwPolyline { + vertices: [ + (-10.0, -10.0), + (10.0, -10.0), + (10.0, 10.0), + (-10.0, 10.0), + ] + .iter() + .map(|(x, y)| LwVertex::new(Vector2::new(*x, *y))) + .collect(), + is_closed: true, + ..Default::default() + }; + let handle = app.tabs[0] + .scene + .document + .add_entity(acadrust::EntityType::LwPolyline(rect)) + .unwrap(); + + let _ = app.update(Message::CommandInput("EXTRUDE".to_string())); + let _ = app.update(Message::CommandSubmit); + let _ = app.feed_command(crate::command::StepInput::EntityPick( + handle, + glam::DVec3::new(-10.0, -10.0, 0.0), + )); + // Bare Enter finishes selection → Height step. + let _ = app.update(Message::CommandFinalize); + + // Focused command line: the typed height goes through the input + // widget, not the dynamic-input field buffer. + let _ = app.update(Message::CommandInput("5".to_string())); + let _ = app.update(Message::CommandSubmit); + + let (min_z, max_z) = + solid_wire_extent(&app, |p| p.z).expect("typed height must extrude"); + assert!( + (max_z - min_z - 5.0).abs() < 1e-6, + "extruded extent must be the typed 5, got {min_z}..{max_z}" + ); + assert!( + app.tabs[0].active_cmd.is_none(), + "EXTRUDE must finish after the typed height" + ); + } + + /// Issue #1082 class regression for CYLINDER: the Height prompt accepts + /// keyword letters (2Point/Axis), so on web the typed digits land in the + /// dynamic-input Height box and the commit must parse them as the scalar + /// height instead of resolving an in-plane point whose height is ~0. + #[test] + fn cylinder_typed_height_from_dyn_field() { + use crate::app::Message; + let mut app = OpenCADStudio::new_for_test(); + app.automation_op(r#"{"op":"new"}"#); + + let _ = app.update(Message::CommandInput("CYLINDER".to_string())); + let _ = app.update(Message::CommandSubmit); + // Base center and radius through the command line. + let _ = app.update(Message::CommandInput("0,0".to_string())); + let _ = app.update(Message::CommandSubmit); + let _ = app.update(Message::CommandInput("3".to_string())); + let _ = app.update(Message::CommandSubmit); + // → Height step with a Height dynamic-input box. + assert!( + app.tabs[0] + .dyn_fields + .iter() + .any(|f| f.role == crate::command::DynRole::Height) + ); + + for ch in ["5"] { + let _ = app.update(Message::CommandAppendChar(ch.to_string())); + } + assert_eq!(app.command_line.input, ""); + assert!( + app.tabs[0] + .dyn_fields + .iter() + .any(|f| f.buffer.as_deref() == Some("5")) + ); + + let _ = app.update(Message::CommandFinalize); + + let (min_z, max_z) = solid_wire_extent(&app, |p| p.z) + .expect("typed height must build the cylinder"); + assert!( + (max_z - 5.0).abs() < 1e-6 && min_z.abs() < 1e-6, + "cylinder height must be the typed 5, got {min_z}..{max_z}" + ); + assert!( + app.tabs[0].active_cmd.is_none(), + "CYLINDER must finish after the typed height" + ); + } + + /// Issue #1082 class regression for BOX (and WEDGE, which shares + /// `BoxStep::Height`): the typed height must commit as the scalar height, + /// not resolve to an in-plane point whose plane-normal ordinate is ~0. + #[test] + fn box_and_wedge_typed_height_from_dyn_field() { + use crate::app::Message; + let mut app = OpenCADStudio::new_for_test(); + app.automation_op(r#"{"op":"new"}"#); + + // ── BOX: 10×5 footprint, typed height 7 ────────────────────────── + let _ = app.update(Message::CommandInput("BOX".to_string())); + let _ = app.update(Message::CommandSubmit); + let _ = app.update(Message::CommandInput("0,0".to_string())); + let _ = app.update(Message::CommandSubmit); + let _ = app.update(Message::CommandInput("10,5".to_string())); + let _ = app.update(Message::CommandSubmit); + assert!( + app.tabs[0] + .dyn_fields + .iter() + .any(|f| f.role == crate::command::DynRole::Height) + ); + for ch in ["7"] { + let _ = app.update(Message::CommandAppendChar(ch.to_string())); + } + let _ = app.update(Message::CommandFinalize); + + assert_eq!(solid_count(&app), 1, "BOX must commit after the typed height"); + let (min_x, max_x) = solid_wire_extent(&app, |p| p.x).expect("box solid"); + let (min_y, max_y) = solid_wire_extent(&app, |p| p.y).expect("box solid"); + let (min_z, max_z) = solid_wire_extent(&app, |p| p.z).expect("box solid"); + assert!( + (min_x - 0.0).abs() < 1e-6 + && (max_x - 10.0).abs() < 1e-6 + && (min_y - 0.0).abs() < 1e-6 + && (max_y - 5.0).abs() < 1e-6, + "box footprint must stay 10×5, got x {min_x}..{max_x}, y {min_y}..{max_y}" + ); + assert!( + (min_z - 0.0).abs() < 1e-6 && (max_z - 7.0).abs() < 1e-6, + "box height must be the typed 7, got {min_z}..{max_z}" + ); + assert!(app.tabs[0].active_cmd.is_none()); + + // ── WEDGE: same Height step (BoxStep::Height), typed height 9 ──── + let _ = app.update(Message::CommandInput("WEDGE".to_string())); + let _ = app.update(Message::CommandSubmit); + let _ = app.update(Message::CommandInput("0,0".to_string())); + let _ = app.update(Message::CommandSubmit); + let _ = app.update(Message::CommandInput("10,5".to_string())); + let _ = app.update(Message::CommandSubmit); + assert!( + app.tabs[0] + .dyn_fields + .iter() + .any(|f| f.role == crate::command::DynRole::Height) + ); + for ch in ["9"] { + let _ = app.update(Message::CommandAppendChar(ch.to_string())); + } + let _ = app.update(Message::CommandFinalize); + + assert_eq!(solid_count(&app), 2, "WEDGE must commit after the typed height"); + let (min_z, max_z) = solid_wire_extent(&app, |p| p.z).expect("wedge solid"); + assert!( + (min_z - 0.0).abs() < 1e-6 && (max_z - 9.0).abs() < 1e-6, + "wedge height must be the typed 9 (aggregate over both solids), got {min_z}..{max_z}" + ); + assert!(app.tabs[0].active_cmd.is_none()); + } + + /// Issue #1082 class regression for CONE and PYRAMID: their Height (and + /// HeightAfterTopRadius) prompts accept keyword letters, so the typed + /// digits must commit through `on_text_input` as the scalar height. Both + /// legs drive the Top-radius sub-prompt first so the committed solid is a + /// frustum whose top face lands in the wireframe (an apex cone's wires + /// carry only the base ellipse), and so the shared + /// `HeightAfterTopRadius` arm is covered too. + #[test] + fn cone_and_pyramid_typed_height_from_dyn_field() { + use crate::app::Message; + let mut app = OpenCADStudio::new_for_test(); + app.automation_op(r#"{"op":"new"}"#); + + // ── CONE: base center 0,0 radius 3, top radius 1, typed height 5 ─ + let _ = app.update(Message::CommandInput("CONE".to_string())); + let _ = app.update(Message::CommandSubmit); + let _ = app.update(Message::CommandInput("0,0".to_string())); + let _ = app.update(Message::CommandSubmit); + let _ = app.update(Message::CommandInput("3".to_string())); + let _ = app.update(Message::CommandSubmit); + assert!( + app.tabs[0] + .dyn_fields + .iter() + .any(|f| f.role == crate::command::DynRole::Height) + ); + // Top-radius sub-prompt, then the height (HeightAfterTopRadius). + let _ = app.update(Message::CommandInput("T".to_string())); + let _ = app.update(Message::CommandSubmit); + let _ = app.update(Message::CommandInput("1".to_string())); + let _ = app.update(Message::CommandSubmit); + assert!( + app.tabs[0] + .dyn_fields + .iter() + .any(|f| f.role == crate::command::DynRole::Height) + ); + for ch in ["5"] { + let _ = app.update(Message::CommandAppendChar(ch.to_string())); + } + let _ = app.update(Message::CommandFinalize); + + assert_eq!(solid_count(&app), 1, "CONE must commit after the typed height"); + let (min_z, max_z) = solid_wire_extent(&app, |p| p.z).expect("cone solid"); + let (.., max_r) = solid_wire_extent(&app, |p| p.x.hypot(p.y)).expect("cone solid"); + assert!( + (max_r - 3.0).abs() < 1e-6, + "cone base radius must stay 3, got {max_r}" + ); + assert!( + min_z.abs() < 1e-6 && (max_z - 5.0).abs() < 1e-6, + "cone height must be the typed 5, got {min_z}..{max_z}" + ); + assert!(app.tabs[0].active_cmd.is_none()); + + // ── PYRAMID: base center 0,0 radius 3, top radius 2, typed height 8 + let _ = app.update(Message::CommandInput("PYRAMID".to_string())); + let _ = app.update(Message::CommandSubmit); + let _ = app.update(Message::CommandInput("0,0".to_string())); + let _ = app.update(Message::CommandSubmit); + let _ = app.update(Message::CommandInput("3".to_string())); + let _ = app.update(Message::CommandSubmit); + assert!( + app.tabs[0] + .dyn_fields + .iter() + .any(|f| f.role == crate::command::DynRole::Height) + ); + let _ = app.update(Message::CommandInput("T".to_string())); + let _ = app.update(Message::CommandSubmit); + let _ = app.update(Message::CommandInput("2".to_string())); + let _ = app.update(Message::CommandSubmit); + assert!( + app.tabs[0] + .dyn_fields + .iter() + .any(|f| f.role == crate::command::DynRole::Height) + ); + for ch in ["8"] { + let _ = app.update(Message::CommandAppendChar(ch.to_string())); + } + let _ = app.update(Message::CommandFinalize); + + assert_eq!( + solid_count(&app), + 2, + "PYRAMID must commit after the typed height" + ); + let (min_z, max_z) = solid_wire_extent(&app, |p| p.z).expect("pyramid solid"); + assert!( + min_z.abs() < 1e-6 && (max_z - 8.0).abs() < 1e-6, + "pyramid height must be the typed 8 (aggregate over both solids), got {min_z}..{max_z}" + ); + assert!(app.tabs[0].active_cmd.is_none()); + } + + /// Issue #1082 class regression for HELIX: at the Final (height) prompt + /// the unfixed dyn commit resolves the typed span into an in-plane point + /// anchored at the last pick and commits that point's distance from the + /// center as the height — a wrong value whenever the previous pick wasn't + /// the center. Entering the top radius through its dynamic box anchors + /// the height prompt at (3,0,0), setting up exactly that case. + #[test] + fn helix_typed_height_from_dyn_field() { + use crate::app::Message; + let mut app = OpenCADStudio::new_for_test(); + app.automation_op(r#"{"op":"new"}"#); + + let _ = app.update(Message::CommandInput("HELIX".to_string())); + let _ = app.update(Message::CommandSubmit); + // Center, then a typed base radius. + let _ = app.update(Message::CommandInput("0,0".to_string())); + let _ = app.update(Message::CommandSubmit); + let _ = app.update(Message::CommandInput("2".to_string())); + let _ = app.update(Message::CommandSubmit); + // Pick the top radius through the dynamic-input field (the only web + // route): the resolved point (3,0,0) becomes the anchor/last point, + // off the center — exactly the case where an unfixed point-resolved + // typed height would commit the wrong value. + for ch in ["3"] { + let _ = app.update(Message::CommandAppendChar(ch.to_string())); + } + let _ = app.update(Message::CommandFinalize); + // → Final step with a Distance dynamic-input box. + assert!( + app.tabs[0] + .dyn_fields + .iter() + .any(|f| f.role == crate::command::DynRole::Distance) + ); + assert_eq!(app.last_point, Some(glam::DVec3::new(3.0, 0.0, 0.0))); + + for ch in ["5"] { + let _ = app.update(Message::CommandAppendChar(ch.to_string())); + } + assert_eq!(app.command_line.input, ""); + assert!( + app.tabs[0] + .dyn_fields + .iter() + .any(|f| f.buffer.as_deref() == Some("5")) + ); + + let _ = app.update(Message::CommandFinalize); + + let helix = app.tabs[0] + .scene + .document + .entities() + .find_map(|e| match e { + acadrust::EntityType::Helix(helix) => Some(helix.clone()), + _ => None, + }) + .expect("typed height must create the helix"); + let height = helix.turn_height * helix.turns; + assert!( + (height - 5.0).abs() < 1e-6, + "helix height must be the typed 5, got {height}" + ); + assert!( + (helix.axis_vector.z - 1.0).abs() < 1e-9, + "helix axis must stay the plane normal" + ); + assert!( + app.tabs[0].active_cmd.is_none(), + "HELIX must finish after the typed height" + ); + } + + /// Issue #1082 class regression for SWEEP's reference-scale prompts: the + /// typed reference length must be parsed as a length (advancing to the + /// new-length prompt), and the typed new length must set the scale + /// (new/reference) — visible in the swept solid's size. Unfixed, the + /// typed reference length is dropped as a stray first reference point. + #[test] + fn sweep_typed_scale_lengths_from_dyn_field() { + use crate::app::Message; + use acadrust::types::Vector3; + let mut app = OpenCADStudio::new_for_test(); + app.automation_op(r#"{"op":"new"}"#); + // A unit circle profile swept along a vertical line path. + let mut circle = acadrust::Circle::new(); + circle.radius = 1.0; + let profile = app.tabs[0] + .scene + .document + .add_entity(acadrust::EntityType::Circle(circle)) + .unwrap(); + let path = app.tabs[0] + .scene + .document + .add_entity(acadrust::EntityType::Line(acadrust::Line::from_points( + Vector3::new(0.0, 0.0, 0.0), + Vector3::new(0.0, 0.0, 10.0), + ))) + .unwrap(); + + let _ = app.update(Message::CommandInput("SWEEP".to_string())); + let _ = app.update(Message::CommandSubmit); + // Select the profile, bare Enter finishes selection → path prompt. + let _ = app.feed_command(crate::command::StepInput::SelectionComplete(vec![profile])); + let _ = app.update(Message::CommandFinalize); + // Enter the reference-scale sub-prompt through the command line. + let _ = app.update(Message::CommandInput("SCALE".to_string())); + let _ = app.update(Message::CommandSubmit); + let _ = app.update(Message::CommandInput("REFERENCE".to_string())); + let _ = app.update(Message::CommandSubmit); + // → ReferenceLength prompt with dynamic-input boxes showing. + assert!(!app.tabs[0].dyn_fields.is_empty()); + + // Typed reference length 2 commits through the dyn-field buffer. + for ch in ["2"] { + let _ = app.update(Message::CommandAppendChar(ch.to_string())); + } + assert_eq!(app.command_line.input, ""); + let _ = app.update(Message::CommandFinalize); + let prompt = app.tabs[0] + .active_cmd + .as_ref() + .map(|c| c.prompt()) + .unwrap_or_default(); + assert!( + prompt.to_lowercase().contains("new length"), + "typed reference length must advance to the new-length prompt, got: {prompt}" + ); + + // Typed new length 6 → scale 6/2 = 3 → back to the path prompt. + for ch in ["6"] { + let _ = app.update(Message::CommandAppendChar(ch.to_string())); + } + let _ = app.update(Message::CommandFinalize); + let prompt = app.tabs[0] + .active_cmd + .as_ref() + .map(|c| c.prompt()) + .unwrap_or_default(); + assert!( + prompt.to_lowercase().contains("sweep path"), + "typed new length must return to the path prompt, got: {prompt}" + ); + + // Picking the path finishes the sweep: the unit circle must have been + // scaled by 3, so the swept solid's wires reach radius 3. + let _ = app.feed_command(crate::command::StepInput::EntityPick( + path, + glam::DVec3::new(0.0, 0.0, 5.0), + )); + + let (.., max_r) = solid_wire_extent(&app, |p| p.x.hypot(p.y)) + .expect("sweep must create a solid"); + assert!( + (max_r - 3.0).abs() < 1e-3, + "swept radius must be the profile 1 × scale 3, got {max_r}" + ); + assert!( + app.tabs[0].active_cmd.is_none(), + "SWEEP must finish after the path pick" + ); + } } diff --git a/src/app/mod.rs b/src/app/mod.rs index a61def029..0a1dbae15 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -3260,6 +3260,11 @@ pub enum Message { /// Web never constructs it — printing opens the PDF preview instead. #[cfg_attr(target_arch = "wasm32", allow(dead_code))] PrintResult(Result), + /// Printers enumerated off the UI thread for the Plot dialog's dropdown; + /// the dialog opens immediately and the list lands whenever the spooler + /// query finishes. Web never constructs it — it has no system printers. + #[cfg_attr(target_arch = "wasm32", allow(dead_code))] + PrintersListed(Vec), /// Open the full Plot / Print dialog (seeds state from the layout). PlotDialogOpen, /// An edit inside the Plot / Print dialog. @@ -3290,8 +3295,12 @@ pub enum Message { /// Open file dialog to load a CTB/STB plot style table. PlotStyleLoad, /// Callback when the user picks (or cancels) a CTB/STB file. - /// `Ok(None)` = cancelled; `Err` = the picked file failed to parse. - PlotStyleLoaded(Result, String>), + /// `Ok(None)` = cancelled; `Err` = the picked file failed to parse. The + /// table carries an optional non-fatal note for the command line (e.g. + /// registration kept an already-registered table of the same name). + PlotStyleLoaded( + Result)>, String>, + ), /// Clear the active plot style table. PlotStyleClear, /// Open/close the Plot Style panel. diff --git a/src/app/update/file.rs b/src/app/update/file.rs index 96706865d..7defd369a 100644 --- a/src/app/update/file.rs +++ b/src/app/update/file.rs @@ -4041,7 +4041,6 @@ impl OpenCADStudio { .collect(); // Keep session-only choices while loading drawing fields from the layout. let d = &mut self.plot_dialog; - d.printers = crate::io::print_to_printer::list_printers(); d.plot_styles = crate::io::plot_style::available_ctb_names(); d.scales = scales; d.plot_views = plot_views; @@ -4124,6 +4123,21 @@ impl OpenCADStudio { } } self.active_modal = Some(crate::app::ModalKind::Plot); + // Enumerating printers can block for seconds on spooler RPC (dead + // print servers), so it must not run on the UI thread: open the + // dialog immediately and let the printer dropdown populate when the + // worker replies with Message::PrintersListed. The list is session + // state on the dialog, so the late assignment re-renders the pick + // list without touching the user's selection. Native only — the web + // build has no system printers and offers PDF output alone. + #[cfg(not(target_arch = "wasm32"))] + { + background_task( + crate::io::print_to_printer::list_printers, + Message::PrintersListed, + ) + } + #[cfg(target_arch = "wasm32")] Task::none() } diff --git a/src/app/update/mod.rs b/src/app/update/mod.rs index b34ce459e..532676b60 100644 --- a/src/app/update/mod.rs +++ b/src/app/update/mod.rs @@ -8335,12 +8335,19 @@ impl OpenCADStudio { .push_error(crate::tf!("Print failed: {e}").as_ref()); Task::none() } + // The printer list arrives after the Plot dialog already opened + // (enumeration runs off the UI thread); assigning it re-renders + // the dropdown while leaving the user's selection untouched. + Message::PrintersListed(printers) => { + self.plot_dialog.printers = printers; + Task::none() + } // ── Plot Style Table ────────────────────────────────────────────── Message::PlotStyleLoad => { Task::perform(crate::io::pick_plot_style(), Message::PlotStyleLoaded) } - Message::PlotStyleLoaded(Ok(Some(table))) => { + Message::PlotStyleLoaded(Ok(Some((table, note)))) => { if table.is_stb { self.command_line.push_error( crate::t!( @@ -8364,6 +8371,11 @@ impl OpenCADStudio { ) .as_ref(), ); + // Non-fatal registration feedback — e.g. the picked file was + // already registered and the existing table was kept. + if let Some(note) = note { + self.command_line.push_info(¬e); + } self.active_plot_style = Some(table); self.plot_dialog.plot_styles = crate::io::plot_style::available_ctb_names(); Task::none() diff --git a/src/app/view/mod.rs b/src/app/view/mod.rs index d45aabcfd..d7b39ceb9 100644 --- a/src/app/view/mod.rs +++ b/src/app/view/mod.rs @@ -771,6 +771,9 @@ bg={bg_ms:.1}ms n={view_count}" // state machine stay untouched. let (_, (ux, uy, _)) = self.drafting_grid_basis(i); let (ux, uy) = (ux.as_dvec3(), uy.as_dvec3()); + // Guides quantize to the polar increment even + // while a window-corner drag suppresses polar in + // the snapper (drafting gate) — cosmetic only. let step = self.polar_mode.then_some(self.polar_increment_deg); let perp_on = self.snapper.is_on(crate::snap::SnapType::Perpendicular); diff --git a/src/io/mod.rs b/src/io/mod.rs index 7236ba97c..b92bba1d0 100644 --- a/src/io/mod.rs +++ b/src/io/mod.rs @@ -1526,8 +1526,11 @@ mod save_failure_tests { /// `Ok(None)` means the user cancelled the dialog; `Err` carries a parse /// failure so it reaches the command line instead of dying silently (the /// legacy-CTB complaint in #1261d was partly that a file which failed to -/// parse produced no feedback at all). -pub async fn pick_plot_style() -> Result, String> { +/// parse produced no feedback at all). The table is paired with an optional +/// non-fatal note for the command line — currently the registration +/// collision message below. +pub async fn pick_plot_style() +-> Result)>, String> { let dialog = crate::sys::file_dialog() .set_title(crate::t!("Load Plot Style Table").as_ref()) .add_filter( @@ -1553,17 +1556,137 @@ pub async fn pick_plot_style() -> Result, Str })?; // Loading a table registers it, the way AutoCAD does: copy it next to the // built-ins so it stays in the Plot dialog's table list after restart. - // Failures are non-fatal — the table is already applied to this plot. + // Only real AutoCAD containers are registered — the permissive heuristic + // also accepts junk (any text file, a stray deflate stream), and writing + // those into the folder would pollute it permanently — so heuristic + // parses stay loadable for this session only. Failures are non-fatal — + // the table is already applied to this plot. #[cfg(not(target_arch = "wasm32"))] - if !table.is_stb { - if let (Ok(dir), Some(file_name)) = (plot_style::plot_styles_dir(), path.file_name()) { - let registered = dir.join(file_name); - if !registered.exists() { - let _ = std::fs::copy(&path, ®istered); - } - } + let note = should_register_plot_style(&table) + .then(|| register_picked_plot_style(&path)) + .flatten(); + #[cfg(target_arch = "wasm32")] + let note = None; + Ok(Some((table, note))) +} + +/// Whether a loaded table should be registered permanently: only real +/// AutoCAD CTB containers. Named (STB) tables are not offered in the +/// color-dependent list, and heuristic parses (plain text, a stray deflate +/// stream) must not be written into the plot-styles folder. +#[cfg(not(target_arch = "wasm32"))] +fn should_register_plot_style(table: &plot_style::PlotStyleTable) -> bool { + !table.is_stb && table.structured +} + +/// Copy the picked CTB into the per-user plot-styles folder, reporting a +/// command-line note when the already-registered table was kept instead. +#[cfg(not(target_arch = "wasm32"))] +fn register_picked_plot_style(path: &Path) -> Option { + plot_style::plot_styles_dir() + .ok() + .and_then(|dir| register_plot_style_into(&dir, path)) +} + +/// Copy `path` into `dir` unless a table with the same file name is already +/// registered there. A collision keeps the registered file — overwriting it +/// from a stray pick would silently destroy edits made through the Plot +/// Style panel — and returns a note naming the kept table so the user hears +/// about it. Copy failures stay silent: the table is already applied to the +/// current plot. +#[cfg(not(target_arch = "wasm32"))] +fn register_plot_style_into(dir: &Path, path: &Path) -> Option { + let file_name = path.file_name()?; + let registered = dir.join(file_name); + if registered.exists() { + return Some( + crate::tf!( + "'{}' is already registered; the existing table was kept.", + file_name.to_string_lossy() + ) + .into_owned(), + ); + } + let _ = std::fs::copy(path, ®istered); + None +} + +#[cfg(all(test, not(target_arch = "wasm32")))] +mod plot_style_registration_tests { + use super::{register_plot_style_into, should_register_plot_style}; + use crate::io::plot_style::{self, PlotStyleTable}; + + fn scratch_dir(tag: &str) -> std::path::PathBuf { + let dir = std::env::temp_dir().join(format!( + "ocs_ctb_register_{tag}_{}_{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + std::fs::create_dir_all(&dir).unwrap(); + dir + } + + /// A name collision keeps the registered table and surfaces a note + /// naming it instead of silently discarding the pick. + #[test] + fn a_collision_keeps_the_registered_table_and_notes_it() { + let dir = scratch_dir("collide"); + let picked = dir.join("picked.ctb"); + std::fs::write(&picked, b"freshly picked").unwrap(); + std::fs::write(dir.join("picked.ctb"), b"already registered").unwrap(); + let note = register_plot_style_into(&dir, &picked).expect("collision note"); + assert!(note.contains("picked.ctb"), "{note}"); + assert!(note.contains("already registered"), "{note}"); + assert!(note.contains("kept"), "{note}"); + assert_eq!( + std::fs::read(dir.join("picked.ctb")).unwrap(), + b"already registered" + ); + let _ = std::fs::remove_dir_all(&dir); + } + + /// Without a collision the picked table is copied and no note is pushed. + #[test] + fn a_fresh_table_is_copied_without_a_note() { + let dir = scratch_dir("fresh"); + let picked = dir.join("source").join("new.ctb"); + std::fs::create_dir_all(picked.parent().unwrap()).unwrap(); + std::fs::write(&picked, b"fresh bytes").unwrap(); + assert!(register_plot_style_into(&dir, &picked).is_none()); + assert_eq!( + std::fs::read(dir.join("new.ctb")).unwrap(), + b"fresh bytes" + ); + let _ = std::fs::remove_dir_all(&dir); + } + + /// Registration is reserved for real AutoCAD containers: the permissive + /// heuristic loads junk files for the session but they must never be + /// written into the plot-styles folder. + #[test] + fn only_structured_containers_are_registrable() { + let builtin = PlotStyleTable::builtin(plot_style::DEFAULT_PLOT_STYLE).unwrap(); + assert!(builtin.structured); + assert!(should_register_plot_style(&builtin)); + let mut stb = builtin.clone(); + stb.is_stb = true; + assert!(!should_register_plot_style(&stb)); + + // A loose zlib stream without the container header still parses… + use std::io::Write; + let mut enc = + flate2::write::ZlibEncoder::new(Vec::new(), flate2::Compression::default()); + enc.write_all(b"description=\"junk\n").unwrap(); + let mut loose = b"any first line\n".to_vec(); + loose.extend_from_slice(&enc.finish().unwrap()); + let junk = PlotStyleTable::from_bytes("junk.ctb", &loose).unwrap(); + // …but stays session-only. + assert!(!junk.structured); + assert!(!should_register_plot_style(&junk)); } - Ok(Some(table)) } // ── Image file picker ───────────────────────────────────────────────────── diff --git a/src/io/pdf_export.rs b/src/io/pdf_export.rs index b989dd0c2..883bc1873 100644 --- a/src/io/pdf_export.rs +++ b/src/io/pdf_export.rs @@ -506,6 +506,11 @@ pub fn render_pdf_bytes( // Glyph Form XObjects keyed by the atlas tile key — shared across pages so // multi-layout exports bake each unique glyph once. let mut glyph_forms = std::collections::HashMap::new(); + // Snapshot the atlas once for the whole export (see `atlas_export_snapshot`): + // `emit_text` used to re-snapshot per TEXT entity, re-cloning every baked + // glyph's strokes and fill triangulation each call while the forms cache + // here makes only the first copy useful. + let atlas = atlas_export_snapshot(pages); for (index, page) in pages.iter().enumerate() { append_pdf_page( &mut doc, @@ -513,6 +518,7 @@ pub fn render_pdf_bytes( &mut glyph_forms, page, plot_style, + atlas.as_ref(), ) .map_err(|error| format!("Page {}: {error}", index + 1))?; } @@ -520,12 +526,48 @@ pub fn render_pdf_bytes( Ok(doc.save(&PdfSaveOptions::default(), &mut warnings)) } +/// Per-export atlas snapshot: every baked glyph's vector geometry keyed by its +/// tile `uv_min`, plus the reserved solid-texel key for decoration bars. +type AtlasExport = ( + std::collections::HashMap, + u64, +); + +/// Snapshot the atlas' baked-glyph geometry ONCE per export, not per TEXT +/// entity. +/// +/// `export_table` re-resolves each baked entry through its `Face` and clones +/// its strokes and fill triangulation, so calling it from every `emit_text` +/// re-copied the whole atlas for each of the hundreds of text wires a drawing +/// carries — while holding the global atlas mutex. Export is single-threaded +/// here and the glyph forms are cached document-wide, so one snapshot serves +/// every page and entity. Returns `None` when no page carries text (nothing +/// to snapshot) or the atlas mutex is poisoned (the old per-entity code +/// skipped text in that case too). +fn atlas_export_snapshot(pages: &[PdfPageInput]) -> Option { + let has_text = pages.iter().any(|page| { + page.content + .wires + .iter() + .any(|wire| !wire.wire.text_verts.is_empty()) + }); + if !has_text { + return None; + } + let atlas = crate::scene::text::sdf_atlas::text_atlas().lock().ok()?; + Some(( + atlas.export_table(), + crate::scene::text::sdf_atlas::uv_key(atlas.solid_uv()), + )) +} + fn append_pdf_page( doc: &mut PdfDocument, image_resources: &mut std::collections::HashMap<(usize, u32, u32), printpdf::XObjectId>, glyph_forms: &mut std::collections::HashMap, page: &PdfPageInput, fallback_plot_style: Option<&PlotStyleTable>, + atlas: Option<&AtlasExport>, ) -> Result<(), String> { // Paper dimensions are validated like the content coordinates below: a // NaN/inf/zero sheet (`width < MIN` compares false for NaN) would be @@ -758,6 +800,7 @@ fn append_pdf_page( emit_text( doc, glyph_forms, + atlas, &mut ops, std::slice::from_ref(&wire.wire), ox, @@ -1551,10 +1594,12 @@ fn emit_hatch( // ── Text (SDF glyph quads → vector strokes / fills) ──────────────────────── /// Format a coordinate as the shortest PDF real with 0.01 resolution -/// (`3`, `3.5`, `-0.25`). 0.01 pt ≈ 3.5 µm on paper — visually lossless, but -/// it keeps path operands half the size of printpdf's default -/// shortest-round-trip f32 text (`41.5445 28.712158` → `41.54 28.71`), which -/// dominates text-heavy vector pages (#702). +/// (`3`, `3.5`, `-0.25`). The resolution is in the units of whichever stream +/// the operand lands in: 0.01 pt (≈ 3.5 µm on paper) for page-space operands, +/// 0.01 glyph-plane units (≈ 0.1% of the 9-unit atlas tile) for Form XObject +/// content. Both are visually lossless, and half the size of printpdf's +/// default shortest-round-trip f32 text (`41.5445 28.712158` → `41.54 28.71`), +/// which dominates text-heavy vector pages (#702). fn pdf_real(v: f32) -> String { let rounded = (v * 100.0).round() / 100.0; if rounded == rounded.trunc() && rounded.abs() < 1e15 { @@ -1570,6 +1615,23 @@ fn pdf_real(v: f32) -> String { s } +/// Round a `cm` scale operand to 4 significant digits (`0.7874015` → `0.7874`). +/// +/// The glyph `cm` scale entries are pt per glyph-plane unit — ~0.5–0.9 for +/// small text — so quantizing them on the absolute 0.01 grid used for +/// translations fixed the ratio by up to ~1%, accumulating visible +/// advance-width drift across a text run. Four significant digits keep the +/// relative error within ~0.05%; translations stay at 0.01 because they are +/// absolute page positions, not ratios. +fn round_sig4(v: f64) -> f64 { + if !v.is_finite() || v == 0.0 { + return v; + } + let decimals = (3 - v.abs().log10().floor() as i32).clamp(0, 17); + let factor = 10f64.powi(decimals); + (v * factor).round() / factor +} + /// Serialize one glyph's vector geometry as a compact PDF content stream in /// glyph-plane units (the 9-unit space the atlas tile spans). /// @@ -1698,9 +1760,13 @@ fn adapt_text_color([r, g, b]: [f32; 3]) -> [f32; 3] { /// quad's `uv_min` and map it into that quad by affine interpolation of the /// plane rect — so a stroke (LFF) font emits polylines and a filled TrueType /// glyph emits filled triangles, exactly where the SDF quad sits. +/// +/// The atlas geometry comes from `atlas`, the export-wide snapshot taken once +/// in `render_pdf_bytes`. fn emit_text( doc: &mut PdfDocument, glyph_forms: &mut std::collections::HashMap, + atlas: Option<&AtlasExport>, ops: &mut Vec, wires: &[WireModel], ox: f64, @@ -1714,12 +1780,8 @@ fn emit_text( if wires.iter().all(|w| w.text_verts.is_empty()) { return; } - // Snapshot the atlas' baked-glyph geometry once; drop the lock before use. - let (table, solid_key) = { - let Ok(atlas) = sdf_atlas::text_atlas().lock() else { - return; - }; - (atlas.export_table(), sdf_atlas::uv_key(atlas.solid_uv())) + let Some((table, solid_key)) = atlas else { + return; }; // `Op::SetLineDashPattern` is persistent graphics state and the wire pass @@ -1816,11 +1878,79 @@ fn emit_text( let uy = (br[1] - bl[1]) / sx as f64; let vx = (tl[0] - bl[0]) / sy as f64; let vy = (tl[1] - bl[1]) / sy as f64; + // Degenerate quad (collapsed corners → zero-area transform): + // skip emission, mirroring the plane guard above, instead of + // letting a near-zero scale reach the pen division below and + // paint a page-spanning hairline. + let det = ux * vy - uy * vx; + if det.abs() < 1e-9 { + continue; + } let origin_x = bl[0] - ux * pmin[0] as f64 - vx * pmin[1] as f64; let origin_y = bl[1] - uy * pmin[0] as f64 - vy * pmin[1] as f64; let m2pt = MM_TO_PT as f64; let r2 = |v: f64| ((v * 100.0).round() / 100.0) as f32; + if let Some(ctb_pen) = lw_override.filter(|_| ge.fill_tris.is_empty()) { + // Heavy CTB pen: bypass the glyph Form XObject and stroke the + // centrelines DIRECTLY in page space. The Form /BBox is the + // atlas tile rect, whose 1.5-unit SDF spread margin bounds + // the nominal pen — an absolute CTB lineweight is arbitrary + // pt and can exceed it (2.5 mm text + 1.0 mm pen → 1.79-unit + // half-width > 1.5), squaring round caps off at the tile + // edge. The override is per text entity, so every stroked + // glyph of the entity takes this one path — no per-glyph + // mixing of Form and direct pens (a filled TrueType + // fallback glyph keeps its Form; it has no pen to clip). + // The Form-dedupe win is irrelevant for the rare heavy-pen + // case. The pen is already divided by the page CTM scale + // above, so it stays absolute on paper. + if last_outline.map(|c| color_differs(c, [r, g, b])).unwrap_or(true) { + ops.push(Op::SetOutlineColor { + col: Color::Rgb(Rgb { + r, + g, + b, + icc_profile: None, + }), + }); + last_outline = Some([r, g, b]); + } + ops.push(Op::SetOutlineThickness { + pt: Pt(if ge.bold { + ctb_pen * 1.7 + } else { + ctb_pen + }), + }); + let map = |p: [f32; 2]| -> Point { + let u = ((p[0] - pmin[0]) / sx) as f64; + let v = ((p[1] - pmin[1]) / sy) as f64; + point( + bl[0] + u * (br[0] - bl[0]) + v * (tl[0] - bl[0]), + bl[1] + u * (br[1] - bl[1]) + v * (tl[1] - bl[1]), + ) + }; + for stroke in &ge.strokes { + if stroke.len() < 2 { + continue; + } + ops.push(Op::DrawLine { + line: Line { + points: stroke + .iter() + .map(|&p| LinePoint { + p: map(p), + bezier: false, + }) + .collect(), + is_closed: false, + }, + }); + } + continue; + } + // Colour ops go OUTSIDE the per-glyph q/Q: the CTM must be // scoped, but fill colour has to persist past the Q or the // next glyph would fill with the page-default black despite @@ -1840,12 +1970,13 @@ fn emit_text( last_fill = Some([r, g, b]); } } else { - // Stroke (LFF/SHX pen) font or hollow glyph. - // Match the SDF atlas' nominal glyph-space pen instead of - // borrowing the entity lineweight: Roman Duplex and similar + // Stroke (LFF/SHX pen) font or hollow glyph, nominal pen: + // match the SDF atlas' glyph-space pen instead of borrowing + // the entity lineweight — Roman Duplex and similar // multi-stroke faces rely on that band to close the narrow - // gaps between parallel centrelines. An explicit CTB - // lineweight still wins and stays absolute under the plot CTM. + // gaps between parallel centrelines. (An explicit CTB + // lineweight never reaches this Form path; it takes the + // direct heavy-pen path above.) if last_outline.map(|c| color_differs(c, [r, g, b])).unwrap_or(true) { ops.push(Op::SetOutlineColor { col: Color::Rgb(Rgb { @@ -1857,28 +1988,27 @@ fn emit_text( }); last_outline = Some([r, g, b]); } - let pen = if let Some(ctb_pen) = lw_override { - if ge.bold { - ctb_pen * 1.7 - } else { - ctb_pen - } - } else { - let glyph_unit_mm = (((tl[0] - bl[0]).powi(2) + (tl[1] - bl[1]).powi(2)) - .sqrt() - / sy.abs() as f64) as f32; + let glyph_unit_mm = (((tl[0] - bl[0]).powi(2) + (tl[1] - bl[1]).powi(2)) + .sqrt() + / sy.abs() as f64) as f32; + let pen = (2.0 * sdf_atlas::stroke_pen_half_units(ge.bold) * glyph_unit_mm * MM_TO_PT) - .max(0.1) - }; + .max(0.1); stroke_pen = Some(((pen * 100.0).round() / 100.0).max(0.1)); } ops.push(Op::SaveGraphicsState); ops.push(Op::SetTransformationMatrix { matrix: printpdf::CurTransMat::Raw([ - r2(ux * m2pt), - r2(uy * m2pt), - r2(vx * m2pt), - r2(vy * m2pt), + // Scale entries carry the pt-per-glyph-unit ratio + // (~0.5–0.9 for small text) and are rounded to 4 + // significant digits — the 0.01 absolute grid would + // quantize them by ~1% and visibly drift the advance + // width across a text run. + round_sig4(ux * m2pt) as f32, + round_sig4(uy * m2pt) as f32, + round_sig4(vx * m2pt) as f32, + round_sig4(vy * m2pt) as f32, + // Translations are absolute page positions: 0.01 pt. r2((origin_x + ox) * m2pt), r2((origin_y + oy) * m2pt), ]), @@ -1886,10 +2016,12 @@ fn emit_text( if let Some(pen) = stroke_pen { // Line width is GEOMETRIC state: the CTM above scales a `w` // set before it, so the pen is re-set per glyph, under its - // own CTM, pre-divided by the quad's area scale to keep the - // on-paper width identical to stroking in page units. - let det = (ux * vy - uy * vx).abs() * m2pt * m2pt; - let unit_scale = det.sqrt().max(1e-6) as f32; + // own CTM, pre-divided by the quad's transform. + // sqrt(|det|) is the geometric-mean width — it matches the + // on-screen SDF band, and is exact only for uniform scales + // (an anisotropic quad strokes marginally off in its thin + // axis, the same compromise the SDF render makes). + let unit_scale = (det.abs() * m2pt * m2pt).sqrt() as f32; ops.push(Op::SetOutlineThickness { pt: Pt((pen / unit_scale * 100.0).round() / 100.0), }); @@ -1912,7 +2044,7 @@ fn emit_text( transform: Default::default(), }); ops.push(Op::RestoreGraphicsState); - } else if key == solid_key { + } else if key == *solid_key { // Decoration bar (underline / overline / strike): the quad is a // solid-texel rectangle — fill it directly from its corners. if last_fill.map(|c| color_differs(c, [r, g, b])).unwrap_or(true) { @@ -2030,6 +2162,19 @@ mod tests { assert_eq!(downsample_target(2, 2, 1500.0, 150), (1, 1)); } + #[test] + fn round_sig4_keeps_four_significant_digits() { + // Small-text pt-per-unit ratios keep 4 decimals… + assert!((round_sig4(0.78740156) - 0.7874).abs() < 1e-12); + assert!((round_sig4(-0.512399) - (-0.5124)).abs() < 1e-12); + // …larger magnitudes shift the kept digits, not the precision… + assert!((round_sig4(7.8740156) - 7.874).abs() < 1e-12); + assert!((round_sig4(28.346456) - 28.35).abs() < 1e-12); + // …and tiny ratios gain decimals instead of collapsing onto 0.01/0.1. + assert!((round_sig4(0.078740156) - 0.07874).abs() < 1e-12); + assert_eq!(round_sig4(0.0), 0.0); + } + #[test] fn plot_quality_dpi_caps_match_the_dialog_presets() { assert_eq!(PlotQuality::High.target_dpi(), None); diff --git a/src/io/plot_style.rs b/src/io/plot_style.rs index e0894362f..9e79d4d66 100644 --- a/src/io/plot_style.rs +++ b/src/io/plot_style.rs @@ -166,6 +166,12 @@ pub struct PlotStyleTable { pub name: String, /// Whether this is a named-style (STB) table rather than color-based (CTB). pub is_stb: bool, + /// Whether the bytes parsed through the real AutoCAD container + /// (`PIAFILEVERSION_2.0,CTBVER1/STBVER1,compress`) rather than the + /// permissive heuristic fallback that also accepts loose text or deflate + /// streams. Only container tables are registered permanently; heuristic + /// parses stay loadable for the session only. + pub structured: bool, pub description: String, pub scale_factor: f32, pub apply_factor: bool, @@ -183,6 +189,7 @@ impl PlotStyleTable { PlotStyleTable { name: name.into(), is_stb: false, + structured: false, description: String::new(), scale_factor: 1.0, apply_factor: false, @@ -207,8 +214,12 @@ impl PlotStyleTable { pub fn from_bytes(name: impl Into, raw: &[u8]) -> Result { let name = name.into(); let is_stb = name.to_ascii_lowercase().ends_with(".stb"); - let text = decompress_ctb(raw)?; - parse_plot_style_text(&text, name, is_stb) + let (text, structured) = decompress_ctb(raw)?; + let mut table = parse_plot_style_text(&text, name, is_stb)?; + // Record the parse route so callers can refuse to permanently + // register tables that only the heuristic accepted. + table.structured = structured; + Ok(table) } pub fn builtin(name: &str) -> Result { @@ -434,17 +445,19 @@ impl PlotStyleTable { // ── Deflate helpers ─────────────────────────────────────────────────────────── -/// Decompress a CTB/STB file's raw bytes into the text content. -/// -fn decompress_ctb(data: &[u8]) -> Result { +/// Decompress a CTB/STB file's raw bytes into the text content, reporting +/// whether the real AutoCAD container was used. `false` means the permissive +/// heuristic fallback (skip the first line, try zlib/deflate) accepted it. +fn decompress_ctb(data: &[u8]) -> Result<(String, bool), String> { // CTB and STB files share this container; only the embedded version tag // differs ("CTBVER1" vs "STBVER1"). Accept both so real AutoCAD STB files // take the structured path instead of falling through to the raw // deflate heuristic below (where they fail with a confusing zlib error). const CTB_PREFIX: &[u8] = b"PIAFILEVERSION_2.0,CTBVER1,compress\r\npmzlibcodec"; const STB_PREFIX: &[u8] = b"PIAFILEVERSION_2.0,STBVER1,compress\r\npmzlibcodec"; + let structured = data.starts_with(CTB_PREFIX) || data.starts_with(STB_PREFIX); let mut decoded = Vec::new(); - if data.starts_with(CTB_PREFIX) || data.starts_with(STB_PREFIX) { + if structured { if data.len() < 60 { return Err("CTB header is truncated".into()); } @@ -490,7 +503,8 @@ fn decompress_ctb(data: &[u8]) -> Result { if decoded.last() == Some(&0) { decoded.pop(); } - String::from_utf8(decoded).map_err(|e| format!("CTB text is not UTF-8: {e}")) + let text = String::from_utf8(decoded).map_err(|e| format!("CTB text is not UTF-8: {e}"))?; + Ok((text, structured)) } /// Compress plot-style text content as a CTB/STB file. @@ -694,6 +708,7 @@ fn parse_plot_style_text(text: &str, name: String, is_stb: bool) -> Result std::io::Result> { // but on Windows 11 it can miss per-user network printers that only the // spooler RPC (level 2) reports (#1261a). The level-2 query is expensive — // it opens every remote connection and can wait out RPC timeouts on dead - // print servers — so it only runs when the cached query came back empty; - // machines whose level-4 list works never pay for it. (The flags stay - // LOCAL|CONNECTIONS on purpose: the EnumPrintersW documentation restricts - // level 4 to exactly those flags, and PRINTER_ENUM_NETWORK is only valid - // with level 1, so completeness comes from the level-2 retry, not flags.) + // print servers — so it only runs when the cached list looks incomplete: + // either it came back empty, or it does not contain the configured + // default printer, which is exactly the case where the default is one of + // the hidden per-user connections. Machines whose level-4 list already + // covers the default never pay for it. A level-2 failure is not fatal: + // the level-4 names stand and the default merge below still guarantees + // the default is offered. (The flags stay LOCAL|CONNECTIONS on purpose: + // the EnumPrintersW documentation restricts level 4 to exactly those + // flags, and PRINTER_ENUM_NETWORK is only valid with level 1, so + // completeness comes from the level-2 retry, not flags.) let mut names = enum_printer_names( 4, PRINTER_ENUM_LOCAL | PRINTER_ENUM_CONNECTIONS, @@ -129,18 +134,26 @@ fn windows_printers() -> std::io::Result> { // pPrinterName is the first field of PRINTER_INFO_4W. 0, )?; - if names.is_empty() { - names = enum_printer_names( + let default = windows_default_printer(); + let hides_default = default + .as_ref() + .is_some_and(|name| !names.iter().any(|n| n.eq_ignore_ascii_case(name))); + if names.is_empty() || hides_default { + // Errors here are deliberately swallowed: falling through with the + // level-4 names still lets the default merge below run. + if let Ok(extra) = enum_printer_names( 2, PRINTER_ENUM_LOCAL | PRINTER_ENUM_CONNECTIONS, std::mem::size_of::(), // pPrinterName is the second pointer field of PRINTER_INFO_2W. std::mem::size_of::(), - )?; + ) { + names.extend(extra); + } } // The spooler's own default is authoritative even when enumeration missed // it; always offer it explicitly so the list can never hide it (#1261b). - if let Some(default) = windows_default_printer() { + if let Some(default) = default { if !names.iter().any(|name| name.eq_ignore_ascii_case(&default)) { names.push(default); } @@ -301,6 +314,18 @@ fn windows_default_printer() -> Option { (!trimmed.is_empty()).then(|| trimmed.to_string()) } +/// Build the `lpParameters` string for the shell `printto` verb. The verb +/// passes four arguments — file, printer, driver, port — and registered PDF +/// handlers quote each slot (Acrobat: `/t "%1" "%2" "%3" "%4"`), i.e. they +/// whitespace-tokenise the parameter string. A bare printer name therefore +/// shatters on spaces ("Microsoft Print to PDF" becomes printer=Microsoft, +/// driver=Print, port=to PDF) and the job is lost with all UI suppressed. +/// Quote the name and pass the empty driver/port slots handlers expect. +#[cfg(target_os = "windows")] +fn printto_params(printer: &str) -> String { + format!("\"{printer}\" \"\" \"\"") +} + /// Probe the spooler for `name` before dispatching a job (#1261c). /// `OpenPrinterW` with no `PRINTER_DEFAULTSW` is a pure /// existence/permission check: it spools nothing and shows no UI, and gives a @@ -368,11 +393,24 @@ fn describe_shell_execute_failure(code: u32) -> &'static str { #[cfg(all(test, target_os = "windows"))] mod printer_buffer_tests { use super::{ - describe_shell_execute_failure, ensure_printer_exists, windows_default_printer, - windows_printer_names, + describe_shell_execute_failure, ensure_printer_exists, printto_params, + windows_default_printer, windows_printer_names, }; use windows_sys::Win32::Graphics::Printing::{PRINTER_INFO_2W, PRINTER_INFO_4W}; + /// The `printto` parameters are whitespace-tokenised by registered PDF + /// handlers (Acrobat: `/t "%1" "%2" "%3" "%4"`), so the printer name must + /// arrive quoted with the empty driver/port slots padded — a bare + /// multi-word name used to split into printer/driver/port garbage. + #[test] + fn printto_params_quote_the_printer_and_pad_empty_slots() { + assert_eq!( + printto_params("Microsoft Print to PDF"), + "\"Microsoft Print to PDF\" \"\" \"\"" + ); + assert_eq!(printto_params("LaserJet"), "\"LaserJet\" \"\" \"\""); + } + #[test] fn printer_names_stay_within_the_returned_buffer() { assert!( @@ -682,8 +720,15 @@ fn dispatch_to_printer_opts(path: &std::path::Path, opts: &PrintOptions) -> Resu if let Some(name) = &target { ensure_printer_exists(name)?; } + // One construction point serves both the resolved default and a + // named selection (`target` covers the two); the name is quoted via + // `printto_params` so handlers cannot tokenize it on spaces. let (verb, params, label) = match &target { - Some(name) => (wide("printto"), Some(wide(name)), name.clone()), + Some(name) => ( + wide("printto"), + Some(wide(&printto_params(name))), + name.clone(), + ), None => (wide("print"), None, "default printer".to_string()), }; let params_ptr = params diff --git a/src/modules/draw/draw/helix.rs b/src/modules/draw/draw/helix.rs index 18547a70d..a1f2637c4 100644 --- a/src/modules/draw/draw/helix.rs +++ b/src/modules/draw/draw/helix.rs @@ -353,6 +353,21 @@ impl CadCommand for HelixCommand { } } + // The Final step's dynamic box is the helix height — a span along the + // plane normal — but its typed value would otherwise resolve into an + // in-plane point anchored at the last pick, and `on_point` commits that + // point's distance from the center as the height: a wrong value whenever + // the previous pick (e.g. the top radius) wasn't the center. This is the + // only entry route on web, where the canvas — not the command line — + // holds keyboard focus. `on_text_input` parses the same string the + // command-line path would (keywords and `parse_typed_length`), so both + // entry routes behave identically (#1082). The Base/TopRadius steps keep + // committing typed digits as points: their distance from the center IS + // the radius. + fn dyn_commit_as_text(&self) -> bool { + self.step == Step::Final + } + fn on_text_input(&mut self, text: &str) -> Option { let value = text.trim(); let keyword = value.to_ascii_uppercase(); diff --git a/src/modules/insert/solid3d_cmds.rs b/src/modules/insert/solid3d_cmds.rs index 5b3d76277..387260738 100644 --- a/src/modules/insert/solid3d_cmds.rs +++ b/src/modules/insert/solid3d_cmds.rs @@ -1862,6 +1862,21 @@ impl CadCommand for SweepCommand { matches!(self.step, SweepStep::PickPath | SweepStep::ReferenceLength | SweepStep::NewLength) } + // The reference-scale prompts read a typed length, but they also accept + // keyword letters, so without this a value committed from the dynamic + // box (the only route on web, where the canvas — not the command line — + // holds keyboard focus) resolves into an in-plane point instead: at + // `ReferenceLength` the typed length is dropped and stored as a stray + // first reference pick, at `NewLength` the scale is computed from the + // distance of that arbitrary point to the length anchor. `on_text_input` + // parses the same strings the command-line path would (keywords and + // `parse_typed_length`), so both entry routes behave identically (#1082). + // PickPath stays a point step: its letters are option keywords and its + // picks are path entities. + fn dyn_commit_as_text(&self) -> bool { + matches!(self.step, SweepStep::ReferenceLength | SweepStep::NewLength) + } + fn on_text_input(&mut self, text: &str) -> Option { let value = text.trim(); if value.is_empty() { diff --git a/src/modules/model/cylinder_cmd.rs b/src/modules/model/cylinder_cmd.rs index dad043283..f275bcbb2 100644 --- a/src/modules/model/cylinder_cmd.rs +++ b/src/modules/model/cylinder_cmd.rs @@ -779,6 +779,18 @@ impl CadCommand for CylinderCommand { _ => None, } } + + // The Height step's dynamic box is a span along the base-plane normal, + // not a point in the drawing plane. Without this, a typed value committed + // from the box (the only route on web, where the canvas — not the command + // line — holds keyboard focus) is resolved into an in-plane point whose + // height projects to ~0 and `commit_height` silently re-prompts. + // `on_text_input` parses the same string the command-line path would + // (keywords and `parse_typed_length`), so both entry routes behave + // identically (#1082). + fn dyn_commit_as_text(&self) -> bool { + matches!(self.step, Step::Height) + } } fn push_break(points: &mut Vec<[f32; 3]>) { diff --git a/src/modules/model/primitive_cmd.rs b/src/modules/model/primitive_cmd.rs index bc586caca..ffe7a8e6c 100644 --- a/src/modules/model/primitive_cmd.rs +++ b/src/modules/model/primitive_cmd.rs @@ -3405,6 +3405,33 @@ impl CadCommand for PrimitiveCommand { } self.height_step.then(|| self.cursor_height(cursor)) } + + // The height prompts' dynamic boxes are spans along the working-plane + // normal, not points in the drawing plane. These steps accept keyword + // letters (2Point / Axis endpoint / Top radius), so without this a typed + // value committed from the box (the only route on web, where the canvas — + // not the command line — holds keyboard focus) is resolved into an + // in-plane point whose height projects to ~0 and the commit is silently + // dropped with a re-prompt. `on_text_input` parses the same string the + // command-line path would (keywords and `parse_typed_length`), so both + // entry routes behave identically (#1082). BOX and WEDGE share + // `BoxStep::Height`; the 2Point/AxisEndpoint sub-steps keep committing + // as points, and SPHERE/TORUS radius steps type digits as in-plane + // distances by design. + fn dyn_commit_as_text(&self) -> bool { + match self.shape { + Shape::Cone => matches!( + self.cone_step, + ConeStep::Height | ConeStep::HeightAfterTopRadius + ), + Shape::Pyramid => matches!( + self.pyramid_step, + PyramidStep::Height | PyramidStep::HeightAfterTopRadius + ), + Shape::Box | Shape::Wedge => self.box_step == BoxStep::Height, + _ => false, + } + } } // ── Footprint preview ─────────────────────────────────────────────────────── diff --git a/src/scene/model/mesh_model.rs b/src/scene/model/mesh_model.rs index e3d211081..13624f742 100644 --- a/src/scene/model/mesh_model.rs +++ b/src/scene/model/mesh_model.rs @@ -10,37 +10,96 @@ use super::wire_model::WireModel; /// of the mesh converters' `SNAP_TABLE_MAX_VERTICES`: a mesh dense enough to /// exceed this has edges far too close together to pick apart at snap /// aperture size, and duplicating them into the wire set would cost more -/// memory than the whole rest of the interaction set. +/// memory than the whole rest of the interaction set. The builder budgets +/// SEGMENTS against this (each costs two endpoints plus a NaN separator) and +/// decimates rather than dropping when it is exceeded. pub const SNAP_EDGE_WIRE_MAX_POINTS: usize = 100_000; +/// Segment budget implied by [`SNAP_EDGE_WIRE_MAX_POINTS`]: S segments cost +/// `3·S − 1` wire points (two endpoints each plus a NaN separator between +/// neighbours), so the point cap admits at most `(cap + 1) / 3` segments. +const MAX_EDGE_SEGMENTS: usize = (SNAP_EDGE_WIRE_MAX_POINTS + 1) / 3; + +/// Halve `kept` in place, retaining every second segment (indices 0, 2, …). +fn halve_kept_segments(kept: &mut Vec<([f64; 3], [f64; 3])>) { + let mut write = 0; + for read in (0..kept.len()).step_by(2) { + kept[write] = kept[read]; + write += 1; + } + kept.truncate(write); +} + /// Build the interaction-only edge wire shared by every volume entity /// (3D solids, regions, bodies, surfaces, plain meshes). /// /// `pairs` yields one `(a, b)` world-space segment per edge. The wire carries /// the segments as NaN-separated point pairs (so `points` never chains two -/// unrelated edges together) plus the same pairs in `key_vertices` (so -/// Endpoint/Midpoint snap sees every true edge end / edge middle, not the -/// ends of an artificial chain). It is `display_visible: false` / -/// `plot_visible: false`: the mesh pipeline already draws these edges, the -/// wire exists so the SNAPPER and the proximity pick see them exactly like -/// any 2-D entity's geometry (#1030). +/// unrelated edges together) plus the same NaN-separated pairs in +/// `key_vertices` (so Endpoint/Midpoint snap sees every true edge end / edge +/// middle, not the ends of an artificial chain between unrelated edges — +/// Midpoint consumers treat consecutive `key_vertices` as one segment). It is +/// `display_visible: false` / `plot_visible: false`: the mesh pipeline +/// already draws these edges, the wire exists so the SNAPPER and the +/// proximity pick see them exactly like any 2-D entity's geometry (#1030). /// -/// Returns `None` when the edge count exceeds [`SNAP_EDGE_WIRE_MAX_POINTS`]. +/// Past [`MAX_EDGE_SEGMENTS`] the wire is not dropped but DECIMATED: the +/// segment pairs are uniformly subsampled (every Nth, N a power of two, with +/// the first and last segments always retained) down to the budget. Dropping +/// the wire outright used to cost dense meshes ALL endpoint snap; decimation +/// keeps partial coverage at the same bounded memory. The tradeoff: mid-wire +/// edges thin out (their Midpoint/Nearest coverage becomes sparser) while +/// both ends of the mesh stay snap-reachable. +/// +/// Returns `None` only when `pairs` yields no segments at all. pub fn edge_snap_wire( name: String, color: [f32; 4], selected: bool, pairs: impl Iterator, ) -> Option { - let mut points: Vec<[f32; 3]> = Vec::new(); - let mut points_low: Vec<[f32; 3]> = Vec::new(); - let mut key_vertices: Vec<[f64; 3]> = Vec::new(); + // ── Segment selection with bounded memory ───────────────────────────── + // Stream the pairs and keep at most MAX_EDGE_SEGMENTS of them: whenever + // the kept buffer fills, halve it and double the stride. That is uniform + // "every Nth" subsampling (power-of-two N) without ever buffering more + // than the budget — a runaway mesh costs O(cap) here, not O(edges), and + // its (already resident) tessellation is never duplicated. + let mut kept: Vec<([f64; 3], [f64; 3])> = Vec::new(); + let mut stride = 1usize; + let mut total = 0usize; + let mut last_pair: Option<([f64; 3], [f64; 3])> = None; + for pair in pairs { + if total % stride == 0 { + if kept.len() >= MAX_EDGE_SEGMENTS { + halve_kept_segments(&mut kept); + stride *= 2; + } + kept.push(pair); + } + last_pair = Some(pair); + total += 1; + } + // Force-retain the final segment so decimation never removes the far end + // of the mesh: `(total - 1) % stride != 0` proves it is not already kept + // (every kept entry sits at an index that is a multiple of `stride`). + if let Some(last) = last_pair { + if (total - 1) % stride != 0 { + if kept.len() >= MAX_EDGE_SEGMENTS { + halve_kept_segments(&mut kept); + } + kept.push(last); + } + } + + let mut points: Vec<[f32; 3]> = Vec::with_capacity(kept.len().saturating_mul(3)); + let mut points_low: Vec<[f32; 3]> = Vec::with_capacity(kept.len().saturating_mul(3)); + let mut key_vertices: Vec<[f64; 3]> = Vec::with_capacity(kept.len().saturating_mul(3)); let mut aabb = WireModel::UNBOUNDED_AABB; - let mut extend = |p: [f64; 3]| { + let extend = |p: [f64; 3], aabb: &mut [f32; 4]| { let (x, y) = (p[0] as f32, p[1] as f32); if x.is_finite() && y.is_finite() { - if aabb == WireModel::UNBOUNDED_AABB { - aabb = [x, y, x, y]; + if *aabb == WireModel::UNBOUNDED_AABB { + *aabb = [x, y, x, y]; } else { aabb[0] = aabb[0].min(x); aabb[1] = aabb[1].min(y); @@ -49,16 +108,14 @@ pub fn edge_snap_wire( } } }; - for (a, b) in pairs { - if points.len() + 2 > SNAP_EDGE_WIRE_MAX_POINTS { - return None; - } - if !points.is_empty() { + for (index, (a, b)) in kept.into_iter().enumerate() { + if index > 0 { points.push([f32::NAN; 3]); points_low.push([0.0; 3]); - if points.len() + 2 > SNAP_EDGE_WIRE_MAX_POINTS { - return None; - } + // Mirrors `points`: without this separator Midpoint would pair + // the last vertex of one edge with the first vertex of the next + // and fabricate a floating midpoint between unrelated edges. + key_vertices.push([f64::NAN; 3]); } for p in [a, b] { let (hx, lx) = WireModel::split_ds(p[0]); @@ -67,7 +124,7 @@ pub fn edge_snap_wire( points.push([hx, hy, hz]); points_low.push([lx, ly, lz]); key_vertices.push(p); - extend(p); + extend(p, &mut aabb); } } if points.is_empty() { diff --git a/src/scene/model/solid_model.rs b/src/scene/model/solid_model.rs index 1dd9de497..7e97e43f9 100644 --- a/src/scene/model/solid_model.rs +++ b/src/scene/model/solid_model.rs @@ -273,7 +273,8 @@ pub fn edge_wires(body: &Body) -> Vec { /// (`Solid3D::wires` and friends — pre-computed B-rep edge chains, also set /// on the temporary live-section entity). Each chain contributes its /// consecutive point pairs as snap segments (#1030). Returns `None` when the -/// chains carry no usable segments or exceed the shared snap-wire point cap. +/// chains carry no usable segments; past the shared snap-wire point cap +/// `edge_snap_wire` decimates instead of dropping. pub fn entity_edge_snap_wire( chains: &[acadrust::entities::Wire], name: String, diff --git a/src/snap.rs b/src/snap.rs index 51d94868b..ba2999f07 100644 --- a/src/snap.rs +++ b/src/snap.rs @@ -2491,37 +2491,64 @@ fn wire_plane(wire: &WireModel) -> Option { Some(WirePlane { origin, axis_x, axis_y, normal: axis_x.cross(axis_y).normalize() }) } -/// The f64 endpoints behind a `TangentGeom::Line`. The geom itself stores -/// f32, which quantises survey-scale coordinates by centimetres; the wire's -/// tessellation points (a double-single pair) hold the same vertices in full -/// precision, so when a consecutive point pair matches the geom, use it. -fn line_geom_endpoints_f64(wire: &WireModel, p1: [f32; 3], p2: [f32; 3]) -> (DVec3, DVec3) { - if wire.points.len() == 2 { - return (wp_f64(wire, 0), wp_f64(wire, 1)); - } - let f32_pair = (Vec3::from_array(p1).as_dvec3(), Vec3::from_array(p2).as_dvec3()); - // The f64 pair matches the geom's own segment when it sits within the - // f32 cast's rounding error (≤ 0.5 ulp of the coordinate magnitude). - // Take the CLOSEST pair, not the first within tolerance: adjacent - // polyline vertices can sit closer together than the quantisation at - // survey scale, and a first-match scan would then grab a neighbour's - // segment. - let mag = p1.iter().chain(p2.iter()).fold(0.0f32, |m, c| m.max(c.abs())); - let tol = (mag * f32::EPSILON) as f64 + 1e-9; - let mut best: Option<(f64, DVec3, DVec3)> = None; +/// Lookup key for [`line_endpoint_map`]: the f32 bit patterns of the six +/// endpoint coordinates with the two lowest mantissa bits cleared (a ~4-ulp +/// grid). The geom's stored f32 and the f32 cast of the wire's double-single +/// reconstruction sit within an ulp of each other, so both quantise into the +/// same bucket; distinct segments only collide when they share an endpoint +/// to well below snap distinguishability. +type LineGeomKey = ([u32; 3], [u32; 3]); + +fn line_geom_key(p1: [f32; 3], p2: [f32; 3]) -> LineGeomKey { + const MASK: u32 = !0b11; + let quantise = + |p: [f32; 3]| [p[0].to_bits() & MASK, p[1].to_bits() & MASK, p[2].to_bits() & MASK]; + (quantise(p1), quantise(p2)) +} + +/// One-shot map from a `TangentGeom::Line`'s quantised f32 endpoint pair to +/// the wire's full-precision f64 pair behind it, so every Line geom of the +/// wire resolves with an O(1) lookup instead of rescanning the whole +/// tessellation per geom (which made `curves_in_frame` O(points × geoms) for +/// every wire pair on every snap). First-wins on quantisation collisions: +/// adjacent vertices closer than the grid are indistinguishable at snap +/// scale anyway. +fn line_endpoint_map(wire: &WireModel) -> HashMap { + let mut map = + HashMap::with_capacity_and_hasher(wire.points.len() / 2 + 1, Default::default()); for index in 0..wire.points.len().saturating_sub(1) { let a = wp_f64(wire, index); let b = wp_f64(wire, index + 1); - // NaN separators split runs; either side being NaN never matches. + // NaN separators split runs; neither side of them can match a geom. if !a.is_finite() || !b.is_finite() { continue; } - let miss = (a - f32_pair.0).length_squared() + (b - f32_pair.1).length_squared(); - if miss <= 4.0 * tol * tol && best.map_or(true, |(m, _, _)| miss < m) { - best = Some((miss, a, b)); - } + let key = line_geom_key(a.as_vec3().to_array(), b.as_vec3().to_array()); + map.entry(key).or_insert((a, b)); } - best.map(|(_, a, b)| (a, b)).unwrap_or(f32_pair) + map +} + +/// The f64 endpoints behind a `TangentGeom::Line`. The geom itself stores +/// f32, which quantises survey-scale coordinates by centimetres; the wire's +/// tessellation points (a double-single pair) hold the same vertices in full +/// precision, so resolve the geom through [`line_endpoint_map`] (built once +/// per wire by the caller). A geom that misses the map falls back to its own +/// f32 pair widened — exactly the no-match behaviour of the scan this +/// replaced. +fn line_geom_endpoints_f64( + wire: &WireModel, + p1: [f32; 3], + p2: [f32; 3], + endpoints: &HashMap, +) -> (DVec3, DVec3) { + if wire.points.len() == 2 { + return (wp_f64(wire, 0), wp_f64(wire, 1)); + } + endpoints + .get(&line_geom_key(p1, p2)) + .copied() + .unwrap_or((Vec3::from_array(p1).as_dvec3(), Vec3::from_array(p2).as_dvec3())) } /// Decompose a wire into its analytic curves inside `frame`. @@ -2549,10 +2576,14 @@ fn curves_in_frame(wire: &WireModel, frame: &WirePlane, tol: f64) -> Option> = None; for geom in &wire.tangent_geoms { let curve = match geom { TangentGeom::Line { p1, p2 } => { - let (p1, p2) = line_geom_endpoints_f64(wire, *p1, *p2); + let endpoints = line_endpoints.get_or_insert_with(|| line_endpoint_map(wire)); + let (p1, p2) = line_geom_endpoints_f64(wire, *p1, *p2, endpoints); (frame.contains(p1, tol) && frame.contains(p2, tol)).then(|| { let (start, end) = (frame.to_2d(p1), frame.to_2d(p2)); // A crossing can sit exactly on a segment endpoint (the @@ -2668,6 +2699,9 @@ fn exact_curve_intersections(wire_a: &WireModel, wire_b: &WireModel) -> Option = Vec::new(); @@ -4259,6 +4293,11 @@ mod ext_tests { ]; let wire = entity_edge_snap_wire(&chains, "42".to_owned(), [0.8, 0.8, 0.85, 1.0]).expect("wire"); + // Segment pairs stay NaN-separated across CHAIN junctions too (3 + 3 + // + 1 = 7 pairs → 6 separators), so Midpoint never pairs the last + // vertex of one chain with the first vertex of the next. + let separators = wire.key_vertices.iter().filter(|p| !p[0].is_finite()).count(); + assert_eq!(separators, 6); let wires = [wire]; let mut snapper = Snapper::default(); snapper.snap_enabled = true; @@ -4283,24 +4322,107 @@ mod ext_tests { ); } - #[test] - fn edge_snap_wire_caps_runaway_meshes() { - use crate::scene::model::mesh_model::{edge_snap_wire, SNAP_EDGE_WIRE_MAX_POINTS}; - let segment = ([0.0, 0.0, 0.0], [1.0, 0.0, 0.0]); - // Each pair costs up to 3 points (2 endpoints + a NaN separator), so - // a third of the cap fits and the full cap must not. - let pairs = std::iter::repeat(segment).take(SNAP_EDGE_WIRE_MAX_POINTS / 3); - assert!(edge_snap_wire("ok".into(), [1.0; 4], false, pairs).is_some()); - let pairs = std::iter::repeat(segment).take(SNAP_EDGE_WIRE_MAX_POINTS); + /// Inverse of [`snap_one`]: the same camera scaffolding, asserting that + /// nothing snap-enabled lands anywhere near `near`. + fn snap_none(snapper: &mut Snapper, wires: &[WireModel], near: DVec3) { + // Zoomed out: 1 world unit ≈ 5 px, same as `snap_one`. + let view_rot = Mat4::from_scale(Vec3::splat(0.01)); + let bounds = Rectangle { + x: 0.0, + y: 0.0, + width: 1000.0, + height: 1000.0, + }; + let eye = DVec3::new(near.x, near.y, 500.0); + let cursor_screen = world_to_screen(near, view_rot, eye, bounds); + let res = snapper.snap( + near, cursor_screen, wires, view_rot, eye, bounds, Vec3::ZERO, + (Vec3::X, Vec3::Y, Vec3::Z), None, + ); assert!( - edge_snap_wire("too big".into(), [1.0; 4], false, pairs).is_none(), - "meshes past the point cap must ship no snap wire" + res.is_none(), + "nothing must snap near {near:?}, got {:?}", + res.map(|r| (r.snap_type, r.world)) ); } + #[test] + fn edge_snap_wire_disjoint_edges_have_no_phantom_midpoint() { + use crate::scene::model::mesh_model::edge_snap_wire; + // Two DISJOINT triangles 10 units apart, fed exactly like + // `MeshLodSet::snap_edge_wire` feeds `edge_snap_wire` (three segment + // pairs per triangle). + let triangle = |ox: f64| { + let a = [ox, 0.0, 0.0]; + let b = [ox + 1.0, 0.0, 0.0]; + let c = [ox + 0.5, 1.0, 0.0]; + [(a, b), (b, c), (c, a)] + }; + let pairs = triangle(0.0).into_iter().chain(triangle(10.0)); + let wire = edge_snap_wire("t".into(), [1.0; 4], false, pairs).expect("wire"); + // `key_vertices` must separate the segment pairs with NaN exactly + // like `points` does. Without the separators, every Midpoint consumer + // pairs the LAST vertex of one edge with the FIRST vertex of the next + // — here triangle 0's (0,0,0) with triangle 10's (10,0,0) — and + // fabricates a floating midpoint between unrelated edges. + let separators = wire.key_vertices.iter().filter(|p| !p[0].is_finite()).count(); + assert_eq!(separators, 5, "one NaN separator between each of the 6 pairs"); + + let wires = [wire]; + let mut snapper = Snapper::default(); + snapper.snap_enabled = true; + snapper.enabled = [SnapType::Midpoint].into_iter().collect(); + // The junction's phantom midpoint would sit at (5, 0, 0); the nearest + // true midpoint is ~4.3 world units away, far outside the aperture. + snap_none(&mut snapper, &wires, DVec3::new(5.0, 0.0, 0.0)); + // Sanity: a true edge midpoint still snaps through key_vertices. + snap_one( + &mut snapper, + &wires, + DVec3::new(0.6, 0.05, 0.0), + SnapType::Midpoint, + DVec3::new(0.5, 0.0, 0.0), + ); + } + + #[test] + fn edge_snap_wire_caps_runaway_meshes() { + use crate::scene::model::mesh_model::{edge_snap_wire, SNAP_EDGE_WIRE_MAX_POINTS}; + let segment = |i: usize| ([i as f64, 0.0, 0.0], [(i + 1) as f64, 0.0, 0.0]); + let segment_count = |wire: &WireModel| { + // One NaN separator between neighbouring pairs → runs − 1. + wire.points.iter().filter(|p| !p[0].is_finite()).count() + 1 + }; + // Each pair costs up to 3 points (2 endpoints + a NaN separator), so + // a third of the point cap in SEGMENTS fits without decimation. + let fitting = SNAP_EDGE_WIRE_MAX_POINTS / 3; + let wire = edge_snap_wire("ok".into(), [1.0; 4], false, (0..fitting).map(segment)) + .expect("wire at the segment budget"); + assert_eq!(segment_count(&wire), fitting); + assert!(wire.points.len() <= SNAP_EDGE_WIRE_MAX_POINTS); + + // Past the budget the wire is DECIMATED, not dropped: the segment + // pairs are uniformly subsampled down to the budget so dense meshes + // keep (partial) endpoint coverage instead of losing their edge wire + // — and with it all endpoint snap — entirely. + let total = SNAP_EDGE_WIRE_MAX_POINTS; // 3x the segment budget + let wire = edge_snap_wire("decimated".into(), [1.0; 4], false, (0..total).map(segment)) + .expect("over-cap mesh must still ship a decimated snap wire"); + let kept = segment_count(&wire); + assert!(kept < total, "over-cap wire must be decimated, kept {kept}"); + assert!( + wire.points.len() <= SNAP_EDGE_WIRE_MAX_POINTS, + "decimated wire must honour the point cap ({} points)", + wire.points.len() + ); + // The first and last source segments survive decimation, so both ends + // of the mesh stay endpoint-snappable. + assert!(wire.key_vertices.contains(&[0.0, 0.0, 0.0])); + assert!(wire.key_vertices.contains(&[total as f64, 0.0, 0.0])); + } + #[test] fn scene_solid_with_mesh_cache_snaps_endpoint_midpoint_nearest() { - use crate::scene::model::mesh_model::MeshLodSet; use crate::scene::Scene; use acadrust::entities::Solid3D; diff --git a/src/ui/overlay.rs b/src/ui/overlay.rs index e35865933..ee4711108 100644 --- a/src/ui/overlay.rs +++ b/src/ui/overlay.rs @@ -567,12 +567,6 @@ pub struct GridCanvasState { pub cache: canvas::Cache, } -/// Compute the adaptive grid step size (world units) from camera zoom. -/// -/// Returns the smallest power-of-5 multiple of 1.0 that places grid lines at -/// least `MIN_GRID_PX` pixels apart at the camera pivot. Both orthographic and -/// perspective cameras have the same vertical scale there. Camera rotation is -/// intentionally absent so orbiting cannot rescale the visible grid or snap. /// Perpendicular screen distance (px) from `p` to the infinite line through /// `a` and `b`. Degenerate (`a`≈`b`) falls back to the point distance. Used /// to tell whether a tracking vector really passes through the locked cursor @@ -632,6 +626,12 @@ fn clip_seg(p0: Point, p1: Point, bounds: iced::Rectangle) -> Option<(Point, Poi )) } +/// Compute the adaptive grid step size (world units) from camera zoom. +/// +/// Returns the smallest power-of-5 multiple of 1.0 that places grid lines at +/// least `MIN_GRID_PX` pixels apart at the camera pivot. Both orthographic and +/// perspective cameras have the same vertical scale there. Camera rotation is +/// intentionally absent so orbiting cannot rescale the visible grid or snap. pub fn compute_grid_step(distance: f32, fov_y: f32, bounds: iced::Rectangle) -> f32 { let half_height = distance * (fov_y * 0.5).tan(); if !half_height.is_finite() || half_height <= 1e-9 || bounds.height <= 0.0 { diff --git a/tests/pdf_size_repro.rs b/tests/pdf_size_repro.rs index a32652488..aee2c9a5e 100644 --- a/tests/pdf_size_repro.rs +++ b/tests/pdf_size_repro.rs @@ -11,6 +11,7 @@ use OpenCADStudio::io::pdf_export::{ render_pdf_bytes, PdfPageInput, PdfPlotOptions, PlotContent, PlotWire, }; +use OpenCADStudio::io::plot_style::{LW_TABLE, PlotStyleTable}; use OpenCADStudio::scene::model::hatch_model::{HatchModel, HatchPattern, PatFamily}; use OpenCADStudio::scene::pipeline::text_gpu::push_glyph_vertices; use OpenCADStudio::scene::text::glyph_quads::layout_glyph_quads; @@ -153,6 +154,82 @@ fn stroke_text_page_size_is_bounded() { ); } +/// Heavy-pen (CTB lineweight override) leg: an absolute pen is arbitrary pt and +/// can exceed the glyph Form XObject /BBox's 1.5-unit SDF spread margin +/// (2.5 mm text + 1.0 mm pen → 1.79-unit half-width), squaring round caps off +/// at the tile edge. Heavy-pen entities must bypass the Forms and stroke +/// directly in page space, while the default-pen export of the same page keeps +/// its Form replay (and size class). +#[test] +fn heavy_pen_text_bypasses_forms_and_light_pen_sizes_hold() { + let heavy_ctb = { + let mut ctb = PlotStyleTable::identity("heavy-pen.ctb"); + let one_mm = LW_TABLE.iter().position(|&mm| mm == 1.0).unwrap() as u8; + let entry = &mut ctb.aci_entries[1]; + entry.lineweight = one_mm; + entry.color = Some([0, 0, 0]); + ctb + }; + // The same 200-entity stroke-text page as the bounded leg above, with the + // wires on ACI 1 so the CTB's 1.0 mm pen applies. + let wires: Vec = (0..200) + .map(|i| { + let x = 10.0 + (i % 5) as f64 * 40.0; + let y = 10.0 + (i / 5) as f64 * 7.0; + let mut wire = text_wire( + "txt", + "DIMENSION TEXT 12345.6789 - TEXT ANNOTATION LINE", + [x, y, 0.0], + 2.5, + ); + wire.wire.aci = 1; + wire + }) + .collect(); + let glyph_quads: usize = wires.iter().map(|w| w.wire.text_verts.len() / 6).sum(); + let page = a4_page(wires, Vec::new()); + + let heavy = + render_pdf_bytes(std::slice::from_ref(&page), Some(&heavy_ctb)).expect("heavy export"); + println!( + "heavy-pen (1.0 mm CTB) stroke-text 200-entity A4 page: {} bytes for {} glyph quads", + heavy.len(), + glyph_quads + ); + assert!(heavy.starts_with(b"%PDF"), "not a PDF"); + assert!( + heavy.len() > 100_000, + "heavy-pen text page suspiciously small: {}", + heavy.len() + ); + // The bypass engaged: zero glyph Form `/Do` replays in the heavy render. + // (Page content streams are uncompressed, so the operator is greppable.) + let do_count = |bytes: &[u8]| bytes.windows(3).filter(|w| w == b" Do").count(); + assert_eq!( + do_count(&heavy), + 0, + "heavy-pen glyphs still replay clipped Form XObjects" + ); + + // Light-pen regression fence: the same page without a plot style keeps + // every glyph on the Form path (one `/Do` per quad) and its size class. + let light = render_pdf_bytes(std::slice::from_ref(&page), None).expect("light export"); + println!( + "same page without CTB: {} bytes ({} Form /Do replays)", + light.len(), + do_count(&light) + ); + assert!( + do_count(&light) >= glyph_quads, + "Form glyph replay vanished from the light path" + ); + assert!( + light.len() < 2_000_000, + "light-pen text page regressed to {} bytes", + light.len() + ); +} + /// Pattern hatch + dense polyline comparison leg: identifies which content /// type dominates a mixed page. ANSI31-style 45° hatch over an A4-sized rect. #[test]