Merge pull request #1338 from barfowl/nonman_crease_patches

Fix sharpening of vertices on potential non-manifold creases

opensubdiv/bfr/faceVertex.cpp

@@ -24,6 +24,8 @@
 
 #include "../bfr/faceVertex.h"
 #include "../sdc/crease.h"
+#include "../vtr/array.h"
+#include "../vtr/stackBuffer.h"
 
 #include <algorithm>
 #include <cstring>
@@ -628,7 +630,7 @@ FaceVertex::ConnectUnOrderedFaces(Index const fvIndices[]) {
     //  properties of the corner:
     assignUnOrderedFaceNeighbors(edges, feEdges);
 
-    finalizeUnOrderedTags(edges, numEdges);
+    finalizeUnOrderedTags(edges, numEdges, fvIndices);
 }
 
 //
@@ -797,7 +799,8 @@ FaceVertex::assignUnOrderedFaceNeighbors(Edge const  edges[],
 }
 
 void
-FaceVertex::finalizeUnOrderedTags(Edge const edges[], int numEdges) {
+FaceVertex::finalizeUnOrderedTags(Edge const edges[], int numEdges,
+                                  Index const fvIndices[]) {
 
     //
     //  Summarize properties of the corner given the number and nature of
@@ -850,9 +853,12 @@ FaceVertex::finalizeUnOrderedTags(Edge const edges[], int numEdges) {
 
         if (!hasDegenerateEdges && !hasDuplicateEdges && !hasBoundaryEdges) {
             //  Special crease case that avoids sharpening: two interior
-            //  non-manifold edges radiating more than two sets of faces:
+            //  non-manifold edges radiating more than two sets of faces.
-            isNonManifoldCrease = (numNonManifoldEdges == 2) &&
+            //  This requires closer inspection to confirm, so make use of
-                                  (GetNumFaces() > numEdges);
+            //  as many pre-conditions available here to avoid doing so:
+            isNonManifoldCrease = !_isExpInfSharp &&
+                (numNonManifoldEdges == 2) && (GetNumFaces() > numEdges) &&
+                testNonManifoldCrease(edges, numEdges, fvIndices);
         }
     } else {
         //  Mismatch between number of incident faces and edges:
@@ -897,6 +903,102 @@ FaceVertex::finalizeUnOrderedTags(Edge const edges[], int numEdges) {
     }
 }
 
+bool
+FaceVertex::testNonManifoldCrease(Edge const edges[], int numEdges,
+                                  Index const fvIndices[]) const {
+    //
+    //  Local struct that keeps track of face-corners remaining around
+    //  a vertex as those visited as part of connected manifold subsets
+    //  are removed during inspection:
+    //
+    struct FaceCornerArray {
+        typedef Vtr::internal::StackBuffer<int,32,true> MemberArray;
+        typedef Vtr::ConstArray<int>                    SearchArray;
+
+        MemberArray leading;
+        MemberArray trailing;
+        int         size;
+
+        FaceCornerArray(int n) : leading(n), trailing(n), size(n) { }
+
+        int FindLeading(int vertex) const {
+            return SearchArray(leading, size).FindIndex(vertex);
+        }
+        int FindTrailing(int vertex) const {
+            return SearchArray(trailing, size).FindIndex(vertex);
+        }
+        void RemoveFace(int index) {
+            std::swap(leading[index],  leading[size - 1]);
+            std::swap(trailing[index], trailing[size - 1]);
+            -- size;
+        }
+        int RemoveManifoldSubset(int startVertex, int endVertex) {
+            assert(startVertex != endVertex);
+
+            for (int nextVertex = startVertex; nextVertex != endVertex; ) {
+                int faceIndex = FindLeading(nextVertex);
+                if (faceIndex < 0) return (nextVertex == startVertex) ? 0 : -1;
+
+                nextVertex = trailing[faceIndex];
+                if (nextVertex == startVertex) return -1;
+
+                RemoveFace(faceIndex);
+            }
+            return 1;
+        }
+    };
+
+    //
+    //  Identify the vertices at the ends of the two non-manifold edges
+    //  of the potential crease:
+    //
+    int creaseEnd[2] = { -1, -1 };
+
+    for (int i = 0; i < numEdges; ++i) {
+        if (edges[i].nonManifold) {
+            creaseEnd[creaseEnd[0] >= 0] = edges[i].endVertex;
+        }
+    }
+    assert((creaseEnd[0] >= 0) && (creaseEnd[1] >= 0));
+
+    //
+    //  Use the locally defined set of face-edge pairs to determine if
+    //  the collection of faces around the vertex forms a non-manifold
+    //  crease.
+    //
+    //  First initialize the FaceCornerArray from the face vertices:
+    //
+    int numFaces = GetNumFaces();
+
+    FaceCornerArray faceCorners(numFaces);
+
+    for (int i = 0; i < numFaces; ++i) {
+        faceCorners.leading[i]  = GetFaceIndexLeading( i, fvIndices);
+        faceCorners.trailing[i] = GetFaceIndexTrailing(i, fvIndices);
+    }
+
+    //  Remove faces for manifold subsets in one direction:
+    int removed = 0;
+    do {
+        removed = faceCorners.RemoveManifoldSubset(creaseEnd[0], creaseEnd[1]);
+        if (removed < 0) {
+            return false;
+        }
+    } while (removed);
+
+    if (faceCorners.size == 0) return true;
+
+    //  Remove faces for manifold subsets in the other direction:
+    do {
+        removed = faceCorners.RemoveManifoldSubset(creaseEnd[1], creaseEnd[0]);
+        if (removed < 0) {
+            return false;
+        }
+    } while (removed);
+
+    return (faceCorners.size == 0);
+}
+
 } // end namespace Bfr
 
 } // end namespace OPENSUBDIV_VERSION

