Skip to content

Conversation

@vissarion
Copy link
Member

No description provided.

@vissarion vissarion requested a review from barendgehrels July 26, 2024 14:38
(turn.operations[0].seg_id.segment_index == 0
&& turn.operations[1].seg_id.segment_index == last_index);
return turn.operations[0].seg_id.segment_index == 0
&& turn.operations[1].seg_id.segment_index == last_index;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think what was meant here was that the second condition had 1 vs 0 instead.
But it's hard to guess. I didn't write it (I think). @awulkiew ?

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, I kept the second condition with reversed 0 and 1.

Out of curiosity, is it possible for the second condition to be true in the way turns are computed in boost geometry?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, I think so. But maybe better to check that before merging it.
I'll try to check one of the coming days.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@vissarion I forgot, and don't have the time to do it now in detail (it would require a quite specific testcase).
The operations are symmetric (have the same role) and unordered, so I'm quite sure it's right now.
Feel free to merge it.

Copy link
Collaborator

@barendgehrels barendgehrels left a comment

Choose a reason for hiding this comment

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

Thanks! One question but good looks

@vissarion vissarion added this to the 1.87 milestone Aug 14, 2024
@vissarion vissarion merged commit 26fb3e5 into boostorg:develop Sep 30, 2024
@vissarion vissarion deleted the fix/remove_unused branch September 30, 2024 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants