Buffer fixes #938
Buffer fixes #938
Conversation
|
@stebogit Wow 🎉 Really cool! The |
| var needsTransverseMercator = bbox[1] > 50 && bbox[3] > 50; | ||
|
|
||
| if (needsTransverseMercator) { | ||
| var projection = defineProjection(geometry); |
There was a problem hiding this comment.
@stebogit Did you want to move over this defineProjection to @turf/projection (as define).
/**
* Define Transverse Mercator projection
*
* @private
* @param {Geometry|Feature<any>} geojson Base projection on center of GeoJSON
* @returns {GeoProjection} D3 Geo Transverse Mercator Projection
*/
function defineProjection(geojson) {
var coords = center(geojson).geometry.coordinates.reverse();
var rotate = coords.map(function (coord) { return -coord; });
var projection = d3.geoTransverseMercator()
.center(coords)
.rotate(rotate)
.scale(6373000);
return projection;
}There was a problem hiding this comment.
@DenisCarriere wouldn't it be even more useful having a @turf/projection.toTransverse method for the actual conversion? I mean, why would you define the (Transverse in this case) projection but for converting the coordinates?
In that regard, at this point we might need to re-think the structure of the projection module to leave it open to accept maybe other projections in the future, if ever needed, like:
@turf/projection.toMercator@turf/projection.fromMercator@turf/projection.toTransverse@turf/projection.fromTransverse- ...
knowing the base projection for Turf is WGS84
|
Ooops... 😨 I was convinced this branch had been merged already... 😅 |
|
Lol it can be, I only looked at it real quick, when I have more time I'll dive into it more, unless you feel it's good to merge. |
|
At this point I guess we could merge since all tests pass and they actually look better (less distortion); we can always reopen any issue if necessary. |
|
@stebogit merge away! 👍 |
|
Woo-hoo!! |
|
Hello @stebogit, |
|
@ArtCodeKarine what module are you talking about? Do you mean the updated |
|
@ArtCodeKarine Just published a patch release for $ npm install --save @turf/[email protected] |
|
@DenisCarriere @stebogit thanks, I will try it :D |
|
I think i have to wait the new version of turf because my application is running with nodejs and bower. Bower uses the @turf/turf folder and this doesn't contain the patch version of the buffer. |
The existing buffer code has three problems: - it uses two different projections depending on the latitude of the input geometry. This results in an error discontinuity at 50N. - it switches projections only for the northern hemisphere. - it mistakenly reverses the lonlat center, which has the eventual result of rotating the projection to a skewed center. The third problem is one of the causes of `buffer`'s size issues. At latitudes above 50N, the projection distorts input distances due to not being rotated to the correct input geometry center. At latitudes below 50N, the Mercator projection does not receive a correction for the input distance. If using the Mercator projection, the actual distance used as an input to JSTS BufferOp should be distance / cos((lat * π) / 180). [1] As recommended in the original[2] exploration that prompted the fix introduced in Turfjs#938, this patch switches the buffer prep code to consistently using the Azimuthal Equidistant projection rather than switching between Mercator and Transverse Mercator. When centered on the input geometry, this projection should produce less distortion in all directions for input geometries of reasonable size compared to a cylindrical one. I took the opportunity to optimize slightly; buffering now calls `center` only once, and we don't need to `bbox` the input geom at all. [1] https://gis.stackexchange.com/a/347573 [2] https://github.com/w8r/moscow-rings#solution
) The existing buffer code has three problems: - it uses two different projections depending on the latitude of the input geometry. This results in an error discontinuity at 50N. - it switches projections only for the northern hemisphere. - it mistakenly reverses the lonlat center, which has the eventual result of rotating the projection to a skewed center. The third problem is one of the causes of `buffer`'s size issues. At latitudes above 50N, the projection distorts input distances due to not being rotated to the correct input geometry center. At latitudes below 50N, the Mercator projection does not receive a correction for the input distance. If using the Mercator projection, the actual distance used as an input to JSTS BufferOp should be distance / cos((lat * π) / 180). [1] As recommended in the original[2] exploration that prompted the fix introduced in #938, this patch switches the buffer prep code to consistently using the Azimuthal Equidistant projection rather than switching between Mercator and Transverse Mercator. When centered on the input geometry, this projection should produce less distortion in all directions for input geometries of reasonable size compared to a cylindrical one. I took the opportunity to optimize slightly; buffering now calls `center` only once, and we don't need to `bbox` the input geom at all. [1] https://gis.stackexchange.com/a/347573 [2] https://github.com/w8r/moscow-rings#solution Co-authored-by: Rowan Winsemius <[email protected]> Co-authored-by: mfedderly <[email protected]>
I was working on solving #900, and I fixed it simply using the new
@turf/projection, but then find out that seems to fix also #815 and #916. 😃Still for some reason the actual size of the buffer is slightly off, i.e. smaller than the expected one.
cc: @sc-keyzo @ArtCodeKarine @stephanbogner