Fixed a few more minor issues with face-varying patches in uniform PatchTables:

- corrected assertion to use face-varying index offset instead of vertex
    - added missing face-varying conditional when generateAllLevels set
    - factored base-level index offsets into local options for clarity
    - added Doxygen comments to describe overall indexing behavior

opensubdiv/far/patchTableFactory.cpp

@@ -921,6 +921,13 @@ PatchTableFactory::createUniform(TopologyRefiner const & refiner, Options option
 
     BuilderContext context(refiner, options);
 
+    // Default behavior is to include base level vertices in the patch vertices for
+    // vertex and varying patches, but not face-varying.  Consider exposing these as
+    // public options in future so that clients can create consistent behavior:
+
+    bool includeBaseLevelIndices     = true;
+    bool includeBaseLevelFVarIndices = false;
+
     // ensure that triangulateQuads is only set for quadrilateral schemes
     options.triangulateQuads &= (refiner.GetSchemeType()==Sdc::SCHEME_BILINEAR ||
                                  refiner.GetSchemeType()==Sdc::SCHEME_CATMARK);
@@ -990,8 +997,7 @@ PatchTableFactory::createUniform(TopologyRefiner const & refiner, Options option
     Index         ** fptr  = 0;
     PatchParam    ** fpptr = 0;
 
-    // we always skip level=0 vertices (control cages)
+    Index levelVertOffset = includeBaseLevelIndices ? refiner.GetLevel(0).GetNumVertices() : 0;
-    Index levelVertOffset = refiner.GetLevel(0).GetNumVertices();
 
     Index * levelFVarVertOffsets = 0;
     if (context.RequiresFVarPatches()) {
@@ -1004,6 +1010,11 @@ PatchTableFactory::createUniform(TopologyRefiner const & refiner, Options option
         for (int fvc=0; fvc<(int)context.fvarChannelIndices.size(); ++fvc) {
             fptr[fvc] = table->getFVarValues(fvc).begin();
             fpptr[fvc] = table->getFVarPatchParams(fvc).begin();
+
+            if (includeBaseLevelFVarIndices) {
+                int refinerChannel = context.fvarChannelIndices[fvc];
+                levelFVarVertOffsets[fvc] = refiner.GetLevel(0).GetNumFVarValues(refinerChannel);
+            }
         }
     }
 
@@ -1033,7 +1044,7 @@ PatchTableFactory::createUniform(TopologyRefiner const & refiner, Options option
 
                         ConstIndexArray fvalues = refLevel.GetFaceFVarValues(face, refinerChannel);
                         for (int vert=0; vert<fvalues.size(); ++vert) {
-                            assert((levelVertOffset + fvalues[vert]) < (int)table->getFVarValues(fvc).size());
+                            assert((levelFVarVertOffsets[fvc] + fvalues[vert]) < (int)table->getFVarValues(fvc).size());
                             fptr[fvc][vert] = levelFVarVertOffsets[fvc] + fvalues[vert];
                         }
                         fptr[fvc]+=fvalues.size();
@@ -1066,9 +1077,12 @@ PatchTableFactory::createUniform(TopologyRefiner const & refiner, Options option
 
         if (options.generateAllLevels) {
             levelVertOffset += refiner.GetLevel(level).GetNumVertices();
-            for (int fvc=0; fvc<(int)context.fvarChannelIndices.size(); ++fvc) {
+
-                int refinerChannel = context.fvarChannelIndices[fvc];
+            if (context.RequiresFVarPatches()) {
-                levelFVarVertOffsets[fvc] += refiner.GetLevel(level).GetNumFVarValues(refinerChannel);
+                for (int fvc=0; fvc<(int)context.fvarChannelIndices.size(); ++fvc) {
+                    int refinerChannel = context.fvarChannelIndices[fvc];
+                    levelFVarVertOffsets[fvc] += refiner.GetLevel(level).GetNumFVarValues(refinerChannel);
+                }
             }
         }
     }

opensubdiv/far/patchTableFactory.h

@@ -98,7 +98,25 @@ public:
         int const *  fvarChannelIndices;       ///< List containing the indices of the channels selected for the factory
     };
 
-    /// \brief Factory constructor for PatchTable
+    /// \brief Instantiates a PatchTable from a client-provided TopologyRefiner.
+    ///
+    ///  A PatchTable can be constructed from a TopologyRefiner that has been
+    ///  either adaptively or uniformly refined.  In both cases, the resulting
+    ///  patches reference vertices in the various refined levels by index,
+    ///  and those indices accumulate with the levels in different ways.
+    ///
+    ///  For adaptively refined patches, patches are defined at different levels,
+    ///  including the base level, so the indices of patch vertices include
+    ///  vertices from all levels.
+    ///
+    ///  For uniformly refined patches, all patches are completely defined within
+    ///  the last level.  There is often no use for intermediate levels and they
+    ///  can usually be ignored.  Indices of patch vertices might therefore be
+    ///  expected to be defined solely within the last level.  While this is true
+    ///  for face-varying patches, for historical reasons it is not the case for
+    ///  vertex and varying patches.  Indices for vertex and varying patches include
+    ///  the base level in addition to the last level while indices for face-varying
+    ///  patches include only the last level.
     ///
     /// @param refiner              TopologyRefiner from which to generate patches
     ///