Skip to content

Commit 2d9b34c

Browse files
DGuidiDGuidi
committed
introduced conditional flag to handle new polygon builder logic
TODO: flag needs to be handled differently, suggestions well accepted
1 parent 6dd104a commit 2d9b34c

File tree

3 files changed

+135
-46
lines changed

3 files changed

+135
-46
lines changed

‎src/NetTopologySuite.IO.ShapeFile/Handlers/PolygonHandler.cs‎

Lines changed: 100 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@ namespace NetTopologySuite.IO.Handlers
1212
/// </summary>
1313
public class PolygonHandler : ShapeHandler
1414
{
15+
public static bool ExperimentalPolygonBuilderEnabled { get; set; }
16+
17+
//Thanks to Bruno.Labrecque
18+
private static readonly ProbeLinearRing ProbeLinearRing = new ProbeLinearRing();
19+
1520
public PolygonHandler() : base(ShapeGeometryType.Polygon)
1621
{
1722
}
@@ -27,8 +32,6 @@ public PolygonHandler(ShapeGeometryType type)
2732
/// <param name="totalRecordLength">Total length of the record we are about to read</param>
2833
/// <param name="factory">The geometry factory to use when making the object.</param>
2934
/// <returns>The Geometry object that represents the shape file record.</returns>
30-
/// <exception cref="InvalidOperationException"></exception>
31-
/// <exception cref="ShapefileException"></exception>
3235
public override Geometry Read(BigEndianBinaryReader file, int totalRecordLength, GeometryFactory factory)
3336
{
3437
int totalRead = 0;
@@ -85,6 +88,92 @@ public override Geometry Read(BigEndianBinaryReader file, int totalRecordLength,
8588
// Geometries via CoordinateSequence further down.
8689
GetZMValues(file, totalRecordLength, ref totalRead, buffer, skippedList);
8790

91+
var polys = !ExperimentalPolygonBuilderEnabled
92+
? InternalBuildPolygons(factory, buffer)
93+
: InternalBuildPolygonsExperimental(factory, buffer);
94+
95+
if (polys.Length == 0)
96+
geom = factory.CreatePolygon();
97+
else if (polys.Length == 1)
98+
geom = polys[0];
99+
else
100+
geom = factory.CreateMultiPolygon(polys);
101+
return geom;
102+
}
103+
104+
private static Polygon[] InternalBuildPolygons(GeometryFactory factory, CoordinateBuffer buffer)
105+
{
106+
// Get the resulting sequences
107+
var sequences = buffer.ToSequences(factory.CoordinateSequenceFactory);
108+
var shells = new List<LinearRing>();
109+
var holes = new List<LinearRing>();
110+
for (int i = 0; i < sequences.Length; i++)
111+
{
112+
//Skip garbage input data with 0 points
113+
if (sequences[i].Count < 1) continue;
114+
115+
var tmp = EnsureClosedSequence(sequences[i], factory.CoordinateSequenceFactory);
116+
if (tmp == null) continue;
117+
var ring = factory.CreateLinearRing(tmp);
118+
if (ring.IsCCW)
119+
holes.Add(ring);
120+
else
121+
shells.Add(ring);
122+
}
123+
124+
// Ensure the ring is encoded right
125+
if (shells.Count == 0 && holes.Count == 1)
126+
{
127+
shells.Add(factory.CreateLinearRing(holes[0].CoordinateSequence.Reversed()));
128+
holes.Clear();
129+
}
130+
131+
// Now we have lists of all shells and all holes
132+
var holesForShells = new List<List<LinearRing>>(shells.Count);
133+
for (int i = 0; i < shells.Count; i++)
134+
holesForShells.Add(new List<LinearRing>());
135+
136+
//Thanks to Bruno.Labrecque
137+
//Sort shells by area, rings should only be added to the smallest shell, that contains the ring
138+
shells.Sort(ProbeLinearRing);
139+
140+
// Find holes
141+
foreach (var testHole in holes)
142+
{
143+
var testEnv = testHole.EnvelopeInternal;
144+
var testPt = testHole.GetCoordinateN(0);
145+
146+
//We have the shells sorted
147+
for (int j = 0; j < shells.Count; j++)
148+
{
149+
var tryShell = shells[j];
150+
var tryEnv = tryShell.EnvelopeInternal;
151+
bool isContained = tryEnv.Contains(testEnv) && PointLocation.IsInRing(testPt, tryShell.Coordinates);
152+
153+
// Check if this new containing ring is smaller than the current minimum ring
154+
if (isContained)
155+
{
156+
// Suggested by Brian Macomber and added 3/28/2006:
157+
// holes were being found but never added to the holesForShells array
158+
// so when converted to geometry by the factory, the inner rings were never created.
159+
var holesForThisShell = holesForShells[j];
160+
holesForThisShell.Add(testHole);
161+
162+
//Suggested by Bruno.Labrecque
163+
//A LinearRing should only be added to one outer shell
164+
break;
165+
}
166+
}
167+
}
168+
169+
var polygons = new Polygon[shells.Count];
170+
for (int i = 0; i < shells.Count; i++)
171+
polygons[i] = factory.CreatePolygon(shells[i], holesForShells[i].ToArray());
172+
return polygons;
173+
}
174+
175+
private static Polygon[] InternalBuildPolygonsExperimental(GeometryFactory factory, CoordinateBuffer buffer)
176+
{
88177
// Get the resulting sequences
89178
var sequences = buffer.ToSequences(factory.CoordinateSequenceFactory);
90179
// Read all rings
@@ -105,6 +194,12 @@ bool IsHoleContainedInShell(LinearRing shell, LinearRing hole) =>
105194
shell.EnvelopeInternal.Contains(hole.EnvelopeInternal)
106195
&& PointLocation.IsInRing(hole.GetCoordinateN(0), shell.Coordinates);
107196

197+
// Utility function to ensure that shell and holes are correctly oriented
198+
LinearRing EnsureOrientation(LinearRing ring, bool asShell) =>
199+
ring.IsCCW
200+
? !asShell ? ring : ring.Factory.CreateLinearRing(ring.CoordinateSequence.Reversed())
201+
: asShell ? ring : ring.Factory.CreateLinearRing(ring.CoordinateSequence.Reversed());
202+
108203
// Sort rings by area, from bigger to smaller
109204
rings = rings.OrderByDescending(r => r.Factory.CreatePolygon(r).Area).ToList();
110205

@@ -150,34 +245,9 @@ bool IsHoleContainedInShell(LinearRing shell, LinearRing hole) =>
150245
}
151246
}
152247

153-
var polygons = data.Select(t => factory.CreatePolygon(
154-
t.shell, t.holes.ToArray())).ToArray();
155-
if (polygons.Length == 0)
156-
geom = factory.CreatePolygon();
157-
else if (polygons.Length == 1)
158-
geom = polygons[0];
159-
else
160-
geom = factory.CreateMultiPolygon(polygons);
161-
return geom;
162-
}
163-
164-
165-
private LinearRing EnsureOrientation(LinearRing ring, bool asShell)
166-
{
167-
if (ring == null)
168-
throw new ArgumentNullException(nameof(ring));
169-
170-
if (ring.IsCCW)
171-
return asShell
172-
? ring.Factory.CreateLinearRing(
173-
ring.CoordinateSequence.Reversed())
174-
: ring;
175-
176-
return !asShell
177-
? ring.Factory.CreateLinearRing(
178-
ring.CoordinateSequence.Reversed())
179-
: ring;
180-
248+
return data
249+
.Select(t => factory.CreatePolygon(t.shell, t.holes.ToArray()))
250+
.ToArray();
181251
}
182252

