search

yeesian / ArchGDAL.jl

A high level API for GDAL - Geospatial Data Abstraction Library
https://yeesian.github.io/ArchGDAL.jl/stable/
Other
137 stars 25 forks source link

[WIP] Check if a geometry is a linear ring before assigning trait #421

Open asinghvi17 opened 2 months ago

asinghvi17 commented 2 months ago

Fix #420 ;)

Needs tests.

rafaqz commented 2 months ago

Runtime 😳

asinghvi17 commented 2 months ago

There is a wkbLinearRing type, but for some reason

julia> typeof(GI.getgeom(water1, 1))
ArchGDAL.IGeometry{ArchGDAL.wkbLineString}

returns a linestring, even though it's a polygon read through ArchGDAL. Maybe that's a bug in the retrieval code?

rafaqz commented 2 months ago

I left some comments about this here:

https://github.com/yeesian/ArchGDAL.jl/blob/master/src/ogr/geometry.jl#L136-L146

Basically GDAL is weird with LinearRing. If you can work out a better way to handle that weirdness go for it.

But imagine what runtime traits will do to compile time and type stabiltity in e.g. GeometryOps apply ;)

(GDAL traits used to be all type unstable and fixing the trait in the type made a lot of things like 20x faster)

evetion commented 4 weeks ago

Yeah I don't think this is fixable. GDAL doesn't treat LinearRing as a stand-alone thing (see https://gdal.org/api/ogrgeometry_cpp.html#_CPPv413OGRLinearRing), and hence there are no LinearRing traits for 2.5D etc (even if you would intercept it in getgeom(polygon)). A way out would be adding isring as a type information, but that's a rather large change. Another way out is adding new LinearRing25D types ourselves, diverging/patching from GDAL calls itself, which is also rather ugly. I fear there's no easy way out.