Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Dateline #310

Open
wants to merge 20 commits into
base: develop
Choose a base branch
from
Open

Dateline #310

wants to merge 20 commits into from

Conversation

EmileSonneveld
Copy link
Contributor

@EmileSonneveld EmileSonneveld commented Jun 28, 2024 •
edited
Loading

The issue was an intersection calculation losing a lot of precision due to reprojecting and back.
Now the intersection between the feature and the requested extent is calculated in the target extent crs.

This made that some tests now load a smaller piece of the feature, on some places {FloatConstantNoDataArrayTile@16026} ArrayTile(256,256,float32) became {PaddedTile@15829} PaddedTile(ArrayTile(140,122,float32),17,134,256,256)

@EmileSonneveld EmileSonneveld linked an issue Jun 28, 2024 that may be closed by this pull request
no results for bbox near date line #279
Open
@EmileSonneveld EmileSonneveld requested a review from jdries June 28, 2024 13:21
jdries
jdries requested changes Jul 22, 2024
View reviewed changes
Copy link
Contributor

@jdries jdries left a comment

Choose a reason for hiding this comment

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

Can you also add a test where the feature extent is outside of the valid bounds of target CRS? For instance, a feature with global extent, and target extent being some utm zone.
I think this is the reason for going via EPSG:4326 to compute the intersection safely.

@EmileSonneveld
Copy link
Contributor Author

Added a test for feature in LatLng and target extent in UTM.
Had to change the code too. This comment describes the change:

We convert both extents to a common CRS before taking the intersection.
If one of the CRSes can cover the whole world (non-UTM), this will be used as common CRS.
We give priority to use the target CRS as common one, because the intersection will be converted to it anyway

@EmileSonneveld EmileSonneveld requested a review from jdries July 25, 2024 11:28
@EmileSonneveld EmileSonneveld marked this pull request as draft August 9, 2024 07:24
@EmileSonneveld
Copy link
Contributor Author

There where already many cases where 2 different UTM extentds where used. Confirmed by logging

@EmileSonneveld EmileSonneveld marked this pull request as ready for review August 9, 2024 08:49
jdries
jdries requested changes Aug 14, 2024
View reviewed changes
Copy link
Contributor

@jdries jdries left a comment

Choose a reason for hiding this comment

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

it now assumes that utm is the only projection that has this issue.
EPSG:3035 is another example that we work with a lot.
It could be that the valid area of a crs is in fact available, maybe we can check on that.

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

@jdries jdries jdries requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

no results for bbox near date line
2 participants
@EmileSonneveld @jdries