From c60fc5b7f55091203b1922138a33c45b47f4b179 Mon Sep 17 00:00:00 2001 From: Fernando Date: Tue, 12 May 2026 11:25:42 -0400 Subject: [PATCH 1/3] feat(perf): draw-call analysis + merge suggestions (Phase 6 slice B) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Builds the second pillar of the optimization pipeline on top of Slice A's reporting scaffold (#494). DrawCallAnalyzer (new) — pure-data analyzer. Takes any list of Ogre::Entity* and produces a DrawCallReport: - totals: entities, submeshes, draw calls (1 per SubEntity), unique materials, projected draw-call count after merging, total savings; - clusters: every material plus the entities that use it and the draw-call savings unlocked by merging them; - suggestions: clusters with >=2 entities, ranked by savings. Surfaced through: - MeshInfoOverlay — appends "Draws: N (save K by merging)" so the overlay shows the merge potential at a glance, in sync with the current selection. - MCP tool analyze_draw_calls — returns the formatted summary in the standard content field plus a structured `drawCalls` payload (the same shape as get_memory_usage from slice A). - CLI subcommand `qtmesh analyze [--json]` for headless / CI workflows. The one-click merge action listed in #261's slice B scope is deliberately deferred: it is a write operation that touches the undo system, material/skeleton remapping, and submesh combination logic. The analysis alone (the harder pure-data work) is the bulk of the value — the merge suggestion in the JSON already tells users or automation which entities to combine. Tests: 12 DrawCallAnalyzerTest cases cover the byte math, suggestion filtering, JSON/text serialisation, and null-entity handling. The new MeshInfoOverlay test confirms the "Draws:" line appears. Ogre- backed paths follow the existing tryInitOgre() pattern. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/CLIPipeline.cpp | 53 +++++++++ src/CLIPipeline.h | 3 + src/CMakeLists.txt | 2 + src/DrawCallAnalyzer.cpp | 201 ++++++++++++++++++++++++++++++++++ src/DrawCallAnalyzer.h | 68 ++++++++++++ src/DrawCallAnalyzer_test.cpp | 162 +++++++++++++++++++++++++++ src/MCPServer.cpp | 36 ++++++ src/MCPServer.h | 1 + src/MeshInfoOverlay.cpp | 13 +++ src/MeshInfoOverlay_test.cpp | 24 ++++ src/main.cpp | 3 +- tests/CMakeLists.txt | 1 + 12 files changed, 566 insertions(+), 1 deletion(-) create mode 100644 src/DrawCallAnalyzer.cpp create mode 100644 src/DrawCallAnalyzer.h create mode 100644 src/DrawCallAnalyzer_test.cpp diff --git a/src/CLIPipeline.cpp b/src/CLIPipeline.cpp index 64e7d7c78..56a1b20de 100644 --- a/src/CLIPipeline.cpp +++ b/src/CLIPipeline.cpp @@ -13,6 +13,7 @@ #include "TextureChannelPacker.h" #include "NormalMapGenerator.h" #include "MemoryEstimator.h" +#include "DrawCallAnalyzer.h" #include "QtMeshCloudClient.h" #include #include @@ -608,6 +609,8 @@ void CLIPipeline::printUsage() " If --budget omitted and a token is set, the project's\n" " memory_budget_mb is fetched from QtMesh Cloud rules.\n" " --no-cloud opts out.\n" + " analyze [--json] Analyze draw calls: per-material grouping plus\n" + " merge suggestions for entities sharing a material.\n" "\n" "Global options:\n" " --help, -h Show this help\n" @@ -971,6 +974,7 @@ int CLIPipeline::run(int argc, char* argv[]) else if (cmd == "pack-textures") rc = cmdPackTextures(argc, argv); else if (cmd == "normal-from-height") rc = cmdNormalFromHeight(argc, argv); else if (cmd == "memory") rc = cmdMemory(argc, argv); + else if (cmd == "analyze") rc = cmdAnalyze(argc, argv); if (rc < 0) { err() << "Error: Unknown command '" << cmd << "'" << Qt::endl; @@ -3417,3 +3421,52 @@ int CLIPipeline::cmdMemory(int argc, char* argv[]) emitMemoryReport(report, fi, budgetSource, cmdArgs.jsonOutput); return report.overBudget() ? 1 : 0; } + +int CLIPipeline::cmdAnalyze(int argc, char* argv[]) +{ + // Parse: analyze [--json] + QString filePath; + bool jsonOutput = false; + + for (int i = 1; i < argc; ++i) { + const QString arg(argv[i]); + if (arg == "analyze" || arg == "--cli") continue; + if (arg == "--json") { jsonOutput = true; continue; } + if (!arg.startsWith("-") && filePath.isEmpty()) filePath = arg; + } + + if (filePath.isEmpty()) { + err() << "Error: No input file specified." << Qt::endl; + err() << "Usage: qtmesh analyze [--json]" << Qt::endl; + return 2; + } + + const QFileInfo fi(filePath); + if (!fi.exists()) { + err() << "Error: File not found: " << filePath << Qt::endl; + return 1; + } + + if (!initOgreHeadless()) return 1; + + SentryReporter::addBreadcrumb("cli.analyze", + QString("Analyze .%1").arg(fi.suffix())); + + MeshImporterExporter::importer({fi.absoluteFilePath()}, 0); + const auto& entities = Manager::getSingleton()->getEntities(); + if (entities.isEmpty()) { + err() << "Error: Failed to load file: " << filePath << Qt::endl; + return 1; + } + + const DrawCallReport report = DrawCallAnalyzer::analyze(entities); + + if (jsonOutput) { + QJsonObject obj = DrawCallAnalyzer::toJson(report); + obj["file"] = fi.fileName(); + cliWrite(QString::fromUtf8(QJsonDocument(obj).toJson(QJsonDocument::Indented))); + } else { + cliWrite(DrawCallAnalyzer::toText(report)); + } + return 0; +} diff --git a/src/CLIPipeline.h b/src/CLIPipeline.h index 66f13e96f..e0566b90c 100644 --- a/src/CLIPipeline.h +++ b/src/CLIPipeline.h @@ -88,6 +88,9 @@ class CLIPipeline { /// Phase 6 slice A: estimate GPU memory & VRAM for a mesh file /// (per-submesh + per-texture, optional --json, optional --budget). static int cmdMemory(int argc, char* argv[]); + /// Phase 6 slice B: analyze draw calls and surface merge opportunities + /// (per-material grouping, optional --json). + static int cmdAnalyze(int argc, char* argv[]); /// Map file extension to MeshImporterExporter format string. static QString formatForExtension(const QString& path); diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 2646400bd..01b02d79e 100755 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -80,6 +80,7 @@ MeshLodController.cpp TextureChannelPacker.cpp NormalMapGenerator.cpp MemoryEstimator.cpp +DrawCallAnalyzer.cpp MeshValidator.cpp AIChatManager.cpp WelcomeScreenController.cpp @@ -167,6 +168,7 @@ MeshLodController.h TextureChannelPacker.h NormalMapGenerator.h MemoryEstimator.h +DrawCallAnalyzer.h MeshValidator.h AIChatManager.h WelcomeScreenController.h diff --git a/src/DrawCallAnalyzer.cpp b/src/DrawCallAnalyzer.cpp new file mode 100644 index 000000000..ab87f4d0b --- /dev/null +++ b/src/DrawCallAnalyzer.cpp @@ -0,0 +1,201 @@ +#include "DrawCallAnalyzer.h" +#include "Manager.h" + +#include +#include +#include +#include + +#include +#include +#include + +namespace { + +QString materialNameOrPlaceholder(const Ogre::SubEntity* sub) +{ + if (!sub) return QStringLiteral("(none)"); + const auto mat = sub->getMaterial(); + if (!mat) return QStringLiteral("(none)"); + return QString::fromStdString(mat->getName()); +} + +} // namespace + +DrawCallReport DrawCallAnalyzer::analyze(const QList& entities) +{ + DrawCallReport report; + + // Hash. We accumulate in insertion order via a + // parallel list of keys so the output is stable across runs. + QHash byMaterial; + QStringList materialOrder; + + for (const Ogre::Entity* entity : entities) { + if (!entity) continue; + report.totalEntities++; + const QString entityName = QString::fromStdString(entity->getName()); + const unsigned int numSubs = entity->getNumSubEntities(); + report.totalSubmeshes += static_cast(numSubs); + + // Per-entity material set: a single entity that has 3 submeshes all + // bound to the same material still costs 3 draw calls (Ogre cannot + // batch them), but the merge-suggestion grouping should count the + // entity once per unique material it uses. + QSet entityMaterials; + for (unsigned int i = 0; i < numSubs; ++i) { + const Ogre::SubEntity* sub = entity->getSubEntity(i); + const QString matName = materialNameOrPlaceholder(sub); + report.totalDrawCalls++; // one draw call per SubEntity + + MaterialCluster& cluster = byMaterial[matName]; + if (!materialOrder.contains(matName)) { + materialOrder.append(matName); + cluster.materialName = matName; + } + cluster.submeshCount++; + entityMaterials.insert(matName); + } + + // Now record entity-per-material membership (once per material). + for (const QString& matName : entityMaterials) { + MaterialCluster& cluster = byMaterial[matName]; + if (!cluster.entityNames.contains(entityName)) + cluster.entityNames.append(entityName); + } + } + + report.uniqueMaterials = materialOrder.size(); + for (const QString& matName : materialOrder) + report.clusters.append(byMaterial.value(matName)); + + report.suggestions = buildSuggestions(report.clusters); + for (const MergeSuggestion& s : report.suggestions) + report.totalSavings += s.estimatedSavings; + + // Sort suggestions by savings (descending) so the most valuable merges + // surface first. Stable order on ties so output is deterministic. + std::stable_sort(report.suggestions.begin(), report.suggestions.end(), + [](const MergeSuggestion& a, const MergeSuggestion& b) { + return a.estimatedSavings > b.estimatedSavings; + }); + + report.potentialDrawCalls = report.totalDrawCalls - report.totalSavings; + return report; +} + +// LCOV_EXCL_START — exercised only when Ogre is initialised (skipped in unit tests). +DrawCallReport DrawCallAnalyzer::analyzeScene() +{ + QList entities; + Manager* mgr = Manager::getSingletonPtr(); + if (!mgr) return analyze(entities); + + for (Ogre::SceneNode* node : mgr->getSceneNodes()) { + if (!node) continue; + for (unsigned int i = 0; i < node->numAttachedObjects(); ++i) { + Ogre::MovableObject* obj = node->getAttachedObject(i); + if (!obj || obj->getMovableType() != "Entity") continue; + entities.append(static_cast(obj)); + } + } + return analyze(entities); +} +// LCOV_EXCL_STOP + +QList DrawCallAnalyzer::buildSuggestions( + const QList& clusters, int minSharedEntities) +{ + QList out; + for (const MaterialCluster& c : clusters) { + if (c.entityNames.size() < minSharedEntities) continue; + MergeSuggestion s; + s.materialName = c.materialName; + s.entityNames = c.entityNames; + s.estimatedSavings = c.mergeSavings(); + out.append(s); + } + return out; +} + +QJsonObject DrawCallAnalyzer::toJson(const DrawCallReport& report) +{ + QJsonObject obj; + QJsonObject totals; + totals["entities"] = report.totalEntities; + totals["submeshes"] = report.totalSubmeshes; + totals["drawCalls"] = report.totalDrawCalls; + totals["uniqueMaterials"] = report.uniqueMaterials; + totals["potentialDrawCalls"] = report.potentialDrawCalls; + totals["totalSavings"] = report.totalSavings; + obj["totals"] = totals; + + QJsonArray clusters; + for (const MaterialCluster& c : report.clusters) { + QJsonObject co; + co["material"] = c.materialName; + co["submeshCount"] = c.submeshCount; + QJsonArray names; + for (const QString& n : c.entityNames) names.append(n); + co["entities"] = names; + co["mergeSavings"] = c.mergeSavings(); + clusters.append(co); + } + obj["clusters"] = clusters; + + QJsonArray suggestions; + for (const MergeSuggestion& s : report.suggestions) { + QJsonObject so; + so["material"] = s.materialName; + so["estimatedSavings"] = s.estimatedSavings; + QJsonArray names; + for (const QString& n : s.entityNames) names.append(n); + so["entities"] = names; + suggestions.append(so); + } + obj["suggestions"] = suggestions; + + return obj; +} + +QString DrawCallAnalyzer::toText(const DrawCallReport& report) +{ + QString out; + QTextStream s(&out); + + s << "Draw Call Analysis\n"; + s << "==================\n\n"; + + s << "Entities: " << report.totalEntities << "\n"; + s << "Submeshes: " << report.totalSubmeshes << "\n"; + s << "Draw calls: " << report.totalDrawCalls << "\n"; + s << "Unique mats: " << report.uniqueMaterials << "\n"; + s << "After merges: " << report.potentialDrawCalls + << " (saves " << report.totalSavings << ")\n\n"; + + if (!report.clusters.isEmpty()) { + s << "Materials:\n"; + for (const MaterialCluster& c : report.clusters) { + s << " " << c.materialName + << " submeshes=" << c.submeshCount + << " entities=" << c.entityNames.size() << "\n"; + } + s << "\n"; + } + + if (!report.suggestions.isEmpty()) { + s << "Merge suggestions (saves >0 draw calls):\n"; + for (const MergeSuggestion& sug : report.suggestions) { + s << " " << sug.materialName + << " merge " << sug.entityNames.size() + << " entities → save " << sug.estimatedSavings + << " draw calls\n"; + for (const QString& n : sug.entityNames) + s << " - " << n << "\n"; + } + } else if (report.totalEntities > 0) { + s << "No merge opportunities (each material is used by at most one entity).\n"; + } + + return out; +} diff --git a/src/DrawCallAnalyzer.h b/src/DrawCallAnalyzer.h new file mode 100644 index 000000000..586c9a78a --- /dev/null +++ b/src/DrawCallAnalyzer.h @@ -0,0 +1,68 @@ +#ifndef DRAWCALLANALYZER_H +#define DRAWCALLANALYZER_H + +#include +#include +#include +#include + +namespace Ogre { + class Entity; +} + +// One row per (material, entity-list) cluster. A draw call is counted per +// SubEntity that uses the material — see DrawCallAnalyzer::analyze for the +// counting rule. +struct MaterialCluster { + QString materialName; + int submeshCount = 0; // total SubEntities bound to this material + QStringList entityNames; // names of entities that use this material + // Merge potential: every additional entity past the first is a draw call + // we could save by merging — provided the geometry is compatible. + int mergeSavings() const { return entityNames.isEmpty() ? 0 : entityNames.size() - 1; } +}; + +struct MergeSuggestion { + QString materialName; + QStringList entityNames; + int estimatedSavings = 0; // draw calls saved if the merge is performed +}; + +struct DrawCallReport { + int totalEntities = 0; + int totalSubmeshes = 0; // sum of SubEntity counts across entities + int totalDrawCalls = 0; // current estimated draw-call count + int uniqueMaterials = 0; + int potentialDrawCalls = 0; // draw calls if all viable merges happened + int totalSavings = 0; // totalDrawCalls - potentialDrawCalls + QList clusters; + QList suggestions; +}; + +// Pure-data analyzer. All methods are static and side-effect free. +class DrawCallAnalyzer { +public: + // Analyze a list of entities and produce a DrawCallReport. Null pointers + // in the input are skipped. The draw-call estimate counts one call per + // SubEntity (Ogre's coarsest granularity is the SubEntity render op). + static DrawCallReport analyze(const QList& entities); + + // Build a report for every entity currently attached under the scene + // root. Convenience wrapper. + static DrawCallReport analyzeScene(); + + // Serialize the report as JSON (CLI / MCP). + static QJsonObject toJson(const DrawCallReport& report); + + // Serialize the report as human-readable text (CLI default). + static QString toText(const DrawCallReport& report); + + // Suggestion-list filter: only clusters with `>= minSharedEntities` + // entities are reported, so the noise of single-instance materials + // does not crowd the output. Default 2 (two entities = at least one + // draw call saved by a merge). + static QList buildSuggestions( + const QList& clusters, int minSharedEntities = 2); +}; + +#endif // DRAWCALLANALYZER_H diff --git a/src/DrawCallAnalyzer_test.cpp b/src/DrawCallAnalyzer_test.cpp new file mode 100644 index 000000000..bc6e1a5c6 --- /dev/null +++ b/src/DrawCallAnalyzer_test.cpp @@ -0,0 +1,162 @@ +#include + +#include "DrawCallAnalyzer.h" + +#include +#include + +// All tests here exercise the pure-data path. They never touch Ogre, so they +// run on every CI build regardless of whether tryInitOgre() succeeded. + +namespace { +// Build a synthetic cluster directly to test buildSuggestions / +// toJson / toText without standing up an Ogre Entity. analyze() itself is +// covered by integration tests in a separate Ogre-backed suite. +MaterialCluster makeCluster(const QString& name, int submeshes, + const QStringList& entities) +{ + MaterialCluster c; + c.materialName = name; + c.submeshCount = submeshes; + c.entityNames = entities; + return c; +} +} // namespace + +TEST(DrawCallAnalyzerTest, BuildSuggestionsFiltersSingletons) +{ + QList clusters; + clusters << makeCluster("Mat.A", 1, {"E1"}) // 1 entity → skipped + << makeCluster("Mat.B", 3, {"E2","E3","E4"}) // 3 entities → kept + << makeCluster("Mat.C", 2, {"E5","E6"}); // 2 entities → kept + auto out = DrawCallAnalyzer::buildSuggestions(clusters); + ASSERT_EQ(2, out.size()); + EXPECT_EQ(QString("Mat.B"), out[0].materialName); + EXPECT_EQ(2, out[0].estimatedSavings); // 3 entities → save 2 calls + EXPECT_EQ(QString("Mat.C"), out[1].materialName); + EXPECT_EQ(1, out[1].estimatedSavings); +} + +TEST(DrawCallAnalyzerTest, BuildSuggestionsRespectsCustomThreshold) +{ + QList clusters; + clusters << makeCluster("Mat.A", 1, {"E1"}) + << makeCluster("Mat.B", 3, {"E2","E3","E4"}); + // Threshold 4 → nothing qualifies + auto out = DrawCallAnalyzer::buildSuggestions(clusters, 4); + EXPECT_TRUE(out.isEmpty()); +} + +TEST(DrawCallAnalyzerTest, BuildSuggestionsEmptyClusters) +{ + EXPECT_TRUE(DrawCallAnalyzer::buildSuggestions({}).isEmpty()); +} + +TEST(DrawCallAnalyzerTest, MergeSavingsArithmetic) +{ + MaterialCluster empty; + EXPECT_EQ(0, empty.mergeSavings()); + + MaterialCluster single = makeCluster("M", 1, {"E1"}); + EXPECT_EQ(0, single.mergeSavings()); + + MaterialCluster five = makeCluster("M", 5, {"E1","E2","E3","E4","E5"}); + EXPECT_EQ(4, five.mergeSavings()); +} + +TEST(DrawCallAnalyzerTest, JsonEmptyReport) +{ + DrawCallReport empty; + QJsonObject obj = DrawCallAnalyzer::toJson(empty); + EXPECT_TRUE(obj.contains("totals")); + EXPECT_TRUE(obj.contains("clusters")); + EXPECT_TRUE(obj.contains("suggestions")); + EXPECT_EQ(0, obj["totals"].toObject()["entities"].toInt()); + EXPECT_EQ(0, obj["clusters"].toArray().size()); + EXPECT_EQ(0, obj["suggestions"].toArray().size()); +} + +TEST(DrawCallAnalyzerTest, JsonShapeWithSuggestion) +{ + DrawCallReport report; + report.totalEntities = 4; + report.totalSubmeshes = 4; + report.totalDrawCalls = 4; + report.uniqueMaterials = 2; + report.totalSavings = 2; + report.potentialDrawCalls = 2; + report.clusters << makeCluster("Mat.A", 3, {"E1","E2","E3"}) + << makeCluster("Mat.B", 1, {"E4"}); + report.suggestions = DrawCallAnalyzer::buildSuggestions(report.clusters); + + QJsonObject obj = DrawCallAnalyzer::toJson(report); + EXPECT_EQ(4, obj["totals"].toObject()["drawCalls"].toInt()); + EXPECT_EQ(2, obj["totals"].toObject()["totalSavings"].toInt()); + EXPECT_EQ(2, obj["clusters"].toArray().size()); + EXPECT_EQ(1, obj["suggestions"].toArray().size()); + + QJsonObject sug = obj["suggestions"].toArray()[0].toObject(); + EXPECT_EQ(QString("Mat.A"), sug["material"].toString()); + EXPECT_EQ(2, sug["estimatedSavings"].toInt()); + EXPECT_EQ(3, sug["entities"].toArray().size()); +} + +TEST(DrawCallAnalyzerTest, TextHasHeaderAndMaterials) +{ + DrawCallReport report; + report.totalEntities = 2; + report.totalSubmeshes = 2; + report.totalDrawCalls = 2; + report.uniqueMaterials = 1; + report.clusters << makeCluster("Mat.A", 2, {"E1","E2"}); + report.suggestions = DrawCallAnalyzer::buildSuggestions(report.clusters); + report.totalSavings = 1; + report.potentialDrawCalls = 1; + + QString text = DrawCallAnalyzer::toText(report); + EXPECT_TRUE(text.contains("Draw Call Analysis")); + EXPECT_TRUE(text.contains("Mat.A")); + EXPECT_TRUE(text.contains("Merge suggestions")); + EXPECT_TRUE(text.contains("E1")); + EXPECT_TRUE(text.contains("E2")); +} + +TEST(DrawCallAnalyzerTest, TextHandlesNoMergeOpportunities) +{ + DrawCallReport report; + report.totalEntities = 2; + report.totalSubmeshes = 2; + report.totalDrawCalls = 2; + report.uniqueMaterials = 2; + report.clusters << makeCluster("Mat.A", 1, {"E1"}) + << makeCluster("Mat.B", 1, {"E2"}); + // No suggestions because each cluster has only 1 entity + QString text = DrawCallAnalyzer::toText(report); + EXPECT_TRUE(text.contains("No merge opportunities")); +} + +TEST(DrawCallAnalyzerTest, TextHandlesEmpty) +{ + DrawCallReport empty; + QString text = DrawCallAnalyzer::toText(empty); + EXPECT_TRUE(text.contains("Draw Call Analysis")); + EXPECT_FALSE(text.contains("Merge suggestions")); +} + +TEST(DrawCallAnalyzerTest, AnalyzeNullEntityList) +{ + QList empty; + DrawCallReport report = DrawCallAnalyzer::analyze(empty); + EXPECT_EQ(0, report.totalEntities); + EXPECT_EQ(0, report.totalDrawCalls); + EXPECT_TRUE(report.suggestions.isEmpty()); +} + +TEST(DrawCallAnalyzerTest, AnalyzeListOfNullPointers) +{ + QList nulls; + nulls << nullptr << nullptr; + DrawCallReport report = DrawCallAnalyzer::analyze(nulls); + EXPECT_EQ(0, report.totalEntities); // nulls skipped + EXPECT_EQ(0, report.totalDrawCalls); +} diff --git a/src/MCPServer.cpp b/src/MCPServer.cpp index c148cba0b..48dd7c271 100644 --- a/src/MCPServer.cpp +++ b/src/MCPServer.cpp @@ -17,6 +17,7 @@ #include "MeshValidator.h" #include "MeshLodController.h" #include "MemoryEstimator.h" +#include "DrawCallAnalyzer.h" #include #include #include @@ -432,6 +433,7 @@ const QMap& MCPServer::toolHandlers() {QStringLiteral("remove_lods"), &MCPServer::toolRemoveLods}, {QStringLiteral("get_lod_info"), &MCPServer::toolGetLodInfo}, {QStringLiteral("get_memory_usage"), &MCPServer::toolGetMemoryUsage}, + {QStringLiteral("analyze_draw_calls"), &MCPServer::toolAnalyzeDrawCalls}, {QStringLiteral("list_files"), &MCPServer::toolListFiles}, {QStringLiteral("search_files"), &MCPServer::toolSearchFiles}, {QStringLiteral("read_file"), &MCPServer::toolReadFile}, @@ -2768,6 +2770,26 @@ QJsonObject MCPServer::toolGetMemoryUsage(const QJsonObject &args) } } +// NOSONAR(cpp:S5817) — ToolHandler is a non-const member-fn pointer (matching +// every other tool method in this class); marking just this one const would +// break the registry signature in MCPServer.h. +QJsonObject MCPServer::toolAnalyzeDrawCalls(const QJsonObject &args) +{ + Q_UNUSED(args); + try { + if (const Manager* mgr = Manager::getSingletonPtr(); !mgr) + return makeErrorResult("Error: Manager not available"); + + const DrawCallReport report = DrawCallAnalyzer::analyzeScene(); + QJsonObject result = makeSuccessResult(DrawCallAnalyzer::toText(report)); + result["drawCalls"] = DrawCallAnalyzer::toJson(report); + return result; + } catch (Ogre::Exception& e) { + return makeErrorResult( + QString("Ogre error: %1").arg(QString::fromStdString(e.getFullDescription()))); + } +} + // Helper methods QJsonObject MCPServer::toolListFiles(const QJsonObject &args) @@ -4120,6 +4142,20 @@ QJsonArray MCPServer::buildToolsList() ); } + // analyze_draw_calls + { + appendTool( + "analyze_draw_calls", + "Estimate scene draw-call cost and surface merge opportunities. Groups every " + "loaded entity by the materials its submeshes use, counts one draw call per " + "SubEntity, and lists the materials shared by multiple entities (the merge " + "candidates that would reduce draw-call count). The response includes a " + "human-readable summary in 'content' and a structured 'drawCalls' object with " + "totals, clusters, and ranked suggestions for machine consumers.", + QJsonObject() + ); + } + // delete_entity { QJsonObject props; diff --git a/src/MCPServer.h b/src/MCPServer.h index 3d9defd4f..597acb913 100644 --- a/src/MCPServer.h +++ b/src/MCPServer.h @@ -166,6 +166,7 @@ private slots: QJsonObject toolRemoveLods(const QJsonObject &args); QJsonObject toolGetLodInfo(const QJsonObject &args); QJsonObject toolGetMemoryUsage(const QJsonObject &args); + QJsonObject toolAnalyzeDrawCalls(const QJsonObject &args); QJsonObject toolListFiles(const QJsonObject &args); QJsonObject toolSearchFiles(const QJsonObject &args); QJsonObject toolReadFile(const QJsonObject &args); diff --git a/src/MeshInfoOverlay.cpp b/src/MeshInfoOverlay.cpp index 5d762fe61..cdc1a1cf3 100644 --- a/src/MeshInfoOverlay.cpp +++ b/src/MeshInfoOverlay.cpp @@ -5,6 +5,7 @@ #include "OgreWidget.h" #include "CLIPipeline.h" #include "MemoryEstimator.h" +#include "DrawCallAnalyzer.h" #include "mainwindow.h" #include @@ -165,6 +166,18 @@ QString MeshInfoOverlay::formatStats(const QList& entities, bool text += QString("\nGPU: %1").arg(MemoryEstimator::formatBytes(totalGpuBytes)); } + // Phase 6 slice B: draw-call cost and merge potential. We deliberately + // run analyze() over the same `valid` list so the overlay's draw-call + // count stays in sync with whatever the user has selected. + const DrawCallReport drawReport = DrawCallAnalyzer::analyze(valid); + if (drawReport.totalDrawCalls > 0) { + text += QString("\nDraws: %1").arg(drawReport.totalDrawCalls); + if (drawReport.totalSavings > 0) { + text += QString(" (save %1 by merging)") + .arg(drawReport.totalSavings); + } + } + return text; } diff --git a/src/MeshInfoOverlay_test.cpp b/src/MeshInfoOverlay_test.cpp index 3aefd664b..3a11cd56d 100644 --- a/src/MeshInfoOverlay_test.cpp +++ b/src/MeshInfoOverlay_test.cpp @@ -256,6 +256,30 @@ TEST_F(MeshInfoOverlayIntegrationTest, FormatStatsWithSkeleton) Manager::getSingleton()->getSceneMgr()->destroySceneNode(node); } +TEST_F(MeshInfoOverlayIntegrationTest, FormatStatsIncludesDrawCalls) +{ + ASSERT_TRUE(canLoadMeshFiles()) << "mesh loading requires GL (Xvfb in CI)"; + auto meshPtr = createInMemoryTriangleMesh("MeshInfoDrawCallMesh"); + ASSERT_TRUE(meshPtr); + + auto* sceneMgr = Manager::getSingleton()->getSceneMgr(); + auto* node = sceneMgr->getRootSceneNode()->createChildSceneNode("MeshInfoDrawCallNode"); + auto* entity = sceneMgr->createEntity("MeshInfoDrawCallEntity", meshPtr); + node->attachObject(entity); + + QList entities; + entities << entity; + + QString result = MeshInfoOverlay::formatStats(entities, false); + // Phase 6 slice B: draw-call line appears when at least one SubEntity exists. + EXPECT_TRUE(result.contains("Draws:")) + << "Result: " << result.toStdString(); + + node->detachObject(entity); + sceneMgr->destroyEntity(entity); + sceneMgr->destroySceneNode(node); +} + TEST_F(MeshInfoOverlayIntegrationTest, FormatStatsIncludesGpuBytes) { ASSERT_TRUE(canLoadMeshFiles()) << "mesh loading requires GL (Xvfb in CI)"; diff --git a/src/main.cpp b/src/main.cpp index ce4d4f51b..29297f7e1 100755 --- a/src/main.cpp +++ b/src/main.cpp @@ -88,7 +88,8 @@ int main(int argc, char *argv[]) if (arg == "info" || arg == "fix" || arg == "convert" || arg == "anim" || arg == "validate" || arg == "lod" || arg == "pose" || arg == "scan" || arg == "material" || arg == "pack-textures" - || arg == "normal-from-height" || arg == "memory") + || arg == "normal-from-height" || arg == "memory" + || arg == "analyze") cliMode = true; break; // first non-flag arg determines mode } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index e75f6fd7c..24c134751 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -89,6 +89,7 @@ if(BUILD_TESTS) ${CMAKE_CURRENT_SOURCE_DIR}/../src/TextureChannelPacker.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../src/NormalMapGenerator.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../src/MemoryEstimator.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/../src/DrawCallAnalyzer.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../src/MeshLodController.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../src/MeshValidator.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../src/AIChatManager.cpp From cc88b7ebf2971656bec13dc607f1cb31a0ffb0dd Mon Sep 17 00:00:00 2001 From: Fernando Date: Tue, 12 May 2026 12:09:30 -0400 Subject: [PATCH 2/3] refactor(perf): address CodeRabbit + SonarCloud findings on slice B (#497) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Quality Gate already passed; this clears the open issues: CodeRabbit - Added a `file.import` Sentry breadcrumb in cmdAnalyze immediately before the import call, matching the project convention for I/O operations (CLAUDE.md guidance). - Text output now prepends `File: ` so it mirrors the JSON's `file` field — same correlation in CI logs. - Added explicit #include in DrawCallAnalyzer.cpp instead of relying on transitive includes. SonarCloud - S5276 (size_t → unsigned int): switched the SubEntity iteration in analyze() to size_t to match Ogre::Entity::getNumSubEntities()'s return type. - S5276 (qsizetype → int): cast Container::size() at the assignment site in mergeSavings() and the uniqueMaterials accumulator. - S5350: const Ogre::SceneNode* in the analyzeScene scene-graph walk. - S6004: drawReport now declared inside an if-init-statement in MeshInfoOverlay::formatStats. The remaining MCPServer.cpp:2776 S5817 (`should be const`) is the same intentional non-const tool method covered by the existing NOSONAR comment in slice A. MCPServer.cpp:3394 S1116 is a pre-existing Q_UNUSED(args); in toolGetPivotMode that Sonar mis-attributed to the new code window — not touched. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/CLIPipeline.cpp | 3 +++ src/DrawCallAnalyzer.cpp | 9 +++++---- src/DrawCallAnalyzer.h | 4 +++- src/MeshInfoOverlay.cpp | 4 ++-- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/CLIPipeline.cpp b/src/CLIPipeline.cpp index 56a1b20de..844a89559 100644 --- a/src/CLIPipeline.cpp +++ b/src/CLIPipeline.cpp @@ -3451,6 +3451,8 @@ int CLIPipeline::cmdAnalyze(int argc, char* argv[]) SentryReporter::addBreadcrumb("cli.analyze", QString("Analyze .%1").arg(fi.suffix())); + SentryReporter::addBreadcrumb("file.import", + QString("Importing %1").arg(fi.absoluteFilePath())); MeshImporterExporter::importer({fi.absoluteFilePath()}, 0); const auto& entities = Manager::getSingleton()->getEntities(); @@ -3466,6 +3468,7 @@ int CLIPipeline::cmdAnalyze(int argc, char* argv[]) obj["file"] = fi.fileName(); cliWrite(QString::fromUtf8(QJsonDocument(obj).toJson(QJsonDocument::Indented))); } else { + cliWrite(QString("File: %1\n").arg(fi.fileName())); cliWrite(DrawCallAnalyzer::toText(report)); } return 0; diff --git a/src/DrawCallAnalyzer.cpp b/src/DrawCallAnalyzer.cpp index ab87f4d0b..222a08a15 100644 --- a/src/DrawCallAnalyzer.cpp +++ b/src/DrawCallAnalyzer.cpp @@ -8,6 +8,7 @@ #include #include +#include #include namespace { @@ -35,7 +36,7 @@ DrawCallReport DrawCallAnalyzer::analyze(const QList& entities) if (!entity) continue; report.totalEntities++; const QString entityName = QString::fromStdString(entity->getName()); - const unsigned int numSubs = entity->getNumSubEntities(); + const size_t numSubs = entity->getNumSubEntities(); report.totalSubmeshes += static_cast(numSubs); // Per-entity material set: a single entity that has 3 submeshes all @@ -43,7 +44,7 @@ DrawCallReport DrawCallAnalyzer::analyze(const QList& entities) // batch them), but the merge-suggestion grouping should count the // entity once per unique material it uses. QSet entityMaterials; - for (unsigned int i = 0; i < numSubs; ++i) { + for (size_t i = 0; i < numSubs; ++i) { const Ogre::SubEntity* sub = entity->getSubEntity(i); const QString matName = materialNameOrPlaceholder(sub); report.totalDrawCalls++; // one draw call per SubEntity @@ -65,7 +66,7 @@ DrawCallReport DrawCallAnalyzer::analyze(const QList& entities) } } - report.uniqueMaterials = materialOrder.size(); + report.uniqueMaterials = static_cast(materialOrder.size()); for (const QString& matName : materialOrder) report.clusters.append(byMaterial.value(matName)); @@ -91,7 +92,7 @@ DrawCallReport DrawCallAnalyzer::analyzeScene() Manager* mgr = Manager::getSingletonPtr(); if (!mgr) return analyze(entities); - for (Ogre::SceneNode* node : mgr->getSceneNodes()) { + for (const Ogre::SceneNode* node : mgr->getSceneNodes()) { if (!node) continue; for (unsigned int i = 0; i < node->numAttachedObjects(); ++i) { Ogre::MovableObject* obj = node->getAttachedObject(i); diff --git a/src/DrawCallAnalyzer.h b/src/DrawCallAnalyzer.h index 586c9a78a..7869cdc6d 100644 --- a/src/DrawCallAnalyzer.h +++ b/src/DrawCallAnalyzer.h @@ -19,7 +19,9 @@ struct MaterialCluster { QStringList entityNames; // names of entities that use this material // Merge potential: every additional entity past the first is a draw call // we could save by merging — provided the geometry is compatible. - int mergeSavings() const { return entityNames.isEmpty() ? 0 : entityNames.size() - 1; } + int mergeSavings() const { + return entityNames.isEmpty() ? 0 : static_cast(entityNames.size()) - 1; + } }; struct MergeSuggestion { diff --git a/src/MeshInfoOverlay.cpp b/src/MeshInfoOverlay.cpp index cdc1a1cf3..d0f37ee3b 100644 --- a/src/MeshInfoOverlay.cpp +++ b/src/MeshInfoOverlay.cpp @@ -169,8 +169,8 @@ QString MeshInfoOverlay::formatStats(const QList& entities, bool // Phase 6 slice B: draw-call cost and merge potential. We deliberately // run analyze() over the same `valid` list so the overlay's draw-call // count stays in sync with whatever the user has selected. - const DrawCallReport drawReport = DrawCallAnalyzer::analyze(valid); - if (drawReport.totalDrawCalls > 0) { + if (const DrawCallReport drawReport = DrawCallAnalyzer::analyze(valid); + drawReport.totalDrawCalls > 0) { text += QString("\nDraws: %1").arg(drawReport.totalDrawCalls); if (drawReport.totalSavings > 0) { text += QString(" (save %1 by merging)") From 552a811e6d0650d547e0853b209ba137ae8d3858 Mon Sep 17 00:00:00 2001 From: Fernando Date: Tue, 12 May 2026 14:10:59 -0400 Subject: [PATCH 3/3] feat(validate): per-check feedback rows + draw-call & memory analyses (#497) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User feedback on the slice B PR: "Run Validation" today only performs the three geometry/UV checks and reports a bare "No issues found." line when they all pass, hiding the scope of the analysis from the user. This commit: - Folds the new draw-call (slice B) and GPU-memory (slice A) analyses into the validator's flow, so a single click reports everything we can derive without exporting the mesh. - Replaces the single-line success message with a per-dimension checklist. Every check now emits its own row in the issues list — errors and warnings stay as before, passing checks become "ok" rows ("Geometry: 1,234 triangle(s) across 3 submesh(es), no degenerate faces"), and neutral observations become a new "info" row type ("Draws: 5 — save 3 by merging", "GPU: ~245 KB of vertex + index buffers"). - QML icon palette extended for the new "info" type (ℹ blue). Why include this on slice B's branch instead of a separate PR: the validator change is what makes the slice B feature land in the single place users already know to click for asset health, and it shares the analyzers introduced in slice B (DrawCallAnalyzer) and slice A (MemoryEstimator). Tests: refreshed two MeshValidatorTest cases to assert the new checklist shape (geometry / UVs / draws / GPU rows present, none of them error/warning on a clean mesh). Existing DoValidateDetectsDegeneratesAndUvProblems narrowed its substring match to the still-present words ("non-finite", "extreme values") to match the new "Geometry: " / "UVs: " row prefixes. Co-Authored-By: Claude Opus 4.7 (1M context) --- qml/PropertiesPanel.qml | 2 + src/MeshValidator.cpp | 125 +++++++++++++++++++++++++++++++------ src/MeshValidator_test.cpp | 46 +++++++++++--- 3 files changed, 145 insertions(+), 28 deletions(-) diff --git a/qml/PropertiesPanel.qml b/qml/PropertiesPanel.qml index 4bf5ced88..ca43fe051 100644 --- a/qml/PropertiesPanel.qml +++ b/qml/PropertiesPanel.qml @@ -2535,9 +2535,11 @@ Rectangle { Text { text: modelData.type === "error" ? "\u2718" : modelData.type === "warning" ? "\u26A0" + : modelData.type === "info" ? "\u2139" : "\u2714" color: modelData.type === "error" ? "#e05050" : modelData.type === "warning" ? "#e0a030" + : modelData.type === "info" ? "#5090d0" : "#60c060" font.pixelSize: 13 anchors.verticalCenter: parent.verticalCenter diff --git a/src/MeshValidator.cpp b/src/MeshValidator.cpp index 1d48b8909..dcc1eae18 100644 --- a/src/MeshValidator.cpp +++ b/src/MeshValidator.cpp @@ -3,10 +3,13 @@ #include "SelectionSet.h" #include "MeshImporterExporter.h" #include "SentryReporter.h" +#include "DrawCallAnalyzer.h" +#include "MemoryEstimator.h" #include #include #include #include +#include #include namespace { @@ -156,6 +159,11 @@ void MeshValidator::doValidate() int totalDegenerates = 0; int totalNonFiniteUV = 0; int totalOutOfRangeUV = 0; + int totalTris = 0; + int totalVerts = 0; + int totalSubmeshes = 0; + int meshesWithUVs = 0; + int meshesWithoutUVs = 0; for (Ogre::Entity* entity : targets) { Ogre::MeshPtr mesh = entity->getMesh(); @@ -167,11 +175,18 @@ void MeshValidator::doValidate() Ogre::IndexData* id = sub->indexData; if (!vd || !id || !id->indexBuffer) continue; + ++totalSubmeshes; + totalVerts += static_cast(vd->vertexCount); + totalTris += static_cast(id->indexCount / 3); + const Ogre::VertexElement* posElem = vd->vertexDeclaration->findElementBySemantic(Ogre::VES_POSITION); const Ogre::VertexElement* texElem = vd->vertexDeclaration->findElementBySemantic(Ogre::VES_TEXTURE_COORDINATES); + if (texElem) ++meshesWithUVs; + else ++meshesWithoutUVs; + // ---- lock position buffer ---- Ogre::HardwareVertexBufferSharedPtr vbuf; const unsigned char* vdata = nullptr; @@ -255,39 +270,113 @@ void MeshValidator::doValidate() } } - // ---- build issues list ---- + // ---- build the checklist ---- + // + // Every checked dimension produces a row: errors/warnings when something is + // wrong, an "ok" row when the check passed, an "info" row for neutral + // observations (draw calls / memory — neither pass nor fail, just data). + // This way the user always sees what was actually analyzed rather than a + // bare "No issues found." that hides the scope of the validation. + QLocale locale; + + // 1. Geometry — degenerate triangles if (totalDegenerates > 0) { QVariantMap issue; issue["type"] = "error"; - issue["description"] = QString("%1 degenerate triangle(s) — zero-area faces").arg(totalDegenerates); + issue["description"] = QString("Geometry: %1 degenerate triangle(s) — zero-area faces") + .arg(totalDegenerates); issue["count"] = totalDegenerates; issue["fixable"] = true; m_issues.append(issue); + } else { + QVariantMap issue; + issue["type"] = "ok"; + issue["description"] = QString("Geometry: %1 triangle(s) across %2 submesh(es), no degenerate faces") + .arg(locale.toString(totalTris)) + .arg(totalSubmeshes); + issue["count"] = 0; + issue["fixable"] = false; + m_issues.append(issue); } - if (totalNonFiniteUV > 0) { + + // 2. UVs — finite / range. Skip the check entirely when the mesh has no UVs. + if (meshesWithUVs == 0 && meshesWithoutUVs > 0) { QVariantMap issue; - issue["type"] = "error"; - issue["description"] = QString("%1 vertex(es) with non-finite UV coordinates (NaN/Inf)").arg(totalNonFiniteUV); - issue["count"] = totalNonFiniteUV; - issue["fixable"] = true; + issue["type"] = "info"; + issue["description"] = QStringLiteral("UVs: no texture coordinates on this mesh — skipped"); + issue["count"] = 0; + issue["fixable"] = false; m_issues.append(issue); + } else { + if (totalNonFiniteUV > 0) { + QVariantMap issue; + issue["type"] = "error"; + issue["description"] = QString("UVs: %1 vertex(es) with non-finite coordinates (NaN/Inf)") + .arg(totalNonFiniteUV); + issue["count"] = totalNonFiniteUV; + issue["fixable"] = true; + m_issues.append(issue); + } + if (totalOutOfRangeUV > 0) { + QVariantMap issue; + issue["type"] = "warning"; + issue["description"] = QString("UVs: %1 vertex(es) with extreme values (outside ±10)") + .arg(totalOutOfRangeUV); + issue["count"] = totalOutOfRangeUV; + issue["fixable"] = false; + m_issues.append(issue); + } + if (totalNonFiniteUV == 0 && totalOutOfRangeUV == 0) { + QVariantMap issue; + issue["type"] = "ok"; + issue["description"] = QStringLiteral("UVs: all finite, all within ±10 range"); + issue["count"] = 0; + issue["fixable"] = false; + m_issues.append(issue); + } } - if (totalOutOfRangeUV > 0) { + + // 3. Draw-call analysis (Phase 6 slice B). Neutral observation: flag merge + // opportunities as "info" so they show up in the report without looking + // like a failure. + const DrawCallReport drawReport = DrawCallAnalyzer::analyze(targets); + if (drawReport.totalDrawCalls > 0) { QVariantMap issue; - issue["type"] = "warning"; - issue["description"] = QString("%1 vertex(es) with extreme UV values (outside ±10)").arg(totalOutOfRangeUV); - issue["count"] = totalOutOfRangeUV; + issue["count"] = drawReport.totalDrawCalls; issue["fixable"] = false; + if (drawReport.totalSavings > 0) { + issue["type"] = "info"; + issue["description"] = QString("Draws: %1 across %2 material(s) — save %3 by merging " + "entities that share a material") + .arg(drawReport.totalDrawCalls) + .arg(drawReport.uniqueMaterials) + .arg(drawReport.totalSavings); + } else { + issue["type"] = "ok"; + issue["description"] = QString("Draws: %1 across %2 material(s) — no merge opportunities") + .arg(drawReport.totalDrawCalls) + .arg(drawReport.uniqueMaterials); + } m_issues.append(issue); } - if (m_issues.isEmpty()) { - QVariantMap ok; - ok["type"] = "ok"; - ok["description"] = "No issues found."; - ok["count"] = 0; - ok["fixable"] = false; - m_issues.append(ok); + // 4. Memory / VRAM (Phase 6 slice A). Always info — no pass/fail without + // a configured budget; we just report what the asset costs on the GPU. + quint64 meshBytes = 0; + for (Ogre::Entity* entity : targets) { + const MeshMemoryEstimate est = MemoryEstimator::estimateEntity(entity); + meshBytes += est.totalBytes(); + } + if (meshBytes > 0) { + QVariantMap issue; + issue["type"] = "info"; + issue["description"] = QString("GPU: ~%1 of vertex + index buffers (%2 vert / %3 tri)") + .arg(MemoryEstimator::formatBytes(meshBytes)) + .arg(locale.toString(totalVerts)) + .arg(locale.toString(totalTris)); + issue["count"] = 0; + issue["fixable"] = false; + m_issues.append(issue); } m_validated = true; diff --git a/src/MeshValidator_test.cpp b/src/MeshValidator_test.cpp index f94e5b8a7..4b6c3804d 100644 --- a/src/MeshValidator_test.cpp +++ b/src/MeshValidator_test.cpp @@ -241,8 +241,12 @@ TEST_F(MeshValidatorTest, HasSelectionTrueWhenSubEntitySelected) validator->doValidate(); EXPECT_TRUE(validator->validated()); const QVariantList issues = validator->issues(); - ASSERT_EQ(issues.size(), 1); + // Phase 6: validation now emits a checklist (geometry/UVs/draws/memory) + // instead of a single "No issues found." row. At minimum the geometry + // row must be present and be "ok" for this valid mesh. + ASSERT_GE(issues.size(), 1); EXPECT_EQ(issues.first().toMap().value("type").toString(), QStringLiteral("ok")); + EXPECT_TRUE(issues.first().toMap().value("description").toString().startsWith("Geometry:")); } TEST_F(MeshValidatorTest, ValidateWithoutSelectionDoesNotEnterPending) @@ -266,7 +270,7 @@ TEST_F(MeshValidatorTest, DoValidateWithoutSelectionKeepsStateUnvalidated) EXPECT_FALSE(validator->hasFixableIssues()); } -TEST_F(MeshValidatorTest, DoValidateValidMeshReturnsOkIssue) +TEST_F(MeshValidatorTest, DoValidateValidMeshReportsChecklist) { ASSERT_TRUE(canLoadMeshFiles()); @@ -281,14 +285,35 @@ TEST_F(MeshValidatorTest, DoValidateValidMeshReturnsOkIssue) EXPECT_FALSE(validator->validating()); EXPECT_FALSE(validator->hasFixableIssues()); + // Phase 6: the validator now emits a per-dimension checklist for a valid + // mesh — at minimum a Geometry-ok row, a UVs row, plus the new Draws/GPU + // info rows from slices A+B. The user sees what was actually checked + // instead of a bare "No issues found." line. const QVariantList issues = validator->issues(); - ASSERT_EQ(issues.size(), 1); + ASSERT_GE(issues.size(), 2); - const QVariantMap issue = issues.first().toMap(); - EXPECT_EQ(issue.value("type").toString(), QStringLiteral("ok")); - EXPECT_EQ(issue.value("description").toString(), QStringLiteral("No issues found.")); - EXPECT_EQ(issue.value("count").toInt(), 0); - EXPECT_FALSE(issue.value("fixable").toBool()); + bool sawGeometryOk = false; + bool sawUvsOk = false; + bool sawDraws = false; + bool sawGpu = false; + for (const QVariant& issueVariant : issues) { + const QVariantMap issue = issueVariant.toMap(); + const QString type = issue.value("type").toString(); + const QString description = issue.value("description").toString(); + + // No row may be an error or warning on a clean mesh. + EXPECT_NE(type, QStringLiteral("error")) << description.toStdString(); + EXPECT_NE(type, QStringLiteral("warning")) << description.toStdString(); + + if (description.startsWith("Geometry:") && type == "ok") sawGeometryOk = true; + if (description.startsWith("UVs:") && type == "ok") sawUvsOk = true; + if (description.startsWith("Draws:")) sawDraws = true; + if (description.startsWith("GPU:")) sawGpu = true; + } + EXPECT_TRUE(sawGeometryOk); + EXPECT_TRUE(sawUvsOk); + EXPECT_TRUE(sawDraws); + EXPECT_TRUE(sawGpu); } TEST_F(MeshValidatorTest, DoValidateDetectsDegeneratesAndUvProblems) @@ -311,9 +336,10 @@ TEST_F(MeshValidatorTest, DoValidateDetectsDegeneratesAndUvProblems) for (const QVariant& issueVariant : validator->issues()) { const QVariantMap issue = issueVariant.toMap(); const QString description = issue.value("description").toString(); + // Phase 6: rows now have "Geometry:" / "UVs:" prefixes. sawDegenerate |= description.contains("degenerate triangle"); - sawNonFinite |= description.contains("non-finite UV"); - sawExtremeUv |= description.contains("extreme UV values"); + sawNonFinite |= description.contains("non-finite"); + sawExtremeUv |= description.contains("extreme values"); } EXPECT_TRUE(sawDegenerate);