From cbcbb26efed58fdea45cade3bb09321b3a6174b4 Mon Sep 17 00:00:00 2001 From: bherbruck Date: Thu, 29 Oct 2026 22:08:16 -0400 Subject: [PATCH] fix: opt-in local operation refuses a tangent edge left across a gap (WR-530, fork 23c5b0a; #402) Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/solidcore/src/kernel.rs | 2 + .../src/offset/offset_executive/edgeuse.rs | 6 +- .../src/offset/offset_geometry_creation.rs | 36 +++++- ..._530_local_operation_loose_tangent_edge.rs | 107 ++++++++++++++++++ ...tion-leaves-a-tangent-edge-across-a-gap.md | 13 +++ 5 files changed, 161 insertions(+), 3 deletions(-) create mode 100644 crates/solidcore/tests/wr_530_local_operation_loose_tangent_edge.rs create mode 100644 plan/upstream-issues/WR-530--local-operation-leaves-a-tangent-edge-across-a-gap.md diff --git a/crates/solidcore/src/kernel.rs b/crates/solidcore/src/kernel.rs index 5aefde4d96..f5f5e8287d 100644 --- a/crates/solidcore/src/kernel.rs +++ b/crates/solidcore/src/kernel.rs @@ -223,6 +223,8 @@ pub struct UpstreamFixes { pub offset_walls_touch_refused: bool, /// `SmBrep::LocalOperation` returns the failed extended offset's own status (e.g. `SM_ERR_OFFSET_WALLS_TOUCH`, 1042, or `SM_ERR_BAD_INTERSECTIONS`, 1027) instead of a generic `SM_ERR` (WR-345 addendum, #1442; tangency-labs/smlib a759622). pub local_operation_offset_status: bool, + /// In a local operation (`SmBrep::LocalOperation`), `SmOffsetGeometryCreation::ExtendTrimEdges` fails a tangent or lamina edge whose lift onto one face's new surface drops onto the other's farther than ten times `max(the offset edge's tolerance, the approximation tolerance)`, and `EdgeuseOffset` passes that failure on, so the operation is refused; upstream ignores it ("will be fixed later", which no later step of a local operation does) and returns a body with an edge as loose as the gap between a moved face and its unmoved tangent neighbour (WR-530, #402; the fork's tangency-labs/smlib@23c5b0a). Offsets and shells are unchanged. + pub local_operation_refuses_loose_tangent_edges: bool, /// An inward shell mates the walls across rounds thinner than it (collapsed faces) (#397). pub offset_collapsed_rounds_mated: bool, /// `FaceuseOffset` fails on a face whose offset surface is missing without collapsing (`SmSurface::OffsetCollapses`: a cone, cylinder, sphere or torus offset onto its spine), where upstream reads any missing surface as a collapsed round (WR-464, #1072). diff --git a/crates/solidcore/src/offset/offset_executive/edgeuse.rs b/crates/solidcore/src/offset/offset_executive/edgeuse.rs index 8395f549bb..908f83e5da 100644 --- a/crates/solidcore/src/offset/offset_executive/edgeuse.rs +++ b/crates/solidcore/src/offset/offset_executive/edgeuse.rs @@ -47,7 +47,7 @@ use crate::topology::face::{p1 as face1, p2 as face2}; use crate::topology::proto_topology::ProtoGeometry; use crate::topology::{vertex, Brep, EdgeId, EdgeuseId, FaceId, VertexId}; -use super::p1::OffsetExecutive; +use super::p1::{OffsetExecutive, OffsetOperation}; const OP: &str = "offset::offset_executive::edgeuse"; @@ -1182,6 +1182,10 @@ impl CreatorExecutive for Executive<'_> { .collect() } + fn is_local_operation(&self) -> bool { + self.inner.operation() == OffsetOperation::LocalOperation + } + fn has_collapsed_faces(&self) -> bool { !self.inner.collapsed_faces.is_empty() } diff --git a/crates/solidcore/src/offset/offset_geometry_creation.rs b/crates/solidcore/src/offset/offset_geometry_creation.rs index 2904407e61..5c23a8975e 100644 --- a/crates/solidcore/src/offset/offset_geometry_creation.rs +++ b/crates/solidcore/src/offset/offset_geometry_creation.rs @@ -73,6 +73,11 @@ //! [`SolidError::InvalidInput`] with [`UNDEFINED_UPSTREAM`] as operation, which tells //! it apart from a callee's real `SM_ERR_INVALID_INPUT`; the statuses upstream //! ignores are ignored whatever they are, except these and [`SolidError::Unported`]. +//! - **Opt-in (WR-530, #402).** With `Kernel::fixes.local_operation_refuses_loose_tangent_edges`, +//! in a local operation ([`OffsetExecutive::is_local_operation`]) `ExtendTrimEdges` +//! fails when the lifted edge drops onto the other extended surface farther than ten +//! times `max(edge tolerance, approximation tolerance)`, and `EdgeuseOffset` passes +//! that failure on (the fork's tangency-labs/smlib@23c5b0a). //! - **`ExtendTrimEdges`' empty seam drop.** When a candidate's `GlobalPointSolve` //! (`SM_SR_SINGLE`, minimize) finds no solution, upstream reads `sSolutions2[0]` //! anyway (cpp:1610): defined behaviour, the array's backing `sData2[0]` (a default @@ -318,6 +323,11 @@ pub trait OffsetExecutive { fn extend_convex_edges(&self) -> bool; /// `GetShellFaces`: the faces to shell rather than offset (empty without a list). fn shell_faces(&self) -> Vec; + /// `GetOffsetOperation() == SM_OO_LOCAL_OPERATION`: whether the executive runs a + /// local operation (`SmBrep::LocalOperation`); `false` unless the adapter says so. + fn is_local_operation(&self) -> bool { + false + } /// Not upstream (WR-346): whether any inset face's offset collapsed (a convex round /// thinner than the wall); `false` unless `Kernel::fixes.offset_collapsed_rounds_mated`. fn has_collapsed_faces(&self) -> bool { @@ -3007,6 +3017,19 @@ impl OffsetGeometryCreation { true, None, )?; + // Opt-in (WR-530, #402; the fork's tangency-labs/smlib@23c5b0a): the edge is + // lifted onto one face's new surface and dropped onto the other's. Faces offset + // alike stay tangent, so the drop lands on the edge; when only one of them got a + // new surface (a local operation moving a face beside a blend, or one piece of a + // split face), the drop crosses the gap between them and the faces no longer + // meet there + if k.fixes.local_operation_refuses_loose_tangent_edges && executive.is_local_operation() + { + let meet_tol = 10.0 * smos_max(off_tol, self.this_approx_tol_3d()); + if dropped.max_drop_to_surf > meet_tol { + return Err(sm_err(OP_EXTEND)); + } + } let uv_curves = dropped.uv_curves; let mut closest_idx = 0usize; @@ -3574,7 +3597,7 @@ impl OffsetGeometryCreation { if tan_sec || lamina { // Extend or Trim Lamina or Tangent Edges (a problem is only reported) - let _ = ignore_soft(self.extend_trim_edges( + let extended = self.extend_trim_edges( k, brep, executive, @@ -3583,7 +3606,16 @@ impl OffsetGeometryCreation { offset_edgeuse, other_offset_edgeuse, mark, - ))?; + ); + // Opt-in (WR-530): a local operation has no later step that would close + // the gap: it fails + if k.fixes.local_operation_refuses_loose_tangent_edges + && executive.is_local_operation() + { + extended?; + } else { + let _ = ignore_soft(extended)?; + } return Ok(out); } diff --git a/crates/solidcore/tests/wr_530_local_operation_loose_tangent_edge.rs b/crates/solidcore/tests/wr_530_local_operation_loose_tangent_edge.rs new file mode 100644 index 0000000000..a9713637be --- /dev/null +++ b/crates/solidcore/tests/wr_530_local_operation_loose_tangent_edge.rs @@ -0,0 +1,107 @@ +//! Regression test for WR-530 (tangency-labs/solidcore#402; the fork's +//! tangency-labs/smlib@23c5b0a): a 60 x 40 x 10 plate with an r10 boss on it and an r2 +//! round at the boss's base has its boss wall moved out by 1 (`SmBrep::LocalOperation`, +//! the capi's `smlib_offset_faces`). Only the wall gets a new surface, so the tangent +//! edge between it and the round is dropped across the 1 mm gap between them. Upstream +//! ignores that ("will be fixed later") and returns a body with an edge as loose as the +//! gap; with `local_operation_refuses_loose_tangent_edges` the local operation is +//! refused. +#![allow( + clippy::unwrap_used, + clippy::expect_used, + clippy::panic, + clippy::indexing_slicing +)] // test helpers + +#[path = "support/offset_fixture.rs"] +mod offset_fixture; + +use offset_fixture::Notifications; +use solidcore::blend::fillet_executive::api::fillet_edges; +use solidcore::boolean::api_brep::boolean_union; +use solidcore::error::{SolidError, SM_ERR}; +use solidcore::math::Point3; +use solidcore::primitive::api_primitives::{create_box, create_cylinder, create_cylinder_surface}; +use solidcore::surface::surface::Surface; +use solidcore::topology::brep::local_operation::local_operation; +use solidcore::topology::FaceId; +use solidcore::{Brep, Kernel}; + +const AXIS: (f64, f64) = (30.0, 20.0); + +fn axis_distance(p: Point3) -> f64 { + (p.x - AXIS.0).hypot(p.y - AXIS.1) +} + +/// The plate with its boss and the r2 round at the boss's base. +fn part(k: &mut Kernel) -> Brep { + let plate = create_box(k, Point3::new(0.0, 0.0, 0.0), 60.0, 40.0, 10.0).unwrap(); + let boss = create_cylinder(k, Point3::new(AXIS.0, AXIS.1, 5.0), 10.0, 15.0).unwrap(); + let mut b = boolean_union(k, plate, boss).unwrap(); + let base: Vec<_> = b + .edges() + .iter() + .filter(|(_, e)| { + let c = e.curve.as_ref().unwrap(); + let p = c.evaluate_point(k, e.interval.mid()).unwrap(); + (p.z - 10.0).abs() < 1e-9 && (axis_distance(p) - 10.0).abs() < 1e-9 + }) + .map(|(id, _)| id) + .collect(); + assert!(!base.is_empty(), "the boss's base edge"); + fillet_edges(k, &mut b, &base, 2.0, 1, 1, 1.0).unwrap(); + b.refresh_cached_topology(); + b +} + +/// The faces of the boss's wall: on the r10 circle about the boss's axis, above the round. +fn wall(k: &Kernel, b: &Brep) -> Vec { + b.face_list() + .iter() + .copied() + .filter(|&f| { + let face = b.face(f).unwrap(); + let s = face.surface.as_ref().unwrap(); + let p = s.evaluate_point(k, face.uv_domain.mid()).unwrap(); + p.z > 12.5 && (axis_distance(p) - 10.0).abs() < 1e-6 + }) + .collect() +} + +fn move_wall(fixed: bool) -> solidcore::Result<()> { + let mut k = Kernel::default(); + k.fixes.local_operation_refuses_loose_tangent_edges = fixed; + let mut b = part(&mut k); + let faces = wall(&k, &b); + assert!(!faces.is_empty(), "the boss's wall"); + let surfaces = faces + .iter() + .map(|_| { + Surface::from( + create_cylinder_surface(&k, Point3::new(AXIS.0, AXIS.1, 5.0), 11.0, 15.0).unwrap(), + ) + }) + .collect(); + let mut notifications = Notifications::new(&mut b); + local_operation( + &mut k, + &mut b, + &faces, + surfaces, + &mut notifications.context(), + ) +} + +#[test] +fn wr_530_moved_boss_wall_beside_its_round_is_refused() { + match move_wall(true) { + Err(SolidError::Upstream { code, .. }) => assert_eq!(code, SM_ERR), + other => panic!("{other:?}"), + } +} + +/// Without the opt-in the local operation returns a body, as upstream does. +#[test] +fn wr_530_without_the_opt_in_the_local_operation_succeeds() { + move_wall(false).unwrap(); +} diff --git a/plan/upstream-issues/WR-530--local-operation-leaves-a-tangent-edge-across-a-gap.md b/plan/upstream-issues/WR-530--local-operation-leaves-a-tangent-edge-across-a-gap.md new file mode 100644 index 0000000000..a9dcc1e632 --- /dev/null +++ b/plan/upstream-issues/WR-530--local-operation-leaves-a-tangent-edge-across-a-gap.md @@ -0,0 +1,13 @@ +#### WR-530. A local operation that moves a face beside a tangent neighbour returns an edge as loose as the gap +- **Severity:** Wrong result (an invalid body returned as a success: SmBrep Test #59 after its tolerances are settled) +- **Area:** Offsets and shells +- **Location:** `source/SMLib/src/SmOffsetGeometryCreation.cpp`, `SmOffsetGeometryCreation::ExtendTrimEdges` (def. `:1384`): the drop of the lifted curve onto the other extended surface (`:1583`); `SmOffsetGeometryCreation::EdgeuseOffset` (def. `:2205`): the ignored failure (`:2294`, "Problem Extending Lamina/Tangent Edge - Will be Fixed Later"). +- **What happens:** A local operation (`SmBrep::LocalOperation`) rebuilds a tangent edge by lifting it onto one face's new surface and dropping it onto the other face's surface. When only one of the two faces gets a new surface, the drop crosses the gap between them and nothing reports it. `EdgeuseOffset` also ignores any failure there, and no later step of a local operation closes the gap, so the body comes back with an edge as loose as the gap. +- **Trigger:** a 60 x 40 x 10 plate with an r10 boss and an r2 round at the boss's base, with the boss wall offset out by 1 (the capi's `smlib_offset_faces`). Also one piece of a split top face moved up. +- **How found:** reported by SolidCraft (tangency-labs/solidcore#402). The fork's fix is tangency-labs/smlib@23c5b0a ("a local operation fails where a moved face leaves its tangent neighbour"), which is not merged upstream. +- **Confirmed:** no C++ run was made here. The Rust port reproduces both sites line for line at the pin (`3fd5b95`, usd-brep v1.0.1). +- **Rust port:** behaves as upstream by default. With `Kernel::fixes.local_operation_refuses_loose_tangent_edges`, a local operation (`OffsetExecutive::is_local_operation`, true for the library executive under `OffsetOperation::LocalOperation`) is refused in either case below; offsets and shells are unchanged: + - `extend_trim_edges` returns `SM_ERR` when the drop's `max_drop_to_surf` is more than `10 * max(offset edge tolerance, approximation tolerance)`; + - `edgeuse_offset` passes `extend_trim_edges`'s error on. + + `tests/wr_530_local_operation_loose_tangent_edge.rs` uses the boss case. It checks that the local operation is refused with the flag, and returns a body without it.