ogr2ogr/shapefile fails to generate valid polygons with hole touching boundary

[llo@…]

Using ogr2ogr to generate a shapefile will produce invalid polygons with hole if the hole touches the outer-boundary in one point. It produces a simple polygon without any hole, and ONLY a self-intersecting boundary.

This ONLY applies to shapefiles, no problems found with postgis or tab-file writer.

Example polygon in WKT that cannot be converted to a valid shapefile: POLYGON ((0 0, 0 3, 3 3, 3 0, 0 0), (0 0, 1 2, 2 1, 0 0))

/Regards Lars

[fredc]

OGR shapefile patched for touching polygons

[fredc]

We (think we) fixed this bug for touching polygons : see attached file shpopen.c, from line 1925 to line 1980.

All polygon pairs were tested for inclusion of its first polygon starting vertex in the other polygon, with a non strict inclusion. So in your case, the outer ring starting vertex was incorrectly considered "inside" the inner ring.

=> the inclusion is now strict in shpopen.c

note : that issue can also appear with outer touching polygons.

[warmerdam]

Mateusz,

Can you look into this for 1.4.2? Once you are happy with the change, I can upstream it to the shapelib repository.

[Markus Neteler]

added myself in CC since GRASS 6 uses shpopen.c, too (lib/external/shapelib/).

Markus

[Mateusz Łoskot]

Lars,

BTW, Your polygon is invalid in terms of ​Shapefile Specification because the inner ring is clockwise instead of counterclockwise.

Certainly, it doesn't change the problem because translation of valid version:

POLYGON ((0 0,0 3,3 3,3 0,0 0), (0 0,2 1,1 2,0 0))

produces incorrect output:

MULTIPOLYGON (((0 0,0 3,3 3,3 0,0 0)),((0 0,1 2,2 1,0 0)))

Fixing it.

[Mateusz Łoskot]

The original attachement but in form of a patch

[Mateusz Łoskot]

Unfortunately, the patch attached to this Ticket doesn't solve the problem. I tested it taking following steps:

• Imported following sample polygon to PostGIS database

  POLYGON ((0 0, 0 3, 3 3, 3 0, 0 0), (0 0, 1 2, 2 1, 0 0))
  

• Dumped to Shapefile with pgsql2shp and here is the result:

D:\dev\gdal\bugs\976\data>ogrinfo 1415.shp 1415
Layer name: 1415
Geometry: Polygon
Feature Count: 1
Extent: (0.000000, 0.000000) - (3.000000, 3.000000)
Layer SRS WKT:
(unknown)
ID: Real (11.0)
INFO: String (120.0)
OGRFeature(1415):0
  ID (Real) =           4
  INFO (String) = Polygon with 1 clockwise hole touching the outer ring
  POLYGON ((0 0,0 3,3 3,3 0,0 0),(0 0,2 1,1 2,0 0))

As you see, the pgsql2shp rewinded the inner ring to make it counter-clockwise, according to the Shapefile Specification.

• Next, I converted my test file 1415.shp to another Shapefile with ogr2ogr patched using attached version of shpopen.c

D:\dev\gdal\bugs\976\data>ogr2ogr out.shp 1415.shp

• The result is still incorrect:

D:\dev\gdal\bugs\976\data>ogrinfo out.shp out

Layer name: out
Geometry: Polygon
Feature Count: 1
Extent: (0.000000, 0.000000) - (3.000000, 3.000000)
Layer SRS WKT:
(unknown)
ID: Real (11.0)
INFO: String (120.0)
OGRFeature(out):0
  ID (Real) =           4
  INFO (String) = Polygon with 1 clockwise hole touching the outer ring
  MULTIPOLYGON (((0 0,0 3,3 3,3 0,0 0)),((0 0,1 2,2 1,0 0)))

[warmerdam]

