Skip to content
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.

Sign up for GitHub

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

[traverse] reverse meaning of isolation in difference #887

Merged
merged 3 commits into from
Jul 28, 2021
Merged

[traverse] reverse meaning of isolation in difference #887

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4597a16
[traverse] reverse meaning of isolation in difference
barendgehrels Jul 8, 2021
68ed6fe
[traverse] avoid using isolated for originally non-isolated regions
barendgehrels Jul 14, 2021
4b3f655
[test] add testcase for issue #888 polygon 53
barendgehrels Jul 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions include/boost/geometry/algorithms/detail/overlay/is_self_turn.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ struct is_self_turn_check
template <typename Turn>
static inline bool apply(Turn const& turn)
{
return turn.operations[0].seg_id.source_index
== turn.operations[1].seg_id.source_index;
return turn.is_self();
}
};

Expand Down
98 changes: 55 additions & 43 deletions include/boost/geometry/algorithms/detail/overlay/traversal.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -621,31 +621,32 @@ public :
return m_turns[rp.turn_index].operations[rp.operation_index];
}

inline sort_by_side::rank_type select_rank(sbs_type const& sbs,
bool skip_isolated) const
inline sort_by_side::rank_type select_rank(sbs_type const& sbs) const
{
static bool const is_intersection
= target_operation == operation_intersection;

// Take the first outgoing rank corresponding to incoming region,
// or take another region if it is not isolated
turn_operation_type const& incoming_op
= operation_from_rank(sbs.m_ranked_points.front());
auto const& in_op = operation_from_rank(sbs.m_ranked_points.front());

for (std::size_t i = 0; i < sbs.m_ranked_points.size(); i++)
{
typename sbs_type::rp const& rp = sbs.m_ranked_points[i];
auto const& rp = sbs.m_ranked_points[i];
if (rp.rank == 0 || rp.direction == sort_by_side::dir_from)
{
continue;
}
turn_operation_type const& op = operation_from_rank(rp);
auto const& out_op = operation_from_rank(rp);

if (op.operation != target_operation
&& op.operation != operation_continue)
if (out_op.operation != target_operation
&& out_op.operation != operation_continue)
{
continue;
}

if (op.enriched.region_id == incoming_op.enriched.region_id
|| (skip_isolated && ! op.enriched.isolated))
if (in_op.enriched.region_id == out_op.enriched.region_id
|| (is_intersection && ! out_op.enriched.isolated))
{
// Region corresponds to incoming region, or (for intersection)
// there is a non-isolated other region which should be taken
Expand All @@ -660,7 +661,7 @@ public :
int& op_index, sbs_type const& sbs,
signed_size_type start_turn_index, int start_op_index) const
{
sort_by_side::rank_type const selected_rank = select_rank(sbs, false);
sort_by_side::rank_type const selected_rank = select_rank(sbs);

int current_priority = 0;
for (std::size_t i = 1; i < sbs.m_ranked_points.size(); i++)
Expand Down Expand Up @@ -688,49 +689,59 @@ public :
inline bool analyze_cluster_intersection(signed_size_type& turn_index,
int& op_index, sbs_type const& sbs) const
{
sort_by_side::rank_type const selected_rank = select_rank(sbs, true);
// Select the rank based on regions and isolation
sort_by_side::rank_type const selected_rank = select_rank(sbs);

if (selected_rank > 0)
if (selected_rank <= 0)
{
typename turn_operation_type::comparable_distance_type
min_remaining_distance = 0;
return false;
}

std::size_t selected_index = sbs.m_ranked_points.size();
for (std::size_t i = 0; i < sbs.m_ranked_points.size(); i++)
// From these ranks, select the index: the first, or the one with
// the smallest remaining distance
typename turn_operation_type::comparable_distance_type
min_remaining_distance = 0;

std::size_t selected_index = sbs.m_ranked_points.size();
for (std::size_t i = 0; i < sbs.m_ranked_points.size(); i++)
{
auto const& ranked_point = sbs.m_ranked_points[i];

if (ranked_point.rank > selected_rank)
{
break;
}
else if (ranked_point.rank == selected_rank)
{
typename sbs_type::rp const& ranked_point = sbs.m_ranked_points[i];
auto const& op = operation_from_rank(ranked_point);

if (ranked_point.rank == selected_rank)
if (op.visited.finalized())
{
turn_operation_type const& op = operation_from_rank(ranked_point);

if (op.visited.finalized())
{
// This direction is already traveled before, the same
// cannot be traveled again
continue;
}

// Take turn with the smallest remaining distance
if (selected_index == sbs.m_ranked_points.size()
|| op.remaining_distance < min_remaining_distance)
{
selected_index = i;
min_remaining_distance = op.remaining_distance;
}
// This direction is already traveled,
// it cannot be traveled again
continue;
}
}

if (selected_index < sbs.m_ranked_points.size())
{
typename sbs_type::rp const& ranked_point = sbs.m_ranked_points[selected_index];
turn_index = ranked_point.turn_index;
op_index = ranked_point.operation_index;
return true;
if (selected_index == sbs.m_ranked_points.size()
|| op.remaining_distance < min_remaining_distance)
{
// It was unassigned or it is better
selected_index = i;
min_remaining_distance = op.remaining_distance;
}
}
}

return false;
if (selected_index == sbs.m_ranked_points.size())
{
// Should not happen, there must be points with the selected rank
return false;
}

auto const& ranked_point = sbs.m_ranked_points[selected_index];
turn_index = ranked_point.turn_index;
op_index = ranked_point.operation_index;
return true;
}

inline bool fill_sbs(sbs_type& sbs,
Expand Down Expand Up @@ -819,6 +830,7 @@ public :
return result;
}

// Analyzes a non-clustered "ii" intersection, as if it is clustered.
inline bool analyze_ii_intersection(signed_size_type& turn_index, int& op_index,
turn_type const& current_turn,
segment_identifier const& previous_seg_id)
Expand Down
143 changes: 108 additions & 35 deletions include/boost/geometry/algorithms/detail/overlay/traversal_switch_detector.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ struct traversal_switch_detector

enum isolation_type
{
isolation_unknown = -1,
isolation_no = 0,
isolation_yes = 1,
isolation_multiple = 2
Expand Down Expand Up @@ -121,7 +120,7 @@ struct traversal_switch_detector
struct region_properties
{
signed_size_type region_id = -1;
isolation_type isolated = isolation_unknown;
isolation_type isolated = isolation_no;
set_type unique_turn_ids;
connection_map connected_region_counts;
};
Expand Down Expand Up @@ -374,7 +373,7 @@ struct traversal_switch_detector
{
region_properties& properties = key_val.second;

if (properties.isolated == isolation_unknown
if (properties.isolated == isolation_no
&& has_only_isolated_children(properties))
{
properties.isolated = isolation_yes;
Expand All @@ -388,13 +387,36 @@ struct traversal_switch_detector
{
for (turn_type& turn : m_turns)
{
// For difference, for the input walked through in reverse,
// the meaning is reversed: what is isolated is actually not,
// and vice versa.
bool const reverseMeaningInTurn
= (Reverse1 || Reverse2)
&& ! turn.is_self()
&& ! turn.is_clustered()
&& uu_or_ii(turn)
&& turn.operations[0].enriched.region_id
!= turn.operations[1].enriched.region_id;

for (auto& op : turn.operations)
{
auto mit = m_connected_regions.find(op.enriched.region_id);
if (mit != m_connected_regions.end())
{
bool const reverseMeaningInOp
= reverseMeaningInTurn
&& ((op.seg_id.source_index == 0 && Reverse1)
|| (op.seg_id.source_index == 1 && Reverse2));

// It is assigned to isolated if it's property is "Yes",
// (one connected interior, or chained).
// "Multiple" doesn't count for isolation,
// neither for intersection, neither for difference.
region_properties const& prop = mit->second;
op.enriched.isolated = prop.isolated == isolation_yes;
op.enriched.isolated
= reverseMeaningInOp
? false
: prop.isolated == isolation_yes;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the fix

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first version did have prop.isolated == isolation_no but that is the default, we should not revert all of them into yes.

But it now does not look symmetrical, theoretically there should be cases where reversal should be necessary - but we don't have them currently in our test set.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you reverse one of the test cases? To make sure the other option also gets triggered by test suite?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We test all our difference operations in two ways, so both a - b and b - a . If that is what you mean.

Or you probably mean (what I call) inverse them, so make a large polygon around a polygon and make the polygon itself an interior of that. Yes, I could do that, I will try to construct a case where this should happen. I now know where I’m looking for. I will do that, but not today.

Thanks.

}
}
}
Expand Down Expand Up @@ -478,8 +500,12 @@ struct traversal_switch_detector
// Discarded turns don't connect rings to the same region
// Also xx are not relevant
// (otherwise discarded colocated uu turn could make a connection)
return ! turn.discarded
&& ! turn.both(operation_blocked);
return ! turn.discarded && ! turn.both(operation_blocked);
}

inline bool uu_or_ii(turn_type const& turn) const
{
return turn.both(operation_union) || turn.both(operation_intersection);
}

inline bool connects_same_region(turn_type const& turn) const
Expand All @@ -492,7 +518,7 @@ struct traversal_switch_detector
if (! turn.is_clustered())
{
// If it is a uu/ii-turn (non clustered), it is never same region
return ! (turn.both(operation_union) || turn.both(operation_intersection));
return ! uu_or_ii(turn);
}

if (BOOST_GEOMETRY_CONDITION(target_operation == operation_union))
Expand Down Expand Up @@ -565,10 +591,83 @@ struct traversal_switch_detector
}
}

#if defined(BOOST_GEOMETRY_DEBUG_TRAVERSAL_SWITCH_DETECTOR)
void debug_show_results()
Copy link
Collaborator Author

@barendgehrels barendgehrels Jul 14, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this shows quite a neat list of results and I would like to keep it.

For example

BEGIN SWITCH DETECTOR (region_ids and isolation) REVERSE_2
 ADD (s:0, m:0) TO REGION 1
 ADD (s:1, m:0) TO REGION 2
 ADD (s:1, m:1) TO REGION 3
END SWITCH DETECTOR
REGION 1 multiple [turns 0 1 2] regions { 2 : via [ 0 1 2 ] }
REGION 2 yes [turns 0 1 2 3] regions { 1 : via [ 0 1 2 ] } { 3 : via [ 3 ] }
REGION 3 yes [turns 3] regions { 2 : via [ 3 ] }
II 0 (50, 70) ->  [0/1] (1 false) / (2 false)
II 1 (70, 70) ->  [0/1] (1 false) / (2 false)
II 2 (70, 50) ->  [0/1] (1 false) / (2 false)
II 3 (50, 50) ->  [1/1] (2 true) / (3 true)

{
auto isolation_to_string = [](isolation_type const& iso) -> std::string
{
switch(iso)
{
case isolation_no : return "no";
case isolation_yes : return "yes";
case isolation_multiple : return "multiple";
}
return "error";
};
auto set_to_string = [](auto const& s) -> std::string
{
std::ostringstream result;
for (auto item : s) { result << " " << item; }
return result.str();
};

for (auto const& kv : m_connected_regions)
{
auto const& prop = kv.second;

std::ostringstream sub;
sub << "[turns" << set_to_string(prop.unique_turn_ids)
<< "] regions";
for (auto const& kvs : prop.connected_region_counts)
{
sub << " { " << kvs.first
<< " : via [" << set_to_string(kvs.second.unique_turn_ids)
<< " ] }";
}

std::cout << "REGION " << prop.region_id
<< " " << isolation_to_string(prop.isolated)
<< " " << sub.str()
<< std::endl;
}

for (std::size_t turn_index = 0; turn_index < m_turns.size(); ++turn_index)
{
turn_type const& turn = m_turns[turn_index];

if (uu_or_ii(turn) && ! turn.is_clustered())
{
std::cout << (turn.both(operation_union) ? "UU" : "II")
<< " " << turn_index
<< " (" << geometry::get<0>(turn.point)
<< ", " << geometry::get<1>(turn.point) << ")"
<< " -> " << std::boolalpha
<< " [" << turn.operations[0].seg_id.source_index
<< "/" << turn.operations[1].seg_id.source_index << "] "
<< "(" << turn.operations[0].enriched.region_id
<< " " << turn.operations[0].enriched.isolated
<< ") / (" << turn.operations[1].enriched.region_id
<< " " << turn.operations[1].enriched.isolated << ")"
<< std::endl;
}
}

for (auto const& key_val : m_clusters)
{
cluster_info const& cinfo = key_val.second;
std::cout << "CL RESULT " << key_val.first
<< " -> " << cinfo.open_count << std::endl;
}
}
#endif

void iterate()
{
#if defined(BOOST_GEOMETRY_DEBUG_TRAVERSAL_SWITCH_DETECTOR)
std::cout << "BEGIN SWITCH DETECTOR (region_ids and isolation)" << std::endl;
std::cout << "BEGIN SWITCH DETECTOR (region_ids and isolation)"
<< (Reverse1 ? " REVERSE_1" : "")
<< (Reverse2 ? " REVERSE_2" : "")
<< std::endl;
#endif

// Collect turns per ring
Expand Down Expand Up @@ -608,33 +707,7 @@ struct traversal_switch_detector

#if defined(BOOST_GEOMETRY_DEBUG_TRAVERSAL_SWITCH_DETECTOR)
std::cout << "END SWITCH DETECTOR" << std::endl;

for (std::size_t turn_index = 0; turn_index < m_turns.size(); ++turn_index)
{
turn_type const& turn = m_turns[turn_index];

if ((turn.both(operation_union) || turn.both(operation_intersection))
&& ! turn.is_clustered())
{
std::cout << (turn.both(operation_union) ? "UU" : "II")
<< " " << turn_index
<< " (" << geometry::get<0>(turn.point)
<< ", " << geometry::get<1>(turn.point) << ")"
<< " -> " << std::boolalpha
<< "(" << turn.operations[0].enriched.region_id
<< " " << turn.operations[0].enriched.isolated
<< ") / (" << turn.operations[1].enriched.region_id
<< " " << turn.operations[1].enriched.isolated << ")"
<< std::endl;
}
}

for (auto const& key_val : m_clusters)
{
cluster_info const& cinfo = key_val.second;
std::cout << "CL RESULT " << key_val.first
<< " -> " << cinfo.open_count << std::endl;
}
debug_show_results();
#endif

}
Expand Down
5 changes: 5 additions & 0 deletions include/boost/geometry/algorithms/detail/overlay/turn_info.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,11 @@ struct turn_info
{
return cluster_id > 0;
}
inline bool is_self() const
{
return operations[0].seg_id.source_index
== operations[1].seg_id.source_index;
}

private :
inline bool has12(operation_type type1, operation_type type2) const
Expand Down
Loading