[PATCH] New OGR driver for Atlas BNA file format

[rouault]

"Release early, release soon." So, here is a first scratch of an OGR driver for the Atlas BNA file format. Currently, it only supports read-only and doesn't handle correctly complex polygons or ellipses.

After fixing that, next features could be :

• parse an accompagnying CSV data file
• write support

[rouault]

Updated patch with support of ellipses and complex polygons. Complex polygon support is not 100% OK however, as I'm not sure how to translate the following situation : "small lake 'D' in the middle of an island 'C' in the middle of a lake 'B' in the middle of an area 'A'" into OGRGeometry. In BNA, it's a single feature and you can detect area from holes by the winding.

The quite old Atlas MapMaker? (by Golden Software, 1991) can handle this like a charm and export/import it into/from BNA.

It's clear in my mind that in OGRGeometry philosophy, the outline of A is the 'external ring' of a polygon and that B is one inner ring of it. But what is C ? another polygon at the same level of A (thus forming a multipolygon) ? And D an inner ring of C ? But aren't C and A supposed to be disjoint to form a multipolygon, which is not the case here.

[warmerdam]

Evan,

In the Simple Features geometry model used by OGR, an island with a lake appearing inside the lake of an enclosing island would be represented as a multipolygon containing two polygons - one for each island. The fact that the second polygon is nested inside a lake of the first polygon is not implicit in the geometry construction though it can be deduced from analysing the spatial coordinates.

Also, can you confirm that this code, and your contributions in other tickets is being contributed to the project in keeping with the "Legal" section of:

http://trac.osgeo.org/gdal/wiki/rfc3_commiters

That is that you have the right to provide it (ie. support of employers, rights to code etc) and that you understand it will be licensed under the GDAL MIT/X license? Once I have that I'll go ahead and commit the BNA driver. I'd really appreciate some sample data - hopefully of modest size - so I can setup automated test scripts for the driver.

Thanks,

[rouault]

I undestrand your concern about legal issues and I'm going to answer it the best I can. I fully understand what the GDAL MIT/X licence implies.

Concerning my different contributions to GDAL/Mapserver source code , there are in fact 3 cases :

• small/medium patches/fixes done on my work time that my employer is OK to release (saves the burden of keeping and updating corporate patches on our side). This is the case for the patches for the DTED driver, nodata handling in various places, etc..
• more substantial enhancements that my employer is not ready (yet ?) to share with the community. So for the moment and until a change in corporate policy, we keep it private...
• work done on my own free time, not related to my employer in any kind. That is the case for the BNA driver for example. It's written from scratch by myself from the 'BNA specification' and data files. So I have full rights on it.

All in all, what has been released until know and in the future can be considered as safe from the legal point of view for inclusion into GDAL.

---

