OpenSubdiv: Compare edge topology

This change makes it so topology refiner comparison will check vertices
of all existing/provided edges.

The initial claim that due to manifold nature of mesh there is no need
in "deep" edges check was wrong: some areas might only provide edges
with non-zero creases. So if crease of one edge goes changes from 1.0
to 0.0 and crease of other edge goes from 0.0 to 1.0 the old comparison
code would not have caught it.

intern/opensubdiv/internal/topology/mesh_topology.cc

@@ -80,6 +80,8 @@ void MeshTopology::ensureVertexTagsSize(int num_vertices)
 void MeshTopology::setNumEdges(int num_edges)
 {
   num_edges_ = num_edges;
+
+  edges_.resize(num_edges_);
 }
 
 int MeshTopology::getNumEdges() const
@@ -87,11 +89,48 @@ int MeshTopology::getNumEdges() const
   return num_edges_;
 }
 
+void MeshTopology::setEdgevertexIndices(int edge_index, int v1, int v2)
+{
+  assert(edge_index >= 0);
+  assert(edge_index < getNumEdges());
+
+  assert(v1 >= 0);
+  assert(v1 < getNumVertices());
+
+  assert(v2 >= 0);
+  assert(v2 < getNumVertices());
+
+  ensureNumEdgesAtLeast(edge_index + 1);
+
+  EdgeTopology &edge = getEdge(edge_index);
+
+  // Prevent attempts to override edges.
+  // This is currently not supposed to happen.
+  assert(!edge.isValid());
+
+  edge.v1 = v1;
+  edge.v2 = v2;
+}
+
+EdgeTopology &MeshTopology::getEdge(int edge_index)
+{
+  const MeshTopology *const_this = this;
+  return const_cast<EdgeTopology &>(const_this->getEdge(edge_index));
+}
+const EdgeTopology &MeshTopology::getEdge(int edge_index) const
+{
+  assert(edge_index >= 0);
+  assert(edge_index < getNumEdges());
+
+  return edges_[edge_index];
+}
+
 void MeshTopology::setEdgeSharpness(int edge_index, float sharpness)
 {
   assert(edge_index >= 0);
+  assert(edge_index < getNumEdges());
 
-  ensureNumEdgesAtLeast(edge_index + 1);
+  assert(getEdge(edge_index).isValid());
 
   if (sharpness < 1e-6f) {
     return;

intern/opensubdiv/internal/topology/mesh_topology.h

@@ -32,6 +32,17 @@ class VertexTopologyTag {
   float sharpness = 0.0f;
 };
 
+class EdgeTopology {
+ public:
+  bool isValid() const
+  {
+    return v1 >= 0 && v2 >= 0;
+  }
+
+  int v1 = -1;
+  int v2 = -1;
+};
+
 class EdgeTopologyTag {
  public:
   float sharpness = 0.0f;
@@ -68,6 +79,11 @@ class MeshTopology {
   // on last edge index for which topology tag was specified.
   int getNumEdges() const;
 
+  void setEdgevertexIndices(int edge_index, int v1, int v2);
+
+  EdgeTopology &getEdge(int edge_index);
+  const EdgeTopology &getEdge(int edge_index) const;
+
   void setEdgeSharpness(int edge_index, float sharpness);
   float getEdgeSharpness(int edge_index) const;
 
@@ -94,6 +110,7 @@ class MeshTopology {
   vector<VertexTopologyTag> vertex_tags_;
 
   int num_edges_;
+  vector<EdgeTopology> edges_;
   vector<EdgeTopologyTag> edge_tags_;
 
   MEM_CXX_CLASS_ALLOC_FUNCS("MeshTopology");

intern/opensubdiv/internal/topology/mesh_topology_compare.cc

@@ -28,6 +28,52 @@ namespace opensubdiv {
 
 namespace {
 
+////////////////////////////////////////////////////////////////////////////////
+// Geometry.
+
+// Edges.
+
+int getEffectiveNumEdges(const OpenSubdiv_Converter *converter)
+{
+  if (converter->getNumEdges == nullptr) {
+    return 0;
+  }
+
+  return converter->getNumEdges(converter);
+}
+
+bool isEqualEdgeGeometry(const MeshTopology &mesh_topology, const OpenSubdiv_Converter *converter)
+{
+  const int num_requested_edges = getEffectiveNumEdges(converter);
+  if (num_requested_edges != mesh_topology.getNumEdges()) {
+    return false;
+  }
+
+  for (int edge_index = 0; edge_index < num_requested_edges; ++edge_index) {
+    int requested_edge_vertices[2];
+    converter->getEdgeVertices(converter, edge_index, requested_edge_vertices);
+
+    const EdgeTopology &current_edge = mesh_topology.getEdge(edge_index);
+    if (current_edge.v1 != requested_edge_vertices[0] ||
+        current_edge.v2 != requested_edge_vertices[1]) {
+      return false;
+    }
+  }
+
+  return true;
+}
+
+// Geometry comparison entry point.
+
+bool isEqualGeometry(const MeshTopology &mesh_topology, const OpenSubdiv_Converter *converter)
+{
+  if (!isEqualEdgeGeometry(mesh_topology, converter)) {
+    return false;
+  }
+
+  return true;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // Geometry tags.
 
@@ -90,6 +136,20 @@ bool isEqualEdgeTags(const MeshTopology &mesh_topology, const OpenSubdiv_Convert
   return true;
 }
 
+// Tags comparison entry point.
+
+bool isEqualTags(const MeshTopology &mesh_topology, const OpenSubdiv_Converter *converter)
+{
+  if (!isEqualVertexTags(mesh_topology, converter)) {
+    return false;
+  }
+  if (!isEqualEdgeTags(mesh_topology, converter)) {
+    return false;
+  }
+
+  return true;
+}
+
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -97,10 +157,13 @@ bool isEqualEdgeTags(const MeshTopology &mesh_topology, const OpenSubdiv_Convert
 
 bool MeshTopology::isEqualToConverter(const OpenSubdiv_Converter *converter) const
 {
-  if (!isEqualVertexTags(*this, converter)) {
+  // Geometry.
+  if (!isEqualGeometry(*this, converter)) {
     return false;
   }
-  if (!isEqualEdgeTags(*this, converter)) {
+
+  // Tags.
+  if (!isEqualTags(*this, converter)) {
     return false;
   }
 

intern/opensubdiv/internal/topology/topology_refiner_factory.cc

@@ -57,6 +57,28 @@ inline bool TopologyRefinerFactory<TopologyRefinerData>::resizeComponentTopology
   const OpenSubdiv_Converter *converter = cb_data.converter;
   MeshTopology *base_mesh_topology = cb_data.base_mesh_topology;
 
+  // Vertices.
+  const int num_vertices = converter->getNumVertices(converter);
+  base_mesh_topology->setNumVertices(num_vertices);
+  setNumBaseVertices(refiner, num_vertices);
+
+  // Edges.
+  //
+  // NOTE: Always store edges in the base mesh topology so then comparison can
+  // happen, but only provide edges to TopologyRefiner if full topology is
+  // specified (if full topology is not specified then topology refiner must
+  // not see any edges, which will indicate to it that winding and edges are to
+  // be reconstructed).
+  //
+  // NOTE: it is a possible usecase when user code does not need crease at all
+  // (which is the only real reason why converter would want to provide edges in
+  // the case of partial topology specification). So it might be so getNumEdges
+  // callback is nullptr.
+  if (converter->getNumEdges != nullptr) {
+    const int num_edges = converter->getNumEdges(converter);
+    base_mesh_topology->setNumEdges(num_edges);
+  }
+
   // Faces and face-vertices.
   const int num_faces = converter->getNumFaces(converter);
   setNumBaseFaces(refiner, num_faces);
@@ -65,19 +87,16 @@ inline bool TopologyRefinerFactory<TopologyRefinerData>::resizeComponentTopology
     setNumBaseFaceVertices(refiner, face_index, num_face_vertices);
   }
 
-  // Vertices.
-  const int num_vertices = converter->getNumVertices(converter);
-  base_mesh_topology->setNumVertices(num_vertices);
-  setNumBaseVertices(refiner, num_vertices);
-
   // If converter does not provide full topology, we are done.
+  //
+  // The rest is needed to define relations between faces-of-edge and
+  // edges-of-vertex, which is not available for partially specified mesh.
   if (!converter->specifiesFullTopology(converter)) {
     return true;
   }
 
   // Edges and edge-faces.
   const int num_edges = converter->getNumEdges(converter);
-  base_mesh_topology->setNumEdges(num_edges);
   setNumBaseEdges(refiner, num_edges);
   for (int edge_index = 0; edge_index < num_edges; ++edge_index) {
     const int num_edge_faces = converter->getNumEdgeFaces(converter, edge_index);
@@ -99,33 +118,45 @@ template<>
 inline bool TopologyRefinerFactory<TopologyRefinerData>::assignComponentTopology(
     TopologyRefiner &refiner, const TopologyRefinerData &cb_data)
 {
+  using blender::opensubdiv::EdgeTopology;
+  using blender::opensubdiv::MeshTopology;
   using Far::IndexArray;
+
   const OpenSubdiv_Converter *converter = cb_data.converter;
+  MeshTopology *base_mesh_topology = cb_data.base_mesh_topology;
+
   const bool full_topology_specified = converter->specifiesFullTopology(converter);
-  // Face relations.
+
+  // Vertices of an edge.
+  //
+  // NOTE: Only specify for base mesh topology on our side.
+  // They will be provided to the topology refiner only if the full topology is
+  // specified.
+  if (converter->getNumEdges != nullptr) {
+    const int num_edges = converter->getNumEdges(converter);
+    for (int edge_index = 0; edge_index < num_edges; ++edge_index) {
+      int edge_vertices[2];
+      converter->getEdgeVertices(converter, edge_index, edge_vertices);
+
+      base_mesh_topology->setEdgevertexIndices(edge_index, edge_vertices[0], edge_vertices[1]);
+    }
+  }
+
+  // Vertices of face.
   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]);
-    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.
+  //
+  // The rest is needed to define relations between faces-of-edge and
+  // edges-of-vertex, which is not available for partially specified mesh.
   if (!full_topology_specified) {
     return true;
   }
-  // Edge relations.
-  const int num_edges = converter->getNumEdges(converter);
-  for (int edge_index = 0; edge_index < num_edges; ++edge_index) {
-    // Edge-vertices.
-    IndexArray dst_edge_vertices = getBaseEdgeVertices(refiner, edge_index);
-    converter->getEdgeVertices(converter, edge_index, &dst_edge_vertices[0]);
-    // Edge-faces.
-    IndexArray dst_edge_faces = getBaseEdgeFaces(refiner, edge_index);
-    converter->getEdgeFaces(converter, edge_index, &dst_edge_faces[0]);
-  }
+
   // Vertex relations.
   const int num_vertices = converter->getNumVertices(converter);
   vector<int> vertex_faces, vertex_edges;
@@ -135,6 +166,7 @@ inline bool TopologyRefinerFactory<TopologyRefinerData>::assignComponentTopology
     const int num_vertex_faces = converter->getNumVertexFaces(converter, vertex_index);
     vertex_faces.resize(num_vertex_faces);
     converter->getVertexFaces(converter, vertex_index, &vertex_faces[0]);
+
     // Vertex-edges.
     IndexArray dst_vertex_edges = getBaseVertexEdges(refiner, vertex_index);
     const int num_vertex_edges = converter->getNumVertexEdges(converter, vertex_index);
@@ -143,7 +175,29 @@ inline bool TopologyRefinerFactory<TopologyRefinerData>::assignComponentTopology
     memcpy(&dst_vertex_edges[0], &vertex_edges[0], sizeof(int) * num_vertex_edges);
     memcpy(&dst_vertex_faces[0], &vertex_faces[0], sizeof(int) * num_vertex_faces);
   }
+
+  // Edge relations.
+  const int num_edges = converter->getNumEdges(converter);
+  for (int edge_index = 0; edge_index < num_edges; ++edge_index) {
+    // Vertices this edge connects.
+    const EdgeTopology &edge = base_mesh_topology->getEdge(edge_index);
+    IndexArray dst_edge_vertices = getBaseEdgeVertices(refiner, edge_index);
+    dst_edge_vertices[0] = edge.v1;
+    dst_edge_vertices[1] = edge.v2;
+
+    // Faces adjacent to this edge.
+    IndexArray dst_edge_faces = getBaseEdgeFaces(refiner, edge_index);
+    converter->getEdgeFaces(converter, edge_index, &dst_edge_faces[0]);
+  }
+
+  // Face relations.
+  for (int face_index = 0; face_index < num_faces; ++face_index) {
+    IndexArray dst_face_edges = getBaseFaceEdges(refiner, face_index);
+    converter->getFaceEdges(converter, face_index, &dst_face_edges[0]);
+  }
+
   populateBaseLocalIndices(refiner);
+
   return true;
 }
 

intern/opensubdiv/internal/topology/topology_refiner_impl_compare.cc

@@ -226,9 +226,6 @@ bool checkGeometryFacesMatch(const TopologyRefinerImpl *topology_refiner_impl,
 bool checkGeometryMatches(const TopologyRefinerImpl *topology_refiner_impl,
                           const OpenSubdiv_Converter *converter)
 {
-  // NOTE: Since OpenSubdiv's topology refiner doesn't contain loose edges, we
-  // are only checking for faces to be matched. Changes in edges we don't care
-  // here too much (they'll be checked for creases changes later).
   return checkGeometryFacesMatch(topology_refiner_impl, converter);
 }
 
@@ -279,9 +276,6 @@ bool checkUVLayersMatch(const TopologyRefinerImpl *topology_refiner_impl,
 bool checkTopologyAttributesMatch(const TopologyRefinerImpl *topology_refiner_impl,
                                   const OpenSubdiv_Converter *converter)
 {
-  if (!topology_refiner_impl->base_mesh_topology.isEqualToConverter(converter)) {
-    return false;
-  }
   return checkUVLayersMatch(topology_refiner_impl, converter);
 }
 
@@ -289,9 +283,21 @@ bool checkTopologyAttributesMatch(const TopologyRefinerImpl *topology_refiner_im
 
 bool TopologyRefinerImpl::isEqualToConverter(const OpenSubdiv_Converter *converter) const
 {
-  return (blender::opensubdiv::checkPreliminaryMatches(this, converter) &&
-          blender::opensubdiv::checkGeometryMatches(this, converter) &&
-          blender::opensubdiv::checkTopologyAttributesMatch(this, converter));
+  if (!blender::opensubdiv::checkPreliminaryMatches(this, converter)) {
+    return false;
+  }
+  if (!blender::opensubdiv::checkGeometryMatches(this, converter)) {
+    return false;
+  }
+  if (!blender::opensubdiv::checkTopologyAttributesMatch(this, converter)) {
+    return false;
+  }
+
+  if (!base_mesh_topology.isEqualToConverter(converter)) {
+    return false;
+  }
+
+  return true;
 }
 
 }  // namespace opensubdiv