<div dir="ltr">Looks good. The GEOSwift test suite passes with your change. Thanks!</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Nov 30, 2020 at 4:13 PM Paul Ramsey <<a href="mailto:pramsey@cleverelephant.ca">pramsey@cleverelephant.ca</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Thanks for this report! Would you mind trying out the fix here:<br>
<br>
<a href="https://github.com/libgeos/geos/pull/360" rel="noreferrer" target="_blank">https://github.com/libgeos/geos/pull/360</a><br>
<br>
P.<br>
<br>
> On Nov 28, 2020, at 6:33 PM, Andrew Hershberger <<a href="mailto:andrew.d.hershberger@gmail.com" target="_blank">andrew.d.hershberger@gmail.com</a>> wrote:<br>
> <br>
> Hi all, I help maintain <a href="https://github.com/GEOSwift/GEOSwift" rel="noreferrer" target="_blank">https://github.com/GEOSwift/GEOSwift</a><br>
> <br>
> I'm testing out 3.9.0beta1 and have run into a couple things to share.<br>
> <br>
> If you want to try any of these out directly with GEOSwift, I recommend using the Swift Package Manager. You can clone <a href="https://github.com/GEOSwift/GEOSwift.git" rel="noreferrer" target="_blank">https://github.com/GEOSwift/GEOSwift.git</a> and check out branch geos-3.9.0-testing. Then run `$ swift test` to build and run the tests. I'm using Swift 5.3.1. One way to get Swift is to use the Docker image: <a href="https://hub.docker.com/_/swift" rel="noreferrer" target="_blank">https://hub.docker.com/_/swift</a><br>
> <br>
> 1) GeometryConvertible_GEOSTests.testMakeValidWhenItIsAPolygon()<br>
> <br>
> This test is checking that makeValid() (a wrapper around the CAPI's GEOSMakeValid_r) produces an expected result given some pre-defined geometry. Here's what happened:<br>
> <br>
> Input:<br>
> <br>
> Polygon(<br>
> exterior: LinearRing(<br>
> points: [<br>
> Point(x: 0, y: 0),<br>
> Point(x: 2, y: 0),<br>
> Point(x: 1, y: 1),<br>
> Point(x: 0, y: 2),<br>
> Point(x: 2, y: 2),<br>
> Point(x: 1, y: 1),<br>
> Point(x: 0, y: 0)<br>
> ]<br>
> )<br>
> )<br>
> <br>
> Actual (wrapped in a multipolygon)<br>
> <br>
> Polygon(<br>
> exterior: LinearRing(<br>
> points: [<br>
> Point(x: 0, y: 2),<br>
> Point(x: 2, y: 2),<br>
> Point(x: 1, y: 1),<br>
> Point(x: 0, y: 2)<br>
> ]<br>
> )<br>
> )<br>
> <br>
> Polygon(<br>
> exterior: LinearRing(<br>
> points: [<br>
> Point(x: 2, y: 0),<br>
> Point(x: 0, y: 0),<br>
> Point(x: 1, y: 1),<br>
> Point(x: 2, y: 0)<br>
> ]<br>
> )<br>
> )<br>
> <br>
> Expected (wrapped in any order in a multipolygon)<br>
> <br>
> Polygon(<br>
> exterior: LinearRing(<br>
> points: [<br>
> Point(x: 1, y: 1),<br>
> Point(x: 2, y: 0),<br>
> Point(x: 0, y: 0),<br>
> Point(x: 1, y: 1)<br>
> ]<br>
> )<br>
> )<br>
> <br>
> Polygon(<br>
> exterior: LinearRing(<br>
> points: [<br>
> Point(x: 1, y: 1),<br>
> Point(x: 0, y: 2),<br>
> Point(x: 2, y: 2),<br>
> Point(x: 1, y: 1)<br>
> ]<br>
> )<br>
> )<br>
> <br>
> Since these are topologically equivalent, I've just updated the test to check for that instead of exact equality (see GEOSwift commit 6c4b191646623159528433fbd112c11ddce7c5ea on the aforementioned branch), but I thought I'd share here just in case this behavior is unexpected.<br>
> <br>
> 2) GeometryConvertible_GEOSTests.testBufferAllTypes()<br>
> <br>
> This test is intended to be a sort of stress-test to make sure that all the types of geometry on which GEOSwift allows you to invoke buffer(by:) (wrapper around the CAPI's GEOSBuffer_r) can actually be buffered. In geos 3.8.1, all the sample geometries were able to be buffered without any issues.<br>
> <br>
> Here's what we get for one of the test cases with 3.9.0beta1:<br>
> <br>
> Input:<br>
> <br>
> GeometryCollection(<br>
> geometries: [<br>
> GeometryCollection(<br>
> geometries: [<br>
> Point(x: 1.0, y: 2.0),<br>
> MultiPoint(<br>
> points: [<br>
> Point(x: 1.0, y: 2.0),<br>
> Point(x: 3.0, y: 4.0)<br>
> ]<br>
> ),<br>
> LineString(<br>
> points: [<br>
> Point(x: 1.0, y: 2.0),<br>
> Point(x: 3.0, y: 4.0)<br>
> ]<br>
> ),<br>
> MultiLineString(<br>
> lineStrings: [<br>
> LineString(<br>
> points: [<br>
> Point(x: 1.0, y: 2.0),<br>
> Point(x: 3.0, y: 4.0)<br>
> ]<br>
> ),<br>
> LineString(<br>
> points: [<br>
> Point(x: 5.0, y: 6.0),<br>
> Point(x: 7.0, y: 8.0)<br>
> ]<br>
> )<br>
> ]<br>
> ),<br>
> Polygon(<br>
> exterior: LinearRing(<br>
> points: [<br>
> Point(x: 2.0, y: 2.0),<br>
> Point(x: -2.0, y: 2.0),<br>
> Point(x: -2.0, y: -2.0),<br>
> Point(x: 2.0, y: -2.0),<br>
> Point(x: 2.0, y: 2.0)<br>
> ]<br>
> ),<br>
> holes: [<br>
> LinearRing(<br>
> points: [<br>
> Point(x: 1.0, y: 1.0),<br>
> Point(x: 1.0, y: -1.0),<br>
> Point(x: -1.0, y: -1.0),<br>
> Point(x: -1.0, y: 1.0),<br>
> Point(x: 1.0, y: 1.0)<br>
> ]<br>
> )<br>
> ]<br>
> ),<br>
> MultiPolygon(<br>
> polygons: [<br>
> Polygon(<br>
> exterior: LinearRing(<br>
> points: [<br>
> Point(x: 2.0, y: 2.0),<br>
> Point(x: -2.0, y: 2.0),<br>
> Point(x: -2.0, y: -2.0),<br>
> Point(x: 2.0, y: -2.0),<br>
> Point(x: 2.0, y: 2.0)<br>
> ]<br>
> ),<br>
> holes: [<br>
> LinearRing(<br>
> points: [<br>
> Point(x: 1.0, y: 1.0),<br>
> Point(x: 1.0, y: -1.0),<br>
> Point(x: -1.0, y: -1.0),<br>
> Point(x: -1.0, y: 1.0),<br>
> Point(x: 1.0, y: 1.0)<br>
> ]<br>
> )<br>
> ]<br>
> ),<br>
> Polygon(<br>
> exterior: LinearRing(<br>
> points: [<br>
> Point(x: 7.0, y: 2.0),<br>
> Point(x: 3.0, y: 2.0),<br>
> Point(x: 3.0, y: -2.0),<br>
> Point(x: 7.0, y: -2.0),<br>
> Point(x: 7.0, y: 2.0)<br>
> ]<br>
> ),<br>
> holes: []<br>
> )<br>
> ]<br>
> )<br>
> ]<br>
> )<br>
> ]<br>
> )<br>
> <br>
> buffer(by: 0.5)<br>
> <br>
> errorMessage: "IllegalArgumentException: Overlay input is mixed-dimension"<br>
> <br>
> The exception is being thrown from EdgeNodingBuilder.cpp:195.<br>
> <br>
> In frame #2, i = 0<br>
> <br>
> In frame #0, i = 0<br>
> <br>
> This suggests to me that it's checking the dimension of the Point inside of the child GeometryCollection inside of the parent GeometryCollection.<br>
> <br>
> g->getDimension() is returning 0 (geos::geom::Dimension::DimensionType P) and expectedDim is 2.<br>
> <br>
> Here's a relevant stack trace:<br>
> <br>
> #0 geos::operation::overlayng::EdgeNodingBuilder::addGeometryCollection(geos::geom::GeometryCollection const*, int, int) at geos/src/operation/overlayng/EdgeNodingBuilder.cpp:195<br>
> #1 geos::operation::overlayng::EdgeNodingBuilder::add(geos::geom::Geometry const*, int) at geos/src/operation/overlayng/EdgeNodingBuilder.cpp:169<br>
> #2 geos::operation::overlayng::EdgeNodingBuilder::addGeometryCollection(geos::geom::GeometryCollection const*, int, int) at geos/src/operation/overlayng/EdgeNodingBuilder.cpp:197<br>
> #3 geos::operation::overlayng::EdgeNodingBuilder::add(geos::geom::Geometry const*, int) at geos/src/operation/overlayng/EdgeNodingBuilder.cpp:169<br>
> #4 geos::operation::overlayng::EdgeNodingBuilder::build(geos::geom::Geometry const*, geos::geom::Geometry const*) at geos/src/operation/overlayng/EdgeNodingBuilder.cpp:82<br>
> #5 geos::operation::overlayng::OverlayNG::computeEdgeOverlay() at geos/src/operation/overlayng/OverlayNG.cpp:222<br>
> #6 geos::operation::overlayng::OverlayNG::getResult() at geos/src/operation/overlayng/OverlayNG.cpp:179<br>
> #7 geos::operation::overlayng::PrecisionReducer::reducePrecision(geos::geom::Geometry const*, geos::geom::PrecisionModel const*, bool) at geos/src/operation/overlayng/PrecisionReducer.cpp:46<br>
> #8 geos::precision::GeometryPrecisionReducer::reduce(geos::geom::Geometry const&) at geos/src/precision/GeometryPrecisionReducer.cpp:60<br>
> #9 geos::precision::GeometryPrecisionReducer::reduce(geos::geom::Geometry const&, geos::geom::PrecisionModel const&) at geos/include/geos/precision/GeometryPrecisionReducer.h:102<br>
> #10 geos::operation::buffer::BufferOp::bufferFixedPrecision(geos::geom::PrecisionModel const&) at geos/src/operation/buffer/BufferOp.cpp:263<br>
> #11 geos::operation::buffer::BufferOp::bufferReducedPrecision(int) at geos/src/operation/buffer/BufferOp.cpp:218<br>
> #12 geos::operation::buffer::BufferOp::bufferReducedPrecision() at geos/src/operation/buffer/BufferOp.cpp:168<br>
> #13 geos::operation::buffer::BufferOp::computeGeometry() at geos/src/operation/buffer/BufferOp.cpp:150<br>
> #14 geos::operation::buffer::BufferOp::getResultGeometry(double) at geos/src/operation/buffer/BufferOp.cpp:122<br>
> #15 geos::operation::buffer::BufferOp::bufferOp(geos::geom::Geometry const*, double, int, int) at geos/src/operation/buffer/BufferOp.cpp:114<br>
> #16 geos::geom::Geometry::buffer(double, int) const at geos/src/geom/Geometry.cpp:509<br>
> #17 GEOSBuffer_r::$_41::operator()() const at geos/capi/geos_ts_c.cpp:1078<br>
> #18 _Z7executeIZ12GEOSBuffer_rE4$_41LDn0EEDTclfp0_EEP20GEOSContextHandle_HSOT_ at geos/capi/geos_ts_c.cpp:388<br>
> #19 ::GEOSBuffer_r(GEOSContextHandle_t, const geos::geom::Geometry *, double, int) at geos/capi/geos_ts_c.cpp:1077<br>
> #20 GeometryConvertible.buffer(by:) at GEOSwift/GEOSwift/GEOS/GeometryConvertible+GEOS.swift:348<br>
> #21 GeometryConvertible_GEOSTests.testBufferAllTypes() at GEOSwift/GEOSwiftTests/GEOS/GeometryConvertible+GEOSTests.swift:856<br>
> <br>
> Let me know what other info I can provide.<br>
> <br>
> --<br>
> Andrew<br>
> _______________________________________________<br>
> geos-devel mailing list<br>
> <a href="mailto:geos-devel@lists.osgeo.org" target="_blank">geos-devel@lists.osgeo.org</a><br>
> <a href="https://lists.osgeo.org/mailman/listinfo/geos-devel" rel="noreferrer" target="_blank">https://lists.osgeo.org/mailman/listinfo/geos-devel</a><br>
<br>
_______________________________________________<br>
geos-devel mailing list<br>
<a href="mailto:geos-devel@lists.osgeo.org" target="_blank">geos-devel@lists.osgeo.org</a><br>
<a href="https://lists.osgeo.org/mailman/listinfo/geos-devel" rel="noreferrer" target="_blank">https://lists.osgeo.org/mailman/listinfo/geos-devel</a><br>
</blockquote></div>