Optimize the do_intersect() functions of the 2D Regularized Boolean Set Operation" package (made it tolerant to inexact kernels.)

[efifogel]

Summary of Changes

Optimized do_intersect(polygon, polygon), do_intersect(begin, end), and do_intersect(begin1, end1, begin2, end2):
(i) Terminated the execution once an intersection is detected. (In the past, the intersection was computed in one phase and examined in a subsequent phase.)
(ii) Made the variants of the free functions do_intersect() that apply to linear polygons, robust even with an inexact-construction kernel. The variants that apply to generalized polygons endure inexact constructions much more than before; however, there are rare degenerate cases that are still require an exact construction kernel.

In general, the changes described here do not affect the default interface, so a small feature is not required. However, it is a major impact, and it does affect the interface as described bellow, and even somehow break backward compatibility.

Recently, the code of the package "2D Regularized Boolean Set Operations" was optimized. In particular, a 3rd optional parameter was introduced in the free functions. It determined whether the boundaries of the input polygons are treated as cyclic sequences of single (x-monotone) segments or as a cyclic sequences of (x-monotone) polylines. The change described here eliminates this 3rd parameter, and brings the interface of the `do_intersect() function back to the original design with two input polygons.

Release Management

• Affected package(s): Boolean_set_operations_2, Surface_sweep, Arrangement_on_surface_2
• Feature/Small Feature (if any): here
• Link to compiled documentation (obligatory for small feature) wrong link name to be changed
• License and copyright ownership: TAU

[sloriot]

Is there anything from #5284 that we want to keep/import?

[efifogel]

The two conclusions in #5284 <#5284> still hold: 1. Rename the package to Regularized_boolean_set_operations_2 2. Introduce the namespace Regularized_boolean_set_operations_2 BW, it might be easier to re-implement the above rather than to integrate the old branch. Perhaps we can do it as well before the next version is released. PS, I've realized that my implementation is deficient. It works well for linear curves, but if the input consists of general polygons bounded by non-linear curves, it may fail. In the general case 2 curves may intersect (in their interiors) at a point where they only touch (their second derivatives have opposite signs). In such cases two polygons bounded by such curves may not necessarily intersect in their interiors. Anyhow, a simple solution is to add a tag to the relevant traits that indicates that the curves are linear. If the curves are linear, continue as planned, that is, abort the procedure and return true (to indicate that an intersection has been detected). Otherwise, check the multiplicity of the intersection. If the curves change relative places, abort the procedure and return true. Otherwise, continue The consequence of the above is that the procedure will not be stable when using EPIC for non linear curves (cause we generate a new point---the intersection point, and we use it further down the road). It will still abort once a definitive intersection is detected. I wanted to add a tag that indicates that curves are linear for a while now. I believe that we can use it to expedite other procedures as well. The bottom line is the PR is not ready yet... Naturally, I'll add the necessary tests. Efi ____ _ ____ _ /_____/_) o /__________ __ // (____ ( ( ( (_/ (_/-(-'_(/ _/

…

On Tue, 2 Sept 2025 at 09:48, Sebastien Loriot ***@***.***> wrote: *sloriot* left a comment (CGAL/cgal#9050) <#9050 (comment)> Is there anything from #5284 <#5284> that we want to keep/import? — Reply to this email directly, view it on GitHub <#9050 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABVBNOHEF7F3U4HLOMOKJDL3QU4SPAVCNFSM6AAAAACFKNK7SOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTENBUGAYDOOJTGA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[efifogel]

This PR is now ready for integration ____ _ ____ _ /_____/_) o /__________ __ // (____ ( ( ( (_/ (_/-(-'_(/ _/

…

On Tue, 2 Sept 2025 at 12:52, Efi Fogel ***@***.***> wrote: The two conclusions in #5284 <#5284> still hold: 1. Rename the package to Regularized_boolean_set_operations_2 2. Introduce the namespace Regularized_boolean_set_operations_2 BW, it might be easier to re-implement the above rather than to integrate the old branch. Perhaps we can do it as well before the next version is released. PS, I've realized that my implementation is deficient. It works well for linear curves, but if the input consists of general polygons bounded by non-linear curves, it may fail. In the general case 2 curves may intersect (in their interiors) at a point where they only touch (their second derivatives have opposite signs). In such cases two polygons bounded by such curves may not necessarily intersect in their interiors. Anyhow, a simple solution is to add a tag to the relevant traits that indicates that the curves are linear. If the curves are linear, continue as planned, that is, abort the procedure and return true (to indicate that an intersection has been detected). Otherwise, check the multiplicity of the intersection. If the curves change relative places, abort the procedure and return true. Otherwise, continue The consequence of the above is that the procedure will not be stable when using EPIC for non linear curves (cause we generate a new point---the intersection point, and we use it further down the road). It will still abort once a definitive intersection is detected. I wanted to add a tag that indicates that curves are linear for a while now. I believe that we can use it to expedite other procedures as well. The bottom line is the PR is not ready yet... Naturally, I'll add the necessary tests. Efi ____ _ ____ _ /_____/_) o /__________ __ // (____ ( ( ( (_/ (_/-(-'_(/ _/ On Tue, 2 Sept 2025 at 09:48, Sebastien Loriot ***@***.***> wrote: > *sloriot* left a comment (CGAL/cgal#9050) > <#9050 (comment)> > > Is there anything from #5284 <#5284> > that we want to keep/import? > > — > Reply to this email directly, view it on GitHub > <#9050 (comment)>, or > unsubscribe > <https://github.com/notifications/unsubscribe-auth/ABVBNOHEF7F3U4HLOMOKJDL3QU4SPAVCNFSM6AAAAACFKNK7SOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTENBUGAYDOOJTGA> > . > You are receiving this because you authored the thread.Message ID: > ***@***.***> >

[sloriot]

 The following files have tabs:
Boolean_set_operations_2/include/CGAL/Boolean_set_operations_2/Gps_on_surface_base_2.h

[efifogel]

Fixed ____ _ ____ _ /_____/_) o /__________ __ // (____ ( ( ( (_/ (_/-(-'_(/ _/

…

On Tue, 9 Sept 2025 at 20:55, Sebastien Loriot ***@***.***> wrote: *sloriot* left a comment (CGAL/cgal#9050) <#9050 (comment)> The following files have tabs: Boolean_set_operations_2/include/CGAL/Boolean_set_operations_2/Gps_on_surface_base_2.h — Reply to this email directly, view it on GitHub <#9050 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABVBNOGPSD5ENEKXO5VJBPD3R4H75AVCNFSM6AAAAACFKNK7SOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTENZRG4ZTENRWGI> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[efifogel]

I fixed one thing. There might be more, but I'm abroad and the Internet connection is really bad, so I'll check again when I'm back (Tue.) Feel free to run it again or not till then... ____ _ ____ _ /_____/_) o /__________ __ // (____ ( ( ( (_/ (_/-(-'_(/ _/

…

On Fri, 12 Sept 2025 at 10:32, Sebastien Loriot ***@***.***> wrote: *sloriot* left a comment (CGAL/cgal#9050) <#9050 (comment)> Only warnings left now! There might be other warnings coming from the other branch. — Reply to this email directly, view it on GitHub <#9050 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABVBNOFS7G7OLBCWOXITEGL3SJZKRAVCNFSM6AAAAACFKNK7SOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTEOBUGEYTIOBXGA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[efifogel]

I fixed the remaining issues. Please re-test; thanks. ____ _ ____ _ /_____/_) o /__________ __ // (____ ( ( ( (_/ (_/-(-'_(/ _/

…

On Sat, 13 Sept 2025 at 19:19, Efi Fogel ***@***.***> wrote: I fixed one thing. There might be more, but I'm abroad and the Internet connection is really bad, so I'll check again when I'm back (Tue.) Feel free to run it again or not till then... ____ _ ____ _ /_____/_) o /__________ __ // (____ ( ( ( (_/ (_/-(-'_(/ _/ On Fri, 12 Sept 2025 at 10:32, Sebastien Loriot ***@***.***> wrote: > *sloriot* left a comment (CGAL/cgal#9050) > <#9050 (comment)> > > Only warnings left now! There might be other warnings coming from the > other branch. > > — > Reply to this email directly, view it on GitHub > <#9050 (comment)>, or > unsubscribe > <https://github.com/notifications/unsubscribe-auth/ABVBNOFS7G7OLBCWOXITEGL3SJZKRAVCNFSM6AAAAACFKNK7SOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTEOBUGEYTIOBXGA> > . > You are receiving this because you authored the thread.Message ID: > ***@***.***> >

[efifogel]

Fixed ____ _ ____ _ /_____/_) o /__________ __ // (____ ( ( ( (_/ (_/-(-'_(/ _/

…

On Thu, 18 Sept 2025 at 10:21, Andreas Fabri ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In Boolean_set_operations_2/include/CGAL/Boolean_set_operations_2/Gps_on_surface_base_2.h <#9050 (comment)>: > + + // std::cout << std::setw(indent) << "" << "Merging [" << lower << "," << curr_lower << "," << sub_size << "]\n"; + return merge_func(lower, curr_lower, sub_size, arr_vec); + } + + template <typename InputIterator, typename Merge> + bool do_intersect_divide_and_conquer2(InputIterator begin, InputIterator end, std::size_t k, Merge merge) { + std::vector<Arr_entry> arr_entries; + arr_entries.reserve(k); + arr_entries.resize(1); + arr_entries[0].first = m_arr; + arr_entries[0].second = m_vertices; + std::size_t size = std::distance(begin, end); + auto it = begin; + while (it != end) { + std::size_t num = std::min(size+1, k); ⬇️ Suggested change - std::size_t num = std::min(size+1, k); + std::size_t num = (std::min)(size+1, k); — Reply to this email directly, view it on GitHub <#9050 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABVBNOFZGPUWJU73CMHA4ET3TJMPXAVCNFSM6AAAAACFKNK7SOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTEMZXG42DSMRQHA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[sloriot]

Successfully tested in CGAL-6.2-Ic-5

[github-actions]

This pull-request was previously marked with the label Tested, but has been modified with new commits. That label has been removed.

[sloriot]

Successfully tested in CGAL-6.2-Ic-39

TODO: after the merge create an issue to mention to create a non-regularized version of do_intersect, following the discussion (that I cannot find for now) where we agreed to add a new namespace for the regularized version so that the version in the CGAL namespace has the same behavior as the Kernel do-intersect