quads chunk 2: GPU upload n-gon triangulation (re-targeted) - #330
Merged
Conversation
Wires the EditableSubMesh::faces (n-gon canonical) field through the
GPU upload path so quad-bearing meshes render correctly via fan-
triangulation. Triangle-only meshes are byte-identical before/after.
Changes:
- buildSubMeshBuffers (the GPU upload entry point used by
resizeEntityBuffers and createNewMesh) re-triangulates faces into
triangles defensively when faces is non-empty, so the index buffer
always matches the live face data even if the caller forgot to
sync. Triangle-only submeshes pay zero overhead — no copy, no
extra work.
- recalculateNormals / recalculateNormalsFlat call triangulateFaces
first when faces is canonical, so vertex normals always reflect
the current polygon geometry.
- New EditableMesh::syncTriangulation() helper for callers that
mutate faces directly and want to publish the change to triangles.
- New EditableMesh::totalFaceCount() for the n-gon-aware caller —
reports faces.size() when n-gons are present, falls back to
triangles.size() for legacy submeshes.
Tests (+5):
- syncTriangulation fan-triangulates a quad face into 2 triangles.
- syncTriangulation is a no-op on triangle-only submeshes.
- totalFaceCount falls back to triangle count for legacy submeshes
and reports n-gon count when faces is canonical.
- recalculateNormals re-syncs triangles from faces and produces
correct vertex normals on a quad-only mesh.
No behavior change for triangle-only meshes — chunk 1 + chunk 2 keep
the existing path byte-identical. Quad meshes can now render correctly
end-to-end (limited by importers, which still triangulate at load —
that's chunk 3).
Towards #326.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- src/EditableMesh.cpp:355,433: explicit `static_cast<uint32_t>` for `size_t` → Ogre's `vertexCount` / `indexCount` (uint32). The conversion was implicit pre-existing, but Sonar treats my edits to those lines as new and flags the precision-loss warning. - src/EditableMesh.h: convert the new `totalFaceCount()` and `syncTriangulation()` from member methods to free functions taking `std::vector<EditableSubMesh>&`. The class hit Sonar's 35-method ceiling; free functions are the natural pattern here anyway since they mirror the existing `triangulateFaces(EditableSubMesh&)` and `promoteTrianglesToFaces(EditableSubMesh&)` helpers. - Tests updated to call the free-function versions. No behavior change; same 191 tests still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
This was referenced Apr 28, 2026
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
1 participant
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.
Summary
Chunk 2 of the quad migration (#326). Re-opening from #328 because that PR auto-closed when its base branch (
feat/quads-1-foundation) was deleted on chunk 1's merge.Same content as the (now-closed) #328 — fully green CI on the previous run. No code changes since.
Wires
EditableSubMesh::facesthrough the GPU upload path so quad-bearing meshes render correctly via fan-triangulation. No behavior change for triangle-only meshes.Changes
buildSubMeshBuffersre-triangulates faces into triangles defensively whenfacesis non-empty.recalculateNormals/recalculateNormalsFlatre-sync triangles from faces before accumulating.syncTriangulation()andtotalFaceCount()free functions (kept offEditableMeshto stay under Sonar's 35-method ceiling).Tests
+5 standalone tests; full suite stays green (was 173 + 18 → now 191 with chunk 1).
Closes
Re-opens #328 (auto-closed due to base-branch deletion). Towards #326.
🤖 Generated with Claude Code