Skip to content

Addressing PR comments #29

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Copilot wants to merge 6 commits into from
Closed

Addressing PR comments #29

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
00d431d
Initial plan
Copilot Mar 3, 2026
d86a8bc
feat: port core 1.4.0 clipping and offset updates
Copilot Mar 3, 2026
cce8054
test: add large-coordinate trim collinear regression
Copilot Mar 3, 2026
63c7915
fix: align BuildPath tiny-triangle check with upstream
Copilot Mar 3, 2026
d37acbf
fix: align IntersectEdges parity with C# 1.4
Codex Mar 3, 2026
13ce4ab
fix: align Path64/PathD toString formatting with C# 1.4 diff
Copilot Mar 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions src/main/java/clipper2/Clipper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -808,7 +808,7 @@ public static RectD GetBounds(PathD path) {
result.bottom = pt.y;
}
}
return result.left == Double.MAX_VALUE ? new RectD() : result;
return Math.abs(result.left - Double.MAX_VALUE) < InternalClipper.FLOATING_POINT_TOLERANCE ? new RectD() : result;
}

public static RectD GetBounds(PathsD paths) {
Expand All @@ -829,7 +829,7 @@ public static RectD GetBounds(PathsD paths) {
}
}
}
return result.left == Double.MAX_VALUE ? new RectD() : result;
return Math.abs(result.left - Double.MAX_VALUE) < InternalClipper.FLOATING_POINT_TOLERANCE ? new RectD() : result;
}

