incorporation of SAGA GIS binary dataset driver

[vwichmann]

I have implemented a draft version of a SAGA GIS binary dataset driver. Roger Bivand and me tested the driver and it seems to work okay so far. Frank was involved in the discussion offlist and encouraged us to file a ticket with the source code of the driver and a sample dataset. Please find both attached.

It would be nice if someone from the dev-team could review the code and then incorporate it.

Thanks, Volker

[vwichmann]

source code of the driver

[vwichmann]

4 byte float sample grid

[Even Rouault]

OK, I've commited in r18207 (and a test in r18208) an unmodified version of the driver so it's easier for you to track some later cleanup that I may add (I see a lot of copy&paste in IReadBlock()).

Has the driver been tested on big endian hosts ? I see that this has been foreseen, but I have doubt that it actually works. For example IWriteBlock() should do invert byte swapping after commiting the buffer to the file, so that the caller sees an unmodified buffer. The m_ByteOrder variable should also be taken into account.

In Create(), the initial fill of the file with the nodata value seems to be incorrect. It doesn't make sense to write the first byte of a double for example (VSIFWriteL( (GByte *)&fVal, sizeof(GByte), 1, fp ) at line 1208). I guess you should affect the nodata value to a variable of the proper type (a GByte for GDT_Byte, a GInt16 for GDT_Int16 and so on) and write it.

Do you have some doc for the driver ? I see a mention to frmt_various.html#SAGA

[Even Rouault]

OK, I've much simplified and hopefully corrected IReadBlock() and IWriteBlock() in r18210. Test extended in r18209.

I'm wondering: what is the point of CreateCopy() since you have implemented Create() and the default DefaultCreateCopy() implementation can simulate CreateCopy() with just Create().

I'm not sure of what the following snippet in CreateCopy() is supposed to do:

if( bSrcHasNDValue && pfData[iCol] == fSrcNoDataValue )
    pfData[iCol] = fSrcNoDataValue;

[vwichmann]

Hi Even,

thanks a lot for reviewing and improving the driver! I'll try to provide some answers to your questions:

• thanks for cleaning up IReadBlock(), I was unsure how to avoid all the copy/paste

• the driver has not been tested on big endian hosts. As SAGA grid should support this and gsbgdataset.cpp (from which I started) has support for this I thought it would be nice

• your're right, the NoData values in Create() are not filled correctly for the various types

• I don't have written a doc for the driver yet. I thought it would be better to wait with this until it is clear what the driver supports.

• I'm not that familiar with the GDAL concepts, so I didn't know that CreateCopy() isn't needed if Create() is implemented. I just saw that the gsbg driver has it implemented and thought it is necessary.

• the snippet in CreateCopy() is also a leftover from the gsbg driver

[vwichmann]

Even,

thanks for fixing these issues too! I've just checked out trunk and so far everything works fine for me. I will now encourage some SAGA users/devs to help testing.

A minor issue I found: the makegdal71.vcproj and makegdal80.vcproj files are not updated yet.

[Even Rouault]

I don't believe that the makegdal71.vcproj and makegdal80.vcproj files are really actively supported (I haven't seen anyone updating them for ages). Maybe you can raise the question on the mailing list. Most people use "nmake /f makefile.vc" from the command line I guess.

I've added SAGA in the GDAL driver list (r18228). If you can provide an update of frmt_various.html or a standalone frmt_saga.html, I'll make a link to it.

[vwichmann]

update including SAGA driver description

[vwichmann]

Ok, I don't use the vcproj files for building either, I just use them to open the project for editing in visual studio ...

I've attached an update of frmt_various.html including a description of the SAGA driver.

Thanks for all your efforts!

[warmerdam]

I have applied a white space reformatting to meet normal GDAL conventions (r18245) (spaces in stead of tabs, 4 space indentation).

[Even Rouault]

r18250 /trunk/gdal/frmts/ (formats_list.html frmt_various.html): Add doc for SAGA GIS driver (#3267)

Closing as I think everything is in place now.

[Even Rouault]

Additionnal changes :

r18251 /trunk/gdal/frmts/saga/sagadataset.cpp: SAGA driver: remove useless Delete() implementation, but implement GetFileList() instead (that's way .sdat and .sgrd will be deleted); declare the VIRTUALIO capability (#3267)

r18252 /trunk/autotest/gdrivers/saga.py: Update SAGA tests (#3267)