Only define geometry for `Surface`, `Curve`, and `Vertex`

[hannobraun]

Current Situation

At this point, geometry is defined for Surface and HalfEdge. But the geometry defined for HalfEdge doesn't actually belong there:

• path defines the geometry of the Curve that the HalfEdge is on.
• boundary defines the positions of the Vertex instances that bound the HalfEdge on that Curve.

Organizing it like this doesn't make much sense, but was necessary until recently. Geometry in Fornjot is always defined in the most local way possible. For example, vertices are defined in terms of 1-dimensional curve coordinates. Curves are defined in terms 2-dimensional surface coordinates. This is the most general way to define them, as it's always possible to go up to a higher dimension; but so far, we lack the tools to project from a higher dimension into a lower one. (This is something that https://github.com/hannobraun/fornjot/issues/2118 will help with.)

Vertices can exist on multiple curves and curves can exist on multiple surfaces. Defining their geometry locally means that it is defined redundantly. Storing these redundant definitions within Vertex and Curve would have significantly complicated both those objects themselves and the object graph as a whole.

HalfEdge is already local to a a specific curve and surface, and happens to reference both Curve and Vertex. Defining both of their local geometries there, was a convenient way to solve this problem.

New Possibilities

Now that geometry is no longer defined as part of the topological object graph (https://github.com/hannobraun/fornjot/issues/2116), the old restrictions no longer apply. It should not be a problem to tie the multiple redundant definitions directly to Vertex and Curve respectively.

Doing this should provide a number of benefits:

• It will put the geometric definitions into more logical places, meaning the whole system will be easier to explain and understand.
• Putting those multiple redundant definitions together will improve clarity. Yes, it will still be confusing, but the situation is already confusing right now, in addition to being obfuscated.
• It will provide a clearer path towards improving the situation by making geometry definitions no longer redundant.

Future Improvements

As I alluded to in the previous section, I believe there is a path to improving the situation by making geometry definitions no longer redundant. Once https://github.com/hannobraun/fornjot/issues/2118 is implemented, implementing functionality like projecting into a local coordinate system becomes practical without introducing a maintenance burden. Then using a global definition in a local context, or translating a local definition into another local context, becomes possible, allowing users to define geometry in whatever form is most convenient for their model.

Next Steps

I am going to start working on this right away, as I think its best to take care of this before https://github.com/hannobraun/fornjot/issues/2118. The improved clarity should benefit any work that comes after. And doing this cleanup afterwards would likely be significantly more work.

[hannobraun]

I've been working on this for the past few days. As I explained in the issue description, there are basically two parts to this issue:

1. Move the SurfacePath that defines curve geometry from HalfEdgeGeom to a new CurveGeom and integrate that into the geometry layer.
2. Move the CurveBoundary that defines the positions of vertices from HalfEdgeGeom to a new VertexGeom.

I've started with item 1, as those local definitions of the vertex position would need to reference the curve that the definition is local to, so it seemed to make more sense to first get everything in order over there. I have a local branch where I've started to do this, but I'm not fully convinced that I've found the right approach yet. What has come out of that so far, are mostly self-contained cleanups and refactorings (see list of pull requests above).

One thing that I hoped to gain from (which I've outlined in the issue description) is more clarity, by making the currently implicit redundant definitions more explicit, bringing the mess that we have to the forefront. I can say with certainty, that this has worked out well (at least in my local branch). Making a mess seems to be one of the things I'm particularly good at :grin:

One aspect of making the relationships we already have more explicit, is that the new CurveGeom references a surface for each of its local definitions. And this reference must even be optional, as the curve might be defined as part of a sketch, which is a pure 2D construct, and does not have a surface to locate it in 3D.

All of this is a bit of a mess, as I have to pass Option<Handle<Surface>> to a lot of places where something like that was previously not required. Hopefully that mess is just a temporary artifact of the ongoing transition. Once the new structure is in place, it will hopefully be possible to refactor the affected APIs in a way that makes more sense with the changed circumstances.

But either way, it got me thinking: Can the handling of surfaces be simplified, by having a surface that, unlike most surfaces, doesn't represent a surface in 3D space, but represents the whole 2D space? This is possible now. With the geometry layer, assigning geometry to a surface has become optional, and we could just define that special surface and not assign any geometry to it.

If we organized it like that, then we would no longer need a special "no surface" case in code or data structures, which sounds like a nice simplification. I'll think about that over the coming long weekend. If I decide it's a good idea, I'll probably do that first, before continuing here, as it makes what I'm working on here less complicated.

[IamTheCarl]

A feature I am considering asking for in the future (but am holding off because I don't need it yet, you seem busy, and I'm not even sure if it's possible) is to take slices of solids, as in cut the thing in half with a plane and get a 2D shape of where the solid intersects that plane. This would be extremely useful for GCode generation and would have big advantages over current slicer tools since curves would be represented in the 2D slice with curves, something slicer programs for 3D printers are currently incapable of (they approximate with poly-lines). It would also be useful for CNC milling complicated features.

Anyway, I bring that up because I think a purely 2D space would be good for representing the output of such a function.

[hannobraun]

That definitely sounds possible (and reasonable), but it's another one of those things that need a universal intermediate representation (https://github.com/hannobraun/fornjot/issues/2118), before it becomes practical. Note that the current plan for that is to use polylines as the intermediate representation. But as the issue notes, the original curves are still around, and you can use them to get more resolution whenever you need it.

[IamTheCarl]

I gave it a read and I'm not sure if I should put my feedback there or here.

The approach seems reasonable and I like your "planning only gets you so far" approach. I've seen people plan for months and then discover the idea won't work a weeks into the actual project. Prototyping is a really important process, and I consider Fornjot to be in it's prototyping era.

Having a tag of where the polyline came from seems like a reasonable compromise. Knowing where it came from has some additional benefits for me. I want to be able to tag walls with properties to influence gcode generation with things like "when it comes to tolerance, bias toward the inner or outer side of this surface". So many times I found myself wishing I could communicate that to my slicer.

[IamTheCarl]

I'm creating plans to add gcode generation to Command CAD soon, for 2D objects only. I can generate that right from the sketches. Since sketches currently are very limited, I'm thinking about making a temporary solution where I store curves myself and convert them to polylines for Fornjot, similar to what FJ-Text currently does.

How well do you think that will bridge into your eventual long term plan?

[hannobraun]

Thank you for the feedback, @IamTheCarl!

I'm creating plans to add gcode generation to Command CAD soon, for 2D objects only. I can generate that right from the sketches. Since sketches currently are very limited, I'm thinking about making a temporary solution where I store curves myself and convert them to polylines for Fornjot, similar to what FJ-Text currently does.

How well do you think that will bridge into your eventual long term plan?

Whatever ends up happening with curves within Fornjot, I expect that generating your own curves as multi-half-edge polylines will remain a viable approach going forward. So once more powerful curves become available, you can decide to port your code to those, or keep doing what you're doing.

So I seen no reason why you shouldn't move ahead with your approach. Once my geometry work progresses to the point where powerful curves and polylines as intermediate representation are available, we'll see if that suit your needs. And if it doesn't, what needs to improve.

Me making progress with the design, and you making progress with a use case for that design, is probably the best way to eventually land on something that works well.

[hannobraun]

It's been a while since I posted an update here. I've been on vacation in the meantime, but now I'm back, and as you can see from the list of pull requests above this comment, getting back into it!

I've implemented the surface representing 2D space that I talked about in my previous update. So far, that has worked out well. I've also managed to land the first pull requests to define the curve geometry. Making sure all places that need to define curve geometry do so, and all places that use curve geometry read it from the right place, is an incremental process that will still take some time to finish.

Once that has happened, I should be able to remove the SurfacePath from HalfEdgeGeom, finishing that part of this issue. After that, I can focus on migrating the rest of HalfEdgeGeom into a new vertex geometry definition.

[hannobraun]

Work here is progressing, as you can see from the long list of pull requests above this comment. I'm still working on making sure that curve geometry is defined everywhere it needs to be. It's a slow and careful process. Every time I make a local change that uses curve geometry, I find more places that need to be updated.

I guess I don't have much to say, except that this process is still ongoing :smile:

[hannobraun]

I've been tracking down various bugs that were exposed as I was trying to use the new curve geometry definitions in more places. (The pull requests referenced above this comment are the result of that work.) The most recent bug turned out to be a rather fundamental problem though.

Consider the side wall of a cylinder, which is a face:

• If you look at this face in 2D, it's a rectangle, bounded on all four sides by four different half-edges. Each of those half-edges has a different position in the 2D space, so once their geometry is defined by a curve (the goal of this issue), they all need to reference different curves.
• But if you look at the face in 3D, it curves, and two of the half-edges touch. Meaning they are congruent in 3D space, hence they need to reference the same curve under the validity rules of Fornjot.

This is a contradiction. (And by the way, the same is true for vertices. I've only hit this issue with curves so far, but now that I understand it, it's obvious that the same will be true for vertices.)

So, how to solve this? I see the following options:

1. Just not implement this issue. I don't this is would be a good outcome, as then there's no prospect of fixing the redundant geometry definitions of half-edges, which would be a great simplification, thereby hampering all future development with undesired complexity.
2. Change the validity rules of Fornjot. Also not a good outcome, as those validity rules are important for handling shapes robustly, and the way this is done is one of the central premises of Fornjot's architecture. Doing this would require a whole new approach to reliably generating triangle meshes, for example.
3. Forbid faces and half-edges from touches themselves.

I'd like to think some more about this, to see if I can come up with another solution. But right now I see 3. as the best option. And I actually think it would be relatively easy to implement. All it would require is to generate circles as two half-edges instead of one, and add validation checks to make sure this requirement is upheld.

As far as I know, this decision also has precedent in other CAD system, some of which restrict half-edges and faces to only span 180° (as opposed to the full 360° that Fornjot allows), and that's probably to circumvent issues like this one.

Again, I'd like to think about this some more, but it's probably fine to have this restriction. And even if it's not desirable long-term, it's probably better to introduce it right now, and see if it can be lifted later.

[hannobraun]

Work here has been paused for a bit (I switched to https://github.com/hannobraun/fornjot/issues/2157 in the meantime), but I've picked it back up (see pull requests referenced above this comment).

I decided to implement the solution I talked about in my previous comment here (see https://github.com/hannobraun/fornjot/pull/2375 and https://github.com/hannobraun/fornjot/issues/2374), to solve the problem of distinct half-edges mapping to the same positions in 3D.

I hope I can wrap up the rest of the work here quickly, so I can finally move on to https://github.com/hannobraun/fornjot/issues/2118, but we'll see how that goes :joy:

[hannobraun]

As of https://github.com/hannobraun/fornjot/pull/2391, all curve geometry has been removed from HalfEdgeGeom, meaning curve geometry is not defined exclusively on Curve. This change was literally months in the making!

This is an important step forward, for the reasons explained in the issue description above. Most importantly, there's now a clearer path towards making curve geometry no longer redundant (which will also require https://github.com/hannobraun/fornjot/issues/2118). On the negative side though, this has made a lot of code more messy; although I'd argue that it has only brought more of the preexisting conceptual mess to the surface.

This is a transitionary phase. Things have gotten a bit worse (and will get worse still, before this issue is finished), on the path to ultimately making things much better.

---

Speaking of that. As of now, all surface geometry is defined on Surface objects, all curve geometry is defined on Curve objects, and only vertex geometry is still defined on HalfEdge. Addressing that last item is what remains to be done, before I can close this issue.

And this is what I'll start working on now. I'm not sure how difficult this is going to be. I hope less difficult then what I've done here so far, but how knows. I'm sure there are many more problems waiting to be uncovered.

[hannobraun]

And I'm finally done! The last bit, migrating the remaining half-edge geometry to vertices, didn't hold a lot of surprises. Just the usual: Migrating a little bit, hitting an issue, tediously debugging that issue, fixing it, continue.

This means the objective of this issue, defining geometry only in terms of surfaces, curves, and vertices, has been achieved. What this has also done, as I have alluded to multiple times, is make a lot of code messier. While I was a bit surprised by the degree to which that has happened, I'm not surprised that it has happened. What this has done, is put the redundancy that was previously neatly hidden in the object graph into the foreground, where it's visible and can be dealt with more easily. It's fine for now.

Most importantly, this paves the way for https://github.com/hannobraun/fornjot/issues/2118, which I'm finally ready to start working on now. If this works out, it will become practical to build better tools around converting geometry between different spaces, which will make it possible to just remove the redundancy, have a single canonical geometry definition for each surface/curve/vertex. This should get us out of this weird transitional phase and ideally make everything related to geometry neat and tidy.

At least that's the plan. And now I can finally start working on that!