Opened 3 years ago

Closed 4 weeks ago

#102 closed defect (fixed)

Support for PROJ 6.0.0

Reported by: Bas Couwenberg Owned by: Olly Betts
Priority: minor Milestone: 1.4.0
Component: Other Version:
Keywords: Cc:

Description

Survex still uses proj_api.h which is deprecated in PROJ 6.0.0 and will be removed in PROJ 7.0.0 (scheduled for March 2020).

When using proj_api.h with PROJ 6.0.0 -DACCEPT_USE_OF_DEPRECATED_PROJ_API_H=1 needs to be added to CFLAGS to prevent a compiler error.

Survex should be updated to (also) support the proj.h API.

Attachments (1)

proj6-wip.patch (4.2 KB) - added by Olly Betts 3 years ago.
WIP patch

Download all attachments as: .zip

Change History (20)

comment:1 Changed 3 years ago by Bas Couwenberg

Adding -DACCEPT_USE_OF_DEPRECATED_PROJ_API_H to CFLAGS & CXXFLAGS is not sufficient, the build still fails:

/usr/bin/ld: commands.o: in function `cmd_declination':
./src/commands.c:1712: undefined reference to `pj_factors'

comment:2 Changed 3 years ago by Olly Betts

Just adding it to CPPFLAGS would make more sense since it's a preprocessor option.

Unfortunately pj_factors seems to have had a slightly strange status in the proj API over time (as https://github.com/OSGeo/proj.4/issues/98 shows), but I don't know of a better way to get the grid convergence. That ticket suggests it's also used in grass and qgis, and I believe therion uses it too.

I'm afraid I don't have time to work on this issue currently, but patches are certainly most welcome.

comment:3 Changed 3 years ago by Bas Couwenberg

Therion 5.4.4 added support for proj.h.

I'm not able to provide a patch, but by the time the proj transition starts enough projects may have added support for proj.h to have enough examples to enable you to add support for proj.h to Survex.

For questions about using the new new API, the proj@lists.osgeo.org mailinglist is your best option.

comment:4 Changed 3 years ago by Olly Betts

Priority: majorminor

Changed 3 years ago by Olly Betts

Attachment: proj6-wip.patch added

WIP patch

comment:5 Changed 3 years ago by Olly Betts

A quick poke using 6.1.0-1~exp1 from Debian experimental suggests that the compatibility API is simply missing pj_factors().

But it seems you can use the old and new APIs together if you include proj.h first, so I did that and adjusted the Survex code to use proj_factors() instead.

However PROJ issues unrelated to convergence then cause tests to fail.

It seems PROJ 6 fails to understand +init=esri:NNNN for any valid ESRI code NNNN I've tried. I can reproduce that from the command line - with 6.1.0:

$ proj +init=esri:104305
Rel. 6.1.0, May 15th, 2019
<proj>: 
projection initialization failure
cause: no arguments in initialization list
program abnormally terminated

c.f. 5.2.0 (ignore the error - the point is here the ESRI code is actually understood):

$ proj +init=esri:104305
Rel. 5.2.0, September 15th, 2018
<proj>: 
+proj=latlong unsuitable for use with proj program.
program abnormally terminated

It also doesn't give an error for converting out of range coordinates like older releases did - 6.1.0:

$ echo 179 -89 1000|cs2cs +proj=longlat +ellps=WGS84 +datum=WGS84 +no_defs +to +proj=tmerc +lat_0=49 +lon_0=-2 +k=0.9996012717 +x_0=0 +y_0=0 +ellps=airy +datum=OSGB36 +units=m +no_defs
-1842.61	-15536258.81 1000.00

c.f. 5.2.0:

$ echo 179 -89 1000|cs2cs +proj=longlat +ellps=WGS84 +datum=WGS84 +no_defs +to +proj=tmerc +lat_0=49 +lon_0=-2 +k=0.9996012717 +x_0=0 +y_0=0 +ellps=airy +datum=OSGB36 +units=m +no_defs
Rel. 5.2.0, September 15th, 2018
<cs2cs>: while processing file: <stdin>, line 1
pj_transform(): latitude or longitude exceeded limits
*	* 2175.53

Both of these seem to be PROJ 6 bugs rather than problems with our code.

I'm afraid I don't have further time to waste on this at present. Seems to me PROJ 6 simply isn't production ready yet, and I'd advise installing an older PROJ for now.

WIP patch attached in case anyone wants to work further on this. This patch gets things building but only by disabling the tests which fail - I certainly wouldn't advise using this patch in production.

comment:6 Changed 3 years ago by Bas Couwenberg

Thanks for working on the PROJ 6 support.

Once you have time for this again, you should talk to the PROJ developers via the proj@lists.osgeo.org list. They are very helpful to get projects to support the new API.

Using +init= syntax causes fallback to the PROJ 4 API which also causes issues with cs2cs tests and datumgrids, using the modern syntax (without +init= prefix) should help to use the PROJ 6 API. The projection is deprecated according to projinfo:

# projinfo ESRI:104305
Warning: object is deprecated

PROJ.4 string:
+proj=longlat +a=6378249.145 +rf=293.465 +no_defs +type=crs

WKT2_2018 string:
GEOGCRS["GCS_Voirol_Unifie_1960_Degree",
    DATUM["D_Voirol_Unifie_1960",
        ELLIPSOID["Clarke 1880 (RGS)",6378249.145,293.465,
            LENGTHUNIT["metre",1]]],
    PRIMEM["Greenwich",0,
        ANGLEUNIT["degree",0.0174532925199433]],
    CS[ellipsoidal,2],
        AXIS["geodetic latitude (Lat)",north,
            ORDER[1],
            ANGLEUNIT["degree",0.0174532925199433]],
        AXIS["geodetic longitude (Lon)",east,
            ORDER[2],
            ANGLEUNIT["degree",0.0174532925199433]],
    USAGE[
        SCOPE["unknown"],
        AREA["Algeria - north of 32°N"],
        BBOX[31.99,-2.95,37.14,9.09]],
    ID["ESRI",104305]]

There is probably a modern replacement with a different ID, the will likely give better results with PROJ 6.

comment:7 Changed 2 years ago by Olly Betts

[87197115a7b4afbaff3c67a5d852b4359876e873] adds support for PROJ 5.x. 6.x still seems to be broken though.

The projection is deprecated according to projinfo:

It's not ESRI:104305 in particular that's interesting - that's just an arbitrary example the test suite uses to check that you can specify an ESRI code in a .svx file. It seems that +init=esri:NNNNNN no longer works at all in a proj string (e.g. +init=esri:102581 fails too but projinfo ESRI:102581 doesn't show that as deprecated).

