Skip to content

Commit ed47c5f

Browse files
LeoValqueMaelRLsloriot
authored
Apply suggestions from code review
Co-authored-by: Mael <mael.rouxel.labbe@geometryfactory.com> Co-authored-by: Sebastien Loriot <sloriot.ml@gmail.com>
1 parent 8a69e90 commit ed47c5f

3 files changed

Lines changed: 16 additions & 23 deletions

File tree

Snap_rounding_2/doc/Snap_rounding_2/PackageDescription.txt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,12 @@
66
/// Functions to round segments or polygons safely
77
/// \ingroup PkgSnapRounding2Ref
88

9-
/// \defgroup Snap_rounding_hot_pixel_grp Hot pixel snap rounding
10-
/// Classes and functions to round segments or polygons using hot pixel SR method.
9+
/// \defgroup Snap_rounding_hot_pixel_grp Hot Pixel Snap Rounding
10+
/// Classes and functions to round segments or polygons using Hot Pixel SR method.
1111
/// \ingroup PkgSnapRounding2Ref
1212

13-
/// \defgroup Snap_rounding_vertical_slab_grp Vertical slab snap rounding
14-
/// Classes and functions to round segments or polygons using vertical slab SR method.
13+
/// \defgroup Snap_rounding_vertical_slab_grp Vertical Slab Snap Rounding
14+
/// Classes and functions to round segments or polygons using Vertical Slab SR method.
1515
/// \ingroup PkgSnapRounding2Ref
1616

