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.

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

Use PROJ 5.1 API #1218

Closed
wants to merge 2 commits into from
Closed

Use PROJ 5.1 API #1218

wants to merge 2 commits into from

Conversation

pkturner
Copy link
Contributor

This updates Georeferencing to use the PROJ 5.1 API. I'm initiating this PR to show the status of this work. I've tested it on Linux, and will test more thoroughly including on Android.

The pj_strerrno function is replaced with proj_errno_string, which was introduced in 5.1.
Search uses the environment's PROJ_LIB, as suggested by @dg0yt.

@pkturner pkturner mentioned this pull request Mar 22, 2019
@dg0yt
Copy link
Member

dg0yt commented Mar 22, 2019

Please, base you work on dev, not master.

I'm not sure how fast we actually want to remove compatibility with PROJ 4.9. Ubuntu 18.04 will be LTS at least until 2020. I don't know when Debian 9 will be replaced.
https://github.com/OpenOrienteering/mapper/wiki/Target-systems#component-version-matrix

@pkturner
Copy link
Contributor Author

I brought the proj-5 branch up-to-date with dev. Now it looks like a mess. Maybe it needs a different branch and/or a new pull request.

Also, checks are failing due to PROJ 5.0's <proj.h> not found in Azure.

@pkturner
Copy link
Contributor Author

pkturner commented Mar 23, 2019

@dg0yt

I'm not sure how fast we actually want to remove compatibility with PROJ 4.9.

Understood. Seeing the component version matrix, you are juggling a great number of dependencies.

@dg0yt
Copy link
Member

dg0yt commented Mar 23, 2019

I think you could do the following now:

git rebase ed39289 --onto dev

This should cherry-pick your new commits (which are after ed39289) on top of dev.

I'm about to skip PROJ 5 for PROJ 6 in Superbuild if it works for Android, too. However, I'm struggling with the GDAL update to 2.4 which I want to do at the same time. Maybe it is easier to wait a few days.

@pkturner pkturner changed the base branch from master to dev March 23, 2019 15:35
@pkturner
Copy link
Contributor Author

Changing the PR's base branch in GitHub from master to dev cleared up the "mess".

@dg0yt dg0yt changed the base branch from dev to dev-copy May 6, 2019 05:55
@dg0yt dg0yt changed the base branch from dev-copy to dev May 6, 2019 05:56
@dg0yt
Copy link
Member

dg0yt commented Aug 2, 2019

On the PROJ mailing was a thread about porting to PROJ 6. Some takeaways:

  • We need to explicitly handle axis order, at least by axis proj_normalize_for_visualization.
  • In the long run, we may need to handle (at least for parity with GIS)
    • dynamic datum
    • late binding

@dg0yt dg0yt changed the base branch from dev to georeferencing August 30, 2019 18:41
dg0yt pushed a commit that referenced this pull request Aug 30, 2019
Cherry-picked from GH-1218, and edited.
Does not compile with PROJ 4.9!
dg0yt pushed a commit that referenced this pull request Aug 31, 2019
Cherry-picked from GH-1218, and edited.
dg0yt added a commit that referenced this pull request Aug 31, 2019
Cherry-picked from GH-1218, and edited.
Does not compile with PROJ 4.9!
@dg0yt dg0yt force-pushed the georeferencing branch 2 times, most recently from 25ccda8 to 025ff81 Compare August 31, 2019 15:36
@dg0yt dg0yt force-pushed the georeferencing branch 2 times, most recently from 5bdeb20 to a96bc58 Compare August 31, 2019 23:33
@dg0yt dg0yt changed the base branch from georeferencing to dev August 31, 2019 23:37
@dg0yt
Copy link
Member

dg0yt commented Aug 31, 2019

This is now entirely replaced by #1341. Thanks for the excellent starting point. It even helped find bugs in PROJ.

@dg0yt dg0yt closed this Aug 31, 2019
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.

2 participants