Opened 2 months ago

Closed 2 weeks ago

#150 closed enhancement (fixed)

[Cavern][*cs] Support for WKT2 for proj

Reported by: Philip Schuchardt Owned by: Olly Betts
Priority: minor Milestone: 1.4.23
Component: cavern Version: 1.4.15
Keywords: Cc:

Description

I need custom WKT2 strings for CaveWhere to properly use plate-fixed datums with custom tmerc projection.

*cs ... CUSTOM "..." accepts any string PROJ's proj_create() understands, but the .svx string reader cannot represent a " inside a quoted string, and WKT2 is made of them. That leaves a class of coordinate systems that cavern cannot be told about at all: a custom projection on a datum newer than PROJ's +datum= table.

PROJ has deprecated the proj-string form for describing a CRS. From proj.h (proj_as_proj_string, PROJ 9.8.1):

Reproduction

Survex 1.4.15, PROJ 9.8.1, macOS (Homebrew). Attached: wkt.zip, containing the files below plus mkldp.c, which generates the CRS with the PROJ C API and prints both serializations.

projstring.svx — control, works (exit 0, writes .3d), but the frame it georeferences to has no datum:

*cs EPSG:6318
*cs out CUSTOM "+proj=tmerc +lat_0=37.1866 +lon_0=-86.1005 +k=1 +x_0=0 +y_0=0 +ellps=GRS80 +units=m +no_defs +type=crs"

*fix entrance -86.1005 37.1866 200

entrance a 30.0 090 -05
a        b 25.0 135 +02

wkt2.svx — the same CRS as single-line WKT2:2019 (proj_as_wkt(..., PJ_WKT2_2019, {"MULTILINE=NO"})). Exit 1, no .3d:

wkt2.svx:2:16: error: Unknown coordinate system
 *cs out CUSTOM "PROJCRS["Mammoth Cave LDP",BASEGEOGCRS["NAD83(2011)",DATUM[...
                ^~~~~~~~~~~~~~~~~
wkt2.svx:4: error: The input projection is set but the output projection isn't

Neither escaping convention helps, and the two fail differently:

file form result
wkt2.svx raw " error 434 Unknown coordinate system
wkt2-backslash.svx \" error 434 Unknown coordinate system
wkt2-doubled.svx "" error 443 Invalid coordinate system: Invalid PROJ string syntax
wkt2-unquoted.svx no quoting error 443 Invalid coordinate system: Invalid PROJ string syntax

Attachments (3)

wkt.zip​ (4.7 KB ) - added by Philip Schuchardt 2 months ago.
0001-cavern-Allow-cs-custom-to-read-the-CRS-from-a-file.patch​ (19.1 KB ) - added by Philip Schuchardt 2 months ago.
0001-cavern-Add-cs-file-to-read-the-coordinate-system-fro.patch​ (20.1 KB ) - added by Philip Schuchardt 5 weeks ago.

Download all attachments as: .zip

Change History (15)

by Philip Schuchardt, 2 months ago

Attachment: wkt.zip​ added

comment:1 by Philip Schuchardt, 2 months ago

I'm working on a patch for this.

comment:2 by Philip Schuchardt, 2 months ago

Patch is attached.

comment:3 by Olly Betts, 8 weeks ago

Thanks for the patch.

I'm not sure I really like the complication of reading an external file here, but the various WKT descriptions are rather long, and it does have the definite advantage of being able to use existing .prj files - that last one is a particularly strong selling point for it.

I really don't think the @ should be allowed inside the quotes though - while in this context I don't think there are valid existing strings which can start with @, it seems unhelpful to bake into the syntax that you can't specify a string value which starts @ by making "@foo" mean the same as @"foo".

The @ is also a bit cryptic. I realise it matches Microsoft's response file convention, but that's rather obscure and generally the Survex data format uses keywords rather than symbols. How about something like:

*cs out file "mammoth.prj"

That also has the benefit of making it clear that this isn't a general feature that can be used in other contexts which allow a quoted string, and should give a less confusing error if you try to process a dataset using this feature with an old release of Survex.

If PROJ rejects a .prj file with a UTF-8 BOM, why should we accept it? Do other programs which support .prj files allow it?

