Skip to content

GDAL Dataset::SetGeoTransform updates #199

@bhawkins

Description

@bhawkins

In #196 @scottstanie helpfully fixed a compilation issue related to a change in the GDAL API introduced in version 3.12. Specifically, the GDALDataset virtual method

virtual CPLErr SetGeoTransform(double *padfTransform);

got modified to

virtual CPLErr SetGeoTransform(const GDALGeoTransform &gt);
CPLErr SetGeoTransform(const double *padfTransform);  // deprecated

with the idea that you're supposed to use the new GDALGeoTransform type instead of double*. Likewise for the getters.

The problem that @aivazis points out is that the deprecated method had virtual removed, so there's no way to override it in derived classes. Since IH5Dataset inherits from GDALDataset, in principle we don't have a way to guarantee correct behavior whenever something like this happens:

double gt[6];
GDALDataset* dset = new IH5Dataset();
dset->SetGeoTransform(gt);  // calls method of base class, not derived class

We care about this behavior because isce3::io::Raster calls GDALDataset::SetGeoTransform(double*) here:

int status = _dataset->SetGeoTransform(arr);

and we certainly do construct a Raster from an IH5Dataset.

In practice, the current GDAL implementation is to cast the array argument and call the SetGeoTransform(const GDALGeoTransform &) method:
https://github.com/OSGeo/gdal/blob/4ec9f5211bfe98855158d881b6170a2ddfe5db68/gcore/gdaldataset.cpp#L1759
I think that means we get basically the behavior we want in Raster.

I'm not sure what the cleanest fix going forward might be.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions