Fix/start turns #769
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -507,6 +507,46 @@ inline void discard_interior_exterior_turns(Turns& turns, Clusters& clusters) | |
| } | ||
| } | ||
|
|
||
| template<typename Turns, typename Clusters> | ||
| inline void discard_start_turns(Turns& turns, Clusters& clusters) | ||
| { | ||
| for (auto& nv : clusters) | ||
| { | ||
| cluster_info& cinfo = nv.second; | ||
| auto& indices = cinfo.turn_indices; | ||
| std::size_t start_count{0}; | ||
|
Member
There was a problem hiding this comment. Out of curiosity, why not
Collaborator
Author
There was a problem hiding this comment. Habit, it the office we're preferring the new uniform initialization. What do you think?
Member
There was a problem hiding this comment. I used to be very keen in preferring the uniform initialization heavily and everywhere, until I read the abseil guidelines and tips, in particular https://abseil.io/tips/88
It's a pity the uniform initialization is not corner-case free improvement to C++.
Member
There was a problem hiding this comment. In this particular case I'd use This is not a case when an object of a class is created by passing some abstract which may describe something like number of elements. From the same reason I'd probably initialize the vector like the one below. In other words to express that the initializer list on the right represents the vector or that it is the vector (just like with This works right? ;) And then for all other things I don't have an opinion. I'd probably use the old parentheses. But maybe that's just because it's what I'm familiar with. Maybe with the exception of avoiding double parentheses to deal with the most vexing parse. So I guess I could also express this as a conservative stance where old initialization is used when possible as it was in the past and new brackets only for new features.
Collaborator
Author
There was a problem hiding this comment. Interesting visions, thanks. |
||
| for (signed_size_type index : indices) | ||
| { | ||
| auto const& turn = turns[index]; | ||
| if (turn.method == method_start) | ||
| { | ||
| start_count++; | ||
| } | ||
| } | ||
| if (start_count == 0 && start_count == indices.size()) | ||
| { | ||
| // There are no start turns, or all turns in the cluster are start turns. | ||
| continue; | ||
| } | ||
|
|
||
| // Discard the start turns and simultaneously erase them from the indices | ||
| for (auto it = indices.begin(); it != indices.end();) | ||
| { | ||
| auto& turn = turns[*it]; | ||
| if (turn.method == method_start) | ||
| { | ||
| turn.discarded = true; | ||
| turn.cluster_id = -1; | ||
| it = indices.erase(it); | ||
| } | ||
| else | ||
| { | ||
| ++it; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| template <typename Geometry0, typename Geometry1> | ||
| inline segment_identifier get_preceding_segment_id(segment_identifier const& id, | ||
| Geometry0 const& geometry0, Geometry1 const& geometry1) | ||
|
|
@@ -700,6 +740,8 @@ inline bool handle_colocations(Turns& turns, Clusters& clusters, | |
| // on turns which are discarded afterwards | ||
| set_colocation<OverlayType>(turns, clusters); | ||
|
|
||
| discard_start_turns(turns, clusters); | ||
|
|
||
| if (BOOST_GEOMETRY_CONDITION(target_operation == operation_intersection)) | ||
| { | ||
| discard_interior_exterior_turns | ||
|
|
@@ -783,7 +825,7 @@ inline bool fill_sbs(Sbs& sbs, Point& turn_point, | |
| { | ||
| signed_size_type turn_index = *sit; | ||
| turn_type const& turn = turns[turn_index]; | ||
| if (first) | ||
| { | ||
| turn_point = turn.point; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code is relatively simple now, I first had more conditions but they were not relevant in the end.
But I might find cases where some behaviour should be fine-tuned.