From 06e5927ee130640f477921807c3b3074f3694842 Mon Sep 17 00:00:00 2001 From: Thijs Wenker Date: Fri, 8 Mar 2019 23:07:01 +0100 Subject: [PATCH] CR fixes --- interface/src/avatar/AvatarDoctor.cpp | 132 +++++++++++++------------- 1 file changed, 64 insertions(+), 68 deletions(-) diff --git a/interface/src/avatar/AvatarDoctor.cpp b/interface/src/avatar/AvatarDoctor.cpp index 8c22780b52..04a426c3db 100644 --- a/interface/src/avatar/AvatarDoctor.cpp +++ b/interface/src/avatar/AvatarDoctor.cpp @@ -55,7 +55,7 @@ static QStringList HAND_MAPPING_SUFFIXES = { "HandThumb1", }; -const QUrl DEFAULT_URL = QUrl("https://docs.highfidelity.com/create/avatars/create-avatars.html#create-your-own-avatar"); +const QUrl DEFAULT_DOCS_URL = QUrl("https://docs.highfidelity.com/create/avatars/create-avatars.html#create-your-own-avatar"); AvatarDoctor::AvatarDoctor(const QUrl& avatarFSTFileUrl) : _avatarFSTFileUrl(avatarFSTFileUrl) { @@ -85,7 +85,7 @@ void AvatarDoctor::startDiagnosing() { const auto resourceLoaded = [this, resource](bool success) { // MODEL if (!success) { - _errors.push_back({ "Model file cannot be opened.", DEFAULT_URL }); + _errors.push_back({ "Model file cannot be opened.", DEFAULT_DOCS_URL }); emit complete(getErrors()); return; } @@ -93,49 +93,49 @@ void AvatarDoctor::startDiagnosing() { const auto model = resource.data(); const auto avatarModel = resource.data()->getHFMModel(); if (!avatarModel.originalURL.endsWith(".fbx")) { - _errors.push_back({ "Unsupported avatar model format.", DEFAULT_URL }); + _errors.push_back({ "Unsupported avatar model format.", DEFAULT_DOCS_URL }); emit complete(getErrors()); return; } // RIG if (avatarModel.joints.isEmpty()) { - _errors.push_back({ "Avatar has no rig.", DEFAULT_URL }); + _errors.push_back({ "Avatar has no rig.", DEFAULT_DOCS_URL }); } else { auto jointNames = avatarModel.getJointNames(); if (avatarModel.joints.length() > NETWORKED_JOINTS_LIMIT) { - _errors.push_back({tr( "Avatar has over %n bones.", "", NETWORKED_JOINTS_LIMIT), DEFAULT_URL }); + _errors.push_back({tr( "Avatar has over %n bones.", "", NETWORKED_JOINTS_LIMIT), DEFAULT_DOCS_URL }); } // Avatar does not have Hips bone mapped if (!jointNames.contains("Hips")) { - _errors.push_back({ "Hips are not mapped.", DEFAULT_URL }); + _errors.push_back({ "Hips are not mapped.", DEFAULT_DOCS_URL }); } if (!jointNames.contains("Spine")) { - _errors.push_back({ "Spine is not mapped.", DEFAULT_URL }); + _errors.push_back({ "Spine is not mapped.", DEFAULT_DOCS_URL }); } if (!jointNames.contains("Spine1")) { - _errors.push_back({ "Chest (Spine1) is not mapped.", DEFAULT_URL }); + _errors.push_back({ "Chest (Spine1) is not mapped.", DEFAULT_DOCS_URL }); } if (!jointNames.contains("Neck")) { - _errors.push_back({ "Neck is not mapped.", DEFAULT_URL }); + _errors.push_back({ "Neck is not mapped.", DEFAULT_DOCS_URL }); } if (!jointNames.contains("Head")) { - _errors.push_back({ "Head is not mapped.", DEFAULT_URL }); + _errors.push_back({ "Head is not mapped.", DEFAULT_DOCS_URL }); } if (!jointNames.contains("LeftEye")) { if (jointNames.contains("RightEye")) { - _errors.push_back({ "LeftEye is not mapped.", DEFAULT_URL }); + _errors.push_back({ "LeftEye is not mapped.", DEFAULT_DOCS_URL }); } else { - _errors.push_back({ "Eyes are not mapped.", DEFAULT_URL }); + _errors.push_back({ "Eyes are not mapped.", DEFAULT_DOCS_URL }); } } else if (!jointNames.contains("RightEye")) { - _errors.push_back({ "RightEye is not mapped.", DEFAULT_URL }); + _errors.push_back({ "RightEye is not mapped.", DEFAULT_DOCS_URL }); } const auto checkJointAsymmetry = [jointNames] (const QStringList& jointMappingSuffixes) { - foreach (const QString& jointSuffix, jointMappingSuffixes) { + for (const QString& jointSuffix : jointMappingSuffixes) { if (jointNames.contains(LEFT_JOINT_PREFIX + jointSuffix) != jointNames.contains(RIGHT_JOINT_PREFIX + jointSuffix)) { return true; @@ -159,30 +159,30 @@ void AvatarDoctor::startDiagnosing() { }; if (checkJointAsymmetry(ARM_MAPPING_SUFFIXES)) { - _errors.push_back({ "Asymmetrical arm bones.", DEFAULT_URL }); + _errors.push_back({ "Asymmetrical arm bones.", DEFAULT_DOCS_URL }); } if (checkJointAsymmetry(HAND_MAPPING_SUFFIXES)) { - _errors.push_back({ "Asymmetrical hand bones.", DEFAULT_URL }); + _errors.push_back({ "Asymmetrical hand bones.", DEFAULT_DOCS_URL }); } if (checkJointAsymmetry(LEG_MAPPING_SUFFIXES)) { - _errors.push_back({ "Asymmetrical leg bones.", DEFAULT_URL }); + _errors.push_back({ "Asymmetrical leg bones.", DEFAULT_DOCS_URL }); } // Multiple skeleton root joints checkup int skeletonRootJoints = 0; - foreach(const HFMJoint& joint, avatarModel.joints) { + for (const HFMJoint& joint: avatarModel.joints) { if (joint.parentIndex == -1 && joint.isSkeletonJoint) { skeletonRootJoints++; } } if (skeletonRootJoints > 1) { - _errors.push_back({ "Multiple root joints found.", DEFAULT_URL }); + _errors.push_back({ "Multiple top-level joints found.", DEFAULT_DOCS_URL }); } - const auto rig = new Rig(); - rig->reset(avatarModel); - const float eyeHeight = rig->getUnscaledEyeHeight(); + Rig rig; + rig.reset(avatarModel); + const float eyeHeight = rig.getUnscaledEyeHeight(); const float ratio = eyeHeight / DEFAULT_AVATAR_HEIGHT; const float avatarHeight = eyeHeight + ratio * DEFAULT_AVATAR_EYE_TO_TOP_OF_HEAD; @@ -191,71 +191,70 @@ void AvatarDoctor::startDiagnosing() { const float RECOMMENDED_MAX_HEIGHT = DEFAULT_AVATAR_HEIGHT * 1.5f; if (avatarHeight < RECOMMENDED_MIN_HEIGHT) { - _errors.push_back({ "Avatar is possibly too short.", DEFAULT_URL }); + _errors.push_back({ "Avatar is possibly too short.", DEFAULT_DOCS_URL }); } else if (avatarHeight > RECOMMENDED_MAX_HEIGHT) { - _errors.push_back({ "Avatar is possibly too tall.", DEFAULT_URL }); + _errors.push_back({ "Avatar is possibly too tall.", DEFAULT_DOCS_URL }); } // HipsNotOnGround - auto hipsIndex = rig->indexOfJoint("Hips"); + auto hipsIndex = rig.indexOfJoint("Hips"); if (hipsIndex >= 0) { glm::vec3 hipsPosition; - if (rig->getJointPosition(hipsIndex, hipsPosition)) { + if (rig.getJointPosition(hipsIndex, hipsPosition)) { const auto hipJoint = avatarModel.joints.at(avatarModel.getJointIndex("Hips")); if (hipsPosition.y < HIPS_GROUND_MIN_Y) { - _errors.push_back({ "Hips are on ground.", DEFAULT_URL }); + _errors.push_back({ "Hips are on ground.", DEFAULT_DOCS_URL }); } } } // HipsSpineChestNotCoincident - auto spineIndex = rig->indexOfJoint("Spine"); - auto chestIndex = rig->indexOfJoint("Spine1"); + auto spineIndex = rig.indexOfJoint("Spine"); + auto chestIndex = rig.indexOfJoint("Spine1"); if (hipsIndex >= 0 && spineIndex >= 0 && chestIndex >= 0) { glm::vec3 hipsPosition; glm::vec3 spinePosition; glm::vec3 chestPosition; - if (rig->getJointPosition(hipsIndex, hipsPosition) && - rig->getJointPosition(spineIndex, spinePosition) && - rig->getJointPosition(chestIndex, chestPosition)) { + if (rig.getJointPosition(hipsIndex, hipsPosition) && + rig.getJointPosition(spineIndex, spinePosition) && + rig.getJointPosition(chestIndex, chestPosition)) { const auto hipsToSpine = glm::length(hipsPosition - spinePosition); const auto spineToChest = glm::length(spinePosition - chestPosition); if (hipsToSpine < HIPS_SPINE_CHEST_MIN_SEPARATION && spineToChest < HIPS_SPINE_CHEST_MIN_SEPARATION) { - _errors.push_back({ "Hips/Spine/Chest overlap.", DEFAULT_URL }); + _errors.push_back({ "Hips/Spine/Chest overlap.", DEFAULT_DOCS_URL }); } } } - rig->deleteLater(); auto mapping = resource->getMapping(); if (mapping.contains(JOINT_NAME_MAPPING_FIELD) && mapping[JOINT_NAME_MAPPING_FIELD].type() == QVariant::Hash) { const auto& jointNameMappings = mapping[JOINT_NAME_MAPPING_FIELD].toHash(); QStringList jointValues; - foreach(const auto& jointVariant, jointNameMappings.values()) { + for (const auto& jointVariant: jointNameMappings.values()) { jointValues << jointVariant.toString(); } const auto& uniqueJointValues = jointValues.toSet(); - foreach (const auto& jointName, uniqueJointValues) { + for (const auto& jointName: uniqueJointValues) { if (jointValues.count(jointName) > 1) { - _errors.push_back({ tr("%1 is mapped multiple times.").arg(jointName), DEFAULT_URL }); + _errors.push_back({ tr("%1 is mapped multiple times.").arg(jointName), DEFAULT_DOCS_URL }); } } } if (!isDescendantOfJointWhenJointsExist("Spine", "Hips")) { - _errors.push_back({ "Spine is not a child of Hips.", DEFAULT_URL }); + _errors.push_back({ "Spine is not a child of Hips.", DEFAULT_DOCS_URL }); } if (!isDescendantOfJointWhenJointsExist("Spine1", "Spine")) { - _errors.push_back({ "Spine1 is not a child of Spine.", DEFAULT_URL }); + _errors.push_back({ "Spine1 is not a child of Spine.", DEFAULT_DOCS_URL }); } if (!isDescendantOfJointWhenJointsExist("Head", "Spine1")) { - _errors.push_back({ "Head is not a child of Spine1.", DEFAULT_URL }); + _errors.push_back({ "Head is not a child of Spine1.", DEFAULT_DOCS_URL }); } } @@ -270,7 +269,7 @@ void AvatarDoctor::startDiagnosing() { _materialMappingCount = (int)model->getMaterialMapping().size(); _materialMappingLoadedCount = 0; - foreach(const auto& materialMapping, model->getMaterialMapping()) { + for (const auto& materialMapping : model->getMaterialMapping()) { // refresh the texture mappings auto materialMappingResource = materialMapping.second; if (materialMappingResource) { @@ -301,7 +300,7 @@ void AvatarDoctor::startDiagnosing() { connect(resource.data(), &GeometryResource::finished, this, resourceLoaded); } } else { - _errors.push_back({ "Model file cannot be opened", DEFAULT_URL }); + _errors.push_back({ "Model file cannot be opened", DEFAULT_DOCS_URL }); emit complete(getErrors()); } } @@ -309,8 +308,8 @@ void AvatarDoctor::startDiagnosing() { void AvatarDoctor::diagnoseTextures() { const auto model = _model.data(); const auto avatarModel = _model.data()->getHFMModel(); - QStringList externalTextures{}; - QSet textureNames{}; + QVector externalTextures{}; + QVector textureNames{}; int texturesFound = 0; auto addTextureToList = [&externalTextures, &texturesFound](hfm::Texture texture) mutable { if (texture.filename.isEmpty()) { @@ -322,7 +321,7 @@ void AvatarDoctor::diagnoseTextures() { texturesFound++; }; - foreach(const HFMMaterial material, avatarModel.materials) { + for (const auto& material : avatarModel.materials) { addTextureToList(material.normalTexture); addTextureToList(material.albedoTexture); addTextureToList(material.opacityTexture); @@ -336,8 +335,8 @@ void AvatarDoctor::diagnoseTextures() { addTextureToList(material.lightmapTexture); } - foreach(const auto& materialMapping, model->getMaterialMapping()) { - foreach(const auto& networkMaterial, materialMapping.second.data()->parsedMaterials.networkMaterials) { + for (const auto& materialMapping : model->getMaterialMapping()) { + for (const auto& networkMaterial : materialMapping.second.data()->parsedMaterials.networkMaterials) { texturesFound += (int)networkMaterial.second->getTextureMaps().size(); } } @@ -346,39 +345,36 @@ void AvatarDoctor::diagnoseTextures() { QUrl(avatarModel.originalURL)).resolved(QUrl("textures")); if (texturesFound == 0) { - _errors.push_back({ tr("No textures assigned."), DEFAULT_URL }); + _errors.push_back({ tr("No textures assigned."), DEFAULT_DOCS_URL }); } if (!externalTextures.empty()) { // Check External Textures: auto modelTexturesURLs = model->getTextures(); _externalTextureCount = externalTextures.length(); - foreach(const QString textureKey, externalTextures) { + + auto checkTextureLoadingComplete = [this]() mutable { + if (_checkedTextureCount == _externalTextureCount) { + if (_missingTextureCount > 0) { + _errors.push_back({ tr("Missing %n texture(s).","", _missingTextureCount), DEFAULT_DOCS_URL }); + } + if (_unsupportedTextureCount > 0) { + _errors.push_back({ tr("%n unsupported texture(s) found.", "", _unsupportedTextureCount), + DEFAULT_DOCS_URL }); + } + + emit complete(getErrors()); + } + }; + + for (const QString& textureKey : externalTextures) { if (!modelTexturesURLs.contains(textureKey)) { _missingTextureCount++; _checkedTextureCount++; continue; } - const QUrl textureURL = modelTexturesURLs[textureKey].toUrl(); - auto textureResource = DependencyManager::get()->getTexture(textureURL); - auto checkTextureLoadingComplete = [this]() mutable { - qDebug() << "checkTextureLoadingComplete" << _checkedTextureCount << "/" << _externalTextureCount; - - if (_checkedTextureCount == _externalTextureCount) { - if (_missingTextureCount > 0) { - _errors.push_back({ tr("Missing %n texture(s).","", _missingTextureCount), DEFAULT_URL }); - } - if (_unsupportedTextureCount > 0) { - _errors.push_back({ tr("%n unsupported texture(s) found.", "", _unsupportedTextureCount), - DEFAULT_URL }); - } - - emit complete(getErrors()); - } - }; - auto textureLoaded = [this, textureResource, checkTextureLoadingComplete](bool success) mutable { if (!success) { auto normalizedURL = DependencyManager::get()->normalizeURL(textureResource->getURL()); @@ -407,9 +403,9 @@ void AvatarDoctor::diagnoseTextures() { } else { _missingTextureCount++; _checkedTextureCount++; - checkTextureLoadingComplete(); } } + checkTextureLoadingComplete(); } else { emit complete(getErrors()); }