Merge pull request #8471 from AndrewMeadows/dont-assert-on-bad-data

workaround bad FBXMesh data rather than assert

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]];
@@ -735,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();
@@ -768,24 +778,30 @@ void RenderableModelEntityItem::computeShapeInfo(ShapeInfo& info) {
                 triangleIndices.reserve((int32_t)((gpu::Size)(triangleIndices.size()) + indices.getNumElements()));
                 gpu::BufferView::Iterator<const model::Mesh::Part> partItr = parts.cbegin<const model::Mesh::Part>();
                 while (partItr != parts.cend<const model::Mesh::Part>()) {
+                    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<const gpu::BufferView::Index>() + partItr->_startIndex;
-                        auto indexEnd = indexItr + partItr->_numIndices;
+                        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);
+                        // TODO: resurrect assert after we start sanitizing FBXMesh higher up
-                        uint32_t approxNumIndices = 3 * partItr->_numIndices;
+                        //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<const gpu::BufferView::Index>() + 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);
@@ -819,18 +835,24 @@ void RenderableModelEntityItem::computeShapeInfo(ShapeInfo& info) {
                 while (partItr != parts.cend<const model::Mesh::Part>()) {
                     // collect unique list of indices for this part
                     std::set<int32_t> 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<const gpu::BufferView::Index>() + partItr->_startIndex;
-                        auto indexEnd = indexItr + partItr->_numIndices;
+                        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<const gpu::BufferView::Index>() + partItr->_startIndex;
-                        auto indexEnd = indexItr + (partItr->_numIndices - 2);
+                        auto indexEnd = indexItr + (numIndices - 2);
 
                         // first triangle uses the first three indices
                         uniqueIndices.insert(*(indexItr++));
@@ -842,11 +864,11 @@ void RenderableModelEntityItem::computeShapeInfo(ShapeInfo& info) {
                         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));
                                 }

libraries/gpu/src/gpu/Buffer.h

@@ -224,6 +224,8 @@ public:
 
         bool operator==(const Iterator<T>& iterator) const { return (_ptr == iterator.getConstPtr()); }
         bool operator!=(const Iterator<T>& iterator) const { return (_ptr != iterator.getConstPtr()); }
+        bool operator<(const Iterator<T>& iterator) const { return (_ptr < iterator.getConstPtr()); }
+        bool operator>(const Iterator<T>& iterator) const { return (_ptr > iterator.getConstPtr()); }
 
         void movePtr(const Index& movement) {
             auto byteptr = ((Byte*)_ptr);

libraries/physics/src/ShapeFactory.cpp

@@ -164,17 +164,28 @@ 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
+    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) {
@@ -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;
     }