Skip to content

difftest: topology.brep_p9 owns imported geometry under the keys it queries with - #649

Merged
bherbruck merged 2 commits into
tangency-labs:mainfrom
erlis:difftest/brep-p9-owners
Oct 23, 2026
Merged

bherbruck merged 2 commits into
tangency-labs:mainfrom
erlis:difftest/brep-p9-owners

Conversation

@erlis

@erlis erlis commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

topology.brep_p9 imports its B-rep through topology_vertexuse::import, which leaves edge curves unowned and owns face f's surface under key 1 + f. That is the convention several runners' own key_of use, so the shared import stays as it is. Op 0 (FindInfiniteRegion) then queries with BrepQuery::default(), i.e. arena keys. BrepCache::build_trees adds an edge or face only when its curve's or surface's owner equals the caller's key, so the infinite-region ray fires ran on trees with 0 of 15 edges and 0 of 7 faces. Every face surface looked owned by the next face. In the C++ fixture each edge owns its curve and each face its surface.

Fix (runner only): after the import, util::own_arena_geometry and Brep::sync_face_surface_tolerance on every face, as the query.api_queries import does. Without the sync, a face-owned surface reaches the topology.face_p2 SmFace::GetTolerance stub: owning alone moves real_box_with_large_fillet_infinite_region from pass to deferred. Same class as #618.

Results (Linux, pinned reference c9c5979; tolerances and fixtures unchanged), base = main 0123d56:

base:   PARTIAL topology.brep_p9 cases=684 passed=683 failed=0 deferred=1 max_rel_err=0e0 with_values=683 no_values=0 vacuous=0 blocked_by=topology.face_p2(1)
branch: PARTIAL topology.brep_p9 cases=684 passed=683 failed=0 deferred=1 max_rel_err=0e0 with_values=683 no_values=0 vacuous=0 blocked_by=topology.face_p2(1)
  • No case changes (per-case comparison of the two reports). This PR is about input parity, not new passes. Op 0's cases matched the C++ before while firing at empty trees, a path the C++ does not take. Now their trees hold every edge and face.
  • How it was found: a local, unpushed probe in build_trees that reports calls skipping geometry whose owner is no key of the B-rep, run over all 261 green/partial units. On the branch it reports no call in this unit; on main it reported the one above. No other runner's import mismatches its keys. The remaining hits are the library's own two naming schemes (primitive builders vs stitch), filed as Two owner-naming schemes: primitive B-reps are queried under arena keys (stitch ray fires on empty trees) #651.

Checks (on main 0123d56): cargo fmt --all --check; cargo clippy --workspace --all-targets -- -D warnings; cargo clippy -p difftest -p solidcore-ffi --features difftest/cpp --all-targets -- -D warnings; cargo test -p difftest --features cpp (267 passed); difftest --unit topology.brep_p9. No library code changed.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ueries with

The shared import leaves edge curves unowned and owns face f's surface as 1 + f,
while op 0 (FindInfiniteRegion) queries with BrepQuery::default() (arena keys),
so its B-rep cache trees held no edges or faces. Own the geometry with
util::own_arena_geometry and sync each face surface's tolerance, as the native
B-rep and the api_queries import do. No case changes.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@erlis
erlis marked this pull request as ready for review October 23, 2026 18:32
@bherbruck
bherbruck added this pull request to the merge queue Oct 23, 2026
Merged via the queue into tangency-labs:main with commit 169dbc4 Oct 23, 2026
7 checks passed
@erlis
erlis deleted the difftest/brep-p9-owners branch October 23, 2026 18:56
oursland added a commit to oursland/solidcore that referenced this pull request Oct 23, 2026
Picks up tangency-labs#649 (topology.brep_p9 import owners) and tangency-labs#650 (two-tier
mutation harness) so the branch is current for the merge queue.

Local checks after the merge: cargo fmt --all --check, cargo clippy
-p difftest --all-targets -D warnings, cargo test -p difftest --lib
(55 passed).

Co-Authored-By: Claude Code <noreply@anthropic.com>
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