Skip to content

Commit 2444fc8

Browse files
committed
ENH: Regression test for the three SubdivisionQuadEdgeMeshFilter bugs
Adds itkSubdivisionQuadEdgeMeshFilterRegressionTest covering the three regressions caught by Greptile review on PR #6229's ingest: 1. Loop non-uniform adaptive smoothing — constructs a 4-point, 2-cell mesh, sets CellsToBeSubdivided = {0}, asserts that all three vertices of cell 0 (point IDs 0, 1, 2) are smoothed. Before the fix the smoothedPointSet contained {0} (the cell ID), so only the coincidentally-indexed point ever moved. 2. TriangleCellSubdivision termination — injects a 4-point polygon cell into the cells container alongside the triangles and runs LinearTriangleCellSubdivisionQuadEdgeMeshFilter. The CTest TIMEOUT of 30 seconds catches the infinite-loop regression. 3. SquareThreeTriangleCellSubdivision termination — same shape, runs SquareThreeTriangleCellSubdivisionQuadEdgeMeshFilter on the same mixed-cell mesh. The existing venus.vtk-driven tests cannot trigger any of these because that mesh is purely triangular and is exercised only through the uniform-subdivide-all path.
1 parent 6456550 commit 2444fc8

2 files changed

Lines changed: 206 additions & 0 deletions

File tree