183253
/// <summary>

‎src/NetTopologySuite.IO.ShapeFile/Handlers/ProbeLinearRing.cs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
using System;
21
using System.Collections.Generic;
32
using NetTopologySuite.Geometries;
43

@@ -8,7 +7,6 @@ namespace NetTopologySuite.IO.Handlers
87
/// Serves to probe linear rings
98
/// </summary>
109
/// <author>Bruno.Labrecque@mddep.gouv.qc.ca</author>
11-
[Obsolete("unused", true)]
1210
internal class ProbeLinearRing : IComparer<LinearRing>
1311
{
1412

@@ -19,7 +17,7 @@ internal enum Order
1917
}
2018

2119
internal ProbeLinearRing()
22-
:this(Order.Descending)
20+
: this(Order.Descending)
2321
{
2422
}
2523

‎test/NetTopologySuite.IO.ShapeFile.Test/Issue70Fixture.cs‎

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,50 +1,71 @@
11
using NUnit.Framework;
22
using System.IO;
33
using NetTopologySuite.Geometries;
4-
using System;
54
using System.Linq;
65
using System.Collections.Generic;
6+
using NetTopologySuite.IO.Handlers;
7+
using System;
78

89
namespace NetTopologySuite.IO.ShapeFile.Test
910
{
1011
[TestFixture]
1112
[ShapeFileIssueNumber(70)]
1213
public class Issue70Fixture
1314
{
15+
[TearDown]
16+
public void AfterEachTestExecution()
17+
{
18+
Console.WriteLine("disabled");
19+
PolygonHandler.ExperimentalPolygonBuilderEnabled = false;
20+
}
21+
1422
/// <summary>
15-
/// <see href="https://github.com/NetTopologySuite/NetTopologySuite.IO.ShapeFile/issues/70"/>
23+
/// The shell_bad_ccw.shp contains a single polygon, with:
24+
/// - a *shell* CCW-oriented(like a hole from ESRI specs
25+
/// - a *hole* CW-oriented (like a shell from ESRI specs)
1626
/// </summary>
17-
[Test]
18-
public void TestReadPolygonWithWrongShellOrientation()
27+
private static Polygon ReadPolyBadlyOriented()
1928
{
20-
/*
21-
* The shell_bad_ccw.shp contains a single polygon, with:
22-
* - a shell CCW-oriented (like a hole from ESRI specs
23-
* - a hole CW-oriented (like a shell from ESRI specs)
24-
*/
2529
string filePath = Path.Combine(
2630
CommonHelpers.TestShapefilesDirectory,
2731
"shell_bad_ccw.shp");
2832
Assert.That(File.Exists(filePath), Is.True);
2933
string filePathWoExt = Path.Combine(
3034
Path.GetDirectoryName(filePath),
3135
Path.GetFileNameWithoutExtension(filePath));
32-
using var shpReader = new ShapefileDataReader(
33-
filePathWoExt,
34-
GeometryFactory.Default);
36+
var shpReader = Shapefile.CreateDataReader(filePathWoExt, GeometryFactory.Default);
3537
bool success = shpReader.Read();
3638
Assert.That(success, Is.True);
3739
var geom = shpReader.Geometry;
3840
Assert.That(geom, Is.Not.Null);
3941
Assert.That(geom.IsValid, Is.True);
4042
Assert.That(geom.NumGeometries, Is.EqualTo(1));
4143
Assert.That(geom, Is.InstanceOf<Polygon>());
42-
var poly = (Polygon)geom.GetGeometryN(0);
44+
return (Polygon)geom.GetGeometryN(0);
45+
}
46+
47+
/// <summary>
48+
/// <see href="https://github.com/NetTopologySuite/NetTopologySuite.IO.ShapeFile/issues/70"/>
49+
/// </summary>
50+
[Test]
51+
public void TestReadPolygonWithWrongShellOrientationReadsHoleWithFlagEnabled()
52+
{
53+
PolygonHandler.ExperimentalPolygonBuilderEnabled = true;
54+
var poly = ReadPolyBadlyOriented();
4355
Assert.That(poly.Shell, Is.Not.Null);
4456
Assert.That(poly.Holes, Is.Not.Null);
4557
Assert.That(poly.Holes.Length, Is.EqualTo(1));
4658
}
4759

60+
[Test]
61+
public void TestReadPolygonWithWrongShellOrientationDoesntReadHoleWithFlagDisabled()
62+
{
63+
var poly = ReadPolyBadlyOriented();
64+
Assert.That(poly.Shell, Is.Not.Null);
65+
Assert.That(poly.Holes, Is.Not.Null);
66+
Assert.That(poly.Holes.Length, Is.EqualTo(0));
67+
}
68+
4869
private const string WktMultiPoly = @"
4970
MULTIPOLYGON (((-124.134 -79.199, -124.141 -79.316, -124.164 -79.431, -124.202 -79.542, -124.254 -79.647, -124.319 -79.745, -124.396 -79.833, -124.484 -79.91, -124.582 -79.975, -124.687 -80.027, -124.798 -80.065, -124.913 -80.088, -125.03 -80.095, -125.147 -80.088, -125.262 -80.065, -125.373 -80.027, -125.478 -79.975, -125.576 -79.91, -125.664 -79.833, -125.741 -79.745, -125.806 -79.647, -125.858 -79.542, -125.896 -79.431, -125.919 -79.316, -125.926 -79.199, -125.919 -79.082, -125.896 -78.967, -125.858 -78.856, -125.806 -78.751, -125.741 -78.653, -125.664 -78.565, -125.576 -78.488, -125.478 -78.423, -125.373 -78.371, -125.262 -78.333, -125.147 -78.31, -125.03 -78.303, -124.913 -78.31, -124.798 -78.333, -124.687 -78.371, -124.582 -78.423, -124.484 -78.488, -124.396 -78.565, -124.319 -78.653, -124.254 -78.751, -124.202 -78.856, -124.164 -78.967, -124.141 -79.082, -124.134 -79.199),(-124.438 -79.199, -124.443 -79.122, -124.459 -79.046, -124.483 -78.973, -124.518 -78.903, -124.561 -78.839, -124.612 -78.781, -124.67 -78.73, -124.734 -78.687, -124.804 -78.652, -124.877 -78.628, -124.953 -78.612, -125.03 -78.607, -125.107 -78.612, -125.183 -78.628, -125.256 -78.652, -125.326 -78.687, -125.39 -78.73, -125.448 -78.781, -125.499 -78.839, -125.542 -78.903, -125.577 -78.973, -125.601 -79.046, -125.617 -79.122, -125.622 -79.199, -125.617 -79.276, -125.601 -79.352, -125.577 -79.425, -125.542 -79.495, -125.499 -79.559, -125.448 -79.617, -125.39 -79.668, -125.326 -79.711, -125.256 -79.746, -125.183 -79.77, -125.107 -79.786, -125.03 -79.791, -124.953 -79.786, -124.877 -79.77, -124.804 -79.746, -124.734 -79.711, -124.67 -79.668, -124.612 -79.617, -124.561 -79.559, -124.518 -79.495, -124.483 -79.425, -124.459 -79.352, -124.443 -79.276, -124.438 -79.199)),((-124.582 -79.199, -124.586 -79.257, -124.597 -79.315, -124.616 -79.371, -124.642 -79.423, -124.674 -79.472, -124.713 -79.516, -124.757 -79.555, -124.806 -79.587, -124.858 -79.613, -124.914 -79.632, -124.972 -79.643, -125.03 -79.647, -125.088 -79.643, -125.146 -79.632, -125.202 -79.613, -125.254 -79.587, -125.303 -79.555, -125.347 -79.516, -125.386 -79.472, -125.418 -79.423, -125.444 -79.371, -125.463 -79.315, -125.474 -79.257, -125.478 -79.199, -125.474 -79.141, -125.463 -79.083, -125.444 -79.027, -125.418 -78.975, -125.386 -78.926, -125.347 -78.882, -125.303 -78.843, -125.254 -78.811, -125.202 -78.785, -125.146 -78.766, -125.088 -78.755, -125.03 -78.751, -124.972 -78.755, -124.914 -78.766, -124.858 -78.785, -124.806 -78.811, -124.757 -78.843, -124.713 -78.882, -124.674 -78.926, -124.642 -78.975, -124.616 -79.027, -124.597 -79.083, -124.586 -79.141, -124.582 -79.199),(-124.896 -79.199, -124.897 -79.181, -124.9 -79.164, -124.906 -79.148, -124.914 -79.132, -124.923 -79.117, -124.935 -79.104, -124.948 -79.092, -124.963 -79.083, -124.979 -79.075, -124.995 -79.069, -125.012 -79.066, -125.03 -79.065, -125.048 -79.066, -125.065 -79.069, -125.081 -79.075, -125.097 -79.083, -125.112 -79.092, -125.125 -79.104, -125.137 -79.117, -125.146 -79.132, -125.154 -79.148, -125.16 -79.164, -125.163 -79.181, -125.164 -79.199, -125.163 -79.217, -125.16 -79.234, -125.154 -79.25, -125.146 -79.266, -125.137 -79.281, -125.125 -79.294, -125.112 -79.306, -125.097 -79.315, -125.081 -79.323, -125.065 -79.329, -125.048 -79.332, -125.03 -79.333, -125.012 -79.332, -124.995 -79.329, -124.979 -79.323, -124.963 -79.315, -124.948 -79.306, -124.935 -79.294, -124.923 -79.281, -124.914 -79.266, -124.906 -79.25, -124.9 -79.234, -124.897 -79.217, -124.896 -79.199)))
5071
";

0 commit comments

Comments
 (0)