Increment tag to support multiple similar CSGs.

[ikeough]

BACKGROUND:
See #965.

DESCRIPTION:
This PR adds an additional field to the tuple vertex tag which makes vertices unique.

REQUIRED:

• All changes are up to date in CHANGELOG.md. n/a

---

This change is 

[andrewheumann]

It took me a really long time to figure out how this was working — I had to read the code 4-5 times before it clicked. I think some comments would be useful, especially at moments related to this optimization / the new offset.

Let me make sure I understand:

• in ProcessSolidsAsCSG, we were creating a collection of SolidTesselationTargetProviders, one for each solid operation.
• Each SolidTesselationTargetProvider has a GetTessellationTargets method which returns a new SolidFaceTessAdapter.
• each SolidFaceTessAdapter is responsible for the tesselation of a single face.
• the SolidFaceTessAdapter's GetTess method calls face.ToContourVertexArray.
• ToContourVertexArray attaches a faceId to the Data object we hang on the contour vertex.
• most accessors of Data don't care about this faceId at all, and discard it. But PackTessellationsIntoBuffers constructs a map of vertex indices by tag / faceId.
• This is the code that was failing: because faceIds could be the same across solids, we would sometimes accidentally re-use the wrong vertex.
• We solve this in this PR by attempting to guarantee unique faceIds between solids.

So I think all that makes sense now, but I think this whole flow could stand some better commenting.

I also worry about the fragility of modifying the faceId on the contour vertex with an offset — this assumes faceIDs are necessarily sequential/consecutive within a given solid, and that offsetting by the face count guarantees uniqueness. I wonder if it wouldn't be better to be explicit about a solidIndex/solidOperationId or something, as an optional second piece of information in the vertex map's tuple key.

It also bothers me that we're calling something faceId which now is no longer the actual face.Id of the face it came from — If we want to use the offset/sum strategy, we should at least call it something different so future us doesn't assume it will be the faceId.

Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @ikeough)

---

Elements/src/Geometry/Tessellation/SolidTessellationTargetProvider.cs line 19 at r1 (raw file):

        /// </summary>
        /// <param name="solid"></param>
        /// <param name="transform"></param>

Please add xml comment for offset param, and descriptions

[ikeough]

Good points all around. I've used your breakdown of this flow above as the comments, and I've moved to using an increasing solidId as a separate field in the tuple and the vertex map. Now vertices are disambiguated using their vertex id (tag), their face id, and their solid id.

Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @andrewheumann)

---

Elements/src/Geometry/Tessellation/SolidTessellationTargetProvider.cs line 19 at r1 (raw file):

Previously, andrewheumann (Andrew Heumann) wrote…

Please add xml comment for offset param, and descriptions

Done

[andrewheumann]

Reviewed 3 of 6 files at r1, 5 of 5 files at r2, all commit messages.
Reviewable status: complete! 1 of 1 approvals obtained