From 9b112e3ac03e1e302b99feb266b7d55494a11357 Mon Sep 17 00:00:00 2001 From: Hakan Seven Date: Wed, 9 Sep 2026 13:33:14 +0300 Subject: [PATCH] fix: finalize reviewed drawing and interaction paths Reset stale geometry when automation replaces a drawing and resolve material paths from its directory. Cover precise indexed snapping, large-coordinate line intersections, leader ordering, invalid image buffers and browser clipboard fallback. Preserve Options dialog resizing. Validation: cargo test --workspace --all-features (866 passed); wasm32 application check; Node clipboard regression check; Python binding tests (3 passed). --- scripts/test_web_clipboard.cjs | 42 ++++++++++++++++++++++ src/app/automation.rs | 23 +++++++++--- src/scene/pipeline/mesh_gpu.rs | 9 +++++ src/scene/selection.rs | 8 +++++ src/snap.rs | 65 ++++++++++++++++++++++++++++++++-- src/ui/window/options.rs | 5 +-- 6 files changed, 143 insertions(+), 9 deletions(-) create mode 100644 scripts/test_web_clipboard.cjs diff --git a/scripts/test_web_clipboard.cjs b/scripts/test_web_clipboard.cjs new file mode 100644 index 000000000..44575b718 --- /dev/null +++ b/scripts/test_web_clipboard.cjs @@ -0,0 +1,42 @@ +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +(async () => { + const source = fs.readFileSync(path.join(__dirname, '../web/clipboard.js'), 'utf8'); + const { copyHistory } = await import(`data:text/javascript;base64,${Buffer.from(source).toString('base64')}`); + const text = 'LINE\n0,0\n10,10\n\n'; + let copied; + Object.defineProperty(globalThis, 'navigator', { configurable: true, value: { + clipboard: { writeText: async value => { copied = value; } }, + }}); + assert.equal(await copyHistory(text, 'Copy manually', 'Close'), true); + assert.equal(copied, text); + const dialogs = []; + globalThis.document = { + body: { append: dialog => dialogs.push(dialog) }, + createElement: tag => ({ + tag, style: {}, events: {}, + setAttribute(key, value) { this[key] = value; }, + append(...children) { this.children = children; }, + addEventListener(event, handler) { this.events[event] = handler; }, + showModal() { this.open = true; }, + focus() { this.focused = true; }, + select() { this.selected = true; }, + close() { this.events.close(); }, + remove() { this.removed = true; }, + }), + }; + for (const clipboard of [{ writeText: async () => { throw Error('Denied'); } }, undefined]) { + navigator.clipboard = clipboard; + assert.equal(await copyHistory(text, 'Copy manually', 'Close'), false); + const dialog = dialogs.at(-1); + const [label, field, close] = dialog.children; + assert.equal(label.textContent, 'Copy manually'); + assert.equal(field.value, text); + assert.ok(field.readOnly && field.focused && field.selected && dialog.open); + close.onclick(); + assert.ok(dialog.removed); + } + console.log('Browser clipboard: success, denied and unavailable paths passed.'); +})().catch(error => { console.error(error); process.exitCode = 1; }); diff --git a/src/app/automation.rs b/src/app/automation.rs index 0f2a380ef..c02cc80a9 100644 --- a/src/app/automation.rs +++ b/src/app/automation.rs @@ -300,13 +300,12 @@ impl OpenCADStudio { match req["op"].as_str().unwrap_or("") { "new" => { let i = self.active_tab; - self.tabs[i].scene.document = acadrust::CadDocument::new(); - self.tabs[i].scene.deselect_all(); + self.tabs[i].scene.clear(); + self.tabs[i].scene.material_base_dir = None; self.tabs[i].current_path = None; // The headless session starts on the welcome (Start) tab, which // blocks drawing commands; turn it into a real drawing. self.tabs[i].is_start = false; - self.tabs[i].scene.bump_geometry(); self.entity_summary() } #[cfg(not(target_arch = "wasm32"))] @@ -322,15 +321,16 @@ impl OpenCADStudio { match crate::io::load_bytes_finalized(&path_buf, bytes) { Ok((doc, dropped)) => { let i = self.active_tab; + self.tabs[i].scene.clear(); self.tabs[i].scene.document = doc; - self.tabs[i].scene.deselect_all(); + self.tabs[i].scene.material_base_dir = path_buf.parent().map(PathBuf::from); crate::app::style_ops::ensure_standard_styles( &mut self.tabs[i].scene.document, ); self.tabs[i].adopt_active_ucs_from_header(); self.tabs[i].current_path = Some(path_buf); self.tabs[i].is_start = false; - self.tabs[i].scene.bump_geometry(); + self.tabs[i].scene.rebuild_derived_caches(); let mut summary = self.entity_summary(); if dropped > 0 { if let Some(obj) = summary.as_object_mut() { @@ -1401,6 +1401,10 @@ mod tests { #[test] fn open_finalizes_and_purges_like_the_ui_open_path() { let mut app = OpenCADStudio::new_for_test(); + let stale = acadrust::Handle::from(9999); + app.tabs[app.active_tab].scene.solid_models.insert( + stale, cadkernel::brep::make::cuboid([0.0; 3], [1.0; 3]).unwrap(), + ); let path = std::env::temp_dir().join(format!( "ocs_automation_finalize_test_{}.dxf", std::process::id() @@ -1427,11 +1431,20 @@ mod tests { assert_eq!(result["purged"], 1, "the purge count must be reported, matching the UI open path's diagnostics"); let i = app.active_tab; + assert!(!app.tabs[i].scene.solid_models.contains_key(&stale)); + assert_eq!(app.tabs[i].scene.material_base_dir.as_deref(), path.parent()); assert!( app.tabs[i].scene.document.source_path.is_some(), "automation open must run the same finalization as a path-based open, which sets source_path (load_bytes alone never does)" ); + app.tabs[i].scene.solid_models.insert( + stale, cadkernel::brep::make::cuboid([0.0; 3], [1.0; 3]).unwrap(), + ); + assert_eq!(app.automation_op(r#"{"op":"new"}"#)["ok"], true); + assert!(app.tabs[i].scene.solid_models.is_empty()); + assert!(app.tabs[i].scene.material_base_dir.is_none()); + drop(app); let _ = std::fs::remove_file(&path); } diff --git a/src/scene/pipeline/mesh_gpu.rs b/src/scene/pipeline/mesh_gpu.rs index dcd586732..e5c0ee788 100644 --- a/src/scene/pipeline/mesh_gpu.rs +++ b/src/scene/pipeline/mesh_gpu.rs @@ -2492,4 +2492,13 @@ mod texture_limit_tests { let rgba = vec![0u8; 10]; assert!(downscale_rgba_to_limit(1000, 1000, &rgba, 512).is_none()); } + #[test] + fn malformed_material_images_are_rejected_even_below_the_texture_limit() { + for (width, height, pixels) in [(0, 1, vec![]), (1, 0, vec![]), (1, 1, vec![0; 3]), (1, 1, vec![0; 5])] { + assert!(!super::valid_rgba_size(width, height, &pixels)); + assert!(downscale_rgba_to_limit(width, height, &pixels, 16).is_none()); + } + assert!(downscale_rgba_to_limit(1, 1, &[0; 4], 0).is_none()); + } + } diff --git a/src/scene/selection.rs b/src/scene/selection.rs index ced148433..868595fe4 100644 --- a/src/scene/selection.rs +++ b/src/scene/selection.rs @@ -1181,6 +1181,14 @@ mod tests { expanded.contains(&first) && expanded.contains(&second), "both leaders on one annotation must come back, not just one", ); + for handle in [first, annotation, second] { + scene.deselect_all(); + scene.select_entity(handle, false); + let expected = scene.selected_handles_in_order(); + scene.deselect_all(); + scene.select_entities(&[handle]); + assert_eq!(scene.selected_handles_in_order(), expected); + } } #[test] diff --git a/src/snap.rs b/src/snap.rs index d59c9bbc6..b3021cc22 100644 --- a/src/snap.rs +++ b/src/snap.rs @@ -2408,8 +2408,11 @@ fn curve_in_frame(wire: &WireModel, frame: &WirePlane, tol: f64) -> Option { - let p1 = DVec3::new(p1[0] as f64, p1[1] as f64, p1[2] as f64); - let p2 = DVec3::new(p2[0] as f64, p2[1] as f64, p2[2] as f64); + let (p1, p2) = if wire.points.len() == 2 { + (wp_f64(wire, 0), wp_f64(wire, 1)) + } else { + (Vec3::from_array(*p1).as_dvec3(), Vec3::from_array(*p2).as_dvec3()) + }; (frame.contains(p1, tol) && frame.contains(p2, tol)).then(|| Curve::Line(KLine { start: frame.to_2d(p1), end: frame.to_2d(p2) })) } TangentGeom::Circle { center, radius } => { @@ -3382,4 +3385,62 @@ mod ext_tests { assert!(pts[0].y < 0.0, "expected the lower point (normal flip must change which physical arc this is), got {:?}", pts[0]); assert!((pts[0] - DVec3::new(3.75, -(63.0_f64).sqrt() / 4.0, 0.0)).length() < 1e-6); } + #[test] + fn indexed_intersection_uses_true_curves_even_when_chords_miss_the_aperture() { + use crate::scene::pick::interaction_index::InteractionIndex; + use std::sync::Arc; + let circle = |name: &str, x: f64, radius: f64| { + let points = (0..=48).map(|i| { + let angle = i as f64 * std::f64::consts::TAU / 48.0 + if x == 0.0 { 0.03 } else { 0.0 }; + [(x + radius * angle.cos()) as f32, (radius * angle.sin()) as f32, 0.0] + }).collect(); + let mut wire = WireModel::solid(name.to_owned(), points, [1.0; 4], false); + wire.aabb = [(x - radius) as f32, -radius as f32, (x + radius) as f32, radius as f32]; + wire.tangent_geoms = vec![TangentGeom::PlanarCircle { + center: [x, 0.0, 0.0], axis_x: [1.0, 0.0, 0.0], axis_y: [0.0, 1.0, 0.0], radius, + }]; + wire + }; + let wires = Arc::new(vec![circle("1", 0.0, 4.0), circle("2", 3.0, 5.0)]); + let index = InteractionIndex::build(&wires); + let candidates = index.query_xy(Arc::clone(&wires), [-0.0001, 3.9999, 0.0001, 4.0001]); + assert_eq!(candidates.len(), 2); + assert!(candidates.segments().unwrap().iter().all(|segment| segment.wire == 1)); + let point = DVec3::new(0.0, 4.0, 0.0); + let mut snapper = Snapper::default(); + snapper.snap_enabled = true; + snapper.enabled = [SnapType::Intersection].into_iter().collect(); + let result = snapper.snap( + point, Point::new(500.0, 500.0), &candidates, + Mat4::from_scale(Vec3::splat(100.0)), point, + Rectangle { x: 0.0, y: 0.0, width: 1000.0, height: 1000.0 }, + Vec3::ZERO, (Vec3::X, Vec3::Y, Vec3::Z), None, + ).expect("the true intersection remains available at high zoom"); + assert!((result.world - point).length() < 1e-9); + } + + #[test] + fn line_circle_intersection_retains_low_coordinate_bits() { + let origin = 1_000_000_000.0; + let a = [origin - 10.0, origin, 0.0]; + let b = [origin + 10.0, origin, 0.0]; + let points = [a, b].map(|point| point.map(|value| value as f32)); + let low = [a, b].into_iter().zip(points).map(|(point, high)| { + std::array::from_fn(|axis| (point[axis] - high[axis] as f64) as f32) + }).collect(); + let line = WireModel { + points: points.to_vec(), points_low: low, + tangent_geoms: vec![TangentGeom::Line { p1: points[0], p2: points[1] }], + ..Default::default() + }; + let circle = WireModel { + tangent_geoms: vec![TangentGeom::PlanarCircle { + center: [origin, origin, 0.0], axis_x: [1.0, 0.0, 0.0], axis_y: [0.0, 1.0, 0.0], radius: 3.0, + }], ..Default::default() + }; + let points = exact_curve_intersections(&line, &circle).unwrap(); + assert_eq!(points.len(), 2); + assert!(points.iter().all(|point| (point.x - origin).abs() == 3.0 && point.y == origin)); + } + } diff --git a/src/ui/window/options.rs b/src/ui/window/options.rs index 4d3484255..3acc5b8be 100644 --- a/src/ui/window/options.rs +++ b/src/ui/window/options.rs @@ -760,10 +760,11 @@ pub fn view_window<'a>( .width(sizing.width) .height(sizing.height); + let intrinsic = sizing.width == crate::ui::modal::ModalSizing::INTRINSIC.width; container(body) .style(container::rounded_box) .padding([16, 18]) - .width(sizing.width) - .height(sizing.height) + .width(if intrinsic { iced::Length::Fixed(540.0) } else { sizing.width }) + .height(if intrinsic { iced::Length::Fixed(560.0) } else { sizing.height }) .into() }