[geom] Fix tessellated closure checks after initialization#22457
Merged
agheata merged 1 commit intoJun 3, 2026
Conversation
TGeoTessellated::CloseShape(false) initializes the bounding box, BVH and normals, then marks the shape closed for initialization purposes. Since the BVH navigation change added an unconditional fast return for already-initialized shapes, a later explicit CloseShape(true) could no longer run CheckClosure(). This left GDML-imported tessellated solids stuck with fClosedBody=false because TGDMLParse finalizes them with CloseShape(false). Allow the fast path only for repeated unchecked closes. A checked close now reuses the existing initialization data but still runs facet and closure validation, so fClosedBody can be updated without rebuilding the BVH. Also refresh cached normals if CheckClosure fixes flipped facets. Add a regression test for the unchecked-close followed by checked-close sequence using the closed tetrahedron from the issue reproducer.
1 task
11 tasks
Test Results 22 files 22 suites 3d 10h 45m 26s ⏱️ For more details on these failures, see this check. Results for commit d443536. |
dpiparo
approved these changes
Jun 3, 2026
Member
Author
|
/backport to 6.40 |
|
Preparing to backport PR #22457 to branch 6.40 requested by agheata |
|
This PR has been backported to branch 6.40: #22465 |
dpiparo
pushed a commit
that referenced
this pull request
Jun 3, 2026
(cherry picked from commit ede4e83)
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
3 participants
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.
This Pull request:
Changes or fixes:
Fixes a regression in
TGeoTessellated::CloseShape()where a tessellated shape finalized withCloseShape(false)could not later be checked withCloseShape(true).This affects GDML-imported tessellated solids because
TGDMLParse::Tessellated()finalizes them withCloseShape(false). Since the BVH navigation change added an early return for already-initialized shapes, a later explicitCloseShape(true)returned before runningCheckClosure(), leavingfClosedBody == false.The fix keeps the BVH/initialization fast path for repeated unchecked closes, but lets checked closes run facet and closure validation while reusing the existing BVH. This avoids rebuilding the BVH in the GDML reproducer path.
This follows the minimal direction suggested in the issue #22395 comments while avoiding the addition of more state flags or data members. Wouter's broader suggestion of lazy BVH construction on first navigation use was not taken in this PR because it would require a larger redesign of the cache ownership/state model, including const navigation methods, thread-safety of lazy cache construction, and streaming/readback behavior. This PR is intentionally scoped to the regression.
The patch also refreshes cached normals if
CheckClosure(..., fixFlipped=true)flips facets, so navigation data remains consistent with the facet winding.A regression test was added for the failing sequence:
CloseShape(false),CloseShape(true),IsClosedBody()becomes true.Checklist:
This PR fixes #22395