I don't know if it is an issue, but the code that determines if something should be treated as a POLYGON or MULTIPOLYGON as it is read from shapefile to OGR is in shape2ogr.cpp (see RingInRing(), etc). SHPRewind() is only used while writing.

I believe there could be similar issues in the shape2ogr.cpp code (written by another) and that there might be some sort of consolodation with SHPRewind() that could be accomplished.

[Mateusz Łoskot]

The patch applied (r11689) for Ticket #976 does also fix this bug.

I'm closing this bug, but if the problem still leaks, please reopen it. I'd be also thankful for confirmation if the bug has been fixed (or not).

[Mateusz Łoskot]

Test polygon from the ticket report and result of translation with fixed OGR.

[fredc]

I confirm that patch #1415 is not working either on my version, for the (correct) test shapefile you posted in the 1415_fix_test.zip.

The issue comes from the starting points of rings : the inner (triangle) ring starting point lies on the edge of the outer (square) ring, so the patch considers that it is outside of the outer ring, since it is now testing for strict inclusion => so none of the rings is reported as beeing inside of the other, which gives two outer rings (incorrect).

The previous behaviour, with normal inclusion, was to report that the square outer ring starting point (0,0) was inside of the triangle, and that conversely the trinagle starting point (0,0) also was inside of the square ring => that lead to double inversion, and generated two outer rings as well.

For info, I modified your test file 1415.shp so that the starting point of the triangle now lies completely inside of the square. => path 1415 now does its job (see attched gml file 1415g.gml I used to modify the starting point, and its conversion to shape 1415g.shp)

[fredc]

Modified 1415_test_fix.zip, so that the triangle starting point is completely inside of the square

[Mateusz Łoskot]

Replying to fredc:

I confirm that patch #1415 is not working either on my version, for the (correct) test shapefile you posted in the 1415_fix_test.zip. [...]

FYI, I tested your Shapefile and OGR with fix I applied (r11689) translates it correctly:

• Original file

D:\dev\gdal\bugs\976\data>ogrinfo 1415g.shp 1415g

Layer name: 1415g
Geometry: Polygon
Feature Count: 1
Extent: (0.000000, 0.000000) - (3.000000, 3.000000)
Layer SRS WKT:
(unknown)
ID: Real (11.0)
INFO: String (80.0)
OGRFeature(1415g):0
  ID (Real) =           4
  INFO (String) = Polygon with 1 clockwise hole touching the outer ring
  POLYGON ((0 0,0 3,3 3,3 0,0 0),(2 1,1 2,0 0,2 1))

• Translation with ogr2ogr

• Translation output looks as follows:

D:\dev\gdal\bugs\976\data>ogrinfo out.shp out

Layer name: out
Geometry: Polygon
Feature Count: 1
Extent: (0.000000, 0.000000) - (3.000000, 3.000000)
Layer SRS WKT:
(unknown)
ID: Real (11.0)
INFO: String (80.0)
OGRFeature(out):0
  ID (Real) =           4
  INFO (String) = Polygon with 1 clockwise hole touching the outer ring
  POLYGON ((0 0,0 3,3 3,3 0,0 0),(2 1,1 2,0 0,2 1))

Frederic, I assume the bug has been confirmed as fixed.

[Mateusz Łoskot]

Patch for Shapelib upstream with changeset r10975

[Mateusz Łoskot]

Patch for Shapelib upstream with changeset r11008

[zanollim]

I think that there is a correlated problem when importing shapefile in postgis with ogr2ogr. I try to explain my problem.

I have a shapefile that comes from an arcinfo coverage (more or less 5000 polygons). In this shapefile I have some polygons with this 'problem':

these polygons have an outer ring and an inner ring (hole) that touches (tangent) the outer ring in one point. This point is the starting point of the two rings.

with ogr2ogr these POLYGON are interpreted wrongly as MULTIPOLYGON

The solution is to change 'manually' the starting point of the inner ring with another point of the same ring completely inside of the outer ring, but I think this is not the best (final) solution of the problem.

michele