r-spatial / s2

Spherical Geometry Operators Using the S2 Geometry Library
https://r-spatial.github.io/s2/
71 stars 9 forks source link

s2_rebuild transforms `POLYGON FULL` into an empty geometrycollection #262

Open edzer opened 1 month ago

edzer commented 1 month ago

I'm (finally) implementing support for POLYGON FULL in sf, and ran into s2_rebuild converting an s2 full polygon to a GEOMETRYCOLLECTION EMPTY. I don't think it should! I'll work around it in sf, but it would be nice if it wasn't touched in s2_rebuild: In a debug session in sf::st_make_valid():

Browse[1]> s2
<geodesic s2_geography[2] with CRS=OGC:CRS84>
[1] POLYGON ((0 -90, 0 -90))             POLYGON ((0 0, 1 0, 1 1, 0 1, 0 0...
Browse[1]> 
debug: s2 = s2::s2_rebuild(s2, s2_options)
Browse[1]> 
debug: st_as_sfc(s2, crs = crs)
Browse[1]> s2
<geodesic s2_geography[2] with CRS=OGC:CRS84>
[1] GEOMETRYCOLLECTION EMPTY             POLYGON ((0 0, 1 0, 1 1, 0 1, 0 0...
paleolimbot commented 1 month ago

That's a great point...the S2Builder doesn't know about full polygons (I seem to remember) and there are some special gymnastics to ensure that it is handled properly in boolean operations. A single full polygon can probably be special-cased...I am not sure what would happen if one tried to rebuild a polygon that contained a FULL loop, though.

paleolimbot commented 1 month ago

I don't think we have an s2_is_full() to special-case this at the R level but that would basically be the workaround.

edzer commented 1 month ago

s2_is_full() would be great to have!