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 --render=cgal to --render=force to force-convert to the current backend-specific geometry #4822

Merged
merged 5 commits into from Feb 28, 2024

Conversation

kintel
Copy link
Member

@kintel kintel commented Nov 12, 2023

--render=cgal was really meant for converting to the internal format of the current geometry engine to utilize any of its built-in mesh validation and repair tools. When using another geometry backend, we're changing this to --render=force to force conversion into the current backend-specific geometry.

--render=cgal is still useable for backwards compatibility, but not sure for how much longer we need to support it.

Also added some opportunistic refactoring.

@jordanbrown0
Copy link
Contributor

jordanbrown0 commented Nov 12, 2023

Yes, you should rename it. "--render=cgal" should use CGAL, or be an error if CGAL is not available. Otherwise it's confusing.

@kintel
Copy link
Member Author

kintel commented Nov 12, 2023

--render=cgal is also slightly confusing as both the regular backend and fast-csg are CGAL.

I believe the initial purpose of this flag was:

  • --render: Force concrete geometry to be rendered. If the .scad file only contains concrete geometry, e.g. only consists of a primitive, an import(), a basic linear extrude etc., it will use that geometry as is, i.e. a PolySet, otherwise, the boolean engine will be invoked and produce whatever geometry that backend provides (Nef3 polyhedrons for the release build, but could be ManifoldGeometry).
  • --render=cgal: If the resulting geometry from evaluating the file was a PolySet, convert it to CGAL Nef3 polyhedrons, then back to PolySet. This ensures that the geometry passes all checks for valid polyhedrons in CGAL, or allows issues to be triggered instantly.

Ideally, we should rename --render=cgal to something more generic which doesn't involve naming the backend.

If we do ship with multiple backends (either for a release build or as experimental), having a flag to select backend would make sense, but that could be separate from --render=.

@kintel kintel marked this pull request as draft November 22, 2023 05:18
@kintel kintel changed the title Convert to ManifoldGeometry rather than Nef polyhedrons on --render=cgal Rename --render=cgal to --render=force to force-convert to the current backend-specific geometry Feb 25, 2024
@kintel kintel marked this pull request as ready for review February 25, 2024 00:38
@kintel
Copy link
Member Author

kintel commented Feb 25, 2024

Update: Renamed --render=cgal to --render=force

@kintel kintel mentioned this pull request Feb 27, 2024
40 tasks
@kintel kintel merged commit 8bff31d into master Feb 28, 2024
5 of 6 checks passed
@kintel kintel deleted the kintel-prefer-manifold branch February 28, 2024 03:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants