diff --git a/Cargo.lock b/Cargo.lock index 96ae7c765..76a3880ef 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -919,7 +919,7 @@ checksum = "fc652a48c352aef3ea3aed32080501cf3ef6ed5da78602a020c991775b0aff04" [[package]] name = "cadkernel" version = "0.1.0" -source = "git+https://github.com/HakanSeven12/cadkernel.git?rev=eb3cfe7#eb3cfe7b478230897bfb9094e7bc70aa4d00411b" +source = "git+https://github.com/HakanSeven12/cadkernel.git?rev=dcf0840#dcf0840d2e62afdde35cf38c0a3aa914ff305eed" dependencies = [ "acadrust", "cavalier_contours", @@ -930,7 +930,7 @@ dependencies = [ [[package]] name = "cadkernel-constraints" version = "0.1.0" -source = "git+https://github.com/HakanSeven12/cadkernel.git?rev=eb3cfe7#eb3cfe7b478230897bfb9094e7bc70aa4d00411b" +source = "git+https://github.com/HakanSeven12/cadkernel.git?rev=dcf0840#dcf0840d2e62afdde35cf38c0a3aa914ff305eed" dependencies = [ "nalgebra", ] diff --git a/Cargo.toml b/Cargo.toml index 8441fc21f..ed9e2120a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,8 +33,8 @@ rfd = "0.17" clap = { version = "4", features = ["derive", "string"] } env_logger = "0.11" acadrust = { git = "https://github.com/HakanSeven12/cadcodec.git", rev = "568a12c", features = ["serde"] } -cadkernel = { git = "https://github.com/HakanSeven12/cadkernel.git", rev = "eb3cfe7", features = ["acis", "offset"] } -cadkernel-constraints = { git = "https://github.com/HakanSeven12/cadkernel.git", rev = "eb3cfe7" } +cadkernel = { git = "https://github.com/HakanSeven12/cadkernel.git", rev = "dcf0840", features = ["acis", "offset"] } +cadkernel-constraints = { git = "https://github.com/HakanSeven12/cadkernel.git", rev = "dcf0840" } dwg-thumbnailer = { path = "crates/dwg-thumbnailer" } flate2 = "1" image = { version = "0.25", default-features = false, features = ["png", "jpeg", "bmp", "tiff"] } diff --git a/src/app/command_driver.rs b/src/app/command_driver.rs index 9c3d81cd1..168d90be6 100644 --- a/src/app/command_driver.rs +++ b/src/app/command_driver.rs @@ -3599,11 +3599,15 @@ impl OpenCADStudio { // snapshot — the create already pushed one, so the whole object // reverts as a unit. if let Some(old) = self.tabs[i].scene.document.get_entity_mut(handle) { - let old_handle = old.as_entity().handle(); - let layer = old.as_entity().layer().to_string(); + // The initial live commit assigns the handle, owning block, + // current layer and common display properties. Geometry + // refreshes must retain all of that identity; replacing only + // the handle and layer reset owner_handle to NULL and made + // the completed entity unavailable to scoped operations such + // as parametric constraints. + let common = old.common().clone(); let mut new = entity; - new.as_entity_mut().set_handle(old_handle); - new.as_entity_mut().set_layer(layer); + *new.common_mut() = common; *old = new; self.tabs[i] .scene @@ -8017,6 +8021,43 @@ mod command_replacement_tests { assert_ne!(allocated[0], handle); assert!(app.tabs[tab].scene.document.get_entity(handle).is_none()); } + + #[test] + fn live_geometry_updates_keep_document_identity_and_display_properties() { + let mut app = OpenCADStudio::new_for_test(); + let _ = app.automation_op(r#"{"op":"new"}"#); + let tab = app.active_tab; + let handle = app.tabs[tab] + .scene + .add_entity(acadrust::EntityType::Line(Line::from_points( + Vector3::ZERO, + Vector3::new(1.0, 0.0, 0.0), + ))); + let expected = { + let entity = app.tabs[tab].scene.document.get_entity_mut(handle).unwrap(); + entity.common_mut().layer = "LIVE".to_string(); + entity.common_mut().invisible = true; + entity.common_mut().linetype_scale = 2.5; + entity.common().clone() + }; + + let _ = app.apply_cmd_result(CmdResult::UpdateLiveEntity { + handle, + entity: acadrust::EntityType::Line(Line::from_points( + Vector3::ZERO, + Vector3::new(4.0, 0.0, 0.0), + )), + finish: false, + }); + + let updated = app.tabs[tab].scene.document.get_entity(handle).unwrap(); + assert_eq!(updated.common().handle, expected.handle); + assert_eq!(updated.common().owner_handle, expected.owner_handle); + assert_eq!(updated.common().layer, expected.layer); + assert_eq!(updated.common().invisible, expected.invisible); + assert_eq!(updated.common().linetype_scale, expected.linetype_scale); + assert!(matches!(updated, acadrust::EntityType::Line(line) if line.end.x == 4.0)); + } } #[cfg(test)] diff --git a/src/command.rs b/src/command.rs index d88fa1727..25d321e8f 100644 --- a/src/command.rs +++ b/src/command.rs @@ -1866,10 +1866,9 @@ pub enum CmdResult { /// as a single unit. The command stays active. Used by PLINE so the partial /// polyline is a real, snappable entity while later vertices are placed. CommitLiveEntity(EntityType), - /// Replace the geometry of the live entity `handle` in place — preserving - /// its layer — without pushing a new undo snapshot. When `finish` is true - /// the command also exits (the entity is already committed, so no separate - /// commit is needed). + /// Replace the geometry of the live entity `handle` in place while + /// preserving its document identity and common display properties. No new + /// undo snapshot is pushed. When `finish` is true the command also exits. UpdateLiveEntity { handle: Handle, entity: EntityType, diff --git a/src/scene/parametric_solve.rs b/src/scene/parametric_solve.rs index c7ddea8c9..535faf3b1 100644 --- a/src/scene/parametric_solve.rs +++ b/src/scene/parametric_solve.rs @@ -13,7 +13,7 @@ use cadkernel_constraints::constraints::circle_arc::{C2LDistance, P2CDistance, T use cadkernel_constraints::constraints::conic::{ EqualMajorAxesConic, PointOnEllipse, TangentEllipseLine, }; -use cadkernel_constraints::constraints::curve_generic::CurveValue; +use cadkernel_constraints::constraints::curve_generic::{BoundedArcValue, CurveValue}; use cadkernel_constraints::constraints::point_line::{ CenterOfGravity, Difference, Equal, EqualLineLength, MidpointOnLine, P2PDistance, Parallel as ParallelConstraint, Perpendicular as PerpendicularConstraint, PointOnLine, @@ -25,6 +25,8 @@ use cadkernel_constraints::geo::{ Line as GLine, Point as GPoint, }; use cadkernel_constraints::solvers::dogleg::solve_dl; +use cadkernel_constraints::solvers::lm::solve_lm; +use cadkernel_constraints::solvers::SolveStatus; use cadkernel_constraints::system::System; use super::named_parameters::ParameterTable; @@ -364,7 +366,10 @@ fn register_entity( ); let rad = sys.add_param(a.radius, false); let start_angle = sys.add_param(a.start_angle, false); - let end_angle = sys.add_param(a.end_angle, false); + let end_angle = sys.add_param( + a.start_angle + (a.end_angle - a.start_angle).rem_euclid(std::f64::consts::TAU), + false, + ); let start_seed = a.start_point(); let end_seed = a.end_point(); let start = GPoint::new( @@ -855,6 +860,15 @@ fn build_constraint( resolve_constraint_point(document, sys, cache, r) }; + let point_on_bounded_arc = |sys: &mut System, point: GPoint, arc: GArc| { + let initial = BoundedArcValue::initial_parameter(sys.store(), point, arc); + let parameter = sys.add_param(initial, false); + vec![ + Rc::new(BoundedArcValue::new(point, point.x, arc, parameter)) as Rc, + Rc::new(BoundedArcValue::new(point, point.y, arc, parameter)) as Rc, + ] + }; + // `Coincident`/`Concentric`/`CenterPoint` all solve identically — two // points (an endpoint, a circle's center via the existing `-3` marker, // or a plain point) held equal on both axes. Only the DWG-native class @@ -1025,14 +1039,7 @@ fn build_constraint( let zero = sys.add_param(0.0, true); vec![Rc::new(P2CDistance::new(circ, p, zero))] } - // Same "distance to the underlying circle is zero" relation - // as the `Circle` arm — doesn't restrict the point to - // within the arc's own sweep, matching that same - // pre-existing simplification for `Circle`. - EntityGeom::Arc(a) => { - let zero = sys.add_param(0.0, true); - vec![Rc::new(P2CDistance::new(a.circle, p, zero))] - } + EntityGeom::Arc(a) => point_on_bounded_arc(sys, p, a), EntityGeom::Ellipse(ellipse) => { vec![Rc::new(PointOnEllipse::new(p, ellipse))] } @@ -1070,14 +1077,8 @@ fn build_constraint( vec![Rc::new(PointOnLine::new(p, line)) as Rc] }) .or_else(|| { - geom.arc_segment(index).map(|segment| { - let zero = sys.add_param(0.0, true); - vec![Rc::new(P2CDistance::new( - segment.arc.circle, - p, - zero, - )) as Rc] - }) + geom.arc_segment(index) + .map(|segment| point_on_bounded_arc(sys, p, segment.arc)) }) }) .unwrap_or_default(), @@ -1907,33 +1908,6 @@ fn solve_scope( } } - // A point explicitly edited by a grip, STRETCH, or Properties is the - // driver's target for this solve. Pin its current coordinates for this - // solve only so constrained neighbours respond while the chosen point - // stays at the exact cursor or typed coordinate. - for reference in driven_refs { - if !set - .constraints - .iter() - .any(|constraint| constraint.refs.iter().any(|r| r.entity == reference.entity)) - { - continue; - } - let Some(point) = - resolve_constraint_point(document, &mut sys, &mut cache, *reference) - else { - continue; - }; - let (x, y) = { - let store = sys.store(); - (store.get(point.x), store.get(point.y)) - }; - let target_x = sys.add_param(x, true); - let target_y = sys.add_param(y, true); - sys.add_constraint(Rc::new(Equal::new(point.x, target_x, 1.0))); - sys.add_constraint(Rc::new(Equal::new(point.y, target_y, 1.0))); - } - // Ellipse rules (same motivation as arc rules, different mechanism): // `cadkernel_constraints::geo::Ellipse` stores `focus1` as an absolute point, not an // offset from `center`. If some constraint (e.g. Concentric) pulls only @@ -2030,7 +2004,9 @@ fn solve_scope( let partitions = sys.partition(); for sub in &partitions { - solve_dl(sub, sys.store_mut()); + if solve_dl(sub, sys.store_mut()) == SolveStatus::Failed { + solve_lm(sub, sys.store_mut()); + } } // `System::partition` only includes parameters referenced by a // constraint, so an otherwise registered but unconstrained coordinate (e.g. a diff --git a/tests/parametric_constraints_solve.rs b/tests/parametric_constraints_solve.rs index ed71458a8..eb82e2877 100644 --- a/tests/parametric_constraints_solve.rs +++ b/tests/parametric_constraints_solve.rs @@ -1197,10 +1197,10 @@ fn an_arcs_endpoint_stays_consistent_with_its_center_radius_and_angle_after_solv } #[test] -fn point_on_curve_constraint_pulls_a_point_onto_an_arcs_underlying_circle() { +fn point_on_curve_constraint_keeps_a_point_within_an_arcs_sweep() { let mut scene = Scene::new(); let arc = add_arc(&mut scene, 0.0, 0.0, 5.0, 0.0, std::f64::consts::PI); - let marker = add_line(&mut scene, 20.0, 20.0, 21.0, 20.0); + let marker = add_line(&mut scene, 0.0, -20.0, 1.0, -20.0); scene .parametric_constraint_set_mut(ParametricScope::ModelSpace) @@ -1211,12 +1211,73 @@ fn point_on_curve_constraint_pulls_a_point_onto_an_arcs_underlying_circle() { ); scene.bump_entities(&[(marker, ChangeKind::Modified)]); - let (center, radius) = arc_geom(&scene, arc); + let (center, radius, start_angle, end_angle) = + match scene.document.get_entity(arc).expect("entity exists") { + EntityType::Arc(arc) => (arc.center, arc.radius, arc.start_angle, arc.end_angle), + other => panic!("expected an Arc, got {other:?}"), + }; let (p, _) = line_endpoints(&scene, marker); let dist = ((p.x - center.x).powi(2) + (p.y - center.y).powi(2)).sqrt(); assert!( (dist - radius).abs() < 1e-6, - "point should land exactly on the arc's circle: dist={dist} radius={radius}" + "point should land exactly on the arc: dist={dist} radius={radius}" + ); + assert!( + cadkernel::geom2d::angle_within_arc( + (p.y - center.y).atan2(p.x - center.x), + start_angle, + end_angle, + ), + "point must stay within the solved arc sweep: {p:?}" + ); +} + +#[test] +fn point_on_curve_constraint_keeps_a_point_within_a_polyline_arc_segment() { + let mut source = acadrust::entities::LwPolyline::new(); + source.add_point_with_bulge(acadrust::types::Vector2::new(0.0, 0.0), 1.0); + source.add_point(acadrust::types::Vector2::new(10.0, 0.0)); + let mut scene = Scene::new(); + let polyline = scene.add_entity(EntityType::LwPolyline(source)); + let marker = add_line(&mut scene, 5.0, 20.0, 6.0, 20.0); + + scene + .parametric_constraint_set_mut(ParametricScope::ModelSpace) + .add( + ConstraintKind::PointOnCurve, + vec![ + ParametricRef::point(marker, 0), + ParametricRef::segment(polyline, 0), + ], + None, + ); + scene.bump_entities(&[(marker, ChangeKind::Modified)]); + + let (point, _) = line_endpoints(&scene, marker); + let EntityType::LwPolyline(polyline) = scene.document.get_entity(polyline).unwrap() else { + panic!("expected a lightweight polyline"); + }; + let arc = cadkernel::geom2d::BulgeArc::from_bulge( + [ + polyline.vertices[0].location.x, + polyline.vertices[0].location.y, + ], + [ + polyline.vertices[1].location.x, + polyline.vertices[1].location.y, + ], + polyline.vertices[0].bulge, + ) + .unwrap(); + let distance = (point.x - arc.center[0]).hypot(point.y - arc.center[1]); + assert!((distance - arc.radius).abs() < 1e-6); + assert!( + cadkernel::geom2d::angle_within_arc( + (point.y - arc.center[1]).atan2(point.x - arc.center[0]), + arc.start_angle, + arc.start_angle + arc.sweep, + ), + "point must stay within the solved polyline arc segment: {point:?}" ); }