opensubdiv/bfr/faceVertex.h

@@ -200,7 +200,11 @@ private:
     void assignUnOrderedFaceNeighbors(Edge const  edges[],
                                       short const faceEdgeIndices[]);
 
-    void finalizeUnOrderedTags(Edge const edges[], int numEdges);
+    bool testNonManifoldCrease(Edge const edges[], int numEdges,
+                               Index const faceVertIndices[]) const;
+
+    void finalizeUnOrderedTags(Edge const edges[], int numEdges,
+                               Index const faceVertIndices[]);
 
     //  Ordered counterpart to the above method for finalizing tags
     void finalizeOrderedTags();

opensubdiv/far/topologyRefinerFactory.cpp

@@ -304,16 +304,23 @@ TopologyRefinerFactoryBase::prepareComponentTagsAndSharpness(
         bool isSharpenedCorner =  isTopologicalCorner && sharpenCornerVerts;
         if (isSharpenedCorner) {
             vSharpness = Sdc::Crease::SHARPNESS_INFINITE;
-        } else if (vTag._nonManifold && sharpenNonManFeatures) {
+        } else if (vTag._nonManifold && sharpenNonManFeatures &&
+                   !Sdc::Crease::IsInfinite(vSharpness)) {
             //
             //  We avoid sharpening non-manifold vertices when they occur on
             //  interior non-manifold creases, i.e. a pair of opposing non-
             //  manifold edges with more than two incident faces.  In these
             //  cases there are more incident faces than edges (1 more for
-            //  each additional "fin") and no boundaries.
+            //  each additional "fin") and no boundaries. Closer inspection
+            //  of manifold subsets around the vertex is required to truly
+            //  determine the crease case, so avoid it using pre-conditions
+            //  that are available here:
             //
-            if (! ((nonManifoldEdgeCount == 2) && (boundaryEdgeCount == 0) &&
+            bool isNonManCrease = (nonManifoldEdgeCount == 2) &&
-                   (vFaces.size() > vEdges.size()))) {
+                                  (boundaryEdgeCount == 0) &&
+                                  (vFaces.size() > vEdges.size()) &&
+                                  baseLevel.testVertexNonManifoldCrease(vIndex);
+            if (!isNonManCrease) {
                 vSharpness = Sdc::Crease::SHARPNESS_INFINITE;
             }
         }

opensubdiv/vtr/level.cpp

@@ -2029,6 +2029,81 @@ Level::orderVertexFacesAndEdges(Index vIndex) {
     return false;
 }
 
+bool
+Level::testVertexNonManifoldCrease(Index vIndex) const {
+
+    //
+    //  Identify the two non-manifold edges for closer inspection -- the
+    //  two edges must have more than two incident faces and the same
+    //  number of incident faces. All other edges are manifold interior:
+    //
+    Index nonManEdges[2] = { -1, -1 };
+
+    ConstIndexArray vEdges = getVertexEdges(vIndex);
+    for (int i = 0; i < vEdges.size(); ++i) {
+        if (isEdgeNonManifold(vEdges[i])) {
+            if (nonManEdges[1] >= 0) return false;
+            nonManEdges[nonManEdges[0] >= 0] = vEdges[i];
+        } else if (getEdgeFaces(vEdges[i]).size() != 2) {
+            return false;
+        }
+    }
+    if (getEdgeFaces(nonManEdges[0]).size() != getEdgeFaces(nonManEdges[1]).size()) {
+        return false;
+    }
+
+    //
+    //  For each of the two non-manifold edges, inspect their incident faces
+    //  to confirm that the manifold subset connected to each is delimited by
+    //  the two edges (not one):
+    //
+    int numFacesTraversed = 0;
+
+    for (int i = 0; i < 2; ++i) {
+        Index eIndex = nonManEdges[i];
+        Index eStart = nonManEdges[i != 0];
+        Index eEnd   = nonManEdges[i == 0];
+
+        ConstIndexArray      eFaces  = getEdgeFaces(eIndex);
+        ConstLocalIndexArray eInFace = getEdgeFaceLocalIndices(eIndex);
+
+        //
+        //  For each face incident the two non-manifold edges, identify
+        //  manifold subset of faces associated with each. First test
+        //  the orientation of the edge in the starting face -- skipping
+        //  this face if the edge is reversed as it should be at the end
+        //  of a subset starting from the other crease edge:
+        //
+        for (int j = 0; j < eFaces.size(); ++j) {
+            Index fStart = eFaces[j];
+
+            ConstIndexArray fVerts = getFaceVertices(fStart);
+            if (fVerts[eInFace[j]] != vIndex) continue;
+
+            //
+            //  For each successive leading edge of a manifold sequence of
+            //  faces, identify the face following that edge (if not the
+            //  first) and edge that precedes it in the face -- continuing
+            //  until the ending edge is reached:
+            //
+            for (Index eNext = eStart, fNext = fStart; eNext != eEnd; ) {
+                if (eNext != eStart) {
+                    ConstIndexArray fPair = getEdgeFaces(eNext);
+                    fNext = fPair[fPair[0] == fNext];
+                }
+                numFacesTraversed++;
+
+                ConstIndexArray fEdges = getFaceEdges(fNext);
+                int iNext = fEdges.FindIndex(eNext);
+
+                eNext = fEdges[iNext ? (iNext - 1) : (fEdges.size() - 1)];
+                if (eNext == eStart) return false;
+            }
+        }
+    }
+    return (numFacesTraversed == getVertexFaces(vIndex).size());
+}
+
 //
 //  In development -- methods for accessing face-varying data channels...
 //

opensubdiv/vtr/level.h

@@ -472,6 +472,7 @@ public:
     void orientIncidentComponents();
     bool orderVertexFacesAndEdges(Index vIndex, Index* vFaces, Index* vEdges) const;
     bool orderVertexFacesAndEdges(Index vIndex);
+    bool testVertexNonManifoldCrease(Index vIndex) const;
     void populateLocalIndices();
 
     IndexArray shareFaceVertCountsAndOffsets() const;

regression/bfr_evaluate/init_shapes_all.h

@@ -79,6 +79,7 @@ initShapesAll(std::vector<ShapeDesc> & shapes) {
     shapes.push_back(ShapeDesc("catmark_nonman_quadpole64", catmark_nonman_quadpole64, kCatmark));
     shapes.push_back(ShapeDesc("catmark_nonman_quadpole360", catmark_nonman_quadpole360, kCatmark));
     shapes.push_back(ShapeDesc("catmark_nonman_bareverts", catmark_nonman_bareverts, kCatmark));
+    shapes.push_back(ShapeDesc("catmark_nonman_creases", catmark_nonman_creases, kCatmark));
     shapes.push_back(ShapeDesc("catmark_nonquads", catmark_nonquads, kCatmark));
     shapes.push_back(ShapeDesc("catmark_pawn", catmark_pawn, kCatmark));
     shapes.push_back(ShapeDesc("catmark_pole8", catmark_pole8, kCatmark));

regression/shapes/all.h

@@ -80,6 +80,7 @@
 #include "catmark_nonman_quadpole64.h"
 #include "catmark_nonman_quadpole360.h"
 #include "catmark_nonman_bareverts.h"
+#include "catmark_nonman_creases.h"
 #include "catmark_nonquads.h"
 #include "catmark_pawn.h"
 #include "catmark_pyramid_creases0.h"

regression/shapes/catmark_nonman_creases.h

@@ -0,0 +1,210 @@
+//
+//   Copyright 2024 Pixar
+//
+//   Licensed under the Apache License, Version 2.0 (the "Apache License")
+//   with the following modification; you may not use this file except in
+//   compliance with the Apache License and the following modification to it:
+//   Section 6. Trademarks. is deleted and replaced with:
+//
+//   6. Trademarks. This License does not grant permission to use the trade
+//      names, trademarks, service marks, or product names of the Licensor
+//      and its affiliates, except as required to comply with Section 4(c) of
+//      the License and to reproduce the content of the NOTICE file.
+//
+//   You may obtain a copy of the Apache License at
+//
+//       http://www.apache.org/licenses/LICENSE-2.0
+//
+//   Unless required by applicable law or agreed to in writing, software
+//   distributed under the Apache License with the above modification is
+//   distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+//   KIND, either express or implied. See the Apache License for the specific
+//   language governing permissions and limitations under the Apache License.
+//
+
+static const std::string catmark_nonman_creases =
+"#\n"
+"#   Four shapes ordered left->right and top->bottom in the XZ plane\n"
+"#\n"
+"#   Shape 1:  top-left\n"
+"#\n"
+"v -1.25  0.0  0.65\n"
+"v -0.75  0.0  0.90\n"
+"v -0.25  0.0  0.65\n"
+"v -1.25  0.0  1.05\n"
+"v -0.75  0.0  1.25\n"
+"v -0.25  0.0  1.05\n"
+"v -1.25 -0.3  0.25\n"
+"v -0.75 -0.3  0.55\n"
+"v -0.25 -0.3  0.25\n"
+"v -1.25  0.3  0.25\n"
+"v -0.75  0.3  0.55\n"
+"v -0.25  0.3  0.25\n"
+"\n"
+"vt 0.15 0.525\n"
+"vt 0.25 0.575\n"
+"vt 0.35 0.525\n"
+"vt 0.15 0.625\n"
+"vt 0.25 0.675\n"
+"vt 0.35 0.625\n"
+"vt 0.15 0.675\n"
+"vt 0.25 0.725\n"
+"vt 0.35 0.675\n"
+"vt 0.15 0.775\n"
+"vt 0.25 0.825\n"
+"vt 0.35 0.775\n"
+"vt 0.15 0.825\n"
+"vt 0.25 0.875\n"
+"vt 0.35 0.825\n"
+"vt 0.15 0.925\n"
+"vt 0.25 0.975\n"
+"vt 0.35 0.925\n"
+"\n"
+"f  1/1   2/2   5/5   4/4 \n"
+"f  2/2   3/3   6/6   5/5 \n"
+"f  2/8   1/7   7/10  8/11\n"
+"f  3/9   2/8   8/11  9/12\n"
+"f  1/13  2/14 11/17 10/16\n"
+"f  2/14  3/15 12/18 11/17\n"
+"\n"
+"#\n"
+"#   Shape 2:  top-right\n"
+"#\n"
+"v  0.75  0.0  0.25\n"
+"v  1.00  0.0  0.50\n"
+"v  1.25  0.0  0.75\n"
+"v  0.50  0.0  0.50\n"
+"v  0.75  0.0  0.75\n"
+"v  1.00  0.0  1.00\n"
+"v  0.25  0.0  0.75\n"
+"v  0.50  0.0  1.00\n"
+"v  0.75  0.0  1.25\n"
+"v  0.50 -0.3  0.50\n"
+"v  0.75 -0.3  0.75\n"
+"v  1.00 -0.3  0.50\n"
+"\n"
+"vt 0.75 0.675\n"
+"vt 0.80 0.725\n"
+"vt 0.70 0.725\n"
+"vt 0.75 0.775\n"
+"vt 0.80 0.800\n"
+"vt 0.85 0.850\n"
+"vt 0.70 0.800\n"
+"vt 0.75 0.850\n"
+"vt 0.80 0.900\n"
+"vt 0.65 0.850\n"
+"vt 0.70 0.900\n"
+"vt 0.75 0.950\n"
+"vt 0.65 0.550\n"
+"vt 0.75 0.550\n"
+"vt 0.85 0.550\n"
+"vt 0.65 0.625\n"
+"vt 0.75 0.625\n"
+"vt 0.85 0.625\n"
+"\n"
+"f 13/19 14/20 17/22 16/21\n"
+"f 14/23 15/24 18/27 17/26\n"
+"f 16/25 17/26 20/29 19/28\n"
+"f 17/26 18/27 21/30 20/29\n"
+"f 16/34 22/31 23/32 17/35\n"
+"f 17/35 23/32 24/33 14/36\n"
+"\n"
+"#\n"
+"#   Shape 3:  bottom-left\n"
+"#\n"
+"v -1.25  0.0 -0.80\n"
+"v -0.75  0.0 -0.65\n"
+"v -0.25  0.0 -0.80\n"
+"v -1.25  0.0 -1.20\n"
+"v -0.75  0.0 -1.05\n"
+"v -0.25  0.0 -1.20\n"
+"v -1.25 -0.3 -0.80\n"
+"v -0.75 -0.3 -0.65\n"
+"v -0.25 -0.3 -0.80\n"
+"v -1.25  0.3 -0.80\n"
+"v -0.75  0.3 -0.65\n"
+"v -0.25  0.3 -0.80\n"
+"v -1.05  0.0 -0.30\n"
+"v -0.75 -0.3 -0.30\n"
+"v -0.45  0.0 -0.30\n"
+"v -0.75  0.3 -0.30\n"
+"\n"
+"vt 0.15 0.05\n"
+"vt 0.25 0.05\n"
+"vt 0.35 0.05\n"
+"vt 0.15 0.10\n"
+"vt 0.25 0.10\n"
+"vt 0.35 0.10\n"
+"vt 0.15 0.15\n"
+"vt 0.25 0.15\n"
+"vt 0.35 0.15\n"
+"vt 0.15 0.20\n"
+"vt 0.25 0.20\n"
+"vt 0.35 0.20\n"
+"vt 0.15 0.25\n"
+"vt 0.25 0.25\n"
+"vt 0.35 0.25\n"
+"vt 0.25 0.35\n"
+"vt 0.30 0.40\n"
+"vt 0.20 0.40\n"
+"vt 0.20 0.30\n"
+"vt 0.30 0.30\n"
+"\n"
+"f 28/37 29/38 26/41 25/40\n"
+"f 29/38 30/39 27/42 26/41\n"
+"f 31/43 32/44 26/47 25/46\n"
+"f 32/44 33/45 27/48 26/47\n"
+"f 35/50 34/49 25/46 26/47\n"
+"f 36/51 35/50 26/47 27/48\n"
+"f 37/56 26/52 38/53\n"
+"f 38/53 26/52 39/54\n"
+"f 39/54 26/52 40/55\n"
+"f 40/55 26/52 37/56\n"
+"\n"
+"#\n"
+"#   Shape 4:  bottom-right\n"
+"#\n"
+"v  0.75  0.0 -0.60\n"
+"v  0.25 -0.3 -0.60\n"
+"v  0.25  0.0 -0.90\n"
+"v  0.25  0.3 -0.60\n"
+"v  0.25  0.0 -0.30\n"
+"v  1.25 -0.3 -0.60\n"
+"v  1.25  0.0 -0.90\n"
+"v  1.25  0.3 -0.60\n"
+"v  1.25  0.0 -0.30\n"
+"v  0.25  0.0 -1.20\n"
+"v  0.75  0.0 -0.90\n"
+"v  1.25  0.0 -1.20\n"
+"\n"
+"vt 0.65 0.35\n"
+"vt 0.60 0.40\n"
+"vt 0.60 0.30\n"
+"vt 0.70 0.30\n"
+"vt 0.70 0.40\n"
+"vt 0.85 0.35\n"
+"vt 0.80 0.30\n"
+"vt 0.90 0.30\n"
+"vt 0.90 0.40\n"
+"vt 0.80 0.40\n"
+"vt 0.65 0.10\n"
+"vt 0.75 0.15\n"
+"vt 0.85 0.10\n"
+"vt 0.65 0.20\n"
+"vt 0.75 0.25\n"
+"vt 0.85 0.20\n"
+"\n"
+"f 41/57 42/58 43/59\n"
+"f 41/57 43/59 44/60\n"
+"f 41/57 44/60 45/61\n"
+"f 41/57 45/61 42/58\n"
+"f 41/62 46/63 49/66\n"
+"f 41/62 47/64 46/63\n"
+"f 41/62 48/65 47/64\n"
+"f 41/62 49/66 48/65\n"
+"f 41/71 43/70 50/67 51/68\n"
+"f 41/71 51/68 52/69 47/72\n"
+"\n"
+"t interpolateboundary 1/0/0 1\n"
+"\n"
+;