diff --git a/rust/kcl-lib/src/execution/exec_ast.rs b/rust/kcl-lib/src/execution/exec_ast.rs index 3d1230327b3..5ad26c8c594 100644 --- a/rust/kcl-lib/src/execution/exec_ast.rs +++ b/rust/kcl-lib/src/execution/exec_ast.rs @@ -1162,13 +1162,27 @@ impl Node { // Solve constraints. let config = kcl_ezpz::Config::default().with_max_iterations(50); let solve_result = if exec_state.mod_local.freedom_analysis { - kcl_ezpz::solve_analysis(&constraints, initial_guesses.clone(), config) - .map(|outcome| (outcome.outcome, Some(FreedomAnalysis::from(outcome.analysis)))) + kcl_ezpz::solve_analysis(&constraints, initial_guesses.clone(), config).map(|outcome| { + let freedom_analysis = FreedomAnalysis::from(outcome.analysis); + (outcome.outcome, Some(freedom_analysis)) + }) } else { kcl_ezpz::solve(&constraints, initial_guesses.clone(), config).map(|outcome| (outcome, None)) }; + // Build a combined list of all constraints (regular + optional) for conflict detection + let num_required_constraints = sketch_block_state.solver_constraints.len(); + let all_constraints: Vec = sketch_block_state + .solver_constraints + .iter() + .cloned() + .chain(sketch_block_state.solver_optional_constraints.iter().cloned()) + .collect(); + let (solve_outcome, solve_analysis) = match solve_result { - Ok((solved, freedom)) => (Solved::from(solved), freedom), + Ok((solved, freedom)) => ( + Solved::from_ezpz_outcome(solved, &all_constraints, num_required_constraints), + freedom, + ), Err(failure) => { match &failure.error { NonLinearSystemError::FaerMatrix { .. } @@ -1190,6 +1204,7 @@ impl Node { warnings: failure.warnings, unsatisfied: Default::default(), priority_solved: Default::default(), + variables_in_conflicts: Default::default(), }, None, ) diff --git a/rust/kcl-lib/src/execution/freedom_analysis_tests.rs b/rust/kcl-lib/src/execution/freedom_analysis_tests.rs new file mode 100644 index 00000000000..27daa7b1a26 --- /dev/null +++ b/rust/kcl-lib/src/execution/freedom_analysis_tests.rs @@ -0,0 +1,183 @@ +#[cfg(test)] +#[cfg(feature = "artifact-graph")] +mod test { + use std::sync::Arc; + + use crate::{ + ExecutorContext, ExecutorSettings, + engine::conn_mock::EngineConnection, + execution::{ContextType, MockConfig}, + front::{Freedom, ObjectKind}, + frontend::api::ObjectId, + }; + + async fn run_with_freedom_analysis(kcl: &str) -> Vec<(ObjectId, Freedom)> { + let program = crate::Program::parse_no_errs(kcl).unwrap(); + + let exec_ctxt = ExecutorContext { + engine: Arc::new(Box::new(EngineConnection::new().unwrap())), + fs: Arc::new(crate::fs::FileManager::new()), + settings: ExecutorSettings::default(), + context_type: ContextType::Mock, + }; + + let mock_config = MockConfig { + freedom_analysis: true, + ..Default::default() + }; + + let outcome = exec_ctxt.run_mock(&program, &mock_config).await.unwrap(); + + let mut point_freedoms = Vec::new(); + for obj in &outcome.scene_objects { + if let ObjectKind::Segment { + segment: crate::front::Segment::Point(point), + } = &obj.kind + { + point_freedoms.push((obj.id, point.freedom)); + } + } + // Sort by object ID for consistent ordering + point_freedoms.sort_by_key(|(id, _)| id.0); + point_freedoms + } + + #[tokio::test(flavor = "multi_thread")] + async fn test_freedom_analysis_with_conflicts() { + let kcl = r#" +@settings(experimentalFeatures = allow) + +sketch(on = YZ) { + line1 = sketch2::line(start = [var 2mm, var 8mm], end = [var 5mm, var 7mm]) +line1.start.at[0] == 2 +line1.start.at[1] == 8 +line1.end.at[0] == 5 +line1.end.at[1] == 7 + + + line2 = sketch2::line(start = [var 2mm, var 1mm], end = [var -4.75mm, var -0.88mm]) +line2.start.at[0] == 2 +line2.start.at[1] == 1 + + line3 = sketch2::line(start = [var -2.591mm, var -7.081mm], end = [var 1.331mm, var -3.979mm]) +sketch2::distance([line3.start, line3.end]) == 4mm +sketch2::distance([line3.start, line3.end]) == 6mm +} +"#; + + let point_freedoms = run_with_freedom_analysis(kcl).await; + + // Expected: line1 has both ends constrained -> Fixed, Fixed + // line2 has one end constrained -> Fixed, Free (but currently shows Fixed, Conflict - bug) + // line3 has conflicting distance constraints -> Conflict, Conflict (but currently shows Free, Free - bug) + // Note: IDs skip every third because segments don't get freedom values + // Format: (ObjectId, Freedom) + + println!("Point freedoms: {:?}", point_freedoms); + + let expected = vec![ + (ObjectId(1), Freedom::Fixed), + (ObjectId(2), Freedom::Fixed), + (ObjectId(4), Freedom::Fixed), + (ObjectId(5), Freedom::Free), + (ObjectId(7), Freedom::Conflict), + (ObjectId(8), Freedom::Conflict), + ]; + + // This assertion will fail until the bug is fixed + assert_eq!( + point_freedoms, expected, + "Point freedoms should match expected values. Current behavior shows bugs with conflicts and reordered lines." + ); + } + + #[tokio::test(flavor = "multi_thread")] + async fn test_freedom_analysis_without_conflicts() { + let kcl = r#" +@settings(experimentalFeatures = allow) + +sketch(on = YZ) { + line1 = sketch2::line(start = [var 2mm, var 8mm], end = [var 5mm, var 7mm]) +line1.start.at[0] == 2 +line1.start.at[1] == 8 +line1.end.at[0] == 5 +line1.end.at[1] == 7 + + + line2 = sketch2::line(start = [var 2mm, var 1mm], end = [var -4.75mm, var -0.88mm]) +line2.start.at[0] == 2 +line2.start.at[1] == 1 + + line3 = sketch2::line(start = [var -2.591mm, var -7.081mm], end = [var 1.331mm, var -3.979mm]) +sketch2::distance([line3.start, line3.end]) == 4mm +} +"#; + + let point_freedoms = run_with_freedom_analysis(kcl).await; + + // Expected: line1 has both ends constrained -> Fixed, Fixed + // line2 has one end constrained -> Fixed, Free + // line3 has one distance constraint -> Free, Free (both points can move) + + println!("Point freedoms: {:?}", point_freedoms); + + // Expected: Fixed, Fixed, Fixed, Free, Free, Free + let expected = vec![ + (ObjectId(1), Freedom::Fixed), + (ObjectId(2), Freedom::Fixed), + (ObjectId(4), Freedom::Fixed), + (ObjectId(5), Freedom::Free), + (ObjectId(7), Freedom::Free), + (ObjectId(8), Freedom::Free), + ]; + + assert_eq!(point_freedoms, expected, "Point freedoms should match expected values"); + } + + #[tokio::test(flavor = "multi_thread")] + async fn test_freedom_analysis_reordered_lines() { + let kcl = r#" +@settings(experimentalFeatures = allow) + +sketch(on = YZ) { + line1 = sketch2::line(start = [var 2mm, var 8mm], end = [var 5mm, var 7mm]) +line1.start.at[0] == 2 +line1.start.at[1] == 8 +line1.end.at[0] == 5 +line1.end.at[1] == 7 + + line3 = sketch2::line(start = [var -2.591mm, var -7.081mm], end = [var 1.331mm, var -3.979mm]) +sketch2::distance([line3.start, line3.end]) == 4mm +sketch2::distance([line3.start, line3.end]) == 6mm + + line2 = sketch2::line(start = [var 2mm, var 1mm], end = [var -4.75mm, var -0.88mm]) +line2.start.at[0] == 2 +line2.start.at[1] == 1 + +} +"#; + + let point_freedoms = run_with_freedom_analysis(kcl).await; + + // Expected: line1 has both ends constrained -> Fixed, Fixed + // line3 has conflicting distance constraints -> Conflict, Conflict (but bug shows one Conflict, one Free) + // line2 has one end constrained -> Fixed, Free + + println!("Point freedoms: {:?}", point_freedoms); + + let expected = vec![ + (ObjectId(1), Freedom::Fixed), + (ObjectId(2), Freedom::Fixed), + (ObjectId(4), Freedom::Conflict), + (ObjectId(5), Freedom::Conflict), + (ObjectId(9), Freedom::Fixed), + (ObjectId(10), Freedom::Free), + ]; + + // This assertion will fail until the bug is fixed + assert_eq!( + point_freedoms, expected, + "Point freedoms should match expected values. Current behavior shows bug where line3.end is Free instead of Conflict." + ); + } +} diff --git a/rust/kcl-lib/src/execution/mod.rs b/rust/kcl-lib/src/execution/mod.rs index 9f2e25503a5..06d9498b854 100644 --- a/rust/kcl-lib/src/execution/mod.rs +++ b/rust/kcl-lib/src/execution/mod.rs @@ -59,6 +59,9 @@ pub(crate) mod cache; mod cad_op; mod exec_ast; pub mod fn_call; +#[cfg(test)] +#[cfg(feature = "artifact-graph")] +mod freedom_analysis_tests; mod geometry; mod id_generator; mod import; diff --git a/rust/kcl-lib/src/execution/sketch_solve.rs b/rust/kcl-lib/src/execution/sketch_solve.rs index cfb8d59df7e..0bcd75bd5ba 100644 --- a/rust/kcl-lib/src/execution/sketch_solve.rs +++ b/rust/kcl-lib/src/execution/sketch_solve.rs @@ -286,7 +286,7 @@ fn substitute_sketch_var_in_unsolved_expr( source_ranges.to_vec(), ))); }; - let freedom = if solve_outcome.unsatisfied.contains(&var_id.0) { + let freedom = if solve_outcome.variables_in_conflicts.contains(&var_id.0) { Some(Freedom::Conflict) } else if let Some(analysis) = analysis { let solver_var_id = var_id.to_constraint_id(source_ranges.first().copied().unwrap_or_default())?; @@ -306,6 +306,10 @@ fn substitute_sketch_var_in_unsolved_expr( pub(crate) struct Solved { /// Which constraints couldn't be satisfied + #[expect( + dead_code, + reason = "Used internally to build variables_in_conflicts, but not read externally" + )] pub(crate) unsatisfied: Vec, /// Each variable's final value. pub(crate) final_values: Vec, @@ -318,16 +322,130 @@ pub(crate) struct Solved { /// 0 is the highest priority. Larger numbers are lower priority. #[expect(dead_code, reason = "ezpz provides this info, but we aren't using it yet")] pub(crate) priority_solved: u32, + /// Variables involved in unsatisfied constraints (for conflict detection) + pub(crate) variables_in_conflicts: AHashSet, } -impl From for Solved { - fn from(value: kcl_ezpz::SolveOutcome) -> Self { +impl Solved { + /// Create a Solved from a kcl_ezpz::SolveOutcome, building the set of variables + /// involved in unsatisfied constraints by examining the original constraints. + /// Only marks variables from required constraints (not optional) as conflicted. + pub(crate) fn from_ezpz_outcome( + value: kcl_ezpz::SolveOutcome, + constraints: &[kcl_ezpz::Constraint], + num_required_constraints: usize, + ) -> Self { + let unsatisfied = value.unsatisfied().to_owned(); + + // Build a set of variables involved in unsatisfied constraints + // Only include required constraints (not optional ones like from dragging) + let mut variables_in_conflicts = AHashSet::new(); + for &constraint_idx in value.unsatisfied() { + // Only mark as conflicted if it's a required constraint, not an optional one + if constraint_idx < num_required_constraints { + if let Some(constraint) = constraints.get(constraint_idx) { + extract_variable_ids_from_constraint(constraint, &mut variables_in_conflicts); + } + } + } + Self { - unsatisfied: value.unsatisfied().to_owned(), + unsatisfied, final_values: value.final_values().to_owned(), iterations: value.iterations(), warnings: value.warnings().to_owned(), priority_solved: value.priority_solved(), + variables_in_conflicts, + } + } +} + +/// Extract variable IDs from a constraint and add them to the set. +/// This is a helper function to find which variables are involved in a constraint. +fn extract_variable_ids_from_constraint(constraint: &kcl_ezpz::Constraint, variable_set: &mut AHashSet) { + match constraint { + kcl_ezpz::Constraint::Fixed(id, _) => { + variable_set.insert(*id as usize); + } + kcl_ezpz::Constraint::Distance(pt0, pt1, _) => { + extract_ids_from_point(pt0, variable_set); + extract_ids_from_point(pt1, variable_set); + } + kcl_ezpz::Constraint::Horizontal(line) | kcl_ezpz::Constraint::Vertical(line) => { + extract_ids_from_line(line, variable_set); + } + kcl_ezpz::Constraint::PointsCoincident(pt0, pt1) => { + extract_ids_from_point(pt0, variable_set); + extract_ids_from_point(pt1, variable_set); + } + kcl_ezpz::Constraint::Arc(arc) => { + extract_ids_from_arc(arc, variable_set); + } + kcl_ezpz::Constraint::PointLineDistance(point, line, _) => { + extract_ids_from_point(point, variable_set); + extract_ids_from_line(line, variable_set); + } + kcl_ezpz::Constraint::PointArcCoincident(arc, point) => { + extract_ids_from_arc(arc, variable_set); + extract_ids_from_point(point, variable_set); + } + kcl_ezpz::Constraint::LinesEqualLength(line0, line1) => { + extract_ids_from_line(line0, variable_set); + extract_ids_from_line(line1, variable_set); + } + kcl_ezpz::Constraint::LinesAtAngle(line0, line1, _) => { + extract_ids_from_line(line0, variable_set); + extract_ids_from_line(line1, variable_set); + } + _ => { + // This catch-all exists to allow ezpz to add new constraint variants + // If we hit this in a debug build, we should add explicit handling for the new variant. + debug_assert!( + false, + "Unhandled constraint variant: {:?}. Please add explicit handling for this variant in extract_variable_ids_from_constraint.", + constraint + ); + // Fallback: use Debug output to extract IDs heuristically + // This allows release builds to continue working even with new variants + let constraint_str = format!("{:?}", constraint); + extract_ids_from_debug_string(&constraint_str, variable_set); + } + } +} + +/// Extract variable IDs from a DatumPoint. +/// DatumPoint has public fields x_id and y_id that we can access directly. +fn extract_ids_from_point(pt: &kcl_ezpz::datatypes::inputs::DatumPoint, variable_set: &mut AHashSet) { + variable_set.insert(pt.x_id as usize); + variable_set.insert(pt.y_id as usize); +} + +/// Extract variable IDs from a DatumLineSegment. +/// DatumLineSegment has public fields p0 and p1 (start and end points). +fn extract_ids_from_line(line: &kcl_ezpz::datatypes::inputs::DatumLineSegment, variable_set: &mut AHashSet) { + extract_ids_from_point(&line.p0, variable_set); + extract_ids_from_point(&line.p1, variable_set); +} + +/// Extract variable IDs from a DatumCircularArc. +/// DatumCircularArc has public fields center, start, and end (all DatumPoint). +fn extract_ids_from_arc(arc: &kcl_ezpz::datatypes::inputs::DatumCircularArc, variable_set: &mut AHashSet) { + extract_ids_from_point(&arc.center, variable_set); + extract_ids_from_point(&arc.start, variable_set); + extract_ids_from_point(&arc.end, variable_set); +} + +/// Extract numeric IDs from a debug string. +/// This parses the string looking for numeric values that could be variable IDs. +fn extract_ids_from_debug_string(s: &str, variable_set: &mut AHashSet) { + // Use a simple regex-like approach to find numeric values + // This is a heuristic - it will extract all numbers, which might include + // non-ID values, but it's better than missing IDs + for word in s.split_whitespace() { + // Remove common punctuation + let cleaned = word.trim_matches(|c: char| !c.is_ascii_digit()); + if let Ok(id) = cleaned.parse::() { + variable_set.insert(id); } } } @@ -389,7 +507,7 @@ pub(super) fn create_segment_scene_objects( position: ctor.position.clone(), }), owner: None, - freedom: *freedom, + freedom: freedom.unwrap_or(Freedom::Free), constraints: Vec::new(), }), }, @@ -423,7 +541,7 @@ pub(super) fn create_segment_scene_objects( position: start_point2d.clone(), ctor: None, owner: Some(segment.object_id), - freedom: *start_freedom, + freedom: start_freedom.unwrap_or(Freedom::Free), constraints: Vec::new(), }), }, @@ -449,7 +567,7 @@ pub(super) fn create_segment_scene_objects( position: end_point2d.clone(), ctor: None, owner: Some(segment.object_id), - freedom: *end_freedom, + freedom: end_freedom.unwrap_or(Freedom::Free), constraints: Vec::new(), }), }, @@ -505,7 +623,7 @@ pub(super) fn create_segment_scene_objects( position: start_point2d.clone(), ctor: None, owner: Some(segment.object_id), - freedom: *start_freedom, + freedom: start_freedom.unwrap_or(Freedom::Free), constraints: Vec::new(), }), }, @@ -531,7 +649,7 @@ pub(super) fn create_segment_scene_objects( position: end_point2d.clone(), ctor: None, owner: Some(segment.object_id), - freedom: *end_freedom, + freedom: end_freedom.unwrap_or(Freedom::Free), constraints: Vec::new(), }), }, @@ -557,7 +675,7 @@ pub(super) fn create_segment_scene_objects( position: center_point2d.clone(), ctor: None, owner: Some(segment.object_id), - freedom: *center_freedom, + freedom: center_freedom.unwrap_or(Freedom::Free), constraints: Vec::new(), }), }, diff --git a/rust/kcl-lib/src/frontend.rs b/rust/kcl-lib/src/frontend.rs index 01575795485..7e7d5e4213e 100644 --- a/rust/kcl-lib/src/frontend.rs +++ b/rust/kcl-lib/src/frontend.rs @@ -1,4 +1,8 @@ -use std::{cell::Cell, collections::HashSet, ops::ControlFlow}; +use std::{ + cell::Cell, + collections::{HashMap, HashSet}, + ops::ControlFlow, +}; use kcl_error::SourceRange; @@ -8,7 +12,7 @@ use crate::{ exec::WarningLevel, execution::MockConfig, fmt::format_number_literal, - front::{ArcCtor, Distance, LinesEqualLength, Parallel, Perpendicular, PointCtor}, + front::{ArcCtor, Distance, Freedom, LinesEqualLength, Parallel, Perpendicular, PointCtor}, frontend::{ api::{ Error, Expr, FileId, Number, ObjectId, ObjectKind, ProjectId, SceneGraph, SceneGraphDelta, SourceDelta, @@ -90,6 +94,9 @@ enum ChangeKind { pub struct FrontendState { program: Program, scene_graph: SceneGraph, + /// Stores the last known freedom value for each point object. + /// This allows us to preserve freedom values when freedom analysis isn't run. + point_freedom_cache: HashMap, } impl Default for FrontendState { @@ -110,6 +117,7 @@ impl FrontendState { settings: Default::default(), sketch_mode: Default::default(), }, + point_freedom_cache: HashMap::new(), } } } @@ -133,7 +141,8 @@ impl SketchApi for FrontendState { })?; let new_source = source_from_ast(&self.program.ast); let src_delta = SourceDelta { text: new_source }; - let outcome = self.update_state_after_exec(outcome); + // MockConfig::default() has freedom_analysis: true + let outcome = self.update_state_after_exec(outcome, true); let scene_graph_delta = SceneGraphDelta { new_graph: self.scene_graph.clone(), new_objects: Default::default(), @@ -251,7 +260,8 @@ impl SketchApi for FrontendState { let src_delta = SourceDelta { text: new_source }; // Store the object in the scene. self.scene_graph.sketch_mode = Some(sketch_id); - let outcome = self.update_state_after_exec(outcome); + // Uses .no_freedom_analysis() so freedom_analysis: false + let outcome = self.update_state_after_exec(outcome, false); let scene_graph_delta = SceneGraphDelta { new_graph: self.scene_graph.clone(), invalidates_ids: false, @@ -301,7 +311,8 @@ impl SketchApi for FrontendState { } })?; - let outcome = self.update_state_after_exec(outcome); + // MockConfig::default() has freedom_analysis: true + let outcome = self.update_state_after_exec(outcome, true); let scene_graph_delta = SceneGraphDelta { new_graph: self.scene_graph.clone(), invalidates_ids: false, @@ -341,7 +352,8 @@ impl SketchApi for FrontendState { } })?; - self.update_state_after_exec(outcome); + // exit_sketch doesn't run freedom analysis, just clears sketch_mode + self.update_state_after_exec(outcome, false); Ok(self.scene_graph.clone()) } @@ -598,15 +610,17 @@ impl FrontendState { // Execute so that the objects are updated and available for the next // API call. - let outcome = ctx.run_with_caching(program).await.map_err(|err| { - // TODO: sketch-api: Yeah, this needs to change. We need to - // return the full error. - Error { - msg: err.error.message().to_owned(), - } + // This always uses engine execution (not mock) so that things are cached. + // Engine execution now runs freedom analysis automatically. + // Clear the freedom cache since IDs might have changed after direct editing + // and we're about to run freedom analysis which will repopulate it. + self.point_freedom_cache.clear(); + let outcome = ctx.run_with_caching(program).await.map_err(|err| Error { + msg: err.error.message().to_owned(), })?; + let freedom_analysis_ran = true; - let outcome = self.update_state_after_exec(outcome); + let outcome = self.update_state_after_exec(outcome, freedom_analysis_ran); Ok((self.scene_graph.clone(), outcome)) } @@ -725,7 +739,8 @@ impl FrontendState { vec![segment_id] }; let src_delta = SourceDelta { text: new_source }; - let outcome = self.update_state_after_exec(outcome); + // Uses .no_freedom_analysis() so freedom_analysis: false + let outcome = self.update_state_after_exec(outcome, false); let scene_graph_delta = SceneGraphDelta { new_graph: self.scene_graph.clone(), invalidates_ids: false, @@ -845,7 +860,8 @@ impl FrontendState { vec![line.start, line.end, segment_id] }; let src_delta = SourceDelta { text: new_source }; - let outcome = self.update_state_after_exec(outcome); + // Uses .no_freedom_analysis() so freedom_analysis: false + let outcome = self.update_state_after_exec(outcome, false); let scene_graph_delta = SceneGraphDelta { new_graph: self.scene_graph.clone(), invalidates_ids: false, @@ -971,7 +987,8 @@ impl FrontendState { vec![arc.start, arc.end, arc.center, segment_id] }; let src_delta = SourceDelta { text: new_source }; - let outcome = self.update_state_after_exec(outcome); + // Uses .no_freedom_analysis() so freedom_analysis: false + let outcome = self.update_state_after_exec(outcome, false); let scene_graph_delta = SceneGraphDelta { new_graph: self.scene_graph.clone(), invalidates_ids: false, @@ -1302,7 +1319,7 @@ impl FrontendState { let mock_config = MockConfig { freedom_analysis: is_delete, #[cfg(feature = "artifact-graph")] - segment_ids_edited, + segment_ids_edited: segment_ids_edited.clone(), ..Default::default() }; let outcome = ctx.run_mock(&truncated_program, &mock_config).await.map_err(|err| { @@ -1313,7 +1330,8 @@ impl FrontendState { } })?; - let outcome = self.update_state_after_exec(outcome); + // Uses freedom_analysis: is_delete + let outcome = self.update_state_after_exec(outcome, is_delete); #[cfg(feature = "artifact-graph")] let new_source = { @@ -1888,7 +1906,8 @@ impl FrontendState { }; let src_delta = SourceDelta { text: new_source }; - let outcome = self.update_state_after_exec(outcome); + // Uses MockConfig::default() which has freedom_analysis: true + let outcome = self.update_state_after_exec(outcome, true); let scene_graph_delta = SceneGraphDelta { new_graph: self.scene_graph.clone(), invalidates_ids: false, @@ -1971,13 +1990,79 @@ impl FrontendState { Ok(()) } - fn update_state_after_exec(&mut self, outcome: ExecOutcome) -> ExecOutcome { + fn update_state_after_exec(&mut self, outcome: ExecOutcome, freedom_analysis_ran: bool) -> ExecOutcome { #[cfg(not(feature = "artifact-graph"))] - return outcome; + { + let _ = freedom_analysis_ran; // Only used when artifact-graph feature is enabled + outcome + } #[cfg(feature = "artifact-graph")] { let mut outcome = outcome; - self.scene_graph.objects = std::mem::take(&mut outcome.scene_objects); + let new_objects = std::mem::take(&mut outcome.scene_objects); + + if freedom_analysis_ran { + // When freedom analysis ran, replace the cache entirely with new values + // Don't merge with old values since IDs might have changed + self.point_freedom_cache.clear(); + for new_obj in &new_objects { + if let ObjectKind::Segment { + segment: crate::front::Segment::Point(point), + } = &new_obj.kind + { + self.point_freedom_cache.insert(new_obj.id, point.freedom); + } + } + // Objects are already correct from the analysis, just use them as-is + self.scene_graph.objects = new_objects; + } else { + // When freedom analysis didn't run, preserve old values and merge + // Before replacing objects, extract and store freedom values from old objects + for old_obj in &self.scene_graph.objects { + if let ObjectKind::Segment { + segment: crate::front::Segment::Point(point), + } = &old_obj.kind + { + self.point_freedom_cache.insert(old_obj.id, point.freedom); + } + } + + // Update objects, preserving stored freedom values when new is Free (might be default) + let mut updated_objects = Vec::with_capacity(new_objects.len()); + for new_obj in new_objects { + let mut obj = new_obj; + if let ObjectKind::Segment { + segment: crate::front::Segment::Point(point), + } = &mut obj.kind + { + let new_freedom = point.freedom; + // When freedom_analysis=false, new values are defaults (Free). + // Only preserve cached values when new is Free (indicating it's a default, not from analysis). + // If new is NOT Free, use the new value (it came from somewhere else, maybe conflict detection). + // Never preserve Conflict from cache - conflicts are transient and should only be set + // when there are actually unsatisfied constraints. + if new_freedom == Freedom::Free { + // New value is Free (default when analysis didn't run) + if let Some(stored_freedom) = self.point_freedom_cache.get(&obj.id).copied() { + if stored_freedom == Freedom::Conflict { + // Don't preserve Conflict - conflicts are transient + // Keep it as Free + } else if stored_freedom != Freedom::Free { + // Preserve non-Free, non-Conflict cached value (e.g., Fixed) + point.freedom = stored_freedom; + } + // If stored is also Free, keep Free (no change needed) + } + // If no cached value, keep Free (default) + } + // Store the new freedom value (even if it's Free, so we know it was set) + self.point_freedom_cache.insert(obj.id, point.freedom); + } + updated_objects.push(obj); + } + + self.scene_graph.objects = updated_objects; + } outcome } } diff --git a/rust/kcl-lib/src/frontend/sketch.rs b/rust/kcl-lib/src/frontend/sketch.rs index a24b3ef4a2b..46315b2d3ba 100644 --- a/rust/kcl-lib/src/frontend/sketch.rs +++ b/rust/kcl-lib/src/frontend/sketch.rs @@ -121,7 +121,7 @@ pub struct Point { pub position: Point2d, pub ctor: Option, pub owner: Option, - pub freedom: Option, + pub freedom: Freedom, pub constraints: Vec, } diff --git a/src/machines/sketchSolve/segments.test.ts b/src/machines/sketchSolve/segments.test.ts new file mode 100644 index 00000000000..be6854ae905 --- /dev/null +++ b/src/machines/sketchSolve/segments.test.ts @@ -0,0 +1,595 @@ +import { describe, it, expect } from 'vitest' + +import { + deriveSegmentFreedom, + getSegmentColor, +} from '@src/machines/sketchSolve/segmentsUtils' +import type { ApiObject, Freedom } from '@rust/kcl-lib/bindings/FrontendApi' +import { SKETCH_SELECTION_COLOR } from '@src/lib/constants' + +// Helper to create a point object +function createPointObject(id: number, freedom: Freedom): ApiObject { + return { + id, + kind: { + type: 'Segment', + segment: { + type: 'Point', + position: { + x: { value: 0, units: 'Mm' }, + y: { value: 0, units: 'Mm' }, + }, + ctor: null, + owner: null, + freedom, + constraints: [], + }, + }, + label: '', + comments: '', + artifact_id: '0', + source: { type: 'Simple', range: [0, 0, 0] }, + } +} + +// Helper to create a line segment object +function createLineSegmentObject( + id: number, + startId: number, + endId: number +): ApiObject { + return { + id, + kind: { + type: 'Segment', + segment: { + type: 'Line', + start: startId, + end: endId, + ctor: { + type: 'Line', + start: { + x: { type: 'Var', value: 0, units: 'Mm' }, + y: { type: 'Var', value: 0, units: 'Mm' }, + }, + end: { + x: { type: 'Var', value: 0, units: 'Mm' }, + y: { type: 'Var', value: 0, units: 'Mm' }, + }, + }, + ctor_applicable: false, + }, + }, + label: '', + comments: '', + artifact_id: '0', + source: { type: 'Simple', range: [0, 0, 0] }, + } +} + +// Helper to create an arc segment object +function createArcSegmentObject( + id: number, + startId: number, + endId: number, + centerId: number +): ApiObject { + return { + id, + kind: { + type: 'Segment', + segment: { + type: 'Arc', + start: startId, + end: endId, + center: centerId, + ctor: { + type: 'Arc', + start: { + x: { type: 'Var', value: 0, units: 'Mm' }, + y: { type: 'Var', value: 0, units: 'Mm' }, + }, + end: { + x: { type: 'Var', value: 0, units: 'Mm' }, + y: { type: 'Var', value: 0, units: 'Mm' }, + }, + center: { + x: { type: 'Var', value: 0, units: 'Mm' }, + y: { type: 'Var', value: 0, units: 'Mm' }, + }, + }, + ctor_applicable: false, + }, + }, + label: '', + comments: '', + artifact_id: '0', + source: { type: 'Simple', range: [0, 0, 0] }, + } +} + +// Helper to create a circle segment object +function createCircleSegmentObject(id: number, startId: number): ApiObject { + return { + id, + kind: { + type: 'Segment', + segment: { + type: 'Circle', + start: startId, + radius: { value: 1, units: 'Mm' }, + ctor: { + type: 'Circle', + center: { + x: { type: 'Var', value: 0, units: 'Mm' }, + y: { type: 'Var', value: 0, units: 'Mm' }, + }, + radius: { type: 'Var', value: 1, units: 'Mm' }, + }, + ctor_applicable: false, + }, + }, + label: '', + comments: '', + artifact_id: '0', + source: { type: 'Simple', range: [0, 0, 0] }, + } +} + +describe('deriveSegmentFreedom', () => { + it('should return null for non-segment objects', () => { + const nonSegment: ApiObject = { + id: 1, + kind: { + type: 'Sketch', + args: { on: { default: 'xy' } }, + segments: [], + constraints: [], + }, + label: '', + comments: '', + artifact_id: '0', + source: { type: 'Simple', range: [0, 0, 0] }, + } + + expect(deriveSegmentFreedom(nonSegment, [])).toBeNull() + }) + + it('should return freedom directly for Point segments', () => { + const point = createPointObject(1, 'Fixed') + expect(deriveSegmentFreedom(point, [])).toBe('Fixed') + + const freePoint = createPointObject(2, 'Free') + expect(deriveSegmentFreedom(freePoint, [])).toBe('Free') + + const conflictPoint = createPointObject(3, 'Conflict') + expect(deriveSegmentFreedom(conflictPoint, [])).toBe('Conflict') + }) + + it('should return null for Point segments with null freedom', () => { + const point: ApiObject = { + id: 1, + kind: { + type: 'Segment', + segment: { + type: 'Point', + position: { + x: { value: 0, units: 'Mm' }, + y: { value: 0, units: 'Mm' }, + }, + ctor: null, + owner: null, + freedom: null as any, + constraints: [], + }, + }, + label: '', + comments: '', + artifact_id: '0', + source: { type: 'Simple', range: [0, 0, 0] }, + } + expect(deriveSegmentFreedom(point, [])).toBeNull() + }) + + describe('Line segments', () => { + it('should return Fixed when both points are Fixed', () => { + const startPoint = createPointObject(1, 'Fixed') + const endPoint = createPointObject(2, 'Fixed') + const line = createLineSegmentObject(3, 1, 2) + const objects = [startPoint, endPoint, line] + + expect(deriveSegmentFreedom(line, objects)).toBe('Fixed') + }) + + it('should return Free when one point is Free', () => { + const startPoint = createPointObject(1, 'Fixed') + const endPoint = createPointObject(2, 'Free') + const line = createLineSegmentObject(3, 1, 2) + const objects = [startPoint, endPoint, line] + + expect(deriveSegmentFreedom(line, objects)).toBe('Free') + }) + + it('should return Conflict when one point is Conflict', () => { + const startPoint = createPointObject(1, 'Fixed') + const endPoint = createPointObject(2, 'Conflict') + const line = createLineSegmentObject(3, 1, 2) + const objects = [startPoint, endPoint, line] + + expect(deriveSegmentFreedom(line, objects)).toBe('Conflict') + }) + + it('should return Conflict when both points are Conflict', () => { + const startPoint = createPointObject(1, 'Conflict') + const endPoint = createPointObject(2, 'Conflict') + const line = createLineSegmentObject(3, 1, 2) + const objects = [startPoint, endPoint, line] + + expect(deriveSegmentFreedom(line, objects)).toBe('Conflict') + }) + + it('should return Conflict when one point is Conflict and one is Free', () => { + const startPoint = createPointObject(1, 'Conflict') + const endPoint = createPointObject(2, 'Free') + const line = createLineSegmentObject(3, 1, 2) + const objects = [startPoint, endPoint, line] + + expect(deriveSegmentFreedom(line, objects)).toBe('Conflict') + }) + + it('should return null when points are missing', () => { + const line = createLineSegmentObject(3, 1, 2) + const objects = [line] // No point objects + + expect(deriveSegmentFreedom(line, objects)).toBeNull() + }) + + it('should filter out null freedoms and use valid ones', () => { + const startPoint: ApiObject = { + id: 1, + kind: { + type: 'Segment', + segment: { + type: 'Point', + position: { + x: { value: 0, units: 'Mm' }, + y: { value: 0, units: 'Mm' }, + }, + ctor: null, + owner: null, + freedom: null as any, + constraints: [], + }, + }, + label: '', + comments: '', + artifact_id: '0', + source: { type: 'Simple', range: [0, 0, 0] }, + } + const endPoint = createPointObject(2, 'Fixed') + const line = createLineSegmentObject(3, 1, 2) + const objects = [startPoint, endPoint, line] + + // When one point has null freedom, it's filtered out and only the valid one is used + expect(deriveSegmentFreedom(line, objects)).toBe('Fixed') + }) + + it('should return null when all points have null freedom', () => { + const startPoint: ApiObject = { + id: 1, + kind: { + type: 'Segment', + segment: { + type: 'Point', + position: { + x: { value: 0, units: 'Mm' }, + y: { value: 0, units: 'Mm' }, + }, + ctor: null, + owner: null, + freedom: null as any, + constraints: [], + }, + }, + label: '', + comments: '', + artifact_id: '0', + source: { type: 'Simple', range: [0, 0, 0] }, + } + const endPoint: ApiObject = { + id: 2, + kind: { + type: 'Segment', + segment: { + type: 'Point', + position: { + x: { value: 0, units: 'Mm' }, + y: { value: 0, units: 'Mm' }, + }, + ctor: null, + owner: null, + freedom: null as any, + constraints: [], + }, + }, + label: '', + comments: '', + artifact_id: '0', + source: { type: 'Simple', range: [0, 0, 0] }, + } + const line = createLineSegmentObject(3, 1, 2) + const objects = [startPoint, endPoint, line] + + expect(deriveSegmentFreedom(line, objects)).toBeNull() + }) + }) + + describe('Arc segments', () => { + it('should return Fixed when all three points are Fixed', () => { + const startPoint = createPointObject(1, 'Fixed') + const endPoint = createPointObject(2, 'Fixed') + const centerPoint = createPointObject(3, 'Fixed') + const arc = createArcSegmentObject(4, 1, 2, 3) + const objects = [startPoint, endPoint, centerPoint, arc] + + expect(deriveSegmentFreedom(arc, objects)).toBe('Fixed') + }) + + it('should return Free when one point is Free', () => { + const startPoint = createPointObject(1, 'Fixed') + const endPoint = createPointObject(2, 'Fixed') + const centerPoint = createPointObject(3, 'Free') + const arc = createArcSegmentObject(4, 1, 2, 3) + const objects = [startPoint, endPoint, centerPoint, arc] + + expect(deriveSegmentFreedom(arc, objects)).toBe('Free') + }) + + it('should return Conflict when one point is Conflict', () => { + const startPoint = createPointObject(1, 'Fixed') + const endPoint = createPointObject(2, 'Fixed') + const centerPoint = createPointObject(3, 'Conflict') + const arc = createArcSegmentObject(4, 1, 2, 3) + const objects = [startPoint, endPoint, centerPoint, arc] + + expect(deriveSegmentFreedom(arc, objects)).toBe('Conflict') + }) + + it('should return Conflict when multiple points have different freedoms', () => { + const startPoint = createPointObject(1, 'Conflict') + const endPoint = createPointObject(2, 'Free') + const centerPoint = createPointObject(3, 'Fixed') + const arc = createArcSegmentObject(4, 1, 2, 3) + const objects = [startPoint, endPoint, centerPoint, arc] + + expect(deriveSegmentFreedom(arc, objects)).toBe('Conflict') + }) + + it('should use available points when some are missing', () => { + const startPoint = createPointObject(1, 'Fixed') + const arc = createArcSegmentObject(4, 1, 2, 3) + const objects = [startPoint, arc] // Missing end and center + + // When only one point is available, it uses that point's freedom + expect(deriveSegmentFreedom(arc, objects)).toBe('Fixed') + }) + + it('should return null when all points are missing', () => { + const arc = createArcSegmentObject(4, 1, 2, 3) + const objects = [arc] // No point objects + + expect(deriveSegmentFreedom(arc, objects)).toBeNull() + }) + }) + + describe('Circle segments', () => { + it('should return Fixed when center point is Fixed', () => { + const centerPoint = createPointObject(1, 'Fixed') + const circle = createCircleSegmentObject(2, 1) + const objects = [centerPoint, circle] + + expect(deriveSegmentFreedom(circle, objects)).toBe('Fixed') + }) + + it('should return Free when center point is Free', () => { + const centerPoint = createPointObject(1, 'Free') + const circle = createCircleSegmentObject(2, 1) + const objects = [centerPoint, circle] + + expect(deriveSegmentFreedom(circle, objects)).toBe('Free') + }) + + it('should return Conflict when center point is Conflict', () => { + const centerPoint = createPointObject(1, 'Conflict') + const circle = createCircleSegmentObject(2, 1) + const objects = [centerPoint, circle] + + expect(deriveSegmentFreedom(circle, objects)).toBe('Conflict') + }) + }) +}) + +describe('getSegmentColor', () => { + const UNCONSTRAINED_COLOR = parseInt('#3c73ff'.replace('#', ''), 16) // Brand blue + const CONFLICT_COLOR = 0xff5e5b // Coral red + const TEXT_COLOR = 0xffffff // White + const DRAFT_COLOR = 0x888888 // Grey + + it('should return draft color when isDraft is true (highest priority)', () => { + const color = getSegmentColor({ + isDraft: true, + isHovered: true, + isSelected: true, + freedom: 'Conflict', + }) + + expect(color).toBe(DRAFT_COLOR) + }) + + it('should return hover color when isHovered is true (priority 2)', () => { + const color = getSegmentColor({ + isDraft: false, + isHovered: true, + isSelected: true, + freedom: 'Conflict', + }) + + // Hover color is calculated from SKETCH_SELECTION_RGB at 70% brightness + // We can't easily test the exact value without importing SKETCH_SELECTION_RGB, + // but we can verify it's not any of the other colors + expect(color).not.toBe(DRAFT_COLOR) + expect(color).not.toBe(SKETCH_SELECTION_COLOR) + expect(color).not.toBe(CONFLICT_COLOR) + expect(color).not.toBe(TEXT_COLOR) + expect(color).not.toBe(UNCONSTRAINED_COLOR) + }) + + it('should return selection color when isSelected is true (priority 3)', () => { + const color = getSegmentColor({ + isDraft: false, + isHovered: false, + isSelected: true, + freedom: 'Conflict', + }) + + expect(color).toBe(SKETCH_SELECTION_COLOR) + }) + + it('should return conflict color when freedom is Conflict (priority 4)', () => { + const color = getSegmentColor({ + isDraft: false, + isHovered: false, + isSelected: false, + freedom: 'Conflict', + }) + + expect(color).toBe(CONFLICT_COLOR) + }) + + it('should return unconstrained color when freedom is Free (priority 5)', () => { + const color = getSegmentColor({ + isDraft: false, + isHovered: false, + isSelected: false, + freedom: 'Free', + }) + + expect(color).toBe(UNCONSTRAINED_COLOR) + }) + + it('should return constrained color when freedom is Fixed (priority 6)', () => { + const color = getSegmentColor({ + isDraft: false, + isHovered: false, + isSelected: false, + freedom: 'Fixed', + }) + + expect(color).toBe(TEXT_COLOR) + }) + + it('should return unconstrained color when freedom is null (default)', () => { + const color = getSegmentColor({ + isDraft: false, + isHovered: false, + isSelected: false, + freedom: null, + }) + + expect(color).toBe(UNCONSTRAINED_COLOR) + }) + + it('should return unconstrained color when freedom is undefined (default)', () => { + const color = getSegmentColor({ + isDraft: false, + isHovered: false, + isSelected: false, + }) + + expect(color).toBe(UNCONSTRAINED_COLOR) + }) + + it('should prioritize draft over all other states', () => { + const color1 = getSegmentColor({ + isDraft: true, + isHovered: true, + isSelected: false, + freedom: 'Fixed', + }) + + const color2 = getSegmentColor({ + isDraft: true, + isHovered: false, + isSelected: true, + freedom: 'Conflict', + }) + + expect(color1).toBe(DRAFT_COLOR) + expect(color2).toBe(DRAFT_COLOR) + }) + + it('should prioritize hover over selection and freedom', () => { + const hoverColor = getSegmentColor({ + isDraft: false, + isHovered: true, + isSelected: true, + freedom: 'Fixed', + }) + + const selectionColor = getSegmentColor({ + isDraft: false, + isHovered: false, + isSelected: true, + freedom: 'Fixed', + }) + + expect(hoverColor).not.toBe(selectionColor) + expect(hoverColor).not.toBe(TEXT_COLOR) + }) + + it('should prioritize selection over freedom', () => { + const selectedColor = getSegmentColor({ + isDraft: false, + isHovered: false, + isSelected: true, + freedom: 'Free', + }) + + const unselectedColor = getSegmentColor({ + isDraft: false, + isHovered: false, + isSelected: false, + freedom: 'Free', + }) + + expect(selectedColor).toBe(SKETCH_SELECTION_COLOR) + expect(unselectedColor).toBe(UNCONSTRAINED_COLOR) + }) + + it('should prioritize conflict over free and fixed', () => { + const conflictColor = getSegmentColor({ + isDraft: false, + isHovered: false, + isSelected: false, + freedom: 'Conflict', + }) + + const freeColor = getSegmentColor({ + isDraft: false, + isHovered: false, + isSelected: false, + freedom: 'Free', + }) + + const fixedColor = getSegmentColor({ + isDraft: false, + isHovered: false, + isSelected: false, + freedom: 'Fixed', + }) + + expect(conflictColor).toBe(CONFLICT_COLOR) + expect(freeColor).toBe(UNCONSTRAINED_COLOR) + expect(fixedColor).toBe(TEXT_COLOR) + }) +}) diff --git a/src/machines/sketchSolve/segments.ts b/src/machines/sketchSolve/segments.ts index 6ea353dcf1c..4275e75a8ed 100644 --- a/src/machines/sketchSolve/segments.ts +++ b/src/machines/sketchSolve/segments.ts @@ -1,4 +1,4 @@ -import type { SegmentCtor } from '@rust/kcl-lib/bindings/FrontendApi' +import type { SegmentCtor, Freedom } from '@rust/kcl-lib/bindings/FrontendApi' import { SKETCH_LAYER, SKETCH_POINT_HANDLE, @@ -24,13 +24,9 @@ import { createArcGeometry, } from '@src/clientSideScene/segments' import { STRAIGHT_SEGMENT_BODY } from '@src/clientSideScene/sceneConstants' -import { - KCL_DEFAULT_COLOR, - packRgbToColor, - SKETCH_SELECTION_COLOR, - SKETCH_SELECTION_RGB, - SKETCH_SELECTION_RGB_STR, -} from '@src/lib/constants' +import { KCL_DEFAULT_COLOR } from '@src/lib/constants' +// Import and re-export pure utility functions +import { getSegmentColor } from '@src/machines/sketchSolve/segmentsUtils' export const SEGMENT_TYPE_POINT = 'POINT' export const SEGMENT_TYPE_LINE = 'LINE' @@ -44,6 +40,7 @@ interface CreateSegmentArgs { id: number scale: number isDraft?: boolean + freedom?: Freedom | null } interface UpdateSegmentArgs { @@ -54,6 +51,7 @@ interface UpdateSegmentArgs { group: Group selectedIds: Array isDraft?: boolean + freedom?: Freedom | null } /** @@ -111,8 +109,23 @@ class PointSegment implements SketchEntityUtils { isSelected: boolean isHovered: boolean isDraft?: boolean + freedom?: Freedom | null } ): void { + // Use color precedence system + const color = getSegmentColor({ + isDraft: status.isDraft, + isHovered: status.isHovered, + isSelected: status.isSelected, + freedom: status.freedom, + }) + + // Convert hex color to RGB string for CSS + const r = (color >> 16) & 0xff + const g = (color >> 8) & 0xff + const b = color & 0xff + const rgbStr = `${r}, ${g}, ${b}` + // Draft segments are grey if (status.isDraft) { innerCircle.style.backgroundColor = '#888888' @@ -120,20 +133,14 @@ class PointSegment implements SketchEntityUtils { return // draft styles take precedence } if (status.isHovered) { - // Calculate darker version of SKETCH_SELECTION_COLOR (70% brightness) - const darkerSelectionRgb = SKETCH_SELECTION_RGB.map((val) => - Math.round(val * 0.7) - ) - const darkerSelectionRgbStr = darkerSelectionRgb.join(', ') - innerCircle.style.backgroundColor = `rgb(${darkerSelectionRgbStr})` - innerCircle.style.border = `1px solid rgba(${darkerSelectionRgbStr}, 0.5)` - return // Hover styles take precedence over isSelection status + // getSegmentColor already returns the hover color at 70% brightness + innerCircle.style.backgroundColor = `rgb(${rgbStr})` + innerCircle.style.border = `1px solid rgba(${rgbStr}, 0.5)` + return // Hover styles take precedence } - innerCircle.style.backgroundColor = status.isSelected - ? `rgb(${SKETCH_SELECTION_RGB_STR})` - : KCL_DEFAULT_COLOR + innerCircle.style.backgroundColor = `rgb(${rgbStr})` innerCircle.style.border = status.isSelected - ? `2px solid rgba(${SKETCH_SELECTION_RGB_STR}, 0.5)` + ? `2px solid rgba(${rgbStr}, 0.5)` : '0px solid #CCCCCC' } @@ -189,10 +196,12 @@ class PointSegment implements SketchEntityUtils { this.updatePointSize(innerCircle, true) const isSelected = handleDiv.dataset.isSelected === 'true' const isDraft = handleDiv.dataset.isDraft === 'true' + const freedom = (handleDiv.dataset.freedom as Freedom | undefined) || null this.updatePointColors(innerCircle, { isSelected, isHovered: true, isDraft, + freedom, }) }) @@ -202,7 +211,13 @@ class PointSegment implements SketchEntityUtils { // Restore colors based on selection state stored in data attribute const isSelected = handleDiv.dataset.isSelected === 'true' const isDraft = handleDiv.dataset.isDraft === 'true' - this.updatePointColors(innerCircle, { isSelected, isHovered, isDraft }) + const freedom = (handleDiv.dataset.freedom as Freedom | undefined) || null + this.updatePointColors(innerCircle, { + isSelected, + isHovered, + isDraft, + freedom, + }) }) const cssObject = new CSS2DObject(handleDiv) @@ -215,6 +230,9 @@ class PointSegment implements SketchEntityUtils { } segmentGroup.add(cssObject) + // Store freedom in userData for later access + segmentGroup.userData.freedom = args.freedom ?? null + this.update({ input: args.input, theme: args.theme, @@ -223,6 +241,7 @@ class PointSegment implements SketchEntityUtils { group: segmentGroup, selectedIds: [], isDraft: args.isDraft, + freedom: args.freedom, }) return segmentGroup } @@ -247,10 +266,17 @@ class PointSegment implements SketchEntityUtils { if (!innerCircle) return const isSelected = selectedIds.includes(id) + // Get freedom from args or group userData + const freedom = args.freedom ?? group.userData.freedom ?? null + // Update userData for consistency + group.userData.freedom = freedom + // Store selection state in data attribute for hover handlers el.dataset.isSelected = String(isSelected) // Store draft state in data attribute for hover handlers el.dataset.isDraft = String(isDraft ?? false) + // Store freedom state in data attribute for hover handlers + el.dataset.freedom = freedom ?? '' // Only update colors if not hovering (hover styles take precedence) if (!el.matches(':hover')) { @@ -258,6 +284,7 @@ class PointSegment implements SketchEntityUtils { isSelected, isHovered: false, isDraft, + freedom, }) } } @@ -272,25 +299,21 @@ class LineSegment implements SketchEntityUtils { mesh: Mesh, isSelected: boolean, isHovered: boolean, - isDraft?: boolean + isDraft?: boolean, + freedom?: Freedom | null ): void { const material = mesh.material if (!(material instanceof MeshBasicMaterial)) { return } - if (isHovered) { - material.color.set( - packRgbToColor(SKETCH_SELECTION_RGB.map((val) => Math.round(val * 0.7))) - ) - } else if (isSelected) { - material.color.set(SKETCH_SELECTION_COLOR) - } else if (isDraft) { - // Draft segments are grey (0x888888) - material.color.set(0x888888) - } else { - material.color.set(KCL_DEFAULT_COLOR) - } + const color = getSegmentColor({ + isDraft, + isHovered, + isSelected, + freedom, + }) + material.color.set(color) } init = (args: CreateSegmentArgs) => { @@ -334,6 +357,9 @@ class LineSegment implements SketchEntityUtils { segmentGroup.add(mesh) + // Store freedom in userData + segmentGroup.userData.freedom = args.freedom ?? null + this.update({ input: input, theme: theme, @@ -342,6 +368,7 @@ class LineSegment implements SketchEntityUtils { group: segmentGroup, selectedIds: [], isDraft: args.isDraft, + freedom: args.freedom, }) return segmentGroup @@ -386,7 +413,18 @@ class LineSegment implements SketchEntityUtils { const isSelected = selectedIds.includes(id) // Check if this segment is currently hovered (stored in userData) const isHovered = straightSegmentBody.userData.isHovered === true - this.updateLineColors(straightSegmentBody, isSelected, isHovered, isDraft) + // Get freedom from args or group userData + const freedom = args.freedom ?? group.userData.freedom ?? null + // Update userData for consistency + group.userData.freedom = freedom + + this.updateLineColors( + straightSegmentBody, + isSelected, + isHovered, + isDraft, + freedom + ) } } @@ -469,25 +507,21 @@ class ArcSegment implements SketchEntityUtils { mesh: Mesh, isSelected: boolean, isHovered: boolean, - isDraft?: boolean + isDraft?: boolean, + freedom?: Freedom | null ): void { const material = mesh.material if (!(material instanceof MeshBasicMaterial)) { return } - if (isHovered) { - material.color.set( - packRgbToColor(SKETCH_SELECTION_RGB.map((val) => Math.round(val * 0.7))) - ) - } else if (isSelected) { - material.color.set(SKETCH_SELECTION_COLOR) - } else if (isDraft) { - // Draft segments are grey (0x888888) - material.color.set(0x888888) - } else { - material.color.set(KCL_DEFAULT_COLOR) - } + const color = getSegmentColor({ + isDraft, + isHovered, + isSelected, + freedom, + }) + material.color.set(color) } /** @@ -587,6 +621,9 @@ class ArcSegment implements SketchEntityUtils { segmentGroup.add(mesh) + // Store freedom in userData + segmentGroup.userData.freedom = args.freedom ?? null + this.update({ input: input, theme: theme, @@ -595,6 +632,7 @@ class ArcSegment implements SketchEntityUtils { group: segmentGroup, selectedIds: [], isDraft: args.isDraft, + freedom: args.freedom, }) return segmentGroup @@ -641,7 +679,18 @@ class ArcSegment implements SketchEntityUtils { const isSelected = selectedIds.includes(id) // Check if this segment is currently hovered (stored in userData) const isHovered = arcSegmentBody.userData.isHovered === true - this.updateArcColors(arcSegmentBody, isSelected, isHovered, isDraft) + // Get freedom from args or group userData + const freedom = args.freedom ?? group.userData.freedom ?? null + // Update userData for consistency + group.userData.freedom = freedom + + this.updateArcColors( + arcSegmentBody, + isSelected, + isHovered, + isDraft, + freedom + ) } } @@ -674,6 +723,7 @@ export function updateSegmentHover( const isSelected = selectedIds.includes(segmentId) const isDraft = draftEntityIds?.includes(segmentId) ?? false + const freedom = group.userData.freedom ?? null // Dispatch based on segment body type if (mesh.userData.type === STRAIGHT_SEGMENT_BODY) { @@ -681,14 +731,16 @@ export function updateSegmentHover( mesh, isSelected, isHovered, - isDraft + isDraft, + freedom ) } else if (mesh.userData.type === ARC_SEGMENT_BODY) { segmentUtilsMap.ArcSegment.updateArcColors( mesh, isSelected, isHovered, - isDraft + isDraft, + freedom ) } } diff --git a/src/machines/sketchSolve/segmentsUtils.ts b/src/machines/sketchSolve/segmentsUtils.ts new file mode 100644 index 00000000000..420edb1c552 --- /dev/null +++ b/src/machines/sketchSolve/segmentsUtils.ts @@ -0,0 +1,187 @@ +import type { Freedom, ApiObject } from '@rust/kcl-lib/bindings/FrontendApi' +import { + packRgbToColor, + SKETCH_SELECTION_COLOR, + SKETCH_SELECTION_RGB, +} from '@src/lib/constants' + +// TODO get this from theme or CSS? +const TEXT_COLOR = 0xffffff + +// Brand blue for unconstrained segments - KCL_DEFAULT_COLOR is "#3c73ff" which is 0x3c73ff +const UNCONSTRAINED_COLOR = 0x3c73ff + +// Conflict color - red +// A softer, more pinkish-red with a hint of orange. For example: "#ff5e5b" (~coral red) +const CONFLICT_COLOR = 0xff5e5b + +/** + * Derives segment freedom from point freedom. + * A segment is considered fully constrained (Fixed) only if all its points are Fixed. + * If any point is Conflict, the segment is Conflict. + * If any point is Free, the segment is Free. + */ +export function deriveSegmentFreedom( + segment: ApiObject, + objects: Array +): Freedom | null { + if (segment.kind.type !== 'Segment') { + return null + } + + const getObjById = (id?: number) => + typeof id === 'number' ? (objects.find((o) => o?.id === id) ?? null) : null + + const segmentData = segment.kind.segment + + if (segmentData.type === 'Point') { + // Points have freedom directly + return segmentData.freedom ?? null + } + + // For segments, we need to check all their points + const pointFreedoms: Array = [] + + if (segmentData.type === 'Line') { + const startPoint = getObjById(segmentData.start) + const endPoint = getObjById(segmentData.end) + if ( + startPoint?.kind?.type === 'Segment' && + startPoint.kind.segment.type === 'Point' + ) { + pointFreedoms.push(startPoint.kind.segment.freedom ?? null) + } + if ( + endPoint?.kind?.type === 'Segment' && + endPoint.kind.segment.type === 'Point' + ) { + pointFreedoms.push(endPoint.kind.segment.freedom ?? null) + } + } else if (segmentData.type === 'Arc') { + const startPoint = getObjById(segmentData.start) + const endPoint = getObjById(segmentData.end) + const centerPoint = getObjById(segmentData.center) + if ( + startPoint?.kind?.type === 'Segment' && + startPoint.kind.segment.type === 'Point' + ) { + pointFreedoms.push(startPoint.kind.segment.freedom ?? null) + } + if ( + endPoint?.kind?.type === 'Segment' && + endPoint.kind.segment.type === 'Point' + ) { + pointFreedoms.push(endPoint.kind.segment.freedom ?? null) + } + if ( + centerPoint?.kind?.type === 'Segment' && + centerPoint.kind.segment.type === 'Point' + ) { + pointFreedoms.push(centerPoint.kind.segment.freedom ?? null) + } + } else if (segmentData.type === 'Circle') { + // Circle has a start point (center) - need to check if there are other points + // For now, just check the start point + const startPoint = getObjById(segmentData.start) + if ( + startPoint?.kind?.type === 'Segment' && + startPoint.kind.segment.type === 'Point' + ) { + pointFreedoms.push(startPoint.kind.segment.freedom ?? null) + } + } + + // Filter out nulls + const validFreedoms = pointFreedoms.filter((f): f is Freedom => f !== null) + + if (validFreedoms.length === 0) { + return null + } + + // Merge freedoms: Conflict > Free > Fixed + // A segment is Fixed only if ALL points are Fixed + let hasConflict = false + let hasFree = false + let allFixed = true + + for (const freedom of validFreedoms) { + if (freedom === 'Conflict') { + hasConflict = true + allFixed = false + } else if (freedom === 'Free') { + hasFree = true + allFixed = false + } + } + + if (hasConflict) { + return 'Conflict' + } + if (hasFree) { + return 'Free' + } + if (allFixed) { + return 'Fixed' + } + + return null +} + +/** + * Color precedence system: + * 1. Draft color (priority 1) - grey + * 2. Hover color (priority 2) - lighter version of selection color + * 3. Select color (priority 3) - SKETCH_SELECTION_COLOR + * 4. Conflict color (priority 4) - CONFLICT_COLOR (red) + * 5. Constrained color (priority 5) - TEXT_COLOR (white) + * 6. Unconstrained color (priority 6) - UNCONSTRAINED_COLOR (brand blue) + * 7. Default color (lowest priority) - UNCONSTRAINED_COLOR + */ +export function getSegmentColor({ + isDraft, + isHovered, + isSelected, + freedom, +}: { + isDraft?: boolean + isHovered?: boolean + isSelected?: boolean + freedom?: Freedom | null +}): number { + // Priority 1: Draft color + if (isDraft) { + return 0x888888 // Grey for draft + } + + // Priority 2: Hover color + if (isHovered) { + // Lighter version of selection color (70% brightness) + const hoverColor = packRgbToColor( + SKETCH_SELECTION_RGB.map((val) => Math.round(val * 0.7)) + ) + return hoverColor + } + + // Priority 3: Select color + if (isSelected) { + return SKETCH_SELECTION_COLOR + } + + // Priority 4: Conflict color (red) + if (freedom === 'Conflict') { + return CONFLICT_COLOR + } + + // Priority 5: Unconstrained color (blue) + if (freedom === 'Free') { + return UNCONSTRAINED_COLOR + } + + // Priority 6: Constrained color (white) - Fixed or null (default to constrained) + if (freedom === 'Fixed') { + return TEXT_COLOR + } + + // Default: unconstrained color (blue) for null/unknown + return UNCONSTRAINED_COLOR +} diff --git a/src/machines/sketchSolve/sketchSolveImpl.ts b/src/machines/sketchSolve/sketchSolveImpl.ts index 2da69bb43ed..5bdcc94ebae 100644 --- a/src/machines/sketchSolve/sketchSolveImpl.ts +++ b/src/machines/sketchSolve/sketchSolveImpl.ts @@ -4,6 +4,7 @@ import type { SceneGraphDelta, SegmentCtor, SourceDelta, + Freedom, } from '@rust/kcl-lib/bindings/FrontendApi' import { segmentUtilsMap, @@ -47,6 +48,7 @@ import { ARC_SEGMENT_BODY, } from '@src/clientSideScene/sceneConstants' import { jsAppSettings } from '@src/lib/settings/settingsUtils' +import { deriveSegmentFreedom } from '@src/machines/sketchSolve/segmentsUtils' export type EquipTool = keyof typeof equipTools @@ -251,6 +253,7 @@ export function updateSegmentGroup({ scale, theme, draftEntityIds, + objects, }: { group: Group input: SegmentCtor @@ -258,6 +261,7 @@ export function updateSegmentGroup({ scale: number theme: Themes draftEntityIds?: Array + objects?: Array }): void { const idNum = Number(group.name) if (Number.isNaN(idNum)) { @@ -266,6 +270,18 @@ export function updateSegmentGroup({ const isDraft = draftEntityIds?.includes(idNum) ?? false + // Derive freedom from segment freedom + let freedomResult: Freedom | null = null + if (objects) { + const segmentObj = objects[idNum] + if (segmentObj) { + freedomResult = deriveSegmentFreedom(segmentObj, objects) + } + } + + // Store freedom in userData for immediate use (not as a cache - Rust handles that) + group.userData.freedom = freedomResult + if (input.type === 'Point') { segmentUtilsMap.PointSegment.update({ input, @@ -275,6 +291,7 @@ export function updateSegmentGroup({ group, selectedIds, isDraft, + freedom: freedomResult, }) } else if (input.type === 'Line') { segmentUtilsMap.LineSegment.update({ @@ -285,6 +302,7 @@ export function updateSegmentGroup({ group, selectedIds, isDraft, + freedom: freedomResult, }) } else if (input.type === 'Arc') { segmentUtilsMap.ArcSegment.update({ @@ -295,6 +313,7 @@ export function updateSegmentGroup({ group, selectedIds, isDraft, + freedom: freedomResult, }) } } @@ -309,13 +328,24 @@ function initSegmentGroup({ scale, id, isDraft, + objects, }: { input: SegmentCtor theme: Themes scale: number id: number isDraft?: boolean + objects?: Array }): Group | Error { + // Derive freedom from segment freedom + let freedomResult: Freedom | null = null + if (objects) { + const segmentObj = objects[id] + if (segmentObj) { + freedomResult = deriveSegmentFreedom(segmentObj, objects) + } + } + let group if (input.type === 'Point') { group = segmentUtilsMap.PointSegment.init({ @@ -324,6 +354,7 @@ function initSegmentGroup({ scale, id, isDraft, + freedom: freedomResult, }) } else if (input.type === 'Line') { group = segmentUtilsMap.LineSegment.init({ @@ -332,6 +363,7 @@ function initSegmentGroup({ scale, id, isDraft, + freedom: freedomResult, }) } else if (input.type === 'Arc') { group = segmentUtilsMap.ArcSegment.init({ @@ -340,9 +372,14 @@ function initSegmentGroup({ scale, id, isDraft, + freedom: freedomResult, }) } - if (group instanceof Group) return group + if (group instanceof Group) { + // Store freedom in userData for immediate use (not as a cache - Rust handles that) + group.userData.freedom = freedomResult + return group + } return new Error(`Unknown input type: ${(input as any).type}`) } @@ -432,6 +469,7 @@ export function updateSceneGraphFromDelta({ scale: factor, id: obj.id, isDraft, + objects, }) if (newGroup instanceof Error) { console.error('Failed to init segment group for object', obj.id) @@ -469,6 +507,7 @@ export function updateSceneGraphFromDelta({ scale: factor, theme: context.sceneInfra.theme, draftEntityIds, + objects, }) }) } @@ -650,6 +689,7 @@ export function refreshSelectionStyling({ context }: SolveActionArgs) { scale: factor, theme: context.sceneInfra.theme, draftEntityIds, + objects, }) }) } @@ -823,7 +863,6 @@ export async function deleteDraftEntitiesPromise({ segmentIds, await jsAppSettings(context.rustContext.settingsActor) ) - console.log('result', result) // return result || null