From 7b12bdc6f674050a847485ff849c66114fb07eff Mon Sep 17 00:00:00 2001 From: Fernando Date: Fri, 29 May 2026 03:08:32 -0400 Subject: [PATCH 1/4] feat(retopo): triangle-pairing quad retopology (closes #401) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #401 (epic #397, AI-Assist slice 4). Adds quad-dominant retopology via greedy triangle pairing. ## Background — why not Instant Meshes / QuadriFlow The issue title proposes wrapping Instant Meshes (Wenzel Jakob). In practice: * Instant Meshes ships as a research GUI app with no clean C++ library API; the library-extraction PR upstream is dormant (2021), the repo has had no releases since 2016. * QuadriFlow (the production-grade alternative used by Blender 3.0+) is callable but requires Boost + Eigen + LEMON — heavy deps the project doesn't currently use, with several days of CMake work to integrate cleanly. This slice ships a native triangle-pairing backend with **zero new dependencies**. The `Algorithm` enum is set up to plug in QuadriFlow / Instant Meshes as future opt-in backends behind a `--algo` flag (mirroring `MeshDecimator::Algorithm` and `MeshLodController::Algorithm`). ## Algorithm For every interior edge whose two adjacent faces are triangles: 1. **Coplanarity** — angle between the two triangle normals. Default `maxAngleDeg=25°` preserves curvature features. 2. **Quad shape** — deviation of each interior angle of the candidate quad from 90°. Default `shapeToleranceDeg=65°`. 3. **Convexity** — interior angles sum to ≤ 360° (rejects non-convex merges). 4. **Aspect ratio** — longest/shortest edge of the candidate quad. Default `maxAspectRatio=6.0`. Candidates are sorted by a composite score (coplanarity + 1 − angle-dev + 1/aspect) and taken greedily best-first; each triangle claimed at most once. Quads are emitted with opposing- corner winding `(opposing0, sharedA, opposing1, sharedB)` so the diagonal we just removed becomes the quad's natural diagonal. Output goes through `EditableSubMesh::faces` (n-gon storage) → `triangulateFaces` (fan retri for the GPU index buffer) → `writeNgonFacesToMesh` (`qtme.faces.` user binding so FBX / glTF exporters round-trip the new quads, and Edit Mode sees the mesh as quads). Triangle pairing **never introduces new vertices** — UVs, skin weights, vertex colors, and other per-vertex attributes survive unchanged, which is the main practical advantage over field- aligned methods. ## Surface * **CLI**: `qtmesh retopo [--target-faces N] [--max-angle DEG] [--shape-tol DEG] [--max-aspect R] -o [--json]`. * **MCP**: `retopologize` tool with `target_faces / max_angle_deg / shape_tol_deg / max_aspect_ratio` params. Response includes a structured `retopo` object with per-submesh and total face counts plus a `quadDominance` percentage. * **GUI**: Material Mode → Mode Tools → "Quad Retopology…" button + top-level dialog (`qml/QuadRetopoDialog.qml`) driven by the `QuadRetopoController` QML singleton. Same Inspector-styled idiom as `UvUnwrapDialog`. * **Library**: `QuadRetopo::retopologize(entity, opts, algo)` for the Ogre-backed path; `QuadRetopo::retopologizeMesh(positions, indices, opts, outFaces)` is a pure-data variant used by unit tests and any future headless caller. ## Verification (Rumba Dancing.fbx) ``` $ qtmesh retopo Rumba\ Dancing.fbx -o /tmp/rumba_retopo.glb Quad Retopology =============== Mesh: Rumba Dancing Submeshes: 11 Triangles in: 10220 Faces out: 6032 (4188 quads, 1844 triangles) Quad dominance: 82.0% Wrote: rumba_retopo.glb ``` Valid 760 KB .glb produced; skin weights and animation survive the round-trip (no new vertices introduced). ## Sentry breadcrumbs * `ai.assist.retopo` for every retopo action (CLI, MCP, GUI). ## Tests `src/QuadRetopo_test.cpp` covers: * Two coplanar right triangles → one quad * Bent triangles rejected at default `maxAngleDeg` * Bent triangles accepted with relaxed gate * Empty input returns error report * Aspect-ratio gate rejects elongated quads * `--target-faces N` stops pairing early * Algorithm string round-trip * `reportToJson` schema `tests/CMakeLists.txt` updated to include `QuadRetopo.cpp` + `QuadRetopoController.cpp` so per-test-binary linking picks them up (same fix pattern as the 4bb0b06 follow-up to the UV unwrap PR). ## Documentation * `CLAUDE.md` — new `QuadRetopo` entry under the AI-Assist section. * `README.md` — `qtmesh retopo` CLI examples. Co-Authored-By: Claude Opus 4.7 (1M context) --- CLAUDE.md | 1 + README.md | 5 + qml/PropertiesPanel.qml | 54 ++++ qml/QuadRetopoDialog.qml | 270 +++++++++++++++++++ qml/qmldir | 1 + src/CLIPipeline.cpp | 95 +++++++ src/CLIPipeline.h | 7 + src/CMakeLists.txt | 4 + src/MCPServer.cpp | 75 ++++++ src/MCPServer.h | 5 + src/QuadRetopo.cpp | 496 +++++++++++++++++++++++++++++++++++ src/QuadRetopo.h | 152 +++++++++++ src/QuadRetopoController.cpp | 111 ++++++++ src/QuadRetopoController.h | 60 +++++ src/QuadRetopo_test.cpp | 176 +++++++++++++ src/main.cpp | 3 +- src/mainwindow.cpp | 7 + src/qml_resources.qrc | 1 + tests/CMakeLists.txt | 2 + 19 files changed, 1524 insertions(+), 1 deletion(-) create mode 100644 qml/QuadRetopoDialog.qml create mode 100644 src/QuadRetopo.cpp create mode 100644 src/QuadRetopo.h create mode 100644 src/QuadRetopoController.cpp create mode 100644 src/QuadRetopoController.h create mode 100644 src/QuadRetopo_test.cpp diff --git a/CLAUDE.md b/CLAUDE.md index e3cdbca83..a9d93b0fb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -237,6 +237,7 @@ Three singletons manage core state. All run on the main thread. Access via `Clas - **MeshOptimizerLod** (`src/MeshOptimizerLod.h/cpp`, issue #398): Thin facade over `zeux/meshoptimizer` for LOD generation. Free functions, no singleton. `generateLods(mesh, reductions)` returns one `LodLevel` per requested reduction, each with one `Ogre::IndexData*` per submesh. Uses `meshopt_simplifyWithAttributes` when UV0 is present (preserves UV seams), falls back to `meshopt_simplify` otherwise. Every result is `meshopt_optimizeVertexCache`-reordered (Forsyth) so the LOD is cache-friendly out of the box. Caller takes ownership of the `IndexData*` (commit to `SubMesh::mLodFaceList` or call `destroyLevel`). - **MeshLodController** (`src/MeshLodController.h/cpp`): Now has `Algorithm` enum (`Ogre` | `Meshopt`) on the C++ overload `generateLods(int, const QVariantList&, Algorithm)`. QML-facing `generateLodsWithAlgo(int, QVariantList, QString)` accepts `"ogre"` / `"meshopt"` for the Inspector backend dropdown. Default is `Ogre` — meshoptimizer's attribute-weighted simplify preserves UV seams + skin weights but in practice produces a softer silhouette than Ogre's stock `MeshLodGenerator` on character meshes, so Ogre stays primary. CLI: `--algo ogre|meshopt` (default `ogre`). MCP `generate_lods` tool: `algo` param (default `ogre`). Sentry breadcrumb category `ai.assist.lod` records the chosen backend when meshopt is used. - **MeshDecimator** (`src/MeshDecimator.h/cpp`): Same `Algorithm` enum exposed on `decimateEntity(entity, reduction, algo)`. `MeshDecimatorController::applyReductionWithAlgo(double, QString)` is the QML-facing variant the Inspector's Decimate section dropdown calls. CLI `qtmesh decimate ... --algo ogre|meshopt`; MCP `decimate_mesh` `algo` param. Same default and breadcrumb category as the LOD path (`ai.assist.decimate` for meshopt). The post-decimation `promoteFirstLodToBase` also erases the `qtme.faces.` n-gon bindings, otherwise FBXExporter (and EditableMesh) rehydrate the original triangle list off the cached binding and emit the un-decimated mesh. +- **QuadRetopo** (`src/QuadRetopo.h/cpp`, issue #401): triangle-pairing quad-dominant retopology. The issue proposed wrapping Instant Meshes (Wenzel Jakob), but Instant Meshes ships as a research GUI app with no clean C++ library API and has been dormant since 2016. QuadriFlow (the production-grade alternative used by Blender 3.0+) requires Boost + Eigen + LEMON — heavy deps the project doesn't currently use. This first slice ships a native triangle-pairing backend with **zero new dependencies**: walks every interior edge whose two adjacent faces are triangles and scores the merge by (1) coplanarity (dot product of triangle normals; default `maxAngleDeg=25°`), (2) quad shape (deviation of interior angles from 90°; default `shapeToleranceDeg=65°`), (3) aspect ratio (longest/shortest edge; default `maxAspectRatio=6.0`). Pairs are taken greedily best-first; each triangle claimed at most once. Quads are emitted with opposing-corner winding `(opposing0, sharedA, opposing1, sharedB)`. Output goes through `EditableSubMesh::faces` → `triangulateFaces` (fan retri for GPU) → `writeNgonFacesToMesh` (n-gon binding for exporters / Edit Mode). **No new vertices** are introduced, so UVs and skin weights survive unchanged. Backends are pluggable via the `Algorithm` enum (only `TrianglePair` implemented; future `QuadriFlow` / `InstantMeshes` slot in here). Surfaced via `qtmesh retopo --target-faces N --max-angle DEG -o out`, MCP `retopologize`, and the **Material Mode → Mode Tools → "Quad Retopology…" button** (`qml/QuadRetopoDialog.qml`, driven by `QuadRetopoController` singleton). Sentry breadcrumb category `ai.assist.retopo`. Verified on Rumba Dancing.fbx: 10,220 tris → 6,032 faces (4188 quads + 1844 tris), 82% quad dominance. Hard lower bound on face count is ~50% of input (every triangle paired); strict gates typically land 60-70%. - **UvUnwrap** (`src/UvUnwrap.h/cpp`, issue #400): xatlas-backed automatic UV unwrap. xatlas is the MIT library Blender and Godot use under the hood — single-translation-unit `xatlas.cpp` vendored via FetchContent and wrapped in an inline `add_library(xatlas STATIC …)` target (no upstream CMake config). Pipeline: extract (positions, indices) per submesh → `xatlas::AddMesh` → `xatlas::Generate` → for each output mesh, rebuild a single-binding VertexData copying every source attribute from `xref` (input vertex id) and overwriting the target UV channel with `xatlas::Vertex::uv / atlas.{width,height}`. Skinned-mesh bone assignments survive the seam splits because we rebuild `SubMesh::BoneAssignmentList` against the new vertex IDs via xref; for shared-vertex meshes the source assignments come from `Mesh::getBoneAssignments()`, not `SubMesh::getBoneAssignments()`. Surfaced via `qtmesh uv --unwrap`/`--info`, MCP `auto_uv_unwrap`, and the **Material Mode → Mode Tools → "Auto UV Unwrap…" button** (`qml/UvUnwrapDialog.qml`, driven by `UvUnwrapController` singleton). Sentry breadcrumb category `ai.assist.uv_unwrap`. The unwrap also erases `qtme.faces.` n-gon bindings (they reference source vertex IDs and become stale). **GUI-safe entry point** (`unwrapEntityToFile`): live skinned meshes cannot survive in-place vertex-data mutation because the active `Ogre::SkeletonInstance` caches the hardware blend buffer and picks up stale state on the first frame after the swap. The GUI path snapshots `vertexData` / `indexData` / `mBoneAssignments` / `blendIndexToBoneIndexMap` for every submesh + the mesh's shared maps, calls `unwrapEntityKeepingOriginals` (which deliberately leaks its own allocations rather than freeing the originals), exports the unwrapped result, then restores the snapshot pointer-for-pointer (deleting only the unwrap's leaked allocations) and pastes the index maps back directly — `_compileBoneAssignments` is NOT called on restore because it would re-pack BLEND_INDICES/WEIGHTS bytes against the live buffer and shatter the on-screen mesh. CLI path uses the destructive `unwrapEntity` since the process exits before rendering. - **ExportOptimizer** (`src/ExportOptimizer.h/cpp`, issue #399): Pipeline that runs `meshopt_optimizeVertexCache` → `meshopt_optimizeOverdraw` (threshold 1.05) → `meshopt_optimizeVertexFetchRemap` on every submesh of an entity. Surfaced through the **Inspector validation flow** — the "Optimize Geometry (cache + overdraw + fetch)" button in `PropertiesPanel.qml` runs it via `MeshValidator::optimizeVertexCache`. NOT hooked into `MeshImporterExporter::exporter` by default (an earlier draft did this and crashed on macOS during a normal export — silent buffer mutation during export is dangerous; explicit user invocation via the validation button is safer). Vertex-fetch is skipped when the submesh uses `useSharedVertices` since remapping shared verts would scramble other submeshes' indices. `qtmesh info --json` includes `submeshAcmr[]` per submesh so downstream tooling can decide whether to recommend re-optimization. Sentry breadcrumb category `ai.assist.optimize_export`. - **FBX LOD export gotcha**: `FBXExporter` prefers the cached `qtme.faces.` n-gon binding (set up by quad-migration #326) over `SubMesh::indexData`. The CLI `lod` per-LOD export path in `CLIPipeline::cmdLod` temporarily erases those bindings (and restores them after) so the swapped-in LOD indices actually reach the wire. If you add another LOD-export entry point, mirror that erase/restore pair. diff --git a/README.md b/README.md index 6aa0890a7..dbf1d603f 100755 --- a/README.md +++ b/README.md @@ -174,6 +174,11 @@ qtmesh bake-vertex-colors model.fbx -o color_map.png --resolution 1024 --dilatio qtmesh uv model.fbx --unwrap -o unwrapped.glb # overwrite UV0 qtmesh uv model.fbx --unwrap --channel 1 -o lightmap.glb # keep UV0, write UV1 (lightmap workflow) qtmesh uv model.fbx --info --json # report UV channels + coverage + +# Quad retopology (triangle-pairing — no new deps) +qtmesh retopo model.fbx -o quads.glb # pair every viable triangle into quads +qtmesh retopo model.fbx --target-faces 5000 -o lo.glb # stop early once near target face count +qtmesh retopo model.fbx --max-angle 15 -o conservative.glb # tighter coplanarity gate ``` --- diff --git a/qml/PropertiesPanel.qml b/qml/PropertiesPanel.qml index 9eef01fb6..e020e29cb 100644 --- a/qml/PropertiesPanel.qml +++ b/qml/PropertiesPanel.qml @@ -3609,6 +3609,43 @@ Rectangle { } } + // Issue #401: Quad retopology via triangle pairing. + // Operates on the currently selected entity, mutates the + // mesh in place via the qtme.faces. n-gon binding. + Rectangle { + width: Math.min(parent.width - 16, retopoLabel.implicitWidth + 16) + height: 26 + radius: 3 + opacity: QuadRetopoController.hasSelection ? 1.0 : 0.45 + color: retopoMa.containsMouse && QuadRetopoController.hasSelection + ? PropertiesPanelController.highlightColor + : PropertiesPanelController.headerColor + border.color: PropertiesPanelController.borderColor + border.width: 1 + + Text { + id: retopoLabel + anchors.centerIn: parent + text: "Quad Retopology…" + color: PropertiesPanelController.textColor + font.pixelSize: 11 + } + MouseArea { + id: retopoMa + anchors.fill: parent + hoverEnabled: true + enabled: QuadRetopoController.hasSelection + cursorShape: QuadRetopoController.hasSelection + ? Qt.PointingHandCursor : Qt.ForbiddenCursor + onClicked: root.openQuadRetopoDialog() + ToolTip.visible: containsMouse + ToolTip.delay: 500 + ToolTip.text: QuadRetopoController.hasSelection + ? "Pair adjacent triangles into quad-dominant topology. Skin weights survive (no new vertices)." + : "Select a mesh first." + } + } + // Slice I: Material Preview Environment — interactive // preview of the currently-selected material on Sphere/Cube // shapes. Drag horizontally on the thumbnail to rotate the @@ -3844,6 +3881,23 @@ Rectangle { } } + // Issue #401: triangle-pairing quad retopology dialog. Same + // lazy-load idiom as UvUnwrapDialog. + Loader { + id: quadRetopoLoader + active: false + anchors.centerIn: parent + source: "qrc:/MaterialEditorQML/QuadRetopoDialog.qml" + onLoaded: if (item && item.open) item.open() + } + function openQuadRetopoDialog() { + if (!quadRetopoLoader.active) { + quadRetopoLoader.active = true + } else if (quadRetopoLoader.item) { + quadRetopoLoader.item.open() + } + } + // ---- Material Presets Content ---- Component { id: materialPresetsComponent diff --git a/qml/QuadRetopoDialog.qml b/qml/QuadRetopoDialog.qml new file mode 100644 index 000000000..5a4a71c38 --- /dev/null +++ b/qml/QuadRetopoDialog.qml @@ -0,0 +1,270 @@ +import QtQuick +import QtQuick.Controls +import QtQuick.Layouts +import QtQuick.Window +import MaterialEditorQML 1.0 +import PropertiesPanel 1.0 + +// Issue #401: top-level Window for triangle-pairing quad retopology. +// Same Inspector-styled idiom as UvUnwrapDialog (Rectangle + Text + +// MouseArea primitives over PropertiesPanelController.* colors). +// Operates on the currently selected entity — no input-path field. +// The result is committed in place (via the qtme.faces. n-gon +// binding); unlike UV unwrap, no separate export step is needed +// because the triangle index buffer keeps a valid fan-triangulation. +Window { + id: dialog + title: "Quad Retopology" + width: 540 + height: 420 + minimumWidth: 460 + minimumHeight: 380 + flags: Qt.Dialog + modality: Qt.ApplicationModal + color: PropertiesPanelController.panelColor + + // Knobs — defaults match QuadRetopoOptions. + property int targetFaces: -1 + property double maxAngleDeg: 25.0 + property double shapeToleranceDeg: 65.0 + property double maxAspectRatio: 6.0 + + // Last-run result, surfaced as a green status line at the bottom. + property string lastStatus: "" + property bool lastWasError: false + + function open() { + dialog.lastStatus = "" + dialog.lastWasError = false + dialog.show() + dialog.raise() + dialog.requestActivate() + } + + // ── Inline Inspector primitives ───────────────────────────────── + + component InspectorButton: Rectangle { + id: btn + property string label: "" + property bool buttonEnabled: true + signal clicked() + height: 26 + radius: 3 + color: btnMa.containsMouse && buttonEnabled + ? PropertiesPanelController.highlightColor + : PropertiesPanelController.headerColor + border.color: PropertiesPanelController.borderColor + border.width: 1 + opacity: buttonEnabled ? 1.0 : 0.45 + Text { + anchors.centerIn: parent + text: btn.label + color: PropertiesPanelController.textColor + font.pixelSize: 11 + } + MouseArea { + id: btnMa + anchors.fill: parent + hoverEnabled: true + enabled: btn.buttonEnabled + cursorShape: btn.buttonEnabled ? Qt.PointingHandCursor : Qt.ForbiddenCursor + onClicked: btn.clicked() + } + } + + component InspectorLabel: Text { + color: PropertiesPanelController.textColor + font.pixelSize: 11 + } + + component InspectorNumberField: Rectangle { + id: nf + property double value: 0 + property double minValue: 0 + property double maxValue: 1e9 + property bool isInt: false + signal newValue(double v) + height: 24 + color: PropertiesPanelController.inputColor + border.color: ni.activeFocus + ? PropertiesPanelController.highlightColor + : PropertiesPanelController.borderColor + border.width: 1 + radius: 3 + TextInput { + id: ni + anchors.fill: parent + anchors.leftMargin: 6 + anchors.rightMargin: 6 + text: nf.isInt ? Math.round(nf.value).toString() : nf.value.toFixed(2) + color: PropertiesPanelController.textColor + font.pixelSize: 11 + verticalAlignment: TextInput.AlignVCenter + selectByMouse: true + onEditingFinished: { + const n = nf.isInt ? parseInt(text, 10) : parseFloat(text) + if (!isNaN(n) && n >= nf.minValue && n <= nf.maxValue) + nf.newValue(n) + else + text = nf.isInt ? Math.round(nf.value).toString() : nf.value.toFixed(2) + } + } + } + + // ── Layout ─────────────────────────────────────────────────────── + + ColumnLayout { + anchors.fill: parent + anchors.margins: 16 + spacing: 12 + + InspectorLabel { + Layout.fillWidth: true + wrapMode: Text.WordWrap + opacity: 0.85 + text: "Pair adjacent triangles into quads where the merge is " + + "coplanar, near-rectangular, and within the aspect-ratio " + + "gate. The mesh is rewritten in place — quads are kept " + + "via the n-gon binding so the FBX / glTF exporter round-" + + "trips them. No new vertices are introduced; skin " + + "weights and UVs survive unchanged." + } + + // Target faces + RowLayout { + spacing: 8 + Layout.fillWidth: true + InspectorLabel { text: "Target faces:"; Layout.preferredWidth: 110 } + InspectorNumberField { + Layout.preferredWidth: 100 + value: dialog.targetFaces + minValue: -1 + maxValue: 10000000 + isInt: true + onNewValue: dialog.targetFaces = Math.round(v) + } + InspectorLabel { + text: "-1 = pair every viable candidate (~50% reduction max)" + opacity: 0.7 + Layout.fillWidth: true + wrapMode: Text.WordWrap + } + } + + // Max angle + RowLayout { + spacing: 8 + Layout.fillWidth: true + InspectorLabel { text: "Max angle:"; Layout.preferredWidth: 110 } + InspectorNumberField { + Layout.preferredWidth: 100 + value: dialog.maxAngleDeg + minValue: 0 + maxValue: 180 + onNewValue: dialog.maxAngleDeg = v + } + InspectorLabel { + text: "deg between adjacent triangle normals (lower = more curvature-preserving)" + opacity: 0.7 + Layout.fillWidth: true + wrapMode: Text.WordWrap + } + } + + // Shape tolerance + RowLayout { + spacing: 8 + Layout.fillWidth: true + InspectorLabel { text: "Shape tol:"; Layout.preferredWidth: 110 } + InspectorNumberField { + Layout.preferredWidth: 100 + value: dialog.shapeToleranceDeg + minValue: 0 + maxValue: 90 + onNewValue: dialog.shapeToleranceDeg = v + } + InspectorLabel { + text: "deg deviation per interior angle from 90 (lower = stricter quad)" + opacity: 0.7 + Layout.fillWidth: true + wrapMode: Text.WordWrap + } + } + + // Max aspect ratio + RowLayout { + spacing: 8 + Layout.fillWidth: true + InspectorLabel { text: "Max aspect:"; Layout.preferredWidth: 110 } + InspectorNumberField { + Layout.preferredWidth: 100 + value: dialog.maxAspectRatio + minValue: 1 + maxValue: 100 + onNewValue: dialog.maxAspectRatio = v + } + InspectorLabel { + text: "longest edge / shortest edge (lower = more square-like)" + opacity: 0.7 + Layout.fillWidth: true + wrapMode: Text.WordWrap + } + } + + Item { Layout.fillHeight: true } + + // Status line + InspectorLabel { + Layout.fillWidth: true + visible: dialog.lastStatus.length > 0 + text: dialog.lastStatus + wrapMode: Text.WordWrap + color: dialog.lastWasError ? "#cc4444" : "#3a8c3a" + } + + // Buttons + RowLayout { + Layout.fillWidth: true + Item { Layout.fillWidth: true } + InspectorButton { + label: "Close" + Layout.preferredWidth: 90 + onClicked: dialog.close() + } + InspectorButton { + label: QuadRetopoController.busy ? "Retopologizing…" : "Retopologize" + Layout.preferredWidth: 160 + buttonEnabled: !QuadRetopoController.busy + && QuadRetopoController.hasSelection + onClicked: { + const r = QuadRetopoController.retopologizeSelected( + dialog.targetFaces, + dialog.maxAngleDeg, + dialog.shapeToleranceDeg, + dialog.maxAspectRatio) + if (r && r.applied) { + const dom = Math.round((r.quadDominance || 0) * 1000) / 10 + dialog.lastStatus = + "Done: " + r.totalTrianglesBefore + " tris → " + + r.totalFacesAfter + " faces (" + + r.totalQuadsAfter + " quads, " + + r.totalTrianglesAfter + " tris, " + + dom + "% quad dominance)" + dialog.lastWasError = false + } else { + dialog.lastStatus = "Failed: " + (r && r.error ? r.error : "unknown error") + dialog.lastWasError = true + } + } + } + } + } + + Connections { + target: QuadRetopoController + function onError(msg) { + dialog.lastStatus = "Failed: " + msg + dialog.lastWasError = true + } + } +} diff --git a/qml/qmldir b/qml/qmldir index a91bfda51..9566c08eb 100644 --- a/qml/qmldir +++ b/qml/qmldir @@ -8,6 +8,7 @@ NormalMapGeneratorDialog 1.0 NormalMapGeneratorDialog.qml TextureAtlasDialog 1.0 TextureAtlasDialog.qml ApplyAtlasDialog 1.0 ApplyAtlasDialog.qml UvUnwrapDialog 1.0 UvUnwrapDialog.qml +QuadRetopoDialog 1.0 QuadRetopoDialog.qml MaterialListModal 1.0 MaterialListModal.qml ThemedButton 1.0 ThemedButton.qml ThemedCheckBox 1.0 ThemedCheckBox.qml diff --git a/src/CLIPipeline.cpp b/src/CLIPipeline.cpp index fdb1ff337..e3a3516c1 100644 --- a/src/CLIPipeline.cpp +++ b/src/CLIPipeline.cpp @@ -20,6 +20,7 @@ #include "VertexCacheOptimizer.h" #include "ExportOptimizer.h" #include "UvUnwrap.h" +#include "QuadRetopo.h" #include "MeshDecimator.h" #include "EditableMesh.h" #include "TexturePaintBuffer.h" @@ -1257,6 +1258,7 @@ int CLIPipeline::run(int argc, char* argv[]) else if (cmd == "bake-vertex-colors") rc = cmdBakeVertexColors(argc, argv); else if (cmd == "vat") rc = cmdVat(argc, argv); else if (cmd == "uv") rc = cmdUv(argc, argv); + else if (cmd == "retopo") rc = cmdRetopo(argc, argv); else if (cmd == "morph") rc = cmdMorph(argc, argv); else if (cmd == "nodeanim") rc = cmdNodeAnim(argc, argv); @@ -6734,6 +6736,99 @@ int CLIPipeline::cmdUv(int argc, char* argv[]) return 0; } +int CLIPipeline::cmdRetopo(int argc, char* argv[]) +{ + // Parse: retopo [--target-faces N] [--max-angle DEG] + // [--shape-tol DEG] [--max-aspect R] -o [--json] + QString inputPath, outputPath; + bool jsonOutput = false; + int targetFaces = -1; + double maxAngleDeg = 25.0; + double shapeToleranceDeg = 65.0; + double maxAspectRatio = 6.0; + + for (int i = 1; i < argc; ++i) { + const QString arg = QString::fromLocal8Bit(argv[i]); + if (arg == "retopo" || arg == "--cli") continue; + if (arg == "--json") { jsonOutput = true; continue; } + if ((arg == "-o" || arg == "--output") && i + 1 < argc) { + outputPath = QString::fromLocal8Bit(argv[++i]); continue; + } + if (arg == "--target-faces" && i + 1 < argc) { + targetFaces = QString::fromLocal8Bit(argv[++i]).toInt(); continue; + } + if (arg == "--max-angle" && i + 1 < argc) { + maxAngleDeg = QString::fromLocal8Bit(argv[++i]).toDouble(); continue; + } + if (arg == "--shape-tol" && i + 1 < argc) { + shapeToleranceDeg = QString::fromLocal8Bit(argv[++i]).toDouble(); continue; + } + if (arg == "--max-aspect" && i + 1 < argc) { + maxAspectRatio = QString::fromLocal8Bit(argv[++i]).toDouble(); continue; + } + if (!arg.startsWith("-") && inputPath.isEmpty()) { + inputPath = arg; continue; + } + } + + if (inputPath.isEmpty()) { + err() << "Error: No input file specified." << Qt::endl; + err() << "Usage: qtmesh retopo [--target-faces N] " + "[--max-angle DEG] [--shape-tol DEG] [--max-aspect R] -o [--json]" + << Qt::endl; + return 2; + } + if (outputPath.isEmpty()) { + err() << "Error: -o required." << Qt::endl; + return 2; + } + + QFileInfo fi(inputPath); + if (!fi.exists()) { + err() << "Error: file not found: " << inputPath << Qt::endl; return 1; + } + if (!initOgreHeadless()) return 1; + + SentryReporter::addBreadcrumb(QStringLiteral("ai.assist.retopo"), + QString("retopo .%1 target=%2 maxAngle=%3") + .arg(fi.suffix()).arg(targetFaces).arg(maxAngleDeg)); + + MeshImporterExporter::importer({fi.absoluteFilePath()}); + auto& entities = Manager::getSingleton()->getEntities(); + if (entities.isEmpty()) { + err() << "Error: failed to load " << inputPath << Qt::endl; return 1; + } + Ogre::Entity* entity = entities.first(); + + QuadRetopoOptions opts; + opts.targetFaces = targetFaces; + opts.maxAngleDeg = maxAngleDeg; + opts.shapeToleranceDeg = shapeToleranceDeg; + opts.maxAspectRatio = maxAspectRatio; + + const auto report = QuadRetopo::retopologize(entity, opts); + if (!report.applied) { + err() << "Error: retopology failed — " << report.error << Qt::endl; + return 1; + } + + auto* node = entity->getParentSceneNode(); + const QString fmt = formatForExtension(outputPath); + if (MeshImporterExporter::exporter(node, QFileInfo(outputPath).absoluteFilePath(), fmt) != 0) { + err() << "Error: export failed." << Qt::endl; + return 1; + } + + if (jsonOutput) { + cliWrite(QString::fromUtf8( + QJsonDocument(QuadRetopo::reportToJson(report)).toJson(QJsonDocument::Indented)) + "\n"); + } else { + cliWrite(QuadRetopo::reportToText(report) + + QString("Wrote: %1\n").arg(QFileInfo(outputPath).fileName())); + } + return 0; +} + int CLIPipeline::cmdMorph(int argc, char* argv[]) { // Parse: morph --list [--json] diff --git a/src/CLIPipeline.h b/src/CLIPipeline.h index 3231cd1db..fb0bcefb6 100644 --- a/src/CLIPipeline.h +++ b/src/CLIPipeline.h @@ -146,6 +146,13 @@ class CLIPipeline { /// UV channels without mutating. Issue #400. static int cmdUv(int argc, char* argv[]); + /// Quad retopology via triangle pairing. Walks every interior edge + /// whose two adjacent faces are triangles and scores the merge by + /// coplanarity + quad shape + aspect ratio; takes the best pairs + /// greedily. Output is committed via the n-gon binding so quads + /// round-trip through the FBX / glTF exporter. Issue #401. + static int cmdRetopo(int argc, char* argv[]); + /// List the morph targets / blend shapes on a mesh file. Slice A1 /// surfaces a `--list` mode only; subsequent slices add `--set`, /// `--add`, `--delete` once the in-memory authoring path lands. diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 5da491c74..118db7ec7 100755 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -84,6 +84,8 @@ MeshOptimizerLod.cpp ExportOptimizer.cpp UvUnwrap.cpp UvUnwrapController.cpp +QuadRetopo.cpp +QuadRetopoController.cpp TextureChannelPacker.cpp TextureAtlasPacker.cpp PaintBufferImageProvider.cpp @@ -198,6 +200,8 @@ MeshOptimizerLod.h ExportOptimizer.h UvUnwrap.h UvUnwrapController.h +QuadRetopo.h +QuadRetopoController.h TextureChannelPacker.h TextureAtlasPacker.h PaintBufferImageProvider.h diff --git a/src/MCPServer.cpp b/src/MCPServer.cpp index dec0ae209..d396ab1c0 100644 --- a/src/MCPServer.cpp +++ b/src/MCPServer.cpp @@ -27,6 +27,7 @@ #include "VertexCacheOptimizer.h" #include "MeshDecimator.h" #include "UvUnwrap.h" +#include "QuadRetopo.h" #include #include #include @@ -550,6 +551,7 @@ const QMap& MCPServer::toolHandlers() {QStringLiteral("set_texture"), &MCPServer::toolSetTexture}, {QStringLiteral("export_mesh"), &MCPServer::toolExportMesh}, {QStringLiteral("auto_uv_unwrap"), &MCPServer::toolAutoUvUnwrap}, + {QStringLiteral("retopologize"), &MCPServer::toolRetopologize}, {QStringLiteral("get_scene_info"), &MCPServer::toolGetSceneInfo}, {QStringLiteral("take_screenshot"), &MCPServer::toolTakeScreenshot}, {QStringLiteral("create_primitive"), &MCPServer::toolCreatePrimitive}, @@ -1416,6 +1418,49 @@ QJsonObject MCPServer::toolAutoUvUnwrap(const QJsonObject &args) return result; } +QJsonObject MCPServer::toolRetopologize(const QJsonObject &args) +{ + // Issue #401: triangle-pairing quad retopology. Operates on the + // currently selected entity. The mesh is rewritten in place; + // exporters round-trip the new quads via the qtme.faces. + // n-gon binding. + if (!hasSelectedEntities()) + return makeErrorResult("No mesh selected. Load a mesh first with load_mesh."); + + QuadRetopoOptions opts; + if (args.contains("target_faces")) opts.targetFaces = args["target_faces"].toInt(-1); + if (args.contains("max_angle_deg")) opts.maxAngleDeg = args["max_angle_deg"].toDouble(25.0); + if (args.contains("shape_tol_deg")) opts.shapeToleranceDeg = args["shape_tol_deg"].toDouble(65.0); + if (args.contains("max_aspect_ratio"))opts.maxAspectRatio = args["max_aspect_ratio"].toDouble(6.0); + + SelectionSet* sel = SelectionSet::getSingleton(); + if (!sel || sel->getEntitiesCount() == 0) + return makeErrorResult("No selected entity."); + Ogre::Entity* entity = sel->getEntity(0); + if (!entity) return makeErrorResult("Selected entity is null."); + + SentryReporter::addBreadcrumb(QStringLiteral("ai.assist.retopo"), + QStringLiteral("retopologize entity=%1 target=%2 maxAngle=%3") + .arg(QString::fromStdString(entity->getName())) + .arg(opts.targetFaces).arg(opts.maxAngleDeg)); + + QuadRetopoReport report; + try { + report = QuadRetopo::retopologize(entity, opts); + } catch (const Ogre::Exception& e) { + return makeErrorResult(QStringLiteral("Ogre error: %1") + .arg(QString::fromStdString(e.getFullDescription()))); + } + + if (!report.applied) { + return makeErrorResult(QStringLiteral("Quad retopology failed: %1").arg(report.error)); + } + + QJsonObject result = makeSuccessResult(QuadRetopo::reportToText(report)); + result["retopo"] = QuadRetopo::reportToJson(report); + return result; +} + QJsonObject MCPServer::toolGetSceneInfo(const QJsonObject &args) { Q_UNUSED(args); @@ -5480,6 +5525,36 @@ QJsonArray MCPServer::buildToolsList() ); } + // retopologize + { + QJsonObject props; + props["target_faces"] = QJsonObject{{"type", "integer"}, + {"description", + "Target face count. Triangle-pairing has a hard lower bound of ~50% of the " + "input triangle count (every triangle paired). Set to -1 (default) to pair " + "every viable candidate."}}; + props["max_angle_deg"] = QJsonObject{{"type", "number"}, + {"description", + "Maximum angle (degrees) between two adjacent triangle normals for them to " + "be considered for pairing. Lower = more curvature-preserving. Default 25."}}; + props["shape_tol_deg"] = QJsonObject{{"type", "number"}, + {"description", + "Maximum deviation (degrees) of each interior quad angle from 90. Default 65."}}; + props["max_aspect_ratio"] = QJsonObject{{"type", "number"}, + {"description", + "Maximum aspect ratio (longest edge / shortest edge) of an accepted quad. " + "Default 6."}}; + appendTool( + "retopologize", + "Quad-dominant retopology of the selected mesh via triangle pairing. " + "Walks every interior edge whose two adjacent faces are triangles and scores " + "the merge by coplanarity + quad shape + aspect ratio; takes the best pairs " + "greedily. Output faces are committed via the qtme.faces. n-gon binding so " + "FBX and glTF exporters round-trip the new quads. Issue #401.", + props + ); + } + // decimate_mesh { QJsonObject props; diff --git a/src/MCPServer.h b/src/MCPServer.h index b516067a2..dd6d206f2 100644 --- a/src/MCPServer.h +++ b/src/MCPServer.h @@ -145,6 +145,11 @@ private slots: /// chosen channel (default 0). Skin weights survive the seam /// splits via xref remap. QJsonObject toolAutoUvUnwrap(const QJsonObject &args); + /// Issue #401: triangle-pairing quad retopology on the + /// currently selected entity. Pairs adjacent triangles into + /// convex quads when coplanarity + shape + aspect ratio gates + /// pass. Output is committed via the n-gon binding. + QJsonObject toolRetopologize(const QJsonObject &args); QJsonObject toolGetSceneInfo(const QJsonObject &args); QJsonObject toolTakeScreenshot(const QJsonObject &args); QJsonObject toolCreatePrimitive(const QJsonObject &args); diff --git a/src/QuadRetopo.cpp b/src/QuadRetopo.cpp new file mode 100644 index 000000000..ac98a43e4 --- /dev/null +++ b/src/QuadRetopo.cpp @@ -0,0 +1,496 @@ +#include "QuadRetopo.h" +#include "EditableMesh.h" + +#include +#include +#include +#include +#include + +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +// Forward declarations from EditableMesh.cpp; the existing +// implementation already exposes these for use by other slices +// (decimation, UV unwrap, etc). +void writeNgonFacesToMesh(Ogre::Mesh* mesh, + const std::vector& subMeshes); + +namespace { + +// ─── Geometry helpers ─────────────────────────────────────────────────────── + +struct Vec3 { + double x = 0, y = 0, z = 0; + + static Vec3 from(const float* p, unsigned int idx) { + const float* v = p + idx * 3; + return { v[0], v[1], v[2] }; + } + + Vec3 operator-(const Vec3& o) const { return { x - o.x, y - o.y, z - o.z }; } + Vec3 operator+(const Vec3& o) const { return { x + o.x, y + o.y, z + o.z }; } + double dot(const Vec3& o) const { return x * o.x + y * o.y + z * o.z; } + Vec3 cross(const Vec3& o) const { + return { y * o.z - z * o.y, z * o.x - x * o.z, x * o.y - y * o.x }; + } + double lengthSq() const { return x * x + y * y + z * z; } + double length() const { return std::sqrt(lengthSq()); } + Vec3 normalized() const { + const double L = length(); + return L > 1e-12 ? Vec3{ x / L, y / L, z / L } : Vec3{}; + } +}; + +double triNormal(const float* positions, + unsigned int a, unsigned int b, unsigned int c, + Vec3& outNormal) +{ + const Vec3 pa = Vec3::from(positions, a); + const Vec3 pb = Vec3::from(positions, b); + const Vec3 pc = Vec3::from(positions, c); + const Vec3 e1 = pb - pa; + const Vec3 e2 = pc - pa; + const Vec3 n = e1.cross(e2); + const double L = n.length(); + outNormal = L > 1e-12 ? Vec3{ n.x / L, n.y / L, n.z / L } : Vec3{}; + return L; // 2 * triangle area +} + +// Return the interior angle at vertex `b` of the polygon edge a→b→c, +// in degrees. Always in (0, 180]. +double interiorAngleDeg(const Vec3& a, const Vec3& b, const Vec3& c) +{ + const Vec3 ba = (a - b).normalized(); + const Vec3 bc = (c - b).normalized(); + const double d = std::clamp(ba.dot(bc), -1.0, 1.0); + return std::acos(d) * 180.0 / M_PI; +} + +// ─── Edge → adjacent triangle lookup ──────────────────────────────────────── + +// Undirected edge key (smaller index first). +struct EdgeKey { + unsigned int a, b; + + static EdgeKey make(unsigned int u, unsigned int v) { + return u < v ? EdgeKey{ u, v } : EdgeKey{ v, u }; + } + + bool operator==(const EdgeKey& o) const { return a == o.a && b == o.b; } +}; + +struct EdgeKeyHash { + size_t operator()(const EdgeKey& e) const noexcept { + return std::hash{}((uint64_t(e.a) << 32) | uint64_t(e.b)); + } +}; + +// ─── Candidate quad scoring ───────────────────────────────────────────────── + +struct CandidatePair { + int triA = -1; // First triangle index (into `triangleCount` triangles) + int triB = -1; // Second triangle index + double score = 0.0; // Higher = better merge candidate + + // The 4 vertex indices of the resulting quad, in winding order. + unsigned int quad[4] = { 0, 0, 0, 0 }; + + bool operator<(const CandidatePair& o) const { return score > o.score; } +}; + +// Given two triangles sharing the edge (sharedA, sharedB), find the +// non-shared vertex of each triangle and emit the quad winding so the +// quad goes (opposingA, sharedA, opposingB, sharedB) — i.e. opposite +// corners of the diagonal we just removed. +bool buildQuadWinding(const unsigned int* tri0, + const unsigned int* tri1, + unsigned int sharedA, unsigned int sharedB, + unsigned int outQuad[4]) +{ + unsigned int opposing0 = ~0u, opposing1 = ~0u; + for (int i = 0; i < 3; ++i) { + if (tri0[i] != sharedA && tri0[i] != sharedB) opposing0 = tri0[i]; + if (tri1[i] != sharedA && tri1[i] != sharedB) opposing1 = tri1[i]; + } + if (opposing0 == ~0u || opposing1 == ~0u) return false; + outQuad[0] = opposing0; + outQuad[1] = sharedA; + outQuad[2] = opposing1; + outQuad[3] = sharedB; + return true; +} + +// Score a candidate quad. Returns < 0 if the merge should be rejected +// outright (non-coplanar, concave, or out-of-tolerance shape). +double scoreCandidate(const float* positions, + const unsigned int quad[4], + const Vec3& n0, // first triangle's normal + const Vec3& n1, // second triangle's normal + const QuadRetopoOptions& opts) +{ + // 1. Coplanarity check via normal angle. + const double cosNormals = std::clamp(n0.dot(n1), -1.0, 1.0); + const double angleDeg = std::acos(cosNormals) * 180.0 / M_PI; + if (angleDeg > opts.maxAngleDeg) return -1.0; + + const Vec3 p0 = Vec3::from(positions, quad[0]); + const Vec3 p1 = Vec3::from(positions, quad[1]); + const Vec3 p2 = Vec3::from(positions, quad[2]); + const Vec3 p3 = Vec3::from(positions, quad[3]); + + // 2. Interior angles must be in [90 - tol, 90 + tol]. + const double a0 = interiorAngleDeg(p3, p0, p1); + const double a1 = interiorAngleDeg(p0, p1, p2); + const double a2 = interiorAngleDeg(p1, p2, p3); + const double a3 = interiorAngleDeg(p2, p3, p0); + const double tol = opts.shapeToleranceDeg; + if (a0 < 90 - tol || a0 > 90 + tol) return -1.0; + if (a1 < 90 - tol || a1 > 90 + tol) return -1.0; + if (a2 < 90 - tol || a2 > 90 + tol) return -1.0; + if (a3 < 90 - tol || a3 > 90 + tol) return -1.0; + // Convexity check: interior angles of a convex quad sum to 360° + // and each is < 180°. The tolerance guard above already ensures + // each is < 90 + tol; with the default tol=65 (worst case 155°) + // this still rejects non-convex quads. + if (a0 + a1 + a2 + a3 > 360.5) return -1.0; + + // 3. Aspect ratio. + const double e01 = (p1 - p0).length(); + const double e12 = (p2 - p1).length(); + const double e23 = (p3 - p2).length(); + const double e30 = (p0 - p3).length(); + const double longest = std::max({ e01, e12, e23, e30 }); + const double shortest = std::min({ e01, e12, e23, e30 }); + if (shortest < 1e-9) return -1.0; + const double aspect = longest / shortest; + if (aspect > opts.maxAspectRatio) return -1.0; + + // Composite score: prefer near-coplanar (high cos), near-square + // (low angle deviation), and low aspect ratio. We sum normalized + // contributions in [0, 1] so the maximum score is 3. + const double coplanarityScore = cosNormals; // [-1, 1] + const double angleDevScore = 1.0 - (std::abs(a0 - 90) + + std::abs(a1 - 90) + + std::abs(a2 - 90) + + std::abs(a3 - 90)) / (4 * 90); + const double aspectScore = 1.0 / aspect; // (0, 1] + + return coplanarityScore + angleDevScore + aspectScore; +} + +// ─── Per-submesh pairing ───────────────────────────────────────────────────── + +void retopologizeSubmesh(EditableSubMesh& sub, + const QuadRetopoOptions& opts, + int submeshIndex, + QuadRetopoSubmeshReport& report, + int& globalRemainingTargetCount) +{ + report.submeshIndex = submeshIndex; + report.trianglesBefore = static_cast(sub.triangles.size()); + + if (sub.triangles.empty() || sub.vertices.size() < 3) { + report.facesAfter = static_cast(sub.triangles.size()); + report.trianglesAfter = static_cast(sub.triangles.size()); + return; + } + + // Flatten positions for QuadRetopo::retopologizeMesh. + std::vector positions(sub.vertices.size() * 3); + for (size_t i = 0; i < sub.vertices.size(); ++i) { + positions[3 * i + 0] = sub.vertices[i].position.x; + positions[3 * i + 1] = sub.vertices[i].position.y; + positions[3 * i + 2] = sub.vertices[i].position.z; + } + + std::vector indices; + indices.reserve(sub.triangles.size() * 3); + for (const auto& t : sub.triangles) { + indices.push_back(t.indices[0]); + indices.push_back(t.indices[1]); + indices.push_back(t.indices[2]); + } + + // Run the pure-data pairing. + std::vector> faces; + QuadRetopoOptions perSub = opts; + if (opts.targetFaces > 0) + perSub.targetFaces = globalRemainingTargetCount; + + QuadRetopo::retopologizeMesh(positions.data(), + static_cast(sub.vertices.size()), + indices.data(), + static_cast(sub.triangles.size()), + perSub, faces); + + // Decrement the global target by what we used here. The retopo + // returned `faces.size()` faces; if the caller asked for a budget, + // we've consumed (originalTris - facesNow) units of pair budget + // and can stop pairing in later submeshes if we've hit the target. + if (opts.targetFaces > 0) { + const int reductionHere = report.trianglesBefore - static_cast(faces.size()); + globalRemainingTargetCount = std::max(0, + globalRemainingTargetCount - reductionHere); + } + + // Translate the faces back into EditableSubMesh::faces. + sub.faces.clear(); + sub.faces.reserve(faces.size()); + for (const auto& f : faces) { + EditableFace ef; + ef.indices = f; + if (ef.isValid()) + sub.faces.push_back(std::move(ef)); + } + + // Rebuild triangles so the GPU has a valid index buffer. + triangulateFaces(sub); + + report.facesAfter = static_cast(sub.faces.size()); + report.quadsAfter = 0; + report.trianglesAfter = 0; + for (const auto& f : sub.faces) { + if (f.indices.size() == 4) ++report.quadsAfter; + else if (f.indices.size() == 3) ++report.trianglesAfter; + } +} + +} // namespace + +// ─── Public API ──────────────────────────────────────────────────────────── + +QuadRetopoReport QuadRetopo::retopologizeMesh(const float* positions, + int vertexCount, + const unsigned int* indices, + int triangleCount, + const QuadRetopoOptions& opts, + std::vector>& outFaces) +{ + QuadRetopoReport report; + outFaces.clear(); + + if (!positions || !indices || vertexCount < 3 || triangleCount < 1) { + report.error = QStringLiteral("empty or malformed input"); + return report; + } + + report.totalTrianglesBefore = triangleCount; + + // 1. Compute per-triangle normals and area. + std::vector triNormals(triangleCount); + for (int t = 0; t < triangleCount; ++t) { + const unsigned int* tri = indices + 3 * t; + triNormal(positions, tri[0], tri[1], tri[2], triNormals[t]); + } + + // 2. Build the edge → adjacent-triangles lookup. Each interior + // edge appears in exactly two triangles; boundary edges appear + // in one and are skipped. + struct EdgeAdjacency { + int t0 = -1; + int t1 = -1; + }; + std::unordered_map edgeMap; + edgeMap.reserve(triangleCount * 3); + for (int t = 0; t < triangleCount; ++t) { + const unsigned int* tri = indices + 3 * t; + for (int e = 0; e < 3; ++e) { + const EdgeKey key = EdgeKey::make(tri[e], tri[(e + 1) % 3]); + auto& adj = edgeMap[key]; + if (adj.t0 < 0) adj.t0 = t; + else if (adj.t1 < 0) adj.t1 = t; + // Edges shared by >2 triangles (non-manifold) — keep first 2. + } + } + + // 3. Score every interior edge as a candidate quad merge. + std::vector candidates; + candidates.reserve(edgeMap.size()); + for (const auto& [key, adj] : edgeMap) { + if (adj.t0 < 0 || adj.t1 < 0) continue; // boundary + const unsigned int* tri0 = indices + 3 * adj.t0; + const unsigned int* tri1 = indices + 3 * adj.t1; + CandidatePair c; + c.triA = adj.t0; + c.triB = adj.t1; + if (!buildQuadWinding(tri0, tri1, key.a, key.b, c.quad)) continue; + c.score = scoreCandidate(positions, c.quad, + triNormals[adj.t0], triNormals[adj.t1], opts); + if (c.score < 0) continue; + candidates.push_back(c); + } + std::sort(candidates.begin(), candidates.end()); + + // 4. Greedy pair-up: claim each triangle at most once. Stop early + // if the caller specified a target face count and we've reached it. + std::vector claimed(triangleCount, 0); + std::vector> pairs; // (triA, triB) → produces a quad + int facesNow = triangleCount; + for (const auto& c : candidates) { + if (opts.targetFaces > 0 && facesNow <= opts.targetFaces) break; + if (claimed[c.triA] || claimed[c.triB]) continue; + claimed[c.triA] = 1; + claimed[c.triB] = 1; + pairs.emplace_back(c.triA, c.triB); + --facesNow; // 2 tris → 1 quad + } + + // 5. Emit faces. Walk the candidate list again in best-score + // order so the quad windings match what scoreCandidate validated. + outFaces.reserve(triangleCount - pairs.size()); + std::vector emitted(triangleCount, 0); + for (const auto& c : candidates) { + if (!claimed[c.triA] || !claimed[c.triB]) continue; + if (emitted[c.triA] || emitted[c.triB]) continue; + // Check this is one of the *winning* pairs (not just a + // candidate that happens to involve already-claimed tris). + bool isPair = false; + for (const auto& p : pairs) { + if ((p.first == c.triA && p.second == c.triB) || + (p.first == c.triB && p.second == c.triA)) { + isPair = true; + break; + } + } + if (!isPair) continue; + outFaces.push_back({ c.quad[0], c.quad[1], c.quad[2], c.quad[3] }); + emitted[c.triA] = 1; + emitted[c.triB] = 1; + } + + // 6. Emit unpaired triangles. + for (int t = 0; t < triangleCount; ++t) { + if (claimed[t]) continue; + const unsigned int* tri = indices + 3 * t; + outFaces.push_back({ tri[0], tri[1], tri[2] }); + } + + report.totalFacesAfter = static_cast(outFaces.size()); + for (const auto& f : outFaces) { + if (f.size() == 4) ++report.totalQuadsAfter; + else if (f.size() == 3) ++report.totalTrianglesAfterRetopo; + } + report.applied = true; + return report; +} + +QuadRetopoReport QuadRetopo::retopologize(Ogre::Entity* entity, + const QuadRetopoOptions& opts, + Algorithm algo) +{ + QuadRetopoReport report; + if (algo != Algorithm::TrianglePair) { + report.error = QStringLiteral("only TrianglePair backend is implemented"); + return report; + } + if (!entity || !entity->getMesh()) { + report.error = QStringLiteral("null entity / no mesh"); + return report; + } + + Ogre::MeshPtr mesh = entity->getMesh(); + report.meshName = QString::fromStdString(mesh->getName()); + + EditableMesh em; + if (!em.loadFromEntity(entity)) { + report.error = QStringLiteral("EditableMesh load failed"); + return report; + } + + int remainingTarget = opts.targetFaces; + auto& subs = em.subMeshes(); + + for (size_t si = 0; si < subs.size(); ++si) { + QuadRetopoSubmeshReport sub; + retopologizeSubmesh(subs[si], opts, + static_cast(si), + sub, remainingTarget); + report.submeshes.push_back(sub); + report.totalTrianglesBefore += sub.trianglesBefore; + report.totalFacesAfter += sub.facesAfter; + report.totalQuadsAfter += sub.quadsAfter; + report.totalTrianglesAfterRetopo += sub.trianglesAfter; + } + + // Commit the edits back to the live Ogre::Mesh. + if (!em.commitToEntity(entity)) { + report.error = QStringLiteral("commitToEntity failed"); + return report; + } + + // Write the n-gon binding so exporters and Edit Mode see the new + // quad topology rather than the fan-triangulated tris. + writeNgonFacesToMesh(mesh.get(), em.subMeshes()); + + report.applied = true; + return report; +} + +QJsonObject QuadRetopo::reportToJson(const QuadRetopoReport& report) +{ + QJsonObject root; + root["meshName"] = report.meshName; + root["applied"] = report.applied; + root["totalTrianglesBefore"] = report.totalTrianglesBefore; + root["totalFacesAfter"] = report.totalFacesAfter; + root["totalQuadsAfter"] = report.totalQuadsAfter; + root["totalTrianglesAfterRetopo"] = report.totalTrianglesAfterRetopo; + root["quadDominance"] = report.quadDominance(); + if (!report.error.isEmpty()) root["error"] = report.error; + + QJsonArray subs; + for (const auto& s : report.submeshes) { + QJsonObject obj; + obj["submeshIndex"] = s.submeshIndex; + obj["trianglesBefore"] = s.trianglesBefore; + obj["facesAfter"] = s.facesAfter; + obj["quadsAfter"] = s.quadsAfter; + obj["trianglesAfter"] = s.trianglesAfter; + subs.push_back(obj); + } + root["submeshes"] = subs; + return root; +} + +QString QuadRetopo::reportToText(const QuadRetopoReport& report) +{ + QString out; + QTextStream s(&out); + s << "Quad Retopology\n"; + s << "===============\n"; + s << "Mesh: " << report.meshName << "\n"; + s << "Submeshes: " << report.submeshes.size() << "\n"; + s << "Triangles in: " << report.totalTrianglesBefore << "\n"; + s << "Faces out: " << report.totalFacesAfter + << " (" << report.totalQuadsAfter << " quads, " + << report.totalTrianglesAfterRetopo << " triangles)\n"; + s << "Quad dominance: " + << QString::number(report.quadDominance() * 100.0, 'f', 1) << "%\n"; + if (!report.error.isEmpty()) s << "Error: " << report.error << "\n"; + return out; +} + +QString QuadRetopo::algorithmToString(Algorithm algo) +{ + switch (algo) { + case Algorithm::TrianglePair: return QStringLiteral("pair-tris"); + } + return QStringLiteral("pair-tris"); +} + +QuadRetopo::Algorithm QuadRetopo::algorithmFromString(const QString& s) +{ + const QString lc = s.toLower(); + if (lc == "pair-tris" || lc == "pair") return Algorithm::TrianglePair; + return Algorithm::TrianglePair; +} diff --git a/src/QuadRetopo.h b/src/QuadRetopo.h new file mode 100644 index 000000000..68be235a8 --- /dev/null +++ b/src/QuadRetopo.h @@ -0,0 +1,152 @@ +#ifndef QUAD_RETOPO_H +#define QUAD_RETOPO_H + +#include +#include +#include +#include + +namespace Ogre { + class Entity; + class Mesh; +} + +// Quad-dominant retopology (issue #401, epic #397). +// +// The issue title proposes Instant Meshes; the reality is that Instant +// Meshes ships as a research GUI app with no clean C++ library API and +// has been dormant since 2016. Its production-grade successor, +// QuadriFlow, requires Boost + Eigen + LEMON — heavyweight deps the +// project doesn't currently use. +// +// This first slice ships a native triangle-pairing backend with zero +// new dependencies. The pipeline walks every interior edge whose two +// adjacent faces are triangles and scores the merge: +// 1. Coplanarity (dot product of triangle normals) +// 2. Quad shape (deviation of interior angles from 90°) +// 3. Aspect ratio (longest-to-shortest edge of the resulting quad) +// 4. Convexity (the merge is rejected if the resulting quad is concave) +// Pairs are taken greedily in best-score-first order, each triangle +// claimed at most once. Output faces are written to +// `EditableSubMesh::faces` as 4-vert quads (paired) or 3-vert tris +// (unpaired), then committed via the existing n-gon binding +// (`qtme.faces.`) so the FBX / glTF exporters round-trip quads +// cleanly and Edit Mode sees the new topology as quads. +// +// Future backends can plug in behind the `Algorithm` enum without +// breaking the API surface — mirroring `MeshDecimator::Algorithm` +// and `MeshLodController::Algorithm`. The CLI / MCP / GUI already +// expose `--algo` for those tools. +// +// Limitations of the triangle-pairing approach: +// - Not field-aligned. Instant Meshes / QuadriFlow trace quad strips +// along principal curvature directions; this just pairs adjacent +// triangles based on local geometry. +// - Cannot reach `--target-faces` exactly; the lower bound is +// ~50% of the input triangle count (every tri paired). With +// pickier merge scoring it lands closer to 60-70%. +// - UVs and skin weights are preserved per-vertex (triangle-pairing +// never introduces new vertices), which is the main practical +// advantage over field-aligned methods. + +struct QuadRetopoOptions { + // Target face count after retopology. <= 0 means "pair every + // mergeable pair" (typically ~50% reduction). Otherwise we stop + // taking pairs once we hit the target. + int targetFaces = -1; + + // Maximum angle (in degrees) between two adjacent triangle normals + // for them to be considered for pairing. Lower = more conservative + // (stricter coplanarity required); higher = more aggressive. + // 30° preserves curvature features well; 90° pairs almost + // everything. Default 25°. + double maxAngleDeg = 25.0; + + // Minimum allowed deviation from a perfect square. Each interior + // angle of the candidate quad must be within + // [90 - shapeToleranceDeg, 90 + shapeToleranceDeg] degrees. + // 60° accepts most reasonable quads; 30° is strict (near-square). + // Default 65°. + double shapeToleranceDeg = 65.0; + + // Maximum aspect ratio (longest edge / shortest edge) of the + // resulting quad. >= 1.0. 4.0 is permissive; 2.0 is strict. + // Default 6.0 — most character meshes benefit from accepting + // some elongated quads along limbs. + double maxAspectRatio = 6.0; +}; + +struct QuadRetopoSubmeshReport { + int submeshIndex = 0; + int trianglesBefore = 0; + int facesAfter = 0; // triangles + quads after retopology + int quadsAfter = 0; + int trianglesAfter = 0; +}; + +struct QuadRetopoReport { + QString meshName; + QList submeshes; + int totalTrianglesBefore = 0; + int totalFacesAfter = 0; + int totalQuadsAfter = 0; + int totalTrianglesAfterRetopo = 0; + bool applied = false; + QString error; + + /// Fraction of input triangles that got paired into quads. + /// 0.0 = no pairs found (mesh is preserved as-is). + /// 1.0 = every triangle paired (face count halved). + double quadDominance() const + { + return totalTrianglesBefore > 0 + ? static_cast(totalQuadsAfter * 2) / totalTrianglesBefore + : 0.0; + } +}; + +class QuadRetopo { +public: + enum class Algorithm { + TrianglePair, // Default — pure-data triangle pairing into quads. + // Future: + // QuadriFlow — field-aligned via QuadriFlow library + // InstantMeshes — if/when a clean library extraction exists + }; + + // Apply quad retopology to `entity` in place. The base mesh is + // rewritten: triangle pairs become quads on `EditableSubMesh:: + // faces`, the triangle list is rebuilt by fan-triangulating the + // n-gon list, and the `qtme.faces.` binding is updated so + // exporters and Edit Mode see the new topology. + // + // `algo` selects the backend (only `TrianglePair` is implemented + // in this slice). + static QuadRetopoReport retopologize(Ogre::Entity* entity, + const QuadRetopoOptions& opts = {}, + Algorithm algo = Algorithm::TrianglePair); + + // Pure-data variant: in-memory triangle list → quads-plus-tris + // face list. Used by the Ogre-backed `retopologize` and by tests + // that don't want an Ogre context. Positions are passed as a + // flat float array (xyz xyz ...) of length 3 * vertexCount. + // + // The output `outFaces` is one inner vector per face; each inner + // vector holds 3 (triangle) or 4 (quad) source-vertex indices. + // Vertex IDs in the output reference the same `positions` array + // unchanged — triangle pairing never introduces new vertices. + static QuadRetopoReport retopologizeMesh(const float* positions, + int vertexCount, + const unsigned int* indices, + int triangleCount, + const QuadRetopoOptions& opts, + std::vector>& outFaces); + + static QJsonObject reportToJson(const QuadRetopoReport& report); + static QString reportToText(const QuadRetopoReport& report); + + static QString algorithmToString(Algorithm algo); + static Algorithm algorithmFromString(const QString& s); +}; + +#endif // QUAD_RETOPO_H diff --git a/src/QuadRetopoController.cpp b/src/QuadRetopoController.cpp new file mode 100644 index 000000000..360ce917c --- /dev/null +++ b/src/QuadRetopoController.cpp @@ -0,0 +1,111 @@ +#include "QuadRetopoController.h" +#include "QuadRetopo.h" +#include "SelectionSet.h" +#include "SentryReporter.h" + +#include +#include + +QuadRetopoController* QuadRetopoController::m_pSingleton = nullptr; + +QuadRetopoController* QuadRetopoController::instance() +{ + if (!m_pSingleton) + m_pSingleton = new QuadRetopoController(); + return m_pSingleton; +} + +QuadRetopoController* QuadRetopoController::qmlInstance(QQmlEngine* engine, QJSEngine*) +{ + Q_UNUSED(engine); + auto* inst = instance(); + QQmlEngine::setObjectOwnership(inst, QQmlEngine::CppOwnership); + return inst; +} + +void QuadRetopoController::kill() +{ + delete m_pSingleton; + m_pSingleton = nullptr; +} + +QuadRetopoController::QuadRetopoController() : QObject(nullptr) +{ + connect(SelectionSet::getSingleton(), &SelectionSet::selectionChanged, + this, &QuadRetopoController::selectionChanged); +} + +bool QuadRetopoController::hasSelection() const +{ + auto* sel = SelectionSet::getSingleton(); + if (!sel) return false; + return !sel->getResolvedEntities().isEmpty(); +} + +QVariantMap QuadRetopoController::retopologizeSelected(int targetFaces, + double maxAngleDeg, + double shapeToleranceDeg, + double maxAspectRatio) +{ + QVariantMap result; + + auto* sel = SelectionSet::getSingleton(); + if (!sel) { + emit error(QStringLiteral("No selection set.")); + result["applied"] = false; + return result; + } + const auto entities = sel->getResolvedEntities(); + if (entities.isEmpty()) { + emit error(QStringLiteral("No mesh selected.")); + result["applied"] = false; + return result; + } + Ogre::Entity* entity = entities.first(); + + QuadRetopoOptions opts; + opts.targetFaces = targetFaces; + opts.maxAngleDeg = maxAngleDeg; + opts.shapeToleranceDeg = shapeToleranceDeg; + opts.maxAspectRatio = maxAspectRatio; + + SentryReporter::addBreadcrumb(QStringLiteral("ai.assist.retopo"), + QString("UI retopo entity=%1 target=%2 maxAngle=%3 shape=%4 aspect=%5") + .arg(QString::fromStdString(entity->getName())) + .arg(targetFaces) + .arg(maxAngleDeg).arg(shapeToleranceDeg).arg(maxAspectRatio)); + + m_busy = true; + emit busyChanged(); + + QuadRetopoReport report; + try { + report = QuadRetopo::retopologize(entity, opts); + } catch (const Ogre::Exception& e) { + m_busy = false; + emit busyChanged(); + emit error(QString("Ogre error: %1").arg(QString::fromStdString(e.getFullDescription()))); + result["applied"] = false; + result["error"] = QString::fromStdString(e.getFullDescription()); + return result; + } + + m_busy = false; + emit busyChanged(); + + result["applied"] = report.applied; + result["meshName"] = report.meshName; + result["totalTrianglesBefore"] = report.totalTrianglesBefore; + result["totalFacesAfter"] = report.totalFacesAfter; + result["totalQuadsAfter"] = report.totalQuadsAfter; + result["totalTrianglesAfter"] = report.totalTrianglesAfterRetopo; + result["quadDominance"] = report.quadDominance(); + if (!report.error.isEmpty()) result["error"] = report.error; + + if (report.applied) emit retopoApplied(result); + else emit error(report.error.isEmpty() + ? QStringLiteral("Quad retopology failed") + : report.error); + + return result; +} diff --git a/src/QuadRetopoController.h b/src/QuadRetopoController.h new file mode 100644 index 000000000..8ef8c1962 --- /dev/null +++ b/src/QuadRetopoController.h @@ -0,0 +1,60 @@ +#ifndef QUAD_RETOPO_CONTROLLER_H +#define QUAD_RETOPO_CONTROLLER_H + +#include +#include +#include + +// QML-facing singleton for the triangle-pairing quad retopology +// (issue #401). Wraps `QuadRetopo::retopologize` plus selection-state +// property so the Inspector button can disable itself when no entity +// is selected. Headless callers (CLI / MCP) use `QuadRetopo` directly. +class QuadRetopoController : public QObject +{ + Q_OBJECT + QML_ELEMENT + QML_SINGLETON + + Q_PROPERTY(bool hasSelection READ hasSelection NOTIFY selectionChanged) + Q_PROPERTY(bool busy READ busy NOTIFY busyChanged) + +public: + static QuadRetopoController* instance(); + static QuadRetopoController* qmlInstance(QQmlEngine* engine, QJSEngine* scriptEngine); + static void kill(); + + bool hasSelection() const; + bool busy() const { return m_busy; } + + // Run the retopology on the first resolved selected entity. The + // entity's mesh is rewritten in place: triangle pairs become quads + // and the `qtme.faces.` binding is updated so exporters round- + // trip the new topology. Unlike `unwrapEntityToFile`, this does + // NOT require an export — the in-place mutation is safe because + // the n-gon binding leaves the triangle index buffer alone (just + // gets fan-retriangulated by `triangulateFaces`). + // + // Returns a QVariantMap mirroring `QuadRetopoReport::*` so the + // dialog can surface "N quads, M tris, 60% quad dominance" after + // Apply. Emits `retopoApplied(report)` on success or `error(msg)` + // on failure. + Q_INVOKABLE QVariantMap retopologizeSelected(int targetFaces, + double maxAngleDeg, + double shapeToleranceDeg, + double maxAspectRatio); + +signals: + void selectionChanged(); + void busyChanged(); + void retopoApplied(const QVariantMap& report); + void error(const QString& message); + +private: + QuadRetopoController(); + ~QuadRetopoController() override = default; + + static QuadRetopoController* m_pSingleton; + bool m_busy = false; +}; + +#endif // QUAD_RETOPO_CONTROLLER_H diff --git a/src/QuadRetopo_test.cpp b/src/QuadRetopo_test.cpp new file mode 100644 index 000000000..8b86dc048 --- /dev/null +++ b/src/QuadRetopo_test.cpp @@ -0,0 +1,176 @@ +#include "QuadRetopo.h" + +#include + +#include + +// Unit tests for the triangle-pairing quad retopology (issue #401). +// The Ogre-backed entry point `retopologize(Ogre::Entity*, ...)` is +// covered by integration runs against real mesh files; the headless +// CI builds skip it via `tryInitOgre()`. The pure-data +// `retopologizeMesh(positions, indices, ...)` overload IS exercised +// here against synthetic input — it has no Ogre dependency. + +namespace { + +// Two coplanar right triangles sharing the diagonal (0,2): +// +// v3 ─── v2 +// │ ╲ │ +// │ ╲ │ +// v0 ─── v1 +// +// Should pair into one quad (v3, v0, v2, v1) — opposing-corner +// quad winding emitted by buildQuadWinding when the shared edge +// is v0→v2. +std::vector kSquarePositions = { + 0.0f, 0.0f, 0.0f, // v0 + 1.0f, 0.0f, 0.0f, // v1 + 1.0f, 1.0f, 0.0f, // v2 + 0.0f, 1.0f, 0.0f, // v3 +}; +std::vector kSquareTriangles = { + 0, 1, 2, // tri A + 0, 2, 3, // tri B +}; + +} // namespace + +TEST(QuadRetopoTest, PairsTwoCoplanarRightTrianglesIntoOneQuad) +{ + QuadRetopoOptions opts; + std::vector> faces; + auto report = QuadRetopo::retopologizeMesh( + kSquarePositions.data(), 4, + kSquareTriangles.data(), 2, + opts, faces); + + ASSERT_TRUE(report.applied); + EXPECT_EQ(report.totalTrianglesBefore, 2); + EXPECT_EQ(report.totalFacesAfter, 1); + EXPECT_EQ(report.totalQuadsAfter, 1); + EXPECT_EQ(report.totalTrianglesAfterRetopo, 0); + ASSERT_EQ(faces.size(), 1u); + EXPECT_EQ(faces[0].size(), 4u); +} + +TEST(QuadRetopoTest, RejectsNonCoplanarTrianglesByDefault) +{ + // Bend tri B upward: v3 goes from z=0 to z=1, making the + // dihedral angle 45°. Default maxAngleDeg=25° rejects this. + std::vector pos = kSquarePositions; + pos[3 * 3 + 2] = 1.0f; // v3.z = 1 + + QuadRetopoOptions opts; // defaults + std::vector> faces; + auto report = QuadRetopo::retopologizeMesh( + pos.data(), 4, + kSquareTriangles.data(), 2, + opts, faces); + + ASSERT_TRUE(report.applied); + EXPECT_EQ(report.totalQuadsAfter, 0); + EXPECT_EQ(report.totalTrianglesAfterRetopo, 2); // both kept as tris +} + +TEST(QuadRetopoTest, AcceptsBentTrianglesWhenMaxAngleIsRelaxed) +{ + std::vector pos = kSquarePositions; + pos[3 * 3 + 2] = 1.0f; // ~45° dihedral + + QuadRetopoOptions opts; + opts.maxAngleDeg = 90.0; // accept anything up to 90° + opts.shapeToleranceDeg = 90; // and any quasi-square shape + + std::vector> faces; + auto report = QuadRetopo::retopologizeMesh( + pos.data(), 4, + kSquareTriangles.data(), 2, + opts, faces); + + ASSERT_TRUE(report.applied); + EXPECT_EQ(report.totalQuadsAfter, 1); +} + +TEST(QuadRetopoTest, EmptyInputReturnsErrorReport) +{ + QuadRetopoOptions opts; + std::vector> faces; + auto report = QuadRetopo::retopologizeMesh( + nullptr, 0, nullptr, 0, opts, faces); + EXPECT_FALSE(report.applied); + EXPECT_FALSE(report.error.isEmpty()); + EXPECT_TRUE(faces.empty()); +} + +TEST(QuadRetopoTest, AspectRatioGateRejectsElongatedQuads) +{ + // Stretch v1 way out to the right — the resulting quad has + // aspect ratio 10:1. Default opts.maxAspectRatio=6.0 rejects. + std::vector pos = kSquarePositions; + pos[3 * 1 + 0] = 10.0f; + pos[3 * 2 + 0] = 10.0f; + + QuadRetopoOptions opts; + std::vector> faces; + auto report = QuadRetopo::retopologizeMesh( + pos.data(), 4, kSquareTriangles.data(), 2, opts, faces); + ASSERT_TRUE(report.applied); + EXPECT_EQ(report.totalQuadsAfter, 0); +} + +TEST(QuadRetopoTest, TargetFacesStopsPairingEarly) +{ + // Two unit squares side by side (8 tris total). With + // target_faces=8 we want no pairing; with target_faces=6 we + // want exactly one pair (2 tris → 1 quad, 8→7). + std::vector pos = { + 0,0,0, 1,0,0, 2,0,0, + 0,1,0, 1,1,0, 2,1,0, + }; + std::vector tris = { + 0,1,4, 0,4,3, + 1,2,5, 1,5,4, + }; // 4 tris + + QuadRetopoOptions opts; + opts.targetFaces = 4; // no pairing wanted + + std::vector> faces; + auto report = QuadRetopo::retopologizeMesh( + pos.data(), 6, tris.data(), 4, opts, faces); + ASSERT_TRUE(report.applied); + EXPECT_EQ(report.totalFacesAfter, 4); + EXPECT_EQ(report.totalQuadsAfter, 0); + EXPECT_EQ(report.totalTrianglesAfterRetopo, 4); +} + +TEST(QuadRetopoTest, AlgorithmStringRoundTrip) +{ + EXPECT_EQ(QuadRetopo::algorithmToString(QuadRetopo::Algorithm::TrianglePair), + QStringLiteral("pair-tris")); + EXPECT_EQ(QuadRetopo::algorithmFromString("pair-tris"), + QuadRetopo::Algorithm::TrianglePair); + EXPECT_EQ(QuadRetopo::algorithmFromString("pair"), + QuadRetopo::Algorithm::TrianglePair); + EXPECT_EQ(QuadRetopo::algorithmFromString("unknown-fallback"), + QuadRetopo::Algorithm::TrianglePair); // safe default +} + +TEST(QuadRetopoTest, ReportToJsonRoundTrip) +{ + QuadRetopoReport report; + report.meshName = QStringLiteral("test"); + report.totalTrianglesBefore = 100; + report.totalFacesAfter = 60; + report.totalQuadsAfter = 40; + report.totalTrianglesAfterRetopo = 20; + report.applied = true; + + auto json = QuadRetopo::reportToJson(report); + EXPECT_EQ(json["meshName"].toString(), QStringLiteral("test")); + EXPECT_EQ(json["totalTrianglesBefore"].toInt(), 100); + EXPECT_EQ(json["totalQuadsAfter"].toInt(), 40); + EXPECT_TRUE(json["applied"].toBool()); + EXPECT_NEAR(json["quadDominance"].toDouble(), 0.8, 1e-6); +} diff --git a/src/main.cpp b/src/main.cpp index e983d8047..a6d78be7a 100755 --- a/src/main.cpp +++ b/src/main.cpp @@ -92,7 +92,8 @@ int main(int argc, char *argv[]) || arg == "analyze" || arg == "vertex-cache" || arg == "decimate" || arg == "atlas" || arg == "atlas-apply" || arg == "optimize" || arg == "bake-vertex-colors" - || arg == "vat" || arg == "uv" || arg == "morph" || arg == "nodeanim") + || arg == "vat" || arg == "uv" || arg == "retopo" + || arg == "morph" || arg == "nodeanim") cliMode = true; break; // first non-flag arg determines mode } diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 80eb3121f..394581a73 100755 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -75,6 +75,7 @@ #include "MeshDecimatorController.h" #include "MeshValidator.h" #include "UvUnwrapController.h" +#include "QuadRetopoController.h" #include "MaterialPresetLibrary.h" #include "MaterialPreviewRenderer.h" #include "AIChatManager.h" @@ -378,6 +379,7 @@ MainWindow::~MainWindow() CurveEditModel::kill(); MeshLodController::kill(); UvUnwrapController::kill(); + QuadRetopoController::kill(); MeshValidator::kill(); MaterialPresetLibrary::kill(); MaterialPreviewRenderer::kill(); @@ -541,6 +543,11 @@ void MainWindow::initToolBar() [](QQmlEngine* engine, QJSEngine*) -> QObject* { return UvUnwrapController::qmlInstance(engine, nullptr); }); + qmlRegisterSingletonType( + "PropertiesPanel", 1, 0, "QuadRetopoController", + [](QQmlEngine* engine, QJSEngine*) -> QObject* { + return QuadRetopoController::qmlInstance(engine, nullptr); + }); // Open the LOD export directory picker from MainWindow so the dialog has a // proper parent widget — QFileDialog invoked from a QML context doesn't // reliably appear on macOS without a valid parent QWidget. diff --git a/src/qml_resources.qrc b/src/qml_resources.qrc index ee5289928..3b7608580 100644 --- a/src/qml_resources.qrc +++ b/src/qml_resources.qrc @@ -10,6 +10,7 @@ ../qml/TextureAtlasDialog.qml ../qml/ApplyAtlasDialog.qml ../qml/UvUnwrapDialog.qml + ../qml/QuadRetopoDialog.qml ../qml/qmldir ../qml/ThemedButton.qml ../qml/ThemedCheckBox.qml diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index b49798146..a74e5e63f 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -115,6 +115,8 @@ if(BUILD_TESTS) ${CMAKE_CURRENT_SOURCE_DIR}/../src/MeshDecimatorController.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../src/MeshLodController.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../src/UvUnwrapController.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/../src/QuadRetopo.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/../src/QuadRetopoController.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../src/MeshOptimizerLod.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../src/ExportOptimizer.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../src/UvUnwrap.cpp From f1ed49861a158148b5ea60168d7c01a56f988c3f Mon Sep 17 00:00:00 2001 From: Fernando Date: Fri, 29 May 2026 03:41:47 -0400 Subject: [PATCH 2/4] fix(retopo): address Codex + CodeRabbit review on PR #697 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Batch fix of issues raised by automated review on the initial commit. Mix of critical correctness bugs (flipped winding, broken target- faces budgeting) and defensive hardening (input validation, CLI discoverability). ## Codex P1 — Preserve source triangle winding (src/QuadRetopo.cpp) `buildQuadWinding` used the sorted endpoints of `EdgeKey` (which puts `min(u,v)` first regardless of the source triangles' winding direction) when emitting the quad winding. For normally-wound adjacent triangles like `[0,1,2]` and `[0,2,3]`, that produced quad `[1,0,3,2]`; the subsequent fan-triangulation in `triangulateFaces` then emitted `[1,0,3]` and `[1,3,2]`, both with normals opposite the source triangles. Every retopologized quad rendered with inverted normals — broken backface culling and lighting. Fix: detect tri0's winding direction across the shared edge by checking whether it contains the directed edge `e0 → e1` (rather than just the undirected pair). If yes, emit `[opposing0, e0, opposing1, e1]`; otherwise emit `[opposing0, e1, opposing1, e0]`. The chosen winding always matches the source triangles, so fan- triangulation produces normals consistent with the input. Regression test added in `QuadRetopo_test.cpp` (`EmittedQuadPreservesSourceTriangleWinding`). ## Codex P2 — Per-submesh target-faces budgeting (src/QuadRetopo.cpp) `retopologizeSubmesh` passed the *global* remaining target face count straight to `retopologizeMesh.opts.targetFaces`. But `retopologizeMesh` interprets `targetFaces` as a per-submesh hard limit on face count: a submesh with 200 tris and a global target of 600 would see `facesNow(200) <= 600` immediately and exit without pairing anything. Common multi-submesh assets (Mixamo characters, anything imported with material-split submeshes) silently ignored `--target-faces`. Fix: redefine the "global" counter as a *reduction budget* (number of pair operations remaining across all submeshes), seeded from `(totalTris - opts.targetFaces)`. Each submesh converts the remaining budget into its own per-submesh face limit `max(ceil(tris/2), tris - allowedReduction)` and decrements the global counter by the reduction it actually achieved. Sentinel `-1` means unlimited (caller didn't specify `targetFaces`). Verified against Rumba Dancing.fbx (11 submeshes): * `--target-faces 8000`: 10220 → 8000 faces (was: ignored, would return ~6032 like the unconstrained run) * `--target-faces 6000`: 10220 → 6032 (clamped at the natural ~50% reduction floor) * Unconstrained: 10220 → 6032, unchanged from before. ## CodeRabbit fixes * **CLI: add `retopo` to `--help`** (`src/CLIPipeline.cpp`). The subcommand was dispatched but undocumented. * **CLI: validate numeric flags** (`src/CLIPipeline.cpp`). Previously `--target-faces foo` silently became `0` and changed the meaning of the operation. Now rejected with a clear usage error. Bounds-check each flag: target-faces > 0, max-angle ∈ [0,180], shape-tol ∈ [0,90], max-aspect ≥ 1. * **CLI: reject multi-entity inputs fail-fast** (`src/CLIPipeline.cpp`). Matches the existing `cmdDecimate()` convention. * **MCP: validate option ranges** (`src/MCPServer.cpp`). Same bound rules as the CLI surface; returns a structured usage error rather than silently producing a no-op. * **MCP: use `getResolvedEntities()`** (`src/MCPServer.cpp`). The block was using `getEntitiesCount()/getEntity(0)`, which would reject valid node-level or sub-entity selections that `hasSelectedEntities()` already accepted. * **Controller: always populate `error` field on failure** (`src/QuadRetopoController.cpp`). `QuadRetopoDialog.qml` reads `r.error` when `applied=false`; previous version emitted the signal but left the map without an error key. * **M_PI portability** (`src/QuadRetopo.cpp`). `M_PI` is not standard C++; not defined under MSVC without `_USE_MATH_DEFINES`. Use `Ogre::Math::PI` which the project already depends on. * **O(n²) emit pass** (`src/QuadRetopo.cpp`). Step 5 used to re- walk every candidate and linear-scan `pairs` to identify the winning merges. Replaced with a `WinningPair` struct that stores the validated quad winding directly when each pair is claimed, so the emit pass is O(pairs.size()). ## Deliberately NOT addressed in this PR * **CodeRabbit: keyboard accessibility of `InspectorButton`** — every Inspector-styled dialog in the project uses the same mouse-only `Rectangle` + `Text` + `MouseArea` composite. Adding Accessibility to just this dialog would be inconsistent; retrofitting all of them is a larger refactor outside this slice. * **CodeRabbit: move retopology off the GUI thread** — flagged as "heavy lift" by the reviewer itself. The sibling UV-unwrap (#400) runs synchronously too; the epic's parent issue (#397) mentions an `AIAssistManager` worker thread as the home for cross-feature threading. That's the right place for this work. * **CodeRabbit: change Sentry breadcrumb category** — `ai.assist.retopo` is the conventional category for this epic (matches `ai.assist.uv_unwrap`, `ai.assist.lod`, `ai.assist.decimate`). See `CLAUDE.md` for the convention. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/CLIPipeline.cpp | 45 ++++++++++- src/MCPServer.cpp | 21 ++++- src/QuadRetopo.cpp | 148 ++++++++++++++++++++++++----------- src/QuadRetopoController.cpp | 8 +- src/QuadRetopo_test.cpp | 41 ++++++++++ 5 files changed, 210 insertions(+), 53 deletions(-) diff --git a/src/CLIPipeline.cpp b/src/CLIPipeline.cpp index e3a3516c1..ceb24f711 100644 --- a/src/CLIPipeline.cpp +++ b/src/CLIPipeline.cpp @@ -716,6 +716,12 @@ void CLIPipeline::printUsage() " seam-split remap.\n" " uv --info [--json] Report current UV channels + UV0 bounding-box coverage per\n" " submesh without mutating the mesh.\n" + " retopo [--target-faces N] [--max-angle DEG] [--shape-tol DEG] [--max-aspect R] -o [--json]\n" + " Quad-dominant retopology via triangle pairing. Pairs adjacent\n" + " triangles into convex quads where coplanarity + shape + aspect-ratio\n" + " gates pass. Writes quads via the n-gon binding so the FBX / glTF\n" + " exporter round-trips them. No new vertices are introduced — UVs\n" + " and skin weights survive unchanged.\n" " morph --list [--json] List morph targets / blend shapes on a mesh. (Set/add/delete\n" " land in follow-up slices once authoring is in place.)\n" " nodeanim --list [--json] List node-animation clips on a scene (props, doors, machinery,\n" @@ -6755,16 +6761,40 @@ int CLIPipeline::cmdRetopo(int argc, char* argv[]) outputPath = QString::fromLocal8Bit(argv[++i]); continue; } if (arg == "--target-faces" && i + 1 < argc) { - targetFaces = QString::fromLocal8Bit(argv[++i]).toInt(); continue; + bool ok = false; + const int v = QString::fromLocal8Bit(argv[++i]).toInt(&ok); + if (!ok || v <= 0) { + err() << "Error: --target-faces must be a positive integer." << Qt::endl; + return 2; + } + targetFaces = v; continue; } if (arg == "--max-angle" && i + 1 < argc) { - maxAngleDeg = QString::fromLocal8Bit(argv[++i]).toDouble(); continue; + bool ok = false; + const double v = QString::fromLocal8Bit(argv[++i]).toDouble(&ok); + if (!ok || v < 0.0 || v > 180.0) { + err() << "Error: --max-angle must be a number in [0, 180]." << Qt::endl; + return 2; + } + maxAngleDeg = v; continue; } if (arg == "--shape-tol" && i + 1 < argc) { - shapeToleranceDeg = QString::fromLocal8Bit(argv[++i]).toDouble(); continue; + bool ok = false; + const double v = QString::fromLocal8Bit(argv[++i]).toDouble(&ok); + if (!ok || v < 0.0 || v > 90.0) { + err() << "Error: --shape-tol must be a number in [0, 90]." << Qt::endl; + return 2; + } + shapeToleranceDeg = v; continue; } if (arg == "--max-aspect" && i + 1 < argc) { - maxAspectRatio = QString::fromLocal8Bit(argv[++i]).toDouble(); continue; + bool ok = false; + const double v = QString::fromLocal8Bit(argv[++i]).toDouble(&ok); + if (!ok || v < 1.0) { + err() << "Error: --max-aspect must be a number >= 1." << Qt::endl; + return 2; + } + maxAspectRatio = v; continue; } if (!arg.startsWith("-") && inputPath.isEmpty()) { inputPath = arg; continue; @@ -6798,6 +6828,13 @@ int CLIPipeline::cmdRetopo(int argc, char* argv[]) if (entities.isEmpty()) { err() << "Error: failed to load " << inputPath << Qt::endl; return 1; } + if (entities.size() > 1) { + err() << "Error: " << inputPath + << " contains multiple mesh entities. `qtmesh retopo` " + "currently supports one entity per file." + << Qt::endl; + return 1; + } Ogre::Entity* entity = entities.first(); QuadRetopoOptions opts; diff --git a/src/MCPServer.cpp b/src/MCPServer.cpp index d396ab1c0..6dd3a3b28 100644 --- a/src/MCPServer.cpp +++ b/src/MCPServer.cpp @@ -1433,10 +1433,27 @@ QJsonObject MCPServer::toolRetopologize(const QJsonObject &args) if (args.contains("shape_tol_deg")) opts.shapeToleranceDeg = args["shape_tol_deg"].toDouble(65.0); if (args.contains("max_aspect_ratio"))opts.maxAspectRatio = args["max_aspect_ratio"].toDouble(6.0); + // Validate option ranges up front so caller bugs surface as + // clear usage errors rather than silent no-ops / confusing + // output from the algorithm. + if (opts.targetFaces != -1 && opts.targetFaces <= 0) + return makeErrorResult("Error: 'target_faces' must be -1 (unlimited) or a positive integer."); + if (opts.maxAngleDeg < 0.0 || opts.maxAngleDeg > 180.0) + return makeErrorResult("Error: 'max_angle_deg' must be in [0, 180]."); + if (opts.shapeToleranceDeg < 0.0 || opts.shapeToleranceDeg > 90.0) + return makeErrorResult("Error: 'shape_tol_deg' must be in [0, 90]."); + if (opts.maxAspectRatio < 1.0) + return makeErrorResult("Error: 'max_aspect_ratio' must be >= 1."); + + // Use the resolved selection (matches `hasSelectedEntities()` + // above) so valid node / sub-entity selections aren't rejected + // by the raw-entity-count path. SelectionSet* sel = SelectionSet::getSingleton(); - if (!sel || sel->getEntitiesCount() == 0) + const QList resolved = sel ? sel->getResolvedEntities() + : QList{}; + if (resolved.isEmpty()) return makeErrorResult("No selected entity."); - Ogre::Entity* entity = sel->getEntity(0); + Ogre::Entity* entity = resolved.first(); if (!entity) return makeErrorResult("Selected entity is null."); SentryReporter::addBreadcrumb(QStringLiteral("ai.assist.retopo"), diff --git a/src/QuadRetopo.cpp b/src/QuadRetopo.cpp index ac98a43e4..36ae2858b 100644 --- a/src/QuadRetopo.cpp +++ b/src/QuadRetopo.cpp @@ -73,7 +73,7 @@ double interiorAngleDeg(const Vec3& a, const Vec3& b, const Vec3& c) const Vec3 ba = (a - b).normalized(); const Vec3 bc = (c - b).normalized(); const double d = std::clamp(ba.dot(bc), -1.0, 1.0); - return std::acos(d) * 180.0 / M_PI; + return std::acos(d) * 180.0 / Ogre::Math::PI; } // ─── Edge → adjacent triangle lookup ──────────────────────────────────────── @@ -108,25 +108,57 @@ struct CandidatePair { bool operator<(const CandidatePair& o) const { return score > o.score; } }; -// Given two triangles sharing the edge (sharedA, sharedB), find the -// non-shared vertex of each triangle and emit the quad winding so the -// quad goes (opposingA, sharedA, opposingB, sharedB) — i.e. opposite -// corners of the diagonal we just removed. +// Given two triangles sharing an edge whose endpoints are `e0` and +// `e1` (undirected — `EdgeKey::make` returns `(min, max)`), find the +// non-shared "opposing" vertex of each triangle and emit a quad +// winding that preserves the source triangles' winding orientation. +// +// Convention: walk `tri0` in its own (CCW) order. If it goes through +// the shared edge in the direction `e0 → e1`, then walking the quad +// `[opposing0, e0, opposing1, e1]` (with `tri1` providing +// `opposing1`) winds CCW. Otherwise the directed edge is `e1 → e0` +// and the correct winding is `[opposing0, e1, opposing1, e0]`. +// +// This matters because the n-gon fan-triangulation in +// `triangulateFaces` builds `[v0, v_i, v_{i+1}]`; if the winding is +// flipped the resulting triangle normals are opposite the source +// triangles' normals — every retopologized quad would render with +// inverted normals (broken backface culling + lighting). Codex +// review caught this on the merged commit; see GitHub PR #697. bool buildQuadWinding(const unsigned int* tri0, const unsigned int* tri1, - unsigned int sharedA, unsigned int sharedB, + unsigned int e0, unsigned int e1, unsigned int outQuad[4]) { unsigned int opposing0 = ~0u, opposing1 = ~0u; for (int i = 0; i < 3; ++i) { - if (tri0[i] != sharedA && tri0[i] != sharedB) opposing0 = tri0[i]; - if (tri1[i] != sharedA && tri1[i] != sharedB) opposing1 = tri1[i]; + if (tri0[i] != e0 && tri0[i] != e1) opposing0 = tri0[i]; + if (tri1[i] != e0 && tri1[i] != e1) opposing1 = tri1[i]; } if (opposing0 == ~0u || opposing1 == ~0u) return false; + + // Determine tri0's direction over the shared edge. If `tri0` + // contains the directed edge `e0 → e1` (i.e. `e1` immediately + // follows `e0` in winding order), then the quad winding starting + // from `opposing0` should pass through `e0` first, then + // `opposing1`, then `e1`. + bool sharedGoesE0toE1 = false; + for (int i = 0; i < 3; ++i) { + if (tri0[i] == e0 && tri0[(i + 1) % 3] == e1) { + sharedGoesE0toE1 = true; + break; + } + } outQuad[0] = opposing0; - outQuad[1] = sharedA; - outQuad[2] = opposing1; - outQuad[3] = sharedB; + if (sharedGoesE0toE1) { + outQuad[1] = e0; + outQuad[2] = opposing1; + outQuad[3] = e1; + } else { + outQuad[1] = e1; + outQuad[2] = opposing1; + outQuad[3] = e0; + } return true; } @@ -140,7 +172,7 @@ double scoreCandidate(const float* positions, { // 1. Coplanarity check via normal angle. const double cosNormals = std::clamp(n0.dot(n1), -1.0, 1.0); - const double angleDeg = std::acos(cosNormals) * 180.0 / M_PI; + const double angleDeg = std::acos(cosNormals) * 180.0 / Ogre::Math::PI; if (angleDeg > opts.maxAngleDeg) return -1.0; const Vec3 p0 = Vec3::from(positions, quad[0]); @@ -221,11 +253,28 @@ void retopologizeSubmesh(EditableSubMesh& sub, indices.push_back(t.indices[2]); } - // Run the pure-data pairing. + // Run the pure-data pairing. `globalRemainingTargetCount` is the + // remaining *reduction budget* (number of pair operations we can + // still spend across all submeshes), or -1 when unlimited. + // + // Convert that into a per-submesh `targetFaces` for the + // `retopologizeMesh` pure-data call. With unlimited budget we + // pass through `opts.targetFaces` unchanged (it'll be -1 too, + // signalling "no limit"). With a constrained budget, the + // per-submesh limit is `trianglesBefore - allowedReduction`, + // floored at `ceil(trianglesBefore / 2)` (every tri paired). std::vector> faces; QuadRetopoOptions perSub = opts; - if (opts.targetFaces > 0) - perSub.targetFaces = globalRemainingTargetCount; + if (globalRemainingTargetCount >= 0) { + const int floorFaces = (report.trianglesBefore + 1) / 2; + const int desiredFaces = report.trianglesBefore - globalRemainingTargetCount; + perSub.targetFaces = std::max(floorFaces, desiredFaces); + } else { + // Unlimited budget — the caller didn't set a target. + // `retopologizeMesh` treats `targetFaces <= 0` as "pair + // every viable candidate", which is what we want here. + perSub.targetFaces = -1; + } QuadRetopo::retopologizeMesh(positions.data(), static_cast(sub.vertices.size()), @@ -233,11 +282,10 @@ void retopologizeSubmesh(EditableSubMesh& sub, static_cast(sub.triangles.size()), perSub, faces); - // Decrement the global target by what we used here. The retopo - // returned `faces.size()` faces; if the caller asked for a budget, - // we've consumed (originalTris - facesNow) units of pair budget - // and can stop pairing in later submeshes if we've hit the target. - if (opts.targetFaces > 0) { + // Decrement the global reduction budget by what we used. Each + // pair op reduces face count by 1, so `(trianglesBefore - + // facesNow)` units were consumed. + if (globalRemainingTargetCount >= 0) { const int reductionHere = report.trianglesBefore - static_cast(faces.size()); globalRemainingTargetCount = std::max(0, globalRemainingTargetCount - reductionHere); @@ -332,41 +380,39 @@ QuadRetopoReport QuadRetopo::retopologizeMesh(const float* positions, std::sort(candidates.begin(), candidates.end()); // 4. Greedy pair-up: claim each triangle at most once. Stop early - // if the caller specified a target face count and we've reached it. + // if the caller specified a target face count and we've reached + // it. Each winning pair stores its already-validated quad winding + // directly so the emit pass below is O(pairs.size()), not + // O(candidates × pairs). std::vector claimed(triangleCount, 0); - std::vector> pairs; // (triA, triB) → produces a quad + struct WinningPair { + int triA, triB; + unsigned int quad[4]; + }; + std::vector pairs; + pairs.reserve(candidates.size()); int facesNow = triangleCount; for (const auto& c : candidates) { if (opts.targetFaces > 0 && facesNow <= opts.targetFaces) break; if (claimed[c.triA] || claimed[c.triB]) continue; claimed[c.triA] = 1; claimed[c.triB] = 1; - pairs.emplace_back(c.triA, c.triB); + WinningPair wp; + wp.triA = c.triA; + wp.triB = c.triB; + wp.quad[0] = c.quad[0]; + wp.quad[1] = c.quad[1]; + wp.quad[2] = c.quad[2]; + wp.quad[3] = c.quad[3]; + pairs.push_back(wp); --facesNow; // 2 tris → 1 quad } - // 5. Emit faces. Walk the candidate list again in best-score - // order so the quad windings match what scoreCandidate validated. + // 5. Emit winning quads (in score order — preserved by the order + // we accepted them above). outFaces.reserve(triangleCount - pairs.size()); - std::vector emitted(triangleCount, 0); - for (const auto& c : candidates) { - if (!claimed[c.triA] || !claimed[c.triB]) continue; - if (emitted[c.triA] || emitted[c.triB]) continue; - // Check this is one of the *winning* pairs (not just a - // candidate that happens to involve already-claimed tris). - bool isPair = false; - for (const auto& p : pairs) { - if ((p.first == c.triA && p.second == c.triB) || - (p.first == c.triB && p.second == c.triA)) { - isPair = true; - break; - } - } - if (!isPair) continue; - outFaces.push_back({ c.quad[0], c.quad[1], c.quad[2], c.quad[3] }); - emitted[c.triA] = 1; - emitted[c.triB] = 1; - } + for (const auto& p : pairs) + outFaces.push_back({ p.quad[0], p.quad[1], p.quad[2], p.quad[3] }); // 6. Emit unpaired triangles. for (int t = 0; t < triangleCount; ++t) { @@ -407,9 +453,21 @@ QuadRetopoReport QuadRetopo::retopologize(Ogre::Entity* entity, return report; } - int remainingTarget = opts.targetFaces; auto& subs = em.subMeshes(); + // `opts.targetFaces` is a *total* (across-all-submeshes) target. + // Convert it into a global reduction budget: each pair op drops + // the total face count by one (2 tris → 1 quad), so we have + // (totalTris - targetFaces) pair operations to spend across all + // submeshes. `retopologizeSubmesh` consumes from this counter and + // also writes back the per-submesh face limit it actually used. + int totalTris = 0; + for (const auto& s : subs) + totalTris += static_cast(s.triangles.size()); + int remainingTarget = (opts.targetFaces > 0) + ? std::max(0, totalTris - opts.targetFaces) + : -1; // -1 = unlimited budget (signalled by `opts.targetFaces <= 0`) + for (size_t si = 0; si < subs.size(); ++si) { QuadRetopoSubmeshReport sub; retopologizeSubmesh(subs[si], opts, diff --git a/src/QuadRetopoController.cpp b/src/QuadRetopoController.cpp index 360ce917c..389795c93 100644 --- a/src/QuadRetopoController.cpp +++ b/src/QuadRetopoController.cpp @@ -51,14 +51,18 @@ QVariantMap QuadRetopoController::retopologizeSelected(int targetFaces, auto* sel = SelectionSet::getSingleton(); if (!sel) { - emit error(QStringLiteral("No selection set.")); + const auto msg = QStringLiteral("No selection set."); + emit error(msg); result["applied"] = false; + result["error"] = msg; return result; } const auto entities = sel->getResolvedEntities(); if (entities.isEmpty()) { - emit error(QStringLiteral("No mesh selected.")); + const auto msg = QStringLiteral("No mesh selected."); + emit error(msg); result["applied"] = false; + result["error"] = msg; return result; } Ogre::Entity* entity = entities.first(); diff --git a/src/QuadRetopo_test.cpp b/src/QuadRetopo_test.cpp index 8b86dc048..380bb68c5 100644 --- a/src/QuadRetopo_test.cpp +++ b/src/QuadRetopo_test.cpp @@ -2,6 +2,7 @@ #include +#include #include // Unit tests for the triangle-pairing quad retopology (issue #401). @@ -54,6 +55,46 @@ TEST(QuadRetopoTest, PairsTwoCoplanarRightTrianglesIntoOneQuad) EXPECT_EQ(faces[0].size(), 4u); } +TEST(QuadRetopoTest, EmittedQuadPreservesSourceTriangleWinding) +{ + // Regression test for Codex P1 review on PR #697: the original + // implementation used sorted edge endpoints `EdgeKey::make` + // when emitting quad windings, which inverted the quad's + // orientation for normally-wound input. Verify the emitted + // quad winds CCW when both source tris wind CCW. + // + // Source tris (both CCW from +Z view): + // v0(0,0) - v1(1,0) - v2(1,1) and + // v0(0,0) - v2(1,1) - v3(0,1) + // Cross-product of any two consecutive emitted-quad edges + // should give +Z (positive Z component). + QuadRetopoOptions opts; + std::vector> faces; + auto report = QuadRetopo::retopologizeMesh( + kSquarePositions.data(), 4, + kSquareTriangles.data(), 2, + opts, faces); + ASSERT_TRUE(report.applied); + ASSERT_EQ(faces.size(), 1u); + ASSERT_EQ(faces[0].size(), 4u); + + // For the emitted quad [a, b, c, d] (vertex indices), compute + // the cross product (b - a) x (c - a). Its z component should + // be positive for CCW winding when viewed from +Z. + const float* p = kSquarePositions.data(); + auto vert = [&](unsigned int idx) { + return std::array{ p[3 * idx + 0], p[3 * idx + 1], p[3 * idx + 2] }; + }; + const auto a = vert(faces[0][0]); + const auto b = vert(faces[0][1]); + const auto c = vert(faces[0][2]); + const float bax = b[0] - a[0], bay = b[1] - a[1]; + const float cax = c[0] - a[0], cay = c[1] - a[1]; + const float crossZ = bax * cay - bay * cax; + EXPECT_GT(crossZ, 0.0f) + << "Emitted quad winding is flipped relative to source triangles."; +} + TEST(QuadRetopoTest, RejectsNonCoplanarTrianglesByDefault) { // Bend tri B upward: v3 goes from z=0 to z=1, making the From 06f84d5c92b490f32edb79626861cd0692a95383 Mon Sep 17 00:00:00 2001 From: Fernando Date: Fri, 29 May 2026 04:37:08 -0400 Subject: [PATCH 3/4] fix(retopo): simplify algorithmFromString to satisfy SonarCloud MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SonarCloud flagged `cpp:S3923` ("Remove this conditional structure or edit its code blocks so that they're not all the same") at `algorithmFromString` on PR #697. Both branches returned `Algorithm::TrianglePair` because only one backend is implemented today. Replace the `if (recognized) ... else ...` with an unconditional return. The recognized-string set widens here naturally when a second backend (QuadriFlow / InstantMeshes) lands — at that point the conditional structure becomes meaningful again. The behavioural contract is unchanged: any input still maps to `Algorithm::TrianglePair`. Existing unit tests pass without modification. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/QuadRetopo.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/QuadRetopo.cpp b/src/QuadRetopo.cpp index 36ae2858b..a96be2eb0 100644 --- a/src/QuadRetopo.cpp +++ b/src/QuadRetopo.cpp @@ -548,7 +548,14 @@ QString QuadRetopo::algorithmToString(Algorithm algo) QuadRetopo::Algorithm QuadRetopo::algorithmFromString(const QString& s) { - const QString lc = s.toLower(); - if (lc == "pair-tris" || lc == "pair") return Algorithm::TrianglePair; + // Only `TrianglePair` is implemented today; the unrecognized- + // string path is intentionally a safe fallback to the same + // default rather than throwing or signalling an error. When a + // second backend lands (QuadriFlow / InstantMeshes) the + // recognized-string set widens here and the no-op fallback + // keeps its behaviour. The conditional structure was flagged + // by SonarCloud (cpp:S3923) for having identical branches — + // simplify to a single recognized-set check + fallback. + Q_UNUSED(s); return Algorithm::TrianglePair; } From 2e3a84ee6d40135826592e85c1b3a3b2b64493f6 Mon Sep 17 00:00:00 2001 From: Fernando Date: Fri, 29 May 2026 22:02:57 -0400 Subject: [PATCH 4/4] feat(retopo): move button to Edit Mode + keyboard accessibility MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ups to address user feedback + a CodeRabbit review item that I had previously declined on PR #697. ## Move "Quad Retopology…" button to Edit Mode Tools User feedback: "it is not mainly related to materials, it is more about the mesh." Retopology is a topology operation — pairs of triangles becoming quads in the n-gon binding. Lives logically alongside the other Edit Mode topology tools (vertex/edge/face selection, soft selection, normals, wireframe overlay, and the existing mesh-validation block). Moved the button from Material Mode → Mode Tools (which mainly hosts texture / atlas / channel-pack tools) to Edit Mode Tools, above the mesh-validation separator. Edit Mode is the right container because (a) the operation only makes sense on a mesh selection, (b) Edit Mode users are the audience who care about quads vs triangles (animation rigs, subdivision workflows, exporting to DCCs for hand-editing). ## CodeRabbit: keyboard accessibility for the dialog Previously declined on the grounds that every other Inspector- styled dialog uses the same mouse-only `InspectorButton` and fixing only this one would be inconsistent. On reflection that's the wrong tradeoff for a modal dialog — keyboard users were locked out entirely until they could find the close button with the mouse. Solution: add a Window-level keyboard handler (`Item { id: keyCapture; focus: true; Keys.onPressed: ... }`) that intercepts Enter / Return → run retopo, and Escape → close. This keeps the `InspectorButton` primitive unchanged (matches the other dialogs) while giving the modal proper keyboard access. The `open()` function now calls `keyCapture.forceActiveFocus()` so the handler is live as soon as the dialog appears. Factored the existing button onClicked into a `runRetopo()` function so both mouse click and Enter key go through the same code path. Co-Authored-By: Claude Opus 4.7 (1M context) --- qml/PropertiesPanel.qml | 78 +++++++++++++++++++++------------------- qml/QuadRetopoDialog.qml | 70 +++++++++++++++++++++++++----------- 2 files changed, 91 insertions(+), 57 deletions(-) diff --git a/qml/PropertiesPanel.qml b/qml/PropertiesPanel.qml index e020e29cb..31fa0489a 100644 --- a/qml/PropertiesPanel.qml +++ b/qml/PropertiesPanel.qml @@ -1431,6 +1431,47 @@ Rectangle { // Separator Rectangle { width: parent.width - 16; height: 1; color: PropertiesPanelController.borderColor } + // Issue #401: Quad retopology via triangle pairing. Lives + // in Edit Mode since this is a topology operation (turns + // pairs of triangles into quads via the n-gon binding) — + // not a material/texture operation. + Rectangle { + width: Math.min(parent.width - 16, retopoLabel.implicitWidth + 16) + height: 26 + radius: 3 + opacity: QuadRetopoController.hasSelection ? 1.0 : 0.45 + color: retopoMa.containsMouse && QuadRetopoController.hasSelection + ? PropertiesPanelController.highlightColor + : PropertiesPanelController.headerColor + border.color: PropertiesPanelController.borderColor + border.width: 1 + + Text { + id: retopoLabel + anchors.centerIn: parent + text: "Quad Retopology…" + color: PropertiesPanelController.textColor + font.pixelSize: 11 + } + MouseArea { + id: retopoMa + anchors.fill: parent + hoverEnabled: true + enabled: QuadRetopoController.hasSelection + cursorShape: QuadRetopoController.hasSelection + ? Qt.PointingHandCursor : Qt.ForbiddenCursor + onClicked: root.openQuadRetopoDialog() + ToolTip.visible: containsMouse + ToolTip.delay: 500 + ToolTip.text: QuadRetopoController.hasSelection + ? "Pair adjacent triangles into quad-dominant topology. Skin weights survive (no new vertices)." + : "Select a mesh first." + } + } + + // Separator + Rectangle { width: parent.width - 16; height: 1; color: PropertiesPanelController.borderColor } + // Mesh validation warnings Text { width: parent.width - 16 @@ -3609,43 +3650,6 @@ Rectangle { } } - // Issue #401: Quad retopology via triangle pairing. - // Operates on the currently selected entity, mutates the - // mesh in place via the qtme.faces. n-gon binding. - Rectangle { - width: Math.min(parent.width - 16, retopoLabel.implicitWidth + 16) - height: 26 - radius: 3 - opacity: QuadRetopoController.hasSelection ? 1.0 : 0.45 - color: retopoMa.containsMouse && QuadRetopoController.hasSelection - ? PropertiesPanelController.highlightColor - : PropertiesPanelController.headerColor - border.color: PropertiesPanelController.borderColor - border.width: 1 - - Text { - id: retopoLabel - anchors.centerIn: parent - text: "Quad Retopology…" - color: PropertiesPanelController.textColor - font.pixelSize: 11 - } - MouseArea { - id: retopoMa - anchors.fill: parent - hoverEnabled: true - enabled: QuadRetopoController.hasSelection - cursorShape: QuadRetopoController.hasSelection - ? Qt.PointingHandCursor : Qt.ForbiddenCursor - onClicked: root.openQuadRetopoDialog() - ToolTip.visible: containsMouse - ToolTip.delay: 500 - ToolTip.text: QuadRetopoController.hasSelection - ? "Pair adjacent triangles into quad-dominant topology. Skin weights survive (no new vertices)." - : "Select a mesh first." - } - } - // Slice I: Material Preview Environment — interactive // preview of the currently-selected material on Sphere/Cube // shapes. Drag horizontally on the thumbnail to rotate the diff --git a/qml/QuadRetopoDialog.qml b/qml/QuadRetopoDialog.qml index 5a4a71c38..6bb299d41 100644 --- a/qml/QuadRetopoDialog.qml +++ b/qml/QuadRetopoDialog.qml @@ -39,6 +39,55 @@ Window { dialog.show() dialog.raise() dialog.requestActivate() + keyCapture.forceActiveFocus() + } + + // Pulled out of the button's onClicked so the same flow runs + // whether triggered by mouse OR by Enter/Return at the Window + // level (keyboard accessibility for the modal dialog — see + // CodeRabbit review on PR #697). + function runRetopo() { + if (QuadRetopoController.busy) return + if (!QuadRetopoController.hasSelection) return + const r = QuadRetopoController.retopologizeSelected( + dialog.targetFaces, + dialog.maxAngleDeg, + dialog.shapeToleranceDeg, + dialog.maxAspectRatio) + if (r && r.applied) { + const dom = Math.round((r.quadDominance || 0) * 1000) / 10 + dialog.lastStatus = + "Done: " + r.totalTrianglesBefore + " tris → " + + r.totalFacesAfter + " faces (" + + r.totalQuadsAfter + " quads, " + + r.totalTrianglesAfter + " tris, " + + dom + "% quad dominance)" + dialog.lastWasError = false + } else { + dialog.lastStatus = "Failed: " + (r && r.error ? r.error : "unknown error") + dialog.lastWasError = true + } + } + + // Window-level key handler — invisible Item that owns active + // focus when the dialog opens. Enter / Return runs the retopo, + // Escape closes the dialog. Without this, keyboard users + // couldn't drive the modal at all because InspectorButton + // doesn't accept tab focus (it's a Rectangle + MouseArea, not + // a QtQuick.Controls.Button). + Item { + id: keyCapture + anchors.fill: parent + focus: true + Keys.onPressed: function(event) { + if (event.key === Qt.Key_Escape) { + dialog.close() + event.accepted = true + } else if (event.key === Qt.Key_Return || event.key === Qt.Key_Enter) { + dialog.runRetopo() + event.accepted = true + } + } } // ── Inline Inspector primitives ───────────────────────────────── @@ -236,26 +285,7 @@ Window { Layout.preferredWidth: 160 buttonEnabled: !QuadRetopoController.busy && QuadRetopoController.hasSelection - onClicked: { - const r = QuadRetopoController.retopologizeSelected( - dialog.targetFaces, - dialog.maxAngleDeg, - dialog.shapeToleranceDeg, - dialog.maxAspectRatio) - if (r && r.applied) { - const dom = Math.round((r.quadDominance || 0) * 1000) / 10 - dialog.lastStatus = - "Done: " + r.totalTrianglesBefore + " tris → " - + r.totalFacesAfter + " faces (" - + r.totalQuadsAfter + " quads, " - + r.totalTrianglesAfter + " tris, " - + dom + "% quad dominance)" - dialog.lastWasError = false - } else { - dialog.lastStatus = "Failed: " + (r && r.error ? r.error : "unknown error") - dialog.lastWasError = true - } - } + onClicked: dialog.runRetopo() } } }