Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion qml/BottomContextPanel.qml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ Rectangle {
property var materialEditorAction: null
readonly property string currentSummaryObjectName: summaryLoader.item ? summaryLoader.item.objectName : ""

// Count error+warning rows (the "must fix" tier). Info rows are
// perf observations and counted separately via suggestionSummary().
function issueSummary() {
if (!MeshValidator.validated)
return "Not run"
Expand All @@ -29,6 +31,23 @@ Rectangle {
return String(n)
}

// Phase 6: count info rows that carry a one-click fix (e.g. vertex
// cache reorder). These aren't errors but the user has an in-UI
// action available, so surface them as "Suggestions" in the panel.
function suggestionSummary() {
if (!MeshValidator.validated)
return "Not run"
var n = 0
for (var i = 0; i < MeshValidator.issues.length; ++i) {
var item = MeshValidator.issues[i]
var typ = item.type !== undefined ? item.type : item["type"]
var fixable = item.fixable === true
if (typ === "info" && fixable)
++n
}
return String(n)
}

function fileSummary() {
return String(AssetBrowserController.files.length)
}
Expand Down Expand Up @@ -196,7 +215,10 @@ Rectangle {

SummaryText { label: "Selection"; value: MeshValidator.hasSelection ? PropertiesPanelController.selectionName : "None" }
SummaryText { label: "Findings"; value: root.issueSummary() }
SummaryText { label: "Fixable"; value: MeshValidator.hasFixableIssues ? "Yes" : "No" }
SummaryText { label: "Suggestions"; value: root.suggestionSummary() }
SummaryText { label: "Fixable"
value: (MeshValidator.hasFixableIssues
|| MeshValidator.hasCacheOptimization) ? "Yes" : "No" }
SummaryText { label: "Status"; value: MeshValidator.validating ? "Running" : (MeshValidator.validated ? "Ready" : "Idle") }
Item { Layout.fillWidth: true }
}
Expand Down
22 changes: 22 additions & 0 deletions qml/PropertiesPanel.qml
Original file line number Diff line number Diff line change
Expand Up @@ -2568,6 +2568,21 @@ Rectangle {
}
}

// Optimize Vertex Cache button (Phase 6 slice C). Distinct from
// "Fix All" — this only mutates index ordering, never geometry.
Rectangle {
width: parent.width - 16; height: 28; radius: 3
visible: MeshValidator.hasCacheOptimization
color: cacheMouse.pressed ? Qt.darker("#5090d0", 1.2)
: cacheMouse.containsMouse ? Qt.lighter("#5090d0", 1.2)
: "#5090d0"
Text { anchors.centerIn: parent; text: "Optimize Vertex Cache"; color: "white"; font.pixelSize: 11 }
MouseArea {
id: cacheMouse; anchors.fill: parent; hoverEnabled: true
onClicked: MeshValidator.optimizeVertexCache()
}
}

// Fix feedback
Text {
id: fixFeedback
Expand All @@ -2584,6 +2599,13 @@ Rectangle {
fixFeedback.color = "#c06060"
fixFeedback.text = msg
}
// Clear the fix feedback whenever the validation result is
// invalidated — selection-change, or any other reason the
// validator resets `validated` to false.
function onIssuesChanged() {
if (!MeshValidator.validated)
fixFeedback.text = ""
}
}
}
}
Expand Down
124 changes: 124 additions & 0 deletions src/CLIPipeline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "NormalMapGenerator.h"
#include "MemoryEstimator.h"
#include "DrawCallAnalyzer.h"
#include "VertexCacheOptimizer.h"
#include "QtMeshCloudClient.h"
#include <OgreMaterialSerializer.h>
#include <QApplication>
Expand Down Expand Up @@ -569,6 +570,7 @@ void CLIPipeline::printUsage()
" --min-materials <n> Override min_material_count (0 = no limit)\n"
" --max-vertices <n> Override max_vertex_count (0 = no limit)\n"
" --min-vertices <n> Override min_vertex_count (0 = no limit)\n"
" --max-acmr <n> Override max_acmr (0 = no limit, e.g. 1.5)\n"
" --require-skeleton / --no-require-skeleton\n"
" Override require_skeleton\n"
" --require-animations / --no-require-animations\n"
Expand Down Expand Up @@ -611,6 +613,9 @@ void CLIPipeline::printUsage()
" --no-cloud opts out.\n"
" analyze <file> [--json] Analyze draw calls: per-material grouping plus\n"
" merge suggestions for entities sharing a material.\n"
" vertex-cache <file> [-o <output>] [--json]\n"
" Reorder index buffers via Forsyth's algorithm; reports\n"
" before/after ACMR. Without -o, only analyzes (read-only).\n"
"\n"
"Global options:\n"
" --help, -h Show this help\n"
Expand Down Expand Up @@ -975,6 +980,7 @@ int CLIPipeline::run(int argc, char* 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);
else if (cmd == "vertex-cache") rc = cmdVertexCache(argc, argv);

