From 793eb206e5dc04809595b0eb2e4422a8088be58c Mon Sep 17 00:00:00 2001 From: Moritz Kirmse Date: Tue, 16 Sep 2025 10:01:06 +0200 Subject: [PATCH 1/4] Create test which fails for issue 1217 --- tests/unit/operation/buffer/BufferOpTest.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/unit/operation/buffer/BufferOpTest.cpp b/tests/unit/operation/buffer/BufferOpTest.cpp index b51914ca8..1c5d3cead 100644 --- a/tests/unit/operation/buffer/BufferOpTest.cpp +++ b/tests/unit/operation/buffer/BufferOpTest.cpp @@ -623,4 +623,18 @@ void object::test<28> "MULTIPOLYGON (((24 95.239, 24 96, 24 99, 24.816 99, 24 95.239)), ((3 90, 3 93, 3 96, 3 99, 21 99, 21 96, 21 93, 21 90, 3 90)))"); } +// testEndCap +// test end cap for a line where left and right side are simplified differently, so there is an angle at the end +// See https://github.com/libgeos/geos/issues/1217 +template<> +template<> +void object::test<29> +() +{ + std::string wkt("LINESTRING (0.7 4.7, 146.3 137.1, 146.3 137, 146.6 136.7)"); + std::unique_ptr result13 = buffer(wkt, 11); + checkValidPolygon(*result13); + checkNumHoles(*result13, 0); +} + } // namespace tut From 290b2ae534739215912f30b8b0dcd7de933e14f2 Mon Sep 17 00:00:00 2001 From: Moritz Kirmse Date: Tue, 16 Sep 2025 10:28:40 +0200 Subject: [PATCH 2/4] Propose fix --- .../operation/buffer/OffsetSegmentGenerator.h | 3 ++- src/operation/buffer/OffsetCurveBuilder.cpp | 8 ++++--- .../buffer/OffsetSegmentGenerator.cpp | 21 ++++++++++--------- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/include/geos/operation/buffer/OffsetSegmentGenerator.h b/include/geos/operation/buffer/OffsetSegmentGenerator.h index 2f3a369c6..75affa0f3 100644 --- a/include/geos/operation/buffer/OffsetSegmentGenerator.h +++ b/include/geos/operation/buffer/OffsetSegmentGenerator.h @@ -144,7 +144,8 @@ class GEOS_DLL OffsetSegmentGenerator { /// Add an end cap around point p1, terminating a line segment /// coming from p0 void addLineEndCap(const geom::Coordinate& p0, - const geom::Coordinate& p1); + const geom::Coordinate& p1, + const geom::Coordinate& p2); void addSegments(const geom::CoordinateSequence& pts, bool isForward) diff --git a/src/operation/buffer/OffsetCurveBuilder.cpp b/src/operation/buffer/OffsetCurveBuilder.cpp index 1628ef9bb..7dfac45ad 100644 --- a/src/operation/buffer/OffsetCurveBuilder.cpp +++ b/src/operation/buffer/OffsetCurveBuilder.cpp @@ -377,8 +377,6 @@ OffsetCurveBuilder::computeLineBufferCurve(const CoordinateSequence& inputPts, segGen.addNextSegment(simp1[i], true); } segGen.addLastSegment(); - // add line cap for end of line - segGen.addLineEndCap(simp1[n1 - 1], simp1[n1]); //---------- compute points for right side of line // Simplify the appropriate side of the line before generating @@ -387,13 +385,17 @@ OffsetCurveBuilder::computeLineBufferCurve(const CoordinateSequence& inputPts, const CoordinateSequence& simp2 = *simp2_; auto n2 = simp2.size() - 1; + + // add line cap for return of line + segGen.addLineEndCap(simp1[n1 - 1], simp[n1], simp2[n2 - 1]); + segGen.initSideSegments(simp2[n2], simp2[n2 - 1], Position::LEFT); for(std::size_t i = n2 - 1; i > 0; --i) { segGen.addNextSegment(simp2[i - 1], true); } segGen.addLastSegment(); // add line cap for start of line - segGen.addLineEndCap(simp2[1], simp2[0]); + segGen.addLineEndCap(simp2[1], simp2[0], simp1[1]); segGen.closeRing(); } diff --git a/src/operation/buffer/OffsetSegmentGenerator.cpp b/src/operation/buffer/OffsetSegmentGenerator.cpp index 6c7c2c3b0..8240773bd 100644 --- a/src/operation/buffer/OffsetSegmentGenerator.cpp +++ b/src/operation/buffer/OffsetSegmentGenerator.cpp @@ -187,25 +187,21 @@ OffsetSegmentGenerator::computeOffsetSegment(const LineSegment& seg, int p_side, /*public*/ void -OffsetSegmentGenerator::addLineEndCap(const Coordinate& p0, const Coordinate& p1) +OffsetSegmentGenerator::addLineEndCap(const Coordinate& p0, const Coordinate& p1, const Coordinate& p2) { - LineSegment seg(p0, p1); + LineSegment seg1(p0, p1); + LineSegment seg2(p2, p1); LineSegment offsetL; - computeOffsetSegment(seg, Position::LEFT, distance, offsetL); + computeOffsetSegment(seg1, Position::LEFT, distance, offsetL); LineSegment offsetR; - computeOffsetSegment(seg, Position::RIGHT, distance, offsetR); - - double dx = p1.x - p0.x; - double dy = p1.y - p0.y; - double angle = atan2(dy, dx); + computeOffsetSegment(seg2, Position::RIGHT, distance, offsetR); switch(bufParams.getEndCapStyle()) { case BufferParameters::CAP_ROUND: // add offset seg points with a fillet between them segList.addPt(offsetL.p1); - addDirectedFillet(p1, angle + Angle::PI_OVER_2, angle - Angle::PI_OVER_2, - Orientation::CLOCKWISE, distance); + addDirectedFillet(p1, offsetL.p1, offsetR.p1, Orientation::CLOCKWISE, distance); segList.addPt(offsetR.p1); break; case BufferParameters::CAP_FLAT: @@ -216,7 +212,12 @@ OffsetSegmentGenerator::addLineEndCap(const Coordinate& p0, const Coordinate& p1 case BufferParameters::CAP_SQUARE: // add a square defined by extensions of the offset // segment endpoints + Coordinate squareCapSideOffset; + // take average in case angles of left and right sides differ + double dx = p1.x - p0.x/2 - p2.x/2; + double dy = p1.y - p0.y/2 - p2.y/2; + double angle = atan2(dy, dx); double sinangle, cosangle; Angle::sinCosSnap(angle, sinangle, cosangle); squareCapSideOffset.x = fabs(distance) * cosangle; From 7b659546da6bf78174e52a858cfdbfc27bf61cd6 Mon Sep 17 00:00:00 2001 From: Moritz Kirmse Date: Tue, 16 Sep 2025 10:33:52 +0200 Subject: [PATCH 3/4] Rename segment to prevent shadowing member variable --- src/operation/buffer/OffsetSegmentGenerator.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/operation/buffer/OffsetSegmentGenerator.cpp b/src/operation/buffer/OffsetSegmentGenerator.cpp index 8240773bd..435d4770a 100644 --- a/src/operation/buffer/OffsetSegmentGenerator.cpp +++ b/src/operation/buffer/OffsetSegmentGenerator.cpp @@ -189,13 +189,13 @@ OffsetSegmentGenerator::computeOffsetSegment(const LineSegment& seg, int p_side, void OffsetSegmentGenerator::addLineEndCap(const Coordinate& p0, const Coordinate& p1, const Coordinate& p2) { - LineSegment seg1(p0, p1); - LineSegment seg2(p2, p1); + LineSegment segL(p0, p1); + LineSegment segR(p2, p1); LineSegment offsetL; - computeOffsetSegment(seg1, Position::LEFT, distance, offsetL); + computeOffsetSegment(segL, Position::LEFT, distance, offsetL); LineSegment offsetR; - computeOffsetSegment(seg2, Position::RIGHT, distance, offsetR); + computeOffsetSegment(segR, Position::RIGHT, distance, offsetR); switch(bufParams.getEndCapStyle()) { case BufferParameters::CAP_ROUND: From a86fe8463f7b3a25ffde0781634dbd19d936f837 Mon Sep 17 00:00:00 2001 From: Moritz Kirmse Date: Tue, 16 Sep 2025 10:38:10 +0200 Subject: [PATCH 4/4] Fix copy paste error --- src/operation/buffer/OffsetCurveBuilder.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/operation/buffer/OffsetCurveBuilder.cpp b/src/operation/buffer/OffsetCurveBuilder.cpp index 7dfac45ad..1e0f0b175 100644 --- a/src/operation/buffer/OffsetCurveBuilder.cpp +++ b/src/operation/buffer/OffsetCurveBuilder.cpp @@ -387,7 +387,7 @@ OffsetCurveBuilder::computeLineBufferCurve(const CoordinateSequence& inputPts, auto n2 = simp2.size() - 1; // add line cap for return of line - segGen.addLineEndCap(simp1[n1 - 1], simp[n1], simp2[n2 - 1]); + segGen.addLineEndCap(simp1[n1 - 1], simp1[n1], simp2[n2 - 1]); segGen.initSideSegments(simp2[n2], simp2[n2 - 1], Position::LEFT); for(std::size_t i = n2 - 1; i > 0; --i) {