-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Aos 2 fixes efif #8013
Merged
Merged
Aos 2 fixes efif #8013
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ram_on_surface_2::Arrangement, which is now obsolete
…angement obsolete (instead use Base_aos).
…ta() to avoid clashes with other extensions of the Dcel; Added template parameters (vertex-base, halfedge-base, and face-base) with default values to Envelope_pm_dcel
…ta() to avoid clashes with other extensions of the Dcel;
…nv_'); this enables the simultaneous extensions required by other packages; for example, having a lower envelope diagram with history.
…ith a default value (being the 2D arrangement linear traits). Similarly, passed the base class of Env_triangle_traits_3 as a template parameter with a default value (being the 2D arrangement segment traits).
sloriot
reviewed
Feb 14, 2024
sloriot
reviewed
Feb 14, 2024
Fixed both
____ _ ____ _
/_____/_) o /__________ __ //
(____ ( ( ( (_/ (_/-(-'_(/
_/
…On Wed, 14 Feb 2024 at 10:31, Sebastien Loriot ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In Envelope_3/include/CGAL/Env_plane_traits_3.h
<#8013 (comment)>:
>
// our line is a3*x + b3*y + c3 = 0
// it is assumed that the planes intersect over this line
const Line_2& line = cv.supp_line();
- const FT& a3 = line.a(),
- b3 = line.b(),
- c3 = line.c();
+ const FT& a3 = line.a();
+ const FT& b3 = line.b();
+ const FT& c3 = line.c();
Warning: unused variable c3
—
Reply to this email directly, view it on GitHub
<#8013 (review)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABVBNOBW77G7BBCN5OGQIZDYTRY57AVCNFSM6AAAAABCTMCSWKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQNZZGY3DAOBQGQ>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
sloriot
added
Batch_1
First Batch of PRs under testing
and removed
Tests failing (warning)
Under Testing
labels
Feb 14, 2024
Successfully tested in CGA-6.0-Ic-173 |
lrineau
added
the
rm only: ready for master
For the release team only: that indicates that a PR is about to be merged in 'master'
label
Feb 16, 2024
lrineau
removed
the
rm only: ready for master
For the release team only: that indicates that a PR is about to be merged in 'master'
label
Feb 16, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of Changes
This PR contains a bunch of changes that address issues found while I worked on the 2nd edition of the "CGAL Arrangement Book". In the Book we offer the code of several programs in Python. The code is based on the recent Python bindings. Several improvements to the code were necessary to support what I refer to "layered extension". For example,
Arrangement_on_surface_with_history_2
derives fromArrangement_on_surface_2
and requires extensions to the DCEL records.Envelope_diagram_on_surface_2
also derives fromArrangement_on_surface_2
and requires extensions to the DCEL records. The improvements applied enables havingEnvelope_diagram_on_surface_with_history_2
(which, naturally, requires both sets of extensions). I also seized the opportunity and made the code look more modern, for example, replaced a bunch of "typedef" statements with "using" statements.Envelope_diagram_1
.Arr_dcel
, which essentially replaces the formerArr_default_dcel
. Backward compatibility was maintained by the (re) introduction of the alias templateArr_default_dcel
.Arr_dcel
, as opposed to the formerArr_default_dcel
is parameterized by (in addition to the geometry traits) Vertex, Halfedge, and Face template parameters, and they have default type values. All this enables the layered extension of DCEL records; see Arr_dcelCurve_halfedges
nested inArrangement_on_surface_with_history_2
public, as we enable iterating over such types (so it really makes no sense not to have it public). I haven't reflected it in the manual. Publishing this, I believe, requires a small feature, and I'll do it separately.Env_triangle_traits_3
,Env_plane_traits_3
, andEnv_sphere_traits_3
. (Some local objects still remain; however, eliminating them will require breaking backward compatibility, so it is left for a future PR.)Env_plane_traits_3
as a template parameter with a default value (being the 2D arrangement linear traits); see Env_plane_traits_3. Similarly, passed the base class ofEnv_triangle_traits_3
as a template parameter with a default value (being the 2D arrangement segment traits); see Env_triangle_traits_3. See also EnvelopeTraits_3 concept . This chanhes is required for "layered extension".Envelope_pm_dcel
. This is similar toArr_dcel
; see above. BW,Envelope_pm_dcel
is not published, so there is no change to the manual. In fact, I don't see a reason not to publish it, but this will come in a future PR.Release Management