mirror of
https://github.com/bherbruck/solidcraft.git
synced 2026-11-08 04:52:10 +00:00
fix: propagate fillet geometry trim failures (#483)
Preserve successful prefix edits and snapping, but stop on failed trims or intersections instead of assigning unset intervals. Track the pinned C++ correctness differences explicitly and retain expanded native coverage. Co-Authored-By: Codex <noreply@openai.com>
This commit is contained in:
committed by
Kirill Frolov
parent
805733fe97
commit
1a2df8324c
@@ -80,7 +80,9 @@ use crate::runner::default_cases;
|
||||
use crate::{Case, HarnessError, Output, Rng, UnitDiff};
|
||||
|
||||
/// Generated cases per run, whatever `--seeds` says (it can only raise this).
|
||||
pub const MIN_GENERATED: usize = 300;
|
||||
// Keep the larger corpus canonical: it covers additional failed center trims,
|
||||
// disjoint rail intervals and unset edge intervals from #483.
|
||||
pub const MIN_GENERATED: usize = 2000;
|
||||
|
||||
/// `blend.fillet_geom_p1` runner.
|
||||
pub struct Runner;
|
||||
|
||||
@@ -108,6 +108,11 @@
|
||||
//! that B-rep ([`FilletSurfaceOwner`]) and resolve the face's surface in it; callers
|
||||
//! that run before any face is made pass `None`, and a transferred surface without
|
||||
//! (or with another) owner is refused with `InvalidInput`.
|
||||
//! - **Trim failures (#483).** With `Kernel::fixes.fillet_trim_status` enabled,
|
||||
//! `trim_geom` propagates curve/surface trim and interval-intersection errors.
|
||||
//! Default behaviour drops ordinary statuses and installs unset intersections,
|
||||
//! as pinned C++ does. Earlier successful edits and parameter snapping remain;
|
||||
//! the operation is not transactional.
|
||||
//! - **Arguments.** Out-params become return values; `SmContext` arguments are the
|
||||
//! [`Kernel`]; `int iDebugLevel` arguments and debug drawing are dropped.
|
||||
//! `sm_TwoPntsIntersection` takes the start and end *points* of its `SmVertex`
|
||||
@@ -2434,6 +2439,9 @@ impl FilletBrep {
|
||||
///
|
||||
/// [`SolidError::Upstream`] (`SM_ERR_NULL_POINTER`) for a missing rail;
|
||||
/// [`SolidError::Unported`] from a trim; [`SolidError::NotFound`] for dangling ids.
|
||||
/// With [`crate::kernel::UpstreamFixes::fillet_trim_status`] enabled, every
|
||||
/// curve/surface trim and interval-intersection error is propagated. Earlier
|
||||
/// successful edits and parameter snapping remain when a later step fails.
|
||||
pub fn trim_geom(
|
||||
&mut self,
|
||||
k: &Kernel,
|
||||
@@ -2453,11 +2461,18 @@ impl FilletBrep {
|
||||
let edge = self.brep.edge_mut(rail)?;
|
||||
if let Some(curve) = edge.curve.as_mut() {
|
||||
// may snap sIvl by tol to existing knots
|
||||
ignore_non_unported(curve.trim(k, trim_interval, TrimMode::Notify))?;
|
||||
let temp = edge
|
||||
.interval
|
||||
.intersect(trim_interval)
|
||||
.unwrap_or_else(|_| Extent1d::new());
|
||||
let trimmed = curve.trim(k, trim_interval, TrimMode::Notify);
|
||||
if k.fixes.fillet_trim_status {
|
||||
trimmed?;
|
||||
} else {
|
||||
ignore_non_unported(trimmed)?;
|
||||
}
|
||||
let intersection = edge.interval.intersect(trim_interval);
|
||||
let temp = if k.fixes.fillet_trim_status {
|
||||
intersection?
|
||||
} else {
|
||||
intersection.unwrap_or_else(|_| Extent1d::new())
|
||||
};
|
||||
set_interval(&mut self.brep, rail, &temp)?;
|
||||
}
|
||||
}
|
||||
@@ -2466,7 +2481,12 @@ impl FilletBrep {
|
||||
|
||||
// Spine curve:
|
||||
if let Some(center) = geom.center_line_curve.as_mut() {
|
||||
ignore_non_unported(center.trim(k, trim_interval, TrimMode::Notify))?;
|
||||
let trimmed = center.trim(k, trim_interval, TrimMode::Notify);
|
||||
if k.fixes.fillet_trim_status {
|
||||
trimmed?;
|
||||
} else {
|
||||
ignore_non_unported(trimmed)?;
|
||||
}
|
||||
}
|
||||
|
||||
// Fillet Surface:
|
||||
@@ -2474,23 +2494,31 @@ impl FilletBrep {
|
||||
let mut domain = surface.natural_uv_domain_with_nurb(k)?;
|
||||
// Trimming is in the u-direction.
|
||||
let u_domain = domain.u_interval();
|
||||
let temp = u_domain
|
||||
.intersect(trim_interval)
|
||||
.unwrap_or_else(|_| Extent1d::new());
|
||||
let intersection = u_domain.intersect(trim_interval);
|
||||
let temp = if k.fixes.fillet_trim_status {
|
||||
intersection?
|
||||
} else {
|
||||
intersection.unwrap_or_else(|_| Extent1d::new())
|
||||
};
|
||||
domain.set_u_interval(&temp);
|
||||
|
||||
// trim to domain
|
||||
ignore_non_unported(surface.trim_with_domain(k, &mut domain))?;
|
||||
let trimmed = surface.trim_with_domain(k, &mut domain);
|
||||
if k.fixes.fillet_trim_status {
|
||||
trimmed?;
|
||||
} else {
|
||||
ignore_non_unported(trimmed)?;
|
||||
}
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
/// Ignores a status upstream does not check, but never an [`SolidError::Unported`].
|
||||
fn ignore_non_unported<T>(r: Result<T>) -> Result<()> {
|
||||
match r {
|
||||
Err(e) if e.is_unported() => Err(e),
|
||||
/// Ignores a status upstream does not check, but never a missing dependency.
|
||||
fn ignore_non_unported<T>(result: Result<T>) -> Result<()> {
|
||||
match result {
|
||||
Err(error) if error.is_unported() => Err(error),
|
||||
_ => Ok(()),
|
||||
}
|
||||
}
|
||||
@@ -4140,6 +4168,204 @@ mod tests {
|
||||
Surface::from(BSplineSurface::from_nurb(nurb, false))
|
||||
}
|
||||
|
||||
fn trim_test_curve(end: f64) -> BSplineCurve {
|
||||
let mut curve = BSplineCurve::create_line_segment(
|
||||
3,
|
||||
Point3::new(0.0, 0.0, 0.0),
|
||||
Point3::new(1.0, 0.0, 0.0),
|
||||
)
|
||||
.unwrap();
|
||||
curve
|
||||
.edit_parameterization(&Extent1d::from_min_max(0.0, end))
|
||||
.unwrap();
|
||||
curve
|
||||
}
|
||||
|
||||
fn trim_test_geom() -> (FilletBrep, FilletGeomId, [EdgeId; 4]) {
|
||||
let k = Kernel::default();
|
||||
let mut fb = FilletBrep::new();
|
||||
let start = fb
|
||||
.brep_mut()
|
||||
.add_vertex(Point3::new(0.0, 0.0, 0.0))
|
||||
.unwrap();
|
||||
let end = fb
|
||||
.brep_mut()
|
||||
.add_vertex(Point3::new(1.0, 0.0, 0.0))
|
||||
.unwrap();
|
||||
let rails = std::array::from_fn(|_| {
|
||||
let edge = fb.new_fillet_edge(start, end, None).unwrap();
|
||||
let record = fb.brep_mut().edge_mut(edge).unwrap();
|
||||
record.curve = Some(Curve::from(trim_test_curve(1.0)));
|
||||
record.interval = Extent1d::from_min_max(0.0, 1.0);
|
||||
edge
|
||||
});
|
||||
let mut geom = FilletGeom::new(&k, None, FilletGeomType::Blends).unwrap();
|
||||
geom.rails = [Some(rails[0]), Some(rails[1])];
|
||||
geom.other_rails1.push(rails[2]);
|
||||
geom.other_rails2.push(rails[3]);
|
||||
let g = fb.add_geom(geom);
|
||||
(fb, g, rails)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn trim_geom_propagates_each_rail_curve_error_and_stops() {
|
||||
let mut k = Kernel::default();
|
||||
k.fixes.fillet_trim_status = true;
|
||||
for failed in 0..4 {
|
||||
let (mut fb, g, rails) = trim_test_geom();
|
||||
fb.brep_mut().edge_mut(rails[failed]).unwrap().curve =
|
||||
Some(Curve::from(trim_test_curve(0.5)));
|
||||
let mut interval = Extent1d::from_min_max(0.25, 0.75);
|
||||
let mut failing_curve = trim_test_curve(0.5);
|
||||
let expected = failing_curve
|
||||
.trim(&k, &mut interval, TrimMode::Notify)
|
||||
.unwrap_err();
|
||||
assert_eq!(fb.trim_geom(&k, g, &mut interval).unwrap_err(), expected);
|
||||
for (i, rail) in rails.iter().enumerate() {
|
||||
let expected_interval = if i < failed {
|
||||
interval
|
||||
} else {
|
||||
Extent1d::from_min_max(0.0, 1.0)
|
||||
};
|
||||
assert_eq!(fb.brep().edge(*rail).unwrap().interval, expected_interval);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn trim_geom_does_not_install_failed_rail_intersection() {
|
||||
let mut k = Kernel::default();
|
||||
k.fixes.fillet_trim_status = true;
|
||||
let (mut fb, g, rails) = trim_test_geom();
|
||||
let old = Extent1d::from_min_max(2.0, 3.0);
|
||||
fb.brep_mut().edge_mut(rails[0]).unwrap().interval = old;
|
||||
let mut interval = Extent1d::from_min_max(0.25, 0.75);
|
||||
let expected = old.intersect(&interval).unwrap_err();
|
||||
assert_eq!(
|
||||
fb.trim_geom(&k, g, &mut interval)
|
||||
.unwrap_err(),
|
||||
expected
|
||||
);
|
||||
assert_eq!(fb.brep().edge(rails[0]).unwrap().interval, old);
|
||||
// Successful curve edits before the failure remain, but later rails are untouched.
|
||||
assert_eq!(
|
||||
fb.brep()
|
||||
.edge(rails[0])
|
||||
.unwrap()
|
||||
.curve
|
||||
.as_ref()
|
||||
.unwrap()
|
||||
.natural_interval(),
|
||||
interval
|
||||
);
|
||||
assert_eq!(
|
||||
fb.brep().edge(rails[1]).unwrap().interval,
|
||||
Extent1d::from_min_max(0.0, 1.0)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn trim_geom_propagates_center_line_error_before_surface_edit() {
|
||||
let mut k = Kernel::default();
|
||||
k.fixes.fillet_trim_status = true;
|
||||
let (mut fb, g, _) = trim_test_geom();
|
||||
let geom = fb.geom_mut(g).unwrap();
|
||||
geom.center_line_curve = Some(trim_test_curve(0.5));
|
||||
geom.fillet_surface = Some(bilinear_patch());
|
||||
let mut interval = Extent1d::from_min_max(0.25, 0.75);
|
||||
let expected = trim_test_curve(0.5)
|
||||
.trim(&k, &mut interval, TrimMode::Notify)
|
||||
.unwrap_err();
|
||||
assert_eq!(fb.trim_geom(&k, g, &mut interval).unwrap_err(), expected);
|
||||
assert_eq!(
|
||||
fb.geom(g)
|
||||
.unwrap()
|
||||
.fillet_surface
|
||||
.as_ref()
|
||||
.unwrap()
|
||||
.natural_uv_domain()
|
||||
.u_interval(),
|
||||
Extent1d::from_min_max(0.0, 1.0)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn trim_geom_rejects_disjoint_surface_domain() {
|
||||
let mut k = Kernel::default();
|
||||
k.fixes.fillet_trim_status = true;
|
||||
let (mut fb, g, rails) = trim_test_geom();
|
||||
for rail in rails {
|
||||
fb.brep_mut().edge_mut(rail).unwrap().curve = None;
|
||||
}
|
||||
fb.geom_mut(g).unwrap().fillet_surface = Some(bilinear_patch());
|
||||
let mut interval = Extent1d::from_min_max(2.0, 3.0);
|
||||
let expected = Extent1d::from_min_max(0.0, 1.0)
|
||||
.intersect(&interval)
|
||||
.unwrap_err();
|
||||
assert_eq!(
|
||||
fb.trim_geom(&k, g, &mut interval)
|
||||
.unwrap_err(),
|
||||
expected
|
||||
);
|
||||
assert_eq!(
|
||||
fb.geom(g)
|
||||
.unwrap()
|
||||
.fillet_surface
|
||||
.as_ref()
|
||||
.unwrap()
|
||||
.natural_uv_domain()
|
||||
.u_interval(),
|
||||
Extent1d::from_min_max(0.0, 1.0)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn trim_geom_propagates_surface_trim_failure() {
|
||||
let mut k = Kernel::default();
|
||||
k.fixes.fillet_trim_status = true;
|
||||
let (mut fb, g, rails) = trim_test_geom();
|
||||
for rail in rails {
|
||||
fb.brep_mut().edge_mut(rail).unwrap().curve = None;
|
||||
}
|
||||
let mut surface = bilinear_patch();
|
||||
fb.geom_mut(g).unwrap().fillet_surface = Some(bilinear_patch());
|
||||
let mut interval = Extent1d::from_min_max(0.5, 0.5);
|
||||
let mut domain = surface.natural_uv_domain();
|
||||
domain.set_u_interval(&interval);
|
||||
let expected = surface.trim_with_domain(&k, &mut domain).unwrap_err();
|
||||
assert_eq!(fb.trim_geom(&k, g, &mut interval).unwrap_err(), expected);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn trim_geom_keeps_successful_snapping_and_trims_all_geometry() {
|
||||
let mut k = Kernel::default();
|
||||
k.fixes.fillet_trim_status = true;
|
||||
let (mut fb, g, rails) = trim_test_geom();
|
||||
fb.geom_mut(g).unwrap().center_line_curve = Some(trim_test_curve(1.0));
|
||||
fb.geom_mut(g).unwrap().fillet_surface = Some(bilinear_patch());
|
||||
let mut interval = Extent1d::from_min_max(-1e-14, 0.75);
|
||||
fb.trim_geom(&k, g, &mut interval).unwrap();
|
||||
assert_eq!(interval, Extent1d::from_min_max(0.0, 0.75));
|
||||
for rail in rails {
|
||||
let edge = fb.brep().edge(rail).unwrap();
|
||||
assert_eq!(edge.interval, interval);
|
||||
assert_eq!(edge.curve.as_ref().unwrap().natural_interval(), interval);
|
||||
}
|
||||
let geom = fb.geom(g).unwrap();
|
||||
assert_eq!(
|
||||
geom.center_line_curve.as_ref().unwrap().natural_interval(),
|
||||
interval
|
||||
);
|
||||
assert_eq!(
|
||||
geom.fillet_surface
|
||||
.as_ref()
|
||||
.unwrap()
|
||||
.natural_uv_domain()
|
||||
.u_interval(),
|
||||
interval
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn noise_cleanup_rounds_near_decimals() {
|
||||
assert_eq!(clean_up_noise(1.0 + 1e-13), 1.0);
|
||||
|
||||
@@ -145,6 +145,8 @@ impl Caches {
|
||||
#[allow(clippy::struct_excessive_bools)]
|
||||
#[non_exhaustive]
|
||||
pub struct UpstreamFixes {
|
||||
/// `SmFilletGeom::Trim` returns the first curve/surface trim or interval-intersection error instead of dropping it and continuing with an unset interval (#483). Earlier successful edits remain; off by default for pinned C++ parity.
|
||||
pub fillet_trim_status: bool,
|
||||
/// `CalcRailIntEdgeuse`'s retry rail search holds a variable round's radius only where its first search does (at a corner calculated as constant radius), so the rail misses the arc it would meet only at that radius and the corner cuts the round off against the body; needs `fillet_nx1_end_rib_global_merge` there (WR-300, #1023).
|
||||
pub fillet_retry_radius_as_first: bool,
|
||||
/// `TestRollOver` joins a rail's neighbouring intervals on its face or the face's boundary before the rollover, so a rail along its face's rim does not fail "Unable to Process Rollover" (WR-301, #485).
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -0,0 +1,81 @@
|
||||
# blend.fillet_geom_p1: FilletGeom Trim status propagation (#483, PR #674, 2026-10-24)
|
||||
|
||||
`blend.fillet_geom_p1` copied the pinned C++ `SmFilletGeom::Trim`'s dropped
|
||||
statuses. `FilletBrep::trim_geom` now returns the actual error from each primary
|
||||
rail, both other-rail lists, the center line, either interval intersection and
|
||||
the surface trim. A failed intersection has no usable result: it must not become
|
||||
`Extent1d::new()` assigned to an edge or a surface domain. Successful edits and
|
||||
parameter snapping before an error remain, in the original visit order. This
|
||||
is error propagation, not transaction rollback. The wider boolean/offset/sweep
|
||||
sites in #483 remain separate work; this PR does not close that broad issue.
|
||||
|
||||
A small native reproduction uses a degree-one rail over `[0,0.5]` and asks to
|
||||
trim it to `[0.25,0.75]`. Its trim returns `SM_ERR` (1001), but the pinned C++
|
||||
outer operation returns success, sets the edge interval to `[0.25,0.75]` although
|
||||
the curve still ends at 0.5, and edits subsequent rails. Rust now returns 1001
|
||||
and stops at that rail. With a valid curve but edge interval `[2,3]`, the same
|
||||
request makes C++ install its unset intersection `[1e20,-1e20]`; Rust preserves
|
||||
`[2,3]` and returns the intersection error. Earlier successful curve edits remain.
|
||||
No CAD application exists in this kernel repository to capture this status-only
|
||||
API defect; the fixtures contain the complete small pseudo-B-reps and export
|
||||
both statuses and geometry/interval state.
|
||||
|
||||
Six Rust regressions exercise all four rail positions (one parameterized test),
|
||||
a disjoint edge interval, center-line failure before surface edits, disjoint
|
||||
surface U domains, a collapsed surface trim and successful snapping/trimming of
|
||||
all geometry. Five tests fail against the original implementation; all six pass
|
||||
with error propagation. The successful case snaps -1e-14 to 0 and trims every
|
||||
rail, center line and surface to `[0,0.75]`.
|
||||
|
||||
The native corpus retains all original cases and adds nine small fixtures. Its
|
||||
minimum generated corpus increases from 300 to 2000 to keep the additional error
|
||||
paths in the canonical check. The unclassified native run has 2199 cases: 2162
|
||||
bit-identical passes, 13 existing explicit reference guard rejections and 24
|
||||
C++ correctness mismatches. All 24 are recorded individually in
|
||||
the former shared `crates/difftest/known_mismatches.toml`; they are not passes and this unit is
|
||||
**partial**. No tolerance, reference implementation or refusal guard changes.
|
||||
The mismatches are:
|
||||
|
||||
- `fb_trim_outside`, `fb_trim_point`: out-of-domain/collapsed rail trim failures
|
||||
were ignored; C++ changes rail intervals anyway (1001/1007 become success).
|
||||
- `trim_failed_rail0`, `trim_failed_rail1`, `trim_failed_other1`,
|
||||
`trim_failed_other2`: the short rail reproduction above at each visit position.
|
||||
- `trim_disjoint_rail_interval`: failed intersection installs the unset sentinel.
|
||||
- `trim_failed_center_line`: C++ continues to trim the surface after the center
|
||||
curve trim failed; Rust propagates 1001.
|
||||
- `trim_disjoint_surface_domain`, `trim_failed_surface_point`: C++ returns success
|
||||
after the surface intersection/trim failed; Rust propagates 1001/1007.
|
||||
- `gen0139`, `gen0185`, `gen0476`, `gen1351`, `gen1477`: requested first-rail trim
|
||||
exceeds its natural domain; C++ drops the error and continues editing.
|
||||
- `gen0438`: curve-less rails are skipped, then the center trim's lower bound is
|
||||
outside its natural domain; C++ drops the error.
|
||||
- `gen0245`, `gen1452`, `gen1626`: first visited rail has an unset edge interval;
|
||||
C++ ignores the failed intersection and edits later geometry.
|
||||
- `gen0829`: the reversed request is normalized to `[0.2866718515818742,
|
||||
0.4458908358559929]`; the second rail has an unset edge interval. The reversal
|
||||
itself is not an error. C++ ignores the failed intersection.
|
||||
- `gen1609`: an other rail has an unset edge interval; C++ proceeds into the center.
|
||||
- `gen1440`, `gen1647`, `gen1709`: a visited rail's edge interval is disjoint from
|
||||
the request; C++ installs an unset result and returns success.
|
||||
|
||||
Local Rust validation on c44a2e1e3 plus this fix: 2077 kernel tests pass (two
|
||||
pre-existing ignored tests), default workspace Clippy with -D warnings and
|
||||
wasm32-unknown-unknown check pass. The full macOS --green suite remains subject
|
||||
to the already reported C++ reference defects #448/#449/#450; it has not been
|
||||
re-certified by this change. Canonical CI still needs maintainer approval.
|
||||
|
||||
Default and native-feature workspace Clippy with -D warnings pass, as do the
|
||||
46 default difftest unit tests and one floating-point roundtrip integration test.
|
||||
The classified native run is PARTIAL: 2199 / 2162 pass / 24 known mismatches /
|
||||
13 existing guard rejections / 0 unexpected failures. Whole-unit perturbation
|
||||
reports INSENSITIVE because its 24 known-mismatch outcomes are not counted as
|
||||
FAIL; all 2162 originally passing comparisons are detected, and the new
|
||||
successful-snapping fixture independently reports SENSITIVE (1/1). This is not
|
||||
claimed as a clean whole-unit perturbation gate.
|
||||
|
||||
Final rebase onto main 73e557331 preserves all implementation/fixture/mismatch
|
||||
bytes. Repeated fmt, all 2077 kernel tests, default/native workspace Clippy,
|
||||
wasm and the canonical native run pass their stated gates on that base: the
|
||||
native unit remains partial, with 2162 exact passes, 24 explicit reference bugs,
|
||||
13 existing guard rejections and zero unexpected failures. Successful-snapping
|
||||
perturbation remains SENSITIVE (1/1). No conflicts required resolution.
|
||||
File diff suppressed because one or more lines are too long
Reference in New Issue
Block a user