Incorrect orientation of ECW with Y increasing downward

[Mateusz Łoskot]

TL:TR; ECW driver seems to assume ECW images are always oriented with Y coordinates increasing "Upward".

// ecwdataset.cpp (r33717)
adfGeoTransform[5] = -fabs(psFileInfo->fCellIncrementY);

It applies to both, GDAL 2.0.0 as well as the current trunk (GDAL 2.2.0dev).

I have two adjacent ECW images:

• 02QD472.ecw - with Y coordinates increasing "Upward" (Y resolution negative)
• 02QD474.ecw - with Y coordinates increasing "Downward" (Y resolution positive)

Both images are referenced in ​http://epsg.io/2444 (JGD2000 / Japan Plane Rectangular CS II).

Attached screen shows the files info by ECWHeaderEditor from the latest ​ERDAS APOLLO Essentials Utilities 2014).

The problem is visible when inspecting corner coordinates with gdalinfo (trimmed output for clarity):

• 02QD472.ecw - correct coordinates reported

  Driver: ECW/ERDAS Compressed Wavelets (SDK 5.2)
  Files: ..\02QD472.ecw
  Origin = (-10000.100000000000364,-191999.899999999994179)
  Pixel Size = (0.200000000000000,-0.200000000000000)
  Corner Coordinates:
  Upper Left  (  -10000.100, -191999.900)
  Lower Left  (  -10000.100, -193499.900)
  Upper Right (   -8000.100, -191999.900)
  Lower Right (   -8000.100, -193499.900)
  Center      (   -9000.100, -192749.900)
  

• 02QD474.ecw - incorrect coordinates reported

  Driver: ECW/ERDAS Compressed Wavelets (SDK 5.2)
  Files: ..\02QD474.ecw
  Origin = (-10000.100000000000364,-194999.899999999994179)
  Pixel Size = (0.200000000000000,-0.200000000000000)
  Corner Coordinates:
  Upper Left  (  -10000.100, -194999.900)
  Lower Left  (  -10000.100, -196499.900)
  Upper Right (   -8000.100, -194999.900)
  Lower Right (   -8000.100, -196499.900)
  Center      (   -9000.100, -195749.900)
  

Loading the two images with QGIS (screen attached) also shows, they are not adjacent.

In order to fix it, patch is very simple: removed the -fabs call wraping the fCellIncrementY value. Once fixed, gdalinfo reports correct corner coordinates for the "Downward" 02QD474.ecw:

Driver: ECW/ERDAS Compressed Wavelets (SDK 5.2)
Files: ..\02QD474.ecw
Origin = (-10000.100000000000364,-194999.899999999994179)
Pixel Size = (0.200000000000000,0.200000000000000)
Corner Coordinates:
Upper Left  (  -10000.100, -194999.900)
Lower Left  (  -10000.100, -193499.900)
Upper Right (   -8000.100, -194999.900)
Lower Right (   -8000.100, -193499.900)
Center      (   -9000.100, -194249.900)

The two sample ECW images are 20MB each, so I here is the download URL to single 38MB .7z package with both of them: ​https://onedrive.live.com/redir?resid=54EB175E9AC78CE0!1483&authkey=!AMScTQpD5ytTmpQ&ithint=file%2c7z

[Mateusz Łoskot]

ECW files info reported by ECWHeaderEditor

[Mateusz Łoskot]

Incorrect image placement in QGIS (with GDAL).

[Mateusz Łoskot]

Simple patch fixing orientation of the two sample ECW images (linked in the ticket)

[Even Rouault]

Mateusz, digging into history, the current behaviour was introduced in r21590 per #393 .

It doesn't look like there is a universal way of addressing the oddly encoded ECW (apparently by ERDAS Imagine) of #393, and your samples. Apart from a config option (ECW_IGNORE_PIXEL_HEIGHT_SIGN ? Would default to TRUE to keep current behaviour) that would decide which behaviour to apply. On the other hand, one could argue that adfGeoTransform[5] = psFileInfo->fCellIncrementY; is the reasonable behaviour and the change introduced in #393 for odd files. I have no idea which ECW are more widespread : ones that are south-up with a positive psFileInfo->fCellIncrementY (your samples), or ones that are north-up with a positive psFileInfo->fCellIncrementY (ERDAS ones ?)

[Jukka Rahkonen]

Is #4977 related or not?

[Mateusz Łoskot]

Even, I'd not reached the history back that far to come across that ticket.

Your point is clear.

Assuming you are hesitant to change the current behaviour, a config option or compile-time #define controlled via nmake.opt/configure will work for me.

Let me know which option you suggest/prefer and I can implement it.

[Even Rouault]

I'd say let's add a boolean config option ECW_ALWAYS_NORTH_UP (perhaps clearer naming that my previous suggestion?), that defaults to TRUE, and does adfGeoTransform[5] = -fabs(psFileInfo->fCellIncrementY) as currently. If set to FALSE, then do: adfGeoTransform[5] = psFileInfo->fCellIncrementY. Some word about it in the doc page could be welcome.

[Mateusz Łoskot]

Patch with proposed new config option ECW_ALWAYS_UPWARD

[Mateusz Łoskot]

Copy of spif83.ecw test file with modified orientation to "Downward" (using ECWHeaderEditor) required by gdal-ticket-6516-option-ECW_ALWAYS_UPWARD.patch​

[Even Rouault]

The following lines in ecw_48() look to me as they are going to fail if ECW_ALWAYS_UPWARD is not defined (which is the default behaviour) :

2085	    ecw_upward = gdal.GetConfigOption('ECW_ALWAYS_UPWARD')
2086	    if ecw_upward != 'TRUE' and ecw_upward != 'ON':
2087	        gdaltest.post_reason( 'ECW_ALWAYS_UPWARD default value must be TRUE.' )
2088	        return 'fail'

Perhaps that should rather be the following ?

if gdal.GetConfigOption('ECW_ALWAYS_UPWARD') is not None:
    print('Skipping test about default behaviour because ECW_ALWAYS_UPWARD is defined')
    return 'skip'

Otherwise looks good to me. I let you apply.

[Mateusz Łoskot]

Thanks for catching that.

I don't think skipping just because ECW_ALWAYS_UPWARD is defined and without verification the "Upward" policy is forced for any image, including "Downward" ones, is a good idea.

I think, gdal.GetConfigOption('ECW_ALWAYS_UPWARD', 'TRUE') is what I was aiming for.

[Mateusz Łoskot]

Patch submitted to trunk (r34282).

I'm waiting for build(s), then resolve the test issues, if any happen.

[Mateusz Łoskot]

Even, If all looks good, do you mind if I apply this to branches/2.0 and branches/2.1?

[Even Rouault]

If all looks good, do you mind if I apply this to branches/2.0 and branches/2.1?

Please do

[Mateusz Łoskot]

Merged into branches/2.0 (r34286) and branches/2.1 (r34287)

[Even Rouault]

Hum, CPLTestBool() is not available in 2.0. You should use CSLTestBoolean

[Mateusz Łoskot]

Refined in r34288.

ATM, I have to rely on CI services to check the 2.x lines (could sent a pull request).

Thanks!