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

Consider removing region interiors #2132

Open
hannobraun opened this issue Dec 11, 2023 · 1 comment
Open

Consider removing region interiors #2132

hannobraun opened this issue Dec 11, 2023 · 1 comment
Labels
topic: core Issues relating to core geometry, operations, algorithms type: planning Issues about project planning

Comments

@hannobraun
Copy link
Owner

Current situation

Region is an object that is referenced by Sketch and Face. It basically encapsulates their commonalities, sharing code between the 2D sketch and 3D face.

A region is bounded by one exterior Cycle (in other words, its outside boundary), and an arbitrary number of interior cycles (holes in the region). This has worked fine so far, but it presents some complexity. All code that deals with regions, needs to handle both exterior and interior cycles. This would not be necessary, if they could be unified somehow.

The idea

Let's consider this region, which has one exterior and one interior cycle:

region-with-exterior-and-interior-cycle

We can actually connect the two cycles, and then there would only be one cycle that represents the whole boundary:

region-with-only-exterior-cycle

Please note that the gap I've left here is purely illustrative, to show that there would be one half-edge going inside, and another going outside again. Both half-edges along the gap would be coincident, so both regions would be congruent.

The only difference is in how the region boundary is represented: Instead of of doing it using two cycles, one for the inside and one for the outside, we just have the one cycle. The face just happens to touch itself along two of its bounding half-edges.

Advantages

This would simplify Region, maybe to the point where it's no longer useful and can be removed. Hence, this could simplify the whole object graph. As a result, lots of code that deals with sketches/faces/regions will become simpler.

If you search the code for .interiors(), every piece of code that you find (currently 16 results in 11 files) will no longer need to exist. And there are examples of new code, which would end up simpler. For example, we need a validation check that makes sure cycles are not self-intersection. Under the proposed change, this check would also be sufficient to make sure every face boundary is valid. Without this change, we'd need an extra check that makes sure the multiple cycles that make up the face boundary don't intersect with each other.

Possible disadvantages

There are some disadvantages that I can think of:

  1. Some half-edge validation code will become more complicated. Right now, each half-edge in a valid shell has exactly one sibling (where a face touches its neighboring face). If a face can touch itself, then a neighbor could connect to it in the same place, and then we'd have 4 coincident half-edges there. This is probably not the end of the world, and it might be enough to change the rule to "each half-edge needs exactly one sibling that bounds a different face".
  2. We might need special handling for sweeping these "self-touching" parts of the face boundary. It's actually unclear if that's a real disadvantage, as the same code would also benefit from the simplification, so we might end up with roughly the same level of complexity in this specific place.
  3. We could also decide that sweeping faces does not need special handling, but then we would end up with shells that touch themselves. This might also not be bad, as it would re-use infrastructure that we might need anyways. It would also open a path towards handling cavities within solids in the same way: not as a separate shell, but as part of the one shell that forms the solid's boundary. Then Shell would become redundant, leading to further simplification.
  4. Right now, it's really easy to make a hole in a face (just add an interior cycle). With the proposed change, this would become more complicated, as the existing boundary would have to be modified, and a suitable connection between its interior and exterior parts would need to be found. This might be non-trivial, if the face already has a bunch of holes, and the path between the exterior and the new one is not just a straight path. However, we're also getting away a bit too easy right now. If we want to support more complex cases than we do right now (like sketching on a face), then adding interiors is not enough anyway, and we need more complicated logic to figure out where an interior needs to be added, and where whole faces need to be bisected.

As you can see in that list, there are a lot of qualifiers along the lines of "might not be that bad" or "might not even be a disadvantage". The only additional complexity that I'm 100% sure we're going to need is the "find a (possibly non-trivial) connection between exterior and interior when adding a hole". But that would be one focused piece of code, that would live in one module, covered by one test suite. Its existence might easily be outweighed by a simplification to the object graph, which would have positive impact all over the code base.

Next steps

I've opened this as a planning issue, as I'm not sure yet if this is worth doing (although the case I laid out here is pretty strong, I think). I will keep thinking about this, and leave any additional insights here. Everyone else who might have some insight is also invited to share that!

@hannobraun hannobraun added topic: core Issues relating to core geometry, operations, algorithms type: planning Issues about project planning labels Dec 11, 2023
@hannobraun
Copy link
Owner Author

Just out of interest, I tracked down the pull request that introduced the distinction between exterior and interior cycles: #401

It's been a while!

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: planning Issues about project planning
Projects
None yet
Development

No branches or pull requests

1 participant