Skip to content

Commit 2954633

Browse files
committed
LODSphereMesh: address SonarCloud CRITICAL issues
- render(): extract cullLeaves/buildBatch/uploadAndBindBatch/drawBatch to cut cognitive complexity from 60 to within limit. - ensureStitchTemplates(): extract buildStitchTriangles (and the wireframe buildStitchLines) file-local helpers to cut complexity from 33. - bufferOffset(): annotate the GL-mandated void* return with NOSONAR. - LODSPHERE_WIREFRAME: keep the debug macro but mark it NOSONAR.
1 parent 0f6973b commit 2954633

2 files changed

Lines changed: 190 additions & 139 deletions

File tree

src/celengine/lodspheremesh.cpp

Lines changed: 179 additions & 137 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// lodspheremesh.cpp
22
//
3-
// Copyright (C) 2000-2009, theCelestia Development Team
3+
// Copyright (C) 2026-present, the Celestia Development Team
44
// Original version by Chris Laurel <claurel@gmail.com>
55
//
66
// This program is free software; you can redistribute it and/or
@@ -92,7 +92,10 @@ tangentAt(double sf)
9292
static_cast<float>(-std::cos(thetaA)));
9393
}
9494

95-
inline void*
95+
// Encode a byte offset as the pointer argument that glVertexAttribPointer and
96+
// glDrawElements take when a buffer is bound. The void* is mandated by the GL
97+
// API, so there is no more meaningful return type here.
98+
inline void* // NOSONAR
9699
bufferOffset(std::size_t n)
97100
{
98101
return reinterpret_cast<void*>(n);
@@ -232,80 +235,92 @@ LODSphereMesh::ensureBuffers()
232235
}
233236

234237

