Skip to content

Commit 0beb284

Browse files
sawenzelclaude
andcommitted
Fix display fans across wire-trimmed quadric patches
This fixes a problem in the display mesh of wire-trimmed quadric patches and adds a unit test. - The trim loop was ear-clipped by taking the first valid ear, which fans a rectangular trim from one corner. - The last triangle of such a fan spans the whole phi sweep and covers the bore in ROOT's GL view. - The clipping now takes the valid ear whose new diagonal has the shortest 3D chord. - Vertices and their order are unchanged, so navigation and safety are not affected. - If the clipping stalls on a wire, it falls back to the plain triangulation, so no triangle is lost. https://its.cern.ch/jira/browse/O2-7284 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent ab6e1c2 commit 0beb284

2 files changed

Lines changed: 151 additions & 1 deletion

File tree

‎Detectors/CADSupport/src/BoundedSurface.h‎

Lines changed: 93 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2249,6 +2249,97 @@ inline std::vector<Vec2> sampleCurveWireByU(const CurveWire& wire, int segmentsP
22492249
return samples;
22502250
}
22512251

2252+
/// Ear-clipping of a simple parametric wire that clips the ear with the shortest 3D diagonal first, so a strip is not fanned.
2253+
inline std::vector<std::array<int, 3>> triangulateWireByShortestChord(const SurfaceWire& wire,
2254+
const std::vector<Vec3>& points)
2255+
{
2256+
std::vector<int> remainingIndices;
2257+
remainingIndices.reserve(wire.vertices.size());
2258+
if (wire.signedArea() >= 0.) {
2259+
for (size_t vertexIndex = 0; vertexIndex < wire.vertices.size(); ++vertexIndex) {
2260+
remainingIndices.push_back(static_cast<int>(vertexIndex));
2261+
}
2262+
} else {
2263+
for (size_t reverseIndex = wire.vertices.size(); reverseIndex > 0; --reverseIndex) {
2264+
remainingIndices.push_back(static_cast<int>(reverseIndex - 1));
2265+
}
2266+
}
2267+
2268+
// the 3D diagonal of the ear at a position, or infinity when it is not an ear (same tests as triangulateSimpleWire)
2269+
const double notAnEar = std::numeric_limits<double>::infinity();
2270+
auto earChord = [&](size_t indexPosition) {
2271+
const int previousIndex = remainingIndices[(indexPosition + remainingIndices.size() - 1) % remainingIndices.size()];
2272+
const int currentIndex = remainingIndices[indexPosition];
2273+
const int nextIndex = remainingIndices[(indexPosition + 1) % remainingIndices.size()];
2274+
const auto& previousVertex = wire.vertices[previousIndex];
2275+
const auto& currentVertex = wire.vertices[currentIndex];
2276+
const auto& nextVertex = wire.vertices[nextIndex];
2277+
if (cross2D(currentVertex - previousVertex, nextVertex - currentVertex) <= kTolerance) {
2278+
return notAnEar;
2279+
}
2280+
for (int candidateIndex : remainingIndices) {
2281+
if (candidateIndex == previousIndex || candidateIndex == currentIndex || candidateIndex == nextIndex) {
2282+
continue;
2283+
}
2284+
if (pointInTriangle(wire.vertices[candidateIndex], previousVertex, currentVertex, nextVertex)) {
2285+
return notAnEar;
2286+
}
2287+
}
2288+
return norm(points[nextIndex] - points[previousIndex]);
2289+
};
2290+
2291+
std::vector<double> chords(remainingIndices.size());
2292+
for (size_t indexPosition = 0; indexPosition < remainingIndices.size(); ++indexPosition) {
2293+
chords[indexPosition] = earChord(indexPosition);
2294+
}
2295+
2296+
std::vector<std::array<int, 3>> triangles;
2297+
while (remainingIndices.size() > 3) {
2298+
// the shortest diagonal wins; ties go to the lowest position
2299+
size_t bestPosition = remainingIndices.size();
2300+
double bestChord = notAnEar;
2301+
for (size_t indexPosition = 0; indexPosition < remainingIndices.size(); ++indexPosition) {
2302+
if (chords[indexPosition] < bestChord) {
2303+
bestChord = chords[indexPosition];
2304+
bestPosition = indexPosition;
2305+
}
2306+
}
2307+
if (bestPosition == remainingIndices.size()) {
2308+
return triangulateSimpleWire(wire); // near-collinear runs can block every ear in this order
2309+
}
2310+
2311+
const size_t count = remainingIndices.size();
2312+
const int previousIndex = remainingIndices[(bestPosition + count - 1) % count];
2313+
const int clippedIndex = remainingIndices[bestPosition];
2314+
const int nextIndex = remainingIndices[(bestPosition + 1) % count];
2315+
triangles.push_back({previousIndex, clippedIndex, nextIndex});
2316+
remainingIndices.erase(remainingIndices.begin() + bestPosition);
2317+
chords.erase(chords.begin() + bestPosition);
2318+
2319+
// the two neighbours change shape; any other non-ear can only change if the clipped vertex lay in its triangle
2320+
const size_t newCount = remainingIndices.size();
2321+
const size_t beforePosition = (bestPosition + newCount - 1) % newCount;
2322+
const size_t afterPosition = bestPosition % newCount;
2323+
for (size_t indexPosition = 0; indexPosition < newCount; ++indexPosition) {
2324+
if (indexPosition == beforePosition || indexPosition == afterPosition) {
2325+
chords[indexPosition] = earChord(indexPosition);
2326+
} else if (chords[indexPosition] == notAnEar) {
2327+
const auto& previousVertex = wire.vertices[remainingIndices[(indexPosition + newCount - 1) % newCount]];
2328+
const auto& currentVertex = wire.vertices[remainingIndices[indexPosition]];
2329+
const auto& nextVertex = wire.vertices[remainingIndices[(indexPosition + 1) % newCount]];
2330+
if (pointInTriangle(wire.vertices[clippedIndex], previousVertex, currentVertex, nextVertex)) {
2331+
chords[indexPosition] = earChord(indexPosition);
2332+
}
2333+
}
2334+
}
2335+
}
2336+
2337+
if (remainingIndices.size() == 3) {
2338+
triangles.push_back({remainingIndices[0], remainingIndices[1], remainingIndices[2]});
2339+
}
2340+
return triangles;
2341+
}
2342+
22522343
/// Append the display triangulation of a wire-trimmed quadric patch: the sampled outer loop, ear-clipped; holes are omitted.
22532344
template <typename MapUV>
22542345
void appendCurveTrimMesh(const CurveWire& outerWire, const MapUV& mapUV, std::vector<Vec3>& vertices,
@@ -2263,7 +2354,8 @@ void appendCurveTrimMesh(const CurveWire& outerWire, const MapUV& mapUV, std::ve
22632354
for (const auto& sample : sampledWire.vertices) {
22642355
vertices.push_back(mapUV(sample.uCoord, sample.vCoord));
22652356
}
2266-
for (const auto& triangle : triangulateSimpleWire(sampledWire)) {
2357+
const std::vector<Vec3> points(vertices.begin() + firstVertexIndex, vertices.end());
2358+
for (const auto& triangle : triangulateWireByShortestChord(sampledWire, points)) {
22672359
triangles.push_back(
22682360
{firstVertexIndex + triangle[0], firstVertexIndex + triangle[1], firstVertexIndex + triangle[2]});
22692361
}

‎Detectors/CADSupport/test/testBVHSurfaceSolid.cxx‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2186,6 +2186,64 @@ BOOST_AUTO_TEST_CASE(WireTrimmedConeMatchesCone)
21862186
checkClose(solid.Capacity(), reference.Capacity(), 1.e-9);
21872187
}
21882188

2189+
namespace
2190+
{
2191+
// Check that a patch's display mesh keeps its vertex count and stays within the chord sag of radius on the patch;
2192+
// boundarySag adds the sag of a trim side that the wire sampling leaves as one chord.
2193+
void checkDisplayMeshFollowsPatch(const surf::BoundedSurface& patch, double radius, size_t expectedVertices,
2194+
double boundarySag = 0.)
2195+
{
2196+
std::vector<surf::Vec3> vertices;
2197+
std::vector<std::array<int, 3>> triangles;
2198+
patch.appendDisplayMesh(vertices, triangles);
2199+
BOOST_CHECK_EQUAL(vertices.size(), expectedVertices);
2200+
BOOST_CHECK_EQUAL(triangles.size(), expectedVertices - 2);
2201+
const double maxSag = 1.05 * (radius * (1. - std::cos(surf::kPi / surf::kArcSamples)) + boundarySag);
2202+
double worstSag = 0.;
2203+
for (const auto& triangle : triangles) {
2204+
const surf::Vec3& first = vertices[triangle[0]];
2205+
const surf::Vec3& second = vertices[triangle[1]];
2206+
const surf::Vec3& third = vertices[triangle[2]];
2207+
const double longestSq = std::max({normSq(second - first), normSq(third - second), normSq(first - third)});
2208+
BOOST_CHECK_GT(norm(cross(second - first, third - first)), 1.e-6 * longestSq);
2209+
for (const surf::Vec3& probe : {(first + second + third) * (1. / 3.), (first + second) * 0.5,
2210+
(second + third) * 0.5, (third + first) * 0.5}) {
2211+
worstSag = std::max(worstSag, std::sqrt(patch.distanceSqToPatch(probe)));
2212+
}
2213+
}
2214+
BOOST_CHECK_LE(worstSag, maxSag);
2215+
}
2216+
} // namespace
2217+
2218+
// The display triangles of a wire-trimmed quadric follow the patch instead of fanning across it (O2-7284).
2219+
BOOST_AUTO_TEST_CASE(WireTrimmedDisplayMeshFollowsPatch)
2220+
{
2221+
std::string error;
2222+
constexpr double phiMin = 1.6;
2223+
constexpr double phiMax = 4.683;
2224+
2225+
// half-cylinder of the ALICE 3 tracker shell: 12 chords on each long side plus the two corners
2226+
surf::CylindricalBoundedSurface halfCylinder;
2227+
BOOST_REQUIRE(halfCylinder.initialize({0., 0., 0.}, {0., 0., 1.}, {1., 0., 0.}, 10., -20., 20., phiMin,
2228+
phiMax - phiMin, false, paramRectWireCurves(phiMin, phiMax, -20., 20.), {},
2229+
error));
2230+
checkDisplayMeshFollowsPatch(halfCylinder, 10., 26);
2231+
2232+
// quarter-torus: 6 chords on each ring side; each tube side is sampled as a single chord
2233+
surf::TorusBoundedSurface quarterTorus;
2234+
BOOST_REQUIRE(quarterTorus.initialize({0., 0., 0.}, {0., 0., 1.}, {1., 0., 0.}, 10., 0.3, 0., surf::kHalfPi, 0.,
2235+
surf::kPi / 3., false,
2236+
paramRectWireCurves(0., surf::kHalfPi, 0., surf::kPi / 3.), {}, error));
2237+
checkDisplayMeshFollowsPatch(quarterTorus, 10.3, 14, 0.3 * (1. - std::cos(surf::kPi / 6.)));
2238+
2239+
// half-cone
2240+
surf::ConicalBoundedSurface halfCone;
2241+
BOOST_REQUIRE(halfCone.initialize({0., 0., 0.}, {0., 0., 1.}, {1., 0., 0.}, 8., 12., -10., 10., phiMin,
2242+
phiMax - phiMin, false, paramRectWireCurves(phiMin, phiMax, -10., 10.), {},
2243+
error));
2244+
checkDisplayMeshFollowsPatch(halfCone, 12., 26);
2245+
}
2246+
21892247
// Green's theorem for wire-trimmed quadrics.
21902248
//
21912249
// The integrand does not depend on the second parameter for a cylinder, and depends on the first

0 commit comments

Comments
 (0)