Repository navigation
wire: vertex point access in classification merges - #755
Merged
Merged
Conversation
SmVertexDefinition::Imprint merges its classifications through a new ProtoBrep::merge_point_class_into_brep hook; the arena pair (and the topology-intersector runner) implement it with merge_into_object_mut, so MergeIntoObject reads and moves the real vertex (SmVertex::GetPoint/SetPoint) instead of deferring on an opaque handle. PointClassification::Combine takes the caller's SmVertex::SetPoint (combine_setting_vertex_point); InsertCurveInterval and InsertPointClass get _mut entry points over MergeIntervalBrep that make the move where upstream does. The curve_class_p3/p6 runners use them and compare the vertex points after. boolean_post_process puts back the surviving edge curve's owner identity after DeleteTopologicalVertex, so the temporary transform's inverse still finds it (the far-translation cases this wiring newly compares). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RvuXmr7S5RoUXxVohC1giE
# Conflicts: # plan/units/boolean.merge_p3.toml
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RvuXmr7S5RoUXxVohC1giE
# Conflicts: # plan/units/boolean.merge_p3.toml # plan/units/boolean.merge_p4.toml # plan/units/boolean.topology_intersector_p2.toml
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RvuXmr7S5RoUXxVohC1giE
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Machine-generated (Claude Opus 5.5 coordinator and agent) on behalf of @tdamsma.
What was wrong. Two calls went through opaque handles that could not reach the vertex's B-rep:
MergeIntoObjectinSmVertexDefinition::Imprint(the proto-topology imprint) and theSmVertex::SetPointinsidePointClassification::Combine(inInsertCurveInterval/InsertPointClass). That produced 115SmVertex::GetPointdeferrals, 28SmVertex::SetPointdeferrals, and 20SmBrep mutable owning contextdeferrals in merge_p4.What changed.
ProtoBrep::merge_point_class_into_brep(the default keeps the old deferral).ArenaPairand the TI runner'sTwoimplement it withmerge_into_object_mut.imprint_vertex_definitionnow takes&mut Kernel.PointClassification::combine_setting_vertex_point(a caller-supplied SetPoint hook;combine_in_brepis now built on it).Imprint::combineaddsinsert_curve_interval_mutandinsert_point_class_mutoverMergeIntervalBrep, which move the vertex at the point upstream does. UpstreamSetPointonly notifiesChangeGeometryand sets the point: it touches no tolerance and no cache._mutpaths. Both sides (shim and Rust) now also report the vertex points afterwards. The proto runner merges a vertex stand-in in its own record.far-translationcases, and they failed. Cause:BooleanPostProcess'sDeleteTopologicalVertexre-owned the joined curve under a kernel key, so the inverse temporary transform skipped it.boolean_post_processnow puts back the surviving curve's owner identity, and all 3 pass. Details are in plan/findings.md. A cleaner fix would needkernel_adapter.rs(wire: IntersectInsertRelate into Boolean2D (primitive_creation_p1) and the PiecewiseMerge runner (merge_p3) #738).Difftest (Mac Studio; Linux CI is the verdict), before (main CI reports) -> after:
PASS cases=4466 passed=4466 failed=0 deferred=0(now green)PASS cases=3708 passed=3708 failed=0 deferred=0(now green)PARTIAL cases=311 passed=242 failed=0 deferred=66 rejected=3 blocked_by=query.face_p2(66)PARTIAL cases=423 passed=370 failed=0 deferred=53PARTIAL cases=910 passed=838 failed=0 deferred=72PARTIAL cases=1567 passed=1497 failed=0 deferred=70. The 25topology.brep_p1edge/region stand-ins are not part of any B-rep, so they stay deferred.PASS cases=248 passed=248Checks: mac-gates all pass (fmt, clippy with and without cpp, cargo test --workspace, wasm). Full
--greenon the Mac shows no new failures. local_solve1d, query.face_p4 and brep_p10 fail one case each, and they fail the same way on main on the Mac.🤖 Generated with Claude Code
https://claude.ai/code/session_01RvuXmr7S5RoUXxVohC1giE