Opened 15 years ago
Closed 9 years ago
#2755 closed defect (fixed)
Omnibus Improvements for ESRI PE Compatability
Reported by: | warmerdam | Owned by: | warmerdam |
---|---|---|---|
Priority: | normal | Milestone: | |
Component: | OGR_SRS | Version: | 1.6.0 |
Severity: | normal | Keywords: | esri hfa gtiff |
Cc: | gaopeng xzhou@… |
Description
Attached please find the modified files for fix of SR in GDAL.
With this fix, the persist test should have no crash and no failure for all more than 3000 SRs for BMP, IMG and TIF. It also fixes some of bugs reported in 9.2 and 9.3. More test results will be got from other people.
The modified files from me are only ogr_srs_esri.cpp, ogr_srs_esri_names.h (new), hfadataset.cpp, gt_wkt_srs.cpp, geo_normalize.h, geo_normalize.c
Changes in geo_normalize.h and geo_normalize.c are minor, just to add standard geotif supported invFlatening value.
Some functions are added in gt_wkt_srs.cpp and hfadataset.cpp to write pe string, citation sting parse and translation, write citation for special needs. Following the meeting, I removed all private geokeys. Functions for name re-mapping, add parameters and delete parameters are added in ogr_srs_esri.cpp.
All other files keep the same as Frank sent for projectionX fix.
Frank, I managed EQUIRECTANGULAR (Plate_Carree) and EQUIDISTANT_CYLINDRICAL in ogr_srs_esri.cpp. It not only works for HFA, but also for tif and all other formats. Therefore your change for these two in HFA is not needed, I changed back to what it was.
Xiuguang
Attachments (2)
Change History (19)
by , 15 years ago
comment:1 by , 15 years ago
Status: | new → assigned |
---|---|
Version: | unspecified → 1.6.0 |
The gdal_datum.csv changes are handled separately in #2752.
The changes to geo_normalize.h and geo_normalize.cpp are being avoided by keeping the invFlattening value as a local variable.
comment:2 by , 15 years ago
Xiuguang,
I'm attempting to integrate ogr_srs_esri.cpp changes. I have encountered a problem with out test suite with regard to Hotine_Oblique_Mercator_Azimuth_Center. In OGR WKT we use Hotine_Oblique_Mercator with a parameter named Azimuth, but the PE uses a projection name of Hotine_Oblique_Mercator_Azimuth_Center with a parameter named rectified_grid_angle. This appears to be related to bug #423.
There used to be code to do some translation between these forms but it does not seem to be operational after your changes. Can you explain why? I expect I'll integrate things as you have proposed for now, but we will need to resolve this situation.
comment:3 by , 15 years ago
I have merged the updated ogr_srs_esri.cpp, but I have preserved the logic I recently introduced for handling Equidistant_Cylindrical so this area should continue to be reviewed for correctness. So far the change is only in trunk (r15992).
comment:4 by , 15 years ago
I have integrated the GeoTIFF driver changes with a few minor changes (citation code moved to a new file - gt_citation.cpp). I do not understand the following change from this:
adfParm[0] *= psDefn->UOMAngleInDegrees; adfParm[1] *= psDefn->UOMAngleInDegrees; adfParm[2] *= psDefn->UOMAngleInDegrees; adfParm[3] *= psDefn->UOMAngleInDegrees; adfParm[5] /= psDefn->UOMLengthInMeters; adfParm[6] /= psDefn->UOMLengthInMeters;
to this:
if(!aUnitGot) { adfParm[0] *= psDefn->UOMAngleInDegrees; adfParm[1] *= psDefn->UOMAngleInDegrees; adfParm[2] *= psDefn->UOMAngleInDegrees; adfParm[3] *= psDefn->UOMAngleInDegrees; } int unitCode = 0; GTIFKeyGet(hGTIF, ProjLinearUnitsGeoKey, &unitCode, 0, 1 ); if(unitCode != KvUserDefined) { adfParm[5] /= psDefn->UOMLengthInMeters; adfParm[6] /= psDefn->UOMLengthInMeters; }
What is the purpose of the aUnitGot test? Is this to avoid a multiplication by 1.0 if the values are already degrees? Surely this does not introduce a rounding/precision problem!
The unitCode test seems wrong to me, and would appear to preclude appropriate adjustment for a user defined unit.
The changes are integrated in trunk (r15993).
comment:5 by , 15 years ago
I neglected to incorporate ogr_srs_esri_names.h earlier - now in trunk (r15994).
comment:6 by , 15 years ago
Frank,
I just registered to be member of GDAL so that I can talk to you here.
The change is to support angular units that are not in degree. The current tif only uses degree. See the string "Write angular units, Always Degrees for now" in the file. "aUnitGot" means the non-degree angular unit is got from the citation string. Then we don't need to do the transformation of angular unit.
Xiuguang Zhou
comment:7 by , 15 years ago
Frank,
In addition, all my changes try not to block the old logic in hfa and tif. If aUnitGot is FALSE, it keeps the old logic.
Xiuguang
comment:8 by , 15 years ago
I have integrated the HFA driver changes in trunk (r15995) with the following exceptions:
I have preserved my recently updated equidistant cylindrical / plate caree handling.
I removed the following addition to HFAGetBandNoData() which I did not understand (what should it accomplish?):
--- hfaopen.cpp (revision 15991) +++ hfaopen.cpp (working copy) @@ -587,6 +587,9 @@ HFABand *poBand = hHFA->papoBand[nBand-1]; + if (poBand->nOverviews > 0) + poBand = poBand->papoOverviews[0]; + *pdfNoData = poBand->dfNoData; return poBand->bNoDataSet; } @@ -898,7 +901,7 @@ }
comment:9 by , 15 years ago
Keywords: | hfa gtiff added |
---|
comment:10 by , 15 years ago
In r15997, I've fixed several potential stack overflows in gt_citation.cpp that could be triggered easily while reading corrupted/hostile GeoTIFFs, or when writing very long citations.
I've tested the changes as much as I could by creating some files at hand with my understanding of how the code works, but without reference GeoTIFFs. So, this should be carefully tested again. Ideally, we would also need some tests in the autotest suite for those new capabilities.
Side node, with the recent changes in the HFA driver, gcore/hfa_write.py and gdrivers/hfa.py fail on several tests.
comment:11 by , 15 years ago
Frank,
I reviewed equidistant cylindrical / plate caree issue. I don't know if the change I made in ogr_srs_esri.cpp is removed, your change still works for hfa and tif.
Thanks
Xiuguang
comment:12 by , 15 years ago
I discovered that changes in HFADataset::ReadProjection() had broken the ability to read coordinate systems from files with a mapinformation but no mapinfo node. Fixed in trunk (r16050), at least sufficiently for the test suite stuff in hfa.py to pass. But I am still suspicious that this logic "gives up" too easily. For instance, why is it a problem if the datum name is "Unknown"?
if( !psDatum || !psPro || (psMapInfo == NULL && poMapInformation == NULL) || (strlen(psDatum->datumname) == 0 || EQUAL(psDatum->datumname, "Unknown")) && (strlen(psPro->proName) == 0 || EQUAL(psPro->proName, "Unknown")) && (psMapInfo && (strlen(psMapInfo->proName) == 0 || EQUAL(psMapInfo->proName, "Unknown"))) && psPro->proZone == 0 ) { pszProjection = CPLStrdup("LOCAL_CS[\"\"]"); return CE_None; }
comment:13 by , 15 years ago
Frank,
I am still in my vacation.
I put the logic is to match the 9.2/9.3 for an IMG dataset. The dataset read from 9.2/9.3 is unknown coordinate system with empty datum/unit. But uing GDAL, ArcGis gives some unpected text in the property page.
Could you send me/Gao all souce files that you changed? Thanks.
Xiuguang
by , 15 years ago
Attachment: | genbindataset.cpp added |
---|
comment:14 by , 15 years ago
Frank,
There is a bug in Raw format. The inverse flatterning calculation is wrong in the file genbindataset.cpp. Please the find the fix in the attached file.
Inseat of 1.0 / (1.0 - dfSemiMajor/dfSemiMinor), it should be 1.0 / (1.0 - dfSemiMinor/dfSemiMajor).
Xiuguang
comment:15 by , 15 years ago
comment:16 by , 15 years ago
comment:17 by , 9 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Closing in the hope all issues have been addressed...
zip of fixed code.