Independent of this, quoted strings should really allow escaping the quote (and supporting escaping by doubling it shouldn't break any existing data - the case that it could break is adjacent quoted strings if the parser doesn't actually enforce whitespace between them, but AFAICS there aren't any places where adjacent quoted strings are currently allowed).

comment:4 by Olly Betts, 8 weeks ago

Component: Other → cavern
Milestone: → 1.4.23
Type: defect → enhancement
Version: → 1.4.15

comment:5 by Olly Betts, 8 weeks ago

wkt2.svx:2:16: error: Unknown coordinate system
 *cs out CUSTOM "PROJCRS["Mammoth Cave LDP",BASEGEOGCRS["NAD83(2011)",DATUM[...
                ^~~~~~~~~~~~~~~~~

Incidentally, the error context here is wrong - it should use DIAG_STRING for the CS_CUSTOM case, which fixes it to highlight how it was actually parsed - I'll adjust that.

comment:6 by Philip Schuchardt, 8 weeks ago

Good feedback, I'll upload another patch if you like with *cs out file "mammoth.prj" instead of "@". I'll look into the UTF-8 BOM.

comment:7 by Olly Betts, 8 weeks ago

I've fixed the context for that error, now gives:

wkt/wkt2.svx:2:16: error: Invalid coordinate system: Invalid PROJ string syntax
 *cs out CUSTOM "PROJCRS["Mammoth Cave LDP",BASEGEOGCRS["NAD83(2011)",DATUM["NAD83 (National Spatial Reference System 2011)",ELLIPSOID["GRS 1980",6378137,298.257222101,LENGTHUNIT["metre",1]],ANCHOREPOCH[2010]],PRIMEM["Greenwich",0,ANGLEUNIT["degree",0.0174532925199433]],ID["EPSG",6318]],CONVERSION["Transverse Mercator",METHOD["Transverse Mercator",ID["EPSG",9807]],PARAMETER["Latitude of natural origin",37.1866,ANGLEUNIT["degree",0.0174532925199433],ID["EPSG",8801]],PARAMETER["Longitude of natural origin",-86.1005,ANGLEUNIT["degree",0.0174532925199433],ID["EPSG",8802]],PARAMETER["Scale factor at natural origin",1,SCALEUNIT["unity",1],ID["EPSG",8805]],PARAMETER["False easting",0,LENGTHUNIT["Metre",1],ID["EPSG",8806]],PARAMETER["False northing",0,LENGTHUNIT["Metre",1],ID["EPSG",8807]]],CS[Cartesian,2],AXIS["(E)",east,ORDER[1],LENGTHUNIT["Metre",1]],AXIS["(N)",north,ORDER[2],LENGTHUNIT["Metre",1]]]"
                ^~~~~~~~~~

Independent of this, quoted strings should really allow escaping the quote (and supporting escaping by doubling it shouldn't break any existing data - the case that it could break is adjacent quoted strings if the parser doesn't actually enforce whitespace between them, but AFAICS there aren't any places where adjacent quoted strings are currently allowed).

I've implemented this.

I'll upload another patch if you like with *cs out file "mammoth.prj" instead of "@".

Sounds good.

comment:8 by Philip Schuchardt, 5 weeks ago

New patch attached, rebased on current main so it sits on top of your quote doubling and *cs custom error context changes.

Changes from the first version:

  • The @ prefix is gone. It's now a keyword, as you suggested:

*cs out file "mammoth.prj"

custom is back to exactly what it was upstream, so a string starting with @ means nothing special anywhere.

  • BOM handling dropped. I checked what other .prj readers do: gdalsrsinfo on a .prj with a UTF-8 BOM fails with "unhandled keyword: <BOM>PROJCRS", and projinfo fails to parse it too. So cavern now matches them and the file is reported as an invalid coordinate system. csprjbad covers that case with a BOM fixture.
  • The filename goes through read_string, so a quote in a name is doubled like anywhere else, and the *cs docs say so.
  • file is excluded from the isalnum "Unknown coordinate system" check the same way custom is, otherwise junk after a quoted filename gets the wrong error. csprjbad covers that too.

Still kept: the filename is resolved relative to the .svx it's in, and a .prj extension is tried if the name as given doesn't exist, so ESRI .prj files work without renaming.

Full cavern.tst passes here. Docs updated in datafile.rst and 3dformat.htm.

comment:9 by Olly Betts, 4 weeks ago

a .prj extension is tried if the name as given doesn't exist, so ESRI .prj files work without renaming.

Is there a good reason for adding the extension? I don't really follow the logic for "so ESRI .prj files work without renaming" as that seems true without any special handling here (e.g. *cs file esri.prj will load a file with a .prj extension without having to rename it).

If you're just following the model of how we handle .svx, that's really there for historical reasons. The original main platform for Survex was Acorn's RISC OS which had support for file types in the filing system and didn't really have file extensions. The directory separator was . (which is where the choice of that for the default survey level separator in Survex comes from), so you'd just have a file without an extension. For interacting with the outside world, sometimes / would be used for an extension (e.g. path.to.file/ext). The C compiler instead used the convention of c and h directories with the file inside (so e.g. survex.c.cavern and survex.h.cavern). To support maintaining Survex datasets which could be sensibly processed on both RISC OS and other platforms we went for adding an extension if not found without one.

We stopped supporting Survex on RISC OS more than 20 years ago, but dropping the adding of .svx would break existing datasets so that's why it's still done. I don't think there's a relevant modern operating system where files can't have a .prj extension, and so I wonder if requiring an explicit extension (unless there isn't one) would be better. Adding it if the file is not found found creates a nasty trap where someone creating a file can break (or quietly change) processing of a dataset. It's not like people will have to type *cs file lines often.

comment:10 by Olly Betts, 4 weeks ago

I've now merged the patch - we can adjust the extension handling if need be (we just need to resolve that before the next release so leaving this ticket open).

BTW I changed csprj.svx and csprj2.svx to use pos=dump as that means we directly test the .prj to string conversion gives what we expect. (pos=yes is a good choice if only the station names and positions matter; pos=dump allows testing the legs and per-file/per-station/per-leg metadata.)

comment:11 by Philip Schuchardt, 2 weeks ago

Amazing! I merged CaveWhere's survex fork with main. Seems like everything is working. Thanks!

comment:12 by Olly Betts, 2 weeks ago

Resolution: → fixed
Status: new → closed

we can adjust the extension handling if need be (we just need to resolve that before the next release so leaving this ticket open).

I've dropped the .prj default in cda44db2d710027caab217b3f594755632423e0e.

Another point in favour of not having a default occurred to me, which is that multiple formats which use different extensions are reasonable here.

Note: See TracTickets for help on using tickets.