How to manage options for mesh mutations

[nmwsharp]

After #52 SurfaceMesh::flipEdge() now looks like:

  bool flip(Edge e, bool preventSelfEdges = true);

This gets at a design question: how should we expose behavior options?

Two possible strategies are:

• Options per-method to control behavior (as above)
• Stateful (mutable or immutable) flags on the class, which set a consistent behavior for all such function calls.

We should think about which of these strategies we want to pursue, and what the consequences would be.

Some motivating questions to consider:

• What is the most natural for a user? What minimizes verbosity-creep?
• What is least likely to lead to bugs, and easiest to debug?
• What scales well as these classes grow in complexity and functionality?
• What is not-too-painful to implement internally?

[GeometryCollective]

A third possibility that was raised is just a different named method, e.g., flipDelta().

Another natural thing a user might want in the case of an edge flip is to, say, disallow degree-2 vertices (rather than just self-edges), since in 2D such vertices always correspond to degenerate triangles. So, in general, we should be careful about baking in too many assumptions about what the user wants/doesn't want.

A fourth possibility is to handle the most general case by default, and use helper functions to restrict the behavior. E.g.,

if( !flipMakesSelfEdge( e ) ) mesh->flip( e );

or

if( !flipMakesDegreeTwo( e ) ) mesh->flip( e );

These become verbose, but the intent of the code also becomes very clear. To reduce verbosity, the user can always define their own helper function

myFlip( mesh, e ) {
   if( ...do some checks here... ) {
      mesh->flip( e );
   }
}

[nmwsharp]

A related question which we should perhaps ponder at the same time:

What is right way to handle operations which cannot be performed? Right now most options follow the paradigm:

• If the function is called in a situation which could never make sense, and probably represents a logic error by the user, throw an exception. E.g. calling collapseEdgeTriangular() on a non-triangular mesh.
• If the function is called in a situation where it's not possible "right now" (e.g. flipping an edge on a degree-1 vertex), return false (or a sentinel element like Face()).

This mostly seems to work pretty well, but it might play in the discussion about options.

For example two other possibilities which we do not currently use are:

• Always throw exceptions for un-performable operations. (this seems bad because it forces the caller to guard/try-catch at many call sites)
• Return success via an output argument like void removeFace(Face f, bool& wasSuccessful); (this is alright, but we should probably either use output arguments everywhere or nowhere; we currently use them nowhere)

[GeometryCollective]

Regarding un-performable operations: we could always throw exceptions in debug mode (but not in release mode).

[mgsutton]

Regarding operations that cannot be performed, I recently bumped into this when I unwittingly called a mutation operation with a dead entity as input. The basic operation I was performing was as follows:

1. Collect up a bunch of edges that I need to collapse based on some criteria
2. Iterate over this list and collapse the individual edges.

What happened, though, is that while iterating over the list a previous collapse operation killed one of the edges still in my list and when I went to collapse it bad things ensued. Its a an easy fix in user code to put in the check and not try the operation if the entity is dead, but I started thinking that maybe the mutation routines should handle the isDead() case in some way.

The bigger question, though, is whether it should be an exception or just a sentinel type failure? I would vote for the latter so that use cases like mine don't require the extra handling, but that may be a minority view.

There is another option (no pun intended) in the std::optional type, which could wrap all the return types and just be null on any type of error. But you loose the explanatory reason stored in the exception. That type also mandates that the user either check it and/or convert it as needed, which might be onerous.