Thank you for your explanation about the geometry model. I've updated the patch accordingly. I've extended the parser to support more exotic BNA files (it's incredible the number of variations you may find in the wild...)

I'm attaching a small sample file with 2 polygons with lakes and islands in the lakes..., 1 polyline, 2 points and 1 circle. The format of this file is supposed to be the most standard one for a BNA file. It can be imported succesfully by MapViewer?(TM) by Golden Software (demo version available on their website) I've also attached a GIF snapshot. Otherwise there are real data BNA files that you can found on this FTP site :  ftp://ftp.ciesin.org/pub/census/usa/tiger/. However I've not checked whether their IP status allows for redistribution. (The BNA format of these files obliged me to extend the parser)

[warmerdam]

Even,

That all sounds great.

I have committed the latest patch into GDAL/OGR trunk and it seems to build and operate just fine on linux. Could you prepare some documentation for the BNA driver?

[warmerdam]

Even,

I did a quick run of test_ogrsf (a custom testing utility you can build in the gdal/ogr directory by typing "make test_ogrsf") on the test file and it reports:

warmerda@amd64[154]% test_ogrsf -ro test.bna 
OGR: OGROpen(test.bna/0x507f10) succeeded as BNA.
INFO: Open of `test.bna' using driver `BNA' successful.
INFO: Testing layer test_points.
ERROR: Claimed feature count 0 doesn't match actual, 2.
INFO: Feature/layer spatial ref. consistency verified.
INFO: Spatial filter inclusion seems to work.
ERROR: Spatial filter failed to eliminatea feature unexpectedly!
INFO: Testing layer test_lines.
ERROR: Claimed feature count 0 doesn't match actual, 1.
INFO: Feature/layer spatial ref. consistency verified.
INFO: Spatial filter inclusion seems to work.
ERROR: Spatial filter failed to eliminatea feature unexpectedly!
INFO: Testing layer test_polygons.
INFO: Feature count verified.
INFO: Feature/layer spatial ref. consistency verified.
INFO: Spatial filter inclusion seems to work.
ERROR: Spatial filter failed to eliminatea feature unexpectedly!
INFO: Testing layer test_ellipses.
ERROR: Claimed feature count 0 doesn't match actual, 1.
INFO: Feature/layer spatial ref. consistency verified.
INFO: Spatial filter inclusion seems to work.
ERROR: Spatial filter failed to eliminatea feature unexpectedly!

I haven't reviewed the code closely, but it seems a number of behaviors required of OGR drivers are not correctly implemented. If you like I can dig into these, but I thought I would give you the first crack at it.

[warmerdam]

Very modest regression test added in gdalautotest/ogr/ogr_bna.py (r11854).

[rouault]

I'm going to look at the test_ogrsf error report. Actually I didn't expect that you would have commited the patch as soon! It may indeed require some more polishing and testing from mine (I'm not yet sure about my handling of the complex polygon for example). I just send it and update it in order to get early feedback on its behaviour or design (like your test_ogrsf error report ;-)).

[warmerdam]

Even,

Commit early, commit often. That's my theory.

[rouault]

Patch updated (still based on 1.4.2 for the moment) :

• I didn't take into accout spatial filter in GetNextFeature?(). Fixed
• I provided a custom implementation of GetFeatureCount?() that didn't take into accout spatial filter. Removed : the base class GetFeatureCount?() is doing the job correctly and as BNA is in no way spatial indexed, I can't do better
• Change in the complex polygon case (I'm not yet satisfied with it) but should be doing a better job. By the way, I didn't realize that Intersects method on OGRLinearRing always return FALSE because WkbSize?() is redefined and always returns 0... It might be obvious that this use makes no sense, but probably a warning should be emitted somewhere when hitting that execution path. Or at least, the online doc ( http://www.gdal.org/ogr/classOGRLinearRing.html) updated because it doesn't match with the source code.

Now, test_ogrsf is successful on my simple data set and on more complex ones

[warmerdam]

Even,

I have added some notes to the OGRLinearRing docs on this issue. WkbSize?() returns zero because there is no established WKB representation for OGRLinearRing.

Please ensure further work is from the version of the BNA code in SVN. I had to make a number of changes to get it to build on windows, etc.

[rouault]

Here's an updated version of the patch against SVN. Changes :

• add latest changes made against 1.4.2
• add two methods to OGREnvelope (I hope it doesn't break C++ ABI)
• make the BNA parser a bit more tolerant again
• and, a complete change in the handling of complex polygons. I read bug #1217 and think that I was facing the same kinds of issues. I won't explain here in details how I solved the problem but I think I've tried to comment it into enough details in the new method MakeMultiPolygon?. Basically, I'm completely ignoring CW or CCW stories and only consider geometric relationships. It is very general and could probably be put upper in the OGR tree if it is seems right to other people (computation cost might be a major problem). I have also attaching a new test file for studying that case with simple coordinates so that one can convince oneself that the algorithm is doing well (at least on that case). On test.bna, it's difficult to see if the algorithm is doing well and when I convert it into SHP and displays it into QGIS for example, there's something wrong (the lake on the left red polygon is filled). I think it hits bug #1217 as the ogrinfo on the BNA shows 4 toplevel polygons in the red multipolygon (as expected), but ogrinfo on the converted SHP shows 5 toplevel polygons.

[rouault]

Updated patch with write support and HTML documentation

[warmerdam]

Even,

Please go ahead and patch the BNA driver as desired. No real need to keep this ticket up to date unless you want to.

[rouault]

Commited in trunk in revision r11894

[mloskot]

FYI, I added BNA driver to Vector Formats in the Autotest Status.