Skip to content

fix: a surface and a plane's curve traced on the plane, opt-in (smlib#78 5bdcddf, 14a6daa, 4c6571b, #2250) - #2253

Merged
bherbruck merged 5 commits into
tangency-labs:mainfrom
MarkusReichart:fix/smlib78-surface-plane-curve
Nov 1, 2026
Merged

bherbruck merged 5 commits into
tangency-labs:mainfrom
MarkusReichart:fix/smlib78-surface-plane-curve

Conversation

@MarkusReichart

@MarkusReichart MarkusReichart commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Part of #2250 (smlib#78), coordinated on #2243: tangency-labs/smlib 5bdcddf, 14a6daa and 4c6571b, sm_SurfacePlaneCurve and its call in sm_SrfSrfIntersection, as the fork has them (unchanged through d01d7b0). Opt-in behind Kernel::fixes.fillet_surface_plane_curve_on_plane_equation (WR-682). With the flag off, nothing changes.

Units: blend.fillet_geom_p2 (sm_SrfSrfIntersection)
In scope: sm_SurfacePlaneCurve (5bdcddf), its quarter points and start direction (14a6daa), a flat surface left to the analytic intersector (4c6571b), and the condition of the call: no analytic curve, both points given, exactly one plane, a surface that isn't flat, both points on both surfaces, and the surfaces crossing at both
Expected edits: crates/solidcore/src/blend/fillet_geom/surface_plane_curve.rs (new), crates/solidcore/src/blend/fillet_geom/p2.rs, crates/solidcore/src/blend/fillet_geom.rs, crates/solidcore/src/kernel.rs, crates/solidcore/tests/wr682_surface_plane_curve.rs (new), one pinned figure in crates/solidcore/tests/issue_2176_round_of_arc_between_tangent_straights.rs, plan/upstream-issues/WR-682--srfsrf-plane-curve-ends-at-the-plane-s-domain.md (new)
Evidence: below
Related PRs / dependencies: #2243 (coordination), #2250, #2252 (the section cases of the same step). Nothing here depends on 11ea2d9
Available follow-ups: the other commits of #2250. This PR claims nothing else
Status / blockers / next step: complete on 32aea3de1; CI runs when a maintainer approves it

What it changes

A plane goes on as itself past the face it was made for. sm_SrfSrfIntersection meets a plane and a surface with the analytic intersector, bounded by the plane's domain, and then with intersectors that march both surfaces in their domains. Where the curve between two points on both leaves the plane's domain, none of them reaches the end.

With the flag, such a curve is the zero set of the plane's equation on the surface, traced from one point to the other by minimum-norm Newton steps. Its 3D curve and its two uv curves are cubic Hermite interpolants of the stations, a span split where a quarter point is off the exact curve by more than a tenth of the tolerance.

The case: a B-spline cylinder R 10 and the plane z = 20 + x / 2 on a patch that holds their ellipse's points at 45 and 135 degrees, but not the arc between them, which reaches y = 10.

Flag off Flag on
Result status 1001 the curve, ends on the given points to 1e-12
2001 samples on both surfaces n/a within 1e-5
Its uv curves back onto it n/a within 1e-5, either order of the surfaces
Length against the closed-form ellipse n/a within 1e-4, the short way and, with a start direction, the long way

Compared in full with the flag on and off, and identical: an analytic answer (the cylinder as itself), a flat B-spline surface, a surface touching the plane, an end off a surface.