235-
// Build the 16 shared index templates, one per edge-stitch mask. A stitched edge
236-
// snaps its odd boundary vertices to the even neighbour and drops the resulting
237-
// zero-area triangles, matching a coarser neighbour watertight (no T-junctions).
238-
void
239-
LODSphereMesh::ensureStitchTemplates()
238+
// Triangle index template for one edge-stitch mask: a CHUNK_RES grid where each
239+
// stitched edge snaps its odd boundary vertices to the even neighbour, dropping
240+
// the resulting zero-area triangles so the seam matches a coarser neighbour
241+
// watertight (no T-junctions).
242+
static void
243+
buildStitchTriangles(int mask, std::vector<unsigned int>& out)
240244
{
241-
if (stitchTemplatesBuilt)
242-
return;
243-
244245
constexpr int N = CHUNK_RES;
245-
const auto stride = static_cast<unsigned int>(N + 1);
246-
auto vid = [stride](int a, int b) {
246+
constexpr auto stride = static_cast<unsigned int>(N + 1);
247+
auto vid = [](int a, int b) {
247248
return static_cast<unsigned int>(a * static_cast<int>(stride) + b);
248249
};
250+
auto snap = [mask](int a, int b) {
251+
if (a == 0 && (mask & EDGE_LEFT) != 0 && (b & 1) != 0) --b;
252+
if (a == N && (mask & EDGE_RIGHT) != 0 && (b & 1) != 0) --b;
253+
if (b == 0 && (mask & EDGE_BOTTOM) != 0 && (a & 1) != 0) --a;
254+
if (b == N && (mask & EDGE_TOP) != 0 && (a & 1) != 0) --a;
255+
return std::pair<int, int>(a, b);
256+
};
249257

250-
std::vector<unsigned int> indices;
251-
for (int mask = 0; mask < NUM_STITCH_TEMPLATES; ++mask)
258+
out.clear();
259+
out.reserve(static_cast<std::size_t>(N) * N * 6);
260+
for (int ii = 0; ii < N; ++ii)
252261
{
253-
auto snap = [mask, N](int a, int b) {
254-
if (a == 0 && (mask & EDGE_LEFT) != 0 && (b & 1) != 0) --b;
255-
if (a == N && (mask & EDGE_RIGHT) != 0 && (b & 1) != 0) --b;
256-
if (b == 0 && (mask & EDGE_BOTTOM) != 0 && (a & 1) != 0) --a;
257-
if (b == N && (mask & EDGE_TOP) != 0 && (a & 1) != 0) --a;
258-
return std::pair<int, int>(a, b);
259-
};
260-
261-
indices.clear();
262-
indices.reserve(static_cast<std::size_t>(N) * N * 6);
263-
for (int ii = 0; ii < N; ++ii)
262+
for (int jj = 0; jj < N; ++jj)
264263
{
265-
for (int jj = 0; jj < N; ++jj)
264+
auto [a00, b00] = snap(ii, jj);
265+
auto [a10, b10] = snap(ii + 1, jj);
266+
auto [a01, b01] = snap(ii, jj + 1);
267+
auto [a11, b11] = snap(ii + 1, jj + 1);
268+
unsigned int v00 = vid(a00, b00);
269+
unsigned int v10 = vid(a10, b10);
270+
unsigned int v01 = vid(a01, b01);
271+
unsigned int v11 = vid(a11, b11);
272+
// Winding is reversed vs the cube-sphere blueprint: here
273+
// d/dsf x d/dtf points inward, so a CCW quad in (sf,tf) would face
274+
// inward and be back-face culled.
275+
if (v00 != v10 && v10 != v11 && v00 != v11)
266276
{
267-
auto [a00, b00] = snap(ii, jj);
268-
auto [a10, b10] = snap(ii + 1, jj);
269-
auto [a01, b01] = snap(ii, jj + 1);
270-
auto [a11, b11] = snap(ii + 1, jj + 1);
271-
unsigned int v00 = vid(a00, b00);
272-
unsigned int v10 = vid(a10, b10);
273-
unsigned int v01 = vid(a01, b01);
274-
unsigned int v11 = vid(a11, b11);
275-
// Winding is reversed vs the cube-sphere blueprint: here
276-
// d/dsf x d/dtf points inward, so a CCW quad in (sf,tf) would face
277-
// inward and be back-face culled.
278-
if (v00 != v10 && v10 != v11 && v00 != v11)
279-
{
280-
indices.push_back(v00); indices.push_back(v11); indices.push_back(v10);
281-
}
282-
if (v00 != v11 && v11 != v01 && v00 != v01)
283-
{
284-
indices.push_back(v00); indices.push_back(v01); indices.push_back(v11);
285-
}
277+
out.push_back(v00); out.push_back(v11); out.push_back(v10);
278+
}
279+
if (v00 != v11 && v11 != v01 && v00 != v01)
280+
{
281+
out.push_back(v00); out.push_back(v01); out.push_back(v11);
286282
}
287283
}
284+
}
285+
}
288286

289-
stitchTemplate[mask] = indices;
290287

291288
#if LODSPHERE_WIREFRAME
292-
// Wireframe template from the triangle edges, deduped (undirected).
293-
std::set<std::pair<unsigned int, unsigned int>> edges;
294-
for (std::size_t t = 0; t + 2 < indices.size(); t += 3)
295-
{
296-
unsigned int a = indices[t], b = indices[t + 1], c = indices[t + 2];
297-
edges.emplace(std::min(a, b), std::max(a, b));
298-
edges.emplace(std::min(b, c), std::max(b, c));
299-
edges.emplace(std::min(c, a), std::max(c, a));
300-
}
301-
std::vector<unsigned int> lineIndices;
302-
lineIndices.reserve(edges.size() * 2);
303-
for (const auto& e : edges)
304-
{
305-
lineIndices.push_back(e.first);
306-
lineIndices.push_back(e.second);
307-
}
308-
stitchLineTemplate[mask] = std::move(lineIndices);
289+
// Wireframe line template from a triangle template's edges, deduped (undirected).
290+
static void
291+
buildStitchLines(const std::vector<unsigned int>& tris, std::vector<unsigned int>& out)
292+
{
293+
std::set<std::pair<unsigned int, unsigned int>> edges;
294+
for (std::size_t t = 0; t + 2 < tris.size(); t += 3)
295+
{
296+
unsigned int a = tris[t], b = tris[t + 1], c = tris[t + 2];
297+
edges.emplace(std::min(a, b), std::max(a, b));
298+
edges.emplace(std::min(b, c), std::max(b, c));
299+
edges.emplace(std::min(c, a), std::max(c, a));
300+
}
301+
out.clear();
302+
out.reserve(edges.size() * 2);
303+
for (const auto& e : edges)
304+
{
305+
out.push_back(e.first);
306+
out.push_back(e.second);
307+
}
308+
}
309+
#endif
310+
311+
312+
// Build the 16 shared index templates, one per edge-stitch mask.
313+
void
314+
LODSphereMesh::ensureStitchTemplates()
315+
{
316+
if (stitchTemplatesBuilt)
317+
return;
318+
319+
for (int mask = 0; mask < NUM_STITCH_TEMPLATES; ++mask)
320+
{
321+
buildStitchTriangles(mask, stitchTemplate[mask]);
322+
#if LODSPHERE_WIREFRAME
323+
buildStitchLines(stitchTemplate[mask], stitchLineTemplate[mask]);
309324
#endif
310325
}
311326

@@ -701,9 +716,35 @@ LODSphereMesh::render(unsigned int attributes,
701716

702717
glBindVertexArray(vao);
703718

704-
// Pass 2: cull leaves outside the frustum or behind the horizon, then gather
705-
// survivors with their per-texture tile bindings. Culling happens here (not in
706-
// pass 1) so balance and stitch still seal seams against off-screen neighbours.
719+
// Pass 2: cull to a texID-sorted draw list, concatenate the survivors into the
720+
// batch buffers, then bind and draw. Culling happens here (not in pass 1) so
721+
// balance and stitch still seal seams against off-screen neighbours.
722+
cullLeaves(frustum, eyePos, enableHorizonCull);
723+
buildBatch(attributes);
724+
if (!batchIndices.empty())
725+
{
726+
uploadAndBindBatch(attributes, program);
727+
drawBatch(attributes);
728+
}
729+
730+
for (int i = 0; i < nTexturesUsed; ++i)
731+
tex[i]->endUsage();
732+
733+
glBindVertexArray(0);
734+
glBindBuffer(GL_ARRAY_BUFFER, 0);
735+
glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
736+
737+
evictColdChunks();
738+
}
739+
740+
741+
// Cull the balanced leaves against the frustum and horizon, resolving each
742+
// survivor's per-texture tile bindings, then sort by texID so leaves sharing
743+
// bindings (common via coarser-ancestor fallback) can batch into one draw.
744+
void
745+
LODSphereMesh::cullLeaves(const math::Frustum& frustum, const Eigen::Vector3f& eyePos,
746+
bool enableHorizonCull)
747+
{
707748
frameDraws.clear();
708749
for (const ChunkKey& key : frameLeaves)
709750
{
@@ -725,8 +766,6 @@ LODSphereMesh::render(unsigned int attributes,
725766
frameDraws.push_back(leaf);
726767
}
727768

728-
// Sort by per-texture texID so leaves resolving to the same bindings (common via
729-
// coarser-ancestor fallback) draw in one call.
730769
std::sort(frameDraws.begin(), frameDraws.end(),
731770
[n = nTexturesUsed](const DrawLeaf& a, const DrawLeaf& b)
732771
{
@@ -737,9 +776,15 @@ LODSphereMesh::render(unsigned int attributes,
737776
}
738777
return false;
739778
});
779+
}
780+
740781

741-
// Build the batch in sorted order, one DrawGroup per maximal run of identical
742-
// bindings. Atlas UVs are baked, so the index range plus texIDs describe a draw.
782+
// Concatenate the sorted draw list into the batch buffers, opening a new DrawGroup
783+
// at each change of texID bindings. Atlas UVs are baked, so an index range plus
784+
// texIDs fully describe a draw.
785+
void
786+
LODSphereMesh::buildBatch(unsigned int attributes)
787+
{
743788
batchVertices.clear();
744789
batchIndices.clear();
745790
frameGroups.clear();
@@ -774,85 +819,82 @@ LODSphereMesh::render(unsigned int attributes,
774819
appendChunk(*chunk, leaf.mask, leaf.tiles, nTexturesUsed);
775820
frameGroups.back().count += batchIndices.size() - before;
776821
}
822+
}
777823

778-
if (!batchIndices.empty())
779-
{
780-
batchVBO.setData(celestia::util::array_view<void>(
781-
batchVertices.data(),
782-
batchVertices.size() * sizeof(float)),
783-
gl::Buffer::BufferUsage::StreamDraw);
784824

785-
const auto stride = static_cast<GLsizei>(batchVertexSize * sizeof(float));
825+
// Upload the batch buffers and point the active vertex attributes at them.
826+
void
827+
LODSphereMesh::uploadAndBindBatch(unsigned int attributes, CelestiaGLProgram* program)
828+
{
829+
batchVBO.setData(celestia::util::array_view<void>(
830+
batchVertices.data(),
831+
batchVertices.size() * sizeof(float)),
832+
gl::Buffer::BufferUsage::StreamDraw);
786833

787-
glEnableVertexAttribArray(CelestiaGLProgram::VertexCoordAttributeIndex);
788-
glVertexAttribPointer(CelestiaGLProgram::VertexCoordAttributeIndex,
834+
const auto stride = static_cast<GLsizei>(batchVertexSize * sizeof(float));
835+
836+
glEnableVertexAttribArray(CelestiaGLProgram::VertexCoordAttributeIndex);
837+
glVertexAttribPointer(CelestiaGLProgram::VertexCoordAttributeIndex,
838+
3, GL_FLOAT, GL_FALSE, stride, bufferOffset(0));
839+
if ((attributes & Normals) != 0)
840+
{
841+
glEnableVertexAttribArray(CelestiaGLProgram::NormalAttributeIndex);
842+
glVertexAttribPointer(CelestiaGLProgram::NormalAttributeIndex,
789843
3, GL_FLOAT, GL_FALSE, stride, bufferOffset(0));
790-
if ((attributes & Normals) != 0)
791-
{
792-
glEnableVertexAttribArray(CelestiaGLProgram::NormalAttributeIndex);
793-
glVertexAttribPointer(CelestiaGLProgram::NormalAttributeIndex,
794-
3, GL_FLOAT, GL_FALSE, stride, bufferOffset(0));
795-
}
796-
if ((attributes & Tangents) != 0)
797-
{
798-
glEnableVertexAttribArray(CelestiaGLProgram::TangentAttributeIndex);
799-
glVertexAttribPointer(CelestiaGLProgram::TangentAttributeIndex,
800-
3, GL_FLOAT, GL_FALSE, stride, bufferOffset(3 * sizeof(float)));
801-
}
802-
for (int tc = 0; tc < nTexturesUsed; ++tc)
803-
{
804-
// Atlas UVs are baked, so the shader's texcoord transform is identity.
805-
program->texCoordTransforms[tc].base = Vector2f(0.0f, 0.0f);
806-
program->texCoordTransforms[tc].delta = Vector2f(1.0f, 1.0f);
807-
glEnableVertexAttribArray(CelestiaGLProgram::TextureCoord0AttributeIndex + tc);
808-
glVertexAttribPointer(CelestiaGLProgram::TextureCoord0AttributeIndex + tc,
809-
2, GL_FLOAT, GL_FALSE, stride,
810-
bufferOffset((prefixFloats + 2 * tc) * sizeof(float)));
811-
}
844+
}
845+
if ((attributes & Tangents) != 0)
846+
{
847+
glEnableVertexAttribArray(CelestiaGLProgram::TangentAttributeIndex);
848+
glVertexAttribPointer(CelestiaGLProgram::TangentAttributeIndex,
849+
3, GL_FLOAT, GL_FALSE, stride, bufferOffset(3 * sizeof(float)));
850+
}
851+
for (int tc = 0; tc < nTexturesUsed; ++tc)
852+
{
853+
// Atlas UVs are baked, so the shader's texcoord transform is identity.
854+
program->texCoordTransforms[tc].base = Vector2f(0.0f, 0.0f);
855+
program->texCoordTransforms[tc].delta = Vector2f(1.0f, 1.0f);
856+
glEnableVertexAttribArray(CelestiaGLProgram::TextureCoord0AttributeIndex + tc);
857+
glVertexAttribPointer(CelestiaGLProgram::TextureCoord0AttributeIndex + tc,
858+
2, GL_FLOAT, GL_FALSE, stride,
859+
bufferOffset((prefixFloats + 2 * tc) * sizeof(float)));
860+
}
812861

813-
batchIBO.setData(celestia::util::array_view<void>(
814-
batchIndices.data(),
815-
batchIndices.size() * sizeof(unsigned int)),
816-
gl::Buffer::BufferUsage::StreamDraw);
862+
batchIBO.setData(celestia::util::array_view<void>(
863+
batchIndices.data(),
864+
batchIndices.size() * sizeof(unsigned int)),
865+
gl::Buffer::BufferUsage::StreamDraw);
866+
}
817867

818-
#if LODSPHERE_WIREFRAME
819-
const GLenum primitive = GL_LINES;
820-
#else
821-
const GLenum primitive = GL_TRIANGLES;
822-
#endif
823-
for (const DrawGroup& g : frameGroups)
868+
869+
// Draw one glDrawElements per DrawGroup, binding its textures first, then release
870+
// the vertex attributes.
871+
void
872+
LODSphereMesh::drawBatch(unsigned int attributes)
873+
{
874+
const GLenum primitive = LODSPHERE_WIREFRAME ? GL_LINES : GL_TRIANGLES;
875+
for (const DrawGroup& g : frameGroups)
876+
{
877+
for (int tc = 0; tc < nTexturesUsed; ++tc)
824878
{
825-
for (int tc = 0; tc < nTexturesUsed; ++tc)
826-
{
827-
if (nTexturesUsed > 1)
828-
glActiveTexture(GL_TEXTURE0 + tc);
829-
glBindTexture(GL_TEXTURE_2D, g.texID[tc]);
830-
}
831879
if (nTexturesUsed > 1)
832-
glActiveTexture(GL_TEXTURE0);
833-
834-
glDrawElements(primitive, static_cast<GLsizei>(g.count),
835-
GL_UNSIGNED_INT,
836-
bufferOffset(g.first * sizeof(unsigned int)));
880+
glActiveTexture(GL_TEXTURE0 + tc);
881+
glBindTexture(GL_TEXTURE_2D, g.texID[tc]);
837882
}
883+
if (nTexturesUsed > 1)
884+
glActiveTexture(GL_TEXTURE0);
838885

839-
glDisableVertexAttribArray(CelestiaGLProgram::VertexCoordAttributeIndex);
840-
if ((attributes & Normals) != 0)
841-
glDisableVertexAttribArray(CelestiaGLProgram::NormalAttributeIndex);
842-
if ((attributes & Tangents) != 0)
843-
glDisableVertexAttribArray(CelestiaGLProgram::TangentAttributeIndex);
844-
for (int tc = 0; tc < nTexturesUsed; ++tc)
845-
glDisableVertexAttribArray(CelestiaGLProgram::TextureCoord0AttributeIndex + tc);
886+
glDrawElements(primitive, static_cast<GLsizei>(g.count),
887+
GL_UNSIGNED_INT,
888+
bufferOffset(g.first * sizeof(unsigned int)));
846889
}
847890

848-
for (int i = 0; i < nTexturesUsed; ++i)
849-
tex[i]->endUsage();
850-
851-
glBindVertexArray(0);
852-
glBindBuffer(GL_ARRAY_BUFFER, 0);
853-
glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
854-
855-
evictColdChunks();
891+
glDisableVertexAttribArray(CelestiaGLProgram::VertexCoordAttributeIndex);
892+
if ((attributes & Normals) != 0)
893+
glDisableVertexAttribArray(CelestiaGLProgram::NormalAttributeIndex);
894+
if ((attributes & Tangents) != 0)
895+
glDisableVertexAttribArray(CelestiaGLProgram::TangentAttributeIndex);
896+
for (int tc = 0; tc < nTexturesUsed; ++tc)
897+
glDisableVertexAttribArray(CelestiaGLProgram::TextureCoord0AttributeIndex + tc);
856898
}
857899

858900

0 commit comments

Comments
 (0)