[gdal-dev] ENVI Rotation Bug Fix

Even Rouault even.rouault at spatialys.com
Wed Mar 1 06:15:56 PST 2017


Kurt,

I don't think the ENVIDataset class is exported (and that's a good thing), so direct testing of 
the class might only be possible if you build without --with-hide-internal-symbols.

Why can't you test through the public GDAL API instead of directly depnding on 
implementation details ? Actually looking at the class definitions of the ENVI driver, the 
only public methods are virtual methods from the public API, so there's no advantage on 
directly depending on it.

Even

> Warning... I did a cleanup pass and split out the class definitions into a
> header to allow direct C++ testing.
> 
> https://trac.osgeo.org/gdal/changeset/37530
> https://trac.osgeo.org/gdal/changeset/37514
> https://trac.osgeo.org/gdal/changeset/37510
> 
> I haven't written that testing yet, but it will be similar to this:
> 
> https://github.com/schwehr/gdal-autotest2/blob/master/cpp/frmts/jp2kak/jp2ka
> kdataset_test.cc
> 
> -kurt
> 
> On Wed, Mar 1, 2017 at 2:56 AM, Damian Dixon <damian.dixon at gmail.com> wrote:
> > Hi Taylor,
> > 
> > M_PI is defined pretty much everywhere. The only compiler that you have to
> > jump through hoops to get it is Visual Studio where you have to define
> > '#define _USE_MATH_DEFINES'.
> > 
> > As Even has mentioned it's also defined in the GDAL portability header.
> > 
> > The main reason the use of atan jumped out is that I've run into problems
> > with atan on Intel CPUs and GPUs in the past. Also the use of trig' of any
> > sort is expensive so I tend to take a look to see if it can be removed or
> > moved.
> > 
> > I spotted that this is a very old bug so I'm pleased that you've taken the
> > time to fix it.
> > 
> > Regards
> > Damian
> > 
> > 
> > 
> > On 27 February 2017 at 22:12, Brown, Taylor Alexander <
> > 
> > browtayl at oregonstate.edu> wrote:
> >> Damian,
> >> 
> >> I agree that it would be preferable to use a predefined constant.
> >> However, I was under the impression that M_PI is a compiler extension and
> >> not part of standard C++, so I wanted to make sure it was compatible. If
> >> using M_PI is acceptable, I would be happy to change it. I will be
> >> submitting my pull request shortly.
> >> 
> >> 
> >> Taylor
> >> 
> >> On Mon, Feb 27, 2017 at 5:01 AM, Damian Dixon <damian.dixon at gmail.com>
> >> 
> >> wrote:
> >>> Hi Taylor,
> >>> 
> >>> I'm just lurking on the mailing list myself.
> >>> 
> >>> I had a look at the code changes, as this is one of the formats I'm
> >>> interested in, and one point springs out at me:
> >>> 
> >>> const double PI = atan(1.0) * 4;
> >>> 
> >>> I find this line potentially a confusing way to define PI, particularly
> >>> when you combine it with (PI/180) later.
> >>> 
> >>> Personally I would use M_PI.
> >>> 
> >>> Regards
> >>> Damian
> >>> 
> >>> 
> >>> 
> >>> On 27 February 2017 at 11:55, Taylor Alexander Brown <
> >>> 
> >>> browtayl at oregonstate.edu> wrote:
> >>>> Greetings GDAL Community,
> >>>> 
> >>>> I am developing an application [0] to process georeferenced raster
> >>>> hyperspectral imagery in ENVI format from NASA's Airborne
> >>>> Visual/Infrared Imaging Spectrometer (AVIRIS) [1].
> >>>> 
> >>>> The `map info` section of the header files contain an undocumented [2]
> >>>> rotation field which is currently ignored by GDAL. As a result, the
> >>>> image is rotated incorrectly when I process it with `gdalwarp` or the
> >>>> GDAL Python API. Furthermore, the image is scaled incorrectly when I
> >>>> try
> >>>> to override the geotransform.
> >>>> 
> >>>> I documented my experiences on the GIS Stack Exchange [3]. It has
> >>>> previously been described on the GDAL mailing list [4] and bug tracker
> >>>> [5]. I believe this issue also affects ASTER imagery [6][7].
> >>>> 
> >>>> I have implemented a fix based off of branch `tags/2.1.3` and shared my
> >>>> code on GitHub [8]. It reads the `units` and `rotation` parameter from
> >>>> the `map info` section and uses these to calculate the geotransform.
> >>>> The
> >>>> scaling was off because the code expected the `units` parameter to be
> >>>> the last element in the list, when in my case it was the second to
> >>>> last.
> >>>> You can verify this behavior with the single-band image I have been
> >>>> using for testing [9].
> >>>> 
> >>>> You are welcome to evaluate my code and contribute it back to the
> >>>> library. I am a novice at both GDAL and GIS, so there may well be
> >>>> problems. I would be willing to submit a pull request against your
> >>>> GitHub mirror if desired.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/gdal-dev/attachments/20170301/e44c2b29/attachment-0001.html>


More information about the gdal-dev mailing list