From 68e08d55803fa0a51ed57371444e93d252ed274f Mon Sep 17 00:00:00 2001 From: Nicholas Boone Date: Fri, 2 Oct 2026 13:36:24 -0700 Subject: [PATCH] Choose automatic planes for parallel-face distance annotations --- docs/kcl-std/functions/std-gdt-distance.md | 8 +- .../expected-bindings/ts-rs/StdLibCommands.ts | 8 +- rust/kcl-lib/src/std/gdt.rs | 328 ++++++++++++++++-- rust/kcl-lib/std/gdt.kcl | 13 +- src/lang/modifyAst/faces.ts | 4 +- src/lang/modifyAst/gdt.ts | 59 +++- .../modifyAst/gdtDistance.integration.spec.ts | 84 +++++ .../modelingCommandCodemods.ts | 1 + .../modelingCommandConfig.integration.spec.ts | 36 ++ .../modelingCommandConfig.ts | 24 +- .../modelingCommandStdLib.ts | 2 +- src/lib/gdtFramePosition.test.ts | 300 ++++++++++++++++ src/lib/gdtFramePosition.ts | 101 +++++- src/lib/operations.spec.ts | 19 + src/lib/operations.ts | 8 +- 15 files changed, 940 insertions(+), 55 deletions(-) create mode 100644 src/lang/modifyAst/gdtDistance.integration.spec.ts diff --git a/docs/kcl-std/functions/std-gdt-distance.md b/docs/kcl-std/functions/std-gdt-distance.md index f91865cf4de..4c79d36c8ee 100644 --- a/docs/kcl-std/functions/std-gdt-distance.md +++ b/docs/kcl-std/functions/std-gdt-distance.md @@ -29,12 +29,12 @@ This is part of model-based definition (MBD). | Name | Type | Description | Required | |----------|------|-------------|----------| | `tolerance` | [`number(Length)`](/docs/kcl-std/types/std-types-number) | The acceptable distance tolerance. If not given, or 0, tolerance will not be shown. | No | -| `from` | [`Face`](/docs/kcl-std/types/std-types-Face) or [`TaggedFace`](/docs/kcl-std/types/std-types-TaggedFace) or [`Edge`](/docs/kcl-std/types/std-types-Edge) or [`any`](/docs/kcl-std/types/std-types-any) | The face or edge to measure from. Must be used with `to`. The default position is the entity center. Edge specifier objects (`{ sideFaces = [...], endFaces? = [...], index? = 0 }`) are experimental; do not use them in generated or user-facing KCL yet. | No | -| `to` | [`Face`](/docs/kcl-std/types/std-types-Face) or [`TaggedFace`](/docs/kcl-std/types/std-types-TaggedFace) or [`Edge`](/docs/kcl-std/types/std-types-Edge) or [`any`](/docs/kcl-std/types/std-types-any) | The face or edge to measure to. Must be used with `from`. The default position is the entity center. Edge specifier objects (`{ sideFaces = [...], endFaces? = [...], index? = 0 }`) are experimental; do not use them in generated or user-facing KCL yet. | No | +| `from` | [`Face`](/docs/kcl-std/types/std-types-Face) or [`TaggedFace`](/docs/kcl-std/types/std-types-TaggedFace) or [`Edge`](/docs/kcl-std/types/std-types-Edge) or [`any`](/docs/kcl-std/types/std-types-any) | The face or edge to measure from. Must be used with `to`. Two parallel planar faces measure the perpendicular distance between their infinite supporting planes, even when their centers are offset sideways or their boundaries do not overlap. Coincident supporting planes cannot display a distance annotation. Other entity pairs measure between entity centers. Edge specifier objects (`{ sideFaces = [...], endFaces? = [...], index? = 0 }`) are experimental; do not use them in generated or user-facing KCL yet. | No | +| `to` | [`Face`](/docs/kcl-std/types/std-types-Face) or [`TaggedFace`](/docs/kcl-std/types/std-types-TaggedFace) or [`Edge`](/docs/kcl-std/types/std-types-Edge) or [`any`](/docs/kcl-std/types/std-types-any) | The face or edge to measure to. Must be used with `from`. Parallel planar faces use supporting-plane distance; other entity pairs use their centers. Edge specifier objects (`{ sideFaces = [...], endFaces? = [...], index? = 0 }`) are experimental; do not use them in generated or user-facing KCL yet. | No | | `edges` | [[`Edge`](/docs/kcl-std/types/std-types-Edge) or [`any`](/docs/kcl-std/types/std-types-any); 1+] | The edges whose lengths are annotated. Cannot be combined with `from` or `to`. Edge specifier objects (`{ sideFaces = [...], endFaces? = [...], index? = 0 }`) are experimental; do not use them in generated or user-facing KCL yet. | No | | `precision` | [`number(_)`](/docs/kcl-std/types/std-types-number) | The number of decimal places to display. The default is `3`. Must be greater than or equal to `0` and less than or equal to `9`. | No | -| `framePosition` | [`Point2d`](/docs/kcl-std/types/std-types-Point2d) | The position of the distance label relative to the measured geometry. The default is `[100mm, 100mm]`. | No | -| `framePlane` | [`Plane`](/docs/kcl-std/types/std-types-Plane) | The plane in which to display the distance. The default is `XY`. Other standard planes like `XZ` and `YZ` can also be used. The distance may be displayed in a plane parallel to the given plane. | No | +| `framePosition` | [`Point2d`](/docs/kcl-std/types/std-types-Point2d) | The distance label's local offset `[along, setback]`, in length units. `along` moves the label from the measurement midpoint, positive toward `from` and negative toward `to`; `0` centers it when the text fits. `setback` sets the perpendicular leader extent in `framePlane`; the dimension line lies at 80% of this extent. Its sign chooses the side. When omitted, the label is centered with a setback of 150% of the bounding-box diagonal, using the larger of the measured entities and the whole model. The dimension line lies at 80% of this setback, leaving 20% clearance beyond the diagonal for interior features. If no usable bounds are available, use `[0mm, 20mm]` as a fallback. Point-and-click writes the calculated position as literal lengths. Explicit positions are preserved, including `[0, 0]`. | No | +| `framePlane` | [`Plane`](/docs/kcl-std/types/std-types-Plane) | The plane in which to display the distance. For two parallel planar faces, omitting this chooses a display plane containing their common normal automatically. An explicit plane must contain that direction; its origin does not affect the measured distance. Other entity pairs and edge lengths default to `XY`. Standard planes like `XZ` and `YZ` or a custom plane can be used. The distance may be displayed in a plane parallel to the given plane. | No | | `leaderScale` | [`number(_)`](/docs/kcl-std/types/std-types-number) | Scale of the distance arrows. The default is `1.0`. Must be greater than `0`. | No | | `fontSize` | [`number(Length)`](/docs/kcl-std/types/std-types-number) | The model-space height to use for annotation text. The default is `10mm`. Explicit units are supported; bare numbers use the file's default length unit. This changes the scene size, not the internal raster texture quality. | No | | `annotationName` | [`string`](/docs/kcl-std/types/std-types-string) | Human-friendly name for this annotation in exports and model metadata. This is not displayed visually. | No | diff --git a/rust/kcl-lib/expected-bindings/ts-rs/StdLibCommands.ts b/rust/kcl-lib/expected-bindings/ts-rs/StdLibCommands.ts index 50168886bb4..077f8cd565f 100644 --- a/rust/kcl-lib/expected-bindings/ts-rs/StdLibCommands.ts +++ b/rust/kcl-lib/expected-bindings/ts-rs/StdLibCommands.ts @@ -3058,7 +3058,7 @@ export default { { "name": "from", "ty": "Face | TaggedFace | Edge | any", - "docs": "The face or edge to measure from. Must be used with `to`. The default position is the entity center. Edge specifier objects (`{ sideFaces = [...], endFaces? = [...], index? = 0 }`) are experimental; do not use them in generated or user-facing KCL yet.", + "docs": "The face or edge to measure from. Must be used with `to`. Two parallel planar faces measure the perpendicular distance between their infinite supporting planes, even when their centers are offset sideways or their boundaries do not overlap. Coincident supporting planes cannot display a distance annotation. Other entity pairs measure between entity centers. Edge specifier objects (`{ sideFaces = [...], endFaces? = [...], index? = 0 }`) are experimental; do not use them in generated or user-facing KCL yet.", "required": false, "special": false, "experimental": false, @@ -3070,7 +3070,7 @@ export default { { "name": "to", "ty": "Face | TaggedFace | Edge | any", - "docs": "The face or edge to measure to. Must be used with `from`. The default position is the entity center. Edge specifier objects (`{ sideFaces = [...], endFaces? = [...], index? = 0 }`) are experimental; do not use them in generated or user-facing KCL yet.", + "docs": "The face or edge to measure to. Must be used with `from`. Parallel planar faces use supporting-plane distance; other entity pairs use their centers. Edge specifier objects (`{ sideFaces = [...], endFaces? = [...], index? = 0 }`) are experimental; do not use them in generated or user-facing KCL yet.", "required": false, "special": false, "experimental": false, @@ -3106,7 +3106,7 @@ export default { { "name": "framePosition", "ty": "Point2d", - "docs": "The position of the distance label relative to the measured geometry. The default is `[100mm, 100mm]`.", + "docs": "The distance label's local offset `[along, setback]`, in length units. `along` moves the label from the measurement midpoint, positive toward `from` and negative toward `to`; `0` centers it when the text fits. `setback` sets the perpendicular leader extent in `framePlane`; the dimension line lies at 80% of this extent. Its sign chooses the side. When omitted, the label is centered with a setback of 150% of the bounding-box diagonal, using the larger of the measured entities and the whole model. The dimension line lies at 80% of this setback, leaving 20% clearance beyond the diagonal for interior features. If no usable bounds are available, use `[0mm, 20mm]` as a fallback. Point-and-click writes the calculated position as literal lengths. Explicit positions are preserved, including `[0, 0]`.", "required": false, "special": false, "experimental": false, @@ -3118,7 +3118,7 @@ export default { { "name": "framePlane", "ty": "Plane", - "docs": "The plane in which to display the distance. The default is `XY`. Other standard planes like `XZ` and `YZ` can also be used. The distance may be displayed in a plane parallel to the given plane.", + "docs": "The plane in which to display the distance. For two parallel planar faces, omitting this chooses a display plane containing their common normal automatically. An explicit plane must contain that direction; its origin does not affect the measured distance. Other entity pairs and edge lengths default to `XY`. Standard planes like `XZ` and `YZ` or a custom plane can be used. The distance may be displayed in a plane parallel to the given plane.", "required": false, "special": false, "experimental": false, diff --git a/rust/kcl-lib/src/std/gdt.rs b/rust/kcl-lib/src/std/gdt.rs index d6822746f73..b6e3bde2850 100644 --- a/rust/kcl-lib/src/std/gdt.rs +++ b/rust/kcl-lib/src/std/gdt.rs @@ -1,6 +1,7 @@ use kcl_error::SourceRange; use kcmc::ModelingCmd; use kcmc::each_cmd as mcmd; +use kittycad_modeling_cmds::ok_response::OkModelingCmdResponse; use kittycad_modeling_cmds::shared::AnnotationBasicDimension; use kittycad_modeling_cmds::shared::AnnotationFeatureControl; use kittycad_modeling_cmds::shared::AnnotationFeatureTag; @@ -12,6 +13,8 @@ use kittycad_modeling_cmds::shared::AnnotationOptions; use kittycad_modeling_cmds::shared::AnnotationType; use kittycad_modeling_cmds::shared::MbdSymbol; use kittycad_modeling_cmds::shared::Point2d as KPoint2d; +use kittycad_modeling_cmds::units::UnitLength; +use kittycad_modeling_cmds::websocket::OkWebSocketResponseData; use kittycad_modeling_cmds::{self as kcmc}; use crate::ExecState; @@ -27,6 +30,9 @@ use crate::execution::GdtAnnotationArtifact; use crate::execution::Metadata; use crate::execution::ModelingCmdMeta; use crate::execution::Plane; +use crate::execution::PlaneInfo; +use crate::execution::PlaneKind; +use crate::execution::Point3d; use crate::execution::TagIdentifier; use crate::execution::types::ArrayLen; use crate::execution::types::RuntimeType; @@ -989,19 +995,6 @@ async fn inner_distance( args: &Args, ) -> Result, KclError> { let precision = resolve_precision(precision, args)?; - let mut frame_plane = if let Some(plane) = frame_plane { - plane - } else { - xy_plane(exec_state, args).await? - }; - ensure_sketch_plane_in_engine( - &mut frame_plane, - exec_state, - &args.ctx, - args.source_range, - args.node_path.clone(), - ) - .await?; if from.is_some() || to.is_some() { if !edges.is_empty() { @@ -1018,8 +1011,11 @@ async fn inner_distance( ))); }; + let planar_face_pair = matches!(from, DistanceEntity::Face(_) | DistanceEntity::TaggedFace(_)) + && matches!(to, DistanceEntity::Face(_) | DistanceEntity::TaggedFace(_)); let from = from.to_endpoint(exec_state, args).await?; let to = to.to_endpoint(exec_state, args).await?; + let frame_plane = distance_frame_plane(frame_plane, planar_face_pair, &from, &to, exec_state, args).await?; let mut annotations = Vec::with_capacity(1); create_basic_distance_annotation( from, @@ -1045,6 +1041,20 @@ async fn inner_distance( ))); } + let mut frame_plane = if let Some(plane) = frame_plane { + plane + } else { + xy_plane(exec_state, args).await? + }; + ensure_sketch_plane_in_engine( + &mut frame_plane, + exec_state, + &args.ctx, + args.source_range, + args.node_path.clone(), + ) + .await?; + let mut annotations = Vec::with_capacity(edges.len()); for edge in &edges { let (entity_id, edge_reference) = match edge { @@ -1081,6 +1091,178 @@ async fn inner_distance( Ok(annotations) } +// Keep this angular tolerance in sync with the engine's parallelPlaneDimensionPositions. +const DISTANCE_PARALLEL_TOLERANCE: f64 = 1e-9; + +fn parallel_face_frame( + from: &kcmc::ok_response::output::FaceIsPlanar, + to: &kcmc::ok_response::output::FaceIsPlanar, +) -> Option { + let (a, b, normal, other_normal) = (from.origin?, to.origin?, from.z_axis?, to.z_axis?); + let mut normal = nalgebra_glm::vec3(normal.x, normal.y, normal.z).try_normalize(0.0)?; + let other_normal = nalgebra_glm::vec3(other_normal.x, other_normal.y, other_normal.z).try_normalize(0.0)?; + if !normal.iter().chain(other_normal.iter()).all(|value| value.is_finite()) + || normal.cross(&other_normal).norm() > DISTANCE_PARALLEL_TOLERANCE + { + return None; + } + // A canonical sign keeps the display orientation independent of face selection order. + let axis = (0..3).max_by(|&a, &b| normal[a].abs().total_cmp(&normal[b].abs()))?; + if normal[axis] < 0.0 { + normal = -normal; + } + let axis = (0..3).min_by(|&a, &b| normal[a].abs().total_cmp(&normal[b].abs()))?; + let mut tangent = nalgebra_glm::DVec3::zeros(); + tangent[axis] = 1.0; + let tangent = (tangent - normal * tangent.dot(&normal)).try_normalize(0.0)?; + let z_axis = normal.cross(&tangent); + let direction = |v: nalgebra_glm::DVec3| Point3d { + x: v.x, + y: v.y, + z: v.z, + units: None, + }; + Some(PlaneInfo { + origin: Point3d { + x: (a.x.0 + b.x.0) / 2.0, + y: (a.y.0 + b.y.0) / 2.0, + z: (a.z.0 + b.z.0) / 2.0, + units: Some(kcl_api::UnitLength::Millimeters), + }, + x_axis: direction(normal), + y_axis: direction(tangent), + z_axis: direction(z_axis), + }) +} + +async fn planar_face( + face_id: uuid::Uuid, + exec_state: &mut ExecState, + args: &Args, +) -> Result { + let id = exec_state.next_uuid(); + let response = exec_state + .send_modeling_cmd( + ModelingCmdMeta::from_args_id(exec_state, args, id), + mcmd::FaceIsPlanar::builder().object_id(face_id).build().into(), + ) + .await?; + match response { + OkWebSocketResponseData::Modeling { + modeling_response: OkModelingCmdResponse::FaceIsPlanar(face), + } => Ok(face), + _ => Err(KclError::new_engine(KclErrorDetails::new( + "Expected FaceIsPlanar response for distance annotation.".to_owned(), + vec![args.source_range], + ))), + } +} + +async fn distance_frame_plane( + explicit: Option, + planar_face_pair: bool, + from: &DistanceEndpoint, + to: &DistanceEndpoint, + exec_state: &mut ExecState, + args: &Args, +) -> Result { + let automatic = if planar_face_pair && !args.ctx.no_engine_commands().await { + if let (Some(a), Some(b)) = (from.entity_id, to.entity_id) { + let a = planar_face(a, exec_state, args).await?; + let b = planar_face(b, exec_state, args).await?; + parallel_face_frame(&a, &b) + } else { + None + } + } else { + None + }; + let mut plane = match (explicit, automatic) { + (Some(plane), Some(info)) => { + if !distance_plane_contains_direction(&plane.info, &info.x_axis) { + return Err(KclError::new_semantic(KclErrorDetails::new( + "framePlane must contain the direction perpendicular to the measured faces. Omit framePlane to choose it automatically.".to_owned(), + vec![args.source_range], + ))); + } + plane + } + (Some(plane), None) => plane, + (None, Some(info)) => { + let id = exec_state.next_uuid(); + Plane { + id, + artifact_id: id.into(), + object_id: None, + kind: PlaneKind::Custom, + info, + meta: vec![Metadata::from(args.source_range)], + } + } + (None, None) => xy_plane(exec_state, args).await?, + }; + ensure_sketch_plane_in_engine( + &mut plane, + exec_state, + &args.ctx, + args.source_range, + args.node_path.clone(), + ) + .await?; + Ok(plane) +} + +fn distance_plane_contains_direction(plane: &PlaneInfo, direction: &Point3d) -> bool { + let normal = plane.x_axis.axes_cross_product(&plane.y_axis); + let dot = normal.axes_dot_product(direction); + let length = normal.axes_dot_product(&normal).sqrt(); + dot.is_finite() && length > 0.0 && dot.abs() / length <= DISTANCE_PARALLEL_TOLERANCE +} + +fn distance_setback(dimensions: [f64; 3]) -> Option { + dimensions + .into_iter() + .filter(|value| value.is_finite() && *value > 0.0) + .reduce(f64::hypot) + .map(|diagonal| diagonal * 1.5) +} + +async fn default_distance_setback( + from: Option, + to: Option, + exec_state: &mut ExecState, + args: &Args, +) -> f64 { + let mut entity_ids: Vec<_> = from.into_iter().chain(to).collect(); + entity_ids.dedup(); + let mut setback: Option = None; + loop { + let id = exec_state.next_uuid(); + let response = exec_state + .send_modeling_cmd( + ModelingCmdMeta::from_args_id(exec_state, args, id), + mcmd::BoundingBox::builder() + .entity_ids(entity_ids.clone()) + .output_unit(UnitLength::Millimeters) + .build() + .into(), + ) + .await; + if let Ok(OkWebSocketResponseData::Modeling { + modeling_response: OkModelingCmdResponse::BoundingBox(bounds), + }) = response + && let Some(bound_setback) = + distance_setback([bounds.dimensions.x, bounds.dimensions.y, bounds.dimensions.z]) + { + setback = Some(setback.unwrap_or(0.0).max(bound_setback)); + } + if entity_ids.is_empty() { + return setback.unwrap_or(20.0); + } + entity_ids.clear(); + } +} + #[allow(clippy::too_many_arguments)] async fn create_basic_distance_annotation( from: DistanceEndpoint, @@ -1098,6 +1280,17 @@ async fn create_basic_distance_annotation( let meta = vec![Metadata::from(args.source_range)]; let annotation_id = exec_state.next_uuid(); let display_units = exec_state.length_unit(); + let offset = if let Some(offset) = frame_position { + KPoint2d { + x: offset[0].to_mm(), + y: offset[1].to_mm(), + } + } else { + KPoint2d { + x: 0.0, + y: default_distance_setback(from.entity_id, to.entity_id, exec_state, args).await, + } + }; let dimension = AnnotationBasicDimension::builder() .maybe_from_entity_id(from.entity_id) .maybe_from_edge_reference(from.edge_reference) @@ -1116,14 +1309,7 @@ async fn create_basic_distance_annotation( .build(), ) .plane_id(frame_plane_id) - .offset(if let Some(offset) = frame_position { - KPoint2d { - x: offset[0].to_mm(), - y: offset[1].to_mm(), - } - } else { - KPoint2d { x: 100.0, y: 100.0 } - }) + .offset(offset) .precision(precision) .font_scale(gdt_font_scale(font_size, args)?) .font_point_size(GDT_FONT_TEXTURE_POINT_SIZE) @@ -1784,6 +1970,57 @@ mod tests { use crate::execution::MockConfig; use crate::execution::parse_execute; + fn planar_response(origin: [f64; 3], normal: [f64; 3]) -> kcmc::ok_response::output::FaceIsPlanar { + serde_json::from_value(serde_json::json!({ + "origin": { "x": origin[0], "y": origin[1], "z": origin[2] }, + "z_axis": { "x": normal[0], "y": normal[1], "z": normal[2] } + })) + .unwrap() + } + + #[test] + fn parallel_face_distance_frame_contains_measurement_direction() { + for normal in [[0.0, 0.0, 1.0], [1.0, 2.0, 3.0], [0.0, 1.0, 0.0]] { + let a = planar_response([0.0, 0.0, 0.0], normal); + let b = planar_response([12.0, 0.0, 5.0], normal.map(|value| -value)); + let plane = parallel_face_frame(&a, &b).unwrap(); + let reversed = parallel_face_frame(&b, &a).unwrap(); + assert_eq!(plane, reversed); + assert!((plane.x_axis.axes_dot_product(&plane.x_axis) - 1.0).abs() < 1e-12); + assert!(plane.x_axis.axes_dot_product(&plane.z_axis).abs() < 1e-12); + assert!(plane.y_axis.axes_dot_product(&plane.x_axis).abs() < 1e-12); + assert_eq!( + plane.origin, + Point3d::new(6.0, 0.0, 2.5, Some(kcl_api::UnitLength::Millimeters)) + ); + } + } + + #[test] + fn distance_frame_preserves_nonparallel_and_nonplanar_behavior() { + let a = planar_response([0.0; 3], [0.0, 0.0, 1.0]); + let b = planar_response([0.0; 3], [1.0, 0.0, 0.0]); + assert!(parallel_face_frame(&a, &b).is_none()); + let nonplanar = serde_json::from_value(serde_json::json!({})).unwrap(); + assert!(parallel_face_frame(&a, &nonplanar).is_none()); + let invalid = planar_response([0.0; 3], [0.0; 3]); + assert!(parallel_face_frame(&a, &invalid).is_none()); + } + + #[test] + fn explicit_distance_plane_must_contain_face_normal() { + let a = planar_response([0.0; 3], [0.0, 0.0, 1.0]); + let b = planar_response([12.0, 0.0, 5.0], [0.0, 0.0, -1.0]); + let mut plane = parallel_face_frame(&a, &b).unwrap(); + let direction = plane.x_axis.clone(); + assert!(distance_plane_contains_direction(&plane, &direction)); + plane.origin = Point3d::new(100.0, 200.0, 300.0, Some(kcl_api::UnitLength::Millimeters)); + assert!(distance_plane_contains_direction(&plane, &direction)); + plane.x_axis = Point3d::new(1.0, 0.0, 0.0, None); + plane.y_axis = Point3d::new(0.0, 1.0, 0.0, None); + assert!(!distance_plane_contains_direction(&plane, &direction)); + } + const GDT_DISTANCE_KCL_TEMPLATE: &str = r#" @settings(defaultLengthUnit = __UNIT__, kclVersion = 2) @@ -2162,6 +2399,55 @@ gdt::flatness( Ok(()) } + #[test] + fn distance_setback_clears_the_bounding_box_diagonal() { + assert_eq!(distance_setback([30.0, 40.0, 0.0]), Some(75.0)); + assert_eq!(distance_setback([0.0, 0.0, 40.0]), Some(60.0)); + assert_eq!(distance_setback([f64::NAN, -1.0, 10.0]), Some(15.0)); + assert_eq!(distance_setback([0.0, f64::INFINITY, -1.0]), None); + } + + #[tokio::test(flavor = "multi_thread")] + async fn gdt_distance_default_position_has_a_physical_fallback_without_bounds() -> Result<(), KclError> { + for (unit, font_size, expected_setback) in [ + ("mm", "", 20.0), + ("in", "", 20.0), + ("mm", "fontSize = 2mm,", 20.0), + ("cm", "fontSize = 0.2,", 20.0), + ("in", "fontSize = 0.1in,", 20.0), + ] { + for between_faces in [false, true] { + let mut code = gdt_distance_kcl(unit, "0mm", "[0, 0]") + .replace(" framePosition = [0, 0],\n", "") + .replace("fontSize = 2in,", font_size); + if between_faces { + code = code.replace( + "edges = [\n getCommonEdge(faces = [\n region001.tags.line4,\n region001.tags.line1\n ])\n ]", + "from = region001.tags.line4, to = region001.tags.line2", + ); + } + let commands = gdt_commands(&code).await; + let index = new_annotation_command_index(&commands)?; + let dimension = annotation_options(&commands[index])?.dimension.as_ref().unwrap(); + assert_close(dimension.offset.x, 0.0); + assert_close(dimension.offset.y, expected_setback); + assert_eq!(dimension.from_entity_id != dimension.to_entity_id, between_faces); + } + } + Ok(()) + } + + #[tokio::test(flavor = "multi_thread")] + async fn gdt_distance_preserves_explicit_zero_position() -> Result<(), KclError> { + let code = gdt_distance_kcl("in", "0mm", "[0, 0]"); + let commands = gdt_commands(&code).await; + let index = new_annotation_command_index(&commands)?; + let dimension = annotation_options(&commands[index])?.dimension.as_ref().unwrap(); + assert_close(dimension.offset.x, 0.0); + assert_close(dimension.offset.y, 0.0); + Ok(()) + } + const GDT_FACE_API_EDGE_KCL_TEMPLATE: &str = r#" @settings(defaultLengthUnit = mm, kclVersion = 2) diff --git a/rust/kcl-lib/std/gdt.kcl b/rust/kcl-lib/std/gdt.kcl index 0c090859acf..45ab7b28277 100644 --- a/rust/kcl-lib/std/gdt.kcl +++ b/rust/kcl-lib/std/gdt.kcl @@ -1319,9 +1319,9 @@ export fn distance( /// If not given, or 0, tolerance will not be shown. @(includeInSnippet = true) tolerance?: number(Length) = 0mm, - /// The face or edge to measure from. Must be used with `to`. The default position is the entity center. Edge specifier objects (`{ sideFaces = [...], endFaces? = [...], index? = 0 }`) are experimental; do not use them in generated or user-facing KCL yet. + /// The face or edge to measure from. Must be used with `to`. Two parallel planar faces measure the perpendicular distance between their infinite supporting planes, even when their centers are offset sideways or their boundaries do not overlap. Coincident supporting planes cannot display a distance annotation. Other entity pairs measure between entity centers. Edge specifier objects (`{ sideFaces = [...], endFaces? = [...], index? = 0 }`) are experimental; do not use them in generated or user-facing KCL yet. from?: Face | TaggedFace | Edge | any, - /// The face or edge to measure to. Must be used with `from`. The default position is the entity center. Edge specifier objects (`{ sideFaces = [...], endFaces? = [...], index? = 0 }`) are experimental; do not use them in generated or user-facing KCL yet. + /// The face or edge to measure to. Must be used with `from`. Parallel planar faces use supporting-plane distance; other entity pairs use their centers. Edge specifier objects (`{ sideFaces = [...], endFaces? = [...], index? = 0 }`) are experimental; do not use them in generated or user-facing KCL yet. to?: Face | TaggedFace | Edge | any, /// The edges whose lengths are annotated. Cannot be combined with `from` or `to`. Edge specifier objects (`{ sideFaces = [...], endFaces? = [...], index? = 0 }`) are experimental; do not use them in generated or user-facing KCL yet. @(includeInSnippet = true) @@ -1329,10 +1329,13 @@ export fn distance( /// The number of decimal places to display. The default is `3`. Must be greater than or equal to `0` and less than or equal to `9`. @(includeInSnippet = true) precision?: number(Count), - /// The position of the distance label relative to the measured geometry. The default is `[100mm, 100mm]`. - @(snippetArray = ["100", "100"]) + /// The distance label's local offset `[along, setback]`, in length units. + /// `along` moves the label from the measurement midpoint, positive toward `from` and negative toward `to`; `0` centers it when the text fits. + /// `setback` sets the perpendicular leader extent in `framePlane`; the dimension line lies at 80% of this extent. Its sign chooses the side. + /// When omitted, the label is centered with a setback of 150% of the bounding-box diagonal, using the larger of the measured entities and the whole model. The dimension line lies at 80% of this setback, leaving 20% clearance beyond the diagonal for interior features. If no usable bounds are available, use `[0mm, 20mm]` as a fallback. Point-and-click writes the calculated position as literal lengths. Explicit positions are preserved, including `[0, 0]`. + @(snippetArray = ["0mm", "20mm"]) framePosition?: Point2d, - /// The plane in which to display the distance. The default is `XY`. Other standard planes like `XZ` and `YZ` can also be used. The distance may be displayed in a plane parallel to the given plane. + /// The plane in which to display the distance. For two parallel planar faces, omitting this chooses a display plane containing their common normal automatically. An explicit plane must contain that direction; its origin does not affect the measured distance. Other entity pairs and edge lengths default to `XY`. Standard planes like `XZ` and `YZ` or a custom plane can be used. The distance may be displayed in a plane parallel to the given plane. @(includeInSnippet = true) framePlane?: Plane, /// Scale of the distance arrows. The default is `1.0`. Must be greater than `0`. diff --git a/src/lang/modifyAst/faces.ts b/src/lang/modifyAst/faces.ts index 642340b0fcf..2baa96de9c2 100644 --- a/src/lang/modifyAst/faces.ts +++ b/src/lang/modifyAst/faces.ts @@ -303,7 +303,7 @@ export function addDeleteFace({ } } -function getPrimitiveFaceSelectionsFromSelection({ +export function getPrimitiveFaceSelectionsFromSelection({ graphSelections, otherSelections, }: Selections): EnginePrimitiveSelection[] { @@ -1438,7 +1438,7 @@ export function buildSolidsAndFacesExprs( // Adds all the faceId calls needed in the AST so we can refer to them, // keeps track of their names as faces, // and gathers the corresponding solid expressions. -function insertFacePrimitiveVariablesAndOffsetPathToNode({ +export function insertFacePrimitiveVariablesAndOffsetPathToNode({ enginePrimitives, modifiedAst, artifactGraph, diff --git a/src/lang/modifyAst/gdt.ts b/src/lang/modifyAst/gdt.ts index 16060f68fdd..fae0f121484 100644 --- a/src/lang/modifyAst/gdt.ts +++ b/src/lang/modifyAst/gdt.ts @@ -17,8 +17,14 @@ import { import { createEdgeRefObjectExpression, entityReferenceToEdgeRefPayload, + getPrimitiveEdgeSelections, + insertPrimitiveEdgeVariablesAndOffsetPathToNode, } from '@src/lang/modifyAst/edges' -import { isFaceArtifact } from '@src/lang/modifyAst/faces' +import { + getPrimitiveFaceSelectionsFromSelection, + insertFacePrimitiveVariablesAndOffsetPathToNode, + isFaceArtifact, +} from '@src/lang/modifyAst/faces' import { modifyAstWithTagsForSelection } from '@src/lang/modifyAst/tagManagement' import { resolveToCodeRef, traverse, valueOrVariable } from '@src/lang/queryAst' import { @@ -1310,7 +1316,24 @@ export function addDistanceGdt({ resolveToCodeRef(selection, artifactGraph)?.artifact ) ) - if (!mNodeToEdit && targetSelections.length === 0) { + const primitiveEdges = mNodeToEdit + ? [] + : getPrimitiveEdgeSelections(selections) + const primitiveFaces = mNodeToEdit + ? [] + : getPrimitiveFaceSelectionsFromSelection({ + graphSelections: selections.graphSelections.filter( + (selection) => + !targetSelections.includes(selection) && + selection.entityRef?.type !== 'edge' + ), + otherSelections: selections.otherSelections, + }) + if ( + !mNodeToEdit && + targetSelections.length + primitiveEdges.length + primitiveFaces.length === + 0 + ) { return new Error( 'No valid selections found. Select one edge, or exactly two faces or edges.' ) @@ -1344,6 +1367,38 @@ export function addDistanceGdt({ }) } + for (const selection of primitiveEdges) { + const result = insertPrimitiveEdgeVariablesAndOffsetPathToNode({ + primitiveEdgeSelections: [selection], + bodies: new Map(), + modifiedAst, + artifactGraph, + wasmInstance, + }) + if (err(result)) return result + const body = [...result.bodies.values()][0] + const expr = + body?.tagsExpr.type === 'ArrayExpression' + ? body.tagsExpr.elements[0] + : body?.tagsExpr + if (!expr) + return new Error('Could not resolve the selected distance edge in code.') + targets.push({ kind: 'edge', expr }) + } + for (const selection of primitiveFaces) { + const result = insertFacePrimitiveVariablesAndOffsetPathToNode({ + enginePrimitives: [selection], + modifiedAst, + artifactGraph, + wasmInstance, + useLatestBody: true, + }) + if (err(result)) return result + const expr = result.faceExprs[0] + if (!expr) + return new Error('Could not resolve the selected distance face in code.') + targets.push({ kind: 'face', expr }) + } if (targets.length === 0) { return new Error('No valid distance targets could be generated') } diff --git a/src/lang/modifyAst/gdtDistance.integration.spec.ts b/src/lang/modifyAst/gdtDistance.integration.spec.ts new file mode 100644 index 00000000000..5419c2bf00d --- /dev/null +++ b/src/lang/modifyAst/gdtDistance.integration.spec.ts @@ -0,0 +1,84 @@ +import type { KclManager } from '@src/lang/KclManager' +import { createPathToNodeForLastVariable } from '@src/lang/modifyAst' +import { type ArtifactGraph, assertParse, recast } from '@src/lang/wasm' +import { modelingCommandCodemods } from '@src/lib/commandBarConfigs/modelingCommandCodemods' +import type { ConnectionManager } from '@src/lib/engineConnection/connectionManager' +import type { Selections } from '@src/machines/modelingSharedTypes' +import { buildTheWorldAndNoEngineConnection } from '@src/unitTestUtils' +import { describe, expect, it, vi } from 'vitest' + +describe('point-and-click distance annotations', () => { + it.each(['face', 'edge'] as const)( + 'preserves two selected primitive %ss in generated KCL', + async (primitiveType) => { + const { instance } = await buildTheWorldAndNoEngineConnection() + const ast = assertParse( + `@settings(defaultLengthUnit = mm, kclVersion = 2) +profile = sketch(on = XY) { + outer = circle(start = [20mm, 0mm], center = [0mm, 0mm]) +} +plate = extrude(region(segments = [profile.outer]), length = 5mm)`, + instance + ) + const codeRef = { + nodePath: { steps: [] }, + range: [0, 0, 0] as [number, number, number], + pathToNode: createPathToNodeForLastVariable(ast), + } + const artifactGraph: ArtifactGraph = new Map([ + [ + 'plate-body', + { + type: 'sweep', + id: 'plate-body', + subType: 'extrusion', + surfaceIds: [], + edgeIds: [], + trajectoryId: null, + method: 'new', + consumed: false, + codeRef, + }, + ], + ]) + const objects: Selections = { + graphSelections: [], + otherSelections: [1, 2].map((primitiveIndex) => ({ + type: 'enginePrimitive', + primitiveType, + parentEntityId: 'plate-body', + primitiveIndex, + entityId: `selected-${primitiveIndex}`, + })), + } + const sendSceneCommand = vi.fn().mockResolvedValue(undefined) + const result = await modelingCommandCodemods['GDT Distance'].run({ + args: { objects, framePlane: 'Automatic' }, + ast, + kclManager: { + artifactGraph, + code: '', + fileSettings: { defaultLengthUnit: 'mm' }, + engineCommandManager: { + sendSceneCommand, + } as unknown as ConnectionManager, + } as KclManager, + wasmInstance: instance, + }) + if (result instanceof Error) throw result + const code = recast(result.modifiedAst, instance) + expect(code).toContain(`${primitiveType}Id(plate, index = 1)`) + expect(code).toContain(`${primitiveType}Id(plate, index = 2)`) + expect(code).toContain(`from = ${primitiveType}001`) + expect(code).toContain(`to = ${primitiveType}002`) + expect(code).not.toContain('framePlane =') + expect(code).not.toContain('Automatic') + expect(code).not.toContain('tolerance =') + expect(sendSceneCommand).not.toHaveBeenCalledWith( + expect.objectContaining({ + cmd: expect.objectContaining({ type: 'make_plane' }), + }) + ) + } + ) +}) diff --git a/src/lib/commandBarConfigs/modelingCommandCodemods.ts b/src/lib/commandBarConfigs/modelingCommandCodemods.ts index e590b84d2b9..492302cf77b 100644 --- a/src/lib/commandBarConfigs/modelingCommandCodemods.ts +++ b/src/lib/commandBarConfigs/modelingCommandCodemods.ts @@ -246,6 +246,7 @@ const withGdtDefaults = < sourceCode: kclManager.code, outputUnit: kclManager.fileSettings.defaultLengthUnit, wasmInstance, + distance: commandName === 'GDT Distance', }) return add( diff --git a/src/lib/commandBarConfigs/modelingCommandConfig.integration.spec.ts b/src/lib/commandBarConfigs/modelingCommandConfig.integration.spec.ts index 1b4530eefc1..bf1509661b4 100644 --- a/src/lib/commandBarConfigs/modelingCommandConfig.integration.spec.ts +++ b/src/lib/commandBarConfigs/modelingCommandConfig.integration.spec.ts @@ -98,6 +98,42 @@ gdt::datum(face = capEnd001, name = "A")` }) describe('GDT tolerance defaults', () => { + it('lets distance annotations skip tolerance without prepopulating it', () => { + const command = modelingMachineCommandConfig['GDT Distance'] + if (!command || isArray(command)) + throw new Error('Expected distance command') + expect(command.args?.tolerance?.required).toBe(false) + expect(command.args?.tolerance?.prepopulate).toBeFalsy() + expect(command.args?.tolerance?.skip).toBeFalsy() + expect(command.args?.framePlane).toMatchObject({ + defaultValue: 'Automatic', + options: expect.arrayContaining([ + { name: 'Automatic', value: 'Automatic', isCurrent: true }, + ]), + }) + expect(command.args?.objects).toMatchObject({ + selectionTypes: expect.arrayContaining(['enginePrimitiveEdge']), + }) + }) + it('accepts two engine primitive edges as distance targets', () => { + const command = modelingMachineCommandConfig['GDT Distance'] + if (!command || isArray(command)) + throw new Error('Expected distance command') + const objects = command.args?.objects + if (!objects || objects.inputType !== 'selection') + throw new Error('Expected distance selection argument') + expect( + canSubmitSelectionArg( + new Map([['enginePrimitiveEdge', 2]]), + { + inputType: 'selection', + selectionTypes: objects.selectionTypes, + multiple: objects.multiple, + required: true, + } + ) + ).toBe(true) + }) it('uses the current file unit for the tolerance input default', () => { const modelingContext = { kclManager: { diff --git a/src/lib/commandBarConfigs/modelingCommandConfig.ts b/src/lib/commandBarConfigs/modelingCommandConfig.ts index 712f0ac4ff1..c4c346dcd97 100644 --- a/src/lib/commandBarConfigs/modelingCommandConfig.ts +++ b/src/lib/commandBarConfigs/modelingCommandConfig.ts @@ -2013,13 +2013,33 @@ export const modelingMachineCommandConfig: StateMachineCommandSetConfig< overrides: { objects: { inputType: 'selection', - selectionTypes: ['cap', 'wall', 'edgeCut', 'segment', 'sweepEdge'], + selectionTypes: [ + 'cap', + 'wall', + 'edgeCut', + 'segment', + 'sweepEdge', + 'enginePrimitiveEdge', + 'enginePrimitiveFace', + ], multiple: true, required: true, hidden: isEditingNodeSelection, }, - tolerance: gdtToleranceProps, + tolerance: { ...gdtToleranceProps, required: false }, ...gdtFrameArgOverrides, + framePosition: { defaultValue: undefined }, + framePlane: { + inputType: 'options', + defaultValue: 'Automatic', + options: [ + { name: 'Automatic', value: 'Automatic', isCurrent: true }, + ...FRAME_PLANE_OPTIONS.map((option) => ({ + ...option, + isCurrent: false, + })), + ], + }, }, } ), diff --git a/src/lib/commandBarConfigs/modelingCommandStdLib.ts b/src/lib/commandBarConfigs/modelingCommandStdLib.ts index 0dff427a746..4ebac0e5442 100644 --- a/src/lib/commandBarConfigs/modelingCommandStdLib.ts +++ b/src/lib/commandBarConfigs/modelingCommandStdLib.ts @@ -454,7 +454,7 @@ export const modelingCommandStdLibDriftConfig = { 'GDT Distance': { stdLibName: 'gdt::distance', editFlow: true, - flowArgOrder: ['objects', 'tolerance'], + flowArgOrder: ['objects'], omittedStdLibArgs: ['annotationName'], argAliases: { from: 'objects', diff --git a/src/lib/gdtFramePosition.test.ts b/src/lib/gdtFramePosition.test.ts index 60c099c1e7c..a25cc03cebf 100644 --- a/src/lib/gdtFramePosition.test.ts +++ b/src/lib/gdtFramePosition.test.ts @@ -1,4 +1,9 @@ import type { Artifact, Expr } from '@src/lang/wasm' +import { + createBinaryExpression, + createLiteral, + createLocalName, +} from '@src/lang/create' import type { ModelingCommandSchema } from '@src/lib/commandBarConfigs/modelingCommandConfig' import type { KclCommandValue } from '@src/lib/commandTypes' import { @@ -86,6 +91,301 @@ describe('GD&T frame defaults', () => { formatNumberLiteral.mockReturnValue('1.2cm') }) + describe('distance placement', () => { + it.each([undefined, 'Automatic'])( + 'leaves the plane automatic for two selected faces (%s)', + async (framePlane) => { + const sendSceneCommand = vi.fn().mockResolvedValue(undefined) + const objects: Selections = { + graphSelections: ['cap-1', 'cap-2'].map((id) => ({ + codeRef: { range: [0, 1, 0], pathToNode: [] }, + artifact: testArtifact({ type: 'cap', id }), + })), + otherSelections: [], + } + const result = await withDefaultGdtFrameDefaults({ + data: { + objects, + framePlane, + framePosition: testFramePosition(), + fontSize: kclValue('2mm'), + } as ModelingCommandSchema['GDT Distance'], + distance: true, + engineCommandManager: { + sendSceneCommand, + } as unknown as ConnectionManager, + wasmInstance, + }) + expect(result.framePlane).toBeUndefined() + expect(sendSceneCommand).not.toHaveBeenCalled() + expect(result.objects).toBe(objects) + } + ) + + beforeEach(() => { + formatNumberLiteral.mockImplementation( + (value, suffix) => + `${value}${JSON.parse(suffix).toLowerCase().replace('inch', 'in')}` + ) + }) + const objects: Selections = { + graphSelections: [ + { + codeRef: { range: [0, 1, 0], pathToNode: [] }, + artifact: testArtifact({ type: 'cap', id: 'cap-1' }), + }, + ], + otherSelections: [], + } + + it.each(['mm', 'cm', 'in'] as const)( + 'centers the label and scales its setback with the model in %s', + async (outputUnit) => { + formatNumberLiteral.mockImplementation( + (value, suffix) => + `${value}${JSON.parse(suffix).toLowerCase().replace('inch', 'in')}` + ) + const sendSceneCommand = vi.fn().mockResolvedValue({ + success: true, + resp: { + type: 'modeling', + data: { + modeling_response: { + type: 'bounding_box', + data: { + center: { x: 0, y: 0, z: 0 }, + dimensions: { x: 100, y: 50, z: 0 }, + }, + }, + }, + }, + }) + const result = await withDefaultGdtFrameDefaults< + ModelingCommandSchema['GDT Distance'] + >({ + data: { + objects, + framePlane: 'XY', + }, + distance: true, + engineCommandManager: { + sendSceneCommand, + } as unknown as ConnectionManager, + outputUnit, + wasmInstance, + }) + expect(result.fontSize?.valueText).toBe(`5.25${outputUnit}`) + expect(result.framePosition?.valueText).toBe( + `[0mm, 167.7051${outputUnit}]` + ) + expect(result.framePosition?.valueAst).toMatchObject({ + type: 'ArrayExpression', + elements: [ + { type: 'Literal', value: { value: 0, suffix: 'Mm' } }, + { + type: 'Literal', + value: { value: 167.7051 }, + }, + ], + }) + } + ) + + it.each(['failed', 'empty', 'missing selections'])( + 'uses a physical fallback when geometry is %s', + async (scenario) => { + const sendSceneCommand = vi.fn().mockResolvedValue({ + success: true, + resp: { + type: 'modeling', + data: { + modeling_response: { + type: 'bounding_box', + data: { dimensions: { x: 0, y: 0, z: 0 } }, + }, + }, + }, + }) + if (scenario === 'failed') { + sendSceneCommand.mockRejectedValue(new Error('No bounding box')) + } + const result = await withDefaultGdtFrameDefaults< + ModelingCommandSchema['GDT Distance'] + >({ + data: { + objects: + scenario === 'missing selections' + ? { graphSelections: [], otherSelections: [] } + : objects, + }, + distance: true, + engineCommandManager: { + sendSceneCommand, + } as unknown as ConnectionManager, + outputUnit: 'in', + wasmInstance, + }) + expect(result.framePosition?.valueText).toBe('[0mm, 20mm]') + expect(result.fontSize).toBeUndefined() + } + ) + + it('uses bounds independently of the font expression and face-normal signs', async () => { + const valueAst = createBinaryExpression([ + createLocalName('textHeight'), + '+', + createLiteral(1, wasmInstance, 'Mm'), + ]) + const fontSize = { ...kclValue('textHeight + 1mm'), valueAst } + const sendSceneCommand = vi + .fn() + .mockResolvedValueOnce({ + success: true, + resp: { + type: 'modeling', + data: { + modeling_response: { + type: 'face_is_planar', + data: { z_axis: { x: 0, y: 0, z: -1 } }, + }, + }, + }, + }) + .mockResolvedValue({ + success: true, + resp: { + type: 'modeling', + data: { + modeling_response: { + type: 'bounding_box', + data: { dimensions: { x: 40, y: 10, z: 0 } }, + }, + }, + }, + }) + const result = await withDefaultGdtFrameDefaults< + ModelingCommandSchema['GDT Distance'] + >({ + data: { objects, fontSize }, + distance: true, + engineCommandManager: { + sendSceneCommand, + } as unknown as ConnectionManager, + wasmInstance, + }) + expect(result.framePosition?.valueText).toBe('[0mm, 61.8466mm]') + expect(result.framePosition?.valueAst).toMatchObject({ + elements: [ + { value: { value: 0 } }, + { type: 'Literal', value: { value: 61.8466, suffix: 'Mm' } }, + ], + }) + expect(result.fontSize).toBe(fontSize) + expect(result.framePlane).toBe('XZ') + }) + + it.each(['[0, 0]', '[-12mm, -8mm]'])( + 'preserves an explicit position %s', + async (position) => { + const framePosition = kclValue(position) + const data = { + objects, + framePosition, + framePlane: 'YZ', + fontSize: kclValue('2mm'), + } as ModelingCommandSchema['GDT Distance'] + const sendSceneCommand = vi.fn() + const result = await withDefaultGdtFrameDefaults({ + data, + distance: true, + engineCommandManager: { + sendSceneCommand, + } as unknown as ConnectionManager, + wasmInstance, + }) + expect(result).toBe(data) + expect(sendSceneCommand).not.toHaveBeenCalled() + } + ) + + it.each([0, 4])( + 'uses surrounding model bounds even when selected edge bounds span %s mm', + async (edgeSize) => { + const sendSceneCommand = vi + .fn() + .mockResolvedValueOnce({ + success: true, + resp: { + type: 'modeling', + data: { + modeling_response: { + type: 'bounding_box', + data: { dimensions: { x: edgeSize, y: edgeSize, z: 0 } }, + }, + }, + }, + }) + .mockResolvedValue({ + success: true, + resp: { + type: 'modeling', + data: { + modeling_response: { + type: 'bounding_box', + data: { dimensions: { x: 25, y: 40, z: 10 } }, + }, + }, + }, + }) + const selections: Selections = { + graphSelections: [], + otherSelections: [ + { + type: 'enginePrimitive', + primitiveType: 'edge', + primitiveIndex: 0, + parentEntityId: 'body', + entityId: 'hole-rim', + }, + ], + } + const result = await withDefaultGdtFrameDefaults< + ModelingCommandSchema['GDT Distance'] + >({ + data: { + objects: selections, + framePlane: 'XY', + fontSize: kclValue('100mm'), + }, + distance: true, + engineCommandManager: { + sendSceneCommand, + } as unknown as ConnectionManager, + wasmInstance, + }) + expect(result.framePosition?.valueText).toBe('[0mm, 72.3274mm]') + expect(sendSceneCommand).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + cmd: expect.objectContaining({ + type: 'bounding_box', + entity_ids: ['hole-rim'], + }), + }) + ) + expect(sendSceneCommand).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + cmd: expect.objectContaining({ + type: 'bounding_box', + entity_ids: [], + }), + }) + ) + } + ) + }) + it('averages non-zero bounding box dimensions', () => { expect(getAverageBoundingBoxDimension({ x: 4, y: 0, z: 8 })).toBe(6) expect(getAverageBoundingBoxDimension({ x: 4, y: 4, z: 4 })).toBe(4) diff --git a/src/lib/gdtFramePosition.ts b/src/lib/gdtFramePosition.ts index a4c72ee54a8..d09a850acf0 100644 --- a/src/lib/gdtFramePosition.ts +++ b/src/lib/gdtFramePosition.ts @@ -212,7 +212,15 @@ export function getEngineEntityIdsForGdtSelections( ] }) - return deduplicateArtifactIds(entityIds) + const primitiveIds = selections.otherSelections.flatMap((selection) => + typeof selection === 'object' && + 'type' in selection && + selection.type === 'enginePrimitive' && + (selection.primitiveType === 'edge' || selection.primitiveType === 'face') + ? [selection.entityId] + : [] + ) + return deduplicateArtifactIds([...entityIds, ...primitiveIds]) } export function getPlanarFaceEntityIdsForGdtSelections( @@ -427,6 +435,40 @@ function createFontSizeCommandValue( } } +function createDistanceFramePositionCommandValue( + setback: number | undefined, + outputUnit: UnitLength, + wasmInstance: ModuleType +): KclCommandValue { + const offset = createLiteral( + setback ?? 20, + wasmInstance, + setback === undefined ? 'Mm' : baseUnitToNumericSuffix(outputUnit), + 4 + ) + const valueText = `[0mm, ${offset.raw}]` + return { + valueAst: createArrayExpression([ + createLiteral(0, wasmInstance, 'Mm'), + offset, + ]), + valueText, + valueCalculated: valueText, + } +} + +function distanceSetback(bounds: BoundingBox | undefined): number | undefined { + if (!bounds) return undefined + const diagonal = Math.hypot( + ...[bounds.dimensions.x, bounds.dimensions.y, bounds.dimensions.z].filter( + (value) => Number.isFinite(value) && value > 0 + ) + ) + // The dimension line is at 80% of the setback. A full bounding-box + // diagonal plus 20% clearance keeps it outside even for interior features. + return diagonal > 0 ? roundOff(diagonal * 1.5, 4) : undefined +} + function getNormalFromPlanarFace(face: FaceIsPlanar): Point3d | undefined { const normal = face.z_axis if ( @@ -543,6 +585,7 @@ export async function withDefaultGdtFrameDefaults({ sourceCode, outputUnit = DEFAULT_DEFAULT_LENGTH_UNIT, wasmInstance, + distance = false, }: { data: T engineCommandManager: ConnectionManager @@ -550,6 +593,7 @@ export async function withDefaultGdtFrameDefaults({ sourceCode?: string outputUnit?: UnitLength wasmInstance: ModuleType + distance?: boolean }): Promise { const selections = getSelectionsFromGdtData(data) const entityIds = getEngineEntityIdsForGdtSelections(selections) @@ -563,10 +607,35 @@ export async function withDefaultGdtFrameDefaults({ ...data, fontSize: existingFontSize, } - let hasResolvedFramePlane = Boolean(nextData.framePlane) + // Face-pair distances choose their plane in KCL from the actual surface normals. + // Do not bake an axis-aligned plane into the generated annotation. + const automaticDistancePlane = + distance && + selections !== undefined && + (!nextData.framePlane || nextData.framePlane === 'Automatic') && + selections.graphSelections.length + selections.otherSelections.length === + 2 && + selections.otherSelections.every( + (selection) => + typeof selection === 'object' && + 'type' in selection && + selection.type === 'enginePrimitive' && + selection.primitiveType === 'face' + ) && + selections.graphSelections.every( + (selection) => + ['cap', 'wall', 'edgeCut', 'primitiveFace'].includes( + selection.artifact?.type ?? '' + ) || selection.entityRef?.type === 'face' + ) + if (distance && nextData.framePlane === 'Automatic') { + nextData = { ...nextData, framePlane: undefined } + } + let hasResolvedFramePlane = + automaticDistancePlane || Boolean(nextData.framePlane) let framePositionSigns: GdtFramePositionSigns | undefined const shouldQueryNormalDefaults = - !nextData.framePlane || !nextData.framePosition + !automaticDistancePlane && (!nextData.framePlane || !nextData.framePosition) if (shouldQueryNormalDefaults) { const defaultsFromNormal = @@ -625,7 +694,7 @@ export async function withDefaultGdtFrameDefaults({ ? getAverageBoundingBoxDimension(selectionBoundingBox.dimensions) : undefined - if (!nextData.framePosition && averageDimension !== undefined) { + if (!distance && !nextData.framePosition && averageDimension !== undefined) { const [xSign, ySign] = framePositionSigns ?? [1, 1] nextData = { @@ -638,7 +707,8 @@ export async function withDefaultGdtFrameDefaults({ } } - if (!nextData.fontSize) { + let setback = distanceSetback(selectionBoundingBox) + if (!nextData.fontSize || (distance && !nextData.framePosition)) { const modelBoundingBox = await getBoundingBoxForGdtEntities({ engineCommandManager, entityIds: [], @@ -649,14 +719,27 @@ export async function withDefaultGdtFrameDefaults({ ? getAverageBoundingBoxDimension(modelBoundingBox.dimensions) : undefined - if (modelAverageDimension === undefined) { - return nextData + const modelSetback = distanceSetback(modelBoundingBox) + if (modelSetback !== undefined) { + setback = Math.max(setback ?? 0, modelSetback) } + if (!nextData.fontSize && modelAverageDimension !== undefined) { + nextData = { + ...nextData, + fontSize: createFontSizeCommandValue( + modelAverageDimension, + outputUnit, + wasmInstance + ), + } + } + } + if (distance && !nextData.framePosition) { nextData = { ...nextData, - fontSize: createFontSizeCommandValue( - modelAverageDimension, + framePosition: createDistanceFramePositionCommandValue( + setback, outputUnit, wasmInstance ), diff --git a/src/lib/operations.spec.ts b/src/lib/operations.spec.ts index efe223b90ef..11309366c38 100644 --- a/src/lib/operations.spec.ts +++ b/src/lib/operations.spec.ts @@ -1056,6 +1056,25 @@ describe('operations.test.ts', () => { }) describe('GDT edit flow', () => { + it('edits a distance with automatic plane and no tolerance', async () => { + const { rustContext } = await buildTheWorldAndNoEngineConnection() + const result = await enterEditFlow({ + operation: stdlib('gdt::distance'), + code: 'gdt::distance(from = first, to = second)', + artifactGraph: new Map(), + rustContext, + }) + if (isErr(result)) throw result + if (result.type !== 'Find and select command') + throw new Error('Expected edit flow') + expect(result.data.argDefaultValues).toMatchObject({ + framePlane: 'Automatic', + }) + expect(result.data.argDefaultValues).not.toHaveProperty( + 'tolerance', + expect.anything() + ) + }) it.each(['gdt::straightness', 'gdt::distance'])( 'recovers mixed UUID, tagged, and Face API edges in order for %s', async (operationName) => { diff --git a/src/lib/operations.ts b/src/lib/operations.ts index 78d8e283431..bcbe4ff6890 100644 --- a/src/lib/operations.ts +++ b/src/lib/operations.ts @@ -2567,9 +2567,6 @@ const prepareToEditGdtDistance: PrepareToEditCallback = async ({ 'tolerance', rustContext ) - if ('error' in tolerance) { - return { reason: tolerance.error } - } const optionalArgs = await Promise.all([ extractKclArgument(code, operation, 'precision', rustContext), @@ -2582,11 +2579,12 @@ const prepareToEditGdtDistance: PrepareToEditCallback = async ({ (arg) => ('error' in arg ? undefined : arg) ) - const framePlane = extractStringArgument(code, operation, 'framePlane') + const framePlane = + extractStringArgument(code, operation, 'framePlane') ?? 'Automatic' const argDefaultValues: ModelingCommandSchema['GDT Distance'] = { objects, - tolerance, + tolerance: 'error' in tolerance ? undefined : tolerance, precision, framePosition, framePlane,