Re-enabling negative buffers #736
Conversation
rafaelalmeida
force-pushed
the
reenabling-negative-buffers
branch
from
361c73f
to
5a96f93
Compare
| @@ -38,8 +38,8 @@ var distanceToRadians = helpers.distanceToRadians; | |||
| module.exports = function (geojson, radius, units, steps) { | |||
| // validation | |||
| if (!geojson) throw new Error('geojson is required'); | |||
| if (!radius) throw new Error('radius is required'); | |||
| if (radius <= 0) throw new Error('radius must be greater than 0'); | |||
There was a problem hiding this comment.
Ops! @ralmeidastrider You are correct, this should allow positive or negative radius (didn't know jsts was able to support this). We should also include this in the test cases.
👍
|
@ralmeidastrider Was the equidistance change beneficial or did this break any production code? From my point of view this was a bug. 👍 I agree about the re-enabling buffer (backfired attempt to add better input validation). |
|
👍 I can confirm that negative buffers do work now. Blue: Input GeoJSON |
|
@DenisCarriere We hadn't had a chance to test the equidistance change itself since we could not do the negative buffer, unfortunately. Since our use case is a morphological closing (positive buffer followed by negative buffer of the same amount), we're just "gluing" close polygons together, this particular change should make little difference to us. I can definitely see the use in general, however! |
|
Thanks for your input, might be a good idea to include this morphological closing into the test cases. I doubt the equidistance change would impact this type of behavior. |
Pull Request #718 introduced a very welcome improvement in the
turf-buffermodule with buffered polygons being equidistant in all directions.However, one particular change, which I've traced exactly to the line ce8bac8#diff-dfff186196a7762bb1ab6352156dd94fR42, removes the ability to do negative buffers directly at API entrypoint level.
As I understand it,
turfuses thejstsmodule to really compute the buffer. I have inspected this module and it does seem to support negative buffers without any problem, so I see no reason to restrictturf-bufferfrom calculating negative buffers.This change actually broke some production code which had been running for a couple weeks with no issues, which did two buffer operations to do a morphological closing (a dilation followed by an erosion).
Besides, negative buffers are important morphological operations in GIS, to the point of Wikipedia plain mentioning their existence in the relevant entry: https://en.wikipedia.org/wiki/Buffer_(GIS)
I've also took the liberty of also allowing zero-sized buffers, which can be used as a 'repair geometry' operation (see this SO link for example - it's related to a Python lib, but this trick works with other buffer implementations as well).
I removed the line and ran the tests again. Since all pass, I'm submitting this Pull Request for the community to evaluate.