From 41507661d79e0c6d12ef2d255b4563133c0d4204 Mon Sep 17 00:00:00 2001 From: Skef Iterum Date: Sat, 19 Sep 2026 02:09:54 -0700 Subject: [PATCH] Fix extraneous on-curve points after remove overlap The check added in #65, which drops previousIntersectionPoint when it is not on the segment being processed, was also dropping points that were genuinely on it. The start of the segment was then re-derived from the next point in the flattened outline, so an inside corner came out as two on-curve points a few units apart (#72). When checking this case we want to know whether a point is on a segment. My previous fix answered that by recovering a t corresponding to the point, calculating that point on the curve, and measuring the distance. However, in the present (pre-existing) function t is recovered by matching a single axis, which can be quite inaccurate for "steep" curves. We could, and perhaps should, improve that function, but it's preferable to change the code to compare against the flattened outline instead, which is what this updated code does. Clipper computes its intersection points from the flattened chords, so each point will lie exactly on a chord before rounding and within less than an integer distance after. Measuring against the original curve involves an extra degree of approximation: the rounding, and then the distance of the chords from the curve. Checking distances from chords also enables a quick rejection based on bounding-box. Also adds a regression test covering the two contours from #72, and removes tValueToPoint() and _getLinePoint(), which #65 added only to support the old implementation of hasPoint() and which are now unused. Co-Authored-By: Claude Opus 5 (1M context) --- Lib/booleanOperations/flatten.py | 65 +++++++++++++++++++++----------- tests/test_BooleanGlyph.py | 57 ++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 23 deletions(-) diff --git a/Lib/booleanOperations/flatten.py b/Lib/booleanOperations/flatten.py index 1a6832f..3bb7a16 100644 --- a/Lib/booleanOperations/flatten.py +++ b/Lib/booleanOperations/flatten.py @@ -255,26 +255,34 @@ def tValueForPoint(self, point): else: raise NotImplementedError - def tValueToPoint(self, t): - if self.segmentType == "curve": - on1 = self.previousOnCurve - off1 = self.points[0].coordinates - off2 = self.points[1].coordinates - on2 = self.points[2].coordinates - return _getCubicPoint(t, on1, off1, off2, on2) - elif self.segmentType == "line": - return _getLinePoint(t, self.previousOnCurve, self.points[0].coordinates) - elif self.segmentType == "qcurve": - raise NotImplementedError - else: - raise NotImplementedError - def hasPoint(self, p): + """ + Whether p lies on this segment. + + p is tested against the segment's flattened chords. Callers pass + points taken from clipper's output, and clipper builds its polygons + out of those same chords, so a point belonging to this segment lies + on one of them to within clipper's integer rounding. + + Testing against the curve would be looser, as the tolerance would + also have to cover the distance between the chords and the curve, + which varies with the curvature of the segment. + """ if p is None: return False - for t in self.tValueForPoint(p): - pp = self.tValueToPoint(t) - if _distance(p, pp) < _approximateSegmentLength/100: + x, y = _scaleSinglePoint(p, scale=clipperScale, convertToInteger=False) + tolerance = _approximateSegmentLength/100 * clipperScale + toleranceSquared = tolerance * tolerance + previousPoint = self.scaledPreviousOnCurve + for point in self.flat: + (x0, y0), (x1, y1) = previousPoint, point + previousPoint = point + # reject the chords p is nowhere near before doing any real work + if x < min(x0, x1) - tolerance or x > max(x0, x1) + tolerance: + continue + if y < min(y0, y1) - tolerance or y > max(y0, y1) + tolerance: + continue + if _squaredDistanceToLineSegment((x, y), (x0, y0), (x1, y1)) < toleranceSquared: return True return False @@ -1137,6 +1145,23 @@ def _distance(pt1, pt2): return math.sqrt((pt1[0] - pt2[0]) ** 2 + (pt1[1] - pt2[1]) ** 2) +def _squaredDistanceToLineSegment(pt, pt0, pt1): + """ + The square of the distance from pt to the closest point of the line + segment pt0..pt1. + """ + (x, y), (x0, y0), (x1, y1) = pt, pt0, pt1 + dx = x1 - x0 + dy = y1 - y0 + lengthSquared = dx * dx + dy * dy + if lengthSquared: + t = ((x - x0) * dx + (y - y0) * dy) / lengthSquared + t = max(0.0, min(1.0, t)) + x0 += dx * t + y0 += dy * t + return (x - x0) ** 2 + (y - y0) ** 2 + + def _pointOnLine(pt1, pt2, a): return abs(_distance(pt1, a) + _distance(a, pt2) - _distance(pt1, pt2)) < epsilon @@ -1167,12 +1192,6 @@ def _mid(pt1, pt2): (x0, y0), (x1, y1) = pt1, pt2 return 0.5 * (x0 + x1), 0.5 * (y0 + y1) -def _getLinePoint(t, pt0, pt1): - if t == 0: - return pt0 - if t == 1: - return pt1 - return pt0[0] + (pt1[0]-pt0[0]) * t, pt0[1] + (pt1[1]-pt0[1]) * t def _getCubicPoint(t, pt0, pt1, pt2, pt3): if t == 0: diff --git a/tests/test_BooleanGlyph.py b/tests/test_BooleanGlyph.py index 6b78128..e87e85d 100644 --- a/tests/test_BooleanGlyph.py +++ b/tests/test_BooleanGlyph.py @@ -115,5 +115,62 @@ def test_unsupported_qcurve(): with pytest.raises(booleanOperations.exceptions.UnsupportedContourError): booleanOperations.union(g, None) + +# Two self-overlapping contours that meet at a sharp inside corner. Each +# should come back with a single on-curve point at that corner. See issue #72. +_INSIDE_CORNER_CONTOURS = [ + [ + ((550, 380), "line"), ((738, 540), None), ((833, 738), None), + ((833, 973), "curve"), ((833, 1148), None), ((781, 1274), None), + ((707, 1274), "curve"), ((676, 1274), None), ((653, 1251), None), + ((643, 1210), "curve"), ((584, 973), None), ((437, 826), None), + ((215, 782), "curve"), ((253, 629), "line"), ((484, 688), None), + ((656, 850), None), ((731, 1099), "curve"), ((689, 1087), "line"), + ((703, 830), None), ((629, 635), None), ((441, 480), "curve"), + ], + [ + ((289, 923), "line"), ((324, 936), None), ((360, 943), None), + ((396, 943), "curve"), ((521, 943), None), ((609, 858), None), + ((609, 737), "curve"), ((609, 686), None), ((594, 636), None), + ((561, 577), "curve"), ((632, 547), "line"), ((695, 664), None), + ((725, 768), None), ((725, 868), "curve"), ((725, 1047), None), + ((630, 1159), None), ((478, 1159), "curve"), ((398, 1159), None), + ((317, 1129), None), ((237, 1070), "curve"), ((261, 1003), "line"), + ((332, 1059), None), ((399, 1086), None), ((468, 1086), "curve"), + ((590, 1086), None), ((659, 1002), None), ((659, 870), "curve"), + ((659, 829), None), ((651, 781), None), ((643, 740), "curve"), + ((653, 712), "line"), ((683, 884), None), ((568, 1012), None), + ((418, 1012), "curve"), ((368, 1012), None), ((318, 1001), None), + ((269, 978), "curve"), + ], +] + + +@pytest.mark.parametrize("points", _INSIDE_CORNER_CONTOURS) +def test_no_extraneous_oncurve_at_inside_corner(points): + font = defcon.Font() + g = font.newGlyph("test") + p = g.getPointPen() + p.beginPath() + for coordinates, segmentType in points: + p.addPoint(coordinates, segmentType=segmentType) + p.endPath() + + result = defcon.Font().newGlyph("result") + booleanOperations.union(g, result.getPointPen()) + + # An inside corner used to come back as two on-curve points a fraction of + # a flattened step apart, because a valid intersection point was discarded + # and re-derived from the flattened outline. + for contour in result: + oncurves = [pt for pt in contour if pt.segmentType is not None] + for previous, point in zip(oncurves, oncurves[1:] + oncurves[:1]): + distance = math.hypot(previous.x - point.x, previous.y - point.y) + assert distance > 10, ( + "on-curve points (%s, %s) and (%s, %s) are only %s units apart" + % (previous.x, previous.y, point.x, point.y, distance) + ) + + if __name__ == '__main__': sys.exit(unittest.main())