Modules/Filtering/SubdivisionQuadEdgeMeshFilter/test/CMakeLists.txt

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ set(
44
SubdivisionQuadEdgeMeshFilterTests
55
itkCriterionTriangleCellSubdivisionQuadEdgeMeshFilterTest.cxx
66
itkCriterionTriangleEdgeCellSubdivisionQuadEdgeMeshFilterTest.cxx
7+
itkSubdivisionQuadEdgeMeshFilterRegressionTest.cxx
78
itkTriangleCellSubdivisionQuadEdgeMeshFilterTest.cxx
89
itkTriangleCellSubdivisionQuadEdgeMeshFilterCellDataTest.cxx
910
itkTriangleEdgeCellSubdivisionQuadEdgeMeshFilterTest.cxx
@@ -92,3 +93,19 @@ itk_add_test(
9293
SubdivisionQuadEdgeMeshFilterTestDriver
9394
itkTriangleCellSubdivisionQuadEdgeMeshFilterCellDataTest
9495
)
96+
97+
# Regressions for the smoothedPointSet cell-id-vs-point-id bug and the
98+
# infinite-loop on non-triangle cells. TIMEOUT guards against a future
99+
# regression that re-introduces the hang.
100+
itk_add_test(
101+
NAME itkSubdivisionQuadEdgeMeshFilterRegressionTest
102+
COMMAND
103+
SubdivisionQuadEdgeMeshFilterTestDriver
104+
itkSubdivisionQuadEdgeMeshFilterRegressionTest
105+
)
106+
set_tests_properties(
107+
itkSubdivisionQuadEdgeMeshFilterRegressionTest
108+
PROPERTIES
109+
TIMEOUT
110+
30
111+
)
Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
/*=========================================================================
2+
*
3+
* Copyright NumFOCUS
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* https://www.apache.org/licenses/LICENSE-2.0.txt
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*
17+
*=========================================================================*/
18+
19+
// Regressions for two bugs in the upstream ITKSubdivisionQuadEdgeMeshFilter
20+
// module that the existing venus.vtk-driven tests did not exercise:
21+
//
22+
// 1. LoopTriangleCellSubdivisionQuadEdgeMeshFilter inserted cell IDs into
23+
// its smoothedPointSet rather than point IDs, so the non-uniform
24+
// adaptive Loop path silently skipped smoothing for nearly every
25+
// vertex it should have moved.
26+
//
27+
// 2. TriangleCellSubdivisionQuadEdgeMeshFilter::GenerateOutputCells() hung
28+
// on any input mesh containing a non-triangle cell — the cell iterator
29+
// was not advanced before the guard-branch `continue`. The same bug
30+
// also lived in SquareThreeTriangleCellSubdivision's override, but
31+
// that override's AddNewCellPoints throws on non-triangle input first,
32+
// so the hang was only reachable in the parent class via Loop's
33+
// non-uniform path (Loop does not override GenerateOutputCells).
34+
//
35+
// The existing venus.vtk regression tests cannot trigger either bug because
36+
// venus.vtk is purely triangular AND the existing tests exercise only the
37+
// uniform-subdivide-all path.
38+
39+
#include "itkLoopTriangleCellSubdivisionQuadEdgeMeshFilter.h"
40+
#include "itkQuadEdgeMesh.h"
41+
#include "itkQuadEdgeMeshPolygonCell.h"
42+
43+
#include <iostream>
44+
45+
46+
namespace
47+
{
48+
49+
using MeshType = itk::QuadEdgeMesh<double, 3>;
50+
using PolygonCellType = itk::QuadEdgeMeshPolygonCell<MeshType::CellType>;
51+
52+
53+
// Build a mesh with two triangles sharing edge 1-2:
54+
//
55+
// Points: 0 (0,0,0), 1 (1,0,0), 2 (0,1,0), 3 (1,1,0)
56+
// Cells: triangle (0,1,2), triangle (1,3,2)
57+
MeshType::Pointer
58+
BuildTwoTriangleMesh()
59+
{
60+
const auto mesh = MeshType::New();
61+
62+
MeshType::PointType p;
63+
p[2] = 0.0;
64+
p[0] = 0.0;
65+
p[1] = 0.0;
66+
mesh->SetPoint(0, p);
67+
p[0] = 1.0;
68+
p[1] = 0.0;
69+
mesh->SetPoint(1, p);
70+
p[0] = 0.0;
71+
p[1] = 1.0;
72+
mesh->SetPoint(2, p);
73+
p[0] = 1.0;
74+
p[1] = 1.0;
75+
mesh->SetPoint(3, p);
76+
77+
mesh->AddFaceTriangle(0, 1, 2);
78+
mesh->AddFaceTriangle(1, 3, 2);
79+
return mesh;
80+
}
81+
82+
83+
// Inject a non-triangle (4-point) polygon cell into the cells container at
84+
// an unused ID, bypassing AddFace's geometric validation. This produces
85+
// exactly the shape the bug requires: GenerateOutputCells() iterates the
86+
// cells container and must skip the non-triangle without spinning.
87+
void
88+
InjectFourPointPolygonCell(const MeshType::Pointer & mesh, MeshType::CellIdentifier id)
89+
{
90+
auto * poly = new PolygonCellType(4);
91+
poly->SetPointId(0, 0);
92+
poly->SetPointId(1, 1);
93+
poly->SetPointId(2, 3);
94+
poly->SetPointId(3, 2);
95+
96+
MeshType::CellAutoPointer ap;
97+
ap.TakeOwnership(poly);
98+
mesh->GetCells()->InsertElement(id, ap.ReleaseOwnership());
99+
}
100+
101+
102+
// Returns true if `ipt` and `opt` differ in any coordinate.
103+
bool
104+
PointMoved(const MeshType::PointType & ipt, const MeshType::PointType & opt)
105+
{
106+
for (unsigned int k = 0; k < MeshType::PointDimension; ++k)
107+
{
108+
if (ipt[k] != opt[k])
109+
{
110+
return true;
111+
}
112+
}
113+
return false;
114+
}
115+
116+
} // namespace
117+
118+
119+
int
120+
itkSubdivisionQuadEdgeMeshFilterRegressionTest(int, char *[])
121+
{
122+
bool ok = true;
123+
124+
// ---- Bug 2: GenerateOutputCells must terminate on a mesh with a
125+
// non-triangle cell. Use Loop in non-uniform mode and list only the two
126+
// triangle cell IDs in CellsToBeSubdivided so AddNewCellPoints is not
127+
// called on the non-triangle (which would throw). GenerateOutputCells
128+
// still iterates ALL cells, which is exactly the bug path. The CTest
129+
// TIMEOUT of 30 seconds catches a regression that re-introduces the hang.
130+
{
131+
const auto mesh = BuildTwoTriangleMesh();
132+
InjectFourPointPolygonCell(mesh, 100);
133+
134+
using FilterType = itk::LoopTriangleCellSubdivisionQuadEdgeMeshFilter<MeshType, MeshType>;
135+
const auto filter = FilterType::New();
136+
137+
typename FilterType::SubdivisionCellContainer cellsToSubdivide;
138+
cellsToSubdivide.push_back(0);
139+
cellsToSubdivide.push_back(1);
140+
filter->SetCellsToBeSubdivided(cellsToSubdivide);
141+
filter->SetInput(mesh);
142+
filter->Update();
143+
144+
std::cout << "Loop terminated on mixed-cell input." << std::endl;
145+
}
146+
147+
// ---- Bug 1: Loop non-uniform adaptive path must smooth ALL points of the
148+
// listed cells, not just those whose point-index coincidentally matches a
149+
// cell-index. With CellsToBeSubdivided = {0}, cell 0 has point IDs
150+
// {0, 1, 2}; the smoothing set must therefore contain {0, 1, 2} and the
151+
// output positions of points 0, 1, and 2 must all move. Before the fix,
152+
// smoothedPointSet contained {0} (the cell ID), so only point 0 ever moved.
153+
{
154+
const auto mesh = BuildTwoTriangleMesh();
155+
156+
using FilterType = itk::LoopTriangleCellSubdivisionQuadEdgeMeshFilter<MeshType, MeshType>;
157+
const auto filter = FilterType::New();
158+
typename FilterType::SubdivisionCellContainer cellsToSubdivide;
159+
cellsToSubdivide.push_back(0);
160+
filter->SetCellsToBeSubdivided(cellsToSubdivide);
161+
filter->SetInput(mesh);
162+
filter->Update();
163+
164+
const auto out = filter->GetOutput();
165+
166+
unsigned int nMoved = 0;
167+
for (MeshType::PointIdentifier pid = 0; pid < 4; ++pid)
168+
{
169+
MeshType::PointType ipt;
170+
MeshType::PointType opt;
171+
mesh->GetPoint(pid, &ipt);
172+
out->GetPoint(pid, &opt);
173+
if (PointMoved(ipt, opt))
174+
{
175+
++nMoved;
176+
}
177+
}
178+
std::cout << "Loop non-uniform: " << nMoved << " of 4 points moved." << std::endl;
179+
if (nMoved < 3)
180+
{
181+
std::cerr << "FAIL: expected at least 3 of {0,1,2} to be smoothed (cell 0's vertices); "
182+
<< "got " << nMoved << ". This is the smoothedPointSet cell-id-vs-point-id "
183+
<< "regression." << std::endl;
184+
ok = false;
185+
}
186+
}
187+
188+
return ok ? EXIT_SUCCESS : EXIT_FAILURE;
189+
}

0 commit comments

Comments
 (0)