Subsurf: Fix/workaround crashes and failures with non-manifold geometry
authorSergey Sharybin <sergey.vfx@gmail.com>
Wed, 1 Aug 2018 12:48:32 +0000 (14:48 +0200)
committerSergey Sharybin <sergey.vfx@gmail.com>
Wed, 1 Aug 2018 16:42:59 +0000 (18:42 +0200)
The idea is simple: do not provide full topology to OpenSubdiv, leave
edges creation to OpenSubdiv itself. This solves issues with non-manifold
meshes which were known to fail, including the ones from T52059.

On a positive side we can simplify our side of converter, keeping code
shorter.

it is still possible that we'll need to ensure all loops has same
winding, but that is less things to worry about.

intern/opensubdiv/internal/opensubdiv_converter_factory.cc
intern/opensubdiv/opensubdiv_converter_capi.h
source/blender/blenkernel/intern/subdiv_converter_mesh.c

index 02b6e93a1aef85537a3cbd32768db11226e48101..321f580af97881b35a27373e42e5eca4415ee517 100644 (file)
@@ -48,7 +48,7 @@ TopologyRefinerFactory<TopologyRefinerData>::resizeComponentTopology(
     TopologyRefiner& refiner,
     const TopologyRefinerData& cb_data) {
   const OpenSubdiv_Converter* converter = cb_data.converter;
-  /// Faces and face-vertices.
+  // Faces and face-vertices.
   const int num_faces = converter->getNumFaces(converter);
   setNumBaseFaces(refiner, num_faces);
   for (int face_index = 0; face_index < num_faces; ++face_index) {
@@ -56,6 +56,13 @@ TopologyRefinerFactory<TopologyRefinerData>::resizeComponentTopology(
         converter->getNumFaceVertices(converter, face_index);
     setNumBaseFaceVertices(refiner, face_index, num_face_vertices);
   }
+  // Vertices.
+  const int num_vertices = converter->getNumVertices(converter);
+  setNumBaseVertices(refiner, num_vertices);
+  // If converter does not provide full topology, we are done.
+  if (!converter->specifiesFullTopology(converter)) {
+    return true;
+  }
   // Edges and edge-faces.
   const int num_edges = converter->getNumEdges(converter);
   setNumBaseEdges(refiner, num_edges);
@@ -64,9 +71,7 @@ TopologyRefinerFactory<TopologyRefinerData>::resizeComponentTopology(
         converter->getNumEdgeFaces(converter, edge_index);
     setNumBaseEdgeFaces(refiner, edge_index, num_edge_faces);
   }
-  // Vertices and vertex-faces and vertex-edges.
-  const int num_vertices = converter->getNumVertices(converter);
-  setNumBaseVertices(refiner, num_vertices);
+  // Vertex-faces and vertex-edges.
   for (int vertex_index = 0; vertex_index < num_vertices; ++vertex_index) {
     const int num_vert_edges =
         converter->getNumVertexEdges(converter, vertex_index);
@@ -85,13 +90,21 @@ TopologyRefinerFactory<TopologyRefinerData>::assignComponentTopology(
     const TopologyRefinerData& cb_data) {
   using Far::IndexArray;
   const OpenSubdiv_Converter* converter = cb_data.converter;
+  const bool full_topology_specified =
+          converter->specifiesFullTopology(converter);
   // Face relations.
   const int num_faces = converter->getNumFaces(converter);
   for (int face_index = 0; face_index < num_faces; ++face_index) {
     IndexArray dst_face_verts = getBaseFaceVertices(refiner, face_index);
     converter->getFaceVertices(converter, face_index, &dst_face_verts[0]);
-    IndexArray dst_face_edges = getBaseFaceEdges(refiner, face_index);
-    converter->getFaceEdges(converter, face_index, &dst_face_edges[0]);
+    if (full_topology_specified) {
+      IndexArray dst_face_edges = getBaseFaceEdges(refiner, face_index);
+      converter->getFaceEdges(converter, face_index, &dst_face_edges[0]);
+    }
+  }
+  // If converter does not provide full topology, we are done.
+  if (!full_topology_specified) {
+    return true;
   }
   // Edge relations.
   const int num_edges = converter->getNumEdges(converter);
@@ -103,7 +116,7 @@ TopologyRefinerFactory<TopologyRefinerData>::assignComponentTopology(
     IndexArray dst_edge_faces = getBaseEdgeFaces(refiner, edge_index);
     converter->getEdgeFaces(converter, edge_index, &dst_edge_faces[0]);
   }
-// TODO(sergey): Find a way to move this to an utility function.
+  // TODO(sergey): Find a way to move this to an utility function.
 #ifdef OPENSUBDIV_ORIENT_TOPOLOGY
   // Make face normals consistent.
   std::vector<bool> face_used(num_faces, false);
@@ -333,11 +346,28 @@ inline bool TopologyRefinerFactory<TopologyRefinerData>::assignComponentTags(
     const TopologyRefinerData& cb_data) {
   using OpenSubdiv::Sdc::Crease;
   const OpenSubdiv_Converter* converter = cb_data.converter;
+  const bool full_topology_specified =
+          converter->specifiesFullTopology(converter);
   const int num_edges = converter->getNumEdges(converter);
   for (int edge_index = 0; edge_index < num_edges; ++edge_index) {
     const float sharpness =
         converter->getEdgeSharpness(converter, edge_index);
-    setBaseEdgeSharpness(refiner, edge_index, sharpness);
+    if (sharpness < 1e-6f) {
+      continue;
+    }
+    if (full_topology_specified) {
+      setBaseEdgeSharpness(refiner, edge_index, sharpness);
+    } else {
+      int edge_vertices[2];
+      converter->getEdgeVertices(converter, edge_index, edge_vertices);
+      const int base_edge_index = findBaseEdge(
+          refiner, edge_vertices[0], edge_vertices[1]);
+      if (base_edge_index == OpenSubdiv::Far::INDEX_INVALID) {
+        printf("OpenSubdiv Error: failed to find reconstructed edge\n");
+        return false;
+      }
+      setBaseEdgeSharpness(refiner, base_edge_index, sharpness);
+    }
   }
   // OpenSubdiv expects non-manifold vertices to be sharp but at the time it
   // handles correct cases when vertex is a corner of plane. Currently mark
@@ -351,8 +381,8 @@ inline bool TopologyRefinerFactory<TopologyRefinerData>::assignComponentTags(
           refiner, vertex_index, Crease::SHARPNESS_INFINITE);
     } else if (vertex_edges.size() == 2) {
       const int edge0 = vertex_edges[0], edge1 = vertex_edges[1];
-      const float sharpness0 = converter->getEdgeSharpness(converter, edge0);
-      const float sharpness1 = converter->getEdgeSharpness(converter, edge1);
+      const float sharpness0 = refiner._levels[0]->getEdgeSharpness(edge0);
+      const float sharpness1 = refiner._levels[0]->getEdgeSharpness(edge1);
       const float sharpness = std::min(sharpness0, sharpness1);
       setBaseVertexSharpness(refiner, vertex_index, sharpness);
     }
index 58a231deb3001728b43e046ea2e2776d12005bb7..1dd68f43c322427aa5ab709b7674c091bd902a30 100644 (file)
@@ -34,6 +34,17 @@ typedef struct OpenSubdiv_Converter {
   OpenSubdiv_FVarLinearInterpolation (*getFVarLinearInterpolation)(
       const struct OpenSubdiv_Converter* converter);
 
+  // Denotes whether this converter specifies full topology, which includes
+  // vertices, edges, faces, vertices+edges of a face and edges/faces of a
+  // vertex.
+  // Otherwise this converter will only provide number of vertices and faces,
+  // and vertices of faces. The rest of topology will be created by OpenSubdiv.
+  //
+  // NOTE: Even if converter does not provide full topology, it still needs
+  // to provide number of edges and vertices-of-edge. Those are used to assign
+  // topology tags.
+  bool (*specifiesFullTopology)(const struct OpenSubdiv_Converter* converter);
+
   //////////////////////////////////////////////////////////////////////////////
   // Global geometry counters.
 
index 50143dd46e1fcda3ee62de61fb422a8b94bdf9b9..83b1e4a6ce99f94c399f229201aea88eda5d5b21 100644 (file)
@@ -110,6 +110,12 @@ static OpenSubdiv_FVarLinearInterpolation get_fvar_linear_interpolation(
        return BKE_subdiv_converter_fvar_linear_from_settings(&storage->settings);
 }
 
+static bool specifies_full_topology(
+        const OpenSubdiv_Converter *UNUSED(converter))
+{
+       return false;
+}
+
 static int get_num_faces(const OpenSubdiv_Converter *converter)
 {
        ConverterStorage *storage = converter->user_data;
@@ -480,8 +486,8 @@ static void free_user_data(const OpenSubdiv_Converter *converter)
 static void init_functions(OpenSubdiv_Converter *converter)
 {
        converter->getSchemeType = get_scheme_type;
-
        converter->getFVarLinearInterpolation = get_fvar_linear_interpolation;
+       converter->specifiesFullTopology = specifies_full_topology;
 
        converter->getNumFaces = get_num_faces;
        converter->getNumEdges = get_num_edges;