Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
20a272f
ci: also trigger deploy.yml on feat/quads
fernandotonon Apr 28, 2026
0c13695
ci: extend deploy.yml triggers to cover stacked feat/quads-* PRs
fernandotonon Apr 28, 2026
ba49a76
feat(quads): chunk 1 — EditableFace foundation
fernandotonon Apr 28, 2026
e28245d
feat(quads): chunk 2 — GPU upload n-gon triangulation
fernandotonon Apr 28, 2026
f9d1674
fix(quads): address SonarCloud Quality Gate failures (chunk 2)
fernandotonon Apr 28, 2026
a8865ac
feat(quads): chunk 3 — importer quad detection (option A)
fernandotonon Apr 28, 2026
9959832
Merge pull request #327 from fernandotonon/feat/quads-1-foundation
fernandotonon Apr 28, 2026
86d073e
Merge pull request #330 from fernandotonon/feat/quads-2-gpu-upload
fernandotonon Apr 28, 2026
cd97a24
Merge pull request #331 from fernandotonon/feat/quads-3-importer
fernandotonon Apr 28, 2026
93e2b33
feat(quads): chunk 4 — wire enterEditMode to n-gon import
fernandotonon Apr 28, 2026
83e709f
Merge remote-tracking branch 'origin/master' into feat/quads-4-edit-m…
fernandotonon Apr 28, 2026
84ec634
fix(quads): preserve X-handedness in n-gon re-import (chunk 4)
fernandotonon Apr 28, 2026
7a61fcd
feat(quads): chunk 5a — Catmull-Clark subdivide
fernandotonon Apr 28, 2026
82cfb0e
fix(quads): address Codex P1 review on chunk 5a
fernandotonon Apr 28, 2026
575f4f8
feat(quads): chunk 4b — n-gon-aware face selection + edge filtering
fernandotonon Apr 28, 2026
d05b23f
fix(quads): address Codex P2 review on chunk 4b
fernandotonon Apr 28, 2026
fec543a
Merge pull request #332 from fernandotonon/feat/quads-4-edit-mode-ngon
fernandotonon Apr 28, 2026
0533370
Merge pull request #333 from fernandotonon/feat/quads-5a-catmull-clark
fernandotonon Apr 28, 2026
f2d6f8f
Merge pull request #334 from fernandotonon/feat/quads-4b-face-selection
fernandotonon Apr 28, 2026
f09e755
fix(quads): preserve transforms + bone handles in n-gon re-import
fernandotonon Apr 28, 2026
49f5a2b
fix(quads): preserve bump map + per-pixel lighting after topology ops
fernandotonon Apr 29, 2026
60a118d
fix(quads): fill produces a single n-gon; knife works on quad meshes
fernandotonon Apr 29, 2026
1aaff0d
fix(quads): guard normal-map mat->load against broken resources
fernandotonon Apr 29, 2026
4af6cb6
Merge branch 'fix/quads-followup' into fix/knife-and-fill
fernandotonon Apr 29, 2026
4de903a
fix(quads): fill orients to surrounding mesh; knife uses same HE in h…
fernandotonon Apr 29, 2026
197a673
fix(quads): knife accepts OnVertex clicks on dense imported meshes
fernandotonon Apr 29, 2026
4c96d0f
fix(quads): knife hit-test culls back-face geometry
fernandotonon Apr 29, 2026
2b0a1cc
refactor(quads): extract helpers to satisfy Sonar complexity gate
fernandotonon Apr 29, 2026
bbf4f0d
refactor(quads): extract helpers to satisfy Sonar complexity gate
fernandotonon Apr 29, 2026
fcc9c65
Merge pull request #335 from fernandotonon/fix/quads-followup
fernandotonon Apr 29, 2026
c7da02c
fix(quads): knife restores n-gon faces on untouched submeshes
fernandotonon Apr 29, 2026
0761ca1
Merge pull request #337 from fernandotonon/fix/knife-and-fill
fernandotonon Apr 29, 2026
7ac44be
feat(quads): n-gon-aware splitEdge; extrude offset for n-gon caps
fernandotonon Apr 29, 2026
45b6152
feat(quads): n-gon-aware vertex/edge dissolve + merge cleanup
fernandotonon Apr 29, 2026
7563d5a
Merge pull request #338 from fernandotonon/feat/ngon-split-edge
fernandotonon Apr 29, 2026
37e172d
feat(quads): n-gon-aware edge bevel (MVP)
fernandotonon Apr 29, 2026
4e65fbd
feat(quads): n-gon-aware vertex bevel + edge bevel segments
fernandotonon Apr 29, 2026
96930e8
fix(quads): bevelEdgesNgon resolves edge by vertex pair between itera…
fernandotonon Apr 29, 2026
39018ac
fix(quads): bevelEdgesNgon splices chain into neighbor faces
fernandotonon Apr 29, 2026
587cc2e
docs(quads): annotate triangle-only bevel functions as legacy path
fernandotonon Apr 29, 2026
cf46f43
fix(quads): address Codex P1 + P2 review on PR #339
fernandotonon Apr 29, 2026
7ab0b04
Merge pull request #339 from fernandotonon/feat/ngon-bevel
fernandotonon Apr 29, 2026
18a2777
fix(quads): n-gon bevel chamfer twist on imported meshes (#340)
fernandotonon Apr 29, 2026
bf750f3
feat(quads): loop cut (#341)
fernandotonon Apr 29, 2026
9c9bf51
feat(quads): tris→quads converter + quad-aware wireframe (#344)
fernandotonon Apr 29, 2026
aa8910b
Merge remote-tracking branch 'origin/master' into feat/quads
fernandotonon Apr 29, 2026
92d13f5
Merge remote-tracking branch 'origin/master' into feat/quads
fernandotonon Apr 29, 2026
31ed94c
fix(quads): Ctrl+R consume + dedicated edit-hint label (#347 review)
fernandotonon Apr 29, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions .github/workflows/deploy.yml
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
name: Deploy

on:
on:
push:
branches: [ "master" ]
branches: [ "master", "feat/quads", "feat/quads-*" ]
pull_request:
branches: [ "master" ]
# `master` covers shipping work; `feat/quads` is the long-lived
# quad-migration integration branch (#326) that chunked PRs land
# on before the final merge to master. The `feat/quads-*` glob
# covers chunked PRs that target a previous chunk's branch
# (stacked PRs). Without these, chunked PRs would have no CI
# safety net.
branches: [ "master", "feat/quads", "feat/quads-*" ]
release:
types: [published]

Expand Down
1,276 changes: 1,032 additions & 244 deletions src/EditModeController.cpp

Large diffs are not rendered by default.

113 changes: 113 additions & 0 deletions src/EditModeController.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,11 @@ class EditModeController : public QObject
Q_PROPERTY(bool wireframeEnabled READ wireframeEnabled WRITE setWireframeEnabled NOTIFY wireframeChanged)
bool wireframeEnabled() const { return m_wireframeEnabled; }
void setWireframeEnabled(bool enabled);

/// True if any submesh of the current edit mesh has non-empty
/// n-gon `.faces` — i.e. quad-based already. Drives the
/// "Convert to Quads" toolbar button enable state.
Q_INVOKABLE bool isMeshQuadBased() const;
/// @}

/// @name Mesh info (only valid when in edit mode)
Expand Down Expand Up @@ -462,6 +467,47 @@ class EditModeController : public QObject
*/
Q_INVOKABLE int subdivideSelection();

/**
* @brief Insert a loop cut starting from the first selected edge.
*
* Walks the chain of quads adjacent to the start edge via the
* "opposite edge" relation. Each quad in the ring is bisected by
* splitting two parallel edges at their midpoints and connecting
* the new midpoints with a new edge. The walk terminates at
* boundaries, non-quad faces, or when it loops back to the start.
*
* Requires Edge selection mode and at least one selected edge.
* Uses the FIRST selected edge as the start; multi-edge loop cuts
* are out of scope for the MVP (each cut is independent).
*
* Pushes one undo command labeled "Loop Cut".
*
* @return Number of new vertices inserted (0 on no-op / failure).
*/
Q_INVOKABLE int loopCutSelection();

/**
* @brief Subdivide the entire mesh by one Catmull-Clark step.
*
* Unlike `subdivideSelection` (which does a 1-to-4 triangle split
* on the selected faces only), this op operates on the whole mesh
* at once and produces an all-quad output regardless of input
* topology — a triangle becomes 3 quads, a quad becomes 4. Output
* geometry is smoothed via the classic Catmull-Clark rule (face
* points, edge points, smoothed vertex positions) so the surface
* approaches a C¹-continuous limit on closed manifolds.
*
* Selection is cleared after the op (the new face/edge points
* don't have stable analogues in the pre-op selection set, and
* partial-mesh CC needs a more sophisticated boundary blend that
* isn't in this MVP).
*
* Pushes one undo command labeled "Catmull-Clark Subdivide".
*
* @return Number of vertices added (0 on no-op).
*/
Q_INVOKABLE int subdivideCatmullClarkAll();

/**
* @brief Fill the current selection with new face(s).
*
Expand All @@ -481,6 +527,28 @@ class EditModeController : public QObject
* @return Number of triangles created (0 on no-op or rejection).
*/
Q_INVOKABLE int fillSelection();

/**
* @brief Merge coplanar adjacent triangle pairs into quads.
*
* Whole-mesh operation: walks every submesh, looks for triangle pairs
* that share an edge and are within `angleThresholdDeg` of coplanar
* (default 1°), and merges each qualifying pair into a single quad
* face. Promotes the legacy triangle-only representation into the
* n-gon canonical form. After the operation, downstream features
* that branch on n-gon `.faces` (loop cut, n-gon-aware bevel,
* quad-aware wireframe) start working.
*
* No-op when nothing qualifies (all tris are non-coplanar, or the
* mesh is already quad-dominant). Pushes one undo command labeled
* "Convert to Quads" iff merges happened.
*
* @param angleThresholdDeg Maximum dihedral angle (deg) to treat as
* coplanar. 0 = strict, ~5 = forgiving for float-quantised
* imports. Defaults to 1°.
* @return Number of triangle pairs merged across all submeshes.
*/
Q_INVOKABLE int convertToQuads(float angleThresholdDeg = 1.0f);
/// @}

/// @name Vertex transform support
Expand Down Expand Up @@ -681,6 +749,18 @@ class EditModeController : public QObject

/// Convert (subMeshIndex, localTriangleIndex) to a global triangle index.
int localTriToGlobal(size_t subMeshIndex, size_t localTriIndex) const;

/// @brief Convert the current `m_selectedFaces` set (global triangle
/// indices) into a deduplicated list of HE face indices that
/// HalfEdgeMesh ops accept. Each unique HE face is reported once
/// regardless of how many of its fan-triangulated children appear
/// in the selection — so a quad selected via either of its
/// triangles maps to a single HE face.
///
/// HE face indexing matches `HalfEdgeMesh::buildFromEditableMesh`
/// order: submesh 0's faces first (or its triangles, in legacy
/// triangle-only submeshes), then submesh 1's, etc.
std::vector<int> selectedFacesAsHEFaceIndices() const;
/// @}

signals:
Expand Down Expand Up @@ -709,6 +789,12 @@ class EditModeController : public QObject
void knifeSessionChanged();
void vertexPaintChanged();

/// Emitted when an edit-mode op short-circuits and wants to surface
/// a one-line explanation to the user (e.g. "Loop cut requires a
/// quad mesh — try Mesh → Convert to Quads"). QML overlays /
/// status-bar widgets can subscribe.
void editHintMessage(const QString& message);

private slots:
void onSelectionChanged();

Expand Down Expand Up @@ -751,6 +837,12 @@ private slots:
Ogre::ManualObject* m_overlayVertices = nullptr;
Ogre::ManualObject* m_overlayEdges = nullptr;
Ogre::ManualObject* m_overlayFaces = nullptr;
/// Quad-aware wireframe: lines along n-gon face boundaries only,
/// hiding the diagonals introduced by `triangulateFaces()`. Active
/// when `m_wireframeEnabled` AND any submesh has non-empty `.faces`.
/// On legacy triangle-only meshes this stays empty and the
/// PM_WIREFRAME material override does the work instead.
Ogre::ManualObject* m_overlayBoundaryEdges = nullptr;
Ogre::SceneNode* m_overlayNode = nullptr;

// Bevel session state — populated on beginBevel, consumed on commit/cancel.
Expand Down Expand Up @@ -909,6 +1001,27 @@ private slots:
// Wireframe helpers
void applyWireframeMaterials();
void removeWireframeMaterials();
/// True if any submesh has non-empty `.faces` (n-gon canonical).
/// Drives the choice between PM_WIREFRAME (all submeshes pure tris)
/// and the boundary-edge overlay (any n-gon faces present).
bool meshHasNgonFaces() const;
/// (Re)build the n-gon boundary-edge overlay from `m_editableMesh`.
/// Active when `m_wireframeEnabled` AND `meshHasNgonFaces()` —
/// otherwise clears the overlay so it draws nothing.
void updateBoundaryEdgeOverlay();

public:
/// Refresh an entity after a topology mutation: rebuild tangents
/// (when any material is bump-mapped), `_deinitialise/_initialise`
/// the entity, restore per-subentity material overrides, re-attach
/// RTSS SRS_NORMALMAP, and invalidate cached shader programs. Used
/// by every Edit-Mode topology op (subdivide, extrude, bevel, …)
/// AND by `EditMeshTopologyCommand::applyMeshState` so undo/redo
/// preserves bump map / per-pixel lighting state. Static so
/// command code can invoke it without a controller instance.
static void rewriteEntityAfterTopologyChange(Ogre::Entity* ent);

private:

// Wireframe mode
bool m_wireframeEnabled = false;
Expand Down
127 changes: 127 additions & 0 deletions src/EditModeController_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,35 @@ The MIT License
#include "HalfEdgeMesh.h"
#include <Ogre.h>
#include <QSignalSpy>
#include <QDir>
#include <QFile>
#include <QTextStream>
#include <set>
#include <utility>
#include <cmath>
#include <array>
#include <map>

// ===========================================================================
// rewriteEntityAfterTopologyChange — null + no-bump-map shape (chunk: quads
// follow-up). Full bump-map / RTSS exercise needs a GL context, which the
// test infra doesn't provide on macOS; the integration coverage lives in
// hand smoke tests on the bump-mapped Mixamo asset. Here we lock down the
// public-API contract: static, callable from outside the class (notably
// from EditMeshTopologyCommand::applyMeshState in the undo/redo path),
// null-tolerant, and a no-op when no entity material requests tangents.
// ===========================================================================

TEST(EditModeControllerStandalone, RewriteEntityAfterTopologyChangeNullIsNoOp) {
// Regression for chunk 4b → quads follow-up: the static helper is
// reachable from command code in TransformCommands.cpp via a class-
// qualified call, and accepts a null entity without crashing. If
// someone refactors it back to a free function in an anonymous
// namespace, undo/redo will silently lose its lighting hook again.
EditModeController::rewriteEntityAfterTopologyChange(nullptr);
SUCCEED();
}

// ===========================================================================
// Pure geometry tests (no Ogre needed)
// ===========================================================================
Expand Down Expand Up @@ -1937,3 +1960,107 @@ TEST_F(EditModeControllerBevelE2ETest, FillSelectionPushesUndoCommand) {
EXPECT_EQ(trisAfterUndo.size(), triCountBefore - 1)
<< "undo after fill should drop back to the post-delete tri count";
}

// ===========================================================================
// enterEditMode: n-gon import path (chunk 4)
// ===========================================================================

namespace {
// Write a minimal quad OBJ to a temp file. Mirrors the helper in
// EditableMesh_test.cpp (intentionally duplicated rather than shared
// across translation units, since the test files don't share a TU).
QString writeQuadObjForCtrl(const QString& baseName)
{
const QString path = QDir::tempPath() + "/" + baseName + ".obj";
QFile f(path);
if (!f.open(QIODevice::WriteOnly | QIODevice::Truncate)) return {};
QTextStream out(&f);
out << "v 0 0 0\nv 1 0 0\nv 1 1 0\nv 0 1 0\nf 1 2 3 4\n";
f.close();
return path;
}
} // namespace

TEST_F(EditModeControllerBevelE2ETest, EnterEditModeUsesNgonPathWhenSourceCached) {
// When the entity's mesh has qtme.source_path set, enterEditMode
// should re-import via Assimp with aiProcess_Triangulate disabled,
// populating EditableSubMesh::faces with the polygon structure.
const QString objPath = writeQuadObjForCtrl("ctrl_ngon_path");
ASSERT_FALSE(objPath.isEmpty());

// Tag the cube mesh from the fixture's setup with a source path
// pointing at the OBJ. enterEditMode should then re-import the OBJ
// (overriding the cube's geometry — that's fine, the test only
// verifies that the n-gon code path fired, not vertex content).
auto* mesh = m_entity->getMesh().get();
mesh->getUserObjectBindings().setUserAny(
"qtme.source_path", Ogre::Any(objPath.toStdString()));

auto* ctrl = EditModeController::instance();
ASSERT_TRUE(ctrl->enterEditMode());

auto* editableMesh = ctrl->currentMesh();
ASSERT_NE(editableMesh, nullptr);
ASSERT_FALSE(editableMesh->subMeshes().empty());
EXPECT_FALSE(editableMesh->subMeshes()[0].faces.empty())
<< "enterEditMode with source_path set should populate faces "
"via the n-gon-aware loadFromAssimpFile path";
EXPECT_EQ(editableMesh->subMeshes()[0].faces[0].indices.size(), 4u);

QFile::remove(objPath);
}

TEST_F(EditModeControllerBevelE2ETest, EnterEditModeFallsBackToLegacyWhenNoSourcePath) {
// The fixture's cube has no qtme.source_path tag. enterEditMode
// should fall through to loadFromEntity and produce the
// triangle-only legacy submesh shape (faces empty).
auto* mesh = m_entity->getMesh().get();
EXPECT_FALSE(mesh->getUserObjectBindings().getUserAny(
"qtme.source_path").has_value())
<< "fixture cube starts without source path — sanity check";

auto* ctrl = EditModeController::instance();
ASSERT_TRUE(ctrl->enterEditMode());

auto* editableMesh = ctrl->currentMesh();
ASSERT_NE(editableMesh, nullptr);
ASSERT_FALSE(editableMesh->subMeshes().empty());
EXPECT_TRUE(editableMesh->subMeshes()[0].faces.empty())
<< "no source path → legacy path → faces empty (chunk-1 invariant)";
}

TEST_F(EditModeControllerBevelE2ETest, EnterEditModeAfterEditDoesNotReimport) {
// Enter Edit Mode with the n-gon path (sets faces), make a
// committable edit, exit, re-enter. The second entry must use the
// legacy path because the first edit wiped qtme.source_path.
const QString objPath = writeQuadObjForCtrl("ctrl_ngon_post_edit");
ASSERT_FALSE(objPath.isEmpty());

auto* mesh = m_entity->getMesh().get();
mesh->getUserObjectBindings().setUserAny(
"qtme.source_path", Ogre::Any(objPath.toStdString()));

auto* ctrl = EditModeController::instance();
ASSERT_TRUE(ctrl->enterEditMode());
ASSERT_FALSE(ctrl->currentMesh()->subMeshes().empty());
ASSERT_FALSE(ctrl->currentMesh()->subMeshes()[0].faces.empty())
<< "first entry must take the n-gon path";

// Commit any change (translate one vertex by zero — still triggers
// the commitToEntity path that wipes the source path).
ctrl->setSelectionMode(EditModeController::VertexMode);
ctrl->selectVertex(0);
ctrl->translateSelectedVertices(Ogre::Vector3::ZERO);
ctrl->exitEditMode(/*commitChanges*/ true);
Comment on lines +2049 to +2054

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Make this test perform a real mutation.

translateSelectedVertices(Ogre::Vector3::ZERO) is a no-op. If the controller ever short-circuits zero-delta transforms, this test stops covering the commit path that clears qtme.source_path. Use a tiny non-zero move or another guaranteed edit here.

Suggested change
-    // Commit any change (translate one vertex by zero — still triggers
-    // the commitToEntity path that wipes the source path).
+    // Commit a real change so this test definitely exercises the
+    // commitToEntity path that wipes the source path.
     ctrl->setSelectionMode(EditModeController::VertexMode);
     ctrl->selectVertex(0);
-    ctrl->translateSelectedVertices(Ogre::Vector3::ZERO);
+    ctrl->translateSelectedVertices(Ogre::Vector3(0.001f, 0.0f, 0.0f));
     ctrl->exitEditMode(/*commitChanges*/ true);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Commit any change (translate one vertex by zero — still triggers
// the commitToEntity path that wipes the source path).
ctrl->setSelectionMode(EditModeController::VertexMode);
ctrl->selectVertex(0);
ctrl->translateSelectedVertices(Ogre::Vector3::ZERO);
ctrl->exitEditMode(/*commitChanges*/ true);
// Commit a real change so this test definitely exercises the
// commitToEntity path that wipes the source path.
ctrl->setSelectionMode(EditModeController::VertexMode);
ctrl->selectVertex(0);
ctrl->translateSelectedVertices(Ogre::Vector3(0.001f, 0.0f, 0.0f));
ctrl->exitEditMode(/*commitChanges*/ true);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/EditModeController_test.cpp` around lines 2049 - 2054, The test currently
uses translateSelectedVertices(Ogre::Vector3::ZERO) which is a no-op; change it
to perform a real mutation so the commit path that clears qtme.source_path is
exercised: after calling ctrl->setSelectionMode(EditModeController::VertexMode)
and ctrl->selectVertex(0), call ctrl->translateSelectedVertices with a tiny
non-zero vector (e.g. Ogre::Vector3(1e-3f,0,0) or similar) or another guaranteed
edit, then keep ctrl->exitEditMode(true) to ensure the commitToEntity path runs
and qtme.source_path is cleared.


EXPECT_FALSE(mesh->getUserObjectBindings().getUserAny(
"qtme.source_path").has_value())
<< "exitEditMode commit must have wiped the source path";

// Re-enter: should fall back to legacy path now.
ASSERT_TRUE(ctrl->enterEditMode());
EXPECT_TRUE(ctrl->currentMesh()->subMeshes()[0].faces.empty())
<< "second entry (post-edit) must use legacy loadFromEntity path";

QFile::remove(objPath);
}
Loading
Loading