Store only one array of vertex positions in VertexPositionGeometry

[keenancrane]

Currently VertexPositionGeometry contains both vertexPositions and inputVertexPositions, which arose from a well-intentioned design but turns out to be pretty annoying/confusing in most common use cases.

Proposal: Have just one copy of the vertex positions, vertexPositions.

Probably there are quite a few consequences to work through here... but it feels like the right direction moving forward.

[MarkGillespie]

I think that my ideal version of quantities would expose const versions of derived quantities, and only allow mutation of the special input quantity. e.g.

class VertexPositionGeometry {
public:
    const CornerData<double>& cornerAngles;
    const EdgeData<double>& edgeLengths;
    ...
    VertexData<Vector3> vertexPositions;
    
private:
    CornerData<double> privateCornerAngles;
    EdgeData<double> privateEdgeLengths;
    ...
}

class EdgeLengthGeometry {
public:
    const CornerData<double>& cornerAngles;
    const HalfedgeData<double>& halfedgeCotanWeights;
    ...
    EdgeData<double> edgeLengths;

private:
    CornerData<double> privateCornerAngles;
    HalfedgeData<double> privateHalfedgeCotanWeights;
    ...
}

That way the compiler would yell at me if I tried to change quantities that really should be computed from other things.

Then, my ideal superclasses (e.g. IntrinsicGeometryInterface) would just have everything immutable, since it's unclear how the user could change the geometry at this level of abstraction.

I have no idea how this could be implemented in C++. It sounds like a nightmare. But I figured I'd chime in with my dream system anyway.

[nmwsharp]

I totally agree @MarkGillespie!

I tried to set up the const containers once, but got tripped up in the type system and gave up. It really feels it should be doable though, so perhaps we/I just need to try again.

I think design-wise there is still a small question to answer in this case about how to handle the vertex position field for EmbeddedGeometryInterface, which could hypothetically be a derived quantity in some realizations. Either we give it the same name as vertexPositions, then get in to issues of shadowing the parent member, or we give it a different name and force users to access it via nonstandard name---neither are perfect solutions. (this is getting a bit in the weeds, probably needs a class diagram to think through).

Another alternative is to just go ahead and do some weird special-case thing for vertex positions, since they're so insanely common...

[MarkGillespie]

I have a few const containers working in some other code - I just initialize the const references to point to the private data in my constructor and it seems to work fine

HyperbolicTriangulation::HyperbolicTriangulation()
    : logScaleFactors(privateLogScaleFactors) {}

But I don't know if this interacts weirdly with any of the fancy quantity-management systems that you've set up

[nmwsharp]

I think the problem I was having was that even when the member was const, the [] operator was not giving const references when it was supposed to. This is a fuzzy memory though, so perhaps it's a non-issue and your solution is all we need :)

[nmwsharp]

FYI, see discussion in https://github.com/nmwsharp/geometry-central/issues/88 and commits 13208f0 and 1af6a29 for changes specifically with respect to inputVertexPositions.

These alias the VertexPositionGeometry::inputVertexPositions to point to the same buffer as the inherited EmbeddedGeometryInterface::vertexPositions, which will hopefully resolve confusion about having two distinct-but similar members. Moving forward, we can generally write code to just used the vertexPositions buffer. This change is more of a stopgap; it don't address the larger architectural questions above, but it also (hopefully) isn't a breaking change :)