Ticket #3255 (closed enhancement: fixed)

Opened 4 years ago

Last modified 4 years ago

patch to add projection file support to lcpdataset.cpp

Reported by: kyle Owned by: warmerdam
Priority: normal Milestone: 1.7.0
Component: GDAL_Raster Version: svn-trunk
Severity: normal Keywords: lcpdataset
Cc: 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

lcpdataset.patch Download (5.6 KB) - added by kyle 4 years ago.
better diff file
test_lcp.zip Download (16.6 KB) - added by kyle 4 years ago.
lcpdataset_v2.patch Download (4.9 KB) - added by rouault 4 years ago.
Revised version by Even Rouault

Change History

  Changed 4 years ago by rouault

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...

Changed 4 years ago by kyle

better diff file

  Changed 4 years ago by kyle

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   Changed 4 years ago by 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.

Changed 4 years ago by kyle

in reply to: ↑ 3   Changed 4 years ago by kyle

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.

  Changed 4 years ago by rouault

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)

Changed 4 years ago by rouault

Revised version by Even Rouault

  Changed 4 years ago by kyle

I patched my source and everything worked well over here. Thanks for the help.

  Changed 4 years ago by rouault

  • status changed from new to closed
  • resolution set to fixed
  • milestone changed from 1.6.4 to 1.7.0

Patch commited in trunk in r18123

  Changed 4 years ago by kyle

Even, should we add a prj to the autotest data directory at some point for testing this patch?

  Changed 4 years ago by warmerdam

  • cc rouault added

Even, see previous comment.

follow-up: ↓ 11   Changed 4 years ago by rouault

Test updated in r18206

in reply to: ↑ 10   Changed 4 years ago by kyle

Replying to rouault:

Test updated in r18206

Thanks Even

Note: See TracTickets for help on using tickets.