Skip to content

OverlayNG: Merge linear outputs#1459

Draft
dbaston wants to merge 4 commits into
libgeos:mainfrom
dbaston:linebuilder-merge
Draft

OverlayNG: Merge linear outputs#1459
dbaston wants to merge 4 commits into
libgeos:mainfrom
dbaston:linebuilder-merge

Conversation

@dbaston

@dbaston dbaston commented Jun 25, 2026

Copy link
Copy Markdown
Member

@dbaston

dbaston commented Jun 26, 2026

Copy link
Copy Markdown
Member Author

I've updated the XML tests to expect merged output, with one exception. test_split.xml intends to show how a side effect of Geometry::difference allows splitting a linear geometry with a second linear geometry. Since this PR removes that side effect, I removed the test file.

@dbaston dbaston requested a review from pramsey June 26, 2026 02:14
@dbaston

dbaston commented Jun 26, 2026

Copy link
Copy Markdown
Member Author

It appears PostGIS is relying on the splitting side-effect of GEOSDifference:

https://github.com/postgis/postgis/blob/423a5fd79c2e4d2a1fd88911ffbfb9f906d4a6a8/liblwgeom/lwgeom_geos_split.c#L115

So PostGIS would need to be updated to use GEOSSplit in this case instead.

template<>
void object::test<12> ()
{
// TODO: Should Z values be averaged?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

They are in overlay, so they should be in this operation too, I think is consistent. Probably better than unpredictable ordering based first-in / last-in selection.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Agree that averaging is desirable (if the cost is not too great), but after giving the implementation some thought I think it's out-of-scope for this changeset. It's the job of the Noder to average Z values but the noders used in CoverageUnion deliberately avoid the intersection calculations that would produce averages. You can see the same winner-take-all behavior in the polygon CoverageUnion, which is unaffected by this PR:

geosop -a "GEOMETRYCOLLECTION (POLYGON Z ((0 0 0, 1 0 1, 0 1 2, 0 0 0)), POLYGON Z ((1 0 100, 1 1 200, 0 1 300, 1 0 100)))" coverageUnionNG
POLYGON Z ((1 0 1, 0 0 0, 0 1 2, 1 1 200, 1 0 100))

Comment thread tests/xmltester/tests/misc/split.xml

@pramsey pramsey left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we get a NEWS entry for this feature that notes changes in behaviour for downstream projects (what previously returned a MULTILINESTRING may now return a LINESTRING)?

@dbaston dbaston force-pushed the linebuilder-merge branch from 22131a4 to 77eeb1a Compare June 27, 2026 01:34
@dbaston

dbaston commented Jun 29, 2026

Copy link
Copy Markdown
Member Author

It looks like shapely also relies on the current behavior of GEOSDifference to split a line:

https://github.com/shapely/shapely/blob/e10c6da077744005290f77eaa725777d1c631f66/shapely/ops.py#L362

@pramsey

pramsey commented Jun 30, 2026

Copy link
Copy Markdown
Member

I have pushed a change to PostGIS to use GEOSSplit over GEOSDifference in the ST_Split function, from GEOS 3.15 and on.

@dbaston dbaston force-pushed the linebuilder-merge branch from 2e5cddd to f0a718a Compare July 2, 2026 01:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants