vertex indices for Far::PatchTable are non-intuitive for uniformly-refined patches

[barfowl]

When constructing a Far::PatchTable from a uniformly refined mesh, a different indexing scheme is used for the control vertices of the patches compared to what is used for adaptive refinement. This is confusing and warrants attention.

For adaptive patches, vertices for all refined levels are included and indices begin at level 0. For uniform patches, intermediate levels are excluded and vertices are expected from the last level in addition to those of the cage (level 0). Using only vertices in the last level makes some sense, but this latter requirement that the cage vertices also be included is unexpected and unnecessary.

The addition to PatchTableFactory::Options is one possibility to improve this. Three choices that makes sense for patches derived from a uniform refinement are to include all levels (same as adaptive), the last only, or the last level in addition to the cage (a legacy choice that may eventually be removed).

[barfowl]

Just drawing attention to this issue as there will likely be upcoming additions to PatchTableFactory::Options to support face-varying patches in 3.1. So it would be a good time to consider additional Options to address this issue at the time.

[manuelk]

IIRC, the 3 options you described above were lost in the transition from OSD 2.x to 3.x - so to me they make sense.

We might need to add some bit-flags to the data structure itself to track which "mode" it has been created with.

In the long run, we will probably want to move on from patch tables, but that's a topic for another discussion...

[barfowl]

This issue came up again after helping someone debug some unexpected behavior with face-varying patches (leading to #946). The issue here is made worse by the fact that vertex/varying patches are not treated the same as face-varying patches when it comes to indices of the patch points and the levels involved. The vertex/varying patches expect the base level vertices to be include while face-varying patches do not.

I added some improvements in #950, including Doxygen comments for PatchTableFactory::Create(). The options added internal to the source are worth considering for public exposure in a future release that includes API changes (a little simpler than the three options previously noted given the apparent need to treat face-varying separately).

[jtran56]

Filed as internal issue #151657.

[barfowl]

This has been addressed by #986. The default behavior remains non-intuitive, but public options are now available to control it.