From cb9a5466d551e6cac5a2cec9f496f0a4ed76a3a9 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 18 Aug 2016 11:09:48 -0700 Subject: [PATCH 1/6] workaround bad FBXMesh data rather than assert --- .../src/RenderableModelEntityItem.cpp | 57 ++++++++++++------- 1 file changed, 38 insertions(+), 19 deletions(-) diff --git a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp index 2e96cdb4ac..b713a882cb 100644 --- a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp +++ b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp @@ -595,6 +595,9 @@ bool RenderableModelEntityItem::isReadyToComputeShape() { } void RenderableModelEntityItem::computeShapeInfo(ShapeInfo& info) { + const uint32_t TRIANGLE_STRIDE = 3; + const uint32_t QUAD_STRIDE = 4; + ShapeType type = getShapeType(); glm::vec3 dimensions = getDimensions(); if (type == SHAPE_TYPE_COMPOUND) { @@ -611,8 +614,6 @@ void RenderableModelEntityItem::computeShapeInfo(ShapeInfo& info) { // the way OBJ files get read, each section under a "g" line is its own meshPart. We only expect // to find one actual "mesh" (with one or more meshParts in it), but we loop over the meshes, just in case. - const uint32_t TRIANGLE_STRIDE = 3; - const uint32_t QUAD_STRIDE = 4; foreach (const FBXMesh& mesh, collisionGeometry.meshes) { // each meshPart is a convex hull foreach (const FBXMeshPart &meshPart, mesh.parts) { @@ -621,7 +622,10 @@ void RenderableModelEntityItem::computeShapeInfo(ShapeInfo& info) { // run through all the triangles and (uniquely) add each point to the hull uint32_t numIndices = (uint32_t)meshPart.triangleIndices.size(); - assert(numIndices % TRIANGLE_STRIDE == 0); + // TODO: assert rather than workaround after we start sanitizing FBXMesh higher up + //assert(numIndices % TRIANGLE_STRIDE == 0); + numIndices -= numIndices % TRIANGLE_STRIDE; // WORKAROUND lack of sanity checking in FBXReader + for (uint32_t j = 0; j < numIndices; j += TRIANGLE_STRIDE) { glm::vec3 p0 = mesh.vertices[meshPart.triangleIndices[j]]; glm::vec3 p1 = mesh.vertices[meshPart.triangleIndices[j + 1]]; @@ -639,7 +643,10 @@ void RenderableModelEntityItem::computeShapeInfo(ShapeInfo& info) { // run through all the quads and (uniquely) add each point to the hull numIndices = (uint32_t)meshPart.quadIndices.size(); - assert(numIndices % QUAD_STRIDE == 0); + // TODO: assert rather than workaround after we start sanitizing FBXMesh higher up + //assert(numIndices % QUAD_STRIDE == 0); + numIndices -= numIndices % QUAD_STRIDE; // WORKAROUND lack of sanity checking in FBXReader + for (uint32_t j = 0; j < numIndices; j += QUAD_STRIDE) { glm::vec3 p0 = mesh.vertices[meshPart.quadIndices[j]]; glm::vec3 p1 = mesh.vertices[meshPart.quadIndices[j + 1]]; @@ -768,24 +775,30 @@ void RenderableModelEntityItem::computeShapeInfo(ShapeInfo& info) { triangleIndices.reserve((int32_t)((gpu::Size)(triangleIndices.size()) + indices.getNumElements())); gpu::BufferView::Iterator partItr = parts.cbegin(); while (partItr != parts.cend()) { + auto numIndices = partItr->_numIndices; if (partItr->_topology == model::Mesh::TRIANGLES) { - assert(partItr->_numIndices % 3 == 0); + // TODO: assert rather than workaround after we start sanitizing FBXMesh higher up + //assert(numIndices % TRIANGLE_STRIDE == 0); + numIndices -= numIndices % TRIANGLE_STRIDE; // WORKAROUND lack of sanity checking in FBXReader + auto indexItr = indices.cbegin() + partItr->_startIndex; - auto indexEnd = indexItr + partItr->_numIndices; - while (indexItr != indexEnd) { + auto indexEnd = indexItr + numIndices; + while (indexItr < indexEnd) { triangleIndices.push_back(*indexItr + meshIndexOffset); ++indexItr; } } else if (partItr->_topology == model::Mesh::TRIANGLE_STRIP) { - assert(partItr->_numIndices > 2); - uint32_t approxNumIndices = 3 * partItr->_numIndices; + // TODO: resurrect assert after we start sanitizing FBXMesh higher up + //assert(numIndices > 2); + + uint32_t approxNumIndices = TRIANGLE_STRIDE * numIndices; if (approxNumIndices > (uint32_t)(triangleIndices.capacity() - triangleIndices.size())) { // we underestimated the final size of triangleIndices so we pre-emptively expand it triangleIndices.reserve(triangleIndices.size() + approxNumIndices); } auto indexItr = indices.cbegin() + partItr->_startIndex; - auto indexEnd = indexItr + (partItr->_numIndices - 2); + auto indexEnd = indexItr + (numIndices - 2); // first triangle uses the first three indices triangleIndices.push_back(*(indexItr++) + meshIndexOffset); @@ -794,7 +807,7 @@ void RenderableModelEntityItem::computeShapeInfo(ShapeInfo& info) { // the rest use previous and next index uint32_t triangleCount = 1; - while (indexItr != indexEnd) { + while (indexItr < indexEnd) { if ((*indexItr) != model::Mesh::PRIMITIVE_RESTART_INDEX) { if (triangleCount % 2 == 0) { // even triangles use first two indices in order @@ -819,18 +832,24 @@ void RenderableModelEntityItem::computeShapeInfo(ShapeInfo& info) { while (partItr != parts.cend()) { // collect unique list of indices for this part std::set uniqueIndices; + auto numIndices = partItr->_numIndices; if (partItr->_topology == model::Mesh::TRIANGLES) { - assert(partItr->_numIndices % 3 == 0); + // TODO: assert rather than workaround after we start sanitizing FBXMesh higher up + //assert(numIndices% TRIANGLE_STRIDE == 0); + numIndices -= numIndices % TRIANGLE_STRIDE; // WORKAROUND lack of sanity checking in FBXReader + auto indexItr = indices.cbegin() + partItr->_startIndex; - auto indexEnd = indexItr + partItr->_numIndices; - while (indexItr != indexEnd) { + auto indexEnd = indexItr + numIndices; + while (indexItr < indexEnd) { uniqueIndices.insert(*indexItr); ++indexItr; } } else if (partItr->_topology == model::Mesh::TRIANGLE_STRIP) { - assert(partItr->_numIndices > 2); + // TODO: resurrect assert after we start sanitizing FBXMesh higher up + //assert(numIndices > TRIANGLE_STRIDE - 1); + auto indexItr = indices.cbegin() + partItr->_startIndex; - auto indexEnd = indexItr + (partItr->_numIndices - 2); + auto indexEnd = indexItr + (numIndices - 2); // first triangle uses the first three indices uniqueIndices.insert(*(indexItr++)); @@ -839,14 +858,14 @@ void RenderableModelEntityItem::computeShapeInfo(ShapeInfo& info) { // the rest use previous and next index uint32_t triangleCount = 1; - while (indexItr != indexEnd) { + while (indexItr < indexEnd) { if ((*indexItr) != model::Mesh::PRIMITIVE_RESTART_INDEX) { if (triangleCount % 2 == 0) { - // even triangles use first two indices in order + // EVEN triangles use first two indices in order uniqueIndices.insert(*(indexItr - 2)); uniqueIndices.insert(*(indexItr - 1)); } else { - // odd triangles swap order of first two indices + // ODD triangles swap order of first two indices uniqueIndices.insert(*(indexItr - 1)); uniqueIndices.insert(*(indexItr - 2)); } From 2ba35e6398c7aafbee12bfdd1f728f6b606d9dee Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 18 Aug 2016 11:48:56 -0700 Subject: [PATCH 2/6] skip non-existant meshes --- libraries/entities-renderer/src/RenderableModelEntityItem.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp index b713a882cb..e135107323 100644 --- a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp +++ b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp @@ -742,6 +742,9 @@ void RenderableModelEntityItem::computeShapeInfo(ShapeInfo& info) { int32_t meshCount = 0; int32_t pointListIndex = 0; for (auto& mesh : meshes) { + if (!mesh) { + continue; + } const gpu::BufferView& vertices = mesh->getVertexBuffer(); const gpu::BufferView& indices = mesh->getIndexBuffer(); const gpu::BufferView& parts = mesh->getPartBuffer(); From 6446c7c302007ae9d95740097022526157b4380f Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 18 Aug 2016 11:49:15 -0700 Subject: [PATCH 3/6] make createShapeFromInfo() fail gently on bad data --- libraries/physics/src/ShapeFactory.cpp | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/libraries/physics/src/ShapeFactory.cpp b/libraries/physics/src/ShapeFactory.cpp index 137038e133..5182038c27 100644 --- a/libraries/physics/src/ShapeFactory.cpp +++ b/libraries/physics/src/ShapeFactory.cpp @@ -164,13 +164,24 @@ btTriangleIndexVertexArray* createStaticMeshArray(const ShapeInfo& info) { assert(info.getType() == SHAPE_TYPE_STATIC_MESH); // should only get here for mesh shapes const ShapeInfo::PointCollection& pointCollection = info.getPointCollection(); - assert(pointCollection.size() == 1); // should only have one mesh + if (pointCollection.size() < 1) { + // no lists of points to work with + return nullptr; + } + + // we only use the first point collection const ShapeInfo::PointList& pointList = pointCollection[0]; - assert(pointList.size() > 2); // should have at least one triangle's worth of points + if (pointList.size() < 3) { + // not enough distinct points to make a non-degenerate triangle + return nullptr; + } const ShapeInfo::TriangleIndices& triangleIndices = info.getTriangleIndices(); - assert(triangleIndices.size() > 2); // should have at least one triangle's worth of indices + if (triangleIndices.size() < 3) { + // not enough indices to make a single triangle + return nullptr; + } // allocate mesh buffers btIndexedMesh mesh; @@ -328,7 +339,9 @@ btCollisionShape* ShapeFactory::createShapeFromInfo(const ShapeInfo& info) { break; case SHAPE_TYPE_STATIC_MESH: { btTriangleIndexVertexArray* dataArray = createStaticMeshArray(info); - shape = new StaticMeshShape(dataArray); + if (dataArray) { + shape = new StaticMeshShape(dataArray); + } } break; } From 304c313db71939a51e36dde69619e90d8f3ad37c Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Fri, 19 Aug 2016 10:15:30 -0700 Subject: [PATCH 4/6] minor cleanup --- libraries/physics/src/ShapeFactory.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/physics/src/ShapeFactory.cpp b/libraries/physics/src/ShapeFactory.cpp index 5182038c27..67072d46d4 100644 --- a/libraries/physics/src/ShapeFactory.cpp +++ b/libraries/physics/src/ShapeFactory.cpp @@ -178,14 +178,14 @@ btTriangleIndexVertexArray* createStaticMeshArray(const ShapeInfo& info) { } const ShapeInfo::TriangleIndices& triangleIndices = info.getTriangleIndices(); - if (triangleIndices.size() < 3) { + int32_t numIndices = triangleIndices.size(); + if (numIndices < 3) { // not enough indices to make a single triangle return nullptr; } // allocate mesh buffers btIndexedMesh mesh; - int32_t numIndices = triangleIndices.size(); const int32_t VERTICES_PER_TRIANGLE = 3; mesh.m_numTriangles = numIndices / VERTICES_PER_TRIANGLE; if (numIndices < INT16_MAX) { From c3f165d9dafc82d4449ea19b9b54139f40a863dc Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Fri, 19 Aug 2016 10:15:37 -0700 Subject: [PATCH 5/6] BufferView::Iterator doesn't support operator< --- .../entities-renderer/src/RenderableModelEntityItem.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp index e135107323..ac447417aa 100644 --- a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp +++ b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp @@ -786,7 +786,7 @@ void RenderableModelEntityItem::computeShapeInfo(ShapeInfo& info) { auto indexItr = indices.cbegin() + partItr->_startIndex; auto indexEnd = indexItr + numIndices; - while (indexItr < indexEnd) { + while (indexItr != indexEnd) { triangleIndices.push_back(*indexItr + meshIndexOffset); ++indexItr; } @@ -810,7 +810,7 @@ void RenderableModelEntityItem::computeShapeInfo(ShapeInfo& info) { // the rest use previous and next index uint32_t triangleCount = 1; - while (indexItr < indexEnd) { + while (indexItr != indexEnd) { if ((*indexItr) != model::Mesh::PRIMITIVE_RESTART_INDEX) { if (triangleCount % 2 == 0) { // even triangles use first two indices in order @@ -843,7 +843,7 @@ void RenderableModelEntityItem::computeShapeInfo(ShapeInfo& info) { auto indexItr = indices.cbegin() + partItr->_startIndex; auto indexEnd = indexItr + numIndices; - while (indexItr < indexEnd) { + while (indexItr != indexEnd) { uniqueIndices.insert(*indexItr); ++indexItr; } @@ -861,7 +861,7 @@ void RenderableModelEntityItem::computeShapeInfo(ShapeInfo& info) { // the rest use previous and next index uint32_t triangleCount = 1; - while (indexItr < indexEnd) { + while (indexItr != indexEnd) { if ((*indexItr) != model::Mesh::PRIMITIVE_RESTART_INDEX) { if (triangleCount % 2 == 0) { // EVEN triangles use first two indices in order From 5b5ed220bc2b81432e3da965a753661701c1f994 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Fri, 19 Aug 2016 10:26:47 -0700 Subject: [PATCH 6/6] add BufferView::Iterator operator<() and friend --- libraries/gpu/src/gpu/Buffer.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libraries/gpu/src/gpu/Buffer.h b/libraries/gpu/src/gpu/Buffer.h index da1a987bee..d6b8f943dd 100644 --- a/libraries/gpu/src/gpu/Buffer.h +++ b/libraries/gpu/src/gpu/Buffer.h @@ -224,6 +224,8 @@ public: bool operator==(const Iterator& iterator) const { return (_ptr == iterator.getConstPtr()); } bool operator!=(const Iterator& iterator) const { return (_ptr != iterator.getConstPtr()); } + bool operator<(const Iterator& iterator) const { return (_ptr < iterator.getConstPtr()); } + bool operator>(const Iterator& iterator) const { return (_ptr > iterator.getConstPtr()); } void movePtr(const Index& movement) { auto byteptr = ((Byte*)_ptr);