[Gdal-dev] Drivers for Golden Software Grid Formats (long)

[Andrey Kiselev]

Hi, Kevin,

On Sun, Jan 28, 2007 at 10:11:25AM -0500, Kevin Locke wrote:
> I have recently written drivers for the Golden Software ASCII and
> Binary Grid formats that includes read, write, create, delete, and
> copy capabilities.  I was hoping to get some constructive criticism
> and, if no major problems are present, that it could be considered for
> inclusion in future releases of GDAL.
> 
> I have attached the diff against revision 10704 with my
> changes/additions.  If this is not the preferred form for submissions,
> please let me know.  [Correction:  Message got stuck waiting for
> moderator approval so now the patch is downloadable from
> http://kevinlocke.name/gdal/gdal-driver_for_gsg.patch ]

The patch looks fine to me, I just have few notes.

1. The binary driver could be derived from the RAWDataset class, just
   like the PNM driver. It prevents code duplication and provides us
   with free of charge Create() method. Also RAWDataset has several
   optimizations for speed. But the min/max collecting ability will be
   lost here, of course.

2. Ther are people which can be allergic to things like
   'std::numeric_limits<double>::max()' or 'dynamic_cast' :-)
   (BTW, why do you need dynamic_cast here?). Also we have CPLString
   class which is derived from std::string and properly wrapped to be
   portable. Please, use that one instead of std::string.

3. You don't need EXTRAFLAGS parameter in makefile.vc.

4. It is better to place both ASCII and binary modules in the same
   directory, just to be more compact.

5. RCS Id tag should look like $Id:$, otherwise it won't be recognized
   as tag.

> First, is there a portable file truncate function?  I didn't see any
> VSIFTruncate() or a commonly used alternative in existing drivers.  I
> was envisioning something that would call fileno() then ftruncate() or
> ftruncate64() on *nix systems and fileno() then _chsize() or
> _chsize_s() on Windows systems, but I didn't see anything similar in
> the source.  Also, is there a VSIFPrintfL() or similar function?

It seems we do not have the one, though it can be added.

> Next, I am concerned about how to best handle negative values in
> positions 1 or 5 of the GeoTransform.  The grid formats that I am
> working with always keep grid values in the same alignment (min X, min
> Y in the upper left corner), so when SetGeoTransform() is called with
> a negative value in these positions, what should be done?  At the
> moment I just push it to the PAM SetGeoTransform() if possible and
> error otherwise.  Would it make more sense to flip the rows and/or
> columns in the file; in which case the data would have the arrangement
> requested but the next call to GetGeoTransform() would still return
> positive numbers for positions 1 and 5 of the transform?  This makes a
> lot of sense to me (especially in order to translate from the Arc grid
> formats), but would this break anything expecting successful
> SetGeoTransform()s to mean that subsequent GetGeoTransform()s will
> return those values?

I do not think that driver itself should reorder data in any way, it is
a task for gdalwarp to change transformation matrix. I think your
current approach is fine.

Best regards,
Andrey

-- 
Andrey V. Kiselev
ICQ# 26871517