From 8e51664b694ef39dda535fd0298df384fa66c705 Mon Sep 17 00:00:00 2001 From: ksuprynowicz Date: Wed, 28 Feb 2024 22:44:16 +0100 Subject: [PATCH 1/2] Fixed blendshapes from gltf --- .../model-serializers/src/GLTFSerializer.cpp | 67 +++++++++++-------- 1 file changed, 40 insertions(+), 27 deletions(-) diff --git a/libraries/model-serializers/src/GLTFSerializer.cpp b/libraries/model-serializers/src/GLTFSerializer.cpp index 68a2fcecd91..04c4679fceb 100644 --- a/libraries/model-serializers/src/GLTFSerializer.cpp +++ b/libraries/model-serializers/src/GLTFSerializer.cpp @@ -125,7 +125,7 @@ bool GLTFSerializer::generateTargetData(cgltf_accessor *accessor, float weight, } storedValues.resize(accessor->count * 3); size_t numFloats = cgltf_accessor_unpack_floats(accessor, storedValues.data(), accessor->count * 3); - if (numFloats == accessor->count * 3) { + if (numFloats != accessor->count * 3) { return false; } @@ -158,7 +158,7 @@ template bool findPointerInArray(const T *pointer, const T *array, s bool findAttribute(const QString &name, const cgltf_attribute *attributes, size_t numAttributes, size_t &index) { std::string nameString = name.toStdString(); for (size_t i = 0; i < numAttributes; i++) { - if (strcmp(nameString.c_str(), attributes->name) != 0) { + if (strcmp(nameString.c_str(), attributes->name) == 0) { index = i; return true; } @@ -212,27 +212,29 @@ bool GLTFSerializer::buildGeometry(HFMModel& hfmModel, const hifi::VariantHash& // therefore we need to re-initialize the order in which nodes will be parsed QVector hasBeenSorted; hasBeenSorted.fill(false, numNodes); - size_t i = 0; // initial index - while (i < numNodes) { - int currentNode = sortedNodes[i]; - int parentIndex = parents[currentNode]; - if (parentIndex == -1 || hasBeenSorted[parentIndex]) { - hasBeenSorted[currentNode] = true; - ++i; - } else { - size_t j = i + 1; // index of node to be sorted - while (j < numNodes) { - int nextNode = sortedNodes[j]; - parentIndex = parents[nextNode]; - if (parentIndex == -1 || hasBeenSorted[parentIndex]) { - // swap with currentNode - hasBeenSorted[nextNode] = true; - sortedNodes[i] = nextNode; - sortedNodes[j] = currentNode; - ++i; - currentNode = sortedNodes[i]; + { + size_t i = 0; // initial index + while (i < numNodes) { + int currentNode = sortedNodes[i]; + int parentIndex = parents[currentNode]; + if (parentIndex == -1 || hasBeenSorted[parentIndex]) { + hasBeenSorted[currentNode] = true; + ++i; + } else { + size_t j = i + 1; // index of node to be sorted + while (j < numNodes) { + int nextNode = sortedNodes[j]; + parentIndex = parents[nextNode]; + if (parentIndex == -1 || hasBeenSorted[parentIndex]) { + // swap with currentNode + hasBeenSorted[nextNode] = true; + sortedNodes[i] = nextNode; + sortedNodes[j] = currentNode; + ++i; + currentNode = sortedNodes[i]; + } + ++j; } - ++j; } } } @@ -971,7 +973,7 @@ bool GLTFSerializer::buildGeometry(HFMModel& hfmModel, const hifi::VariantHash& } // Build morph targets (blend shapes) - if (!primitive.targets_count) { + if (primitive.targets_count) { // Build list of blendshapes from FST and model. typedef QPair WeightedIndex; @@ -993,8 +995,8 @@ bool GLTFSerializer::buildGeometry(HFMModel& hfmModel, const hifi::VariantHash& } else { // Use blendshape from model. std::string blendshapeNameString = blendshapeName.toStdString(); - for (size_t i = 0; i < node.mesh->target_names_count; i++) { - if (strcmp(node.mesh->target_names[i], blendshapeNameString.c_str()) == 0) { + for (size_t j = 0; j < node.mesh->target_names_count; j++) { + if (strcmp(node.mesh->target_names[j], blendshapeNameString.c_str()) == 0) { blendshapeIndices.insert(blendshapeName, WeightedIndex(i, 1.0f)); break; } @@ -1089,12 +1091,23 @@ bool GLTFSerializer::buildGeometry(HFMModel& hfmModel, const hifi::VariantHash& blendshape.vertices.resize(prevMeshVerticesCount + vertices.size()); blendshape.normals.resize(prevMeshVerticesCount + vertices.size()); } + //TODO: it looks like this can support sparse encoding, since there are indices? for (int i = 0; i < vertices.size(); i++) { blendshape.indices[prevMeshVerticesCount + i] = prevMeshVerticesCount + i; blendshape.vertices[prevMeshVerticesCount + i] += vertices.value(i); - blendshape.normals[prevMeshVerticesCount + i] += normals.value(i); + // Prevent out-of-bounds access if blendshape normals are not available + if (i < normals.size()) { + blendshape.normals[prevMeshVerticesCount + i] += normals.value(i); + } else { + if (prevMeshVerticesCount + i < mesh.normals.size()) { + blendshape.normals[prevMeshVerticesCount + i] = mesh.normals[prevMeshVerticesCount + i]; + } else { + qWarning(modelformat) << "Blendshape has more vertices than original mesh " << _url; + hfmModel.loadErrorCount++; + return false; + } + } } - } } From 77800955902c6616629e714f99f62be658e6cd65 Mon Sep 17 00:00:00 2001 From: ksuprynowicz Date: Sat, 2 Mar 2024 23:50:32 +0100 Subject: [PATCH 2/2] Added check for nullptr in GLTFSerializer --- libraries/model-serializers/src/GLTFSerializer.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/libraries/model-serializers/src/GLTFSerializer.cpp b/libraries/model-serializers/src/GLTFSerializer.cpp index 04c4679fceb..1b205e62d0c 100644 --- a/libraries/model-serializers/src/GLTFSerializer.cpp +++ b/libraries/model-serializers/src/GLTFSerializer.cpp @@ -158,9 +158,13 @@ template bool findPointerInArray(const T *pointer, const T *array, s bool findAttribute(const QString &name, const cgltf_attribute *attributes, size_t numAttributes, size_t &index) { std::string nameString = name.toStdString(); for (size_t i = 0; i < numAttributes; i++) { - if (strcmp(nameString.c_str(), attributes->name) == 0) { - index = i; - return true; + if (attributes->name == nullptr) { + qDebug(modelformat) << "GLTFSerializer: attribute with a null pointer name string"; + } else { + if (strcmp(nameString.c_str(), attributes->name) == 0) { + index = i; + return true; + } } } return false; @@ -1072,7 +1076,7 @@ bool GLTFSerializer::buildGeometry(HFMModel& hfmModel, const hifi::VariantHash& size_t normalAttributeIndex = 0; if (findAttribute("NORMAL", target.attributes, target.attributes_count, normalAttributeIndex)) { if (!generateTargetData(target.attributes[normalAttributeIndex].data, weight, normals)) { - qWarning(modelformat) << "Invalid accessor type on generateTargetData vertices for model " << _url; + qWarning(modelformat) << "Invalid NORMAL accessor on generateTargetData vertices for model " << _url; hfmModel.loadErrorCount++; return false; } @@ -1080,7 +1084,7 @@ bool GLTFSerializer::buildGeometry(HFMModel& hfmModel, const hifi::VariantHash& size_t positionAttributeIndex = 0; if (findAttribute("POSITION", target.attributes, target.attributes_count, positionAttributeIndex)) { if (!generateTargetData(target.attributes[positionAttributeIndex].data, weight, vertices)) { - qWarning(modelformat) << "Invalid accessor type on generateTargetData vertices for model " << _url; + qWarning(modelformat) << "Invalid POSITION accessor on generateTargetData vertices for model " << _url; hfmModel.loadErrorCount++; return false; }