Skip to content
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

Fix OffsetCurve to use a minimum QuadrantSegs value #918

Closed
wants to merge 4 commits into from
Closed

Fix OffsetCurve to use a minimum QuadrantSegs value #918

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
29b8ce5
Port https://github.com/locationtech/jts/pull/981
pramsey May 26, 2023
b8ae251
Harmonize test cases, three changes were just more detailed curves th…
pramsey May 26, 2023
fafdea6
Fix link to geojson.h
pramsey May 27, 2023
3b4f322
remove some precision from test
pramsey May 29, 2023
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
18 changes: 17 additions & 1 deletion include/geos/operation/buffer/OffsetCurve.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,12 @@ class GEOS_DLL OffsetCurve {
// Constants
static constexpr int MATCH_DISTANCE_FACTOR = 10000;

/**
* A QuadSegs minimum value that will prevent generating
* unwanted offset curve artifacts near end caps.
*/
static constexpr int MIN_QUADRANT_SEGMENTS = 8;

/**
* Creates a new instance for computing an offset curve for a geometry at a given distance.
* with default quadrant segments (BufferParameters::DEFAULT_QUADRANT_SEGMENTS)
Expand Down Expand Up @@ -230,7 +236,17 @@ class GEOS_DLL OffsetCurve {
throw util::IllegalArgumentException("OffsetCurve distance must be a finite value");
}
//-- set buffer params, leaving cap style as the default CAP_ROUND
bufferParams.setQuadrantSegments( bp.getQuadrantSegments());

/**
* Prevent using a very small QuadSegs value, to avoid
* offset curve artifacts near the end caps.
*/
int quadSegs = bp.getQuadrantSegments();
if (quadSegs < MIN_QUADRANT_SEGMENTS) {
quadSegs = MIN_QUADRANT_SEGMENTS;
}
bufferParams.setQuadrantSegments(quadSegs);

bufferParams.setJoinStyle( bp.getJoinStyle());
bufferParams.setMitreLimit( bp.getMitreLimit());
};
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/capi/GEOSOffsetCurveTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ void object::test<3>()
{
checkOffset(
"LINESTRING(0 0, 10 0, 10 10)",
"LINESTRING (12 10, 12 0, 10 -2, 0 -2)",
"LINESTRING (0 -2, 10 -2, 10.3901806 -1.9615705, 10.76536686 -1.8477590, 11.11114046 -1.66293922, 11.41421356 -1.41421356, 11.66293922 -1.11114046, 11.84775906 -0.76536686, 11.96157056 -0.3901806, 12 0, 12 10)",
-2, 1, GEOSBUF_JOIN_ROUND, 2,
0.000001
Copy link
Member

Choose a reason for hiding this comment

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

Note that the last argument is a tolerance, so there's no need for more than 6 decimal digits with the current tolerance.

);
Expand Down Expand Up @@ -132,7 +132,7 @@ void object::test<5>()
"LINESTRING(33282908 6005055,33282900 6005050,33282892 6005042,33282876 6005007,33282863 6004982,33282866 6004971,33282876 6004975,33282967 6005018,33282999 6005031)",
// Old algorithm
// "LINESTRING EMPTY",
"LINESTRING (33282939.63869393 6005053.735950421, 33282948.754269257 6005058.043310191, 33282949.873293087 6005058.534544423, 33282982.439409934 6005071.764529393)",
"LINESTRING (33282951.601378817 6005059.236579252, 33282982.439409934 6005071.764529393)",
44, 1, GEOSBUF_JOIN_MITRE, 1,
0.000001
);
Expand Down
29 changes: 28 additions & 1 deletion tests/unit/operation/buffer/OffsetCurveTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ void object::test<38> ()
checkOffsetCurve(
"LINESTRING (20 20, 50 50, 80 20)",
10, 2, -1, -1,
"LINESTRING (12.93 27.07, 42.93 57.07, 50 60, 57.07 57.07, 87.07 27.07)"
"LINESTRING (12.928932188134524 27.071067811865476, 42.928932188134524 57.071067811865476, 44.44429766980398 58.314696123025456, 46.1731656763491 59.23879532511287, 48.04909677983872 59.80785280403231, 50 60, 51.95090322016128 59.80785280403231, 53.8268343236509 59.23879532511287, 55.55570233019602 58.314696123025456, 57.071067811865476 57.071067811865476, 87.07106781186548 27.071067811865476)"
);
}

Expand Down Expand Up @@ -554,5 +554,32 @@ void object::test<40> ()
}


// testMinQuadrantSegments
// See https://github.com/qgis/QGIS/issues/53165
template<>
template<>
void object::test<41> ()
{
checkOffsetCurve(
"LINESTRING (553772.0645892698 177770.05079236583, 553780.9235869241 177768.99614978794, 553781.8325485934 177768.41771963477)",
-11, 0, BufferParameters::JOIN_MITRE, -1,
"LINESTRING (553770.76 177759.13, 553777.54 177758.32)"
);
}

// testMinQuadrantSegments_QGIS
// See https://github.com/qgis/QGIS/issues/53165#issuecomment-1563214857
template<>
template<>
void object::test<42> ()
{
checkOffsetCurve(
"LINESTRING (421 622, 446 625, 449 627)",
133, 0, BufferParameters::JOIN_MITRE, -1,
"LINESTRING (405.15 754.05, 416.3 755.39)"
);
}



} // namespace tut
2 changes: 1 addition & 1 deletion web/content/specifications/geojson.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ Unlike [WKB]({{< ref "wkb" >}}) and [WKT]({{< ref "wkt" >}}), GeoJSON does not r
* [Feature](https://datatracker.ietf.org/doc/html/rfc7946#section-3.2), representation of an object that has a "geometry" and an arbitrary set of other non-geometric "properties".
* [FeatureCollection](https://datatracker.ietf.org/doc/html/rfc7946#section-3.3), representation of a list of Features.

Since GEOS is almost 100% concerned with geometry operations, there is no GEOS abstraction to translate the non-geometric properties of GeoJSON Feature into, so handling of Feature properties is only available via the C++ [GeoJSON.h](/libgeos/geos/blob/main/include/geos/io/GeoJSON.h) utilities.
Since GEOS is almost 100% concerned with geometry operations, there is no GEOS abstraction to translate the non-geometric properties of GeoJSON Feature into, so handling of Feature properties is only available via the C++ [GeoJSON.h](https://github.com/libgeos/geos/blob/main/include/geos/io/GeoJSON.h) utilities.


### Writing GeoJSON
Expand Down