diff --git a/modules/core/src/main/java/org/locationtech/jts/operation/buffer/SubgraphDepthLocater.java b/modules/core/src/main/java/org/locationtech/jts/operation/buffer/SubgraphDepthLocater.java
index 2b76199947..d26aa2eb9d 100644
--- a/modules/core/src/main/java/org/locationtech/jts/operation/buffer/SubgraphDepthLocater.java
+++ b/modules/core/src/main/java/org/locationtech/jts/operation/buffer/SubgraphDepthLocater.java
@@ -162,29 +162,25 @@ static class DepthSegment
public DepthSegment(LineSegment seg, int depth)
{
- // input seg is assumed to be normalized
+ // Assert: input seg is upward (p0.y <= p1.y)
upwardSeg = new LineSegment(seg);
- //upwardSeg.normalize();
this.leftDepth = depth;
}
+
+ public boolean isUpward() {
+ return upwardSeg.p0.y <= upwardSeg.p1.y;
+ }
+
/**
- * Defines a comparison operation on DepthSegments
- * which orders them left to right.
- * Assumes the segments are normalized.
+ * A comparison operation
+ * which orders segments left to right.
*
* The definition of the ordering is:
*
* - -1 : if DS1.seg is left of or below DS2.seg (DS1 < DS2)
- *
- 1 : if DS1.seg is right of or above DS2.seg (DS1 > DS2)
+ *
- 1 : if DS1.seg is right of or above DS2.seg (DS1 > DS2)
*
- 0 : if the segments are identical
*
- *
- * KNOWN BUGS:
- *
- * - The logic does not obey the {@link Comparator.compareTo} contract.
- * This is acceptable for the intended usage, but may cause problems if used with some
- * utilities in the Java standard library (e.g. {@link Collections.sort()}.
- *
*
* @param obj a DepthSegment
* @return the comparison value
@@ -193,9 +189,48 @@ public int compareTo(Object obj)
{
DepthSegment other = (DepthSegment) obj;
+ /**
+ * If segment envelopes do not overlap, then
+ * can use standard segment lexicographic ordering.
+ */
+ if (upwardSeg.minX() >= other.upwardSeg.maxX()
+ || upwardSeg.maxX() <= other.upwardSeg.minX()
+ || upwardSeg.minY() >= other.upwardSeg.maxX()
+ || upwardSeg.maxY() <= other.upwardSeg.minY()) {
+ return upwardSeg.compareTo(other.upwardSeg);
+ };
+
+ /**
+ * Otherwise if envelopes overlap, use relative segment orientation.
+ *
+ * Collinear segments should be evaluated by previous logic
+ */
+ int orientIndex = upwardSeg.orientationIndex(other.upwardSeg);
+ if (orientIndex != 0) return orientIndex;
+
+ /**
+ * If comparison between this and other is indeterminate,
+ * try the opposite call order.
+ * The sign of the result needs to be flipped.
+ */
+ orientIndex = -1 * other.upwardSeg.orientationIndex(upwardSeg);
+ if (orientIndex != 0) return orientIndex;
+
+ /**
+ * If segment envelopes overlap and they are collinear,
+ * since segments do not cross they must be equal.
+ */
+ // assert: segments are equal
+ return 0;
+ }
+
+ public int OLDcompareTo(Object obj)
+ {
+ DepthSegment other = (DepthSegment) obj;
+
// fast check if segments are trivially ordered along X
- if (upwardSeg.minX() >= other.upwardSeg.maxX()) return 1;
- if (upwardSeg.maxX() <= other.upwardSeg.minX()) return -1;
+ if (upwardSeg.minX() > other.upwardSeg.maxX()) return 1;
+ if (upwardSeg.maxX() < other.upwardSeg.minX()) return -1;
/**
* try and compute a determinate orientation for the segments.
@@ -212,30 +247,10 @@ public int compareTo(Object obj)
orientIndex = -1 * other.upwardSeg.orientationIndex(upwardSeg);
if (orientIndex != 0) return orientIndex;
- // otherwise, use standard lexocographic segment ordering
+ // otherwise, use standard lexicographic segment ordering
return upwardSeg.compareTo(other.upwardSeg);
}
- /**
- * Compare two collinear segments for left-most ordering.
- * If segs are vertical, use vertical ordering for comparison.
- * If segs are equal, return 0.
- * Segments are assumed to be directed so that the second coordinate is >= to the first
- * (e.g. up and to the right).
- *
- * @param seg0 a segment to compare
- * @param seg1 a segment to compare
- * @return
- */
- private int compareX(LineSegment seg0, LineSegment seg1)
- {
- int compare0 = seg0.p0.compareTo(seg1.p0);
- if (compare0 != 0)
- return compare0;
- return seg0.p1.compareTo(seg1.p1);
-
- }
-
public String toString()
{
return upwardSeg.toString();
diff --git a/modules/core/src/test/java/org/locationtech/jts/operation/buffer/DepthSegmentTest.java b/modules/core/src/test/java/org/locationtech/jts/operation/buffer/DepthSegmentTest.java
index 489d46285d..08546e284d 100644
--- a/modules/core/src/test/java/org/locationtech/jts/operation/buffer/DepthSegmentTest.java
+++ b/modules/core/src/test/java/org/locationtech/jts/operation/buffer/DepthSegmentTest.java
@@ -30,28 +30,53 @@ public static void main(String[] args) {
junit.textui.TestRunner.run(DepthSegmentTest.class);
}
- public void testContractTipToTail() throws Exception
+ public void testCompareTipToTail() throws Exception
{
SubgraphDepthLocater.DepthSegment ds0 = depthSeg(0.7, 0.2, 1.4, 0.9);
- SubgraphDepthLocater.DepthSegment ds1 = depthSeg(0.3, 1.1, 0.7, 0.2);
- checkContract(ds0, ds1);
+ SubgraphDepthLocater.DepthSegment ds1 = depthSeg(0.7, 0.2, 0.3, 1.1);
+ checkCompare(ds0, ds1, 1);
}
- public void testContract2() throws Exception
+ public void testCompare2() throws Exception
{
- SubgraphDepthLocater.DepthSegment ds0 = depthSeg(0.1, 1.9, 0.5, 1.0);
+ SubgraphDepthLocater.DepthSegment ds0 = depthSeg(0.5, 1.0, 0.1, 1.9);
SubgraphDepthLocater.DepthSegment ds1 = depthSeg(1.0, 0.9, 1.9, 1.4);
- checkContract(ds0, ds1);
+ checkCompare(ds0, ds1, -1);
}
- private void checkContract(
+ public void testCompareVertical() throws Exception
+ {
+ SubgraphDepthLocater.DepthSegment ds0 = depthSeg(1, 1, 1, 2);
+ SubgraphDepthLocater.DepthSegment ds1 = depthSeg(1, 0, 1, 1);
+ checkCompare(ds0, ds1, 1);
+ }
+
+ public void testCompareOrientBug() throws Exception
+ {
+ SubgraphDepthLocater.DepthSegment ds0 = depthSeg(146.268, -8.42361, 146.263, -8.3875);
+ SubgraphDepthLocater.DepthSegment ds1 = depthSeg(146.269, -8.42889, 146.268, -8.42361);
+ checkCompare(ds0, ds1, -1);
+ }
+
+ public void testCompareEqual() throws Exception
+ {
+ SubgraphDepthLocater.DepthSegment ds0 = depthSeg(1, 1, 2, 2);
+ checkCompare(ds0, ds0, 0);
+ }
+
+ private void checkCompare(
SubgraphDepthLocater.DepthSegment ds0,
- SubgraphDepthLocater.DepthSegment ds1) {
- // should never have ds1 < ds2 && ds2 < ds1
- int cmp0 = ds0.compareTo(ds1);
- int cmp1 = ds1.compareTo(ds0);
- boolean isFail = cmp0 != 0 && cmp0 == cmp1;
- assertTrue(! isFail);
+ SubgraphDepthLocater.DepthSegment ds1,
+ int expectedComp)
+ {
+ assertTrue(ds0.isUpward());
+ assertTrue(ds1.isUpward());
+
+ // check compareTo contract - should never have ds1 < ds2 && ds2 < ds1
+ int comp0 = ds0.compareTo(ds1);
+ int comp1 = ds1.compareTo(ds0);
+ assertEquals(expectedComp, comp0);
+ assertTrue( comp0 == -comp1);
}
private SubgraphDepthLocater.DepthSegment depthSeg(double x0, double y0, double x1, double y1) {