From a7d4d445d75e8eb1a20600d2b74b3e2bac756210 Mon Sep 17 00:00:00 2001 From: Fernando Date: Thu, 5 Mar 2026 00:41:12 -0400 Subject: [PATCH 1/3] Add vertex normal visualization feature (Show Normals) Add NormalVisualizer class that draws colored lines from each vertex along its normal direction using ManualObject with OT_LINE_LIST. Normals are color-coded by direction (|X|->Red, |Y|->Green, |Z|->Blue) like standard normal maps. Toggled via Options -> Show Normals menu action. Overlays auto-update when entities are added/removed via Manager signals. ManualObjects are attached to dedicated child scene nodes to avoid crashes from code that does unsafe static_cast on all attached objects. Co-Authored-By: Claude Opus 4.6 --- src/CMakeLists.txt | 2 + src/NormalVisualizer.cpp | 228 ++++++++++++++++++++++++++++++++++ src/NormalVisualizer.h | 40 ++++++ src/NormalVisualizer_test.cpp | 110 ++++++++++++++++ src/mainwindow.cpp | 5 + src/mainwindow.h | 2 + ui_files/mainwindow.ui | 12 ++ 7 files changed, 399 insertions(+) create mode 100644 src/NormalVisualizer.cpp create mode 100644 src/NormalVisualizer.h create mode 100644 src/NormalVisualizer_test.cpp diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 5b4e8e6..fc7c9c5 100755 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -42,6 +42,7 @@ MCPServer.cpp MCPSettingsDialog.cpp SentryReporter.cpp BoneWeightOverlay.cpp +NormalVisualizer.cpp AnimationMerger.cpp ) @@ -88,6 +89,7 @@ MCPServer.h MCPSettingsDialog.h SentryReporter.h BoneWeightOverlay.h +NormalVisualizer.h AnimationMerger.h ) diff --git a/src/NormalVisualizer.cpp b/src/NormalVisualizer.cpp new file mode 100644 index 0000000..8fa2417 --- /dev/null +++ b/src/NormalVisualizer.cpp @@ -0,0 +1,228 @@ +#include "NormalVisualizer.h" +#include "Manager.h" + +NormalVisualizer::NormalVisualizer(Ogre::SceneManager* sceneMgr, QObject* parent) + : QObject(parent) + , mSceneMgr(sceneMgr) +{ + createMaterial(); + + connect(Manager::getSingleton(), &Manager::entityCreated, + this, &NormalVisualizer::onEntityCreated); + connect(Manager::getSingleton(), &Manager::sceneNodeDestroyed, + this, &NormalVisualizer::onSceneNodeDestroyed); +} + +NormalVisualizer::~NormalVisualizer() +{ + destroyAllOverlays(); +} + +void NormalVisualizer::createMaterial() +{ + Ogre::String matName = "NormalVisualizer/Material"; + + mMaterial = Ogre::static_pointer_cast( + Ogre::MaterialManager::getSingleton().getByName(matName)); + + if (!mMaterial) + { + mMaterial = Ogre::static_pointer_cast( + Ogre::MaterialManager::getSingleton().create( + matName, Ogre::ResourceGroupManager::INTERNAL_RESOURCE_GROUP_NAME)); + + Ogre::Pass* p = mMaterial->getTechnique(0)->getPass(0); + p->setLightingEnabled(false); + p->setVertexColourTracking(Ogre::TVC_DIFFUSE); + p->setCullingMode(Ogre::CULL_NONE); + p->setDepthCheckEnabled(false); + p->setDepthWriteEnabled(false); + } +} + +void NormalVisualizer::setVisible(bool visible) +{ + mVisible = visible; + if (visible) + { + // Iterate scene nodes manually and filter by movable type, + // because Manager::getEntities() does unsafe static_cast on all + // attached objects (ManualObjects would crash). + for (Ogre::SceneNode* node : Manager::getSingleton()->getSceneNodes()) + { + if (!node) continue; + for (unsigned short i = 0; i < node->numAttachedObjects(); ++i) + { + Ogre::MovableObject* obj = node->getAttachedObject(i); + if (obj && obj->getMovableType() == "Entity") + buildOverlayForEntity(static_cast(obj)); + } + } + } + else + { + destroyAllOverlays(); + } +} + +void NormalVisualizer::onEntityCreated(Ogre::Entity* const& entity) +{ + if (mVisible) + buildOverlayForEntity(entity); +} + +void NormalVisualizer::onSceneNodeDestroyed(Ogre::SceneNode* const& node) +{ + if (!mVisible) + return; + + // Find and remove overlays for entities attached to this node + QList toRemove; + for (auto it = mOverlays.begin(); it != mOverlays.end(); ++it) + { + if (it.key()->getParentSceneNode() == node) + toRemove.append(it.key()); + } + for (Ogre::Entity* entity : toRemove) + destroyOverlayForEntity(entity); +} + +void NormalVisualizer::buildOverlayForEntity(Ogre::Entity* entity) +{ + if (!entity || !entity->getParentSceneNode()) + return; + + // Don't duplicate if already built + if (mOverlays.contains(entity)) + return; + + Ogre::MeshPtr mesh = entity->getMesh(); + if (!mesh) + return; + + Ogre::String moName = "NormalVisualizer_" + entity->getName(); + Ogre::ManualObject* mo = mSceneMgr->createManualObject(moName); + + const float normalLength = 0.1f; + bool addedShared = false; + bool hasContent = false; + + for (unsigned short si = 0; si < mesh->getNumSubMeshes(); ++si) + { + Ogre::SubMesh* submesh = mesh->getSubMesh(si); + + Ogre::VertexData* vertexData = submesh->useSharedVertices + ? mesh->sharedVertexData : submesh->vertexData; + + if (!vertexData) + continue; + + if (submesh->useSharedVertices && addedShared) + continue; + if (submesh->useSharedVertices) + addedShared = true; + + const auto* posElem = vertexData->vertexDeclaration->findElementBySemantic(Ogre::VES_POSITION); + const auto* normElem = vertexData->vertexDeclaration->findElementBySemantic(Ogre::VES_NORMAL); + + if (!posElem || !normElem) + continue; + + auto posVbuf = vertexData->vertexBufferBinding->getBuffer(posElem->getSource()); + auto* posData = static_cast(posVbuf->lock(Ogre::HardwareBuffer::HBL_READ_ONLY)); + + // Normal may be in a different buffer + unsigned char* normData = nullptr; + Ogre::HardwareVertexBufferSharedPtr normVbuf; + bool sameBuffer = (posElem->getSource() == normElem->getSource()); + if (sameBuffer) + { + normData = posData; + normVbuf = posVbuf; + } + else + { + normVbuf = vertexData->vertexBufferBinding->getBuffer(normElem->getSource()); + normData = static_cast(normVbuf->lock(Ogre::HardwareBuffer::HBL_READ_ONLY)); + } + + mo->begin(mMaterial->getName(), Ogre::RenderOperation::OT_LINE_LIST); + mo->estimateVertexCount(vertexData->vertexCount * 2); + + for (size_t j = 0; j < vertexData->vertexCount; ++j) + { + Ogre::Real* pPos; + posElem->baseVertexPointerToElement(posData + j * posVbuf->getVertexSize(), &pPos); + + Ogre::Real* pNorm; + normElem->baseVertexPointerToElement(normData + j * normVbuf->getVertexSize(), &pNorm); + + Ogre::Vector3 pos(pPos[0], pPos[1], pPos[2]); + Ogre::Vector3 norm(pNorm[0], pNorm[1], pNorm[2]); + + // Color = abs(normal) mapped to RGB (like a normal map) + Ogre::ColourValue color( + std::abs(norm.x), + std::abs(norm.y), + std::abs(norm.z), + 1.0f + ); + + // Start point of the line + mo->position(pos); + mo->colour(color); + + // End point of the line + Ogre::Vector3 endPos = pos + norm * normalLength; + mo->position(endPos); + mo->colour(color); + } + + mo->end(); + hasContent = true; + + posVbuf->unlock(); + if (!sameBuffer) + normVbuf->unlock(); + } + + if (hasContent) + { + // Attach to a dedicated child node so the ManualObject doesn't appear + // as an attached object on the entity's node. Code like + // ObjectItemModel::appendEntitiesFromNode does unsafe static_cast + // on all attached objects and would crash on our ManualObject. + Ogre::SceneNode* overlayNode = entity->getParentSceneNode()->createChildSceneNode(); + overlayNode->attachObject(mo); + mOverlays.insert(entity, {mo, overlayNode}); + } + else + { + mSceneMgr->destroyManualObject(mo); + } +} + +void NormalVisualizer::destroyOverlayForEntity(Ogre::Entity* entity) +{ + auto it = mOverlays.find(entity); + if (it == mOverlays.end()) + return; + + OverlayData& data = it.value(); + data.node->detachObject(data.manualObject); + mSceneMgr->destroyManualObject(data.manualObject); + mSceneMgr->destroySceneNode(data.node); + mOverlays.erase(it); +} + +void NormalVisualizer::destroyAllOverlays() +{ + for (auto it = mOverlays.begin(); it != mOverlays.end(); ++it) + { + OverlayData& data = it.value(); + data.node->detachObject(data.manualObject); + mSceneMgr->destroyManualObject(data.manualObject); + mSceneMgr->destroySceneNode(data.node); + } + mOverlays.clear(); +} diff --git a/src/NormalVisualizer.h b/src/NormalVisualizer.h new file mode 100644 index 0000000..d5df61b --- /dev/null +++ b/src/NormalVisualizer.h @@ -0,0 +1,40 @@ +#ifndef NORMALVISUALIZER_H +#define NORMALVISUALIZER_H + +#include +#include +#include + +class NormalVisualizer : public QObject +{ + Q_OBJECT +public: + NormalVisualizer(Ogre::SceneManager* sceneMgr, QObject* parent = nullptr); + ~NormalVisualizer() override; + + bool isVisible() const { return mVisible; } + +public slots: + void setVisible(bool visible); + +private slots: + void onEntityCreated(Ogre::Entity* const& entity); + void onSceneNodeDestroyed(Ogre::SceneNode* const& node); + +private: + void buildOverlayForEntity(Ogre::Entity* entity); + void destroyOverlayForEntity(Ogre::Entity* entity); + void destroyAllOverlays(); + void createMaterial(); + + Ogre::SceneManager* mSceneMgr; + Ogre::MaterialPtr mMaterial; + bool mVisible = false; + struct OverlayData { + Ogre::ManualObject* manualObject; + Ogre::SceneNode* node; + }; + QMap mOverlays; +}; + +#endif // NORMALVISUALIZER_H diff --git a/src/NormalVisualizer_test.cpp b/src/NormalVisualizer_test.cpp new file mode 100644 index 0000000..cb81fc7 --- /dev/null +++ b/src/NormalVisualizer_test.cpp @@ -0,0 +1,110 @@ +#include +#include +#include +#include +#include +#include "NormalVisualizer.h" +#include "Manager.h" +#include "SelectionSet.h" +#include "TestHelpers.h" + +// =========================================================================== +// Integration tests (require Ogre) +// =========================================================================== + +class NormalVisualizerIntegrationTest : public ::testing::Test { +protected: + QApplication* app = nullptr; + + void SetUp() override { + Manager::kill(); + QThread::msleep(50); + app = qobject_cast(QCoreApplication::instance()); + ASSERT_NE(app, nullptr); + if (!tryInitOgre()) { + GTEST_SKIP() << "Skipping: Ogre initialization failed"; + } + createStandardOgreMaterials(); + + // Remove leftover material so createMaterial() runs fresh + auto existing = Ogre::MaterialManager::getSingleton().getByName( + "NormalVisualizer/Material"); + if (existing) + Ogre::MaterialManager::getSingleton().remove(existing); + } + + void TearDown() override { + if (!Manager::getSingletonPtr()) + return; + auto existing = Ogre::MaterialManager::getSingleton().getByName( + "NormalVisualizer/Material"); + if (existing) + Ogre::MaterialManager::getSingleton().remove(existing); + SelectionSet::getSingleton()->clear(); + Manager::kill(); + if (app) app->processEvents(); + QThread::msleep(50); + } +}; + +TEST_F(NormalVisualizerIntegrationTest, MaterialCreatedOnConstruction) +{ + NormalVisualizer visualizer(Manager::getSingleton()->getSceneMgr()); + + auto mat = Ogre::MaterialManager::getSingleton().getByName( + "NormalVisualizer/Material"); + ASSERT_TRUE(mat) << "NormalVisualizer should create its material on construction"; + + Ogre::Pass* p = mat->getTechnique(0)->getPass(0); + ASSERT_NE(p, nullptr); + + EXPECT_FALSE(p->getLightingEnabled()); + EXPECT_TRUE(p->getVertexColourTracking() & Ogre::TVC_DIFFUSE); + EXPECT_FALSE(p->getDepthWriteEnabled()); + EXPECT_FALSE(p->getDepthCheckEnabled()); +} + +TEST_F(NormalVisualizerIntegrationTest, InitiallyNotVisible) +{ + NormalVisualizer visualizer(Manager::getSingleton()->getSceneMgr()); + EXPECT_FALSE(visualizer.isVisible()); +} + +TEST_F(NormalVisualizerIntegrationTest, SetVisibleToggle) +{ + NormalVisualizer visualizer(Manager::getSingleton()->getSceneMgr()); + visualizer.setVisible(true); + EXPECT_TRUE(visualizer.isVisible()); + visualizer.setVisible(false); + EXPECT_FALSE(visualizer.isVisible()); +} + +TEST_F(NormalVisualizerIntegrationTest, BuildsOverlayForInMemoryMesh) +{ + if (!canLoadMeshFiles()) + GTEST_SKIP() << "Skipping: mesh loading not supported in headless mode"; + + auto meshPtr = createInMemoryTriangleMesh("NormalVisualizerTestMesh"); + ASSERT_TRUE(meshPtr); + + Ogre::Entity* entity = Manager::getSingleton()->getSceneMgr()->createEntity( + "NormalVisualizerTestEntity", meshPtr); + ASSERT_NE(entity, nullptr); + + Ogre::SceneNode* node = Manager::getSingleton()->getSceneMgr() + ->getRootSceneNode()->createChildSceneNode(); + node->attachObject(entity); + + NormalVisualizer visualizer(Manager::getSingleton()->getSceneMgr()); + visualizer.setVisible(true); + + // Manually call buildOverlayForEntity since the entity wasn't created via Manager + // (Manager::entityCreated signal was not emitted for direct SceneManager creation) + // Instead, just verify the toggle works without crash + visualizer.setVisible(false); + SUCCEED(); + + node->detachObject(entity); + Manager::getSingleton()->getSceneMgr()->destroyEntity(entity); + Manager::getSingleton()->getSceneMgr()->destroySceneNode(node); +} diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index e6f83ce..b56e45b 100755 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -41,6 +41,7 @@ #include "LLMSettingsWidget.h" #include "MCPSettingsDialog.h" #include "MCPServer.h" +#include "NormalVisualizer.h" #include "LLMManager.h" #include "QMLMaterialHighlighter.h" #include "ModelDownloader.h" @@ -336,6 +337,10 @@ void MainWindow::initToolBar() // show grid connect(ui->actionShow_Grid, SIGNAL(toggled(bool)),Manager::getSingleton()->getViewportGrid(),SLOT(setVisible(bool))); + // show normals + m_normalVisualizer = new NormalVisualizer(Manager::getSingleton()->getSceneMgr(), this); + connect(ui->actionShow_Normals, &QAction::toggled, m_normalVisualizer, &NormalVisualizer::setVisible); + // AI Settings menu QMenu* aiMenu = menuBar()->addMenu(tr("&AI")); QAction* aiSettingsAction = aiMenu->addAction(QIcon(":/icones/ai.png"), tr("AI Model Settings...")); diff --git a/src/mainwindow.h b/src/mainwindow.h index 9caaae8..65a5bc6 100755 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -13,6 +13,7 @@ class LLMSettingsWidget; class MCPServer; +class NormalVisualizer; namespace Ui { class MainWindow; @@ -121,6 +122,7 @@ public slots: void updateMergeAnimationsButton(); const QPalette& darkPalette(); + NormalVisualizer* m_normalVisualizer = nullptr; MCPServer* m_mcpServer = nullptr; QMenu* m_recentFilesMenu = nullptr; diff --git a/ui_files/mainwindow.ui b/ui_files/mainwindow.ui index 2cbb85a..227a41b 100755 --- a/ui_files/mainwindow.ui +++ b/ui_files/mainwindow.ui @@ -79,6 +79,7 @@ + @@ -213,6 +214,17 @@ Show Grid + + + true + + + false + + + Show Normals + + Import Mesh From 00db440f0669db191ae20e363d10df228ed9c985 Mon Sep 17 00:00:00 2001 From: Fernando Date: Thu, 5 Mar 2026 00:59:24 -0400 Subject: [PATCH 2/3] Add animated normals, MCP toggle_normals tool, and bump to v2.10.0 - Normals now follow skeletal animations in real-time by requesting software-skinned normals via addSoftwareAnimationRequest(true) and updating ManualObject geometry each frame via QTimer - Add toggle_normals MCP tool for enabling/disabling normal visualization programmatically (global toggle, no entity parameter required) - Add Sentry breadcrumb for Show Normals toggle - Expand test coverage: 9 NormalVisualizer tests + 2 MCP toggle_normals tests - Update CLAUDE.md with NormalVisualizer documentation - Bump version to 2.10.0 Co-Authored-By: Claude Opus 4.6 --- CLAUDE.md | 5 ++ CMakeLists.txt | 2 +- src/MCPServer.cpp | 36 ++++++++ src/MCPServer.h | 1 + src/MCPServer_test.cpp | 23 +++++- src/NormalVisualizer.cpp | 151 ++++++++++++++++++++++++++++++++-- src/NormalVisualizer.h | 10 +++ src/NormalVisualizer_test.cpp | 101 +++++++++++++++++++++-- 8 files changed, 313 insertions(+), 16 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 8dcbe7a..44580cf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -65,6 +65,11 @@ Three singletons manage core state. All run on the main thread. Access via `Clas - **MaterialEditorQML** (`src/MaterialEditorQML.h/cpp`): QML_SINGLETON exposing full Ogre material property access (colors, lighting, depth, blending, fog, textures) with undo/redo. QML UI in `qml/`. +### Debug Overlays + +- **NormalVisualizer** (`src/NormalVisualizer.h/cpp`): Draws vertex normals as colored lines (|X|=Red, |Y|=Green, |Z|=Blue). Toggled globally via Options → Show Normals menu or MCP `toggle_normals` tool. Supports real-time animation: requests software-skinned normals via `addSoftwareAnimationRequest(true)` and updates each frame for skeletal entities. Overlays attach to dedicated child scene nodes to avoid unsafe `static_cast` crashes in `ObjectItemModel` and `Manager::getEntities()`. +- **BoneWeightOverlay** (`src/BoneWeightOverlay.h/cpp`): Per-entity bone weight heat-map overlay. + ### MCP Server - **MCPServer** (`src/MCPServer.h/cpp`): JSON-RPC 2.0 over stdio + HTTP REST API on configurable port. diff --git a/CMakeLists.txt b/CMakeLists.txt index e3e35a6..b17e5f9 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -13,7 +13,7 @@ cmake_minimum_required(VERSION 3.24.0) cmake_policy(SET CMP0005 NEW) cmake_policy(SET CMP0048 NEW) # manages project version -project(QtMeshEditor VERSION 2.9.0 LANGUAGES CXX) +project(QtMeshEditor VERSION 2.10.0 LANGUAGES CXX) message(STATUS "Building QtMeshEditor version ${PROJECT_VERSION}") set(QTMESHEDITOR_VERSION_STRING "\"${PROJECT_VERSION}\"") diff --git a/src/MCPServer.cpp b/src/MCPServer.cpp index d4ff3a9..6ad4ba5 100644 --- a/src/MCPServer.cpp +++ b/src/MCPServer.cpp @@ -8,6 +8,7 @@ #include "MeshImporterExporter.h" #include "OgreWidget.h" #include "AnimationWidget.h" +#include "NormalVisualizer.h" #include #include #include @@ -440,6 +441,8 @@ QJsonObject MCPServer::callTool(const QString &name, const QJsonObject &args) toolResult = toolToggleSkeletonDebug(args); } else if (name == "toggle_bone_weights") { toolResult = toolToggleBoneWeights(args); + } else if (name == "toggle_normals") { + toolResult = toolToggleNormals(args); } else if (name == "merge_animations") { toolResult = toolMergeAnimations(args); } else { @@ -1883,6 +1886,21 @@ void MCPServer::onAnimationTick() } } +QJsonObject MCPServer::toolToggleNormals(const QJsonObject &args) +{ + if (!m_mainWindow) + return makeErrorResult("Error: MainWindow not available. Run with --with-mcp flag."); + + NormalVisualizer* visualizer = m_mainWindow->findChild(); + if (!visualizer) + return makeErrorResult("Error: NormalVisualizer not found"); + + bool show = args.contains("show") ? args["show"].toBool() : !visualizer->isVisible(); + visualizer->setVisible(show); + + return makeSuccessResult(QString("Normals %1").arg(show ? "shown" : "hidden")); +} + QJsonObject MCPServer::toolMergeAnimations(const QJsonObject &args) { try { @@ -2411,6 +2429,24 @@ QJsonArray MCPServer::buildToolsList() )); } + // toggle_normals + { + QJsonObject inputSchema; + inputSchema["type"] = "object"; + QJsonObject props; + props["show"] = QJsonObject{{"type", "boolean"}, {"description", "True to show, false to hide. If omitted, toggles the current state."}}; + inputSchema["properties"] = props; + + tools.append(buildToolDefinition( + "toggle_normals", + "Show or hide vertex normal visualization on all entities in the scene. " + "Normals are displayed as colored lines extending from each vertex, " + "color-coded by direction (|X|=Red, |Y|=Green, |Z|=Blue). " + "Normals follow skeletal animations in real-time.", + inputSchema + )); + } + // merge_animations { QJsonObject inputSchema; diff --git a/src/MCPServer.h b/src/MCPServer.h index 7ac9d0a..7e634a1 100644 --- a/src/MCPServer.h +++ b/src/MCPServer.h @@ -141,6 +141,7 @@ private slots: QJsonObject toolPlayAnimation(const QJsonObject &args); QJsonObject toolToggleSkeletonDebug(const QJsonObject &args); QJsonObject toolToggleBoneWeights(const QJsonObject &args); + QJsonObject toolToggleNormals(const QJsonObject &args); QJsonObject toolMergeAnimations(const QJsonObject &args); // Animation diff --git a/src/MCPServer_test.cpp b/src/MCPServer_test.cpp index 73fd01c..db80acd 100644 --- a/src/MCPServer_test.cpp +++ b/src/MCPServer_test.cpp @@ -419,7 +419,7 @@ TEST_F(MCPServerTest, HandleToolsList) "list_skeletal_animations", "get_animation_info", "set_animation_length", "set_animation_time", "add_keyframe", "remove_keyframe", "play_animation", "toggle_skeleton_debug", "toggle_bone_weights", - "merge_animations" + "toggle_normals", "merge_animations" }; for (const QString &tool : knownTools) { @@ -2489,3 +2489,24 @@ TEST_F(MCPServerTest, PlayAnimationWithSkeletonEntity) QJsonObject result = server->callTool("play_animation", args); EXPECT_FALSE(isError(result)); } + +// ========================================================================== +// NEW TESTS: toggle_normals +// ========================================================================== + +TEST_F(MCPServerTest, ToggleNormalsNoMainWindow) +{ + // Server has no MainWindow set — should fail gracefully + QJsonObject args; + args["show"] = true; + QJsonObject result = server->callTool("toggle_normals", args); + EXPECT_TRUE(isError(result)); + EXPECT_TRUE(getResultText(result).contains("MainWindow") || + getResultText(result).contains("NormalVisualizer")); +} + +TEST_F(MCPServerTest, ToggleNormalsIsRecognizedTool) +{ + QJsonObject result = server->callTool("toggle_normals", QJsonObject()); + EXPECT_FALSE(getResultText(result).contains("Unknown tool")); +} diff --git a/src/NormalVisualizer.cpp b/src/NormalVisualizer.cpp index 8fa2417..816d409 100644 --- a/src/NormalVisualizer.cpp +++ b/src/NormalVisualizer.cpp @@ -1,5 +1,6 @@ #include "NormalVisualizer.h" #include "Manager.h" +#include "SentryReporter.h" NormalVisualizer::NormalVisualizer(Ogre::SceneManager* sceneMgr, QObject* parent) : QObject(parent) @@ -11,10 +12,14 @@ NormalVisualizer::NormalVisualizer(Ogre::SceneManager* sceneMgr, QObject* parent this, &NormalVisualizer::onEntityCreated); connect(Manager::getSingleton(), &Manager::sceneNodeDestroyed, this, &NormalVisualizer::onSceneNodeDestroyed); + + connect(&mUpdateTimer, &QTimer::timeout, this, &NormalVisualizer::updateAnimatedOverlays); } NormalVisualizer::~NormalVisualizer() { + mUpdateTimer.stop(); + mUpdateTimer.disconnect(); destroyAllOverlays(); } @@ -42,6 +47,8 @@ void NormalVisualizer::createMaterial() void NormalVisualizer::setVisible(bool visible) { + SentryReporter::addBreadcrumb("ui.action", visible ? "Show normals" : "Hide normals"); + mVisible = visible; if (visible) { @@ -58,9 +65,11 @@ void NormalVisualizer::setVisible(bool visible) buildOverlayForEntity(static_cast(obj)); } } + mUpdateTimer.start(0); } else { + mUpdateTimer.stop(); destroyAllOverlays(); } } @@ -100,10 +109,19 @@ void NormalVisualizer::buildOverlayForEntity(Ogre::Entity* entity) if (!mesh) return; + bool hasSkel = entity->hasSkeleton(); + + // Request software animation with normals for skeletal entities + if (hasSkel && !mSoftwareAnimRequested.contains(entity)) + { + entity->addSoftwareAnimationRequest(true); + mSoftwareAnimRequested.insert(entity); + } + Ogre::String moName = "NormalVisualizer_" + entity->getName(); Ogre::ManualObject* mo = mSceneMgr->createManualObject(moName); + mo->setDynamic(hasSkel); - const float normalLength = 0.1f; bool addedShared = false; bool hasContent = false; @@ -160,7 +178,6 @@ void NormalVisualizer::buildOverlayForEntity(Ogre::Entity* entity) Ogre::Vector3 pos(pPos[0], pPos[1], pPos[2]); Ogre::Vector3 norm(pNorm[0], pNorm[1], pNorm[2]); - // Color = abs(normal) mapped to RGB (like a normal map) Ogre::ColourValue color( std::abs(norm.x), std::abs(norm.y), @@ -168,12 +185,10 @@ void NormalVisualizer::buildOverlayForEntity(Ogre::Entity* entity) 1.0f ); - // Start point of the line mo->position(pos); mo->colour(color); - // End point of the line - Ogre::Vector3 endPos = pos + norm * normalLength; + Ogre::Vector3 endPos = pos + norm * NORMAL_LENGTH; mo->position(endPos); mo->colour(color); } @@ -194,7 +209,7 @@ void NormalVisualizer::buildOverlayForEntity(Ogre::Entity* entity) // on all attached objects and would crash on our ManualObject. Ogre::SceneNode* overlayNode = entity->getParentSceneNode()->createChildSceneNode(); overlayNode->attachObject(mo); - mOverlays.insert(entity, {mo, overlayNode}); + mOverlays.insert(entity, {mo, overlayNode, hasSkel}); } else { @@ -202,6 +217,120 @@ void NormalVisualizer::buildOverlayForEntity(Ogre::Entity* entity) } } +void NormalVisualizer::updateAnimatedOverlays() +{ + for (auto it = mOverlays.begin(); it != mOverlays.end(); ++it) + { + Ogre::Entity* entity = it.key(); + OverlayData& data = it.value(); + + if (!data.hasSkeleton || !entity->hasSkeleton()) + continue; + + entity->_updateAnimation(); + + Ogre::MeshPtr mesh = entity->getMesh(); + bool addedShared = false; + unsigned short sectionIndex = 0; + + for (unsigned short si = 0; si < mesh->getNumSubMeshes(); ++si) + { + Ogre::SubMesh* submesh = mesh->getSubMesh(si); + + Ogre::VertexData* vertexData = submesh->useSharedVertices + ? mesh->sharedVertexData : submesh->vertexData; + + if (!vertexData) + continue; + + if (submesh->useSharedVertices && addedShared) + continue; + if (submesh->useSharedVertices) + addedShared = true; + + // Check bind-pose has normals (same check as buildOverlayForEntity) + const auto* bindPosElem = vertexData->vertexDeclaration->findElementBySemantic(Ogre::VES_POSITION); + const auto* bindNormElem = vertexData->vertexDeclaration->findElementBySemantic(Ogre::VES_NORMAL); + if (!bindPosElem || !bindNormElem) + continue; + + // Get animated vertex data + Ogre::VertexData* animData = nullptr; + if (submesh->useSharedVertices) + animData = entity->_getSkelAnimVertexData(); + else + animData = entity->getSubEntity(si)->_getSkelAnimVertexData(); + + if (!animData) + { + ++sectionIndex; + continue; + } + + const auto* posElem = animData->vertexDeclaration->findElementBySemantic(Ogre::VES_POSITION); + const auto* normElem = animData->vertexDeclaration->findElementBySemantic(Ogre::VES_NORMAL); + if (!posElem || !normElem) + { + ++sectionIndex; + continue; + } + + auto posVbuf = animData->vertexBufferBinding->getBuffer(posElem->getSource()); + auto* posBytes = static_cast(posVbuf->lock(Ogre::HardwareBuffer::HBL_READ_ONLY)); + + unsigned char* normBytes = nullptr; + Ogre::HardwareVertexBufferSharedPtr normVbuf; + bool sameBuffer = (posElem->getSource() == normElem->getSource()); + if (sameBuffer) + { + normBytes = posBytes; + normVbuf = posVbuf; + } + else + { + normVbuf = animData->vertexBufferBinding->getBuffer(normElem->getSource()); + normBytes = static_cast(normVbuf->lock(Ogre::HardwareBuffer::HBL_READ_ONLY)); + } + + data.manualObject->beginUpdate(sectionIndex); + + for (size_t j = 0; j < animData->vertexCount; ++j) + { + Ogre::Real* pPos; + posElem->baseVertexPointerToElement(posBytes + j * posVbuf->getVertexSize(), &pPos); + + Ogre::Real* pNorm; + normElem->baseVertexPointerToElement(normBytes + j * normVbuf->getVertexSize(), &pNorm); + + Ogre::Vector3 pos(pPos[0], pPos[1], pPos[2]); + Ogre::Vector3 norm(pNorm[0], pNorm[1], pNorm[2]); + + Ogre::ColourValue color( + std::abs(norm.x), + std::abs(norm.y), + std::abs(norm.z), + 1.0f + ); + + data.manualObject->position(pos); + data.manualObject->colour(color); + + Ogre::Vector3 endPos = pos + norm * NORMAL_LENGTH; + data.manualObject->position(endPos); + data.manualObject->colour(color); + } + + data.manualObject->end(); + + posVbuf->unlock(); + if (!sameBuffer) + normVbuf->unlock(); + + ++sectionIndex; + } + } +} + void NormalVisualizer::destroyOverlayForEntity(Ogre::Entity* entity) { auto it = mOverlays.find(entity); @@ -213,6 +342,12 @@ void NormalVisualizer::destroyOverlayForEntity(Ogre::Entity* entity) mSceneMgr->destroyManualObject(data.manualObject); mSceneMgr->destroySceneNode(data.node); mOverlays.erase(it); + + if (mSoftwareAnimRequested.contains(entity)) + { + entity->removeSoftwareAnimationRequest(true); + mSoftwareAnimRequested.remove(entity); + } } void NormalVisualizer::destroyAllOverlays() @@ -225,4 +360,8 @@ void NormalVisualizer::destroyAllOverlays() mSceneMgr->destroySceneNode(data.node); } mOverlays.clear(); + + for (Ogre::Entity* entity : mSoftwareAnimRequested) + entity->removeSoftwareAnimationRequest(true); + mSoftwareAnimRequested.clear(); } diff --git a/src/NormalVisualizer.h b/src/NormalVisualizer.h index d5df61b..bfc96c6 100644 --- a/src/NormalVisualizer.h +++ b/src/NormalVisualizer.h @@ -2,8 +2,11 @@ #define NORMALVISUALIZER_H #include +#include #include #include +#include +#include class NormalVisualizer : public QObject { @@ -26,15 +29,22 @@ private slots: void destroyOverlayForEntity(Ogre::Entity* entity); void destroyAllOverlays(); void createMaterial(); + void updateAnimatedOverlays(); Ogre::SceneManager* mSceneMgr; Ogre::MaterialPtr mMaterial; bool mVisible = false; + QTimer mUpdateTimer; + + static constexpr float NORMAL_LENGTH = 0.1f; + struct OverlayData { Ogre::ManualObject* manualObject; Ogre::SceneNode* node; + bool hasSkeleton; }; QMap mOverlays; + QSet mSoftwareAnimRequested; }; #endif // NORMALVISUALIZER_H diff --git a/src/NormalVisualizer_test.cpp b/src/NormalVisualizer_test.cpp index cb81fc7..f9d947f 100644 --- a/src/NormalVisualizer_test.cpp +++ b/src/NormalVisualizer_test.cpp @@ -64,6 +64,19 @@ TEST_F(NormalVisualizerIntegrationTest, MaterialCreatedOnConstruction) EXPECT_FALSE(p->getDepthCheckEnabled()); } +TEST_F(NormalVisualizerIntegrationTest, MaterialUsesDiffuseNotAmbient) +{ + NormalVisualizer visualizer(Manager::getSingleton()->getSceneMgr()); + + auto mat = Ogre::MaterialManager::getSingleton().getByName( + "NormalVisualizer/Material"); + ASSERT_TRUE(mat); + + Ogre::Pass* p = mat->getTechnique(0)->getPass(0); + EXPECT_TRUE(p->getVertexColourTracking() & Ogre::TVC_DIFFUSE) + << "Must use TVC_DIFFUSE for vertex colours to work with lighting disabled (RTSS)"; +} + TEST_F(NormalVisualizerIntegrationTest, InitiallyNotVisible) { NormalVisualizer visualizer(Manager::getSingleton()->getSceneMgr()); @@ -79,6 +92,53 @@ TEST_F(NormalVisualizerIntegrationTest, SetVisibleToggle) EXPECT_FALSE(visualizer.isVisible()); } +TEST_F(NormalVisualizerIntegrationTest, SetVisibleOnEmptySceneNoError) +{ + NormalVisualizer visualizer(Manager::getSingleton()->getSceneMgr()); + // Should not crash on empty scene + visualizer.setVisible(true); + EXPECT_TRUE(visualizer.isVisible()); + visualizer.setVisible(false); + EXPECT_FALSE(visualizer.isVisible()); +} + +TEST_F(NormalVisualizerIntegrationTest, DoubleSetVisibleTrueNoError) +{ + NormalVisualizer visualizer(Manager::getSingleton()->getSceneMgr()); + visualizer.setVisible(true); + visualizer.setVisible(true); // Should not crash or duplicate + EXPECT_TRUE(visualizer.isVisible()); + visualizer.setVisible(false); +} + +TEST_F(NormalVisualizerIntegrationTest, DestructorCleansUpWhileVisible) +{ + if (!canLoadMeshFiles()) + GTEST_SKIP() << "Skipping: mesh loading not supported in headless mode"; + + auto meshPtr = createInMemoryTriangleMesh("NormVizDestructorMesh"); + ASSERT_TRUE(meshPtr); + + Ogre::SceneNode* node = Manager::getSingleton()->getSceneMgr() + ->getRootSceneNode()->createChildSceneNode("NormVizDestructorNode"); + Ogre::Entity* entity = Manager::getSingleton()->getSceneMgr()->createEntity( + "NormVizDestructorEntity", meshPtr); + node->attachObject(entity); + + { + NormalVisualizer visualizer(Manager::getSingleton()->getSceneMgr()); + visualizer.setVisible(true); + // Destructor runs here — should clean up ManualObjects and child nodes + } + + // Verify the entity's node still exists and the entity is valid + EXPECT_EQ(entity->getParentSceneNode(), node); + + node->detachObject(entity); + Manager::getSingleton()->getSceneMgr()->destroyEntity(entity); + Manager::getSingleton()->getSceneMgr()->destroySceneNode(node); +} + TEST_F(NormalVisualizerIntegrationTest, BuildsOverlayForInMemoryMesh) { if (!canLoadMeshFiles()) @@ -87,23 +147,48 @@ TEST_F(NormalVisualizerIntegrationTest, BuildsOverlayForInMemoryMesh) auto meshPtr = createInMemoryTriangleMesh("NormalVisualizerTestMesh"); ASSERT_TRUE(meshPtr); + Ogre::SceneNode* node = Manager::getSingleton()->getSceneMgr() + ->getRootSceneNode()->createChildSceneNode("NormVizTestNode"); Ogre::Entity* entity = Manager::getSingleton()->getSceneMgr()->createEntity( "NormalVisualizerTestEntity", meshPtr); - ASSERT_NE(entity, nullptr); - - Ogre::SceneNode* node = Manager::getSingleton()->getSceneMgr() - ->getRootSceneNode()->createChildSceneNode(); node->attachObject(entity); NormalVisualizer visualizer(Manager::getSingleton()->getSceneMgr()); visualizer.setVisible(true); - // Manually call buildOverlayForEntity since the entity wasn't created via Manager - // (Manager::entityCreated signal was not emitted for direct SceneManager creation) - // Instead, just verify the toggle works without crash + // The overlay ManualObject should be on a child node, not directly on entity's node + // So entity's node should still only have 1 attached object (the entity itself) + // but should have a child node with the ManualObject + EXPECT_GE(node->numChildren(), 1u); + + visualizer.setVisible(false); + + node->detachObject(entity); + Manager::getSingleton()->getSceneMgr()->destroyEntity(entity); + Manager::getSingleton()->getSceneMgr()->destroySceneNode(node); +} + +TEST_F(NormalVisualizerIntegrationTest, OverlayCreatedForSkeletalEntity) +{ + if (!canLoadMeshFiles()) + GTEST_SKIP() << "Skipping: mesh loading not supported in headless mode"; + + Ogre::Entity* entity = createAnimatedTestEntity("NormVizAnimEntity"); + if (!entity) + GTEST_SKIP() << "Skipping: could not create animated test entity"; + + ASSERT_TRUE(entity->hasSkeleton()); + + NormalVisualizer visualizer(Manager::getSingleton()->getSceneMgr()); + visualizer.setVisible(true); + + Ogre::SceneNode* node = entity->getParentSceneNode(); + ASSERT_NE(node, nullptr); + EXPECT_GE(node->numChildren(), 1u); + visualizer.setVisible(false); - SUCCEED(); + // Clean up node->detachObject(entity); Manager::getSingleton()->getSceneMgr()->destroyEntity(entity); Manager::getSingleton()->getSceneMgr()->destroySceneNode(node); From 39b532ba3eb852c9e7d6cc1e41708a439a0d728a Mon Sep 17 00:00:00 2001 From: Fernando Date: Thu, 5 Mar 2026 08:05:01 -0400 Subject: [PATCH 3/3] Fix CI build and address CodeRabbit review findings - Add NormalVisualizer to tests/CMakeLists.txt (fixes linker errors) - Destroy NormalVisualizer before Manager::kill() to prevent UAF - Clean up stale overlays when entities are recreated with same name - Cap animation timer at ~60 FPS (start(16) instead of start(0)) - Bump MCP SERVER_VERSION to 1.1.0 for toggle_normals addition Co-Authored-By: Claude Opus 4.6 --- src/MCPServer.h | 2 +- src/NormalVisualizer.cpp | 26 ++++++++++++++++++++++++-- src/mainwindow.cpp | 5 +++++ tests/CMakeLists.txt | 2 ++ 4 files changed, 32 insertions(+), 3 deletions(-) diff --git a/src/MCPServer.h b/src/MCPServer.h index 7e634a1..8c5c871 100644 --- a/src/MCPServer.h +++ b/src/MCPServer.h @@ -193,7 +193,7 @@ private slots: // MCP Protocol version static constexpr const char* MCP_VERSION = "2024-11-05"; static constexpr const char* SERVER_NAME = "QtMeshEditor"; - static constexpr const char* SERVER_VERSION = "1.0.0"; + static constexpr const char* SERVER_VERSION = "1.1.0"; }; #endif // MCPSERVER_H diff --git a/src/NormalVisualizer.cpp b/src/NormalVisualizer.cpp index 816d409..a6db851 100644 --- a/src/NormalVisualizer.cpp +++ b/src/NormalVisualizer.cpp @@ -65,7 +65,7 @@ void NormalVisualizer::setVisible(bool visible) buildOverlayForEntity(static_cast(obj)); } } - mUpdateTimer.start(0); + mUpdateTimer.start(16); } else { @@ -109,6 +109,29 @@ void NormalVisualizer::buildOverlayForEntity(Ogre::Entity* entity) if (!mesh) return; + // Clean up stale overlay if an entity with the same name was replaced + // (e.g. PrimitiveObject::updatePrimitive destroys/recreates entities + // without emitting sceneNodeDestroyed, leaving a dangling entry). + Ogre::String moName = "NormalVisualizer_" + entity->getName(); + if (mSceneMgr->hasManualObject(moName)) + { + // Find and remove the stale entry keyed by the old (dangling) pointer + for (auto it = mOverlays.begin(); it != mOverlays.end(); ++it) + { + if (it.value().manualObject->getName() == moName) + { + OverlayData& stale = it.value(); + stale.node->detachObject(stale.manualObject); + mSceneMgr->destroyManualObject(stale.manualObject); + mSceneMgr->destroySceneNode(stale.node); + Ogre::Entity* staleEntity = it.key(); + mOverlays.erase(it); + mSoftwareAnimRequested.remove(staleEntity); + break; + } + } + } + bool hasSkel = entity->hasSkeleton(); // Request software animation with normals for skeletal entities @@ -118,7 +141,6 @@ void NormalVisualizer::buildOverlayForEntity(Ogre::Entity* entity) mSoftwareAnimRequested.insert(entity); } - Ogre::String moName = "NormalVisualizer_" + entity->getName(); Ogre::ManualObject* mo = mSceneMgr->createManualObject(moName); mo->setDynamic(hasSkel); diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index b56e45b..51dd2b9 100755 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -140,6 +140,11 @@ MainWindow::MainWindow(QWidget *parent) : /// /////////////////////// TODO improve the ui (toolbar, menubar,....) and add translation (obviously Portuguese but french, english, may be japaneese !) MainWindow::~MainWindow() { + // Destroy NormalVisualizer early — it connects to Manager signals and + // accesses Ogre resources, so it must be deleted while Manager is alive. + delete m_normalVisualizer; + m_normalVisualizer = nullptr; + // Stop MCP server if running if (m_mcpServer) { m_mcpServer->stop(); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 72414b9..bf71496 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -51,6 +51,7 @@ if(BUILD_TESTS) ${CMAKE_CURRENT_SOURCE_DIR}/../src/ModelDownloader.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../src/SentryReporter.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../src/BoneWeightOverlay.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/../src/NormalVisualizer.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../src/AnimationMerger.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../src/MCPServer.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../src/MCPSettingsDialog.cpp @@ -98,6 +99,7 @@ if(BUILD_TESTS) ${CMAKE_CURRENT_SOURCE_DIR}/../src/ModelDownloader.h ${CMAKE_CURRENT_SOURCE_DIR}/../src/SentryReporter.h ${CMAKE_CURRENT_SOURCE_DIR}/../src/BoneWeightOverlay.h + ${CMAKE_CURRENT_SOURCE_DIR}/../src/NormalVisualizer.h ${CMAKE_CURRENT_SOURCE_DIR}/../src/AnimationMerger.h ${CMAKE_CURRENT_SOURCE_DIR}/../src/MCPServer.h ${CMAKE_CURRENT_SOURCE_DIR}/../src/MCPSettingsDialog.h