For the maintainers

  • A pinned figure changes in another test. issue_2176's the_neighbours_keep_their_rounds runs with every flag SolidCraft turns on, so this one too. The round of the edge at (4, 28, 2) removes 0.375911976512 with the flag and 0.375911985299 without, 8.8e-9 less. The body is sound either way: same face count, manifold, check level 2 clean. I re-pinned that one figure and kept the tolerance at 1e-9. The test's volume accuracy is 1e-8. I can't say which of the two figures is nearer the true volume.
  • SolidCraft's preset. It turns every flag on except geometric_g2_continuity, so it would pick this flag up.
  • "Difftest against the fork revision". The harness builds only usd-brep at the pin, so this is not compared with the fork's C++, and bit-identical results with the fork are not shown. The checks here are the closed-form ones above. The question is open in fix: a rail's end on its side edge is the rail/edge crossing, opt-in (smlib#76 9bebf34, a87c502, #2243) #2245.
  • Kept as the fork has them, so you may want to look at them:
    • The plane's uv curve runs past the plane's domain, where Plane::evaluate_point clamps unless out-of-bounds evaluation is on. The test reads it with evaluate_point_fast.
    • The plane's origin is read at uv (0, 0), which clamps too when the domain doesn't hold it.
    • A trace that runs off the surface's domain isn't stopped by that. It ends at the routine's limit of 100000 steps. A start direction across a cylinder's seam took more than 5 minutes in a debug build before I stopped it.

Deviations from the fork are in the header of surface_plane_curve.rs: out-params become return values, the context argument is dropped, and UVRate's (void)Eval becomes ?.

Tests

crates/solidcore/tests/wr682_surface_plane_curve.rs:

  • the_curve_runs_past_the_plane_s_domain_to_its_end
  • the_uv_curves_follow_the_order_of_the_surfaces
  • the_start_direction_chooses_the_way_round
  • without_the_flag_the_curve_is_not_found
  • an_analytic_answer_is_kept
  • a_flat_surface_is_not_traced
  • a_surface_touching_the_plane_is_not_traced
  • an_end_off_a_surface_is_not_traced

Commands

On 32aea3d (main f59f1a8 merged in, with #2245), Rust 1.98.0, unless noted.

Linux (WSL2, x86_64), with the pinned C++ reference:
cargo fmt --all --check: clean
cargo clippy --workspace --all-targets -- -D warnings: clean
cargo test --profile difftest -p solidcore --no-fail-fast
  3931 passed, 0 failed, 30 ignored, 317 targets
cargo run --profile difftest -p difftest --features cpp -- --unit blend.fillet_geom_p2 --no-report
  1 unit passed, 1151 cases

On bce2f4a8a (main 8bc7f8599 merged in), Linux, flag off:
cargo run --profile difftest -p difftest --features cpp -- --green --jobs 8
  280 units, 280 passed, 0 partial, 0 failed

On bce2f4a8a, Linux, a local patch forcing the flag on in every kernel, not committed:
cargo test --profile difftest -p solidcore --no-fail-fast
  3923 passed, 2 failed, 30 ignored, 316 targets
  the 2: issue_2176's the_neighbours_keep_their_rounds (the figure above) and
  wr682's without_the_flag_the_curve_is_not_found (the flag is forced on)

Windows (x86_64):
cargo check -p solidcore --target wasm32-unknown-unknown: clean
cargo test -p solidcore --test wr682_surface_plane_curve --test issue_2176_round_of_arc_between_tangent_straights
  8, 5 passed

Not run: --perturb, and the forced-on suite and --green again after #2245 came in from main.

Completion evidence

Operation / upstream symbols Implementation Successful native comparison Remaining gap
sm_SurfacePlaneCurve, its call in sm_SrfSrfIntersection (fork) complete, behind the flag none against the fork's C++; closed-form checks above. Flag off: difftest at parity with the pinned C++ no run against the fork revision

Assisted by Claude Code (Claude Opus 5.5). All local runs were made by the assistant.

🤖 Generated with Claude Code

MarkusReichart and others added 5 commits November 1, 2026 18:20
… 4c6571b, tangency-labs#2250)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t-in (WR-682)

The fork's sm_SurfacePlaneCurve (smlib 5bdcddf, 14a6daa, 4c6571b) behind
fillet_surface_plane_curve_on_plane_equation: between two points on both, where
they cross and the surface isn't flat, the curve is the zero set of the plane's
equation on the surface, so the plane's own domain doesn't end it. Default
behaviour is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Its first figure was pinned before WR-682's flag existed. The flag takes 8.8e-9 off what
that round removes; the body is sound either way. The tolerance stays 1e-9.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@MarkusReichart MarkusReichart changed the title [WIP] fix: a surface and a plane's curve traced on the plane, opt-in (smlib#78 5bdcddf, 14a6daa, 4c6571b, #2250) fix: a surface and a plane's curve traced on the plane, opt-in (smlib#78 5bdcddf, 14a6daa, 4c6571b, #2250) Nov 1, 2026
@MarkusReichart
MarkusReichart marked this pull request as ready for review November 1, 2026 18:15

@bherbruck bherbruck left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved by the automated first-pass review: CI green, mergeable, no CI/build/dependency file changes, no red-flag patterns in added code (process/network/env/unsafe outside FFI/blobs/hidden unicode), 0 known-mismatch entries added. Logic is gated by CI and the difftest merge queue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants