Write projection string to GeoTIFF produced by cam2map#6039
Write projection string to GeoTIFF produced by cam2map#6039oleg-alexandrov wants to merge 7 commits into
Conversation
Cam2map GTiff output (and any other ISIS app that writes a Format::GTiff cube with a Mapping group) carried no SRS or GeoTransform unless the useproj=true path had stuffed in a ProjStr keyword. The standard projection path and asp_map mode left the output GTiff with no projection info, so gdalinfo on the result showed nothing under Coordinate System. Cubes were unaffected because GDAL's ISIS3 driver derives the SRS from the Mapping PVL on read. Move the WKT-from-Mapping helper out of the cam2map ASP_MAP source into a shared static method, ProjectionFactory::PvlToWkt, so any caller can use it. The body is taken from apps/cam2map/AspMapProjection.cpp and extended with the two projections the GDAL ISIS3 driver supports that the original lacked: PointPerspective (SetVerticalPerspective, with height = Distance*1000 - semi_major) and ObliqueCylindrical (PROJ4 +proj=ob_tran string with ISIS-to-PROJ rotated-pole convention fix). The WKT is verified byte-identical (PointPerspective) and mathematically identical (ObliqueCylindrical, cosmetic-only differences) against the GDAL ISIS3 driver round-trip. Cube::writeLabels for Format::GTiff now uses ProjStr if present (useproj override path) and otherwise synthesizes the WKT from the Mapping group via ProjectionFactory::PvlToWkt. SetSpatialRef and SetGeoTransform proceed as before. Cubs are untouched - the entire block is gated on Format::GTiff. Unsupported projections cleanly skip the geo-tag write rather than fail the cube write. apps/cam2map/AspMapProjection.cpp now calls ProjectionFactory::PvlToWkt; its private pvlToWkt is removed. Fixes DOI-USGS#6033.
Update the FROM and TO parameter descriptions in cam2map.xml to mention GeoTIFF input and output, the +gtiff/+tile/+bsq output attributes, and that this is new in ISIS 10 and still being evaluated. Expand both parameter filters from *.cub to *.cub *.tif. Add changes/6033.fix.md describing the geo-tag fix.
Three gtest cases in isis/tests/Cam2mapTests.cpp: - FunctionalTestCam2mapGTiffOutputDefault (DefaultCube): standard cam2map cub -> tif via +gtiff, exercises the synthesized-WKT path in Cube::writeLabels via ProjectionFactory::PvlToWkt. Verifies SRS is present, ellipsoid radius matches the Mapping group, and the projection name is "Sinusoidal". - FunctionalTestCam2mapGTiffOutputUseproj (DefaultCube): useproj=true + projstring + +gtiff, exercises the ProjStr override path in Cube::writeLabels (PvlToWkt bypassed). Backfills a pre-existing coverage gap: useproj had no prior test coverage at all. - FunctionalTestCam2mapAspMapGTiffOutput (DemCube): asp_map=true cub -> tif via +gtiff. Demonstrates asp_map mode shares the same Cube::writeLabels path as standard cam2map. All three call GDALAllRegister() before cam2map (so its tiff writer has the GTiff driver) and then assert via GDALOpen + GetSpatialRef.
Shorten changes/6033.fix.md. In cam2map.xml TO description, replace the "to=out.cub+gtiff" example with "to=out+gtiff -> out.tif" so it's clear how the output filename is built (ISIS appends .tif to the name you provide).
Tests now use to=stem+gtiff (so the output file is stem.tif rather than stem.cub.tif). Matches the example in the cam2map.xml TO doc. Also tighten the asp_map test comment.
|
The build and test suite have started for your pull request. To view your build log, please reference the build with source version: "PR_6039". Additionally, check the latest "dev" source version to identify existing test failures. Please note that you are not responsible for the test failures that exist on both your PR and the dev branch. |
|
Note that my PR_6039 and unrelated PR_6027 are both failing with identical errors: undefined reference to `testing::internal::MakeAndRegisterTestInfo(std::__cxx11::basic_string<char, std::char_traits, std::allocator >, char const*, char const*, char const*, testing::internal::CodeLocation, void const*, void ()(), void ()(), testing::internal::TestFactoryBase*)' something changed in the env. PR_6036 from 3 hours ago worked. |
|
Only thing I can find is a minizip version change: |
| double defaultSemiMajor, | ||
| double defaultSemiMinor) { | ||
|
|
||
| std::string projName = mapGrp["ProjectionName"][0].toLower().toStdString(); |
There was a problem hiding this comment.
@oleg-alexandrov Thoughts on doing something like this rather than copy code from gdal?
QString file = "temp.cub";
Cube cube;
cube.setDimensions(1, 1, 1);
cube.create(file);
PvlObject &isisCube = cube.label()->findObject("IsisCube");
isisCube.addGroup(mapGrp);
cube.close();
GDALDataset *dataset = GDALDataset::FromHandle(GDALOpen(file.toStdString().c_str(), GA_ReadOnly));
if (!dataset) {
QString msg = "Failed opening GDALDataset from [" + file + "]";
throw IException(IException::Programmer, msg, _FILEINFO_);
}
const OGRSpatialReference *oSRS = dataset->GetSpatialRef();
if (!oSRS) {
QString msg = "Failed obtain spatial reference from [" + file + "]";
throw IException(IException::Programmer, msg, _FILEINFO_);
}
char *wkt = nullptr;
oSRS->exportToWkt(&wkt);
std::cout << "Banana " << wkt << std::endl;
GDALClose(dataset);
if (!wkt)
throw IException(IException::Programmer,
"Failed to export projection to WKT", _FILEINFO_);
std::string result(wkt);
CPLFree(wkt);
return result;I know its code that is unlikely to change but not having to maintain it in two places seems better
There was a problem hiding this comment.
Adam, are you saying we should create on-the fly temporary cubes on disk to be able to parse a a pvl string?
If that's the case, we likely better reach out to GDAL folks and request they expose a function in the ISIS driver we could use?
There was a problem hiding this comment.
Or potentially allow for the OGRSpatialReference to import a pvl mapping group
There was a problem hiding this comment.
So, more work on the GDAL side. Presumably we can work with them, wait for them to release GDAL with it, pin to that, and wipe our own implementation in due time. Which would be a cycle of some length of time.
|
The build and test suite have started for your pull request. To view your build log, please reference the build with source version: "PR_6039". Additionally, check the latest "dev" source version to identify existing test failures. Please note that you are not responsible for the test failures that exist on both your PR and the dev branch. |
|
Anything holding this up? |
|
Just confirming, this needs work on the GDAL side still? |
|
Ideally yes, I hesitate to add functions on the ISIS side that will eventually be deprecated but there has been zero movement on gdal for this. OSGeo/gdal#14555 |
Write projection string to GeoTIFF produced by cam2map
Description
This fixes #6033. When cam2map or any other geotiff writing logic (via Cube::writeLabels) writes a GeoTIFF cube, the proj string is always written. Before, this was true only when a projection string was set as argument.
This uses a function named PvlToWkt that was previously used only for writing a cam2map-ed cub in asp_map format.
The documentation in cam2map.xml got expanded a bit to reflect that geotiffs can be inputs and outputs.
How Has This Been Validated?
Manually ran cam2map on an MRO CTX cube (B17_016219_1978_XN_17N282W.cub) before and after the fix.
Cross-checked against the GDAL ISIS3 driver which can convert a cam2map-ed cub to geotif.
Added three unit tests to create a geotif with and without setting a projection string, and when writing a geotiff with asp_map = true.
Types of changes
Checklist:
document.
changes/6033.fix.md.)Licensing
benefit of the public at large and to the detriment of my heirs and successors. I intend this dedication to be an
overt act of relinquishment in perpetuity of all present and future rights to this software under copyright law.