Fix degenerate linestring relation on boundary of areal

[mjacobse]

Basically makes it so covered_by returns true for LINESTRING(0 0,0 0) and POLYGON((0 0, 0 2, 2 2, 2 0, 0 0)) instead of false as it did previously, which could be quite surprising.

Fixes #1473

The bit flags for interrupt aren't exactly pretty, but it follows the present implementation.

[barendgehrels]

Thanks for the PR, is it a bit larger than earlier? Anyway, that's not a problem, will continue the review later

[mjacobse]

Thanks for the PR, is it a bit larger than earlier? Anyway, that's not a problem, will continue the review later

Adding the handling of the interrupt flags for this case now made it a bit bigger yes

[tinko92]

Thank you for the PR. The changes look suitable to me for but the goal of this PR is but I have not yet run the full test suite locally.

I have two general questions on this approach:

1. Have you considered your approach against an implementation on the dispatch level, i.e. before the dispatch to the detail/relate/linear_areal code test the linestring for degeneracy and call the detail/relate/point_geometry code in case of degeneracy? This way may be easier and and more clear to wrap into a macro guard to allow restoring the old behaviour.
2. Related to this (and mentioned in another code comment) since we change behavior here, do we want a macro that can be defined to restore the old behavior?

[barendgehrels]

2. Related to this (and mentioned in another code comment) since we change behavior here, do we want a macro that can be defined to restore the old behavior?

I'm not sure if this is necessary for this fix.

[mjacobse]

1. Have you considered your approach against an implementation on the dispatch level, i.e. before the dispatch to the detail/relate/linear_areal code test the linestring for degeneracy and call the detail/relate/point_geometry code in case of degeneracy? This way may be easier and and more clear to wrap into a macro guard to allow restoring the old behaviour.

My concern with that was that checking the linestring for degeneracy didn't seem completely trivial. Checking boost::geometry::is_valid for failure_wrong_topological_dimension would have been my first approach which would be O(n) I guess? Probably negligible compared to what follows, but still some non-trivial extra code only to make some arguably out-of-contract behavior less surprising. On the other hand the present approach basically just handles a natural third case in an already present if statement. Seemed simpler, but I'd be happy to explore the dispatch level option if that's preferred.

[barendgehrels]

1. Have you considered your approach against an implementation on the dispatch level, i.e. before the dispatch to the detail/relate/linear_areal code test the linestring for degeneracy and call the detail/relate/point_geometry code in case of degeneracy? This way may be easier and and more clear to wrap into a macro guard to allow restoring the old behaviour.

My concern with that was that checking the linestring for degeneracy didn't seem completely trivial. Checking boost::geometry::is_valid for failure_wrong_topological_dimension would have been my first approach which would be O(n) I guess? Probably negligible compared to what follows, but still some non-trivial extra code only to make some arguably out-of-contract behavior less surprising. On the other hand the present approach basically just handles a natural third case in an already present if statement. Seemed simpler, but I'd be happy to explore the dispatch level option if that's preferred.

In general we dispatch by type, not by contents. So I think the choice made here is the correct one.

[barendgehrels]

Thanks for the improvement!

[tinko92]

In general we dispatch by type, not by contents.

Sorry, I was imprecise, I was thinking of right after the type dispatch, to keep it more easily separable by a macro if details change later. But if we don't need a macro for to preserve the prior behaviour as an option, then that's moot. I approve the current state (with the release calendar in mind in case you want to merge it for 1.92).

[barendgehrels]

@mjacobse is this PR as is OK?
@tinko92 @vissarion will we merge this as is?

I don't think @mjacobse can, so we'll have to take care

[mjacobse]

OK for me, yes 👍

[vissarion]

I am OK with merging, thanks!
Should we add it in 1.92? If so @mjacobse feel free to add an item in release notes in this PR, otherwise I will open a new PR with this addition.