CGAL / cgal

The public CGAL repository, see the README below
https://github.com/CGAL/cgal#readme
Other
4.67k stars 1.35k forks source link

The width of documentation of CGAL::Polygon_mesh_processing::corefine_and_compute_boolean_operations is too large #8176

Closed lrineau closed 1 month ago

lrineau commented 2 months ago

https://doc.cgal.org/latest/Polygon_mesh_processing/group__PMP__corefinement__grp.html#gaaeb559e2f901b418ddd563e8aace83eb

That documentation does not fit in the width of any of my screens.

That seems to be because of const std::tuple< NamedParametersOut0, NamedParametersOut1, NamedParametersOut2, NamedParametersOut3 > & nps_out = std::tuple<NamedParametersOut0,NamedParametersOut1,NamedParametersOut2,NamedParametersOut3>() that is too wide, and not wrapped by Doxygen CSS.

Same issue with CGAK-5.5.x (and even previous versions). I am surprised it was not pointed out by the review of the documentation... :smiling_imp:

sloriot commented 2 months ago

If you use the horizontal scrolling bar it works. It's not broken. I can rename the template parameters if you want. image

lrineau commented 2 months ago

It is not broken, but it make the documentation really difficult to read. In particular the paragraph:

The positions of the meshes in the array output are specific to the Boolean operation to compute and Corefinement::Boolean_operation_type encodes and describes the ordering. Constructing the default array means that no Boolean operation will be done. Overwriting a default value will trigger the corresponding operation. In such a case, the address to a valid surface mesh must be provided. The optional named parameters for all output meshes are provided as a tuple and follow the same order as the array output. A call to corefine_and_compute_boolean_operations() with optional named parameters passed for output meshes should be done using make_tuple() as the types of named parameters are unspecified.

On my screen, it is really large, twice wider than the screen: Screenshot_20240502_124729 Screenshot_20240502_124744 and it is difficult to follow the stream of the sentences, using the horizontal scrollbar.

afabri commented 2 months ago

Let's just change it to NPOut1

afabri commented 1 month ago

Tested in CGAL-Ic-237 I am wondering if we should not also have a shorter namespace PMP.

lrineau commented 1 month ago

Tested in CGAL-Ic-237 I am wondering if we should not also have a shorter namespace PMP.

I plan to merge #8178. Do you think we should keep this issue open, and add another patch to have a nicer documentation? @afabri

lrineau commented 1 month ago

Tested in CGAL-Ic-237 I am wondering if we should not also have a shorter namespace PMP.

I plan to merge #8178. Do you think we should keep this issue open, and add another patch to have a nicer documentation? @afabri

Ping @afabri, you have not yet reacted to the question above.