Skip to content

Fix for rectangle and square grid dimensions#2106

Merged
mfedderly merged 13 commits intomasterfrom
fix-rect-grid
Aug 5, 2021
Merged

Fix for rectangle and square grid dimensions#2106
mfedderly merged 13 commits intomasterfrom
fix-rect-grid

Conversation

@rowanwins
Copy link
Copy Markdown
Member

@rowanwins rowanwins commented Jun 18, 2021

So in digging into #2079 I found that we using the haversine distance formula for calculating how big to make the grid cells.

Where this falls down is where you bbox if > 180 degrees because it then finds the shortest distance, and the wider your bbox of interest the worse the effects would be.

// A straight line 
turf.distance([-180, -90], [180, -90], {units: 'degrees'})
==> 0

Swapping to a flat earth earth calc we do something like

180 - -180
==> 360

There doesn't seem to be an easy way to hack our distance calc to produce something similar (at least in my initial play) so adjusting the approach seems reasonable and gives more expected results when generating large grids (eg see https://github.com/Turfjs/turf/blob/master/packages/turf-rectangle-grid/test/out/big-bbox-500x100-miles.geojson)

Resolves #2079
Resolves #1948
Resolves #1598

The same lines of code possibly need to be ported to point-grid and triangle-grid. See #1886

@rowanwins rowanwins mentioned this pull request Jun 18, 2021
@rowanwins rowanwins changed the title Use non-haversine distance calc Fix for rectangle and square grid dimensions Jun 18, 2021
@rowanwins
Copy link
Copy Markdown
Member Author

Alternatively I need to look into how calculate a harversine distance going the long way around (when the bbox is >180), that seems achievable, although a bit fiddlier

Copy link
Copy Markdown
Collaborator

@mfedderly mfedderly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this broke the tests in @turf/square-grid, they probably just need to be re-updated. Github also isn't letting me do the rich diff view for these geojsons, but I'm assuming you looked at the output and think its better?

@rowanwins
Copy link
Copy Markdown
Member Author

I've checked the results of the proposed PR against QGIS and we now generate almost the exact same results.

Copy link
Copy Markdown
Collaborator

@mfedderly mfedderly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Global squaregrid squareGrid returns rectangles? Grids generation problems on the whole sphere

2 participants