Update boolean-point-in-polygon to use point-in-polygon-hao library

[rowanwins]

• Use a meaningful title for the pull request. Include the name of the package modified.
• Have read How To Contribute.
• Run npm test at the sub modules where changes have occurred.
• Run npm run lint to ensure code style at the turf module level.

This PR updates boolean-point-in-polygon module to use the point-in-polygon-hao library which is more robust and faster than the current implementation.

I'm also wondering if we want to tweak the API a little - we currently have an option called ignoreBoundary which modifies how we handle the result. However I'm wondering if we should actually provide an option that allows us to report a point lying on the boundary because there may be scenarios where that is helpful to know, however it seems. a bit clunky to have another option thats called reportOnBoundary... so I'm open to ideas on if and how we might tackle that. Any changes here might also be potentially breaking so we'd need to consider that...

Benchmarks

// This PR
simple x 16,449,144 ops/sec ±1.45% (88 runs sampled)
multiPolyHole - inside x 7,624,379 ops/sec ±2.08% (89 runs sampled)
multiPolyHole - outside x 88,233,389 ops/sec ±1.70% (87 runs sampled)

// OLD
simple x 9,041,375 ops/sec ±2.02% (90 runs sampled)
multiPolyHole - inside x 3,853,073 ops/sec ±0.61% (88 runs sampled)
multiPolyHole - outside x 72,504,154 ops/sec ±0.47% (92 runs sampled)

[mfedderly]

Code looks cleaner than when you started! Perf benefits are nice as well.

Is the boundary stuff better served by explicitly asking for point-on-ring? Or is there a reason to do it at the same time as the polygon? In any case if we wanted to be mindful of API breaks we could file a ticket, give it a breaking label, and assign it to the next major milestone (which might require some clean up of the existing v7 stuff).

Does changing ignoreBoundary to something like boundary = "in" | "out" = "in" and possibly changing the return value to 0/1/2 depending on the boundary. Still castable to a boolean easily. I might be misunderstanding the details of the boundary thing.

[rowanwins]

I reckon let's leave the API change till later until we've got a bit more of a rhythm and we might identify other breaking changes to tackle at the same time.

Re a seperate point-on-ring module we already have booleanPointOnLine which is pretty much the same.

[kaligrafy]

Any news on this? I don't see why it was not merged already since the current implementation does not detect boundary nodes correctly and is slower...

[mfedderly]

This just needs a new release from the point-in-polygon-hao module for rowanwins/point-in-polygon-hao#14 and then it should be ready to merge.

[rowanwins]

Just released v1.1.0 for point-in-polygon-hao

[mfedderly]

@rowanwins any idea why this does not address #1755 and #2048? They both have points that appear to not be in their polygons, but they return true on the current version of this branch.

Here's the two tickets as tests:

test("boolean-point-in-polygon -- issue #1755", (t) => {
  const pt = point([18.957922, 61.5439]);
  const poly = polygon([
    [
      [42.14295462, 71.62860768],
      [37.39686087, 54.86312041],
      [-10.59142038, 71.40570518],
      [42.14295462, 71.62860768],
    ],
  ]);
  t.false(booleanPointInPolygon(pt, poly));
  t.end();
});

test("boolean-point-in-polygon -- issue #2048", (t) => {
  const pt = point([-84.44876, 42.79741]);
  const poly = polygon([
    [
      [-86.3559, 41.2203],
      [-69.9985, 54.7459],
      [-69.9782, 54.7441],
      [-69.9725, 54.7484],
      [-69.9672, 54.7496],
      [-69.9514, 54.758],
      [-69.9469, 54.7625],
      [-69.9457, 54.7669],
      [-69.9458, 54.7736],
      [-69.9479, 54.7769],
      [-69.9546, 54.7777],
      [-69.9612, 54.775],
      [-69.9644, 54.7769],
      [-69.9644, 54.7803],
      [-69.9553, 54.7926],
      [-69.9587, 54.793],
      [-86.3559, 41.2203],
    ],
  ]);
  t.false(booleanPointInPolygon(pt, poly));
  t.end();
});

[rowanwins]

@mfedderly this is one of the quirks of projecting coords onto a map using WebMercator. Attached is a pic showing the first case from a planar geometry viewer

QGIS also shows similar when rendering the points using WGS84.

The existing implementation in master returns the same thing so I don't think we've got any regression.

I must admit you had me very concerned for 20 mins. 🎢 of emotions like 😭 then 😌
😆

[mfedderly]

@rowanwins I'm going to get this merged. If you want to add more behavior changes to ignoreBoundary/reportOnBoundary before 7.0.0 releases, either file a PR, or an issue in the 7.0 milestone.