Opened 3 years ago

Closed 3 years ago

Last modified 2 years ago

#5129 closed enhancement (fixed)

Patches that implement RFC 40

Reported by: gillins Owned by: warmerdam
Priority: normal Milestone: 1.11.0
Component: default Version: svn-trunk
Severity: normal Keywords:
Cc:

Description

This ticket tracks the implementation of RFC40.

All patches will be attached to the following page: http://trac.osgeo.org/gdal/wiki/rfc40_enhanced_rat_support

Currently the patches that update the definition of GDALRasterAttributeTable and affected drivers are attached.

Next I will submit changes to the HFA driver to implement on-demand reading/writing of the RAT. Finally, patches for the updated Python bindings will be submitted.

Change History (9)

comment:1 Changed 3 years ago by gillins

Patch added for changes to HFA driver along with test program. Output tested with Imagine 8.4.

comment:2 Changed 3 years ago by rouault

The duplication of code in HFARasterAttributeTable::Serialize() with the one of GDALDefaultRasterAttributeTable is not ideal. I'd suggest the code from GDALDefaultRasterAttributeTable to be moved back into GDALRasterAttributeTable and that HFARasterAttributeTable::Serialize() just checks that it is not too big before calling GDALRasterAttributeTable::Serialize()

As far as rfc40_tests_hfa.cpp, it would have been good if it had been written in Python, that way it could have been integrated into the Python autotest suite and it would also have tested the updated bindings. But as this might be a not-fun effort, I guess we could live with the current .cpp test integrated in the cpp tests, and some smaller testing of the new RAT Python API.

comment:3 Changed 3 years ago by gillins

Updated patches to remove duplication of Serialize() as suggested by rouault.

I am intending to submit a version of rfc40_tests_hfa.cpp in Python along with the updated Python bindings. I just wanted to have something to check what I had done already. So maybe hold off updating the test suite until I have done this. I should have something for you when I return from leave (in 2 weeks).

comment:4 Changed 3 years ago by gillins

I managed to finish off this stuff before I leave. I have updated all the patches - I found a problem where GDALRATValuesIOString was not implemented under the right name, and a problem where I hadn't allocated quite enough memory in the HFA driver.

Also attached to the RFC is a set of patches to implement WriteArray/ReadAsArray? on the attribute table. These calls map to ValuesIO. ChangesAreWrittenToFile?() is also supported.

I have also attached a version of the test program written in Python. Since Serialize() isn't supported in the Python bindings this call isn't tested.

I had hoped to do some more testing on other systems and compilers and tidy up the test script, but I will do this in a few weeks time.

Let me know if there are any issues you can see with this stuff as it stands.

comment:5 Changed 3 years ago by gillins

Just updated to the HFA patch to include the null byte in calculations of string length like SetDefaultRAT which also means I can remove +1 in memory allocate added before.

comment:6 Changed 3 years ago by rouault

All patches commited in trunk r26117. The rfc40_tests_hfa.py has been turned into a more classical GDAL autotest as autotest/gcore/hfa_rfc40.py

comment:7 Changed 3 years ago by rouault

I've noticed a compilation failure in the GeoRaster driver in the logs of http://www.gisinternals.com/sdk/build-output/vc7-20130630-4-12-31-64-vc7-dev.txt . Hopefully r26119 should fix it.

comment:8 Changed 3 years ago by gillins

  • Resolution set to fixed
  • Status changed from new to closed

Tested on Linux 64bit with gcc and Intel Compilers, Sparc Solaris 64 with Sun Compilers and Windows 32 with VS 2010.

Also tested with large RAT's that used to be problematic - much better performance now.

I'm closing this ticket - completed as far as I am concerned.

comment:9 Changed 2 years ago by rouault

  • Milestone changed from 2.0 to 1.11.0
Note: See TracTickets for help on using tickets.