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) <noreply@anthropic.com>
This commit is contained in:
bherbruck
2026-10-29 22:08:16 -04:00
parent e016b8cf37
commit cbcbb26efe
5 changed files with 161 additions and 3 deletions
+2
View File
@@ -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).
@@ -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<Handle> 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()
}
@@ -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<O> {
fn extend_convex_edges(&self) -> bool;
/// `GetShellFaces`: the faces to shell rather than offset (empty without a list).
fn shell_faces(&self) -> Vec<O>;
/// `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);
}
@@ -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<FaceId> {
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();
}
@@ -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.