Survex .3d files include details of their projection specified as a proj string, and if the .svx file specifies ESRI:102581, that will be in the form +init=esri:102581. The user can also specify an arbitrary proj string in their .svx file.

comment:8 Changed 2 years ago by Bas Couwenberg

You really need to talk to the PROJ developers about what seems to broken for Survex's use.

comment:9 Changed 2 years ago by Olly Betts

Lack of error for out of range coordinates reported to PROJ upstream: https://github.com/OSGeo/PROJ/issues/1524

comment:10 Changed 2 years ago by Olly Betts

The missing error isn't a bug but rather than PROJ6 uses a more accurate algorithm. I've applied a workaround in [26f1fb2856eff48364257c264afbc02a4cd03ca1].

comment:11 Changed 2 years ago by Olly Betts

[3f1dd91f9fff5ab056d8dd4eb0c68b0f126a4bf2] works around the +init=esri:NNNNNN issue by installing a copy of the esri file from older PROJ and arranging for PROJ to find it. I'll forward the issue upstream too, but at this point Survex now works with PROJ 6.1.0 (though using the old API still).

comment:12 Changed 10 months ago by Bas Couwenberg

The first PROJ 8.0.0 release candidate has been released which removes proj_api.h.

comment:13 Changed 3 months ago by Olly Betts

I've managed to get things mostly ported over to proj.h, but grid convergence seems to be a blocker - I just can't get proj_factors() to work properly after this porting, which breaks the testsuite and also actual use. Oddly we were using proj_factors() before for PROJ 6, and it worked there. I guess I'll try to come up with a reduced testcase and if that doesn't reveal the problem report to PROJ upstream and hope they can help.

There's also a testcase failing because the altitude comes out 46.82m higher than expected, but I think that's probably newer PROJ taking vertical datums into account and older PROJ not. Unfortunately the test harness doesn't provide a way to allow for one of two answers though, and I don't see a good way to add it. Maybe I can adjust the testcase to use a different CRS which gives the same answer with old and new PROJ.

I also need to check the output for KML and GPX export (but don't have automated test coverage currently but really should) and rendering of terrain data (which doesn't have automated test coverage, but would be hard to add).

And there are almost certainly bugs introduced by these changes which the test suite isn't catching, as they're quite extensive. Also the PROJ API now seems to use/return degrees instead of radians in some places (but not all) and sometimes x=lat, y=long - I bet these have tripped me up somewhere.

comment:14 Changed 3 months ago by Olly Betts

As of c2ea0c5919b0a6251df327d1b6cb5ae09193d345 there are now simple testcases for GPX and KML export, and these now both use the new PROJ API.

comment:15 Changed 3 months ago by Bas Couwenberg

The PROJ mailinglist (https://lists.osgeo.org/mailman/listinfo/proj) is the place to ask for help with the proj_factors() issue.

comment:16 Changed 3 months ago by Olly Betts

Sure, but I need to be able to explain the problem coherently - "proj_factors() doesn't work" isn't likely to get a useful response.

Meanwhile it seems useful to keep this ticket up to date with the status. It helps me keep track even if nobody else is interested.

I've now eliminated all use of proj_api.h outside of cavern, and have ensured that terrain rendering still works correctly (in fact it seems to work better now - it looks like the heights match up better, which is probably the same change that causes the cs testcase to fail; experimenting it seems EPSG:4326 vs +proj=longlat +ellps=WGS84 +datum=WGS84 is what makes the difference for terrain, so we probably don't actually need to worry about getting different output with different PROJ versions for the testcase).

I've rebased the proj-api-update branch (which has the WIP changes) onto current master and force pushed it, mostly so there's a remote backup.

comment:17 Changed 2 months ago by Olly Betts

PROJ 8.2.0 should fix proj_factors() to actually be usable in the "new PROJ" world.

Not sure how feasible it is to essentially backport the fix outside of PROJ to get it to work with older versions. Requiring PROJ 8.2.0 would presumably be workable as a fix for Debian, but really isn't helpful for people wanting to build the latest Survex. We could conditionalise the two code versions, but it's quite a lot to maintain in parallel.

comment:18 Changed 4 weeks ago by Olly Betts

The code on git master now supports PROJ >= 8.2.0.

I'll work on making a release soon, but meanwhile 3b69f291337b86848ae44f3d4e4432234b8470a3 is a good commit to test.

comment:19 Changed 4 weeks ago by Olly Betts

Milestone: 1.4.0
Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.