Skip to content

Make logic better match inspiration implementations#3

Merged
apendleton merged 2 commits intomainfrom
jts-ify-redux
Apr 27, 2026
Merged

Make logic better match inspiration implementations#3
apendleton merged 2 commits intomainfrom
jts-ify-redux

Conversation

@apendleton
Copy link
Copy Markdown
Collaborator

My initial pass at this work was a rough port that didn't quite produce perfect results, and subsequent passes were mostly hamfisted attempts at repairing the output to make it more correct. This PR attempts to much more closely match the upstream (JTS, GEOS, etc.) algorithmic approaches and uncovers some early errors (mostly mistakes on my part due to differences in winding-order conventions between GEOS and geo, the latter of which follows OGC rules) that are now corrected.

The result is that almost all of the post-processing to correct the incorrect topology the early stages produce can be eliminated: the output is correct from the get-go. This eliminates lots of code, and eliminates a bunch of quite-expensive point-in-poly and area calculations, which makes everything dramatically faster. Plus I'm pretty sure it's more correct (hooray).

[[3, 3], [9, 3], [9, 9], [3, 9], [3, 3]]
]
}
},
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This linework should only produce three polygons; this fourth one overlapped existing ones and was not correct.

Comment thread src/tests.rs
polygons
}

fn multipolygon_to_geojson_string(multi: &MultiPolygon<f64>) -> String {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I got rid of the print statement that was the only consumer of this code.

Comment thread src/tests.rs
/// Minimal reproducer for split-touching-hole-drop: a hole that touches
/// the shell boundary must be cleanly split.
///
/// Depends on passes: 3 (infer_contained), 7 (carve_contained, second).
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

These correction passes are all gone, so the comments aren't needed.

Comment thread src/tests.rs
assert_eq!(
actual_canonical, expected_canonical,
"fixture mismatch: split_touching_two_points"
);
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

There was a bunch of code handling the processing of improperly noded lines, but we actually expect input linework to be noded, same as other implementers, and the existence of this improperly noded fixture was keeping us from realizing the simplification that allows. So: consistent with the contract, now we nodify it ourselves, and then we can get rid of the fallback code.

Copy link
Copy Markdown
Contributor

@bvacherot-sofar bvacherot-sofar left a comment

Choose a reason for hiding this comment

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

👍

@apendleton apendleton merged commit 777f0c1 into main Apr 27, 2026
4 checks passed
@apendleton apendleton deleted the jts-ify-redux branch April 27, 2026 17:15
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