diff --git a/opensubdiv/bfr/faceVertex.cpp b/opensubdiv/bfr/faceVertex.cpp index 9492e4f5..273e95eb 100644 --- a/opensubdiv/bfr/faceVertex.cpp +++ b/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 #include @@ -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: - isNonManifoldCrease = (numNonManifoldEdges == 2) && - (GetNumFaces() > numEdges); + // non-manifold edges radiating more than two sets of faces. + // This requires closer inspection to confirm, so make use of + // 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 MemberArray; + typedef Vtr::ConstArray 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 diff --git a/opensubdiv/bfr/faceVertex.h b/opensubdiv/bfr/faceVertex.h index 407ff32b..94da1d8f 100644 --- a/opensubdiv/bfr/faceVertex.h +++ b/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(); diff --git a/opensubdiv/far/topologyRefinerFactory.cpp b/opensubdiv/far/topologyRefinerFactory.cpp index 1e469497..f713a517 100644 --- a/opensubdiv/far/topologyRefinerFactory.cpp +++ b/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) && - (vFaces.size() > vEdges.size()))) { + bool isNonManCrease = (nonManifoldEdgeCount == 2) && + (boundaryEdgeCount == 0) && + (vFaces.size() > vEdges.size()) && + baseLevel.testVertexNonManifoldCrease(vIndex); + if (!isNonManCrease) { vSharpness = Sdc::Crease::SHARPNESS_INFINITE; } } diff --git a/opensubdiv/vtr/level.cpp b/opensubdiv/vtr/level.cpp index 46fad186..29723303 100644 --- a/opensubdiv/vtr/level.cpp +++ b/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... // diff --git a/opensubdiv/vtr/level.h b/opensubdiv/vtr/level.h index 507edb84..4380225c 100644 --- a/opensubdiv/vtr/level.h +++ b/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; diff --git a/regression/bfr_evaluate/init_shapes_all.h b/regression/bfr_evaluate/init_shapes_all.h index 0401d8f2..ea6cb8b9 100644 --- a/regression/bfr_evaluate/init_shapes_all.h +++ b/regression/bfr_evaluate/init_shapes_all.h @@ -79,6 +79,7 @@ initShapesAll(std::vector & 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)); diff --git a/regression/shapes/all.h b/regression/shapes/all.h index 89c69b3c..af874df5 100644 --- a/regression/shapes/all.h +++ b/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" diff --git a/regression/shapes/catmark_nonman_creases.h b/regression/shapes/catmark_nonman_creases.h new file mode 100644 index 00000000..a6509270 --- /dev/null +++ b/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" +;