#3255 closed enhancement (fixed)
patch to add projection file support to lcpdataset.cpp.
Reported by: | Kyle Shannon | Owned by: | Kyle Shannon |
---|---|---|---|
Priority: | normal | Milestone: | 1.10.1 |
Component: | GDAL_Raster | Version: | svn-trunk |
Severity: | normal | Keywords: | lcpdataset |
Cc: | Even Rouault |
Description
Add the functionality for a lcpdataset to look for and load a spatial reference to a farsite landscape file. This was implemented the same way as it is in the AAIGrid driver.
Attachments (3)
Change History (20)
comment:1 by , 14 years ago
comment:2 by , 14 years ago
Sorry about the diff file, it's my first patch. I think I got a better one up there. Also you can download this small lcp file with a prj file in a zip archive here: http://www.smoke-fire.us/trac/kyle/export/75/lcp_driver/test_lcp.zip
Let me know if I can do anything else. Thanks.
follow-up: 4 comment:3 by , 14 years ago
The patch looks much better. Thanks. However I can't access your link. It asks me a login / password. I tried anonymous but it didn't work. If it's small enough, you could try attaching it to this ticket. Otherwise you can try to my email : even dot rouault at mines dash paris dot org.
by , 14 years ago
Attachment: | test_lcp.zip added |
---|
comment:4 by , 14 years ago
Replying to rouault:
The patch looks much better. Thanks. However I can't access your link. It asks me a login / password. I tried anonymous but it didn't work. If it's small enough, you could try attaching it to this ticket. Otherwise you can try to my email : even dot rouault at mines dash paris dot org.
Just attached the zip archive.
comment:5 by , 14 years ago
I've attached a revised version of the patch because I noticed you've copied&pasted stuff that isn't really used. The poDS->adfGeoTransform array which existed before your wasn't used as the GetGeoTransform() method doesn't use it. So I've removed all its use as well as the OSR_GDS() function. It was probably something specific to the AAIGrid driver that doesn't make sense for LCP ? I've also fixed the missing initialization of pszProjection and it's deallocation.
Could you confirm that this version is OK ? (it gives identical results as your version on your sample)
comment:6 by , 14 years ago
I patched my source and everything worked well over here. Thanks for the help.
comment:7 by , 14 years ago
Milestone: | 1.6.4 → 1.7.0 |
---|---|
Resolution: | → fixed |
Status: | new → closed |
Patch commited in trunk in r18123
comment:8 by , 14 years ago
Even, should we add a prj to the autotest data directory at some point for testing this patch?
comment:12 by , 10 years ago
Milestone: | 1.7.0 → 1.10.1 |
---|---|
Resolution: | fixed |
Status: | closed → reopened |
Re-opening as I found a couple issues with the feature. 1) pszProjection was initialized to NULL, so GetProjectionRef() returned NULL if no prj was found. 2) Initializing pszProjection to non-NULL ("") caused the prj file to be added to the sibling list even if it didn't exist.
I have a fix, just going to do a bit of testing before I submit.
comment:14 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:15 by , 10 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Kyle,
I believe this will leak memory. You should likely CPLFree(poDS->pszProjection); just before oSRS.exportToWkt( &(poDS->pszProjection) ); Other solution you keep the null initialization of pszProjection, and in GetProjectionRef() you return pszProjection ? pszProjection : ""
comment:16 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
comment:17 by , 10 years ago
Summary: | patch to add projection file support to lcpdataset.cpp → patch to add projection file support to lcpdataset.cpp. |
---|
I eventually remembered, :). I knew I forgot something.
Kyle,
would you mind providing a link to a downloadable dataset that uses a .prj so that we can test your change ?
Also it would be far easier to review if your patch only contained the semantic changes, not all those whitespaces changes...