Skip to content

Conversation

@Jesperpaulsen
Copy link
Contributor

Changes GEOS_PREC_NO_TOPO to GEOS_PREC_VALID_OUTPUT in get_gridded() to ensure topological validity during grid snapping operations.

The current ST_AsMVTGeom sometimes produces invalid geometries because the topology is not preserved.

Screenshot 2025-10-23 at 09 32 31 Screenshot 2025-10-23 at 09 32 28

Changes GEOS_PREC_NO_TOPO to GEOS_PREC_VALID_OUTPUT in get_gridded() to ensure topological validity during grid snapping operations.
@Maxxen
Copy link
Member

Maxxen commented Oct 24, 2025

Hey! Thanks for opening this PR. Don't worry about the CI failure, I'll take care of it.

I didn't use GEOS_PREC_VALID_OUTPUT because AFAIK it might cause GEOS to collapse invalid geometries into lower dimensions (e.g. polygons to lines, and lines to points) and I wasnt sure how well visualization libraries would handle MVT tiles with mixed geometry types in practice. But I think this change might be preferable, lets see if we get any complaints in the future 🤷

@Jesperpaulsen
Copy link
Contributor Author

Hey! Thanks for opening this PR. Don't worry about the CI failure, I'll take care of it.

I didn't use GEOS_PREC_VALID_OUTPUT because AFAIK it might cause GEOS to collapse invalid geometries into lower dimensions (e.g. polygons to lines, and lines to points) and I wasnt sure how well visualization libraries would handle MVT tiles with mixed geometry types in practice. But I think this change might be preferable, lets see if we get any complaints in the future 🤷

Ah, yeah, that makes sense! Haven't worked to much with GEOS, but that's probably true. When it comes to the visualisation of mixed geometry types it shouldn't be any problem with Maplibre and Mapbox at least. It support multiple types in one source and its also possible to filter the layers based on what geometry types you want to show in the layer 😄

@Maxxen Maxxen changed the base branch from main to v1.4-andium October 24, 2025 15:17
@Maxxen
Copy link
Member

Maxxen commented Oct 31, 2025

Thanks for the PR! I've included it as part of this merge #700

@Maxxen Maxxen closed this Oct 31, 2025
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