Merge pull request #1289 from ramox81/fix/coincident-standard-command

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",
 ]

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"] }

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)]

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,

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<dyn Constraint>,
+            Rc::new(BoundedArcValue::new(point, point.y, arc, parameter)) as Rc<dyn Constraint>,
+        ]
+    };
+
     // `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<dyn Constraint>]
                             })
                             .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<dyn Constraint>]
-                                })
+                                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

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:?}"
     );
 }