ogr2ogr kml export of polygons can have incorrect vertex winding order

[mashbridge]

Exporting to KML via ogr2ogr, particularly where the source data is an ESRI Shapefile, often results in an incorrect polygon winding order.

3D implementations of KML use the vertex winding order of a polygon to calculate lighting. The "front" of a face is defined as a face with an anti-clockwise winding order (positive surface normal). I have attached two images showing the effect of correct and incorrect winding orders.

[mashbridge]

screenshot of exported poly with incorrect (clockwise) winding order

[mashbridge]

screenshot of polygon with corrected (anti-clockwise) winding order

[mashbridge]

patch for top of trunk

[mashbridge]

I created a patch for the current TOT. I regret to say that this evening's experimentation is the totality of my familiarity with the GDAL codebase. Apologies for the inevitable naivety of my fix.

Briefly, I:

- added a generic reverseWindingOrder() method to the OGRLinearRing class in ogr/ogr_geometry.h - which is implemented in ogr/ogrlinearring.cpp as an efficient array swap - and called in ogr/ogrsf_frmts/kml/ogr2kmlgeometry.cpp if the winding order is determined to be clockwise

No doubt there are many issues of which I'm unaware, but I hope this helps a little.

[warmerdam]

Jens / Matuesz,

Could you jointly review this patch for appropriateness?

[mloskot]

I'm pasting e-mail from Jens:

Jens Oberender wrote:
> The patch looks fine, clear and efficient.
> I haven't testet it with data,
> but tried the algorithm with pen and paper.
> 
> Ciao,
> 	Jens

Jens, thanks for the review. I will test it.

[mloskot]

Clockwise order of KML polygon coords generated with ogr2ogr before applying the patch

[mloskot]

Correct KML polygon with anti-clockwise coords generated with patched OGR

[mloskot]

I've also tested the patch. It works correct, so it's been applied (r1185).

I'm closing this Ticket.

[mashbridge]

Cool. Thanks guys.

[mloskot]

One note of clarification. The fix was applied to the trunk as r11885, but not r1185.

[mloskot]

The fix has been ported to branches/1.4 (r12517)

[mloskot]

This patch has broken the C++ ABI so it should be removed from 1.4.x line. More details here: [ http://lists.osgeo.org/pipermail/gdal-dev/2007-November/014975.html GDAL/OGR 1.4.3 ABI incompatability with 1.4.2}

[mloskot]

The patch has been removed from branches/1.4 (r12921). It's still available in trunk and targets 1.5.0.