mirror of
https://github.com/bherbruck/solidcraft.git
synced 2026-11-08 04:52:10 +00:00
Merge pull request #2198 from brett19/fix/1952-extended-rail-end-skip
fix: an extended rail end whose section can't be solved is skipped instead of failing the round (#1952, WR-641)
This commit is contained in:
@@ -823,6 +823,17 @@ fn c1_collect_points(
|
||||
) {
|
||||
Ok(()) => {}
|
||||
Err(e) if e.is_unported() => return Err(e),
|
||||
// SolidCraft (tangency-labs/solidcore#1952, WR-641, under
|
||||
// `fillet_through_extended_rail_end`): an extended rail end's knot is a
|
||||
// through point only where its section can be solved. Where the edge's end
|
||||
// meets the wall obliquely, or the section's rail lies past the side face's
|
||||
// edge, the guess is off the face and no section is found there: the trace
|
||||
// goes on without that point, as it does without the flag, rather than
|
||||
// failing the fillet.
|
||||
Err(_) if k >= ops.solver().g1_knots.len() => {
|
||||
k += 1;
|
||||
continue;
|
||||
}
|
||||
Err(_) => return Err(SolidError::sm_err(OP)),
|
||||
}
|
||||
// add new TsectPnt to sAllTsectPnts array (and sDeletePnts)
|
||||
|
||||
@@ -936,7 +936,7 @@ pub struct UpstreamFixes {
|
||||
pub fillet_point_bigger_step_retry: bool,
|
||||
/// `SmFilletSolver::ComputePointValues` sets a tangent point's curve tangent from the forward finite-difference step alone (along the trace, the previous point's tangent), and from the backward step only when the forward one finds nothing, not from their central difference: the intersection curve ends or branches at a tangent point, and where the offset surfaces touch only to within the model's accuracy (a sketch's 1e-10 residues) the backward step lands on the near-tangent crossing beyond it, 18 degrees off the curve, so every step after the point failed the angle test and the trace crept on in steps down to 8e-10 (repeated rail control points, an internal pole) (WR-595; the fork's tangency-labs/smlib@f8fd83e). Use with `fillet_trace_from_tangent_point_toward_next`, as the fork has both: alone, the forward step from a trace's tangent first point runs along the edge's end tangent, and `Fillet/nurb_cone.smb` edge 8 at r 0.5, which upstream builds, fails.
|
||||
pub fillet_tangent_point_one_sided_step: bool,
|
||||
/// A fillet's trace also runs through the filleted edge's end where a rail ends on an extended edge, so the surface reaches that end (#1022, WR-468).
|
||||
/// A fillet's trace also runs through the filleted edge's end where a rail ends on an extended edge, so the surface reaches that end (#1022, WR-468). An end whose section cannot be solved there (the guess past an oblique wall, or the section's rail past the side face's edge) is skipped, and the trace runs on without it, where the fork's d33890f fails the round with 1001 (WR-641, #1952).
|
||||
pub fillet_through_extended_rail_end: bool,
|
||||
/// `SmFace::ComputePreciseProperties` scales the volume and moment error bounds by the body size (ten times the root of the estimated area, or the origin's distance when larger), not 1.0.1's tenth of it, so the integration converges at tight accuracies instead of returning `SM_ERR` (WR-472; the SolidCraft fork's tangency-labs/smlib@073d4740).
|
||||
pub precise_properties_body_length_scale: bool,
|
||||
|
||||
@@ -0,0 +1,150 @@
|
||||
//! tangency-labs/solidcore#1952 (WR-641): with `Kernel::fixes.fillet_through_extended_rail_end`
|
||||
//! (WR-468, the fork's tangency-labs/smlib@d33890f) on, the filleted edge's end at a rail
|
||||
//! that ends on an extended edge is a through point of the trace. Where no section can
|
||||
//! be solved there, `CalcFilletGeom` failed and the round with it (1001, "Fillet Surface
|
||||
//! Computation Failure"), on rounds that build without the flag. The knot is now skipped
|
||||
//! when its section is not found, so the trace goes on as it does without the flag.
|
||||
//!
|
||||
//! The case is a box 40 long, 10 wide and 15 high joined with `create_cylinder` r 10, h 30
|
||||
//! at the origin (bases coplanar). The concave arc where the box's top meets the cylinder
|
||||
//! is rounded r 2, with the box along +X, -X, +Y and -Y, so the cylinder's seam (+X)
|
||||
//! falls at the arc's middle, opposite it, and a quarter turn either way. At the arc's
|
||||
//! ends the section's rail on the top face lies past the box's side face (y 6 against
|
||||
//! the face's y 5), off the top plane's domain, so the flag's through point there
|
||||
//! cannot be solved.
|
||||
//!
|
||||
//! - Flag off: the +X and +Y boxes round. The -X and -Y boxes fail with 1001 at their
|
||||
//! corner, and so does the pinned C++ (`blend.fillet_executive_p3`,
|
||||
//! `attributed-box-cylinder-arc-round-*`). That is WR-568, which
|
||||
//! `fillet_torus_span_from_start_section` fixes.
|
||||
//! - `fillet_through_extended_rail_end` alone: every box failed with 1001. Now it rounds
|
||||
//! as with the flag off.
|
||||
//! - With `fillet_torus_span_from_start_section` as well, every box rounds.
|
||||
//!
|
||||
//! The rounded volume is the box-cylinder union plus the material under the round:
|
||||
//! `6000 + 3000 pi - 15 (2 (2.5 sqrt 75 + 50 asin 0.5))` plus
|
||||
//! `int_10^12 2 r asin(5 / r) (2 - sqrt(4 - (12 - r)^2)) dr` (the region between the
|
||||
//! top plane, the cylinder and the ball's circle, swept round the axis between the
|
||||
//! box's side planes y = +-5), 13998.81358766597.
|
||||
#![allow(
|
||||
clippy::unwrap_used,
|
||||
clippy::expect_used,
|
||||
clippy::panic,
|
||||
clippy::indexing_slicing
|
||||
)] // test helpers
|
||||
|
||||
use solidcore::blend::fillet_executive::api::fillet_edges_per_edge;
|
||||
use solidcore::blend::fillet_solver::p1::FilletSurfaceGeneratorType;
|
||||
use solidcore::boolean::api_brep::boolean_union;
|
||||
use solidcore::geom::attribute::{AttributeStore, ObjectKey};
|
||||
use solidcore::mass::integrator::properties::{brep_volume, PropertyAttributes};
|
||||
use solidcore::math::Point3;
|
||||
use solidcore::primitive::api_primitives::{create_box, create_cylinder};
|
||||
use solidcore::topology::brep::p1;
|
||||
use solidcore::topology::brep::p2::is_manifold_solid;
|
||||
use solidcore::topology::edge;
|
||||
use solidcore::{Brep, Kernel, SolidError};
|
||||
|
||||
/// The union and the round's material, in closed form (see the module docs).
|
||||
const ROUNDED_VOLUME: f64 = 13_998.813_587_665_97;
|
||||
|
||||
/// The boxes: origin and size, and whether the arc rounds with no flag on.
|
||||
const BOXES: [([f64; 3], [f64; 3], bool); 4] = [
|
||||
([0.0, -5.0, 0.0], [40.0, 10.0, 15.0], true),
|
||||
([-40.0, -5.0, 0.0], [40.0, 10.0, 15.0], false),
|
||||
([-5.0, 0.0, 0.0], [10.0, 40.0, 15.0], true),
|
||||
([-5.0, -40.0, 0.0], [10.0, 40.0, 15.0], false),
|
||||
];
|
||||
|
||||
fn volume(k: &Kernel, b: &mut Brep) -> f64 {
|
||||
let store = AttributeStore::default();
|
||||
let attrs = PropertyAttributes {
|
||||
store: &store,
|
||||
key_of: &|_| ObjectKey(0),
|
||||
};
|
||||
brep_volume(k, b, 1e-9, &attrs).unwrap()
|
||||
}
|
||||
|
||||
/// Rounds the box's top arc r 2; the body and the round's result.
|
||||
fn round(k: &mut Kernel, origin: [f64; 3], size: [f64; 3]) -> (Brep, solidcore::Result<()>) {
|
||||
let block = create_box(k, Point3::from(origin), size[0], size[1], size[2]).unwrap();
|
||||
let cylinder = create_cylinder(k, Point3::new(0.0, 0.0, 0.0), 10.0, 30.0).unwrap();
|
||||
let mut b = boolean_union(k, block, cylinder).unwrap();
|
||||
// the arc: the edges at z 15 on the cylinder, between two faces, over the box
|
||||
let inside = |p: Point3| {
|
||||
(p.x - origin[0]) * (p.x - origin[0] - size[0]) <= 0.0
|
||||
&& (p.y - origin[1]) * (p.y - origin[1] - size[1]) <= 0.0
|
||||
};
|
||||
let arc: Vec<_> = p1::edges(&b, None)
|
||||
.unwrap()
|
||||
.into_iter()
|
||||
.filter(|&e| {
|
||||
let ivl = edge::interval(&b, e).unwrap();
|
||||
let curve = edge::curve(&b, e).unwrap().unwrap();
|
||||
let m = curve
|
||||
.evaluate(k, 0.5 * (ivl.min() + ivl.max()), 0, true, false)
|
||||
.unwrap()[0];
|
||||
(m.z - 15.0).abs() < 1e-6
|
||||
&& ((m.x * m.x + m.y * m.y).sqrt() - 10.0).abs() < 1e-6
|
||||
&& edge::faces(&b, e, None).unwrap().len() == 2
|
||||
&& inside(m)
|
||||
})
|
||||
.collect();
|
||||
assert!(!arc.is_empty() && arc.len() <= 2, "{arc:?}");
|
||||
let r = fillet_edges_per_edge(
|
||||
k,
|
||||
&mut b,
|
||||
&arc,
|
||||
&vec![2.0; arc.len()],
|
||||
&vec![FilletSurfaceGeneratorType::Circular; arc.len()],
|
||||
1,
|
||||
1.0,
|
||||
);
|
||||
(b, r)
|
||||
}
|
||||
|
||||
fn check(torus_span: bool, extended_rail_end: bool) {
|
||||
for (origin, size, rounds_unflagged) in BOXES {
|
||||
let mut k = Kernel::default();
|
||||
k.fixes.fillet_torus_span_from_start_section = torus_span;
|
||||
k.fixes.fillet_through_extended_rail_end = extended_rail_end;
|
||||
let (mut b, r) = round(&mut k, origin, size);
|
||||
let case =
|
||||
format!("box at {origin:?}, torus span {torus_span}, rail end {extended_rail_end}");
|
||||
if rounds_unflagged || torus_span {
|
||||
r.unwrap_or_else(|e| panic!("{case}: {e:?}"));
|
||||
assert!(is_manifold_solid(&b).unwrap(), "{case}");
|
||||
let v = volume(&k, &mut b);
|
||||
assert!((v - ROUNDED_VOLUME).abs() < 1e-5, "{case}: {v}");
|
||||
} else {
|
||||
assert!(
|
||||
matches!(r, Err(SolidError::Upstream { code: 1001, .. })),
|
||||
"{case}: {r:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// No flag: the pinned behaviour (the C++ fails the -X and -Y boxes too).
|
||||
#[test]
|
||||
fn issue_1952_flags_off_round_as_the_pinned_cpp() {
|
||||
check(false, false);
|
||||
}
|
||||
|
||||
/// The extended rail end alone: was 1001 for every box, now as with the flag off.
|
||||
#[test]
|
||||
fn issue_1952_extended_rail_end_skips_an_unsolved_end_section() {
|
||||
check(false, true);
|
||||
}
|
||||
|
||||
/// With the torus span fix as well, every box rounds.
|
||||
#[test]
|
||||
fn issue_1952_extended_rail_end_with_torus_span_rounds_every_box() {
|
||||
check(true, true);
|
||||
}
|
||||
|
||||
/// The torus span fix alone rounds every box (WR-568).
|
||||
#[test]
|
||||
fn issue_1952_torus_span_alone_rounds_every_box() {
|
||||
check(true, false);
|
||||
}
|
||||
@@ -14,9 +14,9 @@
|
||||
//! (WR-641): without WR-468 the round succeeds, 18807.824471783 here and
|
||||
//! 18807.824471782 in the fork's main without the hunk. With WR-468 the trace point
|
||||
//! it adds at the edge's end cannot be solved (`sm_FindFilletTsectPnt` from a guess
|
||||
//! point past the wall) and the round fails with 1001 "Fillet Surface Computation
|
||||
//! Failure", as it does in the fork's main C++. This test pins that parity; it is the
|
||||
//! fork's defect (`plan/upstream-issues/WR-641--*.md`), not the port's.
|
||||
//! point past the wall). The fork's main C++ then fails the round with 1001 "Fillet
|
||||
//! Surface Computation Failure" (WR-641, the fork's defect). Under the same flag the
|
||||
//! port now skips that knot (#1952), so the round builds as it does without WR-468.
|
||||
#![allow(
|
||||
clippy::unwrap_used,
|
||||
clippy::expect_used,
|
||||
@@ -35,7 +35,7 @@ use solidcore::primitive::api_primitives::{create_box, create_cylinder};
|
||||
use solidcore::sweep::api_brep::create_linear_sweep;
|
||||
use solidcore::topology::brep::p2::is_manifold_solid;
|
||||
use solidcore::topology::{edge, EdgeId};
|
||||
use solidcore::{Brep, Kernel, SolidError};
|
||||
use solidcore::{Brep, Kernel};
|
||||
|
||||
fn volume(k: &Kernel, b: &mut Brep) -> f64 {
|
||||
let store = AttributeStore::default();
|
||||
@@ -182,14 +182,15 @@ fn wr_641_wall_boss_rounds_without_wr468() {
|
||||
assert!((v - 18807.824471782).abs() < 1e-9 * v, "{v}");
|
||||
}
|
||||
|
||||
/// WR-641: the fork's main C++ (with d33890f) fails on this boss with 1001, and so does the
|
||||
/// port with WR-468 on. Pinned for parity; the defect is the fork's.
|
||||
/// WR-641: the fork's main C++ (with d33890f) fails on this boss with 1001. With WR-468
|
||||
/// the port skips the end knot whose section it cannot solve (#1952) and rounds the boss
|
||||
/// as it does without WR-468.
|
||||
#[test]
|
||||
fn wr_641_wr468_fails_the_wall_boss_as_the_fork_does() {
|
||||
let (_, b, r) = wall_boss(true);
|
||||
assert!(
|
||||
matches!(r, Err(SolidError::Upstream { code: 1001, .. })),
|
||||
"{r:?}"
|
||||
);
|
||||
assert!(is_manifold_solid(&b).unwrap(), "the body is left as it was");
|
||||
fn wr_641_wr468_rounds_the_wall_boss_as_without_it() {
|
||||
let (k, mut b, r) = wall_boss(true);
|
||||
r.unwrap();
|
||||
assert!(is_manifold_solid(&b).unwrap());
|
||||
let v = volume(&k, &mut b);
|
||||
// the fork's main without d33890f's hunk: 18807.824471782
|
||||
assert!((v - 18807.824471782).abs() < 1e-9 * v, "{v}");
|
||||
}
|
||||
|
||||
@@ -0,0 +1,157 @@
|
||||
{
|
||||
"cases": [
|
||||
{
|
||||
"name": "attributed-box-cylinder-arc-round-seam-mid-r2",
|
||||
"upstream_ref": "source/SM_API/src/SmApiBrep.cpp (SmApiFilletEdgesPerEdge)",
|
||||
"note": "A box 40 long, 10 wide and 15 high joined with SmApiCreateCylinder r 10, h 35 at (0, 0, -5) (below the box's base, so no face is coplanar); the box along +X: the cylinder's seam (+X) crosses the arc's middle, so the arc is two edges. The concave arc where the box's top (z 15) meets the cylinder is rounded r 2 (SmApiFilletEdgesPerEdge, circular).",
|
||||
"inputs": {
|
||||
"attributed": {
|
||||
"api": "per_edge",
|
||||
"body": {
|
||||
"box_cylinder_union": {
|
||||
"a": [
|
||||
[
|
||||
0,
|
||||
-5,
|
||||
0
|
||||
],
|
||||
[
|
||||
40,
|
||||
10,
|
||||
15
|
||||
]
|
||||
],
|
||||
"origin": [
|
||||
0,
|
||||
0,
|
||||
-5
|
||||
],
|
||||
"radius": 10,
|
||||
"height": 35
|
||||
}
|
||||
},
|
||||
"radius": 2.0,
|
||||
"xsect": 2,
|
||||
"edges": [
|
||||
10,
|
||||
11
|
||||
]
|
||||
}
|
||||
}
|
||||
},
|
||||
{
|
||||
"name": "attributed-box-cylinder-arc-round-seam-opposite-r2",
|
||||
"upstream_ref": "source/SM_API/src/SmApiBrep.cpp (SmApiFilletEdgesPerEdge)",
|
||||
"note": "A box 40 long, 10 wide and 15 high joined with SmApiCreateCylinder r 10, h 35 at (0, 0, -5) (below the box's base, so no face is coplanar); the box along -X: the arc is centred opposite the seam (180 degrees). The concave arc where the box's top (z 15) meets the cylinder is rounded r 2 (SmApiFilletEdgesPerEdge, circular).",
|
||||
"inputs": {
|
||||
"attributed": {
|
||||
"api": "per_edge",
|
||||
"body": {
|
||||
"box_cylinder_union": {
|
||||
"a": [
|
||||
[
|
||||
-40,
|
||||
-5,
|
||||
0
|
||||
],
|
||||
[
|
||||
40,
|
||||
10,
|
||||
15
|
||||
]
|
||||
],
|
||||
"origin": [
|
||||
0,
|
||||
0,
|
||||
-5
|
||||
],
|
||||
"radius": 10,
|
||||
"height": 35
|
||||
}
|
||||
},
|
||||
"radius": 2.0,
|
||||
"xsect": 2,
|
||||
"edges": [
|
||||
9
|
||||
]
|
||||
}
|
||||
}
|
||||
},
|
||||
{
|
||||
"name": "attributed-box-cylinder-arc-round-seam-quarter-r2",
|
||||
"upstream_ref": "source/SM_API/src/SmApiBrep.cpp (SmApiFilletEdgesPerEdge)",
|
||||
"note": "A box 40 long, 10 wide and 15 high joined with SmApiCreateCylinder r 10, h 35 at (0, 0, -5) (below the box's base, so no face is coplanar); the box along +Y: the arc is centred 90 degrees from the seam. The concave arc where the box's top (z 15) meets the cylinder is rounded r 2 (SmApiFilletEdgesPerEdge, circular).",
|
||||
"inputs": {
|
||||
"attributed": {
|
||||
"api": "per_edge",
|
||||
"body": {
|
||||
"box_cylinder_union": {
|
||||
"a": [
|
||||
[
|
||||
-5,
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
10,
|
||||
40,
|
||||
15
|
||||
]
|
||||
],
|
||||
"origin": [
|
||||
0,
|
||||
0,
|
||||
-5
|
||||
],
|
||||
"radius": 10,
|
||||
"height": 35
|
||||
}
|
||||
},
|
||||
"radius": 2.0,
|
||||
"xsect": 2,
|
||||
"edges": [
|
||||
9
|
||||
]
|
||||
}
|
||||
}
|
||||
},
|
||||
{
|
||||
"name": "attributed-box-cylinder-arc-round-seam-three-quarter-r2",
|
||||
"upstream_ref": "source/SM_API/src/SmApiBrep.cpp (SmApiFilletEdgesPerEdge)",
|
||||
"note": "A box 40 long, 10 wide and 15 high joined with SmApiCreateCylinder r 10, h 35 at (0, 0, -5) (below the box's base, so no face is coplanar); the box along -Y: the arc is centred 270 degrees from the seam. The concave arc where the box's top (z 15) meets the cylinder is rounded r 2 (SmApiFilletEdgesPerEdge, circular).",
|
||||
"inputs": {
|
||||
"attributed": {
|
||||
"api": "per_edge",
|
||||
"body": {
|
||||
"box_cylinder_union": {
|
||||
"a": [
|
||||
[
|
||||
-5,
|
||||
-40,
|
||||
0
|
||||
],
|
||||
[
|
||||
10,
|
||||
40,
|
||||
15
|
||||
]
|
||||
],
|
||||
"origin": [
|
||||
0,
|
||||
0,
|
||||
-5
|
||||
],
|
||||
"radius": 10,
|
||||
"height": 35
|
||||
}
|
||||
},
|
||||
"radius": 2.0,
|
||||
"xsect": 2,
|
||||
"edges": [
|
||||
9
|
||||
]
|
||||
}
|
||||
}
|
||||
}
|
||||
]
|
||||
}
|
||||
@@ -0,0 +1,15 @@
|
||||
Updates: WR-641
|
||||
|
||||
#### WR-641 (update, 2026-11-01). Under `fillet_through_extended_rail_end`, an end knot whose section is not found is skipped
|
||||
|
||||
WR-641 is from the flag audit that #1902 merged. This addendum only records the port's change under the same flag (tangency-labs/solidcore#1952).
|
||||
|
||||
- **Port change:** `c1_collect_points` (`crates/solidcore/src/blend/fillet_solver/p4.rs`) traces the extra through knots that `c1_extended_rail_end_knots` adds (d33890f's hunk). If `sm_FindFilletTsectPnt` finds no section at one of them, that knot is now skipped, and the trace runs on without it, as it does with the flag off. Before, the loop's `SER(SM_ERR)` aborted `CalcFilletGeom`. A failure at a G1 knot still aborts, as upstream's does. The flag stays off by default, so the pinned behaviour is unchanged.
|
||||
- **Second trigger:** a box 40 long, 10 wide and 15 high joined with `create_cylinder` r 10, h 30 at the origin (bases coplanar), with the arc where the box's top meets the cylinder rounded r 2.
|
||||
- At the arc's ends, the section's rail on the top face lies past the box's side face (y 6 against 5). That is outside the top plane's domain, so the section's plane solve finds nothing.
|
||||
- With the flag alone, every orientation of the box (+X, -X, +Y, -Y) failed with 1001 "Fillet Surface Computation Failure". Each now rounds as it does without the flag.
|
||||
- With `fillet_torus_span_from_start_section` (WR-568) on as well, all four round to the closed form 13998.81358767 (to 1e-5).
|
||||
- Without either flag, the -X and -Y boxes fail at their corner (1001), as the pinned C++ does (`blend.fillet_executive_p3`, `attributed-box-cylinder-arc-round-*`, with the cylinder's base moved below the box to avoid #2195).
|
||||
- **WR-641's own boss** (30 x 60 x 10 block, boss r 5 at (4, 33)) now rounds with WR-468 on, to 18807.824471782 (1e-9 relative). That is the fork's main without d33890f's hunk. The fork's main as it is still fails with 1001: the port deliberately differs from the fork here, under the flag.
|
||||
- **The fork's own d33890f trigger** (rig fillets/4369) is unchanged: 2.49e-8 from its closed form, with the flag on and off.
|
||||
- **Tests:** `crates/solidcore/tests/issue_1952_extended_rail_end_unsolved_section.rs` and `crates/solidcore/tests/wr_468_overhanging_boss_fork_parity.rs` (`wr_641_wr468_rounds_the_wall_boss_as_without_it`).
|
||||
Reference in New Issue
Block a user