From f0ed672917d10e33ee4ac4cf7d10566417167c68 Mon Sep 17 00:00:00 2001 From: Kenneth VanderLinde Date: Fri, 8 Jul 2022 15:16:12 -0700 Subject: [PATCH 1/4] Improve precision of points when building `AreaTree` By reading `double`s instead of `float` from the `PathIterator`, `AreaMeta` is better able to calculate angles to determine whether a subpath is a hole or not. This is especially crucial at angles near +/-180 degrees. --- .../java/net/rptools/maptool/client/ui/zone/vbl/AreaMeta.java | 2 +- .../java/net/rptools/maptool/client/ui/zone/vbl/AreaTree.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/net/rptools/maptool/client/ui/zone/vbl/AreaMeta.java b/src/main/java/net/rptools/maptool/client/ui/zone/vbl/AreaMeta.java index 14502c89f0..5183daca56 100644 --- a/src/main/java/net/rptools/maptool/client/ui/zone/vbl/AreaMeta.java +++ b/src/main/java/net/rptools/maptool/client/ui/zone/vbl/AreaMeta.java @@ -103,7 +103,7 @@ public boolean isHole() { return isHole; } - public void addPoint(float x, float y) { + public void addPoint(double x, double y) { // Cut out redundant points // TODO: This works ... in concept, but in practice it can create holes that pop outside of // their parent bounds diff --git a/src/main/java/net/rptools/maptool/client/ui/zone/vbl/AreaTree.java b/src/main/java/net/rptools/maptool/client/ui/zone/vbl/AreaTree.java index 08975078fa..9c472f36ee 100644 --- a/src/main/java/net/rptools/maptool/client/ui/zone/vbl/AreaTree.java +++ b/src/main/java/net/rptools/maptool/client/ui/zone/vbl/AreaTree.java @@ -65,7 +65,7 @@ private void digest(Area area) { List islandList = new ArrayList(); // Break the big area into independent areas - float[] coords = new float[6]; + double[] coords = new double[6]; AreaMeta areaMeta = new AreaMeta(); for (PathIterator iter = area.getPathIterator(null); !iter.isDone(); iter.next()) { int type = iter.currentSegment(coords); From 94f7777a51a893e335398c3cf226e6b91bb03d96 Mon Sep 17 00:00:00 2001 From: Kenneth VanderLinde Date: Fri, 8 Jul 2022 15:43:54 -0700 Subject: [PATCH 2/4] Use JTS to determine subpath direction in AreaMeta The subpath direction determines whether a subpath is a hole or not. The previous angle-summing algorithm is sensitive to small errors when angles are near +/-180 degrees. By contrast, the JTS Orientation algorithm is more robust and uses double-double precision to help out as well. --- .../maptool/client/ui/zone/vbl/AreaMeta.java | 81 +++++-------------- 1 file changed, 19 insertions(+), 62 deletions(-) diff --git a/src/main/java/net/rptools/maptool/client/ui/zone/vbl/AreaMeta.java b/src/main/java/net/rptools/maptool/client/ui/zone/vbl/AreaMeta.java index 5183daca56..6d9c5fea28 100644 --- a/src/main/java/net/rptools/maptool/client/ui/zone/vbl/AreaMeta.java +++ b/src/main/java/net/rptools/maptool/client/ui/zone/vbl/AreaMeta.java @@ -16,11 +16,8 @@ import java.awt.geom.Area; import java.awt.geom.GeneralPath; -import java.awt.geom.Point2D; import java.util.ArrayList; import java.util.List; -import net.rptools.lib.GeometryUtil; -import net.rptools.lib.GeometryUtil.PointNode; import org.locationtech.jts.algorithm.Orientation; import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.GeometryFactory; @@ -35,9 +32,7 @@ public class AreaMeta { // Only used during construction boolean isHole; - PointNode pointNodeList; GeneralPath path; - PointNode lastPointNode; public AreaMeta() {} @@ -95,10 +90,6 @@ public List getFacingSegments( return segments; } - private static Coordinate toCoordinate(Point2D point2D) { - return new Coordinate(point2D.getX(), point2D.getY()); - } - public boolean isHole() { return isHole; } @@ -115,75 +106,41 @@ public void addPoint(double x, double y) { // skippedPoints++; // return; // } - PointNode pointNode = new PointNode(new Point2D.Double(x, y)); - - // Don't add if we haven't moved - if (lastPointNode != null && lastPointNode.point.equals(pointNode.point)) { - return; + final var vertex = new Coordinate(x, y); + if (!vertices.isEmpty()) { + final var lastVertex = vertices.get(vertices.size() - 1); + // Don't add if we haven't moved + if (lastVertex.equals(vertex)) { + return; + } } + vertices.add(vertex); + if (path == null) { path = new GeneralPath(); path.moveTo(x, y); - - pointNodeList = pointNode; } else { path.lineTo(x, y); - - lastPointNode.next = pointNode; - pointNode.previous = lastPointNode; } - lastPointNode = pointNode; } public void close() { area = new Area(path); - // Close the circle - lastPointNode.next = pointNodeList; - pointNodeList.previous = lastPointNode; - lastPointNode = null; - - // For some odd reason, sometimes the first and last point are the same, which causes - // bugs in the way areas are calculated - if (pointNodeList.point.equals(pointNodeList.previous.point)) { - // Pull out the dupe node - PointNode trueLastPoint = pointNodeList.previous.previous; - trueLastPoint.next = pointNodeList; - pointNodeList.previous = trueLastPoint; + // Close the circle. + // For some odd reason, sometimes the first and last point are already the same, so don't add + // the point again in that case. + final var first = vertices.get(0); + final var last = vertices.get(vertices.size() - 1); + if (!first.equals(last)) { + vertices.add(first); } - computeIsHole(); - computeVertices(); - // Don't need point list anymore - pointNodeList = null; + isHole = Orientation.isCCW(vertices.toArray(Coordinate[]::new)); + + // Don't need this anymore path = null; // System.out.println("AreaMeta.skippedPoints: " + skippedPoints + " h:" + isHole + " f:" + // faceList.size()); } - - private void computeIsHole() { - double angle = 0; - - PointNode currNode = pointNodeList.next; - - while (currNode != pointNodeList) { - double currAngle = - GeometryUtil.getAngleDelta( - GeometryUtil.getAngle(currNode.previous.point, currNode.point), - GeometryUtil.getAngle(currNode.point, currNode.next.point)); - - angle += currAngle; - currNode = currNode.next; - } - isHole = angle < 0; - } - - private void computeVertices() { - PointNode node = pointNodeList; - vertices.add(toCoordinate(node.point)); - do { - vertices.add(toCoordinate(node.next.point)); - node = node.next; - } while (!node.point.equals(pointNodeList.point)); - } } From 2c483bd438a6433c028e8900cff78ca9e4350466 Mon Sep 17 00:00:00 2001 From: Kenneth VanderLinde Date: Fri, 8 Jul 2022 17:02:08 -0700 Subject: [PATCH 3/4] Encapsulate AreaMeta There is no reason for `AreaMeta`'s fields to be accessed directly, so we privated them away and provided the necessary methods to interact with them. --- .../maptool/client/ui/zone/vbl/AreaIsland.java | 4 ++-- .../maptool/client/ui/zone/vbl/AreaMeta.java | 17 +++++++++++++---- .../maptool/client/ui/zone/vbl/AreaOcean.java | 4 ++-- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/main/java/net/rptools/maptool/client/ui/zone/vbl/AreaIsland.java b/src/main/java/net/rptools/maptool/client/ui/zone/vbl/AreaIsland.java index 18aa8c793c..16c4dd5f2c 100644 --- a/src/main/java/net/rptools/maptool/client/ui/zone/vbl/AreaIsland.java +++ b/src/main/java/net/rptools/maptool/client/ui/zone/vbl/AreaIsland.java @@ -57,7 +57,7 @@ public void setParentOcean(AreaOcean parentOcean) { @Override public @Nullable AreaContainer getDeepestContainerAt(Point2D point) { - if (!meta.area.contains(point)) { + if (!meta.contains(point)) { // Point not contained within this island, so nothing to return. return null; } @@ -83,7 +83,7 @@ public void addOcean(AreaOcean ocean) { //// // AREA CONTAINER public Area getBounds() { - return meta.area; + return meta.getBounds(); } @Override diff --git a/src/main/java/net/rptools/maptool/client/ui/zone/vbl/AreaMeta.java b/src/main/java/net/rptools/maptool/client/ui/zone/vbl/AreaMeta.java index 6d9c5fea28..6224cf7964 100644 --- a/src/main/java/net/rptools/maptool/client/ui/zone/vbl/AreaMeta.java +++ b/src/main/java/net/rptools/maptool/client/ui/zone/vbl/AreaMeta.java @@ -16,6 +16,7 @@ import java.awt.geom.Area; import java.awt.geom.GeneralPath; +import java.awt.geom.Point2D; import java.util.ArrayList; import java.util.List; import org.locationtech.jts.algorithm.Orientation; @@ -27,15 +28,23 @@ /** Represents the boundary of a piece of topology. */ public class AreaMeta { - Area area; - List vertices = new ArrayList<>(); + private Area area; + private List vertices = new ArrayList<>(); // Only used during construction - boolean isHole; - GeneralPath path; + private boolean isHole; + private GeneralPath path; public AreaMeta() {} + public boolean contains(Point2D point) { + return area.contains(point); + } + + public Area getBounds() { + return new Area(area); + } + /** * @param origin * @param faceAway If `true`, only return segments facing away from origin. diff --git a/src/main/java/net/rptools/maptool/client/ui/zone/vbl/AreaOcean.java b/src/main/java/net/rptools/maptool/client/ui/zone/vbl/AreaOcean.java index 0d222034b2..04868d97d5 100644 --- a/src/main/java/net/rptools/maptool/client/ui/zone/vbl/AreaOcean.java +++ b/src/main/java/net/rptools/maptool/client/ui/zone/vbl/AreaOcean.java @@ -71,7 +71,7 @@ public List getVisionBlockingBoundarySegments( @Override public @Nullable AreaContainer getDeepestContainerAt(Point2D point) { - if (meta != null && !meta.area.contains(point)) { + if (meta != null && !meta.contains(point)) { // Point not contained within this ocean, so nothing to return. return null; } @@ -97,6 +97,6 @@ public void addIsland(AreaIsland island) { @Override public Area getBounds() { - return meta != null ? meta.area : null; + return meta != null ? meta.getBounds() : null; } } From c07e89637431cc07aaa0abffe45f103b637baf0d Mon Sep 17 00:00:00 2001 From: Kenneth VanderLinde Date: Fri, 8 Jul 2022 17:12:36 -0700 Subject: [PATCH 4/4] Remove unused GeometryUtil methods Most of the code in GeometryUtil is no longer in use, so it has been removed. --- .../java/net/rptools/lib/GeometryUtil.java | 157 ------------------ 1 file changed, 157 deletions(-) diff --git a/src/main/java/net/rptools/lib/GeometryUtil.java b/src/main/java/net/rptools/lib/GeometryUtil.java index c991b16813..6934019eb0 100644 --- a/src/main/java/net/rptools/lib/GeometryUtil.java +++ b/src/main/java/net/rptools/lib/GeometryUtil.java @@ -14,52 +14,9 @@ */ package net.rptools.lib; -import java.awt.geom.Area; -import java.awt.geom.GeneralPath; -import java.awt.geom.Line2D; -import java.awt.geom.PathIterator; import java.awt.geom.Point2D; -import java.util.HashSet; -import java.util.List; -import java.util.Set; public class GeometryUtil { - public static Line2D findClosestLine(Point2D origin, PointNode pointList) { - Line2D line = null; - double distance = 0; - - PointNode node = pointList; - do { - Line2D newLine = new Line2D.Double(node.previous.point, node.point); - double newDistance = getDistanceToCenter(origin, newLine); - if (line == null || newDistance < distance) { - line = newLine; - distance = newDistance; - } - node = node.next; - } while (node != pointList); - - return line; - } - - public static double getDistanceToCenter(Point2D p, Line2D line) { - Point2D midPoint = - new Point2D.Double( - (line.getP1().getX() + line.getP2().getX()) / 2, - (line.getP1().getY() + line.getP2().getY()) / 2); - - return Math.hypot(midPoint.getX() - p.getX(), midPoint.getY() - p.getY()); - } - - public static Point2D getCloserPoint(Point2D origin, Line2D line) { - double dist1 = - Math.hypot(origin.getX() - line.getP1().getX(), origin.getY() - line.getP1().getY()); - double dist2 = - Math.hypot(origin.getX() - line.getP2().getX(), origin.getY() - line.getP2().getY()); - - return dist1 < dist2 ? line.getP1() : line.getP2(); - } - public static double getAngle(Point2D origin, Point2D target) { double angle = Math.toDegrees( @@ -82,118 +39,4 @@ public static double getAngleDelta(double sourceAngle, double targetAngle) { } return targetAngle; } - - public static class PointNode { - public PointNode previous; - public PointNode next; - public Point2D point; - - public PointNode(Point2D point) { - this.point = point; - } - } - - public static double getDistanceXXX(Point2D p1, Point2D p2) { - double a = p2.getX() - p1.getX(); - double b = p2.getY() - p1.getY(); - return Math.sqrt( - a * a + b * b); // Was just "a+b" -- was that on purpose? A shortcut speed-up perhaps? - } - - public static Set getFrontFaces(PointNode nodeList, Point2D origin) { - Set frontFaces = new HashSet(); - - Line2D closestLine = GeometryUtil.findClosestLine(origin, nodeList); - Point2D closestPoint = GeometryUtil.getCloserPoint(origin, closestLine); - PointNode closestNode = nodeList; - do { - if (closestNode.point.equals(closestPoint)) { - break; - } - closestNode = closestNode.next; - } while (closestNode != nodeList); - - Point2D secondPoint = - closestLine.getP1().equals(closestPoint) ? closestLine.getP2() : closestLine.getP1(); - Point2D thirdPoint = - secondPoint.equals(closestNode.next.point) - ? closestNode.previous.point - : closestNode.next.point; - - // Determine whether the first line segment is visible - Line2D l1 = new Line2D.Double(origin, secondPoint); - Line2D l2 = new Line2D.Double(closestNode.point, thirdPoint); - boolean frontFace = !(l1.intersectsLine(l2)); - if (frontFace) { - frontFaces.add(new Line2D.Double(closestPoint, secondPoint)); - } - Point2D startPoint = - closestNode.previous.point.equals(secondPoint) ? secondPoint : closestNode.point; - Point2D endPoint = closestNode.point.equals(startPoint) ? secondPoint : closestNode.point; - double originAngle = GeometryUtil.getAngle(origin, startPoint); - double pointAngle = GeometryUtil.getAngle(startPoint, endPoint); - int lastDirection = GeometryUtil.getAngleDelta(originAngle, pointAngle) > 0 ? 1 : -1; - - // System.out.format("%s: %.2f %s, %.2f %s => %.2f : %d : %s\n", frontFace, originAngle, - // startPoint.toString(), pointAngle, endPoint.toString(), getAngleDelta(originAngle, - // pointAngle), - // lastDirection, (closestNode.previous.point.equals(secondPoint) ? "second" : - // "closest").toString()); - PointNode node = secondPoint.equals(closestNode.next.point) ? closestNode.next : closestNode; - do { - Point2D point = node.point; - Point2D nextPoint = node.next.point; - - originAngle = GeometryUtil.getAngle(origin, point); - pointAngle = GeometryUtil.getAngle(origin, nextPoint); - - // System.out.println(point + ":" + originAngle + ", " + nextPoint + ":"+ pointAngle + ", " + - // getAngleDelta(originAngle, pointAngle)); - if (GeometryUtil.getAngleDelta(originAngle, pointAngle) > 0) { - if (lastDirection < 0) { - frontFace = !frontFace; - lastDirection = 1; - } - } else { - if (lastDirection > 0) { - frontFace = !frontFace; - lastDirection = -1; - } - } - if (frontFace) { - frontFaces.add(new Line2D.Double(nextPoint, point)); - } - node = node.next; - } while (!node.point.equals(closestPoint)); - - return frontFaces; - } - - public static int countAreaPoints(Area area) { - int count = 0; - for (PathIterator iter = area.getPathIterator(null); !iter.isDone(); iter.next()) { - count++; - } - return count; - } - - public static Area createLine(List points, int width) { - Point2D lastPoint = null; - Line2D lastLine = null; - for (Point2D point : points) { - if (lastPoint == null) { - lastPoint = point; - continue; - } - if (lastLine == null) { - // First line segment - lastLine = new Line2D.Double(lastPoint, point); - - // Keep track - continue; - } - } - GeneralPath path = new GeneralPath(); - return new Area(path); - } }