turf-boolean-contains + turf-boolean-within: Fix line in polygon

[samuelarbibe]

This PR fixes a false-positive when running boolean-contains (or boolean-within) of lineString inside a polygon, as demonstrated in #2441.

The current implementation of boolean-contains checks if all the midpoints of the line segments are inside the polygon, and results in a false-positive in the following cases:

This PR proposes the following logic:

1. The line and polygon's bboxes must overlap. (optimization)
2. All line points must be contained by the polygon.
3. If the line intersects with polygon - all segments must be contained by the polygon. At least one of them should be fully contained by the polygon. (optimization - midpoint is sufficient)

This "waterfall" approach makes sure no extra computation is performed - unless necessary. Only quirky cases will require the extra computation of step 3 (In most cases, a single iteration of step 3 will suffice).
The only exception is that the line's points are checked to be contained instead of the midpoints - but it returns false-positives for pretty basic cases, so I find this to be a reasonable compromise.

A test case for the second image has been added.

Resolves #2441
Resolves #2049
Resolves #1443

[Janickvw]

Hey @samuelarbibe i've found an issue with this fix. I'll test out a fix and get back to you.

[samuelarbibe]

Hey @samuelarbibe i've found an issue with this fix. I'll test out a fix and get back to you.

I'm curious. Let me know :)

[Janickvw]

@samuelarbibe I think you focused too much on advanced use cases and forgot the simple ones 😂
This implementation fails for this use case:

Sandbox link: https://turf-sandbox.netlify.app/?gist=6ce3e4e6d674e6dd9c92c5265b6a25e0

The issue is this line not returning any segments when there are no intersections.

const lineSegments = turf.lineSplit(turf.feature(linestring), turf.feature(polygon));

Here is a potential fix:

// ...
  const lineSegmentsOrLineString = lineSegments.features.length > 0 ? lineSegments.features : [turf.feature(linestring)];

  for (const lineSegment of lineSegmentsOrLineString) {
    const midpoint = turf.midpoint(
      lineSegment.geometry.coordinates[0],
      lineSegment.geometry.coordinates[1]
    );
// ...

Fix sandbox: https://turf-sandbox.netlify.app/?gist=57378d936e774ac353de86e975060bf9

[Janickvw]

As commented above

[samuelarbibe]

Thank you.... got too focused on the complicated cases 😅.
Implemented a fix similar to you suggestion, with a little fix (The problem is that if the line sting has 2 or more vertices on the boundary of the polygon, the midpoint might be contained by the polygon, so I split the linestring into multiple linestring, one per segment).

[smallsaucepan]

Looks really good thanks @samuelarbibe. Have asked for a few short code comments here and there, and some rewording of the documentation (will put in the PR conversation).

[smallsaucepan]

Would you mind doing some rewording of the documentation while you're here? The "exact opposite result" phrasing isn't very satisfying (and I'm not sure it's true anyway if taken literally).

From

Boolean-contains returns True if the second geometry is completely contained by the first geometry. The interiors of both geometries must intersect and, the interior and boundary of the secondary (geometry b) must not intersect the exterior of the primary (geometry a). Boolean-contains returns the exact opposite result of the @turf/boolean-within.

returns true/false

To

Tests whether geometry a contains geometry b. The interiors of both geometries must intersect, and the interior and boundary of geometry b must not intersect the exterior of geometry a.

booleanContains(a, b) is equivalent to booleanWithin(b, a)

returns true if geometry a contains geometry b, false otherwise

Happy to hear it if you have other suggestions as well!

[smallsaucepan]

Awesome. Thanks @samuelarbibe 👍

[smallsaucepan]

Thanks for your work on this too @Janickvw ⭐