if (rc < 0) {
err() << "Error: Unknown command '" << cmd << "'" << Qt::endl;
Expand Down Expand Up @@ -2761,6 +2767,7 @@ int CLIPipeline::cmdScan(int argc, char* argv[])

int maxVerticesOverride = -1;
int minVerticesOverride = -1;
double maxAcmrOverride = -1.0;
int maxMeshesOverride = -1;
int minMeshesOverride = -1;
int maxMaterialsOverride = -1;
Expand Down Expand Up @@ -2906,6 +2913,12 @@ int CLIPipeline::cmdScan(int argc, char* argv[])
if (!parseNonNegativeInt("--min-vertices", value, minVerticesOverride)) return 2;
continue;
}
parseResult = parseValueArg(arg, "--max-acmr", i, value);
if (parseResult == ParseValueResult::Error) return 2;
if (parseResult == ParseValueResult::Matched) {
if (!parseNonNegativeDouble("--max-acmr", value, maxAcmrOverride)) return 2;
continue;
}
parseResult = parseValueArg(arg, "--max-meshes", i, value);
if (parseResult == ParseValueResult::Error) return 2;
if (parseResult == ParseValueResult::Matched) {
Expand Down Expand Up @@ -3064,6 +3077,7 @@ int CLIPipeline::cmdScan(int argc, char* argv[])
if (minMaterialsOverride >= 0) config.minMaterialCount = minMaterialsOverride;
if (maxVerticesOverride >= 0) config.maxVertexCount = maxVerticesOverride;
if (minVerticesOverride >= 0) config.minVertexCount = minVerticesOverride;
if (maxAcmrOverride >= 0.0) config.maxAcmr = maxAcmrOverride;
if (maxAnimKeyframesOverride >= 0) config.maxAnimKeyframes = maxAnimKeyframesOverride;
if (minAnimKeyframesOverride >= 0) config.minAnimKeyframes = minAnimKeyframesOverride;
if (maxAnimDurationOverride >= 0.0) config.maxAnimDuration = maxAnimDurationOverride;
Expand Down Expand Up @@ -3473,3 +3487,113 @@ int CLIPipeline::cmdAnalyze(int argc, char* argv[])
}
return 0;
}

namespace {

struct VertexCacheCmdArgs {
QString filePath;
QString outputPath;
bool jsonOutput = false;
};

// Parse argv for cmdVertexCache. Returns true on success.
bool parseVertexCacheArgs(int argc, char* argv[], VertexCacheCmdArgs& out)
{
int i = 1;
while (i < argc) {
const QString arg(argv[i]);
++i;
if (arg == "vertex-cache" || arg == "--cli") continue;
if (arg == "--json") { out.jsonOutput = true; continue; }
if (arg == "-o" && i < argc) { out.outputPath = argv[i++]; continue; }
if (!arg.startsWith("-") && out.filePath.isEmpty()) out.filePath = arg;
}
return !out.filePath.isEmpty();
}

// Export the (potentially mutated) scene rooted at `entities.first()` to
// `outputPath`. Returns 0 on success, 1 on export failure.
int exportRewrittenMesh(const QList<Ogre::Entity*>& entities,
const QFileInfo& srcFi, const QString& outputPath)
{
const Ogre::Entity* entity = entities.first();
const auto* node = entity->getParentSceneNode();
const QFileInfo outFi(outputPath);
const QString fmt = CLIPipeline::formatForExtension(outputPath);
SentryReporter::addBreadcrumb("file.export",
QString("Exporting %1").arg(outFi.absoluteFilePath()));
if (MeshImporterExporter::exporter(node, outFi.absoluteFilePath(), fmt) != 0) {
SentryReporter::captureMessage(
QString("CLI vertex-cache: export failed (.%1 -> .%2)")
.arg(srcFi.suffix(), outFi.suffix()), "error");
err() << "Error: Export failed." << Qt::endl;
return 1;
}
return 0;
}

void emitVertexCacheReport(const VertexCacheReport& report, const QFileInfo& fi,
const QString& outputPath, bool jsonOutput)
{
const bool rewrite = !outputPath.isEmpty();
if (jsonOutput) {
QJsonObject obj = VertexCacheOptimizer::toJson(report);
obj["file"] = fi.fileName();
if (rewrite) obj["output"] = QFileInfo(outputPath).fileName();
cliWrite(QString::fromUtf8(QJsonDocument(obj).toJson(QJsonDocument::Indented)));
} else {
cliWrite(QString("File: %1%2\n")
.arg(fi.fileName(),
rewrite ? QString(" -> %1").arg(QFileInfo(outputPath).fileName())
: QString()));
cliWrite(VertexCacheOptimizer::toText(report));
}
}

} // namespace

