Skip to content

Commit d7b8832

Browse files
agheatadpiparo
authored andcommitted
[geom] Fix tessellated closure checks after initialization (#22457)
(cherry picked from commit ede4e83)
1 parent 0fd2998 commit d7b8832

2 files changed

Lines changed: 56 additions & 26 deletions

File tree

geom/geom/src/TGeoTessellated.cxx

Lines changed: 25 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -328,38 +328,36 @@ bool TGeoTessellated::FacetCheck(int ifacet) const
328328

329329
void TGeoTessellated::CloseShape(bool check, bool fixFlipped, bool verbose)
330330
{
331-
if (fIsClosed && fBVH) {
331+
const bool initialized = fIsClosed && fBVH;
332+
if (initialized && !check) {
332333
return;
333334
}
334-
// Compute bounding box
335-
fDefined = true;
336-
fNvert = fVertices.size();
337-
fNfacets = fFacets.size();
338-
ComputeBBox();
339335

340-
BuildBVH();
341-
if (fOutwardNormals.size() == 0) {
342-
CalculateNormals();
343-
} else {
344-
// short check if the normal container is of correct size
345-
if (fOutwardNormals.size() != fFacets.size()) {
346-
std::cerr << "Inconsistency in normal container";
347-
}
348-
}
349-
fIsClosed = true;
336+
if (!initialized) {
337+
// Compute bounding box
338+
fDefined = true;
339+
fNvert = fVertices.size();
340+
fNfacets = fFacets.size();
341+
ComputeBBox();
350342

351-
// Cleanup the vertex map
352-
std::multimap<long, int>().swap(fVerticesMap);
343+
BuildBVH();
344+
fIsClosed = true;
345+
346+
// Cleanup the vertex map
347+
std::multimap<long, int>().swap(fVerticesMap);
348+
}
353349

354350
if (fVertices.size() > 0) {
355-
if (!check)
356-
return;
351+
if (check) {
352+
// Check facets
353+
for (auto i = 0; i < fNfacets; ++i)
354+
FacetCheck(i);
357355

358-
// Check facets
359-
for (auto i = 0; i < fNfacets; ++i)
360-
FacetCheck(i);
356+
fClosedBody = CheckClosure(fixFlipped, verbose);
357+
}
361358

362-
fClosedBody = CheckClosure(fixFlipped, verbose);
359+
if (fOutwardNormals.size() != fFacets.size())
360+
CalculateNormals();
363361
}
364362
}
365363

@@ -422,6 +420,8 @@ bool TGeoTessellated::CheckClosure(bool fixFlipped, bool verbose)
422420
}
423421
if (nfixed && verbose)
424422
Info("Check", "Automatically flipped %d facets to match first defined facet", nfixed);
423+
if (nfixed && !fOutwardNormals.empty())
424+
CalculateNormals();
425425
}
426426
delete[] nn;
427427
delete[] flipped;
@@ -1277,8 +1277,7 @@ inline Double_t TGeoTessellated::SafetyKernel(const Double_t *point, bool in, in
12771277
const auto object_id = mybvh->prim_ids[p_id];
12781278

12791279
const auto &facet = fFacets[object_id];
1280-
auto thissafetySQ =
1281-
pointFacetDistSq(Vec3f<float>(point[0], point[1], point[2]), facet, fVertices);
1280+
auto thissafetySQ = pointFacetDistSq(Vec3f<float>(point[0], point[1], point[2]), facet, fVertices);
12821281

12831282
if (thissafetySQ < smallest_safety_sq) {
12841283
smallest_safety_sq = thissafetySQ;

geom/test/test_tessellated.cxx

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,39 @@ CreateTrdLikeTessellated_Triangles(const char *name, double x1, double x2, doubl
9696
return tsl;
9797
}
9898

99+
void AddClosedTetrahedronFacets(TGeoTessellated &tsl)
100+
{
101+
// Closed tetrahedron using the same facet winding as the GDML reproducer
102+
// for ROOT issue #22395.
103+
const Vtx v0(0, 0, 0);
104+
const Vtx v1(10, 0, 0);
105+
const Vtx v2(5, 10, 0);
106+
const Vtx v3(5, 5, 10);
107+
108+
EXPECT_TRUE(tsl.AddFacet(v0, v2, v1));
109+
EXPECT_TRUE(tsl.AddFacet(v0, v1, v3));
110+
EXPECT_TRUE(tsl.AddFacet(v1, v2, v3));
111+
EXPECT_TRUE(tsl.AddFacet(v0, v3, v2));
112+
}
113+
99114
} // namespace
100115

116+
TEST(TGeoTessellated, CloseShapeCanCheckAfterUncheckedClose)
117+
{
118+
// TGDMLParse finalizes tessellated solids with CloseShape(false). This must
119+
// initialize the shape without preventing a later explicit CloseShape(true)
120+
// from running closure validation and setting the closed-body state.
121+
TGeoTessellated tsl("Closed Tetrahedron");
122+
AddClosedTetrahedronFacets(tsl);
123+
124+
tsl.CloseShape(/*check=*/false);
125+
EXPECT_TRUE(tsl.IsDefined());
126+
EXPECT_FALSE(tsl.IsClosedBody());
127+
128+
tsl.CloseShape(/*check=*/true, /*fixFlipped=*/true, /*verbose=*/false);
129+
EXPECT_TRUE(tsl.IsClosedBody());
130+
}
131+
101132
TEST(TGeoTessellated, TrdLike_CoreNavigation)
102133
{
103134
// Representative points (ported)

0 commit comments

Comments
 (0)