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

Add cut_with_plane and new fast clip method #8721

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sloriot
Copy link
Member

@sloriot sloriot commented Jan 28, 2025

Replaces #6588

TODO

  • small feature page
  • rename new_clip() to clip()
  • add (new_)split()
  • add the new PMP::kernel() based on clip()
    • replace QP_solver for most interior point in 3D convex hull
  • add parallelism

@MaelRL MaelRL added Not yet approved The feature or pull-request has not yet been approved. CHANGES.md not updated labels Jan 28, 2025
}
};

// TODO here we should integrate the epsilon to reuse existing points (moving them on the grid as an option?)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually we might want to move input points in the plane and update the oriented side.

@MaelRL MaelRL added this to the 6.1-beta milestone Feb 20, 2025
* \cgalParamNBegin{geom_traits}
* \cgalParamDescription{an instance of a geometric traits class}
* \cgalParamType{a class model of `Kernel`}
* \cgalParamDefault{a \cgal Kernel deduced from the point type, using `CGAL::Kernel_traits`}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* \cgalParamDefault{a \cgal Kernel deduced from the point type, using `CGAL::Kernel_traits`}
* \cgalParamDefault{a \cgal kernel deduced from the point type, using `CGAL::Kernel_traits`}

* If `tm` is closed, the clipped part can be closed too if the named parameter `clip_volume` is set to `true`.
* See Subsection \ref coref_clip for more details.
*
* \note `Plane_3` must be from the same %Kernel as the point of the internal vertex point map of `PolygonMesh`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here as well I would suggest to lowercase the word "kernel" as it is just a normal noun.

* @tparam NamedParameters a sequence of \ref bgl_namedparameters "Named Parameters"
*
* @param tm input triangulated surface mesh
* @param plane plane whose negative side defines the half-space to intersect `tm` with.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @param plane plane whose negative side defines the half-space to intersect `tm` with.
* @param plane plane whose negative side defines the halfspace to intersect `tm` with.

* \cgalParamType{Boolean}
* \cgalParamDefault{`false`}
* \cgalParamExtra{If this option is set to `true`, `tm` is no longer required to be without self-intersection.
* Setting this option to `true` will automatically set `throw_on_self_intersection` to `false`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will it override in case I use the wrong value for throw_on_self_intersection ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's what the last part of the sentence is saying. set -> reset would be better?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like "overwrites" or "has precedence over a contradicting named parameter"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CHANGES.md not updated Not yet approved The feature or pull-request has not yet been approved. Pkg::PMP Small feature TODO
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants