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

Rename fj::Union to fj::Group #219

Closed
4 tasks done
therealprof opened this issue Feb 20, 2022 · 7 comments
Closed
4 tasks done

Rename fj::Union to fj::Group #219

therealprof opened this issue Feb 20, 2022 · 7 comments
Assignees
Labels
good first issue Good for newcomers type: feature New features and improvements to existing features

Comments

@therealprof
Copy link
Contributor

therealprof commented Feb 20, 2022

I noticed that there's an example csg-union-disjoint which (as implied by the name) creates a union of two disjoint objects. In my mental model a union in CAD CSG implies that the union members are overlapping or at least touching to form one combined solid body. "Disjoint objects" sound more like a group to me.


Edit (@hannobraun):

Adding the list of action items that I originally included in the comment below. That way, they should show up in the issue list view.

@hannobraun
Copy link
Owner

As a general note, none of the nomenclature used right now is meant to be the last word. It all just reflects a current state, that is meant to be improved upon iteratively.

As for the specific question: I haven't much thought about this before I read your question just now. This definition of union matches the one used in OpenSCAD, hence probably why it seemed natural to me.

From a technical perspective, implementing the union operation involves checking for intersections between the different bodies, and adding/removing vertices/edges/faces along these intersections as required. A union of disjoint objects is just one case that this algorithm handles naturally: No intersections detected, no removals/additions required. (I've added two notes with additional context below.)

I see no reason to call disjoint unions something different, when it's really the same thing under the hood. Does that make sense to you, given my explanation?

In any case, it wouldn't hurt to mention this in the documentation of fj::Union. I'm going to edit the issue title accordingly.


Additional notes on the implementation of disjoint unions:

  • The proper union operation as I described it is not implemented right now (see Implement union operation #42). I'm slowly working towards implementing that (and other CSG operations) right now.
  • The description above relates to boundary representation, the approach used by Fornjot. But it's the same for other approaches. If you use a representation based on implicit functions, disjoint unions also naturally fall out of the union algorithm; no extra handling required. I imagin (but don't know for sure) that the same goes for the mesh-based approach used by OpenSCAD.

@hannobraun hannobraun added topic: documentation Improvements or additions to documentation topic: model type: feature New features and improvements to existing features labels Feb 20, 2022
@hannobraun hannobraun changed the title Definition of a "union"? Explain disjoint unions in fj::Union documentation Feb 20, 2022
@therealprof
Copy link
Contributor Author

You explanation makes sense but I'm still wondering on whether that will not prevent the addition of proper model checking since there's not way to figure out whether the disjoint bodies happen to not be touching (or overlapping) by accident or on purpose.

@hannobraun
Copy link
Owner

Hmm, that's a good point. It probably makes sense to have a split between the implementation side, and the semantic meaning that reflects the user's intent.

So a solution could be to have fj::Group and fj::Union as separate concepts. When they are processed by the kernel, it checks whether the different objects overlap. If you have an overlapping group or a disjoint union, that check fails. If the check doesn't fail, it's just handled by the same union algorithm from then on.

Is that along the lines of what you had in mind?

@therealprof
Copy link
Contributor Author

Yes, that's exactly what I was thinking of.

@hannobraun
Copy link
Owner

hannobraun commented Feb 20, 2022

Okay, let's do that then 😄

So, the list of action items for this issue would be something like this:
Moved list of action items to issue description above.

Labeling https://github.com/hannobraun/Fornjot/labels/good%20first%20issue, as we've already determined what needs to be done, and doing it doesn't require any deep insight into Fornjot (but can serve as an introduction to the fj library and the parts of the kernel that interface with it).

@hannobraun hannobraun added good first issue Good for newcomers and removed topic: documentation Improvements or additions to documentation labels Feb 20, 2022
@hannobraun hannobraun changed the title Explain disjoint unions in fj::Union documentation Rename fj::Union to fj::Group Feb 20, 2022
@hannobraun
Copy link
Owner

I've started looking into this.

@hannobraun
Copy link
Owner

#366 has been merged, #367 has been opened, #42 has been updated. This is done. Closing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers type: feature New features and improvements to existing features
Projects
None yet
Development

No branches or pull requests

2 participants