Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate geometry from topology #2116

Closed
hannobraun opened this issue Nov 29, 2023 · 8 comments · Fixed by #2288
Closed

Separate geometry from topology #2116

hannobraun opened this issue Nov 29, 2023 · 8 comments · Fixed by #2288
Assignees
Labels
topic: core Issues relating to core geometry, operations, algorithms type: development Work to ease development or maintenance, without direct effect on features or bugs

Comments

@hannobraun
Copy link
Owner

As far as I know, b-rep CAD kernels traditionally separate topology (how objects relate to each other) from geometry (where objects are). At least that's what I got from Boundary Representation Modelling Techniques by Ian Stroud. Earlier on, Fornjot followed that separation, but that didn't survive the many improvements and refactorings since then.

I think it's time to bring that idea back, but with a different twist: Originally the topological data referred to the geometry (and that's how it sill works, kinda, but without a clear separation). I'd like to do it the other way around: Geometrical data exists as a separate layer that refers to the topological object graph, which itself is isolated and doesn't concern itself with anything else. This is possible, because the (topological) objects in the graph have unique IDs, and thus can be referenced easily by an outside entity.

I see the following advantages in this arrangement:

  • Simplified object graph: I'd expect the object graph to survive this transition mostly unchanged, but some objects will lose some of their attributes, simplifying the graph overall.
  • Easier to update geometry: Topological objects are immutable and stored in a centralized data structure. Updating an object means creating a new version of it (as well as new versions of all objects that reference it), and storing those new versions.
    This is not always appropriate. For example, if you have a user drag a vertex in a GUI, you'd have to create new versions of those objects every pixel.
  • Easier to evolve geometry: The geometric primitives in Fornjot are currently very basic. Evolving that in the current structure would mean evolving the object graph with it, creating friction in the development process.
    Having geometry be separate, would make it easier to experiment with geometry, as changes there would no longer affect the object graph. It would even be possible to have multiple competing geometry layers, and experiment with them in parallel.

Initially, the geometry layer could be a struct (Geometry), that is passed into all operations (and other code) that need it. Eventually, we should find a better way to organize this, as to not have a million different arguments in every function. I already have some ideas here, but that's a topic for another day (I have a long list of issues I need to open, so it might take me a bit to get to that one).

Unless I'm missing something (which is quite possible), this should be a relatively straight-forward refactoring. We should definitely attempt to do this before making any other significant changes to the geometry representation or object graph.

@hannobraun
Copy link
Owner Author

#2117, which I wanted to do as a test run for this issue, is completed now. I will now start working on this one.

@hannobraun
Copy link
Owner Author

As I'm starting to get into this, I've decided that I'm going to migrate surface geometry to the new geometry layer first. This has already turned up two issues that need handling:

  • Now, you create a surface and its geometry without needing to store it or needing access to Core. Soon, you'll create a surface without geometry, but need to store and access Core to define its geometry. This is going to require changes to BuildSurface and some of its use sites.
  • Transforming objects needs to change. It can no longer work on bare objects, as those won't have any geometry soon. Also, it will be possible to transform objects without creating new instances of them.

Not sure which issue I'll handle first. Both require some thinking.

@hannobraun
Copy link
Owner Author

I have figured out some minimally invasive solutions for addressing my problems with BuildSurface and TransformObject for surfaces. You can check out the list of pull requests above for the full details.

In each case, this leaves the code in a state that isn't ideal for the new world of separate topology and geometry. This needs to be cleaned up later, but for now, it's not important. Doing it like this allows me to make progress with the geometry layer without requiring me to do everything at once.

As of the latest pull requests, the geometry layer is in place, and surface geometry is stored there. The surface geometry still redundantly exists in the Surface object, and all code dealing with surface geometry reads from there. Changing this is what I'm working on next.

@hannobraun
Copy link
Owner Author

This work has been on pause for a bit, while I was distracted with other projects. But I've resumed it today, and manged to finish the migration of surface geometry to the geometry layer.

Next up, I'll set my sights on HalfEdge, which contains two more pieces of geometry: The definition of the surface path and the curve boundary.

@hannobraun
Copy link
Owner Author

I just discovered a bug caused by the work here: #2266

I'm hoping I can ignore it for now and move forward.

@hannobraun
Copy link
Owner Author

I'm currently working on migrating the definition of the surface path out of HalfEdge and into the geometry layer. I'm almost done.

In my opinion, every piece of code that defines the surface path writes to the geometry layer, and every piece of code that needs the surface path reads it from the geometry layer. Except, when I actually remove the respective field from HalfEdge, which I don't think is still used anywhere, I'm getting validation failures in the example models.

So far, I don't understand at all why that is, despite a few hours of debugging. I'll stay on it. This makes so little sense, it must be some stupid oversight.

@hannobraun
Copy link
Owner Author

So, I thought I had run out of steam today, but apparently not. I found the bug. I said it must be stupid, and it certainly was. Good thing is, it made me realize some things that will make handling objects simpler and more robust.

Other good thing is, I can move on now to migrating half-edge boundaries to the geometry layer. This is the last piece that's missing before this issue is completed!

@hannobraun
Copy link
Owner Author

And it's done! All geometry lives in the respective layer, and the object graph is now a purely topographical data structure!

Next, I intend to build on that, by cleaning up the geometry a bit. I have some specific ideas here, that I haven't yet opened an issue for. After that, I'll hopefully be ready to start working on #2118.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: core Issues relating to core geometry, operations, algorithms type: development Work to ease development or maintenance, without direct effect on features or bugs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant