Mitre buffer returns invalid geometry

[mwtoews]

This appears to be a minor robustness corner case with a miter buffer using the following shapely script:

from shapely import geos, wkt

print("geos", geos.geos_version_string)

A_wkt = """\
    POLYGON ((-23.989123360549296 73.1287474328027,
              -22.537997105552297 94.06368412079055,
              -18.796973600895146 93.80437130274495,
              -17.80121237894408 108.16990157009043,
              -21.542235883601226 108.42921438813606,
              -20.967403753721864 116.7221345967023,
              -4.728530705460814 116.7221568196225,
              -7.82790182044367 72.00851605865441,
              -23.989123360549296 73.1287474328027))"""

# this is the only invalid buffer example
A = wkt.loads(A_wkt)
B = A.buffer(10, join_style=2)
print("main bug:", B.is_valid)

# the remaining examples are valid
A = wkt.loads(A_wkt)
B = A.buffer(10 + 1e-14, join_style=2)
print("small buffer distance adjustment:", B.is_valid)

Ap = wkt.loads(wkt.dumps(A, rounding_precision=14))
B = Ap.buffer(10, join_style=2)
print("rounding_precision=14:", B.is_valid)

Note that the error only happens in the machine epsilon (1e-14) scale of floats, suggesting a robustness issue. While I haven't tested all GEOS versions, I see the same behaviour with 3.9.1, so it isn't new. I don't see the behaviour with JTS, but here is a JTS figure showing valid input A in blue and invalid B in red.

Originally reported in shapely/shapely#1792

[pramsey]

In general we aren't guaranteeing valid output from Buffer, though that might change if/when it gets moved off the old graph structure and onto the NG foundation. However, no great hurry on that. As mentioned in the other similar ticket, we could post-check validity and make valid, but... actually in this case you'd end up with a spurious hole, so the valid output wouldn't actually be any better.

[pramsey]

Still seeing this? I just tried it directly on main, and it passed.

// Buffer produces invalid output
// https://github.com/libgeos/geos/issues/856
template<>
template<>
void object::test<23>()
{
    auto geom = wktreader.read("POLYGON ((-23.989123360549296 73.1287474328027, -22.537997105552297 94.06368412079055,-18.796973600895146 93.80437130274495,-17.80121237894408 108.16990157009043,-21.542235883601226 108.42921438813606,-20.967403753721864 116.7221345967023,-4.728530705460814 116.7221568196225,-7.82790182044367 72.00851605865441,-23.989123360549296 73.1287474328027))");

    BufferParameters bp;
    bp.setJoinStyle(BufferParameters::JOIN_MITRE);
    BufferOp op(geom.get(), bp);

    auto result = op.getResultGeometry(10);
    ensure(result->isValid());
}

[mwtoews]

Running a bunch of bisections, I can see that this was resolved with bf18ba8 (#937).
Possible actions are to add this test (tests/unit/operation/buffer/BufferParametersTest.cpp) to main, consider any backports, and/or simply close this issue as resolved only for next release.

[pramsey]

Huh, it's a relatively small change, back-porting a couple versions would probably be good, IMO.

[mwtoews]

Closed via backports and the above test (only on main with 0a8fc37).

The backports are made back to 3.9, when DoubleDouble math ops were added (bed36f1).
Here are the backport branch commit details:

• 3.12 via 037ccb9, 540a6aa
• 3.11 via 1d75f8f, 04e7cbe
• 3.10 via 6d0c831, 078d549
• 3.9 via c7769ea, d1d221f