Skip to content

New package: Feature graph#9465

Draft
ange-clement wants to merge 31 commits into
CGAL:mainfrom
ange-clement:Feature_graph-aclement
Draft

New package: Feature graph#9465
ange-clement wants to merge 31 commits into
CGAL:mainfrom
ange-clement:Feature_graph-aclement

Conversation

@ange-clement

@ange-clement ange-clement commented May 6, 2026

Copy link
Copy Markdown
Member

Summary of Changes

New package "Feature graph" that contains methods to extract sharp features from 3D images.

Tasks

  • Validate API
  • Integrate code
  • Test
  • Manual and examples

Release Management

  • Affected package(s):
  • Issue(s) solved (if any):
  • Feature/Small Feature (if any):
  • Link to compiled documentation (obligatory for small feature)
  • License and copyright ownership:

Feature graph is now an VertexAndEdgeListGraph
Lowercase in passive form
in --> on
regularization isthmus distance parameter
Element type is now CGAL::Dimension_tag

@albert-github albert-github left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should be /// @{ instead of // @{

occurs in multiple files.

(I didn't really review it but this caught my eye)

@ange-clement

Copy link
Copy Markdown
Member Author
Feature_graph

@lrineau

lrineau commented May 7, 2026

Copy link
Copy Markdown
Member

/build:doc-wip

@lrineau lrineau added Not yet approved The feature or pull-request has not yet been approved. Work in progress labels May 7, 2026
@lrineau lrineau changed the title Feature graph: Package creation New package: Feature graph May 7, 2026
It will be placed back once the API is validated
@github-actions

github-actions Bot commented May 7, 2026

Copy link
Copy Markdown

There was an error while building the doc:

/home/runner/work/cgal/cgal/Feature_graph/doc/Feature_graph/PackageDescription.txt:42: warning: unable to resolve reference to 'PkgDEPENDENCY' for \ref command
/home/runner/work/cgal/cgal/Feature_graph/doc/Feature_graph/CGAL/detect_sharp_features.h:26: warning: unable to resolve reference to 'bgl_namedparameters' for \ref command
/home/runner/work/cgal/cgal/Feature_graph/doc/Feature_graph/CGAL/detect_sharp_features.h:30: warning: unable to resolve reference to 'bgl_namedparameters' for \ref command
/home/runner/work/cgal/cgal/Feature_graph/doc/Feature_graph/CGAL/detect_sharp_features.h:91: warning: unable to resolve reference to 'bgl_namedparameters' for \ref command
/home/runner/work/cgal/cgal/Feature_graph/doc/Feature_graph/CGAL/optimization_parameters.h:171: warning: unable to resolve reference to 'bgl_namedparameters' for \ref command
/home/runner/work/cgal/cgal/Feature_graph/doc/Feature_graph/CGAL/optimization_parameters.h:173: warning: unable to resolve reference to 'bgl_namedparameters' for \ref command
/home/runner/work/cgal/cgal/Feature_graph/doc/Feature_graph/CGAL/optimization_parameters.h:171: warning: argument 'plane_detection_distance' of command @param is not found in the argument list of CGAL::Optimization_parameters_on_image< NormalEstimator >::Optimization_parameters_on_image(const NamedParameters &np=parameters::default_values())
/home/runner/work/cgal/cgal/Feature_graph/doc/Feature_graph/CGAL/optimization_parameters.h:171: warning: argument 'normal_estimator' of command @param is not found in the argument list of CGAL::Optimization_parameters_on_image< NormalEstimator >::Optimization_parameters_on_image(const NamedParameters &np=parameters::default_values())
/home/runner/work/cgal/cgal/Feature_graph/doc/Feature_graph/CGAL/optimization_parameters.h:274: warning: unable to resolve reference to 'bgl_namedparameters' for \ref command
/home/runner/work/cgal/cgal/Feature_graph/doc/Feature_graph/CGAL/optimization_parameters.h:276: warning: unable to resolve reference to 'bgl_namedparameters' for \ref command
/home/runner/work/cgal/cgal/Feature_graph/doc/Feature_graph/CGAL/optimization_parameters.h:274: warning: argument 'plane_detection_distance' of command @param is not found in the argument list of CGAL::Optimization_parameters_on_surface< NormalEstimator >::Optimization_parameters_on_surface(const NamedParameters &np=parameters::default_values())
/home/runner/work/cgal/cgal/Feature_graph/doc/Feature_graph/CGAL/optimization_parameters.h:274: warning: argument 'normal_estimator' of command @param is not found in the argument list of CGAL::Optimization_parameters_on_surface< NormalEstimator >::Optimization_parameters_on_surface(const NamedParameters &np=parameters::default_values())
/home/runner/work/cgal/cgal/Feature_graph/doc/Feature_graph/CGAL/regularization_parameters.h:102: warning: unable to resolve reference to 'bgl_namedparameters' for \ref command
/home/runner/work/cgal/cgal/Feature_graph/doc/Feature_graph/CGAL/regularization_parameters.h:104: warning: unable to resolve reference to 'bgl_namedparameters' for \ref command
/home/runner/work/cgal/cgal/Feature_graph/doc/Feature_graph/CGAL/regularization_parameters.h:212: warning: unable to resolve reference to 'bgl_namedparameters' for \ref command
/home/runner/work/cgal/cgal/Feature_graph/doc/Feature_graph/CGAL/regularization_parameters.h:214: warning: unable to resolve reference to 'bgl_namedparameters' for \ref command
/home/runner/work/cgal/cgal/Feature_graph/doc/Feature_graph/Feature_graph.txt:22: warning: unable to resolve reference to '\n' for \ref command
/home/runner/work/cgal/cgal/Feature_graph/doc/Feature_graph/Feature_graph.txt:22: warning: unexpected token TK_COMMAND_BS as the argument of include
/home/runner/work/cgal/cgal/Feature_graph/doc/Feature_graph/Feature_graph.txt:24: warning: image file bench.png is not found in IMAGE_PATH: assuming external image.
/home/runner/work/cgal/cgal/Feature_graph/doc/Feature_graph/Feature_graph.txt:24: warning: image file bench.png is not found in IMAGE_PATH: assuming external image.
Error: Figure numbering; fig__figPck has not been collected

https://github.com/CGAL/cgal/actions/runs/25483628171

@ange-clement

Copy link
Copy Markdown
Member Author

/build:doc-wip

@github-actions

github-actions Bot commented May 7, 2026

Copy link
Copy Markdown

The documentation is built. It will be available, after a few minutes, here: https://cgal.github.io/9465/doc/Manual/index.html

@ange-clement

Copy link
Copy Markdown
Member Author

/build:doc-wip

@github-actions

github-actions Bot commented May 7, 2026

Copy link
Copy Markdown

The documentation is built. It will be available, after a few minutes, here: https://cgal.github.io/9465/doc/Manual/index.html

@MaelRL MaelRL added the Feature label May 18, 2026
@MaelRL MaelRL modified the milestones: 6.2-beta, 6.3-beta May 18, 2026
@lrineau

lrineau commented May 19, 2026

Copy link
Copy Markdown
Member

The CI says that the file Feature_graph/package_info/Feature_graph/dependencies is missing and should contain

Installation
STL_Extension

@ange-clement

Copy link
Copy Markdown
Member Author

The CI says that the file Feature_graph/package_info/Feature_graph/dependencies is missing and should contain

Installation
STL_Extension

Ok, I didn't understand because the package creation script only generate the file Feature_graph/doc/Feature_graph/dependencies which did contain STL_Extension...

The API will be templeted internally for the images, but this will stay undocumented until a more suitable location for this concept is created.
@ange-clement

Copy link
Copy Markdown
Member Author

/build:doc-wip

@github-actions

Copy link
Copy Markdown

There was an error while building the doc:


https://github.com/CGAL/cgal/actions/runs/27834859744

@albert-github

Copy link
Copy Markdown
Contributor

I saw this problem regarding the PR's documentation now a couple of times and I have the suspicion that this might have to do with the recent change to a newer version of doxygen 1.9.6 to 1.17.0) probably somewhere some logic is not working as expected.
(@sloriot )

@ange-clement

Copy link
Copy Markdown
Member Author

/build:doc-wip

@github-actions

Copy link
Copy Markdown

There was an error while building the doc:


https://github.com/CGAL/cgal/actions/runs/28083547975

@ange-clement

Copy link
Copy Markdown
Member Author

/build:doc-wip

@github-actions

Copy link
Copy Markdown

There was an error while building the doc:

This round already exists. Overwrite it with /force-build.

https://github.com/CGAL/cgal/actions/runs/28361631146

@ange-clement

Copy link
Copy Markdown
Member Author

/force-build:doc-wip

@github-actions

Copy link
Copy Markdown

The documentation is built. It will be available, after a few minutes, here: https://cgal.github.io/9465/doc/Manual/index.html

@afabri

afabri commented Jun 29, 2026

Copy link
Copy Markdown
Member

@ange-clement est-ce que tu peux repondre aux remarques que j'ai fait.

@afabri

afabri commented Jun 29, 2026

Copy link
Copy Markdown
Member
image

@afabri

afabri commented Jun 29, 2026

Copy link
Copy Markdown
Member

There is Reference Manual inside Reference Manual

Comment thread Feature_graph/doc/Feature_graph/Concepts/SharpnessEstimator.h Outdated
Comment thread Feature_graph/doc/Feature_graph/Concepts/NormalEstimator.h
Comment thread Feature_graph/doc/Feature_graph/Concepts/SharpnessEstimator.h
Comment thread Feature_graph/doc/Feature_graph/Concepts/SharpnessEstimator.h Outdated
Comment thread Feature_graph/doc/Feature_graph/PackageDescription.txt Outdated
Comment thread Feature_graph/doc/Feature_graph/PackageDescription.txt Outdated
Comment thread Feature_graph/doc/Feature_graph/PackageDescription.txt Outdated
Comment thread Feature_graph/doc/Feature_graph/PackageDescription.txt Outdated
Renamed ..._on_X to X_..._estimator
Changed Sharpness_value_type to Sharpness_number_type
Add example CMakeLists + added typename
@ange-clement

Copy link
Copy Markdown
Member Author

/force-build:doc-wip

@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown

The documentation is built. It will be available, after a few minutes, here: https://cgal.github.io/9465/doc-wip/Manual/index.html

*
* \cgalParamSectionBegin{point_to_element_output_map}
* \cgalParamDescription{an output property map that will be filled if supplied.
* It allow retrieving of the surface element where a feature graph point is embedded.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
* It allow retrieving of the surface element where a feature graph point is embedded.
* It enables retrieving of the surface element where a feature graph point is embedded.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@efifogel once pointed out to me that we use the word "allow" often in a wrong way. It should be "enable" most of the time. See here for a discussion.

* and must implement `put(output_pmap, feature_vertex_descriptor, facet_descriptor)`
* where the key type is the `vertex_descriptor` of the feature graph, and the value type is
* a descriptor of an element of the surface, with type `std::size_t`.
* The facet descriptor correspond to a facet element with the tag `CGAL::DimensionTag<2>`}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is that on purpose "facet" instead of "face"?

* </UL>
* <LI> The type `DimensionTag` represents the element type, it can either be `CGAL::Dimension_tag<0>`, `CGAL::Dimension_tag<1>` or `CGAL::Dimension_tag<2>`,
* which represent either a vertex element, an edge element, or a facet element respectively.
* <LI> The type `Descriptor` is either a `vertex_descriptor`, `halfedge_descriptor` or `face_descriptor` and

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Coming back to "facet", even if was the correct mathematical term, I suggest to call it face, as we use a face_descriptor for it.

* If it is a scalar field, it must implement
* <UL>
* <LI> `template <typename DimensionTag, typename Descriptor>`
* <BR> `FT operator()(const Point_3& point_in_space, const Descriptor& element_descriptor, const PolygonMesh& pmesh) const`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why the suffix _in_space?

if (MSVC)
add_compile_options(/bigobj)
else ()
add_compile_options(-Wa,-mbig-obj)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this really needed ? It gives error messages in the testsuite

@@ -0,0 +1,127 @@
// WARNING : non-functioning example. This exmaple is to understand how to create and use the feature graph for tetrahedral mesh creation.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What do you mean with "non-functionning"??

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature Not yet approved The feature or pull-request has not yet been approved. Work in progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants