From 41844cd4cf8d4b4bfa71a8204758e19b1d20abdb Mon Sep 17 00:00:00 2001 From: Fernando Date: Tue, 28 Apr 2026 02:13:53 -0400 Subject: [PATCH 1/2] fix(scan): harden scan --fix and improve reporting Fix scan --fix crashes in Ogre/Assimp paths, prevent FBX simplify from bloating files, and enrich scan output with [fixed]/[skipped] tags plus saved-bytes and keys-removed summaries. Default viewport FSAA to 0 when unset and add a regression test. Made-with: Cursor --- src/AnimationMerger.cpp | 4 + src/Assimp/Importer.cpp | 42 ++- src/Assimp/MaterialProcessor.cpp | 37 +- src/CLIPipeline.cpp | 447 +++++++++++++++++++++--- src/FBX/FBXExporter.cpp | 201 ++++++++--- src/FBX/FBXExporter.h | 4 + src/FBX/FBXExporter_test.cpp | 47 +++ src/OgreWidget.cpp | 12 +- src/OgreWidget_test.cpp | 11 + src/ScanConfig.h | 5 +- src/ScanEngine.cpp | 578 +++++++++++++++++++++++++++++-- src/ScanEngine.h | 14 +- src/ScanEngine_test.cpp | 55 ++- 13 files changed, 1305 insertions(+), 152 deletions(-) diff --git a/src/AnimationMerger.cpp b/src/AnimationMerger.cpp index 76673fd5c..75f4c2f06 100644 --- a/src/AnimationMerger.cpp +++ b/src/AnimationMerger.cpp @@ -8,6 +8,8 @@ #include #include #include +#include +#include // Registry: skeleton name โ†’ up-axis (1=Y-up, 2=Z-up). // Populated by AnimationMerger::registerSkeletonUpAxis() at import time. @@ -586,6 +588,8 @@ int AnimationMerger::simplifyAnimation(Ogre::Skeleton* skel, newAnim->setRotationInterpolationMode(rotInterpMode); for (const auto& td : tracks) { + if (td.keys.empty()) + continue; auto* newTrack = newAnim->createNodeTrack(td.handle); if (td.associatedNode) newTrack->setAssociatedNode(td.associatedNode); diff --git a/src/Assimp/Importer.cpp b/src/Assimp/Importer.cpp index 9013efb94..984b1870b 100644 --- a/src/Assimp/Importer.cpp +++ b/src/Assimp/Importer.cpp @@ -33,6 +33,7 @@ THE SOFTWARE. #include "BoneProcessor.h" #include "MeshProcessor.h" #include +#include Ogre::MeshPtr AssimpToOgreImporter::loadModel(const std::string& path, bool convertToLeftHanded, unsigned int additionalFlags) { skeleton.reset(); // Clear any skeleton from a previous import @@ -59,15 +60,54 @@ Ogre::MeshPtr AssimpToOgreImporter::loadModel(const std::string& path, bool conv flags |= aiProcess_ConvertToLeftHanded; flags |= additionalFlags; + auto pathEndsWithInsensitive = [](const std::string& p, const char* suf) -> bool { + const size_t n = std::strlen(suf); + if (p.size() < n) + return false; + for (size_t i = 0; i < n; ++i) { + char a = p[p.size() - n + i]; + char b = suf[i]; + if (a >= 'A' && a <= 'Z') + a = static_cast(a - 'A' + 'a'); + if (b >= 'A' && b <= 'Z') + b = static_cast(b - 'A' + 'a'); + if (a != b) + return false; + } + return true; + }; + const aiScene* scene = importer.ReadFile(path, flags); // Do this immediately after ReadFile while the scene is still valid. m_sceneUpAxis = 1; // default: Y-up if (scene && scene->mMetaData) scene->mMetaData->Get("UpAxis", m_sceneUpAxis); + // Some FBX animation takes fail the full post-process stack (null scene or no root) + // but load with a lighter flag set. Retry once before giving up. + if ((!scene || !scene->mRootNode) && + (pathEndsWithInsensitive(path, ".fbx") || pathEndsWithInsensitive(path, ".fbxa"))) { + unsigned int lightFlags = aiProcess_Triangulate | + aiProcess_ValidateDataStructure | + aiProcess_LimitBoneWeights | + aiProcess_PopulateArmatureData | + aiProcess_GlobalScale; + if (convertToLeftHanded) + lightFlags |= aiProcess_ConvertToLeftHanded; + lightFlags |= additionalFlags; + importer.SetPropertyBool(AI_CONFIG_IMPORT_FBX_PRESERVE_PIVOTS, false); + scene = importer.ReadFile(path, lightFlags); + m_sceneUpAxis = 1; + if (scene && scene->mMetaData) + scene->mMetaData->Get("UpAxis", m_sceneUpAxis); + } + // A null scene or missing root node is always fatal. if(!scene || !scene->mRootNode) { - Ogre::LogManager::getSingleton().logError("ERROR::ASSIMP::" + std::string(importer.GetErrorString())); + const char* errStr = importer.GetErrorString(); + const std::string errMsg = (errStr && *errStr) ? std::string(errStr) + : std::string("ReadFile failed (no scene / no root node)"); + Ogre::LogManager::getSingleton().logError("ERROR::ASSIMP::" + errMsg); return {}; } // animationOnly: the scene has no geometry (e.g. Unreal Engine retarget FBX). diff --git a/src/Assimp/MaterialProcessor.cpp b/src/Assimp/MaterialProcessor.cpp index 45febf467..7d256129b 100644 --- a/src/Assimp/MaterialProcessor.cpp +++ b/src/Assimp/MaterialProcessor.cpp @@ -1,6 +1,25 @@ #include "MaterialProcessor.h" #include "RTShaderHelper.h" +namespace { +static Ogre::Pass* ensureFirstPass(const Ogre::MaterialPtr& mat) +{ + if (!mat) + return nullptr; + Ogre::Technique* tech = nullptr; + if (mat->getNumTechniques() == 0) + tech = mat->createTechnique(); + else + tech = mat->getTechnique(0); + + if (!tech) + return nullptr; + if (tech->getNumPasses() == 0) + return tech->createPass(); + return tech->getPass(0); +} +} // namespace + void MaterialProcessor::loadScene(const aiScene* scene) { for(auto i = 0u; i < scene->mNumMaterials; i++) { @@ -42,6 +61,9 @@ Ogre::MaterialPtr MaterialProcessor::processMaterial(const aiMaterial *material, } if(normalTexPtr) { Ogre::LogManager::getSingleton().logMessage("MaterialProcessor: Applying RTSS normal map '" + normalFilename + "' to existing material '" + materialName + "'"); + // Some materials can exist without any techniques/passes (e.g. partially loaded + // script materials). Ensure a valid pass exists before RTSS touches it. + (void)ensureFirstPass(existingMaterial); applyRTSSNormalMap(existingMaterial, normalTexPtr->getName()); } } @@ -49,30 +71,33 @@ Ogre::MaterialPtr MaterialProcessor::processMaterial(const aiMaterial *material, } Ogre::MaterialPtr ogreMaterial = Ogre::MaterialManager::getSingleton().create(materialName, Ogre::ResourceGroupManager::DEFAULT_RESOURCE_GROUP_NAME); + Ogre::Pass* pass = ensureFirstPass(ogreMaterial); + if (!pass) + return ogreMaterial; aiColor3D color(0.f, 0.f, 0.f); if(AI_SUCCESS == material->Get(AI_MATKEY_COLOR_DIFFUSE, color)) { - ogreMaterial->getTechnique(0)->getPass(0)->setDiffuse(color.r, color.g, color.b, 1.0f); + pass->setDiffuse(color.r, color.g, color.b, 1.0f); } if(AI_SUCCESS == material->Get(AI_MATKEY_COLOR_AMBIENT, color)) { // PBR-workflow exporters often set ambient to (0,0,0) which kills ambient // lighting in Ogre's Phong model. Keep Ogre's default (white) in that case. if(color.r > 0.001f || color.g > 0.001f || color.b > 0.001f) - ogreMaterial->getTechnique(0)->getPass(0)->setAmbient(color.r, color.g, color.b); + pass->setAmbient(color.r, color.g, color.b); } if(AI_SUCCESS == material->Get(AI_MATKEY_COLOR_SPECULAR, color)) { - ogreMaterial->getTechnique(0)->getPass(0)->setSpecular(color.r, color.g, color.b, 1.0f); + pass->setSpecular(color.r, color.g, color.b, 1.0f); } if(AI_SUCCESS == material->Get(AI_MATKEY_COLOR_EMISSIVE, color)) { - ogreMaterial->getTechnique(0)->getPass(0)->setSelfIllumination(color.r, color.g, color.b); + pass->setSelfIllumination(color.r, color.g, color.b); } float shininess = 0.0f; if(AI_SUCCESS == material->Get(AI_MATKEY_SHININESS, shininess)) { - ogreMaterial->getTechnique(0)->getPass(0)->setShininess(shininess); + pass->setShininess(shininess); } // Handle textures @@ -85,7 +110,7 @@ Ogre::MaterialPtr MaterialProcessor::processMaterial(const aiMaterial *material, if(!texturePtr){ texturePtr = loadTexture(textureFilename, path, scene); } - auto* tus = ogreMaterial->getTechnique(0)->getPass(0)->createTextureUnitState(texturePtr->getName()); + auto* tus = pass->createTextureUnitState(texturePtr->getName()); tus->setName("diffuse_map"); } diff --git a/src/CLIPipeline.cpp b/src/CLIPipeline.cpp index a37606448..b6c9e1e15 100644 --- a/src/CLIPipeline.cpp +++ b/src/CLIPipeline.cpp @@ -8,21 +8,29 @@ #include "SentryReporter.h" #include "ScanConfig.h" #include "ScanEngine.h" +#include "FBX/FBXExporter.h" #include "QtMeshCloudClient.h" #include #include #include #include #include +#include #include #include +#include #include #include +#include #include #include #include +#include +#include +#include + #include #include @@ -51,6 +59,250 @@ static void cliWrite(const QString& text) } } +// --------------------------------------------------------------------------- +// Animation-only export helpers (no mesh artifacts) +// --------------------------------------------------------------------------- + +static aiMatrix4x4 ogreTransformToAi(const Ogre::Vector3& pos, + const Ogre::Quaternion& rot, + const Ogre::Vector3& scale) +{ + // Assimp uses row-major aiMatrix4x4; build from SRT. + aiMatrix4x4 s; + s.a1 = scale.x; s.b2 = scale.y; s.c3 = scale.z; + s.d4 = 1.0f; + + Ogre::Matrix3 r3; + rot.ToRotationMatrix(r3); + aiMatrix4x4 r; + r.a1 = r3[0][0]; r.a2 = r3[0][1]; r.a3 = r3[0][2]; + r.b1 = r3[1][0]; r.b2 = r3[1][1]; r.b3 = r3[1][2]; + r.c1 = r3[2][0]; r.c2 = r3[2][1]; r.c3 = r3[2][2]; + r.d4 = 1.0f; + + aiMatrix4x4 t; + t.a4 = pos.x; + t.b4 = pos.y; + t.c4 = pos.z; + t.d4 = 1.0f; + + return t * r * s; +} + +static aiScene* buildAnimOnlyAiSceneFromSkeleton(const Ogre::Skeleton* skel) +{ + if (!skel) + return nullptr; + + auto* scene = new aiScene(); + + // Root node + scene->mRootNode = new aiNode(); + scene->mRootNode->mName = aiString("RootNode"); + scene->mRootNode->mTransformation = aiMatrix4x4(); // identity + + // Build bone node tree. + std::unordered_map boneToNode; + boneToNode.reserve(skel->getNumBones()); + + auto makeNodeForBone = [&](const Ogre::Bone* b) -> aiNode* { + auto it = boneToNode.find(b); + if (it != boneToNode.end()) + return it->second; + + auto* n = new aiNode(); + n->mName = aiString(b->getName().c_str()); + n->mTransformation = ogreTransformToAi(b->getPosition(), b->getOrientation(), b->getScale()); + boneToNode[b] = n; + return n; + }; + + // Attach root bones under scene root; attach children under parents. + std::vector rootBones; + rootBones.reserve(skel->getNumBones()); + + for (unsigned short i = 0; i < skel->getNumBones(); ++i) { + const Ogre::Bone* b = skel->getBone(i); + if (!b) + continue; + + aiNode* node = makeNodeForBone(b); + const Ogre::Node* parent = b->getParent(); + const Ogre::Bone* parentBone = dynamic_cast(parent); + if (!parentBone) { + rootBones.push_back(node); + continue; + } + + aiNode* parentNode = makeNodeForBone(parentBone); + // Append as child + aiNode** newChildren = new aiNode*[parentNode->mNumChildren + 1]; + for (unsigned int ci = 0; ci < parentNode->mNumChildren; ++ci) + newChildren[ci] = parentNode->mChildren[ci]; + newChildren[parentNode->mNumChildren] = node; + delete[] parentNode->mChildren; + parentNode->mChildren = newChildren; + parentNode->mNumChildren += 1; + node->mParent = parentNode; + } + + if (!rootBones.empty()) { + scene->mRootNode->mChildren = new aiNode*[rootBones.size()]; + scene->mRootNode->mNumChildren = static_cast(rootBones.size()); + for (unsigned int i = 0; i < scene->mRootNode->mNumChildren; ++i) { + scene->mRootNode->mChildren[i] = rootBones[i]; + rootBones[i]->mParent = scene->mRootNode; + } + } + + // Animations + scene->mNumAnimations = skel->getNumAnimations(); + scene->mAnimations = scene->mNumAnimations ? new aiAnimation*[scene->mNumAnimations] : nullptr; + + for (unsigned short ai = 0; ai < skel->getNumAnimations(); ++ai) { + const Ogre::Animation* anim = skel->getAnimation(ai); + auto* a = new aiAnimation(); + a->mName = aiString(anim->getName().c_str()); + a->mDuration = anim->getLength(); + a->mTicksPerSecond = 1.0; // key times are in seconds + + // Collect bone tracks. + std::vector tracks; + tracks.reserve(anim->getNumNodeTracks()); + for (unsigned short ti = 0; ti < anim->getNumNodeTracks(); ++ti) { + const Ogre::NodeAnimationTrack* t = anim->getNodeTrack(ti); + if (t) + tracks.push_back(t); + } + + a->mNumChannels = static_cast(tracks.size()); + a->mChannels = a->mNumChannels ? new aiNodeAnim*[a->mNumChannels] : nullptr; + + for (unsigned int ci = 0; ci < a->mNumChannels; ++ci) { + const Ogre::NodeAnimationTrack* t = tracks[ci]; + const Ogre::Node* target = t->getAssociatedNode(); + const Ogre::Bone* bone = dynamic_cast(target); + + auto* ch = new aiNodeAnim(); + ch->mNodeName = aiString(bone ? bone->getName().c_str() : "Unknown"); + + const unsigned short kCount = t->getNumKeyFrames(); + ch->mNumPositionKeys = kCount; + ch->mNumRotationKeys = kCount; + ch->mNumScalingKeys = kCount; + ch->mPositionKeys = kCount ? new aiVectorKey[kCount] : nullptr; + ch->mRotationKeys = kCount ? new aiQuatKey[kCount] : nullptr; + ch->mScalingKeys = kCount ? new aiVectorKey[kCount] : nullptr; + + for (unsigned short ki = 0; ki < kCount; ++ki) { + const Ogre::TransformKeyFrame* kf = t->getNodeKeyFrame(ki); + const double time = kf->getTime(); + + const Ogre::Vector3 p = kf->getTranslate(); + const Ogre::Quaternion r = kf->getRotation(); + const Ogre::Vector3 s = kf->getScale(); + + ch->mPositionKeys[ki].mTime = time; + ch->mPositionKeys[ki].mValue = aiVector3D(p.x, p.y, p.z); + + ch->mRotationKeys[ki].mTime = time; + ch->mRotationKeys[ki].mValue = aiQuaternion(r.w, r.x, r.y, r.z); + + ch->mScalingKeys[ki].mTime = time; + ch->mScalingKeys[ki].mValue = aiVector3D(s.x, s.y, s.z); + } + + a->mChannels[ci] = ch; + } + + scene->mAnimations[ai] = a; + } + + return scene; +} + +static QString assimpExportFormatIdForAnimOnlyPath(const QString& outputPath) +{ + const QString fmt = CLIPipeline::formatForExtension(outputPath); + static const QMap kUiToAssimp = { + {QStringLiteral("Collada (*.dae)"), QStringLiteral("collada")}, + {QStringLiteral("X (*.x)"), QStringLiteral("x")}, + {QStringLiteral("OBJ (*.obj)"), QStringLiteral("obj")}, + {QStringLiteral("STL (*.stl)"), QStringLiteral("stl")}, + {QStringLiteral("PLY (*.ply)"), QStringLiteral("ply")}, + {QStringLiteral("3DS (*.3ds)"), QStringLiteral("3ds")}, + {QStringLiteral("glTF 2.0 (*.gltf)"), QStringLiteral("gltf2")}, + {QStringLiteral("glTF 2.0 (*.gltf2)"), QStringLiteral("gltf2")}, + {QStringLiteral("glTF 2.0 Binary (*.glb)"), QStringLiteral("glb2")}, + {QStringLiteral("glTF 2.0 Binary (*.glb2)"), QStringLiteral("glb2")}, + {QStringLiteral("VRM / glTF 2.0 (*.vrm)"), QStringLiteral("gltf2")}, + {QStringLiteral("FBX Binary (*.fbx)"), QStringLiteral("fbx")}, + {QStringLiteral("Assimp Binary (*.assbin)"), QStringLiteral("assbin")}, + }; + if (auto it = kUiToAssimp.find(fmt); it != kUiToAssimp.end()) + return it.value(); + const QString suf = QFileInfo(outputPath).suffix().toLower(); + static const QMap kExtToAssimp = { + {QStringLiteral("fbx"), QStringLiteral("fbx")}, + {QStringLiteral("dae"), QStringLiteral("collada")}, + {QStringLiteral("obj"), QStringLiteral("obj")}, + {QStringLiteral("stl"), QStringLiteral("stl")}, + {QStringLiteral("ply"), QStringLiteral("ply")}, + {QStringLiteral("3ds"), QStringLiteral("3ds")}, + {QStringLiteral("gltf"), QStringLiteral("gltf2")}, + {QStringLiteral("glb"), QStringLiteral("glb2")}, + {QStringLiteral("vrm"), QStringLiteral("gltf2")}, + {QStringLiteral("assbin"), QStringLiteral("assbin")}, + {QStringLiteral("x"), QStringLiteral("x")}, + }; + return kExtToAssimp.value(suf, QStringLiteral("fbx")); +} + +static bool exportAnimOnlyViaAssimp(const Ogre::SkeletonPtr& skel, const QString& outputPath, QString* outError = nullptr) +{ + if (!skel) { + if (outError) *outError = QStringLiteral("No skeleton"); + return false; + } + + std::unique_ptr scene(buildAnimOnlyAiSceneFromSkeleton(skel.get())); + if (!scene) { + if (outError) *outError = QStringLiteral("Failed to build animation-only scene"); + return false; + } + + const QString formatId = assimpExportFormatIdForAnimOnlyPath(outputPath); + const unsigned int exportFlags = + (formatId == QLatin1String("x")) ? 0u : aiProcess_ConvertToLeftHanded; + + Assimp::Exporter exporter; + const aiReturn r = exporter.Export(scene.get(), formatId.toStdString().c_str(), + outputPath.toStdString().c_str(), exportFlags); + if (r != AI_SUCCESS) { + if (outError) + *outError = QString::fromUtf8(exporter.GetErrorString()); + return false; + } + return true; +} + +static bool exportAnimOnly(const Ogre::SkeletonPtr& skel, const QString& outputPath, QString* outError = nullptr) +{ + const QString suf = QFileInfo(outputPath).suffix().toLower(); + if (suf == QStringLiteral("fbx") || suf == QStringLiteral("fbxa")) { + if (!skel) { + if (outError) *outError = QStringLiteral("No skeleton"); + return false; + } + if (!FBXExporter::exportSkeletonOnlyFBX(skel.get(), outputPath)) { + if (outError) *outError = QStringLiteral("Custom FBX exporter failed"); + return false; + } + return true; + } + return exportAnimOnlyViaAssimp(skel, outputPath, outError); +} + static bool cliSupportsColor() { if (qEnvironmentVariableIsSet("NO_COLOR")) @@ -91,9 +343,13 @@ static QString scanStatusLabel(bool hasError, bool hasWarning, bool colorize) return colorizeWord("OK", "32", colorize); } -static QString findingSeverityTag(Severity severity) +static QString findingSeverityTag(const Finding& f) { - switch (severity) { + if (f.fixed) + return "fixed"; + if (f.skipped) + return "skipped"; + switch (f.severity) { case Severity::Error: return "error"; case Severity::Warning: return "warn"; case Severity::Info: return "info"; @@ -108,6 +364,9 @@ static QString formatScanAssetLine(const AssetInfo& asset, const QList& for (const auto& f : findings) { if (f.fixed) continue; + if (f.skipped) { + continue; + } if (f.severity == Severity::Error) hasError = true; else if (f.severity == Severity::Warning) @@ -125,7 +384,7 @@ static QString formatScanAssetLine(const AssetInfo& asset, const QList& s << status << " " << asset.relativePath << "\n"; for (const auto& f : findings) { - s << " [" << findingSeverityTag(f.severity) << "] " + s << " [" << findingSeverityTag(f) << "] " << f.rule << ": " << f.message << "\n"; } return out; @@ -141,6 +400,8 @@ static QString formatScanSummary(const ScanResult& result, bool colorize) const QString errorIcon = colorizeIconWhenPositive(QStringLiteral("โœ—"), result.errors, "31", colorize); const QString infoIcon = colorizeIconWhenPositive(QStringLiteral("โ„น"), result.infos, "36", colorize); const QString fixedIcon = colorizeIconWhenPositive(QStringLiteral("๐Ÿ”ง"), result.fixed, "32", colorize); + const QString savedIcon = colorizeIconWhenPositive(QStringLiteral("๐Ÿ“‰"), result.bytesSaved > 0 ? 1 : 0, "32", colorize); + const QString keysIcon = colorizeIconWhenPositive(QStringLiteral("๐Ÿงน"), result.keysRemoved > 0 ? 1 : 0, "32", colorize); const QString skippedIcon = colorizeWord(QStringLiteral("โญ"), "90", colorize); const QString timeIcon = colorizeWord(QStringLiteral("โฑ"), "34", colorize); @@ -154,6 +415,12 @@ static QString formatScanSummary(const ScanResult& result, bool colorize) s << " " << infoIcon << " Info: " << result.infos << "\n"; if (result.fixed > 0) s << " " << fixedIcon << " Fixed: " << result.fixed << "\n"; + if (result.bytesSaved > 0) + s << " " << savedIcon << " Saved: " << QString::number(result.bytesSaved / (1024.0 * 1024.0), 'f', 2) << " MB\n"; + if (result.keysRemoved > 0) { + const QString n = QLocale::system().toString(result.keysRemoved); + s << " " << keysIcon << " Keys removed: " << n << "\n"; + } if (result.skipped > 0) s << " " << skippedIcon << " Skipped: " << result.skipped << "\n"; s << " " << timeIcon << " Time: " << QString::number(result.elapsedMs / 1000.0, 'f', 1) << "s\n"; @@ -1014,18 +1281,19 @@ int CLIPipeline::cmdFix(int argc, char* argv[]) int CLIPipeline::cmdAnim(int argc, char* argv[]) { // Parse: anim --list [--json] + // or: anim --analyze [--json] // or: anim --rename [-o ] // or: anim --merge [f2...] [-o ] // or: anim --resample N [-o ] [--animation ] // or: anim --decimate-step S [-o ] [--animation ] QString filePath, oldName, newName, outputPath, animationFilter; bool listMode = false; + bool analyzeMode = false; bool renameMode = false; bool mergeMode = false; bool resampleMode = false; bool decimateMode = false; bool simplifyMode = false; - bool analyzeMode = false; bool jsonOutput = false; int resampleCount = 0; int decimateStep = 0; @@ -1045,6 +1313,7 @@ int CLIPipeline::cmdAnim(int argc, char* argv[]) QString arg(argv[i]); if (arg == "anim" || arg == "--cli") continue; if (arg == "--list") { listMode = true; continue; } + if (arg == "--analyze") { analyzeMode = true; continue; } if (arg == "--json") { jsonOutput = true; continue; } if (arg == "--rename" && i + 2 < argc) { renameMode = true; @@ -1120,6 +1389,7 @@ int CLIPipeline::cmdAnim(int argc, char* argv[]) && !simplifyMode && !analyzeMode) { err() << "Error: Specify --list, --rename, --merge, --resample, --decimate-step, --simplify, or --analyze." << Qt::endl; err() << "Usage: qtmesh anim --list [--json]" << Qt::endl; + err() << " qtmesh anim --analyze [--json]" << Qt::endl; err() << " qtmesh anim --rename [-o ]" << Qt::endl; err() << " qtmesh anim --merge [f2...] [-o ]" << Qt::endl; err() << " qtmesh anim --resample N [-o ] [--animation ]" << Qt::endl; @@ -1152,27 +1422,31 @@ int CLIPipeline::cmdAnim(int argc, char* argv[]) SentryReporter::addBreadcrumb("cli.anim", QString("Anim %1 .%2%3") .arg(animOp, fi.suffix(), mergeMode ? QString(" files=%1").arg(mergeFiles.size()) : "")); - MeshImporterExporter::importer({fi.absoluteFilePath()}); + // Support animation-only files: importer may populate animOnlySkeletons without creating entities. + QList animOnlySkeletons; + MeshImporterExporter::importer({fi.absoluteFilePath()}, 0, &animOnlySkeletons); auto& entities = Manager::getSingleton()->getEntities(); - if (entities.isEmpty()) { - SentryReporter::captureMessage(QString("CLI anim: import failed (.%1)").arg(fi.suffix()), "error"); - err() << "Error: Failed to load file: " << filePath << Qt::endl; - return 1; - } + Ogre::Entity* entity = nullptr; + Ogre::SkeletonPtr skel; - Ogre::Entity* entity = entities.first(); - if (!entity->hasSkeleton()) { - err() << "Error: File has no skeleton/animations." << Qt::endl; - return 1; + if (!entities.isEmpty()) { + entity = entities.first(); + if (entity->hasSkeleton()) + skel = entity->getMesh()->getSkeleton(); } - Ogre::SkeletonPtr skel = entity->getMesh()->getSkeleton(); + if (!skel && !animOnlySkeletons.isEmpty()) + skel = animOnlySkeletons.first(); + if (!skel) { - err() << "Error: No skeleton found." << Qt::endl; + SentryReporter::captureMessage(QString("CLI anim: import failed (.%1)").arg(fi.suffix()), "error"); + err() << "Error: Failed to load file: " << filePath << Qt::endl; return 1; } + const bool isAnimOnlyInput = (entity == nullptr); + if (listMode) { if (skel->getNumAnimations() == 0) { cliWrite(jsonOutput ? "[]\n" : "No animations found.\n"); @@ -1203,16 +1477,53 @@ int CLIPipeline::cmdAnim(int argc, char* argv[]) return 0; } + if (analyzeMode) { + if (jsonOutput) { + QJsonObject root; + root["skeletonName"] = QString::fromStdString(skel->getName()); + root["boneCount"] = static_cast(skel->getNumBones()); + QJsonArray animArr; + for (unsigned short i = 0; i < skel->getNumAnimations(); ++i) { + auto* anim = skel->getAnimation(i); + QJsonObject a; + a["name"] = QString::fromStdString(anim->getName()); + a["duration"] = static_cast(anim->getLength()); + animArr.append(a); + } + root["animations"] = animArr; + cliWrite(QString::fromUtf8(QJsonDocument(root).toJson(QJsonDocument::Indented)) + "\n"); + } else { + QString out; + out += QString("Skeleton: %1 (%2 bones)\n") + .arg(QString::fromStdString(skel->getName())) + .arg(skel->getNumBones()); + out += QString("Animations: %1\n").arg(skel->getNumAnimations()); + for (unsigned short i = 0; i < skel->getNumAnimations(); ++i) { + auto* anim = skel->getAnimation(i); + out += QString(" %1 %2s\n") + .arg(QString::fromStdString(anim->getName())) + .arg(anim->getLength(), 0, 'f', 3); + } + cliWrite(out); + } + return 0; + } + // Merge mode if (mergeMode) { + if (!entity) { + err() << "Error: --merge requires a base file with mesh geometry. " + "An animation-only file cannot be used as the merge base." << Qt::endl; + return 1; + } // Load animation files; animation-only files (no mesh) produce a skeleton instead of entity. - QList animOnlySkeletons; + QList mergeAnimOnlySkeletons; for (const auto& f : mergeFiles) { int entityCountBefore = Manager::getSingleton()->getEntities().size(); - int skelCountBefore = animOnlySkeletons.size(); - MeshImporterExporter::importer({f}, 0, &animOnlySkeletons); + int skelCountBefore = mergeAnimOnlySkeletons.size(); + MeshImporterExporter::importer({f}, 0, &mergeAnimOnlySkeletons); bool gotEntity = Manager::getSingleton()->getEntities().size() > entityCountBefore; - bool gotSkeleton = animOnlySkeletons.size() > skelCountBefore; + bool gotSkeleton = mergeAnimOnlySkeletons.size() > skelCountBefore; if (!gotEntity && !gotSkeleton) { SentryReporter::captureMessage(QString("CLI anim: merge input import failed (.%1)").arg(QFileInfo(f).suffix()), "error"); err() << "Error: Failed to load animation file: " << f << Qt::endl; @@ -1224,13 +1535,13 @@ int CLIPipeline::cmdAnim(int argc, char* argv[]) // allEntities includes the base entity (already validated above) plus any // mesh entities from merge files. If no additional mesh entities AND no // animation-only skeletons were collected, there is nothing to merge. - if (allEntities.size() < 2 && animOnlySkeletons.isEmpty()) { + if (allEntities.size() < 2 && mergeAnimOnlySkeletons.isEmpty()) { err() << "Error: Need at least one source file to merge (got none)." << Qt::endl; return 1; } QString mergeErr; - Ogre::Entity* merged = AnimationMerger::mergeAnimations(allEntities.first(), allEntities, animOnlySkeletons, mergeErr); + Ogre::Entity* merged = AnimationMerger::mergeAnimations(allEntities.first(), allEntities, mergeAnimOnlySkeletons, mergeErr); if (!merged) { SentryReporter::captureMessage("CLI anim: merge failed", "error"); err() << "Error: Merge failed: " << mergeErr << Qt::endl; @@ -1287,15 +1598,23 @@ int CLIPipeline::cmdAnim(int argc, char* argv[]) return 1; } - entity->refreshAvailableAnimationState(); - - auto* node = entity->getParentSceneNode(); QFileInfo outFi(outputPath); - int result = MeshImporterExporter::exporter(node, outFi.absoluteFilePath(), formatForExtension(outputPath)); - if (result != 0) { - SentryReporter::captureMessage(QString("CLI anim: resample export failed (.%1)").arg(outFi.suffix()), "error"); - err() << "Error: Export failed." << Qt::endl; - return 1; + if (isAnimOnlyInput) { + QString exportErr; + if (!exportAnimOnly(skel, outFi.absoluteFilePath(), &exportErr)) { + SentryReporter::captureMessage(QString("CLI anim: resample export failed (anim-only)"), "error"); + err() << "Error: Export failed: " << exportErr << Qt::endl; + return 1; + } + } else { + entity->refreshAvailableAnimationState(); + auto* node = entity->getParentSceneNode(); + int result = MeshImporterExporter::exporter(node, outFi.absoluteFilePath(), formatForExtension(outputPath)); + if (result != 0) { + SentryReporter::captureMessage(QString("CLI anim: resample export failed (.%1)").arg(outFi.suffix()), "error"); + err() << "Error: Export failed." << Qt::endl; + return 1; + } } cliWrite(QString("Resampled %1 animation(s) to %2 keyframes (removed %3 keyframes)\nOutput: %4\n") @@ -1340,15 +1659,23 @@ int CLIPipeline::cmdAnim(int argc, char* argv[]) return 1; } - entity->refreshAvailableAnimationState(); - - auto* node = entity->getParentSceneNode(); QFileInfo outFi(outputPath); - int result = MeshImporterExporter::exporter(node, outFi.absoluteFilePath(), formatForExtension(outputPath)); - if (result != 0) { - SentryReporter::captureMessage(QString("CLI anim: decimate export failed (.%1)").arg(outFi.suffix()), "error"); - err() << "Error: Export failed." << Qt::endl; - return 1; + if (isAnimOnlyInput) { + QString exportErr; + if (!exportAnimOnly(skel, outFi.absoluteFilePath(), &exportErr)) { + SentryReporter::captureMessage(QString("CLI anim: decimate export failed (anim-only)"), "error"); + err() << "Error: Export failed: " << exportErr << Qt::endl; + return 1; + } + } else { + entity->refreshAvailableAnimationState(); + auto* node = entity->getParentSceneNode(); + int result = MeshImporterExporter::exporter(node, outFi.absoluteFilePath(), formatForExtension(outputPath)); + if (result != 0) { + SentryReporter::captureMessage(QString("CLI anim: decimate export failed (.%1)").arg(outFi.suffix()), "error"); + err() << "Error: Export failed." << Qt::endl; + return 1; + } } cliWrite(QString("Decimated %1 animation(s) with step %2 (removed %3 keyframes)\nOutput: %4\n") @@ -1536,17 +1863,25 @@ int CLIPipeline::cmdAnim(int argc, char* argv[]) } AnimationMerger::renameAnimation(skel.get(), oldName.toStdString(), newName.toStdString()); - entity->refreshAvailableAnimationState(); - auto* node = entity->getParentSceneNode(); QFileInfo outFi(outputPath); - QString fmt = formatForExtension(outputPath); - - int result = MeshImporterExporter::exporter(node, outFi.absoluteFilePath(), fmt); - if (result != 0) { - SentryReporter::captureMessage(QString("CLI anim: rename export failed (.%1)").arg(outFi.suffix()), "error"); - err() << "Error: Export failed." << Qt::endl; - return 1; + if (isAnimOnlyInput) { + QString exportErr; + if (!exportAnimOnly(skel, outFi.absoluteFilePath(), &exportErr)) { + SentryReporter::captureMessage(QString("CLI anim: rename export failed (anim-only)"), "error"); + err() << "Error: Export failed: " << exportErr << Qt::endl; + return 1; + } + } else { + entity->refreshAvailableAnimationState(); + auto* node = entity->getParentSceneNode(); + QString fmt = formatForExtension(outputPath); + int result = MeshImporterExporter::exporter(node, outFi.absoluteFilePath(), fmt); + if (result != 0) { + SentryReporter::captureMessage(QString("CLI anim: rename export failed (.%1)").arg(outFi.suffix()), "error"); + err() << "Error: Export failed." << Qt::endl; + return 1; + } } cliWrite(QString("Renamed animation '%1' -> '%2'\nOutput: %3\n").arg(oldName, newName, outFi.fileName())); @@ -2411,10 +2746,22 @@ int CLIPipeline::cmdScan(int argc, char* argv[]) config.failOn = failOn; } + auto stripOuterQuotesToken = [](QString s) { + s = s.trimmed(); + if (s.size() >= 2) { + const QChar a = s.front(), b = s.back(); + if ((a == '"' && b == '"') || (a == '\'' && b == '\'')) + s = s.mid(1, s.size() - 2).trimmed(); + } + return s; + }; + if (!includeArg.isEmpty()) { config.includePatterns.clear(); for (const auto& p : includeArg.split(",")) { - QString pattern = p.trimmed(); + QString pattern = stripOuterQuotesToken(p); + if (pattern.isEmpty()) + continue; // Normalize bare extension patterns: *.fbx โ†’ **/*.fbx if (!pattern.contains("/") && !pattern.startsWith("**/")) pattern = "**/" + pattern; @@ -2423,7 +2770,9 @@ int CLIPipeline::cmdScan(int argc, char* argv[]) } if (!excludeArg.isEmpty()) { for (const auto& p : excludeArg.split(",")) { - QString pattern = p.trimmed(); + QString pattern = stripOuterQuotesToken(p); + if (pattern.isEmpty()) + continue; if (!pattern.contains("/") && !pattern.startsWith("**/")) pattern = "**/" + pattern; config.excludePatterns.append(pattern); diff --git a/src/FBX/FBXExporter.cpp b/src/FBX/FBXExporter.cpp index 569add9e9..018dfe087 100644 --- a/src/FBX/FBXExporter.cpp +++ b/src/FBX/FBXExporter.cpp @@ -474,6 +474,29 @@ class FBXDocumentBuilder return true; } + bool buildSkeletonOnly(const Ogre::Skeleton* skeleton) + { + if (!skeleton) return false; + m_skeletonOnly = true; + m_hasSkeleton = true; + m_skeleton = const_cast(skeleton); + m_entity = nullptr; + m_mesh = nullptr; + + m_skeleton->reset(); + + m_w.writeHeader(); + writeHeaderExtension(); + writeGlobalSettings(); + writeDocuments(); + writeReferences(); + writeDefinitions(); + writeObjects(); + writeConnections(); + m_w.writeFooter(); + return true; + } + private: int64_t nextId() { return m_nextId++; } @@ -607,8 +630,8 @@ class FBXDocumentBuilder { // Count object types int defCount = 1; // GlobalSettings always - int modelCount = m_mesh->getNumSubMeshes(); // one mesh model per submesh - int geomCount = m_mesh->getNumSubMeshes(); + int modelCount = (m_mesh ? static_cast(m_mesh->getNumSubMeshes()) : 0); // one mesh model per submesh + int geomCount = (m_mesh ? static_cast(m_mesh->getNumSubMeshes()) : 0); int matCount = 0; int deformerCount = 0; int nodeAttrCount = 0; @@ -623,18 +646,20 @@ class FBXDocumentBuilder // Count unique materials and textures std::set matNames; std::set texNames; - for (const auto* sub : m_entity->getSubEntities()) - { - auto mat = sub->getMaterial(); - matNames.insert(mat->getName()); - if (mat->getNumTechniques() > 0 && mat->getTechnique(0)->getNumPasses() > 0) + if (m_entity) { + for (const auto* sub : m_entity->getSubEntities()) { - auto* pass = mat->getTechnique(0)->getPass(0); - for (unsigned short ti = 0; ti < pass->getNumTextureUnitStates(); ++ti) + auto mat = sub->getMaterial(); + matNames.insert(mat->getName()); + if (mat->getNumTechniques() > 0 && mat->getTechnique(0)->getNumPasses() > 0) { - auto texName = pass->getTextureUnitState(ti)->getTextureName(); - if (!texName.empty()) - texNames.insert(texName); + auto* pass = mat->getTechnique(0)->getPass(0); + for (unsigned short ti = 0; ti < pass->getNumTextureUnitStates(); ++ti) + { + auto texName = pass->getTextureUnitState(ti)->getTextureName(); + if (!texName.empty()) + texNames.insert(texName); + } } } } @@ -650,16 +675,18 @@ class FBXDocumentBuilder poseCount = 1; // BindPose // Skin deformers (1 per submesh) + cluster deformers (1 per bone-per-submesh that has weights) - deformerCount = geomCount; // skin deformers - for (unsigned int si = 0; si < m_mesh->getNumSubMeshes(); ++si) - { - const auto* subMesh = m_mesh->getSubMesh(si); - const auto& boneAssignments = subMesh->useSharedVertices - ? m_mesh->getBoneAssignments() : subMesh->getBoneAssignments(); - std::set boneIndices; - for (const auto& [_, vba] : boneAssignments) - boneIndices.insert(vba.boneIndex); - deformerCount += static_cast(boneIndices.size()); + if (m_mesh) { + deformerCount = geomCount; // skin deformers + for (unsigned int si = 0; si < m_mesh->getNumSubMeshes(); ++si) + { + const auto* subMesh = m_mesh->getSubMesh(si); + const auto& boneAssignments = subMesh->useSharedVertices + ? m_mesh->getBoneAssignments() : subMesh->getBoneAssignments(); + std::set boneIndices; + for (const auto& [_, vba] : boneAssignments) + boneIndices.insert(vba.boneIndex); + deformerCount += static_cast(boneIndices.size()); + } } if (m_skeleton->getNumAnimations() > 0) @@ -745,14 +772,17 @@ class FBXDocumentBuilder m_w.beginNode("Objects"); m_w.endProperties(); - writeGeometryObjects(); - writeMeshModels(); - writeMaterialObjects(); - writeTextureObjects(); + if (!m_skeletonOnly) { + writeGeometryObjects(); + writeMeshModels(); + writeMaterialObjects(); + writeTextureObjects(); + } if (m_hasSkeleton) { writeBoneModels(); - writeSkinDeformers(); + if (!m_skeletonOnly) + writeSkinDeformers(); writeBindPose(); if (m_skeleton->getNumAnimations() > 0) writeAnimations(); @@ -1400,6 +1430,34 @@ class FBXDocumentBuilder sz[ki] = scl.z; } + // Per-channel time arrays: collapse flat curves to a single key so FBX payloads + // stay smaller without changing motion (constant channel == one sample). + std::vector tTx = times, tTy = times, tTz = times; + std::vector tRx = times, tRy = times, tRz = times; + std::vector tSx = times, tSy = times, tSz = times; + + auto compactIfFlat = [](std::vector& kt, std::vector& v, double eps) { + if (v.size() <= 1) + return; + const double r = v[0]; + for (size_t i = 1; i < v.size(); ++i) { + if (std::fabs(v[i] - r) > eps) + return; + } + kt.resize(1); + v.resize(1); + }; + + compactIfFlat(tTx, tx, 1e-6); + compactIfFlat(tTy, ty, 1e-6); + compactIfFlat(tTz, tz, 1e-6); + compactIfFlat(tRx, rxArr, 1e-4); // Euler degrees + compactIfFlat(tRy, ryArr, 1e-4); + compactIfFlat(tRz, rzArr, 1e-4); + compactIfFlat(tSx, sx, 1e-6); + compactIfFlat(tSy, sy, 1e-6); + compactIfFlat(tSz, sz, 1e-6); + // AnimationCurveNode T int64_t cnT = nextId(); writeAnimCurveNode(cnT, "T", "d|X", "d|Y", "d|Z", @@ -1471,15 +1529,15 @@ class FBXDocumentBuilder m_animCurveConns.push_back({curveId, curveNodeId, channel}); }; - writeCurve(times, tx, cnT, "d|X"); - writeCurve(times, ty, cnT, "d|Y"); - writeCurve(times, tz, cnT, "d|Z"); - writeCurve(times, rxArr, cnR, "d|X"); - writeCurve(times, ryArr, cnR, "d|Y"); - writeCurve(times, rzArr, cnR, "d|Z"); - writeCurve(times, sx, cnS, "d|X"); - writeCurve(times, sy, cnS, "d|Y"); - writeCurve(times, sz, cnS, "d|Z"); + writeCurve(tTx, tx, cnT, "d|X"); + writeCurve(tTy, ty, cnT, "d|Y"); + writeCurve(tTz, tz, cnT, "d|Z"); + writeCurve(tRx, rxArr, cnR, "d|X"); + writeCurve(tRy, ryArr, cnR, "d|Y"); + writeCurve(tRz, rzArr, cnR, "d|Z"); + writeCurve(tSx, sx, cnS, "d|X"); + writeCurve(tSy, sy, cnS, "d|Y"); + writeCurve(tSz, sz, cnS, "d|Z"); } } } @@ -1735,18 +1793,20 @@ class FBXDocumentBuilder { // Track (texName, matName, fbxProperty) tuples std::set> texMatPairs; - for (const auto* sub : m_entity->getSubEntities()) - { - auto mat = sub->getMaterial(); - if (mat->getNumTechniques() == 0 || mat->getTechnique(0)->getNumPasses() == 0) continue; - auto* pass = mat->getTechnique(0)->getPass(0); - for (unsigned short ti = 0; ti < pass->getNumTextureUnitStates(); ++ti) + if (m_entity) { + for (const auto* sub : m_entity->getSubEntities()) { - auto* tus = pass->getTextureUnitState(ti); - std::string texName = tus->getTextureName(); - if (!texName.empty()) { - std::string fbxProp = (tus->getName() == "normal_map") ? "NormalMap" : "DiffuseColor"; - texMatPairs.insert({texName, mat->getName(), fbxProp}); + auto mat = sub->getMaterial(); + if (mat->getNumTechniques() == 0 || mat->getTechnique(0)->getNumPasses() == 0) continue; + auto* pass = mat->getTechnique(0)->getPass(0); + for (unsigned short ti = 0; ti < pass->getNumTextureUnitStates(); ++ti) + { + auto* tus = pass->getTextureUnitState(ti); + std::string texName = tus->getTextureName(); + if (!texName.empty()) { + std::string fbxProp = (tus->getName() == "normal_map") ? "NormalMap" : "DiffuseColor"; + texMatPairs.insert({texName, mat->getName(), fbxProp}); + } } } } @@ -1787,15 +1847,17 @@ class FBXDocumentBuilder } } - // Skin โ†’ Geometry - for (size_t i = 0; i < m_skinIds.size() && i < m_geomIds.size(); ++i) - writeConnection("OO", m_skinIds[i], m_geomIds[i]); + if (!m_skeletonOnly) { + // Skin โ†’ Geometry + for (size_t i = 0; i < m_skinIds.size() && i < m_geomIds.size(); ++i) + writeConnection("OO", m_skinIds[i], m_geomIds[i]); - // Cluster โ†’ Skin, Bone โ†’ Cluster - for (const auto& cc : m_clusterConnections) - { - writeConnection("OO", cc.clusterId, cc.skinId); - writeConnection("OO", m_boneModelIds[cc.boneHandle], cc.clusterId); + // Cluster โ†’ Skin, Bone โ†’ Cluster + for (const auto& cc : m_clusterConnections) + { + writeConnection("OO", cc.clusterId, cc.skinId); + writeConnection("OO", m_boneModelIds[cc.boneHandle], cc.clusterId); + } } // AnimationStack โ†’ scene root @@ -1979,6 +2041,7 @@ class FBXDocumentBuilder const Ogre::Mesh* m_mesh = nullptr; Ogre::Skeleton* m_skeleton = nullptr; bool m_hasSkeleton = false; + bool m_skeletonOnly = false; int64_t m_nextId = 1000000; int64_t m_documentId = 100000; @@ -2052,3 +2115,31 @@ bool FBXExporter::exportFBX(const Ogre::Entity* entity, const QString& filePath) return ok; } + +bool FBXExporter::exportSkeletonOnlyFBX(const Ogre::Skeleton* skeleton, const QString& filePath) +{ + if (!skeleton || filePath.isEmpty()) + return false; + + std::ofstream out(filePath.toStdString(), std::ios::binary); + if (!out.is_open()) + { + Ogre::LogManager::getSingleton().logError( + "FBXExporter: failed to open " + filePath.toStdString() + " for writing"); + return false; + } + + FBXBinaryWriter writer(out); + FBXDocumentBuilder builder(writer); + bool ok = builder.buildSkeletonOnly(skeleton); + + out.close(); + + if (!ok) + { + Ogre::LogManager::getSingleton().logError( + "FBXExporter: failed to build skeleton-only FBX document"); + } + + return ok; +} diff --git a/src/FBX/FBXExporter.h b/src/FBX/FBXExporter.h index 4860f0c8b..f018db573 100644 --- a/src/FBX/FBXExporter.h +++ b/src/FBX/FBXExporter.h @@ -30,12 +30,16 @@ THE SOFTWARE. #define FBXEXPORTER_H #include +#include #include class FBXExporter { public: static bool exportFBX(const Ogre::Entity* entity, const QString& filePath); + /// Export an FBX containing only a skeleton + animations (no geometry). + /// This is intended for animation-only FBX round-tripping without introducing dummy meshes. + static bool exportSkeletonOnlyFBX(const Ogre::Skeleton* skeleton, const QString& filePath); }; #endif // FBXEXPORTER_H diff --git a/src/FBX/FBXExporter_test.cpp b/src/FBX/FBXExporter_test.cpp index da601279d..fd08870c5 100644 --- a/src/FBX/FBXExporter_test.cpp +++ b/src/FBX/FBXExporter_test.cpp @@ -2297,6 +2297,53 @@ TEST_F(FBXExporterCoverageTest, ExportAnimatedMesh_PreservesAnimations) { cleanup(r); } +TEST_F(FBXExporterCoverageTest, ExportSkeletonOnly_CreatesSkeletonAndAnimations) +{ + // Build a skeleton with two bones and a simple animation, without any mesh/entity. + auto skel = Ogre::SkeletonManager::getSingleton().create( + "SkelOnlyTest", Ogre::ResourceGroupManager::DEFAULT_RESOURCE_GROUP_NAME, true); + auto* root = skel->createBone("Root", 0); + root->setPosition(0, 0, 0); + auto* child = skel->createBone("Child", 1); + child->setPosition(0, 1, 0); + root->addChild(child); + + auto* anim = skel->createAnimation("wave", 1.0f); + auto* track = anim->createNodeTrack(0, child); + { + auto* k0 = track->createNodeKeyFrame(0.0f); + k0->setTranslate(Ogre::Vector3(0, 1, 0)); + k0->setRotation(Ogre::Quaternion::IDENTITY); + k0->setScale(Ogre::Vector3::UNIT_SCALE); + auto* k1 = track->createNodeKeyFrame(1.0f); + k1->setTranslate(Ogre::Vector3(0, 1, 0)); + k1->setRotation(Ogre::Quaternion(Ogre::Degree(45), Ogre::Vector3::UNIT_Z)); + k1->setScale(Ogre::Vector3::UNIT_SCALE); + } + + const QString outPath = QDir(QDir::tempPath()).filePath(QString("fbx_skeleton_only_%1.fbx").arg(meshCounter)); + ASSERT_TRUE(FBXExporter::exportSkeletonOnlyFBX(skel.get(), outPath)); + + auto nodes = parseFBX(outPath.toStdString()); + ASSERT_FALSE(nodes.empty()); + auto* objects = findTopLevel(nodes, "Objects"); + ASSERT_NE(objects, nullptr); + + // Bone models & skeleton node attributes should exist. + auto models = objects->findAll("Model"); + auto nodeAttrs = objects->findAll("NodeAttribute"); + EXPECT_GE(models.size(), 2); + EXPECT_GE(nodeAttrs.size(), 2); + + // Animation objects should exist. + auto animStacks = objects->findAll("AnimationStack"); + auto animCurves = objects->findAll("AnimationCurve"); + EXPECT_GE(animStacks.size(), 1); + EXPECT_GE(animCurves.size(), 1); + + QFile::remove(outPath); +} + // ========================================================================== // NEW TESTS: Error handling // ========================================================================== diff --git a/src/OgreWidget.cpp b/src/OgreWidget.cpp index 0f94109f2..dea5529c9 100755 --- a/src/OgreWidget.cpp +++ b/src/OgreWidget.cpp @@ -188,12 +188,12 @@ void OgreWidget::initOgreWindow(void) params["macAPICocoaUseNSView"] = "true"; #endif - { - QSettings settings; - const int fsaa = settings.value(ViewportSettingsKeys::fsaaSamples(), 4).toInt(); - if (fsaa > 0) - params["FSAA"] = Ogre::StringConverter::toString(fsaa); - } + QSettings settings; + // Default FSAA=0 on Linux: MSAA support varies by driver/setup and can cause + // a black viewport in some installed environments. Users can opt-in via settings. + const int requestedFsaa = settings.value(ViewportSettingsKeys::fsaaSamples(), 0).toInt(); + if (requestedFsaa > 0) + params["FSAA"] = Ogre::StringConverter::toString(requestedFsaa); QString name = "Viewport " + QString::number(getIndex()); while (mOgreRoot->getRenderTarget(name.toStdString())) { diff --git a/src/OgreWidget_test.cpp b/src/OgreWidget_test.cpp index 8d7d8a0ee..58af1a093 100644 --- a/src/OgreWidget_test.cpp +++ b/src/OgreWidget_test.cpp @@ -247,3 +247,14 @@ TEST_F(OgreWidgetTest, RebuildRenderWindowPreservesBackgroundAndKeepsCamera) EXPECT_NEAR(widget->getSpaceCamera()->getCamera()->getNearClipDistance(), 0.05, 1e-5); EXPECT_NEAR(widget->getSpaceCamera()->getCamera()->getFarClipDistance(), 5000.0, 1e-3); } + +TEST_F(OgreWidgetTest, FsaaDefaultsToZeroWhenUnset) +{ + QSettings settings; + settings.remove(ViewportSettingsKeys::fsaaSamples()); + + EXPECT_NO_THROW(widget->rebuildRenderWindow()); + app->processEvents(); + + EXPECT_EQ(widget->fsaaSamples(), 0u); +} diff --git a/src/ScanConfig.h b/src/ScanConfig.h index d7775e624..1ddd09035 100644 --- a/src/ScanConfig.h +++ b/src/ScanConfig.h @@ -21,7 +21,8 @@ struct ScanConfig { /// Glob patterns; default ctor fills with all Assimp import extensions (plus Ogre .mesh / .mesh.xml). QStringList includePatterns; QStringList excludePatterns = { - "**/node_modules/**", "**/.git/**", "**/build/**", "**/Build/**" + "**/node_modules/**", "**/.git/**", "**/build/**", "**/Build/**", + "**/dist/**", "**/out/**", "**/.next/**", "**/target/**", "**/.cache/**" }; // rules section โ€” existence checks @@ -53,7 +54,7 @@ struct ScanConfig { // Redundant-keyframe detection. When enabled, the scanner analyzes each // animation's keyframes and warns if a meaningful share could be safely // removed via tolerance-based simplification. Defaults disable the check. - double redundantKeyframesPctThreshold = 0.0; // 0 = disabled; e.g. 30.0 = warn at >=30% + double redundantKeyframesPctThreshold = 40.0; // 0 = disabled; default warns at >=40% redundant keys double redundantKeyframesTranslationTol = 1e-3; // Balanced preset (~1mm) double redundantKeyframesRotationDegTol = 0.5; double redundantKeyframesScaleTol = 1e-3; diff --git a/src/ScanEngine.cpp b/src/ScanEngine.cpp index 24a25f763..cb01fcd65 100644 --- a/src/ScanEngine.cpp +++ b/src/ScanEngine.cpp @@ -9,20 +9,114 @@ #include #include #include +#include +#include #include +#include #include +#include +#include #include #include #include #include "SentryReporter.h" +#include "Manager.h" +#include "SelectionSet.h" +#include "MeshImporterExporter.h" +#include "AnimationMerger.h" +#include "FBX/FBXExporter.h" + +#include #include #include #include #include +namespace { + +bool ensureOgreHeadlessQuiet() +{ + // ScanEngine is normally Assimp-only and shouldn't spam Ogre logs when a fix path + // needs Ogre. Mirror CLIPipeline's default behavior: suppress debug output unless + // the user explicitly asked for verbose logs (ScanEngine has no --verbose flag). + if (!Ogre::LogManager::getSingletonPtr()) { + auto* logMgr = new Ogre::LogManager(); + logMgr->createLog("ogre.log", true, false, true); // default, debugOut=false, suppressFile=true + } else { + auto* log = Ogre::LogManager::getSingleton().getDefaultLog(); + if (log) + log->setDebugOutputEnabled(false); + } + + try { + Manager::getSingleton(); + // ScanEngine runs inside CLI/GUI processes where Qt is already initialized, + // but Ogre still needs a render target for hardware buffers to exist. + auto* root = Manager::getSingleton()->getRoot(); + if (root) { + try { + if (root->getRenderTarget("ScanHidden") || root->getRenderTarget("CLIHidden") || root->getRenderTarget("TestHidden")) + return true; + } catch (...) { + // ignore + } + } + + static QWidget* hiddenWidget = nullptr; + if (!hiddenWidget) { + hiddenWidget = new QWidget(); + hiddenWidget->setAttribute(Qt::WA_DontShowOnScreen); + hiddenWidget->resize(1, 1); + hiddenWidget->show(); + } + + try { + Ogre::NameValuePairList params; + params["externalWindowHandle"] = Ogre::StringConverter::toString( + static_cast(hiddenWidget->winId())); +#ifdef Q_OS_MACOS + params["macAPI"] = "cocoa"; + params["macAPICocoaUseNSView"] = "true"; +#endif + Manager::getSingleton()->getRoot()->createRenderWindow( + "ScanHidden", 1, 1, false, ¶ms); + } catch (...) { + return false; + } + return true; + } catch (...) { + return false; + } +} + +// MeshImporterExporter::importer appends to the scene without clearing prior imports. +// Scan --fix runs multiple FBX imports in one process; stale entities/skeleton +// handles would make entity/skeleton selection wrong and can corrupt Ogre state. +static void clearOgreSceneForScanImport() +{ + if (!Manager::getSingletonPtr()) + return; + SelectionSet::getSingleton()->clearList(); + auto* manager = Manager::getSingleton(); + const QList sceneNodesCopy = manager->getSceneNodes(); + for (auto* sn : sceneNodesCopy) { + if (sn) + manager->destroySceneNode(sn); + } +} + +} // namespace + +static bool pathEndsWithInsensitive(const QString& p, QLatin1String suf) +{ + if (p.size() < suf.size()) + return false; + return p.endsWith(suf, Qt::CaseInsensitive); +} + // --------------------------------------------------------------------------- // Glob matching // --------------------------------------------------------------------------- @@ -69,7 +163,23 @@ QStringList ScanEngine::enumerateFiles(const ScanConfig& config, const QString& QDir rootDir(scanRoot); if (!rootDir.exists()) return result; - QDirIterator it(rootDir.absolutePath(), QDir::Files | QDir::NoDotAndDotDot, + QStringList nameFilters; + bool useNameFilters = !config.includePatterns.isEmpty(); + static const QRegularExpression simpleExtRe(QStringLiteral(R"(^\*\*/\*\.([a-zA-Z0-9]+)$)")); + for (const auto& pattern : config.includePatterns) { + const auto m = simpleExtRe.match(pattern); + if (!m.hasMatch()) { + useNameFilters = false; + break; + } + nameFilters << QStringLiteral("*.%1").arg(m.captured(1)); + } + if (useNameFilters && nameFilters.isEmpty()) + useNameFilters = false; + + QDirIterator it(rootDir.absolutePath(), + useNameFilters ? nameFilters : QStringList(), + QDir::Files | QDir::NoDotAndDotDot, QDirIterator::Subdirectories); while (it.hasNext()) { @@ -133,7 +243,7 @@ struct NodeKey { static aiVector3D sampleVecKeys(const aiVectorKey* keys, unsigned n, double t, const aiVector3D& fallback) { - if (n == 0) return fallback; + if (n == 0 || !keys) return fallback; if (n == 1 || t <= keys[0].mTime) return keys[0].mValue; if (t >= keys[n-1].mTime) return keys[n-1].mValue; // Linear scan โ€” channel arrays are short (~hundreds), no need for binary search. @@ -151,7 +261,7 @@ static aiVector3D sampleVecKeys(const aiVectorKey* keys, unsigned n, double t, static aiQuaternion sampleQuatKeys(const aiQuatKey* keys, unsigned n, double t, const aiQuaternion& fallback) { - if (n == 0) return fallback; + if (n == 0 || !keys) return fallback; if (n == 1 || t <= keys[0].mTime) return keys[0].mValue; if (t >= keys[n-1].mTime) return keys[n-1].mValue; for (unsigned i = 1; i < n; ++i) { @@ -170,14 +280,22 @@ static aiQuaternion sampleQuatKeys(const aiQuatKey* keys, unsigned n, double t, // Union of distinct times across T/R/S streams for one aiNodeAnim, in order. static std::vector unionTimes(const aiNodeAnim* ch) { + if (!ch) + return {}; std::vector times; times.reserve(ch->mNumPositionKeys + ch->mNumRotationKeys + ch->mNumScalingKeys); - for (unsigned i = 0; i < ch->mNumPositionKeys; ++i) - times.push_back(ch->mPositionKeys[i].mTime); - for (unsigned i = 0; i < ch->mNumRotationKeys; ++i) - times.push_back(ch->mRotationKeys[i].mTime); - for (unsigned i = 0; i < ch->mNumScalingKeys; ++i) - times.push_back(ch->mScalingKeys[i].mTime); + if (ch->mPositionKeys) { + for (unsigned i = 0; i < ch->mNumPositionKeys; ++i) + times.push_back(ch->mPositionKeys[i].mTime); + } + if (ch->mRotationKeys) { + for (unsigned i = 0; i < ch->mNumRotationKeys; ++i) + times.push_back(ch->mRotationKeys[i].mTime); + } + if (ch->mScalingKeys) { + for (unsigned i = 0; i < ch->mNumScalingKeys; ++i) + times.push_back(ch->mScalingKeys[i].mTime); + } std::sort(times.begin(), times.end()); times.erase(std::unique(times.begin(), times.end(), [](double a, double b) { return std::fabs(a - b) < 1e-7; }), times.end()); @@ -276,8 +394,15 @@ static void analyzeAnimationRedundancy(const aiAnimation* anim, { int total = 0; int redundant = 0; + if (!anim || !anim->mChannels) { + if (outTotal) *outTotal = 0; + if (outRedundant) *outRedundant = 0; + return; + } for (unsigned c = 0; c < anim->mNumChannels; ++c) { const aiNodeAnim* ch = anim->mChannels[c]; + if (!ch) + continue; const std::vector times = unionTimes(ch); if (times.empty()) continue; @@ -345,9 +470,17 @@ AssetInfo ScanEngine::inspectAsset(const QString& filePath, const QString& scanR importer.SetPropertyBool(AI_CONFIG_IMPORT_FBX_PRESERVE_PIVOTS, false); // Triangulate for consistent vertex/face counts; otherwise minimal processing. - const aiScene* scene = importer.ReadFile( - filePath.toStdString(), - aiProcess_Triangulate | aiProcess_ValidateDataStructure); + unsigned int readFlags = aiProcess_Triangulate | aiProcess_ValidateDataStructure; + const aiScene* scene = importer.ReadFile(filePath.toStdString(), readFlags); + + if (isAssimpResultLoadFailure(scene, importer.GetErrorString(), nullptr) && + (pathEndsWithInsensitive(filePath, QLatin1String(".fbx")) || + pathEndsWithInsensitive(filePath, QLatin1String(".fbxa")))) { + readFlags = aiProcess_Triangulate | aiProcess_ValidateDataStructure | + aiProcess_LimitBoneWeights | aiProcess_PopulateArmatureData | + aiProcess_GlobalScale; + scene = importer.ReadFile(filePath.toStdString(), readFlags); + } // A null scene is a true load failure. Do NOT treat AI_SCENE_FLAGS_INCOMPLETE // as fatal: Assimp sets it on many valid FBX files (e.g. Unreal/ Mixamo @@ -366,10 +499,15 @@ AssetInfo ScanEngine::inspectAsset(const QString& filePath, const QString& scanR std::set uniqueBones; for (unsigned i = 0; i < scene->mNumMeshes; ++i) { const aiMesh* mesh = scene->mMeshes[i]; + if (!mesh) + continue; info.vertexCount += mesh->mNumVertices; info.faceCount += mesh->mNumFaces; - for (unsigned b = 0; b < mesh->mNumBones; ++b) + for (unsigned b = 0; b < mesh->mNumBones; ++b) { + if (!mesh->mBones[b]) + continue; uniqueBones.insert(mesh->mBones[b]->mName.C_Str()); + } } info.boneCount = static_cast(uniqueBones.size()); info.hasSkeleton = !uniqueBones.empty(); @@ -379,17 +517,23 @@ AssetInfo ScanEngine::inspectAsset(const QString& filePath, const QString& scanR // Animation details for (unsigned i = 0; i < scene->mNumAnimations; ++i) { const aiAnimation* anim = scene->mAnimations[i]; + if (!anim) + continue; info.animationNames.append(QString::fromUtf8(anim->mName.C_Str())); double ticksPerSec = anim->mTicksPerSecond > 0 ? anim->mTicksPerSecond : 25.0; info.animationDurations.append(anim->mDuration / ticksPerSec); unsigned maxKeys = 0; - for (unsigned c = 0; c < anim->mNumChannels; ++c) { - const aiNodeAnim* ch = anim->mChannels[c]; - maxKeys = std::max(maxKeys, ch->mNumPositionKeys); - maxKeys = std::max(maxKeys, ch->mNumRotationKeys); - maxKeys = std::max(maxKeys, ch->mNumScalingKeys); + if (anim->mChannels) { + for (unsigned c = 0; c < anim->mNumChannels; ++c) { + const aiNodeAnim* ch = anim->mChannels[c]; + if (!ch) + continue; + maxKeys = std::max(maxKeys, ch->mNumPositionKeys); + maxKeys = std::max(maxKeys, ch->mNumRotationKeys); + maxKeys = std::max(maxKeys, ch->mNumScalingKeys); + } } info.animationKeyframeCounts.append(static_cast(maxKeys)); @@ -403,6 +547,10 @@ AssetInfo ScanEngine::inspectAsset(const QString& filePath, const QString& scanR info.redundantKeyframes += animRedundant; } + info.animationRedundantKeyframeRatio = (info.totalKeyframes > 0) + ? static_cast(info.redundantKeyframes) / static_cast(info.totalKeyframes) + : 0.0; + // Material names + texture references for (unsigned i = 0; i < scene->mNumMaterials; ++i) { const aiMaterial* mat = scene->mMaterials[i]; @@ -710,6 +858,8 @@ QList ScanEngine::evaluateRules(const AssetInfo& asset, const ScanConfi int total = 0; int redundant = 0; for (unsigned i = 0; i < scene->mNumAnimations; ++i) { + if (!scene->mAnimations[i]) + continue; int t = 0, r = 0; analyzeAnimationRedundancy(scene->mAnimations[i], tol, &t, &r); total += t; @@ -739,11 +889,13 @@ QList ScanEngine::evaluateRules(const AssetInfo& asset, const ScanConfi findings.append({asset.relativePath, "redundant_keyframes_pct", Severity::Warning, QString("%1% redundant keyframes (%2/%3). Simplify it to save ~%4. " - "Original size: %5, projected size: %6") + "Original size: %5, projected size: %6. " + "Run `qtmesh scan ... --fix` to apply (FBX uses the same simplify as `qtmesh anim --simplify`; use `--dry-run` to preview).") .arg(pct, 0, 'f', 1).arg(redundant).arg(total) .arg(formatBytes(savedBytes)) .arg(formatBytes(originalSize)) - .arg(formatBytes(projectedSize))}); + .arg(formatBytes(projectedSize)), + /*fixable=*/true}); } } } @@ -770,7 +922,138 @@ QList ScanEngine::evaluateRules(const AssetInfo& asset, const ScanConfi // Auto-fixes // --------------------------------------------------------------------------- -void ScanEngine::applyFixes(const ScanConfig& config, AssetInfo& asset, +namespace { + +bool vecApproxEqualAssimp(const aiVector3D& a, const aiVector3D& b, float eps = 1e-6f) +{ + return (std::fabs(a.x - b.x) < eps && std::fabs(a.y - b.y) < eps && std::fabs(a.z - b.z) < eps); +} + +bool quatApproxEqualAssimp(const aiQuaternion& a, const aiQuaternion& b, float eps = 1e-3f) +{ + const float d = std::fabs(a.w * b.w + a.x * b.x + a.y * b.y + a.z * b.z); + return d > 1.f - eps; +} + +void compactVectorTrackInPlace(aiVectorKey*& keys, unsigned& n) +{ + if (n <= 2 || !keys) + return; + std::vector out; + out.reserve(n); + out.push_back(keys[0]); + for (unsigned i = 1; i + 1 < n; ++i) { + if (!vecApproxEqualAssimp(keys[i].mValue, out.back().mValue)) + out.push_back(keys[i]); + } + if (!vecApproxEqualAssimp(keys[n - 1].mValue, out.back().mValue)) + out.push_back(keys[n - 1]); + if (out.size() == 1 && n >= 2) { + out.clear(); + out.push_back(keys[0]); + out.push_back(keys[n - 1]); + } + const unsigned newN = static_cast(out.size()); + if (newN == n) + return; + auto* nk = new aiVectorKey[newN]; + for (unsigned i = 0; i < newN; ++i) + nk[i] = out[i]; + delete[] keys; + keys = nk; + n = newN; +} + +void compactQuatTrackInPlace(aiQuatKey*& keys, unsigned& n) +{ + if (n <= 2 || !keys) + return; + std::vector out; + out.reserve(n); + out.push_back(keys[0]); + for (unsigned i = 1; i + 1 < n; ++i) { + if (!quatApproxEqualAssimp(keys[i].mValue, out.back().mValue)) + out.push_back(keys[i]); + } + if (!quatApproxEqualAssimp(keys[n - 1].mValue, out.back().mValue)) + out.push_back(keys[n - 1]); + if (out.size() == 1 && n >= 2) { + out.clear(); + out.push_back(keys[0]); + out.push_back(keys[n - 1]); + } + const unsigned newN = static_cast(out.size()); + if (newN == n) + return; + auto* nk = new aiQuatKey[newN]; + for (unsigned i = 0; i < newN; ++i) + nk[i] = out[i]; + delete[] keys; + keys = nk; + n = newN; +} + +void stripRedundantAnimKeys(aiScene* scene) +{ + if (!scene) + return; + for (unsigned ai = 0; ai < scene->mNumAnimations; ++ai) { + aiAnimation* anim = scene->mAnimations[ai]; + if (!anim || !anim->mChannels) + continue; + for (unsigned c = 0; c < anim->mNumChannels; ++c) { + aiNodeAnim* ch = anim->mChannels[c]; + if (!ch) + continue; + compactVectorTrackInPlace(ch->mPositionKeys, ch->mNumPositionKeys); + compactQuatTrackInPlace(ch->mRotationKeys, ch->mNumRotationKeys); + compactVectorTrackInPlace(ch->mScalingKeys, ch->mNumScalingKeys); + } + } +} + +long long totalAnimKeysForScene(const aiScene* scene) +{ + if (!scene) + return 0; + long long total = 0; + for (unsigned ai = 0; ai < scene->mNumAnimations; ++ai) { + const aiAnimation* anim = scene->mAnimations[ai]; + if (!anim || !anim->mChannels) + continue; + for (unsigned c = 0; c < anim->mNumChannels; ++c) { + const aiNodeAnim* ch = anim->mChannels[c]; + if (!ch) + continue; + total += static_cast(ch->mNumPositionKeys); + total += static_cast(ch->mNumRotationKeys); + total += static_cast(ch->mNumScalingKeys); + } + } + return total; +} + +QString assimpExportFormatIdForAssetPath(const QString& outputPath) +{ + const QString suf = QFileInfo(outputPath).suffix().toLower(); + static const QMap fromExt{ + {QStringLiteral("fbx"), QStringLiteral("fbx")}, + {QStringLiteral("dae"), QStringLiteral("collada")}, + {QStringLiteral("obj"), QStringLiteral("obj")}, + {QStringLiteral("stl"), QStringLiteral("stl")}, + {QStringLiteral("ply"), QStringLiteral("ply")}, + {QStringLiteral("3ds"), QStringLiteral("3ds")}, + {QStringLiteral("gltf"), QStringLiteral("gltf2")}, + {QStringLiteral("glb"), QStringLiteral("glb2")}, + {QStringLiteral("assbin"), QStringLiteral("assbin")}, + {QStringLiteral("x"), QStringLiteral("x")}, + }; + return fromExt.value(suf, QStringLiteral("fbx")); +} + +} // namespace + +void ScanEngine::applyFixes(const ScanConfig& config, const QString& scanRoot, AssetInfo& asset, QList& findings) { if (!config.fixEnabled) return; @@ -801,6 +1084,217 @@ void ScanEngine::applyFixes(const ScanConfig& config, AssetInfo& asset, f.message += " [fix failed: could not rename file]"; } } + } else if (f.rule == "redundant_keyframes_pct") { + if (config.dryRun) { + f.message += QStringLiteral(" [dry-run: would simplify animation keys (same as qtmesh anim --simplify)]"); + continue; + } + const QString suf = QFileInfo(asset.filePath).suffix().toLower(); + if (suf == QStringLiteral("fbx") || suf == QStringLiteral("fbxa")) { + // Ogre + tolerance-based simplify (same core as `qtmesh anim --simplify`) + custom FBX exporter. + if (!ensureOgreHeadlessQuiet()) { + f.message = QStringLiteral("Fix failed: could not initialize Ogre"); + continue; + } + + clearOgreSceneForScanImport(); + + QList animOnlySkeletons; + MeshImporterExporter::importer({asset.filePath}, 0, &animOnlySkeletons); + + auto& ents = Manager::getSingleton()->getEntities(); + Ogre::Entity* entity = ents.isEmpty() ? nullptr : ents.last(); + Ogre::SkeletonPtr skel; + if (entity && entity->hasSkeleton()) + skel = entity->getMesh()->getSkeleton(); + if (!skel && !animOnlySkeletons.isEmpty()) + skel = animOnlySkeletons.last(); + + if (!skel) { + f.message = QStringLiteral("Fix failed: could not load skeleton via Ogre importer"); + continue; + } + + AnimationMerger::SimplifyTolerances tol; + tol.translation = static_cast(config.redundantKeyframesTranslationTol); + tol.rotationDeg = static_cast(config.redundantKeyframesRotationDegTol); + tol.scale = static_cast(config.redundantKeyframesScaleTol); + + // Compute total keyframes before simplifying (for reporting). + long long totalKeysBefore = 0; + for (unsigned short ai = 0; ai < skel->getNumAnimations(); ++ai) { + const Ogre::Animation* a = skel->getAnimation(ai); + if (!a) continue; + for (const auto& [handle, track] : a->_getNodeTrackList()) { + Q_UNUSED(handle); + if (!track) continue; + totalKeysBefore += track->getNumKeyFrames(); + } + } + + int totalRemoved = 0; + const unsigned short numAnims = skel->getNumAnimations(); + std::vector animNames; + animNames.reserve(numAnims); + for (unsigned short ai = 0; ai < numAnims; ++ai) + animNames.push_back(skel->getAnimation(ai)->getName()); + + for (const auto& name : animNames) + totalRemoved += AnimationMerger::simplifyAnimation(skel.get(), name, tol); + + if (totalRemoved <= 0) { + f.message = QStringLiteral("Fix not needed: no additional simplification within configured tolerances"); + f.severity = Severity::Info; + f.skipped = true; + continue; + } + + // Export to a temp path first and only apply if it improves size. + const qint64 originalBytes = QFileInfo(asset.filePath).size(); + const QString tmpPath = asset.filePath + QStringLiteral(".qtmesh-simplify.tmp"); + if (QFile::exists(tmpPath)) + QFile::remove(tmpPath); + + bool ok = false; + if (!entity) { + ok = FBXExporter::exportSkeletonOnlyFBX(skel.get(), tmpPath); + } else { + entity->refreshAvailableAnimationState(); + auto* node = entity->getParentSceneNode(); + ok = (MeshImporterExporter::exporter(node, tmpPath, QStringLiteral("FBX Binary (*.fbx)")) == 0); + } + + if (!ok) { + QFile::remove(tmpPath); + f.message = QStringLiteral("Fix failed: FBX export failed"); + continue; + } + + const qint64 rewrittenBytes = QFileInfo(tmpPath).size(); + // The goal of this fix is to reduce animation payload. If the rewritten file + // isn't smaller, keep the original to avoid regressions from exporter variance. + if (originalBytes > 0 && rewrittenBytes >= originalBytes) { + QFile::remove(tmpPath); + f.message = QStringLiteral("output would be larger (%1 KB -> %2 KB), keeping original") + .arg(originalBytes / 1024) + .arg(rewrittenBytes / 1024); + f.severity = Severity::Info; + f.skipped = true; + continue; + } + + QFile orig(asset.filePath); + if (!orig.remove()) { + QFile::remove(tmpPath); + f.message = QStringLiteral("Fix failed: could not replace original file"); + continue; + } + if (!QFile::rename(tmpPath, asset.filePath)) { + f.message = QStringLiteral("Fix failed: could not install rewritten file"); + continue; + } + + SentryReporter::addBreadcrumb(QStringLiteral("file.export"), + QStringLiteral("Scan fix: simplified anim keys (custom FBX): %1").arg(asset.relativePath)); + const qint64 savedBytes = (originalBytes > 0 && rewrittenBytes > 0) ? (originalBytes - rewrittenBytes) : 0; + const double keysPct = (totalKeysBefore > 0) + ? (static_cast(totalRemoved) * 100.0 / static_cast(totalKeysBefore)) + : 0.0; + const double sizePct = (originalBytes > 0) + ? (static_cast(savedBytes) * 100.0 / static_cast(originalBytes)) + : 0.0; + f.severity = Severity::Info; + f.bytesSaved = savedBytes; + f.keysRemoved = totalRemoved; + f.message = QStringLiteral("removed %1/%2 keys (%3%), saved %4 KB (%5%)") + .arg(totalRemoved) + .arg(totalKeysBefore) + .arg(QString::number(keysPct, 'f', 1)) + .arg(savedBytes / 1024) + .arg(QString::number(sizePct, 'f', 1)); + f.fixed = true; + asset = inspectAsset(asset.filePath, scanRoot); + continue; + } + + const qint64 originalBytes = QFileInfo(asset.filePath).size(); + Assimp::Importer imp; + imp.SetPropertyBool(AI_CONFIG_IMPORT_FBX_PRESERVE_PIVOTS, false); + unsigned int impFlags = aiProcess_Triangulate | aiProcess_ValidateDataStructure; + const aiScene* loaded = imp.ReadFile(asset.filePath.toStdString(), impFlags); + if (isAssimpResultLoadFailure(loaded, imp.GetErrorString(), nullptr) && + (pathEndsWithInsensitive(asset.filePath, QLatin1String(".fbx")) || + pathEndsWithInsensitive(asset.filePath, QLatin1String(".fbxa")))) { + impFlags = aiProcess_Triangulate | aiProcess_ValidateDataStructure | + aiProcess_LimitBoneWeights | aiProcess_PopulateArmatureData | + aiProcess_GlobalScale; + loaded = imp.ReadFile(asset.filePath.toStdString(), impFlags); + } + if (isAssimpResultLoadFailure(loaded, imp.GetErrorString(), nullptr)) { + f.message += QStringLiteral(" [fix failed: could not re-read asset]"); + continue; + } + if (loaded->mNumAnimations == 0) { + f.message += QStringLiteral(" [fix failed: no animations in file]"); + continue; + } + aiScene* mutScene = const_cast(loaded); + const long long keysBefore = totalAnimKeysForScene(mutScene); + stripRedundantAnimKeys(mutScene); + const long long keysAfter = totalAnimKeysForScene(mutScene); + if (keysAfter >= keysBefore) { + f.message = QStringLiteral("Fix not needed: no reducible consecutive duplicate keys (per-channel)"); + f.severity = Severity::Info; + f.skipped = true; + continue; + } + + const QString tmpPath = asset.filePath + QStringLiteral(".qtmesh-strip.tmp"); + if (QFile::exists(tmpPath)) + QFile::remove(tmpPath); + + const QString formatId = assimpExportFormatIdForAssetPath(asset.filePath); + const unsigned int exportFlags = + (formatId == QLatin1String("x")) ? 0u : aiProcess_ConvertToLeftHanded; + + Assimp::Exporter exporter; + const aiReturn expRet = exporter.Export(mutScene, formatId.toStdString().c_str(), + tmpPath.toStdString().c_str(), exportFlags); + if (expRet != AI_SUCCESS) { + QFile::remove(tmpPath); + f.message += QStringLiteral(" [fix failed: export: %1]") + .arg(QString::fromUtf8(exporter.GetErrorString())); + continue; + } + const qint64 rewrittenBytes = QFileInfo(tmpPath).size(); + // Assimp FBX export is not size-stable; allow a small overhead so fixes still apply + // when they meaningfully improve animation data. Hard-skip large blowups. + const qint64 maxAllowedGrowthBytes = std::max(64 * 1024, originalBytes / 20); // max(64KB, 5%) + const qint64 maxAllowedBytes = (originalBytes > 0) ? (originalBytes + maxAllowedGrowthBytes) : 0; + if (originalBytes > 0 && rewrittenBytes > maxAllowedBytes) { + QFile::remove(tmpPath); + f.message = QStringLiteral("output would be larger (%1 KB -> %2 KB), keeping original") + .arg(originalBytes / 1024) + .arg(rewrittenBytes / 1024); + f.severity = Severity::Info; + f.skipped = true; + continue; + } + QFile orig(asset.filePath); + if (!orig.remove()) { + QFile::remove(tmpPath); + f.message = QStringLiteral("Fix failed: could not replace original file"); + continue; + } + if (!QFile::rename(tmpPath, asset.filePath)) { + f.message = QStringLiteral("Fix failed: could not install rewritten file"); + continue; + } + SentryReporter::addBreadcrumb(QStringLiteral("file.export"), + QStringLiteral("Scan fix: stripped consecutive duplicate anim keys (Assimp): %1").arg(asset.relativePath)); + f.message = QStringLiteral("Fixed: stripped consecutive duplicate animation keys (non-FBX path)"); + f.fixed = true; + asset = inspectAsset(asset.filePath, scanRoot); } } } @@ -843,15 +1337,25 @@ ScanResult ScanEngine::run(const ScanConfig& config, const QString& rootOverride QList findings = evaluateRules(asset, config); // Apply fixes where possible - applyFixes(config, asset, findings); + applyFixes(config, scanRoot, asset, findings); if (onAssetProcessed) onAssetProcessed(asset, findings); - // Tally โ€” fixed findings don't count toward error/warning totals + // Tally โ€” fixed findings don't count toward error/warning totals. + // Note: Finding::skipped is "fix attempted but intentionally skipped" and should + // still be considered a pass at the asset level (it doesn't mean the asset was + // skipped from scanning). bool hasError = false, hasWarning = false; + bool hasFixSkipped = false; for (const auto& f : findings) { - if (f.fixed) { result.fixed++; continue; } + if (f.fixed) { + result.fixed++; + result.bytesSaved += std::max(0, f.bytesSaved); + result.keysRemoved += std::max(0, f.keysRemoved); + continue; + } + if (f.skipped) { hasFixSkipped = true; continue; } switch (f.severity) { case Severity::Error: result.errors++; hasError = true; break; case Severity::Warning: result.warnings++; hasWarning = true; break; @@ -861,8 +1365,14 @@ ScanResult ScanEngine::run(const ScanConfig& config, const QString& rootOverride if (asset.loadError) result.skipped++; - else if (!hasError && !hasWarning) - result.passed++; + else { + // A fix-skip is still a "pass" (nothing failed), but we also want to + // surface that something was skipped in the summary. + if (!hasError && !hasWarning) + result.passed++; + if (hasFixSkipped) + result.skipped++; + } result.scanned++; result.findings.append(findings); @@ -927,7 +1437,7 @@ QString ScanEngine::formatText(const ScanResult& result, const ScanConfig& confi // Findings detail for (const auto& f : assetFindings) { - QString label = severityLabel(f.severity); + QString label = f.fixed ? QStringLiteral("FIXED") : severityLabel(f.severity); s << " [" << label.trimmed().toLower() << "] " << f.rule << ": " << f.message << "\n"; } @@ -944,6 +1454,12 @@ QString ScanEngine::formatText(const ScanResult& result, const ScanConfig& confi s << " โ„น Info: " << result.infos << "\n"; if (result.fixed > 0) s << " ๐Ÿ”ง Fixed: " << result.fixed << "\n"; + if (result.bytesSaved > 0) + s << " ๐Ÿ“‰ Saved: " << QString::number(result.bytesSaved / (1024.0 * 1024.0), 'f', 2) << " MB\n"; + if (result.keysRemoved > 0) { + const QString n = QLocale::system().toString(result.keysRemoved); + s << " ๐Ÿงน Keys removed: " << n << "\n"; + } if (result.skipped > 0) s << " โญ Skipped: " << result.skipped << "\n"; s << " โฑ Time: " << QString::number(result.elapsedMs / 1000.0, 'f', 1) << "s\n"; @@ -1009,6 +1525,10 @@ QJsonObject ScanEngine::scanReportToJsonObject(const ScanResult& result) summary["infos"] = result.infos; summary["fixed"] = result.fixed; summary["skipped"] = result.skipped; + if (result.bytesSaved > 0) + summary["bytesSaved"] = static_cast(result.bytesSaved); + if (result.keysRemoved > 0) + summary["keysRemoved"] = static_cast(result.keysRemoved); summary["elapsedMs"] = result.elapsedMs; root["summary"] = summary; @@ -1027,6 +1547,8 @@ QJsonObject ScanEngine::scanReportToJsonObject(const ScanResult& result) ao["hasSkeleton"] = asset.hasSkeleton; ao["boneCount"] = static_cast(asset.boneCount); ao["textureRefCount"] = static_cast(asset.textureRefCount); + if (asset.animationRedundantKeyframeRatio > 0.0) + ao["animationRedundantKeyframeRatio"] = asset.animationRedundantKeyframeRatio; if (!asset.animationNames.isEmpty()) { QJsonArray anims; @@ -1064,6 +1586,7 @@ QJsonObject ScanEngine::scanReportToJsonObject(const ScanResult& result) fo["message"] = findingMessageForExport(f); if (f.fixable) fo["fixable"] = true; if (f.fixed) fo["fixed"] = true; + if (f.skipped) fo["skipped"] = true; findingsArr.append(fo); } ao["findings"] = findingsArr; @@ -1198,6 +1721,7 @@ QString ScanEngine::formatSarif(const ScanResult& result) QJsonObject props; props["fixable"] = true; if (f.fixed) props["fixed"] = true; + if (f.skipped) props["skipped"] = true; r["properties"] = props; } diff --git a/src/ScanEngine.h b/src/ScanEngine.h index d78c7cd42..319232906 100644 --- a/src/ScanEngine.h +++ b/src/ScanEngine.h @@ -19,6 +19,13 @@ struct Finding { QString message; bool fixable = false; bool fixed = false; + /// True when a fix was attempted but intentionally skipped (e.g. would increase file size). + /// This is distinct from asset-level "skipped" due to loadError. + bool skipped = false; + /// Positive number of bytes saved by applying the fix (0 when not applicable). + qint64 bytesSaved = 0; + /// Positive number of keyframes removed by applying the fix (0 when not applicable). + qint64 keysRemoved = 0; }; struct AssetInfo { @@ -45,6 +52,8 @@ struct AssetInfo { QList animationDurations; // seconds per animation QList animationKeyframeCounts; // max keyframes per animation QStringList boneNames; // unique bone names + /// Fraction of atomic node keys that are redundant under balanced simplify tolerances (0..1), file-level aggregate. + double animationRedundantKeyframeRatio = 0.0; // Redundant-keyframe analysis (filled when scan rule is active). // Total keyframes summed across all tracks of all animations. @@ -66,6 +75,8 @@ struct ScanResult { int infos = 0; int fixed = 0; int skipped = 0; + qint64 bytesSaved = 0; + qint64 keysRemoved = 0; double elapsedMs = 0; /// Wall-clock bounds for reports, always UTC (`yyyy-MM-dd'T'HH:mm:ss.zzzZ`). Set by `ScanEngine::run`. @@ -101,7 +112,8 @@ class ScanEngine { static QList evaluateRules(const AssetInfo& asset, const ScanConfig& config); /// Apply safe auto-fixes for findings that support it. - static void applyFixes(const ScanConfig& config, AssetInfo& asset, + /// \a scanRoot is the directory \c asset.relativePath is relative to (same as \c ScanEngine::run). + static void applyFixes(const ScanConfig& config, const QString& scanRoot, AssetInfo& asset, QList& findings); // --- Formatters --- diff --git a/src/ScanEngine_test.cpp b/src/ScanEngine_test.cpp index 461287fc1..4dd51d3f4 100644 --- a/src/ScanEngine_test.cpp +++ b/src/ScanEngine_test.cpp @@ -185,6 +185,7 @@ TEST(ScanConfigTest, LoadRedundantKeyframesRule) TEST(ScanConfigTest, DefaultConstructorIncludesAssimpGlobPatterns) { const ScanConfig c; + EXPECT_DOUBLE_EQ(c.redundantKeyframesPctThreshold, 40.0); EXPECT_GT(c.includePatterns.size(), 8); bool hasMeshGlob = false; bool hasFbxGlob = false; @@ -1219,7 +1220,7 @@ TEST(ScanEngineTest, ApplyFixes_DisabledDoesNotChangeFindingsOrPath) config.fixEnabled = false; config.fileNameCase = "snake_case"; - ScanEngine::applyFixes(config, asset, findings); + ScanEngine::applyFixes(config, QStringLiteral("/tmp"), asset, findings); EXPECT_EQ(asset.filePath, "/tmp/PlayerModel.fbx"); EXPECT_FALSE(findings[0].fixed); EXPECT_FALSE(findings[0].message.contains("dry-run")); @@ -1250,7 +1251,7 @@ TEST(ScanEngineTest, ApplyFixes_DryRunDoesNotRename) config.dryRun = true; config.fileNameCase = "snake_case"; - ScanEngine::applyFixes(config, asset, findings); + ScanEngine::applyFixes(config, tmpDir.path(), asset, findings); const QString newPath = QDir(tmpDir.path()).filePath("player_model.obj"); EXPECT_TRUE(QFile::exists(oldPath)); @@ -1284,7 +1285,7 @@ TEST(ScanEngineTest, ApplyFixes_RenameSuccessUpdatesAssetPaths) config.fixEnabled = true; config.fileNameCase = "snake_case"; - ScanEngine::applyFixes(config, asset, findings); + ScanEngine::applyFixes(config, tmpDir.path(), asset, findings); const QString newPath = QDir(tmpDir.path()).filePath("nested/player_model.obj"); EXPECT_FALSE(QFile::exists(oldPath)); @@ -1321,7 +1322,7 @@ TEST(ScanEngineTest, ApplyFixes_RenameFailureAddsFailureMessage) config.fixEnabled = true; config.fileNameCase = "snake_case"; - ScanEngine::applyFixes(config, asset, findings); + ScanEngine::applyFixes(config, tmpDir.path(), asset, findings); EXPECT_TRUE(QFile::exists(oldPath)); EXPECT_TRUE(QFile::exists(newPath)); @@ -1501,7 +1502,7 @@ TEST(ScanEngineTest, AssimpReadPolicy_IncompleteFlagWithAnimIsNotLoadFailure) delete scene; } -TEST(ScanEngineTest, EvaluateRules_RedundantKeyframesDisabledByDefault) +TEST(ScanEngineTest, EvaluateRules_RedundantKeyframesOffWhenThresholdZero) { const QString filePath = testDataDir() + "/Twist Dance.fbx"; if (!QFile::exists(filePath)) { @@ -1510,6 +1511,7 @@ TEST(ScanEngineTest, EvaluateRules_RedundantKeyframesDisabledByDefault) const AssetInfo info = ScanEngine::inspectAsset(filePath, QFileInfo(filePath).absolutePath()); ScanConfig config; + config.redundantKeyframesPctThreshold = 0.0; // opt-out QList findings = ScanEngine::evaluateRules(info, config); for (const auto& f : findings) EXPECT_NE(f.rule, "redundant_keyframes_pct"); @@ -1534,6 +1536,7 @@ TEST(ScanEngineTest, EvaluateRules_RedundantKeyframesFiresOnMixamoFixture) if (f.rule == "redundant_keyframes_pct") { found = true; EXPECT_EQ(f.severity, Severity::Warning); + EXPECT_TRUE(f.fixable); EXPECT_TRUE(f.message.contains("redundant keyframes")); EXPECT_TRUE(f.message.contains("Simplify it to save")); EXPECT_TRUE(f.message.contains("Original size")); @@ -1637,3 +1640,45 @@ TEST(ScanEngineTest, MergeGithubActionsMetaIntoReport_PreservesExistingMeta) EXPECT_EQ(meta.value(QStringLiteral("custom")).toInt(), 1); EXPECT_EQ(meta.value(QStringLiteral("repository")).toString(), QStringLiteral("acme/game")); } + +TEST(ScanEngineTest, EnumerateFiles_SimpleExtensionIncludesSkipUnmatchedExtensions) +{ + QTemporaryDir tmpDir; + ASSERT_TRUE(tmpDir.isValid()); + ASSERT_FALSE(writeMinimalObj(tmpDir.path(), QStringLiteral("a.obj")).isEmpty()); + QFile noise(QDir(tmpDir.path()).filePath(QStringLiteral("readme.txt"))); + ASSERT_TRUE(noise.open(QIODevice::WriteOnly)); + noise.write("x"); + noise.close(); + + ScanConfig config = ScanConfig::defaults(); + config.includePatterns = {QStringLiteral("**/*.obj")}; + config.excludePatterns.clear(); + + const QStringList files = ScanEngine::enumerateFiles(config, tmpDir.path()); + ASSERT_EQ(files.size(), 1); + EXPECT_TRUE(files.first().endsWith(QStringLiteral("a.obj"), Qt::CaseInsensitive)); +} + +TEST(ScanEngineTest, ApplyFixes_RedundantKeyframesPctDryRun) +{ + AssetInfo asset; + asset.filePath = QStringLiteral("/no/such/path.fbx"); + asset.relativePath = QStringLiteral("bad.fbx"); + + Finding finding; + finding.file = asset.relativePath; + finding.rule = QStringLiteral("redundant_keyframes_pct"); + finding.severity = Severity::Warning; + finding.message = QStringLiteral("strip keys"); + finding.fixable = true; + QList findings{finding}; + + ScanConfig config = ScanConfig::defaults(); + config.fixEnabled = true; + config.dryRun = true; + + ScanEngine::applyFixes(config, QStringLiteral("/"), asset, findings); + EXPECT_FALSE(findings[0].fixed); + EXPECT_TRUE(findings[0].message.contains(QStringLiteral("dry-run"))); +} From 2166e59901b7072c1c20496a627345027f79f3e5 Mon Sep 17 00:00:00 2001 From: Fernando Date: Tue, 28 Apr 2026 03:01:27 -0400 Subject: [PATCH 2/2] fix(tests): accept compacted FBX animation curves FBX animation curve channels may be emitted with a single key when the channel is constant. Relax the AnimationCurves test accordingly. Also replace a `strlen`-based suffix length with `std::string_view` sizing for Sonar. Made-with: Cursor --- src/Assimp/Importer.cpp | 6 +++--- src/FBX/FBXExporter_test.cpp | 6 ++++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/Assimp/Importer.cpp b/src/Assimp/Importer.cpp index 984b1870b..bd69d55da 100644 --- a/src/Assimp/Importer.cpp +++ b/src/Assimp/Importer.cpp @@ -33,7 +33,7 @@ THE SOFTWARE. #include "BoneProcessor.h" #include "MeshProcessor.h" #include -#include +#include Ogre::MeshPtr AssimpToOgreImporter::loadModel(const std::string& path, bool convertToLeftHanded, unsigned int additionalFlags) { skeleton.reset(); // Clear any skeleton from a previous import @@ -60,8 +60,8 @@ Ogre::MeshPtr AssimpToOgreImporter::loadModel(const std::string& path, bool conv flags |= aiProcess_ConvertToLeftHanded; flags |= additionalFlags; - auto pathEndsWithInsensitive = [](const std::string& p, const char* suf) -> bool { - const size_t n = std::strlen(suf); + auto pathEndsWithInsensitive = [](const std::string& p, std::string_view suf) -> bool { + const size_t n = suf.size(); if (p.size() < n) return false; for (size_t i = 0; i < n; ++i) { diff --git a/src/FBX/FBXExporter_test.cpp b/src/FBX/FBXExporter_test.cpp index fd08870c5..03578852e 100644 --- a/src/FBX/FBXExporter_test.cpp +++ b/src/FBX/FBXExporter_test.cpp @@ -1733,11 +1733,13 @@ TEST_F(FBXExporterCoverageTest, AnimationCurves) { // Should have KeyTime, KeyValueFloat, KeyAttrFlags auto* keyTime = curve->find("KeyTime"); ASSERT_NE(keyTime, nullptr); - EXPECT_EQ(keyTime->properties[0].longArray.size(), 3u); // 3 keyframes + const size_t keyCount = keyTime->properties[0].longArray.size(); + // Curves may be compacted to a single key when the channel is flat. + EXPECT_TRUE(keyCount == 1u || keyCount == 3u); auto* keyValue = curve->find("KeyValueFloat"); ASSERT_NE(keyValue, nullptr); - EXPECT_EQ(keyValue->properties[0].floatArray.size(), 3u); + EXPECT_EQ(keyValue->properties[0].floatArray.size(), keyCount); auto* keyFlags = curve->find("KeyAttrFlags"); ASSERT_NE(keyFlags, nullptr);