Opened 11 years ago

Closed 11 years ago

Last modified 10 years ago

#5129 closed enhancement (fixed)

Patches that implement RFC 40

Reported by: Sam Gillingham 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 by Sam Gillingham, 11 years ago

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

comment:2 by Even Rouault, 11 years ago

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 by Sam Gillingham, 11 years ago

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 by Sam Gillingham, 11 years ago

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 by Sam Gillingham, 11 years ago

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 by Even Rouault, 11 years ago

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 by Even Rouault, 11 years ago

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 by Sam Gillingham, 11 years ago

Resolution: fixed
Status: newclosed

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 by Even Rouault, 10 years ago

Milestone: 2.01.11.0
Note: See TracTickets for help on using tickets.