int CLIPipeline::cmdVertexCache(int argc, char* argv[])
{
VertexCacheCmdArgs cmdArgs;
if (!parseVertexCacheArgs(argc, argv, cmdArgs)) {
err() << "Error: No input file specified." << Qt::endl;
err() << "Usage: qtmesh vertex-cache <file> [-o <output>] [--json]" << Qt::endl;
return 2;
}

const QFileInfo fi(cmdArgs.filePath);
if (!fi.exists()) {
err() << "Error: File not found: " << cmdArgs.filePath << Qt::endl;
return 1;
}

if (!initOgreHeadless()) return 1;

const bool rewrite = !cmdArgs.outputPath.isEmpty();
SentryReporter::addBreadcrumb("cli.vertex-cache",
QString("Vertex-cache .%1%2").arg(fi.suffix(), rewrite ? " rewrite" : " analyze"));
Comment on lines +3573 to +3574

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Use a guideline-approved breadcrumb category for command invocation.

For this user-triggered action, use ui.action instead of a custom category to stay aligned with telemetry conventions.

Suggested fix
-    SentryReporter::addBreadcrumb("cli.vertex-cache",
+    SentryReporter::addBreadcrumb("ui.action",
         QString("Vertex-cache .%1%2").arg(fi.suffix(), rewrite ? " rewrite" : " analyze"));

As per coding guidelines: “All user-facing actions and significant operations must be tracked with SentryReporter::addBreadcrumb(category, message) using categories: ui.actionfile.import / file.export …”.

📝 Committable suggestion

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

Suggested change
SentryReporter::addBreadcrumb("cli.vertex-cache",
QString("Vertex-cache .%1%2").arg(fi.suffix(), rewrite ? " rewrite" : " analyze"));
SentryReporter::addBreadcrumb("ui.action",
QString("Vertex-cache .%1%2").arg(fi.suffix(), rewrite ? " rewrite" : " analyze"));
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/CLIPipeline.cpp` around lines 3514 - 3515, Replace the custom breadcrumb
category used when logging the vertex-cache command with the guideline-approved
category "ui.action": update the SentryReporter::addBreadcrumb call in
CLIPipeline.cpp (the call that currently uses "cli.vertex-cache" and constructs
the message with QString("Vertex-cache .%1%2").arg(fi.suffix(), rewrite ? "
rewrite" : " analyze")) to use "ui.action" as the first argument while leaving
the message construction unchanged so the event is recorded as a user action.

SentryReporter::addBreadcrumb("file.import",
QString("Importing %1").arg(fi.absoluteFilePath()));

MeshImporterExporter::importer({fi.absoluteFilePath()}, 0);
const auto& entities = Manager::getSingleton()->getEntities();
if (entities.isEmpty()) {
err() << "Error: Failed to load file: " << cmdArgs.filePath << Qt::endl;
return 1;
}

VertexCacheReport aggregate;
for (Ogre::Entity* entity : entities) {
VertexCacheOptimizer::mergeReport(
aggregate, VertexCacheOptimizer::analyzeEntity(entity, rewrite));
}
VertexCacheOptimizer::finalize(aggregate);

if (rewrite) {
if (const int rc = exportRewrittenMesh(entities, fi, cmdArgs.outputPath); rc != 0)
return rc;
}

emitVertexCacheReport(aggregate, fi, cmdArgs.outputPath, cmdArgs.jsonOutput);
return 0;
}
3 changes: 3 additions & 0 deletions src/CLIPipeline.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ class CLIPipeline {
/// Phase 6 slice B: analyze draw calls and surface merge opportunities
/// (per-material grouping, optional --json).
static int cmdAnalyze(int argc, char* argv[]);
/// Phase 6 slice C: vertex-cache (Forsyth) optimization. With -o, write
/// the reordered mesh; without -o, analyze only.
static int cmdVertexCache(int argc, char* argv[]);

/// Map file extension to MeshImporterExporter format string.
static QString formatForExtension(const QString& path);
Expand Down
2 changes: 2 additions & 0 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ TextureChannelPacker.cpp
NormalMapGenerator.cpp
MemoryEstimator.cpp
DrawCallAnalyzer.cpp
VertexCacheOptimizer.cpp
MeshValidator.cpp
AIChatManager.cpp
WelcomeScreenController.cpp
Expand Down Expand Up @@ -169,6 +170,7 @@ TextureChannelPacker.h
NormalMapGenerator.h
MemoryEstimator.h
DrawCallAnalyzer.h
VertexCacheOptimizer.h
MeshValidator.h
AIChatManager.h
WelcomeScreenController.h
Expand Down
53 changes: 53 additions & 0 deletions src/MCPServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "MeshLodController.h"
#include "MemoryEstimator.h"
#include "DrawCallAnalyzer.h"
#include "VertexCacheOptimizer.h"
#include <QDebug>
#include <QFile>
#include <QDir>
Expand Down Expand Up @@ -434,6 +435,7 @@
{QStringLiteral("get_lod_info"), &MCPServer::toolGetLodInfo},
{QStringLiteral("get_memory_usage"), &MCPServer::toolGetMemoryUsage},
{QStringLiteral("analyze_draw_calls"), &MCPServer::toolAnalyzeDrawCalls},
{QStringLiteral("optimize_vertex_cache"), &MCPServer::toolOptimizeVertexCache},
{QStringLiteral("list_files"), &MCPServer::toolListFiles},
{QStringLiteral("search_files"), &MCPServer::toolSearchFiles},
{QStringLiteral("read_file"), &MCPServer::toolReadFile},
Expand Down Expand Up @@ -2790,6 +2792,41 @@
}
}

// 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::toolOptimizeVertexCache(const QJsonObject &args)

Check warning on line 2798 in src/MCPServer.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This function should be declared "const".

See more on https://sonarcloud.io/project/issues?id=fernandotonon_QtMeshEditor&issues=AZ4dxAKLysjSaMGEUSj3&open=AZ4dxAKLysjSaMGEUSj3&pullRequest=498
{
// Args: rewrite (bool, default false). When true, the in-memory index
// buffers are rewritten; otherwise the tool only reports ACMR.
try {
if (const Manager* mgr = Manager::getSingletonPtr(); !mgr)
return makeErrorResult("Error: Manager not available");

const bool rewrite = args.value("rewrite").toBool(false);

VertexCacheReport aggregate;
for (const Ogre::SceneNode* node : Manager::getSingleton()->getSceneNodes()) {
if (!node) continue;
for (unsigned i = 0; i < node->numAttachedObjects(); ++i) {
Ogre::MovableObject* obj = node->getAttachedObject(i);
if (!obj || obj->getMovableType() != "Entity") continue;
auto* entity = static_cast<Ogre::Entity*>(obj);
VertexCacheOptimizer::mergeReport(
aggregate, VertexCacheOptimizer::analyzeEntity(entity, rewrite));
}
}
VertexCacheOptimizer::finalize(aggregate);

QJsonObject result = makeSuccessResult(VertexCacheOptimizer::toText(aggregate));
result["vertexCache"] = VertexCacheOptimizer::toJson(aggregate);
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)
Expand Down Expand Up @@ -4156,6 +4193,22 @@
);
}

// optimize_vertex_cache
{
QJsonObject props;
props["rewrite"] = QJsonObject{{"type", "boolean"},
{"description", "When true, rewrite each submesh's index buffer in place with Forsyth's optimal order. When false (default), only report ACMR — no mutation."}};
appendTool(
"optimize_vertex_cache",
"Run Tom Forsyth's linear-time vertex-cache optimization on every loaded mesh. "
"Reports per-submesh and weighted ACMR (Average Cache Miss Ratio) before / after. "
"Pass `rewrite: true` to actually reorder the index buffers (analysis-only otherwise). "
"The response includes a human-readable summary in 'content' and a structured "
"'vertexCache' object with per-submesh ACMR plus totals for machine consumers.",
props
);
}

// delete_entity
{
QJsonObject props;
Expand Down
1 change: 1 addition & 0 deletions src/MCPServer.h
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ private slots:
QJsonObject toolGetLodInfo(const QJsonObject &args);
QJsonObject toolGetMemoryUsage(const QJsonObject &args);
QJsonObject toolAnalyzeDrawCalls(const QJsonObject &args);
QJsonObject toolOptimizeVertexCache(const QJsonObject &args);
QJsonObject toolListFiles(const QJsonObject &args);
QJsonObject toolSearchFiles(const QJsonObject &args);
QJsonObject toolReadFile(const QJsonObject &args);
Expand Down
Loading
Loading