1717
/*!

Snap_rounding_2/doc/Snap_rounding_2/Snap_rounding_2.txt

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -94,15 +94,15 @@ An arrangement of segments before subdividing the segments (a), after subdividin
9494
We evaluate the different methods on two tests:
9595
In the first test, the segment endpoints are chosen uniformly and independantly in the unit box.
9696
In the second test, we generate perturbed copies of a given segment. This experiment is repeated several times using different input segments.
97-
The two pixel sizes for hot pixel snap rounding are chosen such that the rounding shift is of the same order of magnitude as the rounding induced by using `double` or `float` coordinates.
97+
The two pixel sizes for Hot Pixel Snap Rounding are chosen such that the rounding shift is of the same order of magnitude as the rounding induced by using `double` or `float` coordinates.
9898

9999
The first test is standard but typically exhibits few, if any, rounding issues.
100100
The second test is deliberately designed to be difficult, as it produces a large number of rounding issues when rounding is performed naively.
101-
In these settings, vertical slab snap rounding significantly outperforms hot pixel snap rounding and remains of the same order of magnitude than computing the intersections without snap.
101+
In these settings, vertical slab snap rounding significantly outperforms Hot Pixel Snap Rounding and remains of the same order of magnitude than computing the intersections without snap.
102102
The result of our test is summarized in the next table:
103103

104104
<center>
105-
| Test Case | Computing intersection<br> (Unsafe rounding) | Vertical slab SR <br> (double) | Vertical Slab SR <br> (float) | Hot pixel SR <br>(pixel size \f$10^{-15}\f$ ) | Hot pixel SR <br>(pixel size \f$10^{-7}\f$) |
105+
| Test Case | Computing intersection<br> (Unsafe rounding) | Vertical Slab SR <br> (double) | Vertical Slab SR <br> (float) | Hot Pixel SR <br>(pixel size \f$10^{-15}\f$ ) | Hot Pixel SR <br>(pixel size \f$10^{-7}\f$) |
106106
|-----------|-------|-------|-------|-------|-------|
107107
| 500 Random Segments | 0.066s | 0.213s | 0.172s | 6.14s | 3.81s |
108108
| 1000 Random Segments | 0.241s | 0.928s | 0.769s | 52.1s | 53.5s |
@@ -120,22 +120,19 @@ vertices is \f$O(n^3)\f$, or \f$O(n^2)\f$ if the input segments are free of inte
120120

121121
\section Snap_rounding_2API Snap Rounding API and Examples
122122

123-
Both algorithms can be invoked through the \ref snap_rounding_2_fct "CGAL::snap_rounding_2()" function.
124-
\ref snap_rounding_2_fct "CGAL::snap_rounding_2()" take a range of input segments and, by default, produces a range of polylines, each polyline corresponding to an input segment.
123+
Both algorithms can be invoked through the \ref snap_rounding_2_fct "CGAL::snap_rounding_2()" function, which takes a range of input segments and, by default, produces a range of polylines, each polyline corresponding to an input segment.
125124
Consequently, duplicate segments may appear in the output, for instance when multiple input segments collapse. When the parameter `output_unique_segments` is set to `true`,
126125
the polylines are decomposed into individual segments (they are represented as polylines with two points), and duplicates are removed.
127126
An overload also supports as input and output polygon ranges.
128-
By default, the Vertical Slab Snap Rounding algorithm to double precision is used. The other method or other rounding schemes can be used depending on the traits class provided.
127+
By default, the Vertical Slab Snap Rounding algorithm to double precision is used. Other methods or other rounding schemes can be used depending on the traits class provided.
129128
Specific methods also exist: `CGAL::vertical_slab_snap_rounding_2()` and `CGAL::hot_pixel_snap_rounding_2`.
130129

131130
\subsection Float_snap_rounding_Traits Snap Rounding Traits
132-
133-
134131
When using Hot Pixel SR, the traits class must be a model of `HotPixelSnapRoundingTraits_2`. The class `Hot_pixel_snap_rounding_traits_2` is such a model.
135132

136-
When using Vertical Slab SR, the traits class must be a model of `VerticalSlabSnapRoundingTraits_2`. By default,
137-
the traits class `Double_grid_snap_rounding_traits_2`, which rounds coordinates to double precision floating point,
138-
is used. The package also provides `Float_grid_snap_rounding_traits_2` and `Integer_grid_snap_rounding_traits_2`, which rounds coordinates to
133+
When using Vertical Slab SR, the traits class must be a model of `VerticalSlabSnapRoundingTraits_2`. By default,
134+
the traits class `CGAL::Double_grid_snap_rounding_traits_2`, which rounds coordinates to double precision floating point,
135+
is used. The package also provides `CGAL::Float_grid_snap_rounding_traits_2` and `CGAL::Integer_grid_snap_rounding_traits_2`, which rounds coordinates to
139136
single precision floating point and integers, respectively. Other traits classes can be used to round to different representations or rounding schemes as long as
140137
the rounding operation preserves the order of vertices along the \f$x\f$ and \f$y\f$ directions.
141138
See `VerticalSlabSnapRoundingTraits_2` for more details.
@@ -160,8 +157,6 @@ The package is supplied with a graphical demo program that opens a window,
160157
allows the user to edit segments dynamically, applies a selected
161158
snap-rounding procedures, and displays the result onto the same window
162159
(see `<CGAL_ROOT>/GraphicsView/demo/Snap_rounding_2/Snap_rounding_2.cpp`).
163-
164-
165160
\section Snap_rounding_history Implementation History
166161

167162
Snap Rounding and Iterative Snap Rounding was introduced by Eli Packer in 2001.

Snap_rounding_2/include/CGAL/vertical_slab_snap_rounding_2.h

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -464,16 +464,14 @@ void vertical_slab_snap_rounding_2_polygon(const PolygonRange &polygons,
464464
Polygon_2 P;
465465
std::size_t idx_start = polygon_indices[polygon_idx];
466466
std::size_t idx_end = polygon_indices[polygon_idx+1];
467-
std::size_t last_insert;
467+
std::size_t last_insert = 0;
468468
for(std::size_t pl_idx = idx_start; pl_idx != idx_end; ++pl_idx){
469469
auto &pl = polylines[pl_idx];
470470
// The first element is not add, it is identical to the last
471471

472-
bool go_forward;
473-
if(pl_idx == idx_start)
474-
go_forward = (pl.back() == polylines[pl_idx+1].front()) || (pl.back() == polylines[pl_idx+1].back());
475-
else
476-
go_forward = (last_insert == pl.front());
472+
bool go_forward = (pl_idx == idx_start)
473+
? (pl.back() == polylines[pl_idx+1].front()) || (pl.back() == polylines[pl_idx+1].back())
474+
: (last_insert == pl.front());
477475

478476
// Add the element in forward direction
479477
if(go_forward){

0 commit comments

Comments
 (0)