Opened 17 months ago

Closed 16 months ago

Last modified 16 months ago

#6516 closed defect (fixed)

Incorrect orientation of ECW with Y increasing downward

Reported by: Mateusz Łoskot Owned by: Mateusz Łoskot
Priority: normal Milestone:
Component: default Version: svn-trunk
Severity: normal Keywords: ecw
Cc:

Description (last modified by 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

Attachments (5)

ECWHeaderEditor.png (22.0 KB) - added by Mateusz Łoskot 17 months ago.
ECW files info reported by ECWHeaderEditor
qgis-2.14.2-ecw-not-adjacent.png (445.9 KB) - added by Mateusz Łoskot 17 months ago.
Incorrect image placement in QGIS (with GDAL).
ecw-orientation-bug.patch (531 bytes) - added by Mateusz Łoskot 17 months ago.
Simple patch fixing orientation of the two sample ECW images (linked in the ticket)
gdal-ticket-6516-option-ECW_ALWAYS_UPWARD.patch (4.3 KB) - added by Mateusz Łoskot 16 months ago.
Patch with proposed new config option ECW_ALWAYS_UPWARD
spif83_downward.ecw (21.4 KB) - added by Mateusz Łoskot 16 months ago.
Copy of spif83.ecw test file with modified orientation to "Downward" (using ECWHeaderEditor) required by gdal-ticket-6516-option-ECW_ALWAYS_UPWARD.patch

Download all attachments as: .zip

Change History (21)

Changed 17 months ago by Mateusz Łoskot

Attachment: ECWHeaderEditor.png added

ECW files info reported by ECWHeaderEditor

Changed 17 months ago by Mateusz Łoskot

Incorrect image placement in QGIS (with GDAL).

Changed 17 months ago by Mateusz Łoskot

Attachment: ecw-orientation-bug.patch added

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

comment:1 Changed 17 months ago by Mateusz Łoskot

Description: modified (diff)

comment:2 Changed 17 months ago by Mateusz Łoskot

Description: modified (diff)

comment:3 Changed 17 months ago by 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 ?)

comment:4 Changed 17 months ago by Jukka Rahkonen

Is #4977 related or not?

comment:5 Changed 17 months ago by 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.

comment:6 Changed 17 months ago by 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.

Changed 16 months ago by Mateusz Łoskot

Patch with proposed new config option ECW_ALWAYS_UPWARD

Changed 16 months ago by Mateusz Łoskot

Attachment: spif83_downward.ecw added

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

comment:7 Changed 16 months ago by 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.

comment:8 Changed 16 months ago by 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.

comment:9 Changed 16 months ago by Mateusz Łoskot

Patch submitted to trunk (r34282).

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

comment:10 Changed 16 months ago by Mateusz Łoskot

Owner: changed from warmerdam to Mateusz Łoskot

comment:11 Changed 16 months ago by Mateusz Łoskot

Resolution: fixed
Status: newclosed

comment:12 Changed 16 months ago by Mateusz Łoskot

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

comment:13 in reply to:  12 Changed 16 months ago by Even Rouault

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

Please do

comment:14 Changed 16 months ago by Mateusz Łoskot

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

comment:15 Changed 16 months ago by Even Rouault

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

comment:16 Changed 16 months ago by 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!

Note: See TracTickets for help on using tickets.