Geometry intersection with invalid geometry does not hard error and returns a null

[markgis]

We need to clip large polygons with a bounding box of an area of interest. We were using Gdal bindings from the old lib and it worked fine. Occasionally geometry issues (self touching) are leading to soft errors where data is missing and gdal-next is not throwing an error.

We can use isValid to check before running an intersection, however throwing an error would be preferable.

Old Gdal bindings can return a legit intersection on two geometries even with geometry invalid (self touching polygon).

Gdal-next returns a Null rather than throwing an error. It would be preferable that an error would be thrown. However, is there a reason why gdal-next is being more fussy on geometry validation? Or is that a change to the underlying C lib?

Have created a quick Gist as an example.

https://gist.github.com/markgis/2218363320779f8239d3cb9459650aa6

Gdal returning intersection of both geometries.

Self touching geometry issue causing Null return in gdal-next

@tickticktrack

[yocontra]

There weren't any changes in the binding code related to validation or intersections so I would assume this is just a change in the underlying gdal behavior - it may be possible to return the old behavior but I'll have to check the gdal changelog and see what changed here + add some tests.

[yocontra]

As a test, you can try installing gdal-next in shared mode + make sure you have an older version of gdal (maybe 2.x? an early 3.x?) on your local. If you get the same results then the issue is in our binding code - if you get the expected results on the older version, then the issue is with a change in gdal.

Let me know what you find out since that can save me quite a bit of time debugging.

[markgis]

@contra Sorry about the late reply, I am guessing you mean install from gdal source? as in;

To link against shared libgdal, install using:

# requires libgdal-dev (debian: sudo apt-get install libgdal-dev) 
$ npm install gdal --build-from-source --shared_gdal

[yocontra]

@markgis Yeah - with that you're building it from scratch and telling it to link against a version of GDAL on your system, not the one included in the repo. If your issue is fixed by this it means the breakage is with the version in our repo vs. an older version that your system has and gives us a good place to start looking. Let me know what the result is + which version of GDAL you linked against.

[mmomtchev]

@markgis, there have been very significant changes in GDAL 3 when it comes to geometry operations (they added 3D support based on SFCGAL) and the libgeom (which provides the intersection algorithm) support has been reworked.

As far as I am concerned, the correct response is the one provided by GDAL 3. The old one was, in fact, a bug. As the GDAL documentation states, you have to call geom.isValid() before calling geom.intersection(), otherwise the result is undefined. If you call geom.isValid() on your gdalGeom / nextGeom it will return false with gdal.lastError set to 'Ring Self-intersection at or near point 311405 682025'

As for modifying the API, even if I have to admit that what you are proposing makes sense and it is more in line with the JS design principles than the current solution, I am afraid that this will be a decision where disadvantages far out weight the benefits. For an incompatible API change to happen, the old API must really be broken beyond repair.

What I suggest is that you simply do the following in your code:

const gdalGeomIntersection = gdal.Geometry.prototype.intersection;
gdal.Geometry.prototype.intersection = function(geom) {
  if (!this.isValid() || !geom.isValid())
   throw new Error('invalid geom');
  return gdalGeomIntersection.call(this, geom);
}

If you want to support different GDAL versions, you can check the value of gdal.version - there is code in the async unit tests that does it

[markgis]

Hey @mmomtchev, thanks for the details. We had already implemented a isValid but thought it was worth bringing up the difference in behaviour. Many thanks for checking that out.

[yocontra]

I think this is something worth noting in the docs (Behavior differences between GDAL 2 vs. GDAL 3) since this library supports both.