Antimeridian and bbox calc fix #1115
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
aagm
added
bug
| st_makeenvelope(:xmin, :ymin, :xmax, :ymax, 4326), | ||
| `(st_intersects( | ||
| st_intersection(st_makeenvelope(:...eastBbox, 4326), | ||
| ST_MakeEnvelope(0, -90, 180, 90, 4326)), |
There was a problem hiding this comment.
no need to do anything in this PR about this, from my point of view, but these could be good candidates for a named constant that allows to avoid "magic numbers" that may take a little while for the inexperienced reader to figure out when they look at this code out of context (for example, not knowing that it was introduced while splitting semihemispheres)
There was a problem hiding this comment.
yes in the future we will need to revisit some of this, as well as at some point pay the debt in regarding sql formulation
...essing/src/migrations/geoprocessing/1653565019335-ModifyBboxCalculationTakingAntimeridian.ts
Show resolved
Hide resolved
| */ | ||
| public findTile( | ||
| tileSpecification: TileSpecification, | ||
| bbox?: BBox, | ||
| ): Promise<Buffer> { | ||
| const { z, x, y, id } = tileSpecification; | ||
| const attributes = 'feature_id, properties'; | ||
| const table = `(select (st_dump(the_geom)).geom as the_geom, properties, feature_id from "${this.featuresRepository.metadata.tableName}")`; | ||
| const table = `(select ST_RemoveRepeatedPoints((st_dump(the_geom)).geom, 0.1) as the_geom, properties, feature_id from "${this.featuresRepository.metadata.tableName}")`; |
| * to Nominatim bbox [xmin, xmax, ymin, ymax]. | ||
| * | ||
| */ | ||
| export function bbox2Nominatim(bbox: BBox): BBox { |
There was a problem hiding this comment.
you could consider aliasing BBox as NominatimBBox to improve type expressiveness in function signatures, even though in practice it's still just an array of 4 or 6 numbers.
There was a problem hiding this comment.
nothing to do here though, unless you wish to, of course 😃
Take with care
This fix is touching almost all intersections based on bbox in the application
Overview
So this applies 2 different fixes altogether:
BBox crosses the antimeridian, we are overpassing this in 2 steps, first, we properly calculate min and max longitude values by dividing the bbox in 2 bases on the east and west semihemisphers.
After that both FE and backend will follow the next rule: if min long is > than max long, that means that the bbox crosses the antimeridian and we subdivide them.
Regarding the grid. So as we are already setting the equal area cost surface, I did take the license to every grid that is crossing the antimeridian will be cut in order to avoid it crossing the antimeridian. with this in place calculations should work as expected.
Also, small fixes were applied to everything that depends on bbox filtering in order to be able to overpass the bbox at the antimeridian situation. The only problem here might be a small overhead due to subdividing it into the east and west suboxes.
Testing instructions
Up locally and setup a project in Fiji and in any other place
Feature relevant tickets
Link to the related task manager tickets
https://vizzuality.atlassian.net/browse/MARXAN-985
Checklist before submitting
develop.deploying to staging/production, please add brief testing instructions
to the deploy checklist (
docs/deployment-checklist.md)