diff --git a/src/EditableMesh.cpp b/src/EditableMesh.cpp index 5dfd31f52..ea2b2344d 100644 --- a/src/EditableMesh.cpp +++ b/src/EditableMesh.cpp @@ -35,6 +35,45 @@ THE SOFTWARE. #include #include +void triangulateFaces(EditableSubMesh& sub) +{ + sub.triangles.clear(); + if (sub.faces.empty()) return; + + // Best-effort capacity hint: every face emits N - 2 triangles, so + // for a quad-dominant mesh the average is ~2× the face count. The + // caller may have any mix of polygon sizes, so we don't try to be + // exact here. + sub.triangles.reserve(sub.faces.size() * 2); + + for (const auto& face : sub.faces) { + if (!face.isValid()) continue; + const auto& idx = face.indices; + for (size_t i = 1; i + 1 < idx.size(); ++i) { + EditableTriangle t; + t.indices[0] = idx[0]; + t.indices[1] = idx[i]; + t.indices[2] = idx[i + 1]; + sub.triangles.push_back(t); + } + } +} + +void promoteTrianglesToFaces(EditableSubMesh& sub) +{ + sub.faces.clear(); + sub.faces.reserve(sub.triangles.size()); + for (const auto& tri : sub.triangles) { + EditableFace f; + f.indices = {tri.indices[0], tri.indices[1], tri.indices[2]}; + sub.faces.push_back(std::move(f)); + } + // Note: we deliberately do NOT call triangulateFaces() here. The + // existing `triangles` already mirrors what `faces` would generate + // (each face is a single triangle), so the canonical-faces invariant + // already holds. +} + bool EditableMesh::loadFromEntity(Ogre::Entity* entity) { if (!entity) diff --git a/src/EditableMesh.h b/src/EditableMesh.h index b547086a2..7a38f8926 100644 --- a/src/EditableMesh.h +++ b/src/EditableMesh.h @@ -68,19 +68,99 @@ struct EditableTriangle { unsigned int indices[3] = {0, 0, 0}; }; +/** + * @brief A polygonal face referencing N >= 3 vertices by index. + * + * Used for the n-gon (quad-aware) representation that complements the + * legacy triangle-only path. While the editor is being migrated to + * quads, both `EditableSubMesh::faces` and `EditableSubMesh::triangles` + * coexist: + * + * - When `faces` is empty, the submesh is in legacy triangle-only mode + * and `triangles` is the canonical storage. + * - When `faces` is non-empty, `faces` is canonical and `triangles` is + * a fan-triangulated mirror that downstream code (GPU upload, render, + * legacy topology ops) consumes. Helper utilities keep the two + * representations in sync. + * + * The fan-triangulation rule is `[v0, vi, vi+1]` for `i in [1, N-1)`, + * matching what `HalfEdgeMesh::appendFace` and `fillSelection` already + * produce. Convex polygons (the only case the importer is expected to + * produce) survive that rule cleanly; concave n-gons need a future + * ear-clip pass. + */ +struct EditableFace { + std::vector indices; + + /// @return true if this face has at least 3 vertices and no + /// consecutive duplicate index pair (naive sanity check). + bool isValid() const + { + if (indices.size() < 3) return false; + for (size_t i = 0; i < indices.size(); ++i) { + if (indices[i] == indices[(i + 1) % indices.size()]) return false; + } + return true; + } + + /// @return Number of vertices on this face (>= 3 when valid). + size_t vertexCount() const { return indices.size(); } +}; + /** * @brief An editable copy of a single Ogre SubMesh. * - * Contains all vertices, triangles, and the material name. Tracks whether - * the original SubMesh used shared vertex data. + * Contains all vertices, triangles, faces, and the material name. Tracks + * whether the original SubMesh used shared vertex data. + * + * The `faces` (n-gon) array is the canonical face storage when it is + * non-empty; `triangles` then mirrors it as a fan-triangulation kept in + * sync by `triangulateFaces()`. When `faces` is empty, `triangles` is + * canonical and the submesh is in legacy triangle-only mode. */ struct EditableSubMesh { std::vector vertices; std::vector triangles; + std::vector faces; std::string materialName; bool usesSharedVertices = false; }; +/** + * @brief Fan-triangulate every face in `faces` into `triangles`. + * + * Triangulation rule: each face emits `n - 2` triangles fanned from + * `indices[0]`, i.e. `(v0, v_i, v_{i+1})` for `i in [1, n-1)`. This + * matches `HalfEdgeMesh::appendFace` and `fillSelection` so a face that + * survives an HE round-trip recovers the same triangulation it started + * with. + * + * Caller's responsibility: call this whenever `faces` is mutated to + * keep `triangles` in sync. Code paths that don't yet understand n-gons + * (GPU upload, legacy topology ops) consume `triangles`. + * + * Skips invalid faces (`vertexCount < 3`) silently. Replaces any + * existing contents of `triangles`. + * + * @param sub The submesh whose `faces` will be triangulated. After + * return, `sub.triangles` contains the fan-triangulation. + */ +void triangulateFaces(EditableSubMesh& sub); + +/** + * @brief Build trivial single-triangle faces from existing `triangles`. + * + * Inverse of `triangulateFaces` — used when a legacy triangle-only + * submesh needs to be promoted into the n-gon representation. After the + * call, every triangle has a corresponding 3-index `EditableFace`. The + * existing `triangles` array is preserved (the result still satisfies + * the canonical-faces invariant: `triangles[i] == fan(faces[i])`). + * + * @param sub The submesh to promote. Existing contents of `sub.faces` + * are replaced. + */ +void promoteTrianglesToFaces(EditableSubMesh& sub); + /** * @brief Indexed mesh representation for topology queries and editing. * diff --git a/src/HalfEdgeMesh.cpp b/src/HalfEdgeMesh.cpp index a70928e15..b12a52ace 100644 --- a/src/HalfEdgeMesh.cpp +++ b/src/HalfEdgeMesh.cpp @@ -90,27 +90,65 @@ bool HalfEdgeMesh::buildFromEditableMesh(const EditableMesh& editableMesh) } } - // Phase 2: Append a face/3-HE triangle for each input triangle, skipping - // degenerates. Vertex outgoing-HE pointers are set during this loop. + // Phase 2: Append a face for each input face. Each submesh prefers + // `faces` (n-gon canonical storage) when non-empty; otherwise it + // falls back to `triangles` (legacy storage). The two-stream rule + // mirrors EditableSubMesh's invariant: `faces` is canonical iff it + // is non-empty. Skips degenerates and sets vertex outgoing-HE + // pointers as faces are added. + auto registerVertexHE = [this](int vGlobal, int heStart) { + if (m_vertices[vGlobal].halfEdge == -1) { + m_vertices[vGlobal].halfEdge = heStart; + } + }; + for (int s = 0; s < m_subMeshCount; ++s) { const auto& sub = subMeshes[s]; - for (const auto& tri : sub.triangles) { - int v0 = vertexMap[s][tri.indices[0]]; - int v1 = vertexMap[s][tri.indices[1]]; - int v2 = vertexMap[s][tri.indices[2]]; - // Skip degenerate triangles (any two vertices identical) - if (v0 == v1 || v1 == v2 || v0 == v2) - continue; + if (!sub.faces.empty()) { + for (const auto& face : sub.faces) { + if (!face.isValid()) continue; + std::vector verts; + verts.reserve(face.indices.size()); + bool degenerate = false; + for (unsigned int local : face.indices) { + if (local >= vertexMap[s].size()) { + degenerate = true; + break; + } + verts.push_back(vertexMap[s][local]); + } + if (degenerate) continue; + // Reject faces with any duplicated vertex (would create + // a zero-area corner). Cheap O(N²) check is fine for + // typical N <= 8. + for (size_t i = 0; i < verts.size() && !degenerate; ++i) { + for (size_t j = i + 1; j < verts.size(); ++j) { + if (verts[i] == verts[j]) { degenerate = true; break; } + } + } + if (degenerate) continue; + + int he0 = static_cast(m_halfEdges.size()); + appendFace(verts, s); + for (size_t i = 0; i < verts.size(); ++i) { + registerVertexHE(verts[i], he0 + static_cast(i)); + } + } + } else { + for (const auto& tri : sub.triangles) { + int v0 = vertexMap[s][tri.indices[0]]; + int v1 = vertexMap[s][tri.indices[1]]; + int v2 = vertexMap[s][tri.indices[2]]; + + if (v0 == v1 || v1 == v2 || v0 == v2) continue; - int he0 = static_cast(m_halfEdges.size()); - appendTriangle(v0, v1, v2, s); + int he0 = static_cast(m_halfEdges.size()); + appendTriangle(v0, v1, v2, s); - // Set vertex outgoing half-edge pointers (first wins) - int verts[3] = {v0, v1, v2}; - for (int i = 0; i < 3; ++i) { - if (m_vertices[verts[i]].halfEdge == -1) { - m_vertices[verts[i]].halfEdge = he0 + i; + int verts[3] = {v0, v1, v2}; + for (int i = 0; i < 3; ++i) { + registerVertexHE(verts[i], he0 + i); } } } @@ -392,23 +430,34 @@ bool HalfEdgeMesh::toEditableMesh(EditableMesh& editableMesh) const // Per-submesh: map from HE vertex index -> local vertex index std::vector> vertexRemap(m_subMeshCount); + // Track whether any submesh ended up with a non-triangle face. If so, + // we mirror the face list into `EditableSubMesh::faces` so downstream + // n-gon-aware code (chunks 2+) sees the polygon structure. If every + // face is a triangle, we leave `faces` empty to keep the legacy path + // canonical. + std::vector hasNonTriangleFace(m_subMeshCount, false); + for (int f = 0; f < static_cast(m_faces.size()); ++f) { const auto& heFace = m_faces[f]; int subIdx = heFace.subMeshIndex; auto& outSub = outSubMeshes[subIdx]; auto verts = faceVertices(f); - if (verts.size() != 3) - continue; - - EditableTriangle tri; - for (int i = 0; i < 3; ++i) { - int heVertIdx = verts[i]; - + // Accept any face with 3+ vertices. n-gons (4+) get fan- + // triangulated into the legacy `triangles` array AND mirrored + // into `faces` so the n-gon structure survives the round-trip. + // Faces with < 3 vertices are degenerate — silently skip. + if (verts.size() < 3) continue; + + // Map every face vertex into the submesh's local vertex array, + // collecting the local indices in `localIdxs` for later. + std::vector localIdxs; + localIdxs.reserve(verts.size()); + for (int heVertIdx : verts) { auto it = vertexRemap[subIdx].find(heVertIdx); + int localIdx; if (it == vertexRemap[subIdx].end()) { - // Add this vertex to the submesh - int localIdx = static_cast(outSub.vertices.size()); + localIdx = static_cast(outSub.vertices.size()); vertexRemap[subIdx][heVertIdx] = localIdx; EditableVertex ev; @@ -431,13 +480,39 @@ bool HalfEdgeMesh::toEditableMesh(EditableMesh& editableMesh) const } outSub.vertices.push_back(std::move(ev)); - tri.indices[i] = localIdx; } else { - tri.indices[i] = it->second; + localIdx = it->second; } + localIdxs.push_back(static_cast(localIdx)); + } + + // Fan-triangulate the face into `triangles` (legacy storage): + // tris = (v0, v_i, v_{i+1}) for i in [1, N-1) + for (size_t i = 1; i + 1 < localIdxs.size(); ++i) { + EditableTriangle tri; + tri.indices[0] = localIdxs[0]; + tri.indices[1] = localIdxs[i]; + tri.indices[2] = localIdxs[i + 1]; + outSub.triangles.push_back(tri); } - outSub.triangles.push_back(tri); + // Always record the face in `EditableSubMesh::faces` so the n-gon + // round-trip is information-preserving. We finalise below by + // clearing `faces` on submeshes that turned out to be all + // triangles (legacy invariant: faces is non-empty only when + // there's actually polygonal information to preserve). + EditableFace face; + face.indices = std::move(localIdxs); + if (face.indices.size() > 3) hasNonTriangleFace[subIdx] = true; + outSub.faces.push_back(std::move(face)); + } + + // Finalise: drop `faces` on triangle-only submeshes. This keeps the + // legacy invariant that `faces` is empty unless the submesh really + // contains n-gons, so downstream code that doesn't yet understand + // n-gons (every consumer of EditableSubMesh today) keeps working. + for (int s = 0; s < m_subMeshCount; ++s) { + if (!hasNonTriangleFace[s]) outSubMeshes[s].faces.clear(); } return true; @@ -4833,8 +4908,10 @@ bool HalfEdgeMesh::validate() const return false; } while (he != startHE); - // Triangles should have exactly 3 half-edges - if (count != 3) + // Polygonal faces have at least 3 half-edges. Larger N-gons + // (quads etc.) are allowed by the data model — chunks 1+ of the + // quad migration drop the triangle-only assumption. + if (count < 3) return false; } diff --git a/src/HalfEdgeMesh_test.cpp b/src/HalfEdgeMesh_test.cpp index f734acdc5..8f984189b 100644 --- a/src/HalfEdgeMesh_test.cpp +++ b/src/HalfEdgeMesh_test.cpp @@ -4574,3 +4574,245 @@ TEST(HalfEdgeMeshStandalone, FillSelectionUndoRoundTrip) { EXPECT_TRUE(he2.validate()); EXPECT_EQ(activeFaceCount(he2), 2); } + +// =========================================================================== +// EditableFace (n-gon) round-trip — chunk 1 of the quad migration +// =========================================================================== + +TEST(HalfEdgeMeshStandalone, BuildFromQuadFacePopulatesSingleHEFace) { + // A submesh with a single 4-vertex EditableFace (no triangles) + // should produce ONE HalfEdgeMesh face of valence 4. + EditableMesh mesh; + EditableSubMesh sub; + sub.materialName = "QuadMat"; + auto mkV = [](float x, float y) { + EditableVertex v; + v.position = Ogre::Vector3(x, y, 0); + v.normal = Ogre::Vector3(0, 0, 1); v.hasNormal = true; + v.uv = Ogre::Vector2(x, y); v.hasUV = true; + return v; + }; + sub.vertices = { mkV(0, 0), mkV(1, 0), mkV(1, 1), mkV(0, 1) }; + EditableFace f; + f.indices = {0, 1, 2, 3}; + sub.faces.push_back(std::move(f)); + // Note: triangles intentionally left empty — when faces is non-empty + // it's canonical, and HE should read faces directly. + mesh.subMeshes().push_back(std::move(sub)); + + HalfEdgeMesh he; + ASSERT_TRUE(he.buildFromEditableMesh(mesh)); + EXPECT_EQ(activeFaceCount(he), 1); + EXPECT_EQ(he.faceVertices(0).size(), 4u) + << "quad EditableFace must produce a 4-valence HE face, not 2 triangles"; + EXPECT_TRUE(he.validate()); +} + +TEST(HalfEdgeMeshStandalone, ToEditableMeshPreservesQuadInFaces) { + // Round-trip: build HE from a quad face, write back, verify the + // EditableSubMesh has a 4-vertex face (and triangles is the fan + // triangulation). + EditableMesh mesh; + EditableSubMesh sub; + sub.materialName = "QuadMat"; + auto mkV = [](float x, float y) { + EditableVertex v; + v.position = Ogre::Vector3(x, y, 0); + v.normal = Ogre::Vector3(0, 0, 1); v.hasNormal = true; + return v; + }; + sub.vertices = { mkV(0, 0), mkV(1, 0), mkV(1, 1), mkV(0, 1) }; + EditableFace f; + f.indices = {0, 1, 2, 3}; + sub.faces.push_back(std::move(f)); + mesh.subMeshes().push_back(std::move(sub)); + + HalfEdgeMesh he; + ASSERT_TRUE(he.buildFromEditableMesh(mesh)); + + EditableMesh back; + ASSERT_TRUE(he.toEditableMesh(back)); + ASSERT_EQ(back.subMeshes().size(), 1u); + const auto& outSub = back.subMeshes()[0]; + + // n-gon faces preserved + ASSERT_EQ(outSub.faces.size(), 1u) + << "quad must round-trip as a single 4-vertex face"; + EXPECT_EQ(outSub.faces[0].indices.size(), 4u); + + // triangles is the fan triangulation (2 tris from a quad) + EXPECT_EQ(outSub.triangles.size(), 2u) + << "fan-triangulation: a quad emits N-2 = 2 triangles"; + + // Vertex count preserved + EXPECT_EQ(outSub.vertices.size(), 4u); +} + +TEST(HalfEdgeMeshStandalone, ToEditableMeshLeavesFacesEmptyWhenAllTris) { + // The legacy invariant: if every face in the HE mesh is a triangle, + // `EditableSubMesh::faces` should be left empty so existing + // triangle-only consumers don't have to learn the new representation. + auto em = makeQuadMesh(); // two triangles + HalfEdgeMesh he; + ASSERT_TRUE(he.buildFromEditableMesh(em)); + + EditableMesh back; + ASSERT_TRUE(he.toEditableMesh(back)); + ASSERT_EQ(back.subMeshes().size(), 1u); + EXPECT_TRUE(back.subMeshes()[0].faces.empty()) + << "all-triangle submeshes must NOT populate faces (legacy invariant)"; + EXPECT_EQ(back.subMeshes()[0].triangles.size(), 2u); +} + +TEST(HalfEdgeMeshStandalone, BuildFromMixedTriAndQuadSubMeshes) { + // Two submeshes: one triangle-only (legacy path), one quad-only + // (n-gon path). buildFromEditableMesh should produce 3 HE faces + // total (1 tri + 1 quad ... wait, 1 tri + 1 quad = 2 faces). + EditableMesh mesh; + auto mkV = [](float x, float y, float z) { + EditableVertex v; + v.position = Ogre::Vector3(x, y, z); + v.normal = Ogre::Vector3(0, 0, 1); v.hasNormal = true; + return v; + }; + + { + EditableSubMesh sub; + sub.materialName = "TriMat"; + sub.vertices = { mkV(0, 0, 0), mkV(1, 0, 0), mkV(0, 1, 0) }; + EditableTriangle t; + t.indices[0] = 0; t.indices[1] = 1; t.indices[2] = 2; + sub.triangles.push_back(t); + mesh.subMeshes().push_back(std::move(sub)); + } + { + EditableSubMesh sub; + sub.materialName = "QuadMat"; + sub.vertices = { mkV(0, 0, 1), mkV(1, 0, 1), mkV(1, 1, 1), mkV(0, 1, 1) }; + EditableFace f; + f.indices = {0, 1, 2, 3}; + sub.faces.push_back(std::move(f)); + mesh.subMeshes().push_back(std::move(sub)); + } + + HalfEdgeMesh he; + ASSERT_TRUE(he.buildFromEditableMesh(mesh)); + EXPECT_EQ(activeFaceCount(he), 2); + EXPECT_TRUE(he.validate()); + + EditableMesh back; + ASSERT_TRUE(he.toEditableMesh(back)); + ASSERT_EQ(back.subMeshes().size(), 2u); + EXPECT_TRUE(back.subMeshes()[0].faces.empty()) // triangle submesh stays legacy + << "triangle submesh must not populate faces"; + EXPECT_EQ(back.subMeshes()[1].faces.size(), 1u) // quad submesh has 1 face + << "quad submesh must populate faces with the polygon"; + EXPECT_EQ(back.subMeshes()[1].faces[0].indices.size(), 4u); +} + +TEST(HalfEdgeMeshStandalone, EditableFaceIsValidGuardsBadInputs) { + EditableFace empty; + EXPECT_FALSE(empty.isValid()); + + EditableFace pair; + pair.indices = {0, 1}; + EXPECT_FALSE(pair.isValid()); + + EditableFace dup; + dup.indices = {0, 1, 1, 2}; + EXPECT_FALSE(dup.isValid()) << "consecutive duplicate index pair must fail"; + + EditableFace okQuad; + okQuad.indices = {0, 1, 2, 3}; + EXPECT_TRUE(okQuad.isValid()); + + EditableFace okTri; + okTri.indices = {0, 1, 2}; + EXPECT_TRUE(okTri.isValid()); +} + +TEST(HalfEdgeMeshStandalone, BuildFromQuadFaceRejectsInvalidIndex) { + // An out-of-range index in EditableFace::indices must not crash — + // the face is silently skipped. + EditableMesh mesh; + EditableSubMesh sub; + sub.materialName = "Bad"; + EditableVertex v; + v.position = Ogre::Vector3::ZERO; + sub.vertices = {v, v, v}; // 3 verts available + EditableFace f; + f.indices = {0, 1, 99}; // 99 is out of range + sub.faces.push_back(std::move(f)); + mesh.subMeshes().push_back(std::move(sub)); + + HalfEdgeMesh he; + EXPECT_TRUE(he.buildFromEditableMesh(mesh)); + EXPECT_EQ(activeFaceCount(he), 0) + << "out-of-range face must be skipped, not crash"; +} + +// =========================================================================== +// triangulateFaces / promoteTrianglesToFaces helpers +// =========================================================================== + +TEST(HalfEdgeMeshStandalone, TriangulateFacesQuadProducesTwoTriangles) { + EditableSubMesh sub; + EditableFace f; + f.indices = {0, 1, 2, 3}; + sub.faces.push_back(std::move(f)); + + triangulateFaces(sub); + ASSERT_EQ(sub.triangles.size(), 2u); + // Fan from vertex 0: (0, 1, 2) and (0, 2, 3). + EXPECT_EQ(sub.triangles[0].indices[0], 0u); + EXPECT_EQ(sub.triangles[0].indices[1], 1u); + EXPECT_EQ(sub.triangles[0].indices[2], 2u); + EXPECT_EQ(sub.triangles[1].indices[0], 0u); + EXPECT_EQ(sub.triangles[1].indices[1], 2u); + EXPECT_EQ(sub.triangles[1].indices[2], 3u); +} + +TEST(HalfEdgeMeshStandalone, TriangulateFacesPentagonProducesThreeTriangles) { + EditableSubMesh sub; + EditableFace f; + f.indices = {10, 20, 30, 40, 50}; // arbitrary indices + sub.faces.push_back(std::move(f)); + + triangulateFaces(sub); + EXPECT_EQ(sub.triangles.size(), 3u) << "N=5 fan emits N-2 = 3 triangles"; +} + +TEST(HalfEdgeMeshStandalone, TriangulateFacesSkipsInvalidFaces) { + EditableSubMesh sub; + { + EditableFace bad; // <3 indices, isValid() false + bad.indices = {0, 1}; + sub.faces.push_back(std::move(bad)); + } + { + EditableFace good; + good.indices = {0, 1, 2}; + sub.faces.push_back(std::move(good)); + } + triangulateFaces(sub); + EXPECT_EQ(sub.triangles.size(), 1u) << "invalid face skipped"; +} + +TEST(HalfEdgeMeshStandalone, PromoteTrianglesToFacesProducesMatchingFaces) { + EditableSubMesh sub; + EditableTriangle t1, t2; + t1.indices[0] = 0; t1.indices[1] = 1; t1.indices[2] = 2; + t2.indices[0] = 1; t2.indices[1] = 3; t2.indices[2] = 2; + sub.triangles = {t1, t2}; + + promoteTrianglesToFaces(sub); + ASSERT_EQ(sub.faces.size(), 2u); + EXPECT_EQ(sub.faces[0].indices.size(), 3u); + EXPECT_EQ(sub.faces[0].indices[0], 0u); + EXPECT_EQ(sub.faces[0].indices[1], 1u); + EXPECT_EQ(sub.faces[0].indices[2], 2u); + // triangles should still match the canonical faces invariant + // (every face is a triangle, so triangulateFaces would produce + // identical content) + EXPECT_EQ(sub.triangles.size(), 2u); +}