mirror of
https://github.com/bherbruck/solidcraft.git
synced 2026-11-11 03:01:58 +00:00
Compare commits
2 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
 Daniel Toone
|
24355a8277 | ||
|
Daniel Toone
|
875493992a |
+1
-1
@@ -528,7 +528,7 @@ pub struct UpstreamFixes {
|
||||
/// from parallel ones, subtracting offset distance rather than adding it (WR-32,
|
||||
/// 2026-10-31 addendum). Off by default for pinned C++ parity.
|
||||
pub circle_offset_antiparallel_normal: bool,
|
||||
/// `SmBSplineCurve::AssertValid` measures a UV trim on its face's surface: the radius of curvature test (13, live with `bspline_check_min_radius`) and the coincident interior control point test (15) run on the trim mapped by the surface's first fundamental form at its middle (the Cholesky factor: exact on a plane), with the mapped trim's tolerance and spacing for test 13 (WR-560, #1524; tangency-labs/smlib b4f8c57, 54ca3e0).
|
||||
/// `SmBSplineCurve::AssertValid` measures a UV trim on its face's surface: the coincident interior control point test (15) runs on the trim mapped by the surface's first fundamental form at its middle (the Cholesky factor: exact on a plane), and tests 13/14 do not run on the trim at all -- its curve is a parameter-space representation of its edge's 3D curve, which the edge's own check carries (WR-560, #1524; tangency-labs/smlib b4f8c57, 54ca3e0; test 13 revised by #2785).
|
||||
pub uv_trim_checks_on_surface: bool,
|
||||
/// `SmBSplineCurve::AssertValid` takes a zero-speed tangent end of an edge's curve or UV trim (its end control points coincide, its second derivative does not vanish, the end is a vertex of the edge, and the edge's faces have parallel normals there within `SM_CONTINUITY_ANGLE`): test 3 accepts the coincident first control points, and tests 13/14 (live with `bspline_check_min_radius`) skip a radius below the tolerance within the stretch of the end where its growth `2 |C''|^3 / |C'' x C'''| |t - t_end|` puts it below. A G2 blend's end on a face tangent to its own leaves the vertex as a B-spline in `t ~ sqrt(s)`, with zero speed there (WR-668; tangency-labs/smlib ecf1963, the same patch as 6ca3405 in tangency-labs/smlib#76).
|
||||
pub curve_check_zero_speed_tangent_end: bool,
|
||||
|
||||
+238
-28
@@ -1417,7 +1417,8 @@ impl BSplineCurve {
|
||||
/// [`BSplineCurve::assert_valid`] of a UV trim on `trim_surface`, its face's
|
||||
/// surface: with `Kernel::fixes.uv_trim_checks_on_surface` (WR-560) the radius of
|
||||
/// curvature (13) and coincident control point (15) tests measure the trim on that
|
||||
/// surface; otherwise the same as [`BSplineCurve::assert_valid`].
|
||||
/// surface, so that the lengths they compare are the lengths of the trim's image;
|
||||
/// otherwise the same as [`BSplineCurve::assert_valid`].
|
||||
///
|
||||
/// # Errors
|
||||
///
|
||||
@@ -1823,15 +1824,37 @@ impl BSplineCurve {
|
||||
}
|
||||
}
|
||||
|
||||
// Not upstream (fork tangency-labs/smlib b4f8c57, WR-560, opt-in): a radius of
|
||||
// curvature is a length, and a UV trim's lengths are those of its face's
|
||||
// surface. The trim is tested mapped by the surface's first fundamental form
|
||||
// at its middle, with the mapped trim's tolerance and solution spacing; a
|
||||
// mapped trim that cannot be sized fails test 13 and is not analysed.
|
||||
// Not upstream (tangency-labs/solidcore#2785): the tolerance above is a speed,
|
||||
// `size_xyz / size_t`, which tests 11/12 compare with `|C'|`; a radius of
|
||||
// curvature is a length, so tests 13/14 compare it with a length of its own. The
|
||||
// fraction is upstream's own: its commented-out predecessor is
|
||||
// `dTol = dSizeXYZ / 10000` ("dTol proportional to length"), which `gwc` replaced
|
||||
// with the average speed for both tests (SmBSplineCurve.cpp, `AssertValid`).
|
||||
// Divided by `size_t` the tolerance was an average speed, so stretching a curve's
|
||||
// knot vector moved the kink verdict while the curve in space did not change
|
||||
// (#2785: a cubic Bezier of radius 5.8e-6 was reported up to T = 20 and accepted
|
||||
// from T = 50 on). The test is the dimensionless ratio `r / L >= 1/10000` of the
|
||||
// radius to the curve's extent, which is what makes it independent of the
|
||||
// parameterisation and of the units; `radius_tol` is that ratio times `L`.
|
||||
let mut radius_tol = if ovr.bounded {
|
||||
size_xyz / 10000.0
|
||||
} else {
|
||||
1e-03
|
||||
};
|
||||
let mut radius_sep = min_sep;
|
||||
let mut radius_sized = sized;
|
||||
|
||||
// Not upstream (fork tangency-labs/smlib b4f8c57, WR-560; revised by #2785): a
|
||||
// UV trim's lengths are those of its image on its face's surface, so the ratio is
|
||||
// taken there -- the trim mapped by the surface's first fundamental form at its
|
||||
// middle, with the mapped trim's extent and solution spacing. Both sides of the
|
||||
// comparison are then measured on the same image, which is what a radius of
|
||||
// curvature and a length have to be (a raw (u, v) radius is the surface's
|
||||
// anisotropy, not a length). A mapped trim that cannot be sized fails test 13 and
|
||||
// is not analysed.
|
||||
let mut mapped: Option<BSplineCurve> = None;
|
||||
let (mut radius_sized, mut radius_tol, mut radius_sep) = (sized, tol, min_sep);
|
||||
if sized && k.fixes.uv_trim_checks_on_surface {
|
||||
if let Some(l) = trim_metric(k, self, bounded, ovr.trim_surface, &domain)? {
|
||||
if let Some(l) = trim_metric(k, self, ovr.bounded, ovr.trim_surface, &domain)? {
|
||||
mapped = mapped_trim(self, l);
|
||||
let mapped_box = match mapped.as_ref() {
|
||||
Some(m) => ignored(m.calculate_bounding_box(
|
||||
@@ -1849,7 +1872,7 @@ impl BSplineCurve {
|
||||
rtn &= bool_report(&mut list, key, 13, radius_sized)?;
|
||||
if let Some(mapped_box) = mapped_box {
|
||||
let mapped_size = mapped_box.size().length();
|
||||
radius_tol = (mapped_size / size_t) / 10000.0;
|
||||
radius_tol = mapped_size / 10000.0;
|
||||
radius_sep = mapped_size / max2(20., size_t);
|
||||
}
|
||||
}
|
||||
@@ -1926,18 +1949,29 @@ impl BSplineCurve {
|
||||
}
|
||||
}
|
||||
}
|
||||
let mut first = solutions.first();
|
||||
if stretch.iter().any(|&s| s >= 0.0) {
|
||||
first = None;
|
||||
for s in &solutions {
|
||||
let p = s.param(0)?;
|
||||
let growth = (p - domain.min()).abs() <= stretch[0]
|
||||
|| (p - domain.max()).abs() <= stretch[1];
|
||||
if !(s.solution_value < radius_tol && growth) {
|
||||
first = Some(s);
|
||||
break;
|
||||
}
|
||||
// Not upstream (#2859, tangency-labs/solidcore#2343): a radius minimum on a
|
||||
// natural boundary end is not a kink -- the curve ends there, and the
|
||||
// first-derivative test above already passes over those solutions for the same
|
||||
// reason ("they are not a problem"). Without this a trim whose tightest bend is
|
||||
// at its own end is refused: #2343's countersink trim reads 1.51e-4 against
|
||||
// 1.69e-4 at its domain start.
|
||||
let mut first = None;
|
||||
for s in &solutions {
|
||||
let p = s.param(0)?;
|
||||
if domain
|
||||
.is_value_on_boundary(p, EFF_ZERO * (1.0 + domain.length()))
|
||||
.0
|
||||
{
|
||||
continue;
|
||||
}
|
||||
let growth = stretch.iter().any(|&x| x >= 0.0)
|
||||
&& ((p - domain.min()).abs() <= stretch[0]
|
||||
|| (p - domain.max()).abs() <= stretch[1]);
|
||||
if s.solution_value < radius_tol && growth {
|
||||
continue;
|
||||
}
|
||||
first = Some(s);
|
||||
break;
|
||||
}
|
||||
let first_radius = first.map_or(BIG_DOUBLE, |s| s.solution_value);
|
||||
if k.fixes.bspline_check_min_radius {
|
||||
@@ -2506,24 +2540,200 @@ mod tests {
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// WR-560 (fork tangency-labs/smlib b4f8c57, #891 item 2): the exact trim of a r 1
|
||||
/// circle on a plane spanning 600 x 3 is an ellipse 200 times as long as it is wide
|
||||
/// in (u, v), of radius 8.3e-6 at its ends against a tolerance of about 6.7e-5; on
|
||||
/// the plane it is the circle of radius 1. Test 13 needs `bspline_check_min_radius`
|
||||
/// to fire at all (WR-382).
|
||||
/// WR-560 (fork tangency-labs/smlib b4f8c57, #891 item 2), with #2785's length
|
||||
/// tolerance: the exact trim of a r 1 circle on a plane spanning 600 x 3 is an ellipse
|
||||
/// 200 times as long as it is wide in (u, v), of radius 8.3e-6 at its ends against a
|
||||
/// tolerance of about 6.7e-5; on the plane it is the circle of radius 1. A radius of
|
||||
/// curvature is a length, so the trim's radius is measured where its lengths are -- on
|
||||
/// its image in the surface's metric -- and there it is the circle's, far above
|
||||
/// `mapped_size / 10000`. Test 13 needs `bspline_check_min_radius` to fire at all
|
||||
/// (WR-382).
|
||||
#[test]
|
||||
fn issue_891_uv_trim_radius_is_measured_on_its_surface() {
|
||||
let mut k = Kernel::default();
|
||||
k.fixes.bspline_check_min_radius = true;
|
||||
let (trim, plane) = circle_trim_on_plane((300.0, 1.5, 1.0), (600.0, 3.0));
|
||||
// in raw (u, v) the ellipse's ends are a bend of radius 8.3e-6
|
||||
let idx = failed_tests(&k, &trim, Some(&plane));
|
||||
assert!(idx.contains(&13), "{idx:?}");
|
||||
// on its surface the same trim is the circle of radius 1, which is no bend
|
||||
k.fixes.uv_trim_checks_on_surface = true;
|
||||
let idx = failed_tests(&k, &trim, Some(&plane));
|
||||
assert!(!idx.contains(&13), "{idx:?}");
|
||||
// a curve that is no face's trim is tested as it is
|
||||
let idx = failed_tests(&k, &trim, None);
|
||||
assert!(idx.contains(&13), "{idx:?}");
|
||||
}
|
||||
|
||||
/// #2785: a UV trim whose *image* is bad is still reported, and the tolerance the
|
||||
/// report carries is the length `mapped_size / 10000` -- the dimensionless ratio
|
||||
/// `r / L >= 1/10000` with `L` the trim's extent on its surface.
|
||||
///
|
||||
/// The trim is the issue's cubic `(0,0) (h,0) (h,h) (1,1)`, `h = 1e-4`, in a 1 x 1
|
||||
/// plane's parameters: its image on the plane is that cubic, of radius 5.8e-6, well
|
||||
/// under the plane-mapped tolerance `sqrt(2) / 10000`.
|
||||
#[test]
|
||||
fn issue_2785_a_uv_trim_whose_image_bends_is_reported() {
|
||||
let mut k = Kernel::default();
|
||||
k.fixes.bspline_check_min_radius = true;
|
||||
k.fixes.uv_trim_checks_on_surface = true;
|
||||
let (_, plane) = circle_trim_on_plane((0.5, 0.5, 0.4), (1.0, 1.0));
|
||||
let h = 1e-4;
|
||||
let trim = trim_in_uv(&[
|
||||
Vec3::new(0.0, 0.0, 0.0),
|
||||
Vec3::new(h, 0.0, 0.0),
|
||||
Vec3::new(h, h, 0.0),
|
||||
Vec3::new(1.0, 1.0, 0.0),
|
||||
]);
|
||||
let mut list = AssertArray::new();
|
||||
assert!(!trim
|
||||
.assert_valid_on(&k, Some(&mut list), true, ObjectKey(5), Some(&plane))
|
||||
.unwrap());
|
||||
let reports: Vec<_> = list
|
||||
.reports()
|
||||
.iter()
|
||||
.filter(|r| r.test_index == 13)
|
||||
.collect();
|
||||
assert_eq!(reports.len(), 1, "{list:?}");
|
||||
let report = reports[0];
|
||||
// the bend is the issue's, at the same radius
|
||||
assert!(
|
||||
(report.value - 5.8155015634573e-6).abs() <= 1e-9 * report.value,
|
||||
"{}",
|
||||
report.value
|
||||
);
|
||||
// and the tolerance is a length: the trim's extent on the plane, / 10000
|
||||
let expected = 2f64.sqrt() / 10000.0;
|
||||
assert!(
|
||||
(report.tol3d - expected).abs() <= 1e-6 * expected,
|
||||
"{} against {expected}",
|
||||
report.tol3d
|
||||
);
|
||||
}
|
||||
|
||||
/// The cubic `(0,0) (h,0) (h,h) (1,1)` of #2785, as a 2D curve of a face's
|
||||
/// parameter space (`dim` 2, one span, no weights).
|
||||
fn trim_in_uv(pts: &[Vec3]) -> BSplineCurve {
|
||||
use crate::nurbs::b_spline_curve::p1::{BSplineCurveForm, CanonicalData, KnotType};
|
||||
BSplineCurve::create_canonical(&CanonicalData {
|
||||
dim: 2,
|
||||
degree: 3,
|
||||
control_points: pts,
|
||||
form: BSplineCurveForm::Unspecified,
|
||||
knot_multiplicities: &[4, 4],
|
||||
knots: &[0.0, 1.0],
|
||||
knot_type: KnotType::Unspecified,
|
||||
weights: None,
|
||||
trim: None,
|
||||
})
|
||||
.unwrap()
|
||||
}
|
||||
|
||||
/// tangency-labs/solidcore#2785: tests 13/14 compared a radius of curvature, a length,
|
||||
/// with `tol`, the curve's average speed (`size_xyz / size_t / 10000`), so the two
|
||||
/// sides had different units and the verdict moved when the curve was reparametrised
|
||||
/// although no geometry changed.
|
||||
///
|
||||
/// The curve is the cubic Bezier `(0,0) (h,0) (h,h) (1,1)` with `h = 1e-4`: one sharp
|
||||
/// bend near its start, of radius 5.815568702310391e-6 everywhere (the search's own
|
||||
/// first solution is 5.8155015634573e-6, #2637). Its knot vector is stretched to
|
||||
/// `[0,0,0,0,T,T,T,T]`, so the curve in space is the same for every `T` and only the
|
||||
/// parametrisation differs. The old speed tolerance is `sqrt(2) / T / 10000`, which
|
||||
/// passes below the true radius between `T = 20` and `T = 50`: the bend was reported at
|
||||
/// `T` up to 20 and silently accepted from 50 on.
|
||||
#[test]
|
||||
fn issue_2785_radius_tolerance_is_a_length_not_a_speed() {
|
||||
let mut k = Kernel::default();
|
||||
k.fixes.bspline_check_min_radius = true;
|
||||
let h = 1e-4;
|
||||
// the search's first solution, the same at every T (#2637, #2785)
|
||||
let radius = 5.8155015634573e-6;
|
||||
let mut tol: Option<f64> = None;
|
||||
let mut value: Option<f64> = None;
|
||||
for t in [1.0, 10.0, 20.0, 50.0, 1e2, 1e3, 1e4] {
|
||||
let c = curve(
|
||||
3,
|
||||
&[[0.0, 0.0, 0.0], [h, 0.0, 0.0], [h, h, 0.0], [1.0, 1.0, 0.0]],
|
||||
&[0.0, 0.0, 0.0, 0.0, t, t, t, t],
|
||||
);
|
||||
let mut list = AssertArray::new();
|
||||
let ok = c
|
||||
.assert_valid(&k, Some(&mut list), true, ObjectKey(9))
|
||||
.unwrap();
|
||||
let reports: Vec<_> = list
|
||||
.reports()
|
||||
.iter()
|
||||
.filter(|r| r.test_index == 14)
|
||||
.collect();
|
||||
assert_eq!(reports.len(), 1, "T = {t}: {list:?}");
|
||||
assert!(!ok, "T = {t}");
|
||||
let report = reports[0];
|
||||
// the same bend, at the same radius, for every parametrisation
|
||||
assert!(
|
||||
(report.value - radius).abs() <= 1e-9 * radius,
|
||||
"T = {t}: {} against {radius}",
|
||||
report.value
|
||||
);
|
||||
match value {
|
||||
None => value = Some(report.value),
|
||||
// the search's own value moves by a rounding or two, not with T
|
||||
Some(first) => {
|
||||
assert!((first - report.value).abs() <= 1e-12 * first, "T = {t}");
|
||||
}
|
||||
}
|
||||
// and the same tolerance, which is a length at part scale
|
||||
match tol {
|
||||
None => tol = Some(report.tol3d),
|
||||
Some(first) => assert_eq!(first, report.tol3d, "T = {t}"),
|
||||
}
|
||||
}
|
||||
// the box diagonal is sqrt 2 whatever the parametrisation, so the length
|
||||
// tolerance is size_xyz / 10000 rather than the old speed sqrt(2) / T / 10000
|
||||
let expected = 2f64.sqrt() / 10000.0;
|
||||
assert!((tol.unwrap() - expected).abs() <= 1e-12, "{tol:?}");
|
||||
|
||||
// With the default fixes the radius is never read (WR-382), so no test 13/14
|
||||
// report depends on the tolerance at all: the difftest, which runs the default,
|
||||
// sees the same reports before and after this fix.
|
||||
for t in [1.0, 20.0, 1e2, 1e4] {
|
||||
let c = curve(
|
||||
3,
|
||||
&[[0.0, 0.0, 0.0], [h, 0.0, 0.0], [h, h, 0.0], [1.0, 1.0, 0.0]],
|
||||
&[0.0, 0.0, 0.0, 0.0, t, t, t, t],
|
||||
);
|
||||
let mut list = AssertArray::new();
|
||||
assert!(
|
||||
c.assert_valid(&Kernel::default(), Some(&mut list), true, ObjectKey(9))
|
||||
.unwrap(),
|
||||
"T = {t}: {list:?}"
|
||||
);
|
||||
assert!(list.is_empty(), "T = {t}: {list:?}");
|
||||
}
|
||||
}
|
||||
|
||||
/// The other side of #2785: a bend that is not a kink is accepted at every
|
||||
/// parametrisation. The same cubic with `h = 0.25` has a radius of about 0.13, far
|
||||
/// above the length tolerance `sqrt(2) / 10000 = 1.4e-4`, so no `T` may report it.
|
||||
#[test]
|
||||
fn issue_2785_a_regular_bend_is_accepted_at_every_parametrisation() {
|
||||
let mut k = Kernel::default();
|
||||
k.fixes.bspline_check_min_radius = true;
|
||||
for t in [1.0, 20.0, 1e2, 1e4] {
|
||||
let c = curve(
|
||||
3,
|
||||
&[
|
||||
[0.0, 0.0, 0.0],
|
||||
[0.25, 0.0, 0.0],
|
||||
[0.25, 0.25, 0.0],
|
||||
[1.0, 1.0, 0.0],
|
||||
],
|
||||
&[0.0, 0.0, 0.0, 0.0, t, t, t, t],
|
||||
);
|
||||
let mut list = AssertArray::new();
|
||||
assert!(
|
||||
c.assert_valid(&k, Some(&mut list), true, ObjectKey(9))
|
||||
.unwrap(),
|
||||
"T = {t}: {list:?}"
|
||||
);
|
||||
assert!(list.is_empty(), "T = {t}: {list:?}");
|
||||
}
|
||||
}
|
||||
|
||||
/// WR-560 (fork tangency-labs/smlib 54ca3e0, #1524): the trim of a r 0.003 hole on a
|
||||
|
||||
Binary file not shown.
|
After Width: | Height: | Size: 63 KiB |
File diff suppressed because one or more lines are too long
|
After Width: | Height: | Size: 25 KiB |
File diff suppressed because it is too large
Load Diff
Vendored
+193
@@ -0,0 +1,193 @@
|
||||
# `SmBSplineCurve::AssertValid` tests 13/14 compare a radius of curvature with a speed
|
||||
|
||||
Unit: `nurbs.b_spline_curve_p3` (`crates/solidcore/src/nurbs/b_spline_curve/p3.rs`).
|
||||
Issue: tangency-labs/solidcore#2785. Fix: this PR (a redo of #2859, reverted by #3019; the design
|
||||
below follows the review of #3064).
|
||||
|
||||
|
||||
|
||||
*Figure. The mechanism, drawn for #2859 by @saosebastiao in VectorCraft 0.21.1 through its MCP
|
||||
server and exported with live text (`outlineText: false`); `issue-2785-radius-tolerance.vectorcraft`
|
||||
beside it is the source.*
|
||||
|
||||
## The defect
|
||||
|
||||
`BSplineCurve::assert_valid` built one tolerance and used it for two different kinds of quantity:
|
||||
|
||||
```rust
|
||||
let size_t = domain.length();
|
||||
let tol = if bounded { (size_xyz / size_t) / 10000.0 } else { 1e-03 }; // a speed
|
||||
let (mut radius_sized, mut radius_tol, mut radius_sep) = (sized, tol, min_sep); // a length
|
||||
```
|
||||
|
||||
`size_xyz / size_t` is a length per parameter — the curve's average speed, the right unit for tests
|
||||
11/12, which compare `|C'|`, and for the `speed_clear` bound above them. A radius of curvature is a
|
||||
**length**, so tests 13/14 compared a length with a speed, and stretching a curve's knot vector
|
||||
moved the verdict although the curve in space did not change. The mapped UV-trim branch
|
||||
(`radius_tol = (mapped_size / size_t) / 10000.0`) had the same shape, and
|
||||
`zero_speed_radius_stretch` (WR-668) divides `radius_tol` by the radius' growth rate to get a
|
||||
stretch in *parameter* units, which the speed made dimensionless.
|
||||
|
||||
## The fraction is upstream's own, and it is a fraction of a length
|
||||
|
||||
`SmBSplineCurve::AssertValid` (SmBSplineCurve.cpp:7883) reads
|
||||
|
||||
```cpp
|
||||
double dTol = IsBounded() ? (dSizeXYZ/dSizeT) / 10000 // dTol proportional to average curve speed
|
||||
: 1e-03 ;
|
||||
```
|
||||
|
||||
and directly above it, commented out by `gwc`:
|
||||
|
||||
```cpp
|
||||
// gwc:removed old tolerances in favor of tolerances proportional to average curve speed
|
||||
// double dTol = IsBounded() ? dSizeXYZ / 10000 // dTol proportional to length
|
||||
```
|
||||
|
||||
So `1/10000` is not fitted here: it is upstream's own fraction, and the quantity upstream applied
|
||||
it to **before** `gwc` changed it was the length `dSizeXYZ`, not the average speed. The fix
|
||||
restores upstream's own length form. Written as the ratio it is, the test is
|
||||
`r_min / L >= 1/10000` with `r_min` and `L` measured on the same image of the curve — a
|
||||
dimensionless comparison, which is why the parameterisation, the units of the model and the
|
||||
anisotropy of a surface's (u, v) cannot move it.
|
||||
|
||||
## Measured: the issue's curve
|
||||
|
||||
`(0,0) (h,0) (h,h) (1,1)`, `h = 1e-4`, on `[0,0,0,0,T,T,T,T]`: the same curve in space for every
|
||||
`T`, smallest radius `5.815568702310391e-6` (the search's first solution is `5.8155015634573e-6`,
|
||||
#2637).
|
||||
|
||||
| `T` | old tolerance `sqrt(2)/T/1e4` | new tolerance `sqrt(2)/1e4` | old verdict | new verdict |
|
||||
|---|---|---|---|---|
|
||||
| 1 | 1.414212e-4 | 1.414212e-4 | reported | reported |
|
||||
| 20 | 7.071068e-6 | 1.414212e-4 | reported | reported |
|
||||
| 50 | 2.828425e-6 | 1.414212e-4 | **not reported** | reported |
|
||||
| 1e4 | 1.414212e-8 | 1.414212e-4 | **not reported** | reported |
|
||||
|
||||
The old tolerance crosses the true radius at `T = 24.3177`. `issue_2785_radius_tolerance_is_a_
|
||||
length_not_a_speed` pins the same radius and the same tolerance at seven `T`, and
|
||||
`issue_2785_a_regular_bend_is_accepted_at_every_parametrisation` the accepting side.
|
||||
|
||||
## A UV trim is measured on its image, not skipped
|
||||
|
||||
The review of #3064 asked for two things that this record settles:
|
||||
|
||||
1. **Tests 13/14 must run on a UV trim.** They do. A trim's lengths are those of its image on its
|
||||
face's surface, so the ratio is taken there: the trim mapped by the surface's first fundamental
|
||||
form at its middle (WR-560's `sm_TrimMetric` / `sm_MappedTrim`, which test 15 already uses),
|
||||
with the mapped trim's extent and solution spacing. Both sides of the comparison are then
|
||||
measured on the same image. `issue_891_uv_trim_radius_is_measured_on_its_surface` is WR-560's
|
||||
own case: a r 1 circle's exact trim on a plane spanning 600 x 3 is an ellipse of radius 8.3e-6
|
||||
in raw (u, v) — reported — and the circle of radius 1 on the plane, which is not. A trim whose
|
||||
image really bends is still reported: `issue_2785_a_uv_trim_whose_image_bends_is_reported` puts
|
||||
the issue's cubic in a 1 x 1 plane's parameters and reads test 13 at radius 5.8155e-6 against
|
||||
`sqrt(2)/1e4`.
|
||||
2. **The rig case must pass on its own geometry.** It does not, and the cause is not the check.
|
||||
See below; the check is right about what it measures.
|
||||
|
||||
## A radius minimum on the curve's own end is not a kink
|
||||
|
||||
Restored from #2859. The first-derivative test (11/12) passes over solutions on the natural
|
||||
boundary ends with the comment "they are not a problem"; the radius test read them, so a curve
|
||||
whose tightest bend is at its own end was refused. `crashes/2992`'s trims are not the only case:
|
||||
`issue_2343_curved_setback::countersink_asked_corner_is_certified_with_its_nets` (every flag on)
|
||||
refuses the countersink corner with test 13 at radius `1.51e-4` against `1.69e-4`, at
|
||||
`owner_param = 0.0` — the trim's own domain start. With the boundary-end solutions passed over the
|
||||
corner is certified again, and the test is the regression test for this half.
|
||||
|
||||
## The rig case: `crashes/2992-join-disk-and-36-twisted-loft-blades`
|
||||
|
||||
#2859 was reverted because this case's `union_all` (call 111) came back "the result is not a closed
|
||||
solid" (`is_outward_solid` runs `check_brep` at `Level0`, whose `AssertValid` is the check). With
|
||||
the length tolerance and the mapped measurement, the case still refuses, and the trims it reports
|
||||
are not kinked: their images have a curvature spike where their 3D curves are smooth.
|
||||
|
||||
Measured on the union's result body (the corpus's raw counts, 185 faces, 549 edges, 366 vertices;
|
||||
the rig's `fixes.json` with `bspline_check_min_radius` off so that the union completes). Of 1898 UV
|
||||
trims, the smallest image radius is `1.708027e-4` at `t = 60.694388050` of `[0, 61.38314]`, where
|
||||
the 3D curve's radius is `234.2`; the mapped size is `75.68592`, so the mapped length tolerance is
|
||||
`7.5686e-3` — the report is 44x below it. The next ones are `2.665216e-4` at `54.948894080` (3D
|
||||
radius `75.1`) and `4.322271e-4` at `60.694447967` (3D radius `234.5`).
|
||||
|
||||
The trim that decides it, at its smallest image radius: 121 control points, 41 distinct knots,
|
||||
every interior knot of multiplicity 3 — one cubic Bézier piece per 3D curve span — on a loft whose
|
||||
`v_knots` are the integers `0…19` (so the surface is only C0 in v at each of them, `|S_v|`
|
||||
jumping). The spike sits on the last knot of the cluster `54.943193, 54.948888, 54.948969`, where
|
||||
the trim crosses the surface's `v = 12` line:
|
||||
|
||||
| offset from the spike, x the trim's domain | `\|C'\|` of the image | image radius | the 3D curve's radius | image − 3D curve | tangent, image vs 3D |
|
||||
|---|---|---|---|---|---|
|
||||
| −1e-6 | 1.000000 | 75.244 | 75.244 | 3e-14 | 1e-12 rad |
|
||||
| −1e-7 | 1.000000 | 3.976e-4 | 75.434 | 5e-14 | 1e-10 rad |
|
||||
| −1e-8 | 1.142142 | 6.566e-4 | 75.402 | 4.0e-7 | 1.16e-2 rad |
|
||||
| −1e-9 | 0.715812 | 2.652e-4 | 75.379 | 2.9e-7 | 3.70e-2 rad |
|
||||
| 0 | 0.716720 | **2.665e-4** | 75.389 | 2.7e-7 | 3.68e-2 rad |
|
||||
| +1e-7 | 0.802034 | 4.254e-4 | 75.368 | 1.2e-6 | 2.30e-2 rad |
|
||||
| +1e-6 | 1.071769 | 4.052e-3 | 75.004 | 5.5e-7 | 6.2e-3 rad |
|
||||
| +1e-5 | 1.000000 | 75.255 | 75.431 | 5e-12 | 1.7e-8 rad |
|
||||
|
||||
The image is smooth to `2.7e-7` mm — 74x inside the edge's `2e-5` mm tolerance — and its tangent
|
||||
turns `3.7e-2` rad and returns within `1e-5` of the domain. Its speed dips to `0.717` where the 3D
|
||||
curve's is `1`. So the bend is an artefact of how the trim is represented, not of the model's
|
||||
geometry: the drop (`sm_AddToCurve`, WR-753) emits one Bézier piece per 3D curve span, and the
|
||||
surface's C0 knot line at `v = 12` falls `1.8e-5` inside a span, so the dropped UV derivative's
|
||||
jump there cannot be carried by the piece. The fix is in the drop, not in the check: break the
|
||||
pieces at the surface's knot lines. That moves every pcurve crossing one and needs its own sweep;
|
||||
it is WR-753 and not this PR.
|
||||
|
||||
**Measuring on the mapped trim does not avoid this**, which is why the review's alternative was
|
||||
measured rather than assumed: the mapped trim is a linear image of the same pcurve, so the notch is
|
||||
in it too (the author's earlier measurement read `7.79e-4` on the mapped trim against `8.50e-4` on
|
||||
the true image), and the true image's radius is smaller still (`1.7e-4` here). Neither the raw
|
||||
(u, v) radius nor the mapped one is the model's geometry; both carry the pcurve's own notch.
|
||||
|
||||
## What moves in the corpus
|
||||
|
||||
`bspline_check_min_radius` is `false` in `UpstreamFixes::default()`, so `min_radius` is never read
|
||||
and 13/14 never fire on the default path; the goldens and the difftest cannot see this change, and
|
||||
`issue_2785_radius_tolerance_is_a_length_not_a_speed` pins that at four `T`. `uv_trim_checks_on_
|
||||
surface` is `false` there too, so the mapped branch does not run on the default path either.
|
||||
Goldens `nurbs.b_spline_curve_p3`: `cases=2460 changed=0 added=0 removed=0 broken=0
|
||||
known_failing=0`.
|
||||
|
||||
With the rig's `fixes.json` (both flags on), the whole rig — `cargo test --profile goldens -p
|
||||
solidcore-ops --test kernel_cases -- --ignored`, 909 cases:
|
||||
|
||||
| area | cases | pass | fail | unsupported | known-failing |
|
||||
|---|---|---|---|---|---|
|
||||
| crashes | 12 | 8 | **1** | 3 | 0 |
|
||||
| booleans | 124 | 112 | 0 | 12 | 0 |
|
||||
| fillets | 274 | 239 | 0 | 16 | 19 |
|
||||
| sweeps | 95 | 88 | 0 | 7 | 0 |
|
||||
| sheet-metal | 78 | 12 | 0 | 66 | 0 |
|
||||
| surfaces | 62 | 40 | 0 | 22 | 0 |
|
||||
| shell | 36 | 33 | 0 | 2 | 1 |
|
||||
| the remaining areas and ladder tiers | 228 | 111 | 0 | 117 | 0 |
|
||||
| **all 909** | 909 | 643 | **1** | 245 | 20 |
|
||||
|
||||
The one failure is `crashes/2992-join-disk-and-36-twisted-loft-blades`, and it is WR-753: the
|
||||
union refuses because a trim's image has a curvature spike that its 3D curve does not. Every other
|
||||
case is unchanged, and `issue_2343_curved_setback::countersink_asked_corner_is_certified_with_its_
|
||||
nets` passes again with the boundary-end solutions passed over.
|
||||
|
||||
## What this leaves for #2785's flag
|
||||
|
||||
WR-382's `bspline_check_min_radius` is still off by default, so this reaches no user until the flag
|
||||
is retired. Retiring it is the follow-up #2859 listed: it needs a sweep of every case it makes the
|
||||
check run, and WR-753 first, or the sweep refuses `crashes/2992-…`.
|
||||
|
||||
The other alternative, measured and not taken: gate `radius_tol` at the model's tolerance — the
|
||||
edge's `tau_e`, or `SM_ZONE_TOL_3D` for a bare curve, as test 15 does with `SM_ZONE_TOL_3D / 10`.
|
||||
It accepts this trim (`2.7e-4` against an edge tolerance of `2e-5`) and would keep the rig green,
|
||||
but it turns a part-relative test into an absolute one, it needs the entity's tolerance plumbed into
|
||||
`assert_valid`, and it does not fix the notch — it hides it. WR-753 is the root cause.
|
||||
|
||||
Two more follow-ups, not in this PR:
|
||||
|
||||
1. **WR-753** — break the drop's UV pieces at the surface's knot lines, so no piece straddles a C0
|
||||
line. That is what makes `crashes/2992-…` pass tests 13/14 on its own geometry; it moves every
|
||||
pcurve that crosses a surface knot line and needs its own sweep.
|
||||
2. `radius_sep` is `size_xyz / max2(20., size_t)` (and `mapped_size / max2(20., size_t)` for a
|
||||
trim): a length over a parameter count below 20, a length per parameter above it. It is the
|
||||
search's resolution rather than a compared quantity, so it cannot move a verdict the way the
|
||||
tolerance did, but it is the one mixing left in the function and wants the same audit.
|
||||
Vendored
+59
@@ -0,0 +1,59 @@
|
||||
#### WR-753. `sm_AddToCurve` places a dropped UV curve's pieces at the 3D curve's spans, so a surface knot line inside a span becomes a curvature spike
|
||||
|
||||
- **Severity:** Wrong result (a valid body refused: a UV trim's radius of curvature is read as a
|
||||
kink where the trim's image is smooth to a fraction of the edge's tolerance)
|
||||
- **Area:** Booleans, surfaces and volumes (surface drop library)
|
||||
- **Location:** `source/SMLib/src/SmSurfaceDropLib.cpp:3439`, `sm_AddToCurve`, and its caller's span
|
||||
walk `DropCurve` (`:5621` region), at v1.0.1 (`3fd5b9548d19aa8c12c29bc9637a96d42a06f8a5`; the
|
||||
port is `crates/solidcore/src/intersect/surface_drop_lib/p2.rs`, `add_to_curve`, and
|
||||
`p3.rs`, `drop_curve_base_in`).
|
||||
- **What happens:** the drop walks the 3D curve's own spans and emits one cubic Hermite Bézier
|
||||
piece per span, with `dScale = crEnd.m_dT - crStart.m_dT` (`:3459-3463`). The pieces' breakpoints
|
||||
are therefore the 3D curve's knots. A surface that is only C0 across a knot line — a loft through
|
||||
sections is C0 in v at every section, its `|S_v|` jumping there — has a discontinuous tangent
|
||||
plane on that line, so the dropped UV derivative (`m_UVPD[1]`, the 3D derivative projected onto
|
||||
the offset surface's tangent plane) jumps across it. When the surface's knot line falls strictly
|
||||
inside a 3D span, no cubic piece can carry that jump: the piece's interior control point is
|
||||
placed from the derivative at the span end, which is enormous next to the span's UV chord, and
|
||||
the piece loops. The image `S(u(t), v(t))` then has a curvature spike — a notch a fraction of
|
||||
the edge tolerance deep — although the 3D curve is smooth there. Nothing in the drop checks
|
||||
whether a surface knot line lies inside the span it is about to emit.
|
||||
- **Trigger:** `crates/solidcore/tests/kernel_cases/crashes/2992-join-disk-and-36-twisted-loft-
|
||||
blades` (SolidCraft's `crashes/2992`), `union_all` call 111. Its 36 blades are lofts whose
|
||||
surfaces are C0 in v at every integer v (`v_knots` `0,0,0,0, 1,1,1, 2,2,2, … 19,19,19,19`), and
|
||||
the intersection curve's spans end within `2e-5` of those lines but not on them.
|
||||
- **How found:** tangency-labs/solidcore#2785 (the radius of curvature is a length, so tests 13/14
|
||||
compare it with a length; `plan/findings/nurbs.b_spline_curve_p3--issue-2785-radius-tolerance-is-
|
||||
a-length.md`). The check's report on that case was traced to a UV trim whose image has a
|
||||
curvature spike where its 3D curve does not.
|
||||
- **Confirmed:** yes, on the port, from the union's result body (185 faces, 549 edges, 366
|
||||
vertices, the corpus's raw counts). One trim of it (`[0, 61.38314]`, 121 control points, 41
|
||||
distinct knots, every interior knot of multiplicity 3 — one Bézier piece per 3D span — on a loft
|
||||
whose `v_knots` are the integers 0…19), at its smallest image radius:
|
||||
|
||||
| offset from the spike, x the trim's domain | `\|C'\|` of the image | image radius | the 3D curve's radius | image − 3D curve | tangent, image vs 3D |
|
||||
|---|---|---|---|---|---|
|
||||
| −1e-6 | 1.000000 | 75.244 | 75.244 | 3e-14 | 1e-12 rad |
|
||||
| −1e-7 | 1.000000 | 3.976e-4 | 75.434 | 5e-14 | 1e-10 rad |
|
||||
| −1e-8 | 1.142142 | 6.566e-4 | 75.402 | 4.0e-7 | 1.16e-2 rad |
|
||||
| −1e-9 | 0.715812 | 2.652e-4 | 75.379 | 2.9e-7 | 3.70e-2 rad |
|
||||
| 0 | 0.716720 | **2.665e-4** | 75.389 | 2.7e-7 | 3.68e-2 rad |
|
||||
| +1e-7 | 0.802034 | 4.254e-4 | 75.368 | 1.2e-6 | 2.30e-2 rad |
|
||||
| +1e-6 | 1.071769 | 4.052e-3 | 75.004 | 5.5e-7 | 6.2e-3 rad |
|
||||
| +1e-5 | 1.000000 | 75.255 | 75.431 | 5e-12 | 1.7e-8 rad |
|
||||
|
||||
The spike is at the last of the trim's knot cluster `54.943193, 54.948888, 54.948969` (each of
|
||||
multiplicity 3), and the trim crosses the surface's `v = 12` line there; the 3D curve's radius is
|
||||
75.4 on both sides and its speed is 1 throughout. The image's position error peaks at `2.7e-7`
|
||||
mm against the edge's `2e-5` mm tolerance, and its tangent turns `3.7e-2` rad and returns within
|
||||
`1e-5` of the domain — the image is smooth to well inside the model's tolerance, and only its
|
||||
second derivative is not. On the same body, 184 pcurves have more than 40 knots and 1898 UV trims
|
||||
were scanned; the smallest image radius over all of them is `1.71e-4` at a knot cluster of the
|
||||
same shape, against a 3D radius of `234.2` there.
|
||||
- **Rust port:** faithful to the pinned C++ (`add_to_curve` reproduces `sm_AddToCurve`'s
|
||||
`dScale`-scaled Hermite span, `flush_curve` its `[4 3 … 3 4]` piecewise-Bézier curve), so the
|
||||
notch is upstream's. Not fixed here: the fix is to break the drop's pieces at the surface's knot
|
||||
lines (so no piece straddles a C0 line), which moves every pcurve that crosses one and needs its
|
||||
own sweep. The radius test on a trim reports it as a kink (tests 13/14, `SmBSplineCurve::
|
||||
AssertValid`), which is why `Kernel::fixes.bspline_check_min_radius` (WR-382) is off by default
|
||||
and why the kernel rig, which turns it on, refuses this case.
|
||||
Reference in New Issue
Block a user