public static Path64 MakePath(int[] arr) {
Expand Down Expand Up @@ -1366,10 +1366,10 @@ public static Path64 TrimCollinear(Path64 path, boolean isOpen) {
int len = path.size();
int i = 0;
if (!isOpen) {
while (i < len - 1 && InternalClipper.CrossProduct(path.get(len - 1), path.get(i), path.get(i + 1)) == 0) {
while (i < len - 1 && InternalClipper.IsCollinear(path.get(len - 1), path.get(i), path.get(i + 1))) {
i++;
}
while (i < len - 1 && InternalClipper.CrossProduct(path.get(len - 2), path.get(len - 1), path.get(i)) == 0) {
while (i < len - 1 && InternalClipper.IsCollinear(path.get(len - 2), path.get(len - 1), path.get(i))) {
len--;
}
}
Expand All @@ -1385,7 +1385,7 @@ public static Path64 TrimCollinear(Path64 path, boolean isOpen) {
Point64 last = path.get(i);
result.add(last);
for (i++; i < len - 1; i++) {
if (InternalClipper.CrossProduct(last, path.get(i), path.get(i + 1)) == 0) {
if (InternalClipper.IsCollinear(last, path.get(i), path.get(i + 1))) {
continue;
}
last = path.get(i);
Expand All @@ -1394,10 +1394,10 @@ public static Path64 TrimCollinear(Path64 path, boolean isOpen) {

if (isOpen) {
result.add(path.get(len - 1));
} else if (InternalClipper.CrossProduct(last, path.get(len - 1), result.get(0)) != 0) {
} else if (!InternalClipper.IsCollinear(last, path.get(len - 1), result.get(0))) {
result.add(path.get(len - 1));
} else {
while (result.size() > 2 && InternalClipper.CrossProduct(result.get(result.size() - 1), result.get(result.size() - 2), result.get(0)) == 0) {
while (result.size() > 2 && InternalClipper.IsCollinear(result.get(result.size() - 1), result.get(result.size() - 2), result.get(0))) {
result.remove(result.size() - 1);
}
if (result.size() < 3) {
Expand Down
10 changes: 7 additions & 3 deletions src/main/java/clipper2/core/InternalClipper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ public final class InternalClipper {
private static final long Invalid64 = Long.MAX_VALUE;

public static final double DEFAULT_ARC_TOLERANCE = 0.25;
private static final double FLOATING_POINT_TOLERANCE = 1E-12;
public static final double FLOATING_POINT_TOLERANCE = 1E-12;
// private static final double DEFAULT_MIN_EDGE_LENGTH = 0.1;

private static final String PRECISION_RANGE_ERROR = "Error: Precision is out of range.";
Expand Down Expand Up @@ -52,7 +52,7 @@ public static long CheckCastInt64(double val) {
return (long) Math.rint(val);
}

public static boolean GetIntersectPoint(Point64 ln1a, Point64 ln1b, Point64 ln2a, Point64 ln2b, /* out */ Point64 ip) {
public static boolean GetSegmentIntersectPt(Point64 ln1a, Point64 ln1b, Point64 ln2a, Point64 ln2b, /* out */ Point64 ip) {
double dy1 = (ln1b.y - ln1a.y);
double dx1 = (ln1b.x - ln1a.x);
double dy2 = (ln2b.y - ln2a.y);
Expand Down Expand Up @@ -86,6 +86,10 @@ public static boolean GetIntersectPoint(Point64 ln1a, Point64 ln1b, Point64 ln2a
return true;
}

public static boolean GetIntersectPoint(Point64 ln1a, Point64 ln1b, Point64 ln2a, Point64 ln2b, /* out */ Point64 ip) {
return GetSegmentIntersectPt(ln1a, ln1b, ln2a, ln2b, ip);
}

public static boolean SegsIntersect(Point64 seg1a, Point64 seg1b, Point64 seg2a, Point64 seg2b) {
return SegsIntersect(seg1a, seg1b, seg2a, seg2b, false);
}
Expand Down Expand Up @@ -295,4 +299,4 @@ private static int triSign(long x) {
return x > 0 ? 1 : (x < 0 ? -1 : 0);
}

}
}
18 changes: 9 additions & 9 deletions src/main/java/clipper2/engine/ClipperBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1003,7 +1003,7 @@ private static boolean IsValidAelOrder(Active resident, Active newcomer) {
if (resident.isLeftBound != newcomerIsLeft) {
return newcomerIsLeft;
}
if (InternalClipper.CrossProduct(PrevPrevVertex(resident).pt, resident.bot, resident.top) == 0) {
if (InternalClipper.IsCollinear(PrevPrevVertex(resident).pt, resident.bot, resident.top)) {
return true;
}
// compare turning direction of the alternate bound
Expand Down Expand Up @@ -1727,7 +1727,7 @@ private void DisposeIntersectNodes() {

private void AddNewIntersectNode(Active ae1, Active ae2, long topY) {
Point64 ip = new Point64();
if (!InternalClipper.GetIntersectPoint(ae1.bot, ae1.top, ae2.bot, ae2.top, ip)) {
if (!InternalClipper.GetSegmentIntersectPt(ae1.bot, ae1.top, ae2.bot, ae2.top, ip)) {
ip = new Point64(ae1.curX, topY);
}

Expand Down Expand Up @@ -2232,7 +2232,7 @@ private void CheckJoinLeft(Active e, Point64 pt) {
private void CheckJoinLeft(Active e, Point64 pt, boolean checkCurrX) {
@Nullable
Active prev = e.prevInAEL;
if (prev == null || IsOpen(e) || IsOpen(prev) || !IsHotEdge(e) || !IsHotEdge(prev)) {
if (prev == null || !IsHotEdge(e) || !IsHotEdge(prev) || IsHorizontal(e) || IsHorizontal(prev) || IsOpen(e) || IsOpen(prev)) {
return;
}

Expand All @@ -2249,7 +2249,7 @@ private void CheckJoinLeft(Active e, Point64 pt, boolean checkCurrX) {
} else if (e.curX != prev.curX) {
return;
}
if (InternalClipper.CrossProduct(e.top, pt, prev.top) != 0) {
if (!InternalClipper.IsCollinear(e.top, pt, prev.top)) {
return;
}

Expand All @@ -2272,7 +2272,7 @@ private void CheckJoinRight(Active e, Point64 pt) {
private void CheckJoinRight(Active e, Point64 pt, boolean checkCurrX) {
@Nullable
Active next = e.nextInAEL;
if (next == null || IsOpen(e) || IsOpen(next) || !IsHotEdge(e) || !IsHotEdge(next) || IsJoined(e)) {
if (next == null || !IsHotEdge(e) || !IsHotEdge(next) || IsHorizontal(e) || IsHorizontal(next) || IsOpen(e) || IsOpen(next)) {
return;
}

Expand All @@ -2289,7 +2289,7 @@ private void CheckJoinRight(Active e, Point64 pt, boolean checkCurrX) {
} else if (e.curX != next.curX) {
return;
}
if (InternalClipper.CrossProduct(e.top, pt, next.top) != 0) {
if (!InternalClipper.IsCollinear(e.top, pt, next.top)) {
return;
}

Expand Down Expand Up @@ -2656,7 +2656,7 @@ private void CleanCollinear(OutRec outrec) {
OutPt op2 = startOp;
for (;;) {
// NB if preserveCollinear == true, then only remove 180 deg. spikes
if ((InternalClipper.CrossProduct(op2.prev.pt, op2.pt, op2.next.pt) == 0) && ((op2.pt.opEquals(op2.prev.pt)) || (op2.pt.opEquals(op2.next.pt))
if (InternalClipper.IsCollinear(op2.prev.pt, op2.pt, op2.next.pt) && ((op2.pt.opEquals(op2.prev.pt)) || (op2.pt.opEquals(op2.next.pt))
|| !getPreserveCollinear() || (InternalClipper.DotProduct(op2.prev.pt, op2.pt, op2.next.pt) < 0))) {
if (op2.equals(outrec.pts)) {
outrec.pts = op2.prev;
Expand Down Expand Up @@ -2686,7 +2686,7 @@ private void DoSplitOp(OutRec outrec, OutPt splitOp) {
// OutPt result = prevOp;

Point64 ip = new Point64(); // ip mutated by GetIntersectPoint()
InternalClipper.GetIntersectPoint(prevOp.pt, splitOp.pt, splitOp.next.pt, nextNextOp.pt, ip);
InternalClipper.GetSegmentIntersectPt(prevOp.pt, splitOp.pt, splitOp.next.pt, nextNextOp.pt, ip);

double area1 = Area(prevOp);
double absArea1 = Math.abs(area1);
Expand Down Expand Up @@ -2793,7 +2793,7 @@ public static boolean BuildPath(@Nullable OutPt op, boolean reverse, boolean isO
}
}

if (path.size() == 3 && IsVerySmallTriangle(op2)) {
if (path.size() == 3 && !isOpen && IsVerySmallTriangle(op2)) {
return false;
} else {
return true;
Expand Down
86 changes: 28 additions & 58 deletions src/main/java/clipper2/offset/ClipperOffset.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
package clipper2.offset;

import static clipper2.core.InternalClipper.MAX_COORD;
import static clipper2.core.InternalClipper.MIN_COORD;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
Expand Down Expand Up @@ -69,7 +66,6 @@
public class ClipperOffset {

private static double TOLERANCE = 1.0E-12;
private static final String COORD_RANGE_ERROR = "Error: Coordinate range.";

private final List<Group> groupList = new ArrayList<>();
private Path64 pathOut = new Path64();
Expand Down Expand Up @@ -459,7 +455,7 @@ private void DoSquare(Path64 path, int j, int k) {
}
}

private void DoMiter(Group group, Path64 path, int j, int k, double cosA) {
private void DoMiter(Path64 path, int j, int k, double cosA) {
final double q = groupDelta / (cosA + 1);
pathOut.add(new Point64(path.get(j).x + (normals.get(k).x + normals.get(j).x) * q, path.get(j).y + (normals.get(k).y + normals.get(j).y) * q));
}
Expand Down Expand Up @@ -497,6 +493,10 @@ private void DoRound(Path64 path, int j, int k, double angle) {
private void BuildNormals(Path64 path) {
int cnt = path.size();
normals.clear();
if (cnt == 0) {
return;
}
normals.ensureCapacity(cnt);

for (int i = 0; i < cnt - 1; i++) {
normals.add(GetUnitNormal(path.get(i), path.get(i + 1)));
Expand All @@ -505,6 +505,9 @@ private void BuildNormals(Path64 path) {
}

private int OffsetPoint(Group group, Path64 path, int j, int k) {
if (path.get(j).equals(path.get(k))) {
return j;
}
// Let A = change in angle where edges join
// A == 0: ie no change in angle (flat join)
// A == PI: edges 'spike'
Expand All @@ -529,20 +532,22 @@ private int OffsetPoint(Group group, Path64 path, int j, int k) {
return j;
}

if (cosA > -0.99 && (sinA * groupDelta < 0)) { // test for concavity first (#593)
if (cosA > -0.999 && (sinA * groupDelta < 0)) { // test for concavity first (#593)
// is concave
pathOut.add(GetPerpendic(path.get(j), normals.get(k)));
// this extra point is the only (simple) way to ensure that
// path reversals are fully cleaned with the trailing clipper
pathOut.add(path.get(j)); // (#405)
// this extra point is the only simple way to ensure that path reversals
// are fully cleaned out with the trailing union op.
if (cosA < 0.99) {
pathOut.add(path.get(j)); // (#405)
}
pathOut.add(GetPerpendic(path.get(j), normals.get(j)));
} else if (cosA > 0.999 && joinType != JoinType.Round) {
// almost straight - less than 2.5 degree (#424, #482, #526 & #724)
DoMiter(group, path, j, k, cosA);
DoMiter(path, j, k, cosA);
} else if (joinType == JoinType.Miter) {
// miter unless the angle is sufficiently acute to exceed ML
if (cosA > mitLimSqr - 1) {
DoMiter(group, path, j, k, cosA);
DoMiter(path, j, k, cosA);
} else {
DoSquare(path, j, k);
}
Expand Down Expand Up @@ -584,16 +589,6 @@ private void OffsetOpenPath(Group group, Path64 path) {
return;
}

// Overwrite the polygon-based normals with normals for an open path
normals.clear();
for (int i = 0; i < highI; ++i) {
normals.add(GetUnitNormal(path.get(i), path.get(i + 1)));
}
// The C# version uses a clever trick by calculating n normals and then
// overwriting them. A clearer approach in Java is to build the correct
// n-1 normals first, and then add a temporary one for the logic to work.
normals.add(new PointD(normals.get(highI - 1)));

if (deltaCallback != null) {
groupDelta = deltaCallback.calculate(path, normals, 0, 0);
}
Expand Down Expand Up @@ -659,10 +654,6 @@ private void OffsetOpenPath(Group group, Path64 path) {
solution.add(pathOut);
}

private static boolean ToggleBoolIf(boolean val, boolean condition) {
return condition ? !val : val;
}

private void DoGroupOffset(Group group) {
if (group.endType == EndType.Polygon) {
// a straight path (2 points) can now also be 'polygon' offset
Expand All @@ -676,19 +667,15 @@ private void DoGroupOffset(Group group) {
}

double absDelta = Math.abs(groupDelta);
if (!ValidateBounds(group.boundsList, absDelta)) {
throw new RuntimeException(COORD_RANGE_ERROR);
}

joinType = group.joinType;
endType = group.endType;

if (group.joinType == JoinType.Round || group.endType == EndType.Round) {
// calculate a sensible number of steps (for 360 deg for the given offset
// arcTol - when fArcTolerance is undefined (0), the amount of
// curve imprecision that's allowed is based on the size of the
// offset (delta). Obviously very large offsets will almost always
// require much less precision.
// calculate the number of steps required to approximate a circle
// (see http://www.angusj.com/clipper2/Docs/Trigonometry.htm)
// arcTol - when arcTolerance is undefined (0), curve imprecision
// will be relative to the size of the offset (delta).
double arcTol = arcTolerance > 0.01 ? arcTolerance : Math.log10(2 + absDelta) * InternalClipper.DEFAULT_ARC_TOLERANCE;
double stepsPer360 = Math.PI / Math.acos(1 - arcTol / absDelta);
stepSin = Math.sin((2 * Math.PI) / stepsPer360);
Expand All @@ -699,22 +686,22 @@ private void DoGroupOffset(Group group) {
stepsPerRad = stepsPer360 / (2 * Math.PI);
}

int i = 0;
for (Path64 p : group.inPaths) {
// NOTE use int i rather than 3 iterators
Rect64 pathBounds = group.boundsList.get(i);
boolean isHole = group.isHoleList.get(i++);
if (!pathBounds.IsValid()) {
continue;
}
pathOut = new Path64();
int cnt = p.size();
if ((cnt == 0) || ((cnt < 3) && (endType == EndType.Polygon))) {
continue;
}

pathOut = new Path64();
if (cnt == 1) {
Point64 pt = p.get(0);
if (deltaCallback != null) {
groupDelta = deltaCallback.calculate(p, normals, 0, 0);
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reset normals before invoking single-point delta callbacks

In DoGroupOffset, the new cnt == 1 callback path calls deltaCallback.calculate(p, normals, 0, 0) without rebuilding or clearing normals, so if a multi-vertex path was processed just before, the callback receives stale normals from the previous path. This causes incorrect variable offsets for the single-point path (and can break callbacks that assume path_norms matches the current path). Please clear normals or pass an empty normals list before this callback.

Useful? React with 👍 / 👎.

if (group.pathsReversed) {
groupDelta = -groupDelta;
}
absDelta = Math.abs(groupDelta);
}

// single vertex so build a circle or square ...
if (group.endType == EndType.Round) {
Expand All @@ -730,11 +717,6 @@ private void DoGroupOffset(Group group) {
continue;
} // end of offsetting a single point

// when shrinking outer paths, make sure they can shrink this far (#593)
// also when shrinking holes, make sure they too can shrink this far (#715)
if (((groupDelta > 0) == ToggleBoolIf(isHole, group.pathsReversed)) && (Math.min(pathBounds.getWidth(), pathBounds.getHeight()) <= -groupDelta * 2))
continue;

if (cnt == 2 && group.endType == EndType.Joined) {
endType = (group.joinType == JoinType.Round) ? EndType.Round : EndType.Square;
}
Expand All @@ -750,16 +732,4 @@ private void DoGroupOffset(Group group) {
}
}

private static boolean ValidateBounds(List<Rect64> boundsList, double delta) {
int intDelta = (int) delta;
for (Rect64 r : boundsList) {
if (!r.IsValid()) {
continue; // ignore invalid paths
} else if (r.left < MIN_COORD + intDelta || r.right > MAX_COORD + intDelta || r.top < MIN_COORD + intDelta || r.bottom > MAX_COORD + intDelta) {
return false;
}
}
return true;
}

}
Loading